Skip to content

fix: replace shared IBKR precheck dispatcher#356

Merged
Pigbibi merged 6 commits into
mainfrom
codex/ibkr-direct-precheck-schedulers
Jul 22, 2026
Merged

fix: replace shared IBKR precheck dispatcher#356
Pigbibi merged 6 commits into
mainfrom
codex/ibkr-direct-precheck-schedulers

Conversation

@Pigbibi

@Pigbibi Pigbibi commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • replace the shared Cloud Run /monitor-dispatch precheck fan-out with authenticated per-service Cloud Scheduler /dry-run jobs
  • bound each precheck to 120 seconds with no retries and pause jobs for disabled runtime targets
  • remove the shared dispatcher environment payload and reconcile its Scheduler job as legacy while preserving canonical per-service prechecks

Root cause

All target services use internal Cloud Run ingress. The shared dispatcher ran inside one Cloud Run service and called other services through their run.app URLs, so remote requests were rejected before reaching their revisions. The dispatcher then returned 502. The shared job also competed with the host service at scheduled execution times, producing historical 429 capacity failures.

Cloud Scheduler already reaches the internal services directly with OIDC for normal execution and warmups, so per-service Scheduler jobs remove both failure modes without changing strategy or order execution logic.

Validation

  • uv run --no-sync ruff check --exclude external .
  • uv lock --check
  • uv run --no-sync python scripts/validate_cloud_run_startup.py
  • actionlint .github/workflows/sync-cloud-run-env.yml
  • relevant pytest suites: 36 passed
  • runtime config contract tests: 3 passed
  • git diff --check

No production dependency changes. No /run or /dry-run endpoint was invoked during investigation.

Co-Authored-By: Codex <noreply@openai.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cb3c7bb2ff

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .github/workflows/sync-cloud-run-env.yml Outdated
@github-actions

github-actions Bot commented Jul 22, 2026

Copy link
Copy Markdown

🤖 Codex PR Review

Merge allowed: 1 finding(s) reported but none are blocking

ℹ️ Other Findings

1. 🟡 [MEDIUM] Reliability in .github/workflows/sync-cloud-run-env.yml

This new guard makes every WORKFLOW_TARGET=hk-verify sync fail while interactive-brokers-monitor-dispatcher-scheduler still exists, but the same workflow later deletes that shared scheduler globally via reconcile_cloud_runtime.py --delete-legacy-schedulers. Because HK verify is treated as an isolated target here, its canonical ${cloud_run_service%-service}-precheck-scheduler cannot be created before the shared dispatcher is retired, so HK verify loses all scheduled /dry-run prechecks between the configured migration run and a later manual HK verify deployment. (line 425)

Suggestion: Do not retire interactive-brokers-monitor-dispatcher-scheduler until the HK verify replacement job has been created, or allow the HK verify path to create/resume its canonical precheck before enforcing this guard.


Review by Codex PR Review bot • PR

Co-Authored-By: Codex <noreply@openai.com>
@Pigbibi

Pigbibi commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator Author

Follow-up in eddda76:

  • pause the legacy shared dispatcher before removing its target environment, then delete it only during configured multi-target reconciliation
  • keep isolated hk-verify deployments from retiring that global job
  • make the one-shot retry contract explicit with maxRetryAttempts=0 and maxRetryDuration=0

For the cron observation, no change was made: the previous QPK dispatcher only considers five-field cron expressions, and the configured production plans already emit explicit five-field precheck schedules. Two-field workflow overrides continue to inherit the main job's calendar fields, matching the existing warmup behavior.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: eddda76165

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .github/workflows/sync-cloud-run-env.yml Outdated
Pigbibi and others added 2 commits July 22, 2026 08:15
Co-Authored-By: Codex <noreply@openai.com>
Co-Authored-By: Codex <noreply@openai.com>
@Pigbibi

Pigbibi commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator Author

Follow-up in 03f6e5b:

  • exclude the isolated hk-verify workflow from the shared-dispatcher migration entirely
  • configured multi-target sync remains the only path that creates replacement precheck jobs, retires the dispatcher, and removes its legacy environment
  • preserve normal HK main/warmup scheduler synchronization without creating duplicate prechecks or partially modifying global scheduler state

Fresh local verification: ruff, lock check, startup validator, actionlint, diff check, 37 targeted tests, and 3 runtime-plan contract tests passed.

Co-Authored-By: Codex <noreply@openai.com>
@Pigbibi

Pigbibi commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator Author

Follow-up in a2ddeba addresses the latest non-blocking finding: every configured target now creates or updates its canonical precheck before enablement state is applied. Disabled targets are then paused before legacy cleanup, so migration never leaves them without a canonical scheduler.

Regression test was added and confirmed RED before the fix. Fresh verification: ruff, lock check, startup validator, actionlint, diff check, 38 targeted tests, and 3 runtime-plan contract tests passed.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a2ddeba751

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .github/workflows/sync-cloud-run-env.yml Outdated
Co-Authored-By: Codex <noreply@openai.com>
@Pigbibi

Pigbibi commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator Author

Live read-only scheduler verification for the remaining non-blocking observation:

  • the shared dispatcher currently has 5 targets and does not include hk-verify
  • hk-verify currently has only its main /run scheduler; it has no existing /dry-run precheck to lose
  • therefore the configured migration does not create an HK precheck gap; the subsequent isolated HK deployment adds its first authenticated direct /dry-run scheduler

The pre-deploy guard remains intentional fail-closed protection against a future partial migration if a shared dispatcher is present. No runtime endpoint was invoked during verification.

@Pigbibi
Pigbibi merged commit 041a31a into main Jul 22, 2026
5 checks passed
@Pigbibi
Pigbibi deleted the codex/ibkr-direct-precheck-schedulers branch July 22, 2026 00:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant