Repository navigation
fix(ci): fire CI hooks per-component in deploy --all mode - #2478
Merged
Andriy Knysh (aknysh) merged 9 commits intoMay 23, 2026
Merged
Conversation
Contributor
|
Tip Atmos Pro
No affected stacks workflow was detected for this pull request. 
|
Contributor
📝 WalkthroughWalkthroughDeploy now resets per-run state, runs CI hooks per-component in multi-component deploys via a new runner, wires that runner into multi-component routing, and skips the global PostRunE/defer-hooks when per-component hooks are used. ChangesDeploy multi-component CI hook execution
Sequence Diagram(s)sequenceDiagram
participant terraformRunWithOptions
participant runCIHooksForDeployComponent
participant CIHandler as h.AfterTerraformDeploy
terraformRunWithOptions->>runCIHooksForDeployComponent: call PerComponentHook(info, componentCtx, output, execErr)
runCIHooksForDeployComponent->>runCIHooksForDeployComponent: resolve ForceCIMode (flag/viper) and strip ANSI
runCIHooksForDeployComponent->>CIHandler: invoke AfterTerraformDeploy(CommandError, ExitCode, options)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
cmd/terraform/utils_hooks_test.go (2)
156-172: ⚡ Quick winUse table-driven subtests for the multi-scenario cases.
Both tests cover success/failure mode branches but are written as linear steps. Converting them to table-driven subtests will make failures more targeted and keep style consistent with test conventions.
As per coding guidelines "
**/*_test.go: Use table-driven tests for testing multiple scenarios in Go."Also applies to: 177-196
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/terraform/utils_hooks_test.go` around lines 156 - 172, Convert the linear multi-scenario TestRunCIHooksForDeployComponent_DemoStacks into table-driven subtests: create a slice of test cases (e.g., name, execErr, expected behavior/input) and iterate with t.Run for each case, calling runCIHooksForDeployComponent with the case values; do the same refactor for the other linear test at lines 177-196 (same pattern). Keep the existing setup (t.Chdir and newHookTestCmd()) outside the loop, and use descriptive case names like "success" and "failure" so failures are reported per subtest.
156-229: ⚡ Quick winInitialize command tests with
cmd.NewTestKit(t).These new command-path tests should use the shared test kit to ensure command state isolation across runs.
As per coding guidelines "
cmd/**/*_test.go: ALWAYS usecmd.NewTestKit(t)for cmd tests to auto-clean RootCmd state (flags, args)."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/terraform/utils_hooks_test.go` around lines 156 - 229, Each test must initialize the shared command test kit to auto-clean RootCmd state; add a call to cmd.NewTestKit(t) at the start of each test (TestRunCIHooksForDeployComponent_DemoStacks, TestDeployPostRunE_SuppressedWhenMultiComponent, TestRunCIHooksForDeployComponent_ExitCodeForwarding) before creating cmd via newHookTestCmd() and before any t.Chdir or command invocations so flags/args are isolated across runs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@cmd/terraform/utils_hooks_test.go`:
- Around line 156-172: Convert the linear multi-scenario
TestRunCIHooksForDeployComponent_DemoStacks into table-driven subtests: create a
slice of test cases (e.g., name, execErr, expected behavior/input) and iterate
with t.Run for each case, calling runCIHooksForDeployComponent with the case
values; do the same refactor for the other linear test at lines 177-196 (same
pattern). Keep the existing setup (t.Chdir and newHookTestCmd()) outside the
loop, and use descriptive case names like "success" and "failure" so failures
are reported per subtest.
- Around line 156-229: Each test must initialize the shared command test kit to
auto-clean RootCmd state; add a call to cmd.NewTestKit(t) at the start of each
test (TestRunCIHooksForDeployComponent_DemoStacks,
TestDeployPostRunE_SuppressedWhenMultiComponent,
TestRunCIHooksForDeployComponent_ExitCodeForwarding) before creating cmd via
newHookTestCmd() and before any t.Chdir or command invocations so flags/args are
isolated across runs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 519f558a-0098-416e-88e4-9a83d087243b
📒 Files selected for processing (3)
cmd/terraform/deploy.gocmd/terraform/utils.gocmd/terraform/utils_hooks_test.go
thejrose1984
force-pushed
the
bugfix/2476/atmos-ci
branch
from
May 22, 2026 03:28
1989cb8 to
e07360d
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
cmd/terraform/utils_hooks_test.go (1)
174-197: ⚡ Quick winAdd one regression test for the RunE error-path suppression guard.
This covers PostRunE suppression well, but the new RunE defer guard (
runErr != nil && !wasMultiComponentExecution) should also be tested to prevent double-fire regressions in multi-component failures.As per coding guidelines, "Minimum 80% test coverage ... all features need tests."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/terraform/utils_hooks_test.go` around lines 174 - 197, Add a regression test for the RunE defer-guard that checks the new suppression logic (the condition using runErr and wasMultiComponentExecution): stub or replace runHooksWithOutput to record whether it was invoked, save and restore the global wasMultiComponentExecution and the original runHooksWithOutput, then set wasMultiComponentExecution = true and invoke deployCmd.RunE (or the actual RunE closure) with arguments that cause RunE to return an error and assert that an error is returned but runHooksWithOutput was NOT called; repeat with wasMultiComponentExecution = false and assert runHooksWithOutput IS called. Ensure you restore globals after the test.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/terraform/utils_hooks_test.go`:
- Around line 156-230: Add cmd.NewTestKit(t) at the start of each test to ensure
RootCmd/flag state is reset: insert a call to cmd.NewTestKit(t) in
TestRunCIHooksForDeployComponent_DemoStacks,
TestDeployPostRunE_SuppressedWhenMultiComponent, and
TestRunCIHooksForDeployComponent_ExitCodeForwarding (before any t.Chdir or
newHookTestCmd() calls) so the command test kit auto-cleans state used by
newHookTestCmd(), deployCmd.PostRunE, and runCIHooksForDeployComponent; keep
existing test setup and assertions unchanged.
---
Nitpick comments:
In `@cmd/terraform/utils_hooks_test.go`:
- Around line 174-197: Add a regression test for the RunE defer-guard that
checks the new suppression logic (the condition using runErr and
wasMultiComponentExecution): stub or replace runHooksWithOutput to record
whether it was invoked, save and restore the global wasMultiComponentExecution
and the original runHooksWithOutput, then set wasMultiComponentExecution = true
and invoke deployCmd.RunE (or the actual RunE closure) with arguments that cause
RunE to return an error and assert that an error is returned but
runHooksWithOutput was NOT called; repeat with wasMultiComponentExecution =
false and assert runHooksWithOutput IS called. Ensure you restore globals after
the test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4498711f-6b3e-419b-ba90-529f9795ca06
📒 Files selected for processing (3)
cmd/terraform/deploy.gocmd/terraform/utils.gocmd/terraform/utils_hooks_test.go
…#2476) Extends the per-component hook pattern from PR cloudposse#2430 (plan --all) and cloudposse#2475 (apply --all) to deploy --all so each component produces its own CI summary entry instead of a single misattributed entry for the last component. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Same fix as PR cloudposse#2477: newHookTestCmd() hardcodes Use:"plan" but the test calls deployCmd.PostRunE, causing ProcessCommandLineArgs to see the wrong subcommand name in the non-suppressed path. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
thejrose1984
force-pushed
the
bugfix/2476/atmos-ci
branch
from
May 22, 2026 11:13
e07360d to
78b15dc
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cmd/terraform/utils_hooks_test.go (1)
160-165: ⚡ Quick winAdd a compile-time sentinel for
schema.ConfigAndStacksInfofield references.These tests depend on named fields; add one sentinel so field renames fail at compile time instead of drifting silently.
Proposed change
import ( "testing" @@ "github.com/cloudposse/atmos/pkg/hooks" "github.com/cloudposse/atmos/pkg/schema" ) + +var _ = schema.ConfigAndStacksInfo{ + Stack: "", + Component: "", + ComponentFromArg: "", + ComponentType: "", +}As per coding guidelines, "Add compile-time sentinels for schema field references in tests: when a test uses a specific struct field ... add
var _ = schema.Provider{Kind: "azure"}as a compile guard so field rename immediately fails the build."Also applies to: 205-210
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/terraform/utils_hooks_test.go` around lines 160 - 165, Add a compile-time sentinel that uses named fields of schema.ConfigAndStacksInfo so any future rename breaks the build; e.g. insert a var _ = schema.ConfigAndStacksInfo{Stack: "", Component: "", ComponentFromArg: "", ComponentType: ""} near the test instances (the same pattern should also be added for the other test block around the second occurrence). This creates a compile-time guard referencing the exact field names used in the tests and will fail if those fields are renamed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@cmd/terraform/utils_hooks_test.go`:
- Around line 160-165: Add a compile-time sentinel that uses named fields of
schema.ConfigAndStacksInfo so any future rename breaks the build; e.g. insert a
var _ = schema.ConfigAndStacksInfo{Stack: "", Component: "", ComponentFromArg:
"", ComponentType: ""} near the test instances (the same pattern should also be
added for the other test block around the second occurrence). This creates a
compile-time guard referencing the exact field names used in the tests and will
fail if those fields are renamed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 02834ae4-3850-4db4-bea0-f93784182e0b
📒 Files selected for processing (3)
cmd/terraform/deploy.gocmd/terraform/utils.gocmd/terraform/utils_hooks_test.go
Convert runHooksOnErrorWithOutput to a package-level var so tests can stub it, and add TestDeployRunE_DeferGuard covering all four branches of the deploy.go RunE defer-guard contract: non-nil error must fire the global hook only when wasMultiComponentExecution is false, and nil error must never fire it. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cmd/terraform/utils_hooks_test.go (1)
241-266: ⚡ Quick winConvert the defer-guard scenarios to a table-driven subtest.
This section duplicates the same setup/assert flow across multiple cases. A table-driven loop will reduce drift and keep the branch matrix easier to extend.
As per coding guidelines, "
**/*_test.go: Use table-driven tests for testing multiple scenarios in Go."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/terraform/utils_hooks_test.go` around lines 241 - 266, Refactor the repeated defer-guard scenarios into a table-driven subtest: create a slice of test cases (fields: name, runErr, wasMultiComponentExecution, expectCalled, expectEvent, expectErr, expectOutput) and loop over them with t.Run(case.name, func(t *testing.T) { ... }); inside each subtest set called = false and set wasMultiComponentExecution = case.wasMultiComponentExecution, call invokeDefer(case.runErr, "captured deploy output"), and assert expectations (called → case.expectCalled; if expectEvent/expectErr/expectOutput are relevant assert calledEvent == case.expectEvent, calledErr == case.expectErr, calledOutput == case.expectOutput). Keep existing symbols invokeDefer, wasMultiComponentExecution, called, calledEvent, calledErr, and calledOutput to locate variables and assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@cmd/terraform/utils_hooks_test.go`:
- Around line 241-266: Refactor the repeated defer-guard scenarios into a
table-driven subtest: create a slice of test cases (fields: name, runErr,
wasMultiComponentExecution, expectCalled, expectEvent, expectErr, expectOutput)
and loop over them with t.Run(case.name, func(t *testing.T) { ... }); inside
each subtest set called = false and set wasMultiComponentExecution =
case.wasMultiComponentExecution, call invokeDefer(case.runErr, "captured deploy
output"), and assert expectations (called → case.expectCalled; if
expectEvent/expectErr/expectOutput are relevant assert calledEvent ==
case.expectEvent, calledErr == case.expectErr, calledOutput ==
case.expectOutput). Keep existing symbols invokeDefer,
wasMultiComponentExecution, called, calledEvent, calledErr, and calledOutput to
locate variables and assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cee22908-aef7-418d-b63e-5004bcc2ce04
📒 Files selected for processing (2)
cmd/terraform/utils.gocmd/terraform/utils_hooks_test.go
Contributor
Author
Status summary for maintainersCode state: CodeRabbit APPROVED at the current HEAD What's blocking merge:
CodeRabbit's only remaining comment at this HEAD is a non-blocking nitpick suggesting the defer-guard scenarios be converted to table-driven subtests. The current style mirrors the existing inline-then-tabular pattern used elsewhere in the file; happy to change if a reviewer prefers. |
The previous merge from main left the branch unbuildable: - cmd/terraform/utils.go was missing a closing brace on the deploy PerComponentHook branch, producing a Go syntax error - cmd/terraform/utils_hooks_test.go had deploy and apply test bodies interleaved with overlapping function declarations and orphaned statements Reconstruct the test file to host both the deploy tests (this PR) and the apply tests (brought in from main via cloudposse#2475), and address open CodeRabbit nitpicks at the same time: - Add a compile-time sentinel for schema.ConfigAndStacksInfo so a field rename surfaces as a build failure - Convert plan/deploy/apply demo-stacks tests and the deploy RunE defer-guard scenarios to table-driven subtests - Document why cmd.NewTestKit(t) cannot be used here (circular import between cmd/terraform and cmd, same workaround the workdir tests use) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Code review follow-ups: - Restore the linear style of plan/apply demo-stacks tests to match main; these tests came in via merge and aren't part of this PR's scope. Only the deploy demo-stacks test and the deploy RunE defer-guard scenarios are refactored to table-driven subtests, per the CodeRabbit nitpicks for this PR. - Update the per-component hook wiring comment in utils.go to read "plan, deploy, and apply" now that all three branches are wired. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
Author
|
CodeRabbit (@coderabbitai) resume |
Contributor
✅ Actions performedReviews resumed. |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cmd/terraform/utils_hooks_test.go (1)
199-219: 💤 Low valueConsider table-driven style for consistency with the deploy test.
The deploy component test above (lines 183-197) uses table-driven subtests. This apply test covers the same success/failure paths but uses inline assertions. Not blocking—tests work correctly.
♻️ Optional: convert to table-driven
func TestRunCIHooksForApplyComponent_DemoStacks(t *testing.T) { t.Chdir("../../examples/demo-stacks") cmd := newHookTestCmd() info := &schema.ConfigAndStacksInfo{ Stack: "dev", Component: "myapp", ComponentFromArg: "myapp", ComponentType: "terraform", } - // Success path: execErr is nil, exit code forwarded as 0. - runCIHooksForApplyComponent(cmd, info, "apply output", nil) - - // Failure path: non-nil execErr is forwarded with its exit code. - runCIHooksForApplyComponent(cmd, info, "", errUtils.ExitCodeError{Code: 1}) + tests := []struct { + name string + output string + execErr error + }{ + {name: "success path", output: "apply output", execErr: nil}, + {name: "failure path forwards exit code", output: "", execErr: errUtils.ExitCodeError{Code: 1}}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + runCIHooksForApplyComponent(cmd, info, tc.output, tc.execErr) + }) + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/terraform/utils_hooks_test.go` around lines 199 - 219, The test TestRunCIHooksForApplyComponent_DemoStacks duplicates logic for success/failure paths inline; refactor it to a table-driven subtest like the deploy test by creating a slice of cases (name, execErr, expectedBehavior) and iterating t.Run for each case, calling runCIHooksForApplyComponent with newHookTestCmd() and the existing schema.ConfigAndStacksInfo, asserting behavior per case; keep the same inputs (apply output vs empty string and errUtils.ExitCodeError{Code:1}) and reuse TestRunCIHooksForApplyComponent_DemoStacks, runCIHooksForApplyComponent, newHookTestCmd and the info struct to locate code.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@cmd/terraform/utils_hooks_test.go`:
- Around line 199-219: The test TestRunCIHooksForApplyComponent_DemoStacks
duplicates logic for success/failure paths inline; refactor it to a table-driven
subtest like the deploy test by creating a slice of cases (name, execErr,
expectedBehavior) and iterating t.Run for each case, calling
runCIHooksForApplyComponent with newHookTestCmd() and the existing
schema.ConfigAndStacksInfo, asserting behavior per case; keep the same inputs
(apply output vs empty string and errUtils.ExitCodeError{Code:1}) and reuse
TestRunCIHooksForApplyComponent_DemoStacks, runCIHooksForApplyComponent,
newHookTestCmd and the info struct to locate code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a882658e-2b4d-4cc1-88c8-3881b8193b04
📒 Files selected for processing (2)
cmd/terraform/utils.gocmd/terraform/utils_hooks_test.go
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (65.38%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #2478 +/- ##
==========================================
+ Coverage 78.21% 78.23% +0.02%
==========================================
Files 1119 1119
Lines 106335 106358 +23
==========================================
+ Hits 83171 83214 +43
+ Misses 18522 18499 -23
- Partials 4642 4645 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Andriy Knysh (aknysh)
approved these changes
May 23, 2026
Andriy Knysh (aknysh)
left a comment
Andriy Knysh (aknysh)
left a comment
Member
There was a problem hiding this comment.
thanks thejrose1984
Andriy Knysh (aknysh)
merged commit May 23, 2026
1490d2d
into
cloudposse:main
56 of 57 checks passed
Contributor
Erik Osterman (Cloud Posse) (osterman)
added a commit
that referenced
this pull request
May 23, 2026
Pre-commit was failing on the merge ref for PR #2482 because three commits recently merged to main (#2348, #2478, #2481) contain files that were not gofumpt-formatted and a couple of native-ci test fixtures with trailing whitespace from captured terraform output. These files are not touched by this PR, but they appear in the PR's GitHub-generated merge ref diff and so pre-commit checks them. Apply the auto-fixes that pre-commit produces: - cmd/terraform/output.go: group two consecutive var decls. - internal/exec/terraform_output_getter_test.go: line-break in assert.PanicsWithValue calls (2x). - pkg/terraform/output/executor_test.go: line-break in NewExecutor call. - pkg/terraform/output/get_test.go: line-break in assert.PanicsWithValue calls (3x). - tests/fixtures/scenarios/native-ci/github-output.txt: trim trailing whitespace. - tests/fixtures/scenarios/native-ci/github-step-summary.txt: trim trailing whitespace. The fixture files are runtime outputs set via GITHUB_OUTPUT and GITHUB_STEP_SUMMARY env vars during the native-ci test scenarios and are only checked via file_contains substring assertions, so trimming trailing whitespace is safe. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
6 of 8 tasks
Andriy Knysh (aknysh)
added a commit
that referenced
this pull request
May 25, 2026
…nd skip controls (#2482) * feat(hooks): kind system + scanner kinds + auto-install + --skip-hooks Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * chore(hooks): test coverage + error-builder polish + helper extraction Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * chore: apply gofumpt and trim trailing whitespace after main merge Pre-commit was failing on the merge ref for PR #2482 because three commits recently merged to main (#2348, #2478, #2481) contain files that were not gofumpt-formatted and a couple of native-ci test fixtures with trailing whitespace from captured terraform output. These files are not touched by this PR, but they appear in the PR's GitHub-generated merge ref diff and so pre-commit checks them. Apply the auto-fixes that pre-commit produces: - cmd/terraform/output.go: group two consecutive var decls. - internal/exec/terraform_output_getter_test.go: line-break in assert.PanicsWithValue calls (2x). - pkg/terraform/output/executor_test.go: line-break in NewExecutor call. - pkg/terraform/output/get_test.go: line-break in assert.PanicsWithValue calls (3x). - tests/fixtures/scenarios/native-ci/github-output.txt: trim trailing whitespace. - tests/fixtures/scenarios/native-ci/github-step-summary.txt: trim trailing whitespace. The fixture files are runtime outputs set via GITHUB_OUTPUT and GITHUB_STEP_SUMMARY env vars during the native-ci test scenarios and are only checked via file_contains substring assertions, so trimming trailing whitespace is safe. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * test(snapshots): regenerate help snapshots for --skip-hooks global flag The new --skip-hooks global flag added in pkg/flags/global_builder.go appears in every command's --help output. Regenerated the 39 affected golden snapshots so TestCLICommands passes on Linux, macOS, and Windows. Also includes: - pkg/hooks/hooks_test.go: save and restore prior viper "skip-hooks" value in TestRunAll_SkipHooksBypassesPreflightBinaryCheck to avoid cross-test viper leakage. - website/src/data/roadmap.js: minor copy edit to custom-hooks milestone. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(ci): update test workflow checkout actions * test(hooks): assert normalized legacy hook kind * test: increase hooks coverage * ci: disable checkout credential persistence * ci: pin workflow actions * docs: clarify custom hook outputs * docs: rewrite tool-deps PRD as requirements; split scanner kinds; document hook env var lifetime and --all Rewrite docs/prd/tool-dependencies-integration.md to state requirements and per-feature implementation status instead of narrating its own history. The PRD now leads with a Requirements & Implementation Status table that flags each requirement as Implemented or Not implemented with a code reference. Implementation Plan phases carry Status annotations. Success Criteria are plain numbered items, not a completion tracker. Split the lumped trivy/checkov/kics section in website/docs/stacks/hooks.mdx into three separate sections, one per kind, each with a definition list documenting that scanner's command, args, output handling, on_failure default, runtime requirements, and kind-specific quirks (Checkov's SSL_CERT_FILE workaround, KICS's KICS_QUERIES_PATH and curated registry override). Expand the ATMOS_OUTPUT_DIR / ATMOS_OUTPUT_FILE env-var entries to spell out the per-hook-invocation contract: fresh os.MkdirTemp("atmos-hook-*") directory created before the subprocess runs, deleted automatically when the hook returns, never shared between sibling hooks. Add a new "Hooks with --all" subsection covering per-component firing in dependency order, per-component tool install, shared --skip-hooks, and on_failure:fail aborting the downstream traversal. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * docs(hooks): link examples to website routes; document override semantics; drop stream paragraph from changelog Switch the Examples list in website/docs/stacks/hooks.mdx from raw GitHub URLs to the website's /examples/<name> routes (the file-browser plugin already publishes them). Add an "Overriding Kind Defaults" section to hooks.mdx documenting that any field set on a hook (command, args, env, on_failure) overrides the named kind's default for that field. Includes a caution callout that args and env are full replacement — not merge — so overriding args requires restating the full default arg list. Three examples cover the common cases: bumping a scanner's severity threshold via args, making a finding block the run via on_failure, and injecting a tool config path via env. Code path: pkg/hooks/kind.go::ResolveDefaults. Drop the "Tool output streams naturally" subsection from the custom hooks changelog entry per review feedback. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * docs(hooks): signal overridable defaults up front in Built-in Kinds preamble The per-kind sections list "(default)" values without indicating they're customizable. Add a one-paragraph preamble under "Built-in Kinds" that tells readers every default is overridable and points to the override rules section before they read any individual kind. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * docs(blog): link custom-hooks examples to /examples routes; add docs ActionCard Swap the four GitHub example URLs in the custom-hooks changelog post to the website's /examples/<name> routes (already published by the file-browser plugin), matching the change in website/docs/stacks/hooks.mdx. Append an ActionCard at the bottom pointing readers from the changelog narrative to /stacks/hooks for the full reference (kinds, override semantics, lifecycle events, tool auto-install, --all / --skip-hooks). The remaining github.com URL in the "Try it" section stays — git clone needs the GitHub URL. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * docs(blog): add override-defaults chapter; wire ActionCard CTAs correctly Add an "Override any default" h3 between the built-in-kinds and bring-your-own-command sections, matching the chapter style of "Workdir compatible". The new chapter shows that command, args, env, and on_failure are all overridable on built-in kinds, with a worked example (Trivy with HIGH,CRITICAL severity and on_failure: fail) and a call-out that args/env are full replacement, not merge — linking to the override docs at /stacks/hooks#overriding-kind-defaults. Fix the closing ActionCard so the CTA buttons actually render. The component reads ctaText/ctaLink and secondaryCtaText/secondaryCtaLink, not href. The card now renders a primary "Read the docs" CTA to /stacks/hooks and a secondary "Browse examples" CTA to /examples/hooks-trivy. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> Co-authored-by: Andriy Knysh <aknysh@users.noreply.github.com>
|
These changes were released in v1.220.0-rc.2. |
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
2 participants
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.
what
atmos terraform deploy --all(and--query,--components, stack-without-component) producing only a single CI summary entry for the last component instead of one entry per componentrunCIHooksForDeployComponentas the per-component hook for thedeploysubcommand so$GITHUB_STEP_SUMMARYreceives one entry per component with the correct component/stack contextwasMultiComponentExecutionreset, error-defer guard, andPostRunEguard indeploy.go— the same three-site pattern applied toplanin fix(ci): fire CI hooks per-component in --all / --query plan mode (#2… #2430 andapplyin Bug: CI hooks fire only once in --all mode, producing summary for last component only #2475why
In multi-component mode,
terraformRunWithOptionsroutes toExecuteTerraformQueryand setswasMultiComponentExecution = true, butdeploy.gohad no guard on itsPostRunEor error-path defer. This caused:PostRunEto fire once after all components completed, callingRunCIHookswith an empty output buffer and the last component'sinfo.Component/info.Stack--allfailed mid-walk (per-component hook already ran for the failed component)references
Closes #2476
Related: #2397 (plan fix), #2475 (apply fix)
Summary by CodeRabbit
Bug Fixes
Tests