fix(pi): a failed background-job spawn now says why (#504)#508
Merged
Conversation
`launch` refuses with `undefined` on five paths. Four are policy decisions the
caller can predict; the fifth is a spawn failure it cannot. That error was
discarded by a bare `catch {}`, so a Git Bash launch that failed on CI reached
the operator as `expected undefined to be defined` - the runtime's correct,
fail-closed refusal was indistinguishable from a real fault.
The diagnostic is per-launch rather than runtime-scoped, because four launches
can be in flight at once and a shared slot would report one launch's
environment failure as another's - the same misdiagnosis, relocated.
The detail comes from a closed vocabulary: a recognized Windows containment
reason (#428's extractor) or an errno shape, never `error.message`, which
embeds the resolved shell path and working directory. That follows the
convention already set on the adjacent childFailure() channel rather than
widening a second channel to free text.
AC-4 is deliberately not closed. Diagnostics do not stop the flake; they make
the next occurrence explain itself instead of asserting on `undefined`.
CHANGELOG: A background job that cannot start its shell now reports why, with
a bounded reason code, instead of a generic block message.
Ref: #504
Claude-Session: https://claude.ai/code/session_01JgdLb6n8mUdkFiKDTA37ML
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.
Addresses #504 AC-2. #504 stays open on AC-4 — see "What this does not fix".
The defect
SessionBackgroundJobRuntime.launchrefuses withundefinedon five paths. Four are policy decisions the caller can predict (runtime disposed/unhealthy, unparseable input or a stale lease,createJobdeclined, authorization gone stale after publish). The fifth is an environment failure it cannot: the#openTreespawn threw.That fifth path discarded its error in a bare
catch {}. So a Git Bash launch that genuinely failed onwindows-latestreached the operator as:The runtime was never wrong — it is fail-closed and never owns an untracked process, and #504's own triage confirmed that. What was missing is the diagnosis: a correct, designed refusal was indistinguishable from a real fault, and the release preflight (#385) requires the aggregating gate green for the exact commit being tagged, so this cost a re-run cycle on #503.
The fix
Per-launch, not runtime-scoped.
BackgroundJobLaunchInputgains an optionalonRefusal?: (diagnostic: string) => void, invoked only on the spawn path.MAX_ACTIVE_JOBSis 4, so a runtime-scoped "last refusal" field would let one launch's environment failure be reported as another's — the same misdiagnosis #504 is about, relocated. A test drives two overlapping launches that settle in the reverse of their spawn order specifically to pin this.A closed vocabulary, not free text. The detail is a recognized Windows containment reason via
windowsRefusalReasonFromMessage()— #428's existing extractor, already exported and previously unused by this module — or an errno matching/^E[A-Z]{1,15}$/, or nothing. Nevererror.message, which embeds the resolved shell path and working directory viarealpathSync, and which nothing prevents a future throw inprocess-tree.tsfrom widening further.This follows the convention #428 already set on the adjacent
childFailure()channel, explicitly "without widening it unsafely", rather than opening a second channel to free text.Surfaced where the caller already asks.
native-background.tsalready calledtoolFailure(value.runtime.health().diagnostic)on refusal — it asked the right question, buthealth().diagnosticis only populated when the runtime latches unhealthy, which the spawn path deliberately does not do. It now falls through to the per-launch diagnostic, with the sticky health message still taking precedence when both exist.Fail-closed is unchanged and re-pinned. The terminal transition, the
completedactivity publication, and the ownership release all happen before the caller's sink is invoked, and the sink is fail-soft — a throwing or non-function sink cannot alter the refusal path.launchstill resolvesundefinedon all five paths, so no existing caller or assertion changes meaning.Considered and rejected
{kind,id,label,state}is display-only by contract — fail-soft at every producer boundary, control-stripped, capped at 128 code points. The footer is not a diagnostics channel.error.message. Richest, and the only option scoredStrongon availability — but the channel is open by construction. Rejected on that, not on today's contents.Verification
ca-pisuite;tsc --noEmitclean.code, oversizedcode, numericcode, non-Errorthrow), overlapping-launch correlation, hostile-sink fail-soft, and the tool-level surfacing with health precedence.openTree— an absolute-but-nonexistent shell makesrealpathSyncthrowENOENT:--checks, and the pi platform contract: 24/24 PASS.Pi payload, version, changelog, and root metadata advanced together: 0.1.33 -> 0.1.34.What this does not fix
AC-4 is deliberately open. Diagnostics do not stop the
windows-latestflake — they make the next occurrence explain itself instead of asserting onundefined. #504 should stay open on AC-4 alone until a real CI run either names its cause or stops recurring. Closing it here would claim a proof this PR does not have.Also filed while working this: #507 —
tddPhase 5 cannot run, because no coverage command exists for any plugin. It blocked this change's Phase 5 rather than being papered over.Conflict-hierarchy note: the free-text-vs-closed-vocabulary call was resolved at level 1 (security of the audit/disclosure surface) over level 5 (developer velocity), against a marginally more informative message.
https://claude.ai/code/session_01JgdLb6n8mUdkFiKDTA37ML