Repository navigation
feat(disruption): executor core — builders + orchestration + DDB deps (ADR-031) [#1419] - #1640
Closed
susumutomita wants to merge 4 commits into
Closed
susumutomita wants to merge 4 commits into
susumutomita wants to merge 4 commits into
Conversation
…ADR-031) [#1419] The unit-testable heart of the ADR-031 executor (migration step 2→3): given a fired disruption's `action` + already-folded `parameters` + the team's `stackOutputs`, produce the SDK-agnostic inject + revert descriptors. Pure — no AWS calls (AssumeRole / SendCommand / Invoke / UpdateStack are the handler's job, deferred to the reviewed wiring step). Builds on the action contract from the parser/validator PR. - dispatch-command.ts: buildDisruptionDispatch + buildRevertDispatch → DisruptionDispatch { kind, target, documentName?, params }. - targetRef / functionRef resolved ONLY from stackOutputs keys (no arbitrary resource ids reach the competitor account; loud throw if unresolved). - {{key}} substituted only from fired parameters; a value-less placeholder throws (never sends a literal {{key}} cross-account) = runtime twin of the validator's declaration-time allow-list. - revert reuses the inject kind/target, overridden by action.revert documentName / paramTemplate (ADR-029 INV-2 recovery). afterSeconds is left to the handler/scheduler. - 11 unit tests: all 3 kinds, functionRef fallback, nested substitution, non-string leaves, both throw paths, revert variants. This PR is stacked on the parser/validator PR (#1639, for the DisruptionAction type). The live handler (AssumeRole + send + schedule revert + EXEC# idempotency) + the CDK construct (Lambda + EventBridge rule + IAM) are the next step — kept separate because deploying fault injection into competitor accounts is the owner's review/deploy decision. Relates #1419 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## feat/1419-action-schema-validate #1640 +/- ##
====================================================================
+ Coverage 93.32% 93.36% +0.03%
====================================================================
Files 391 394 +3
Lines 10763 10821 +58
Branches 3303 3324 +21
====================================================================
+ Hits 10045 10103 +58
Misses 222 222
Partials 496 496 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…1419] Adds the executor's decision logic on top of the pure dispatch builders: given a *DisruptionFired detail it resolves the action from the catalog, claims per-team idempotency (EXEC#{requestId}#{teamId}), resolves the team's deployment, injects the built dispatch, and schedules the revert (ADR-029 INV-2). Every I/O boundary (catalog / idempotency claim / deployment resolve / AssumeRole + send / revert schedule) is injected as a dep, mirroring describe-stack-handler's DI pattern, so the orchestration is a pure decision function unit-tested with mocks (6 cases: ok ordering, no_action passthrough, unknown_disruption, duplicate short-circuit, no_deployment, send-fails-so-no-revert). The concrete dep impls (SDK command mapping, aws-scheduler revert, the GSI deployment query) + the Lambda/EventBridge/IAM CDK construct deploy real fault injection into competitor accounts, so they remain the owner's review/deploy step. Relates #1419 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
susumutomita
changed the title
…ent [#1419] The DDB-side concrete deps for the executor orchestration, following existing patterns with no new SDK dependency: - claimExecution: EXEC#{requestId}#{teamId} conditional Put (per-team idempotency vs EventBridge at-least-once), mirroring disruption-fire's REQUEST# claim. - resolveDeployment: GSI1(TENANT#) + eventId/teamId/problemId filter, returns the COMPLETE deployment's cross-account info + parsed stackOutputs, mirroring leaderboard-score-events' query. Skips non-COMPLETE / cross-account-incomplete rows. 9 unit tests (mocked ddb): claimed / duplicate(CCF) / error-propagate / custom TTL; GSI query shape / non-COMPLETE skip / missing-field skip / empty stackOutputs / empty result. Still owner-gated (deploy of real fault injection): the SDK-send dep, the aws-scheduler revert dep, and the Lambda/EventBridge/IAM CDK construct. Relates #1419 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
susumutomita
changed the title
This was referenced Jun 2, 2026
…mposes [#1419] Integration fix surfaced while wiring the handler entry: ExecutorDeps.scheduleRevert omitted `detail`, but the concrete scheduleRevert (#1642) needs it to build the idempotent schedule name (EXEC# twin = requestId/teamId) and the revert invocation payload. The dep is built once at module load, so detail must arrive at call time — executeDisruptionAction now passes it: scheduleRevert(detail, revert, target, afterSeconds). Without this the inject-time orchestration and the revert scheduler did not type-compose in the real wiring. execute.test.ts updated to assert detail is forwarded. Relates #1419 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jun 2, 2026
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
The complete non-deploy core of the ADR-031 cross-account disruption executor (#1419). Three pure/testable modules under
disruption-executor-handler/— no deploy, no new SDK dependency. The only remaining pieces all deploy real fault injection into competitor accounts, so they stay your review/deploy decision.Modules
dispatch-command.ts— pure builders.buildDisruptionDispatch/buildRevertDispatch→{ kind, target, documentName?, params }.targetRef/functionRefresolved only fromstackOutputskeys (loud throw if unresolved);{{key}}substituted only from firedparameters(value-less placeholder throws — runtime twin of feat(disruption): action contract parser + declaration-time enforcement [#1419] #1639's allow-list); revert reuses inject kind/target withaction.revertoverrides (ADR-029 INV-2).execute.ts— orchestration.executeDisruptionAction(detail, deps): resolve action (unknown_disruption/no_actionpassthrough) → claimEXEC#{requestId}#{teamId}(duplicateshort-circuit) → resolve deployment (no_deploymentno-op) → inject → schedule revert. Every I/O boundary injected (describe-stack-handler DI), so it's a pure decision function.executor-store.ts— the DDB deps (no new SDK dep):claimExecution(conditional Put, mirrors disruption-fire's REQUEST# claim) +resolveDeployment(GSI1TENANT#+ event/team/problem filter, COMPLETE rows only, parsed stackOutputs — mirrors leaderboard-score-events).Owner-gated remainder (deploys real fault injection)
sendDispatch—DisruptionDispatch→ SSM SendCommand / Lambda Invoke / CFn UpdateStack via assumed creds (needs@aws-sdk/client-lambda).scheduleRevert—aws-schedulerone-shot (new IAM).sts:AssumeRoleCDK construct, + reference-problem E2E.Building those is fine; deploying them (AdministratorAccess fault injection into third-party accounts) is your call. Precedent for shipping core ahead of wiring: #1606.
Test plan
tsc --noEmit, biome,make harness(no findings),check-no-conflicts: all clean.Regression analysis
Net-new, zero production call sites — three new modules in a new dir; nothing imports them at runtime yet (the entry + CDK construct are the owner-reviewed deploy step). No Lambda/IAM/event/existing-file change. Type-only dep on #1639; rebases onto
maincleanly after it merges. Throw paths are fail-loud by design and covered.Physical impact
NO-OP on CloudFormation / deployed artifacts. Pure TypeScript + tests; no construct, Lambda, IAM, DynamoDB, or EventBridge change. Nothing deployed or invocable until the handler entry + construct land.
Relates #1419