fix: harden IBKR strategy switch contract - #199
Conversation
Co-Authored-By: Codex <noreply@openai.com>
Co-Authored-By: Codex <noreply@openai.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e6df24c860
ℹ️ 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".
| gh variable list \ | ||
| --repo "${TARGET_REPOSITORY}" \ | ||
| --json name,value \ | ||
| > "${repository_variables_file}" |
There was a problem hiding this comment.
When an IBKR switch uses the supported variable_scope=environment path, the earlier inventory step reads CLOUD_RUN_SERVICE_TARGETS_JSON from that GitHub Environment, but this step fetches only repository variables. _candidate_environment therefore runs the planner without any unchanged environment-scoped settings, so it can reject a valid deployment or approve a state different from the one the sync workflow will receive. Fetch the selected environment's variables as well and merge them over repository variables before applying the candidate assignments.
Useful? React with 👍 / 👎.
| snapshot_path = snapshot_path.strip() if isinstance(snapshot_path, str) else "" | ||
| manifest_path = manifest_path.strip() if isinstance(manifest_path, str) else "" | ||
| if explicit and not feature_snapshot and (snapshot_path or manifest_path): | ||
| raise ValueError( | ||
| f"strategy {strategy_profile!r} does not accept feature snapshot artifacts" | ||
| ) | ||
| if bool(snapshot_path) != bool(manifest_path): |
There was a problem hiding this comment.
For a snapshot strategy, explicit values such as /tmp/snapshot.csv and /tmp/snapshot.csv.manifest.json pass this path because only presence is checked. This bypasses the new catalog contract in build_config.validate, which requires snapshot artifacts to be gs:// URIs, and non-IBKR platforms such as LongBridge have no planner preflight to catch the invalid route before it is written. Apply the same URI validation to explicit overrides.
Useful? React with 👍 / 👎.
| - name: Preflight IBKR deployment plan | ||
| if: env.PLATFORM == 'ibkr' && env.SERVICE_TARGETS_MODE != 'off' |
There was a problem hiding this comment.
When service_targets_mode=off is used against an IBKR repository that has no existing CLOUD_RUN_SERVICE_TARGETS_JSON, the earlier bypass guard permits the operation, but this condition skips the deployment planner entirely and apply=true proceeds directly to GitHub variable writes. This leaves the supported single-service/no-inventory path outside the new fail-closed contract and can publish live settings that the IBKR planner would reject; either synthesize a candidate inventory for this path or reject IBKR writes when no planner can run.
Useful? React with 👍 / 👎.
| "allowed_execution_modes": [ | ||
| "live", | ||
| "paper", | ||
| "dry_run" |
There was a problem hiding this comment.
The catalog exposes hk_low_vol_dividend_quality_snapshot as paper-capable, so the console permits that selection, but its generated switch inputs have no snapshot-path controls and therefore omit both artifact variables. Because the same profile is marked feature_snapshot.required=true without catalog paths, build_runtime_switch.py rejects every such console dispatch with requires feature snapshot path and manifest path. Supply a configured paper artifact, add console inputs for the pair, or stop advertising paper mode until the route exists.
Useful? React with 👍 / 👎.
1 participant
Summary
mainpins (required after external repos changed following the PR base)Validation
python3 -m unittest discover -s python/tests(131 passed)python3 python/scripts/build_config.py --checkpython3 python/scripts/runtime_settings.py validateactionlintSafety