fix(sf): keystone ApplyShellRunDefaults — swap JsonMerge args so shellDefaults wins#291
Merged
Merged
Conversation
…lDefaults wins
The prior merge order silently downgraded every shell_run=true execution
to a full-fat real Saturday run instead of --preflight-only mode.
Caught 2026-05-22 evening, immediately after the Saturday-SF trap-escape
fix landed (lib v0.25.0 + 5 cross-repo pin bumps + SF JSON conversion).
The post-fix verification execution `postfix-verify-20260522T224450Z`
passed MorningEnrich cleanly (trap fix worked) and proceeded into
DataPhase1 — where the spot booted and started collecting short
interest, universe returns, fundamentals at FULL POWER. Aborted at
that point to fix this; otherwise the dry-pass would have spent ~$50
LLM + ~3h compute on what was supposed to be a 15-25min preflight
exercise.
**Root cause:**
ApplyShellRunDefaults was `States.JsonMerge(shellDefaults, $, false)`
— "second arg wins" per AWS docs, so `$` wins over shellDefaults.
The original comment justified this as "user input ($) wins for
per-flag overrides." But `$` at ApplyShellRunDefaults entry is NOT raw
user input — InitializeInput has already merged its defaults blob
`{preflight_args: "", research_dry: false, data_phase2_dry: false,
regime_action: "produce"}` into `$`. So "$ wins" meant the NON-DRY
identity defaults won over the dry shellDefaults.
The mechanism produced byte-identical SSM commands to the absent path
(real Saturday) regardless of shell_run=true. Test
`test_user_input_wins_over_shell_defaults` was hard-asserting the
broken order — the bug had test coverage, but the test was wrong
about what it was asserting.
**Why this never fired before:**
The Friday-PM dry-pass EventBridge rule shipped DISABLED (#258), and
the prior trap-escape bug masked any execution past MorningEnrich. The
2026-05-22 Friday-PM dry-pass was the first execution to reach
ApplyShellRunDefaults' actual semantic effect — and immediately
surfaced this gap once the trap fix unblocked MorningEnrich.
**Fix:**
Swap arg order to `States.JsonMerge($, shellDefaults, false)` so
shellDefaults wins over $. The keystone control vars (preflight_args /
research_dry / data_phase2_dry / regime_action) are now forced to
their dry values when shell_run=true.
Skip-flag overrides are UNAFFECTED — shellDefaults contains no skip_*
keys, so `{shell_run: true, skip_backtester: true}` still passes
skip_backtester through $ unchanged to the downstream Choice gate.
The previously-claimed user-override-from-input case for the keystone
control vars (e.g., `{shell_run: true, preflight_args: ""}`) is now
intentionally lost — passing shell_run=true means you want dry mode
for ALL keystone control vars; partial override within the dry-control
set was a footgun, not a feature.
**Tests:**
- `test_user_input_wins_over_shell_defaults` flipped + renamed to
`test_shell_defaults_win_over_current_state` with a long docstring
explaining why the prior order broke and what semantic the new
order enforces.
- Full SF suite 374/374 green.
- Full repo suite 1430/1430 green.
Composes with [[feedback_no_silent_fails]] (the bug silently downgraded
to a less-safe execution mode rather than failing loud) +
[[feedback_test_must_pin_production_behavior_not_just_current_code]]
(the test was asserting code shape, not semantic intent).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the keystone bug surfaced immediately after the 2026-05-22 trap-escape fix landed (PRs #289/#290/lib #57/dashboard #118/predictor #188/backtester #243). The post-fix verification execution
postfix-verify-20260522T224450Zpassed MorningEnrich cleanly (trap fix worked) and proceeded into DataPhase1 — where the spot booted and started collecting short interest / universe returns / fundamentals at full power instead of running--preflight-only. Aborted at that point.Root cause
ApplyShellRunDefaultswas:Per AWS docs: "second arg wins" →
$wins.The original comment justified this as "user input (
$) wins for per-flag overrides." But$at ApplyShellRunDefaults entry is NOT raw user input — InitializeInput has already merged its defaults blob{preflight_args: "", research_dry: false, data_phase2_dry: false, regime_action: "produce"}into$. So "$wins" meant the NON-DRY identity defaults won over the dryshellDefaults. Everyshell_run=trueexecution silently fell back to a full-fat Saturday run.The existing test
test_user_input_wins_over_shell_defaultswas hard-asserting the broken arg order — the bug had test coverage, but the test was wrong about what it was asserting (confused$with raw user input).Why this never fired before
ApplyShellRunDefaults's actual semantic effect — and immediately surfaced this gap once the trap fix unblocked MorningEnrichMemory hint: "shipped disabled" → first execution of the mechanism today caught both bugs in sequence.
Fix
Swap arg order:
States.JsonMerge($, shellDefaults, false)→ shellDefaults wins.Skip flag overrides UNAFFECTED.
shellDefaultscontains noskip_*keys, so{shell_run: true, skip_backtester: true}still passesskip_backtesterthrough$unchanged to the downstream Choice gate. Only the 4 keystone control vars (preflight_args/research_dry/data_phase2_dry/regime_action) are forced to dry whenshell_run=true.The previously-claimed user-override case for keystone control vars (e.g.,
{shell_run: true, preflight_args: ""}) is now intentionally lost — passingshell_run=truemeans you want dry mode for ALL keystone control vars; partial override within the dry-control-var set was a footgun, not a feature.Test changes
test_user_input_wins_over_shell_defaultsflipped + renamed totest_shell_defaults_win_over_current_statewith a long docstring explaining why the prior order broke and what semantic the new order enforces.Test plan
bash infrastructure/deploy_step_function.shto push to AWS{"shell_run": true, ...}inputbash spot_data_weekly.sh --phase1-only --preflight-only(NOT just--phase1-only)🤖 Generated with Claude Code