fix: treat seal Ctrl-C as quiet interrupt, not false Unsealed (Bugbot #397)#408
Merged
Merged
Conversation
…397) The seal check's cancel detection looked only at ctx.Err(). On a terminal Ctrl-C the helm child can exit 130 before NotifyContext flips the context, so the check read as a real failure and rendered a false Unsealed (exit 2) instead of a quiet interrupt (130) — breaking seal's "never claim a verdict you didn't verify" rule. Reuse installerRunInterrupted (shared with upgrade/prepare-host) at both seal cancel sites (mid-suite and enumeration) so the exit-130 race is caught. Adds a regression test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
saadqbal
approved these changes
Jul 24, 2026
Contributor
Functional review \u2014 passed\n\nBasis: the behavioural suites that ran on this PR at merge against real environments, not mocks:\n\n- \u2014 the installer suite exercised for real, plus across all packages\n\nI could not reach the dev API (no dev credentials), so rather than rubber-stamp I used the strongest evidence available: these suites exercise the actual behaviour this change alters, on real infrastructure. Advancing \u2192 .\n\nIf the functional reviewer wants a manual pass on dev in addition, please move it back and say so. |
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
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.
Resolves a Cursor Bugbot finding on the cli promotion PR (#397, develop → main).
Seal Ctrl-C can fake Unsealed (Medium) — the seal check's mid-suite cancel detection looked only at
ctx.Err(). On a terminal Ctrl-C the helm child can exit 130 (128+SIGINT) beforeNotifyContextflips the context — the same raceinstallerRunInterruptedalready covers forupgrade/prepare-host. In that window the failed helm call was read as a real check failure and rendered a falseUnsealed(exit 2) instead of a quiet interrupt (exit 130), breaking the seal check's "never claim a verdict you didn't verify" rule.Fix: reuse the existing
installerRunInterrupted(ctx, err)helper (generalized in #402, merged into develop) at the seal cancel sites — no duplicated logic. It checksctx.Err()first (preserving existing behavior) then the child's exit-130, so a Ctrl-C is treated as a quiet interrupt and no verdict is printed. Applied at both cancel sites ininternal/cli/seal.go: the cited mid-suite per-check loop and the sibling hook-enumeration path, which has the identical race.helm.Runnerreturnsexec.CommandContext(...).CombinedOutput()'s error unwrapped, soerrors.Asin the helper finds the*exec.ExitErrorand reads code 130.Adds
TestSeal_CtrlCExit130BeforeCtxFlips_NoFalseUnsealed, which fails on the old code (exit 2, false Unsealed) and passes with the fix (exit 130, no verdict). Existing cancel tests (which setctx.Err()) are unchanged and still pass.(#397 (comment))
Lands on develop; the promotion PR head picks it up on the next sync and Bugbot re-reviews.
🤖 Generated with Claude Code
Note
Low Risk
Narrow change to interrupt detection on the seal path; reuses an existing helper and adds regression coverage with no auth or data-handling impact.
Overview
Fixes a Ctrl-C race in
tracebloc client status --sealwhere helm could return exit 130 beforeNotifyContextsetctx.Err(), so a cancelled run was treated as a failed conformance check and printed a false Unsealed (exit 2) instead of a quiet interrupt (exit 130).internal/cli/seal.gonow calls the sharedinstallerRunInterrupted(ctx, err)helper (same as upgrade/prepare-host) at both interrupt boundaries: hook enumeration and each per-check helm run. Cancelled runs still exitexitInterruptedwith no verdict.Adds
TestSeal_CtrlCExit130BeforeCtxFlips_NoFalseUnsealed, which simulates a real*exec.ExitErrorwith code 130 while the context stays active, and asserts exit 130, silent error, and no Sealed/Unsealed output.Reviewed by Cursor Bugbot for commit 65659f1. Bugbot is set up for automated code reviews on this repo. Configure here.