fix(problem-deploy): inject disruptions on asymmetric (same-account) rows

[susumutomita]

Summary

PR-1711 のフォローアップ。Lite モードの障害注入がまだ end-to-end で動かない残りギャップを塞ぎます。Closes #1710。

PR-1711 は「Lite 行は competitorRoleArn / externalIdParameterName を両方持たない」前提で resolveDeployment を緩めましたが、実際の deployment 行はそうなっていません:

• deploy-handler は competitorRoleArn を行に永続化する(deploy.ts:177)
• externalIdParameterName は deploy event detail にしか載せず、行には書かない(deploy.ts:259-261)

→ 実際の COMPLETE 行は 「role 有・externalId 無」の非対称(Lite の実機 DDB で確認済み。competitorRoleArn=arn:…:role/TenkaCloud-local-deploy-Role、externalIdParameterName 欠落)。

この行に対し PR-1711 後の resolveDeployment は lone competitorRoleArn を target に載せ、assumeCompetitorRole の both-or-neither ガード(assume-competitor-role.ts:135-137)が competitorRoleArn and externalIdParameterName must be provided together で throw します。注入は依然として走りません(no_deployment の silent no-op が throw に変わっただけ)。

修正

resolveDeployment が cross-account fields を assumeCompetitorRole と同じ both-or-neither 契約で扱うようにします:

• 両方 string → cross-account injection(SaaS、従来想定どおり)
• それ以外(両方欠落 or 片方だけ)→ same-account injection(target に role/externalId を載せない → executor 自身の credentials + PR-1711 で付与済みの ssm:SendCommand grant で注入)

片方だけの行はそもそも AssumeRole 不能なので、same-account 扱いが唯一の動作可能な解釈です。

実機検証(Lite, account 672726205532 / ap-northeast-1)

• tc-hello-world-battle-team-1 の COMPLETE 行が非対称であることを DDB scan で確認
• 注入先 EC2 i-046414a66bfb65dd7 は SSM-managed / Online、stack に InstanceId Output あり → executor が SendCommand を発行すれば着地する
• 反映には problem-deploy backend の再デプロイが必要(PR-1711 の IAM grant + 本修正の Lambda code)

Follow-up(本 PR スコープ外)

SaaS (cross-account) の disruption は externalIdParameterName が行に永続化されていないため従来から一度も動作していません(旧実装でも常に no_deployment)。真の cross-account 注入には deploy-handler での externalId 永続化 + executor role への sts:AssumeRole 付与(IAM)が必要で、別 issue として扱うべきです。本 PR は SaaS の挙動を変えません(後述)。

Test plan

• disruption-executor-store.test.ts に非対称行(role 有・externalId 無)→ same-account target を pin するテストを追加。修正前 FAIL(lone competitorRoleArn が target に混入することを実証)→ 修正後 PASS の両方向確認済み
• 既存の対称ケース(両方有 → cross-account target)・両方欠落ケース(PR-1711 追加)は変更なしで green
• disruption-*.test.ts 全 14 file / 186 tests green、tsc --noEmit clean、make harness no findings、biome clean
• pre-commit hook(audit-deps / cdk synth 検証 / IAM Latin-1)も全て OK

Regression analysis

• SaaS (cross-account): 行に externalId が無い現状、旧実装は no_deployment(no-op)、PR-1711 実装は throw、本修正は same-account injection 試行(対象 instance が control account に無く SendCommand が InvalidInstanceId で fail)。いずれも「注入されない」点は同じで、動いていたものを壊す回帰はなし。将来 externalId が永続化されれば両方 string → cross-account 経路が従来どおり機能する(既存テストで pin 済み)
• Lite (same-account): 非対称行が same-account target になり初めて注入が通る。意図した挙動変更そのもの
• claim / revert / scheduled fire: 変更なし(resolveDeployment の戻り値構築のみの修正)

Physical impact

• Lambda code(disruption-executor handler)= UPDATE(in-place)
• CFn リソースの CREATE / REPLACE / DELETE = なし(IAM・テーブル・イベント配線は不変。handler の TS ロジック + テストのみ)
• 反映には problem-deploy backend の再デプロイが必要(Lite: make deploy)

Summary by CodeRabbit

• Bug Fixes

  • Updated deployment validation to treat cross-account authentication parameters as a coupled pair rather than independently.

• Testing

  • Added test coverage to verify deployment configuration handling with asymmetric parameter combinations.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f12624f9-0027-4d69-a97b-ceb64cc08de7

📥 Commits

Reviewing files that changed from the base of the PR and between d7533e6 and a4fa175.

📒 Files selected for processing (2)

• infrastructure/lib/problem-deploy/handlers/disruption-executor-handler/executor-store.ts
• infrastructure/test/problem-deploy/disruption-executor-store.test.ts

---

📝 Walkthrough

Walkthrough

The PR fixes same-account deployment resolution by coupling competitorRoleArn and externalIdParameterName—both fields are now included in the returned target only when both are present on the resolved row, allowing Lite mode deployments (which lack externalIdParameterName) to be recognized and handled as same-account targets.

Changes

Same-account/cross-account coupling

Layer / File(s)	Summary
Cross-account/same-account coupling logic and validation infrastructure/lib/problem-deploy/handlers/disruption-executor-handler/executor-store.ts, infrastructure/test/problem-deploy/disruption-executor-store.test.ts	resolveDeployment introduces a crossAccount boolean requiring both competitorRoleArn and externalIdParameterName to be present; asymmetric rows (role present but externalId absent) are treated as same-account with both fields undefined. A test validates asymmetric COMPLETE rows are resolved as same-account targets while preserving jobId, region, and parsed stackOutputs.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• susumutomita/TenkaCloud#1646: Introduces resolveDeployment for the cross-account executor; this PR refines its competitorRoleArn/externalIdParameterName coupling logic and adds a targeted test for asymmetric same-account deployments.

Poem

🐰 A rabbit hops through paired fields so bright,
Both arms or none—asymmetry takes flight!
Lite deployments now awake and ready,
Same-account paths keep the disruptions steady.
Cross-account dances still pirouette with grace,
Logic coupling brings all modes to the same place.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately reflects the main change: fixing disruption injection for asymmetric (same-account) deployment rows by ensuring resolveDeployment treats competitorRoleArn and externalIdParameterName as a coupled pair.
Linked Issues check	✅ Passed	The PR fully addresses the linked issue #1710 requirements: it modifies resolveDeployment to treat cross-account fields as a both-or-neither pair, handles asymmetric rows as same-account deployments, and adds test coverage for this scenario.
Out of Scope Changes check	✅ Passed	All changes are scoped to the executor-store handler logic and corresponding test cases; no IAM, CloudFormation, or infrastructure modifications are present, aligning with the stated scope constraints.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/disruption-lite-asymmetric-row

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.75%. Comparing base (d7533e6) to head (a4fa175).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1712   +/-   ##
=======================================
  Coverage   92.75%   92.75%           
=======================================
  Files         434      434           
  Lines       11899    11899           
  Branches     3655     3655           
=======================================
  Hits        11037    11037           
  Misses        295      295           
  Partials      567      567           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.