Repository navigation
Say what the staging arm establishes, instead of a construction law the type does not have - #11880
Merged
Merged
Conversation
…he type does not have Comment-only. #11677 landed carrying three sentences stronger than its source, and the side-chat review that parked it named them; the PR was enqueued and merged before the correction could be pushed, so it lands here instead. - runner_attempt_launch: AttemptLaunchPlan is not sole_constructor, so LaunchAuthorized is assemblable outside the module and its jailer field is readable by any holder -- the raw jailer escapes before AttemptStagingReceipt exists. The arm establishes that a plan from THIS planner carries both fields or neither, not that staging ran or that the jailer cannot be obtained first. - the sealed-receipt paragraph: sealing withholds nothing while the same command sits on the unsealed plan, and the gate accepts caller-authored JitDeviceStaged values -- this module's own witnesses build one -- so the receipt proves the gate was called with positive-shaped arguments, not that the effects ran. - github_effect_perform: the commit-ambiguous classification is right, but JitMintStepNotSucceeded drops the attempt and runner identity, so the promise of reconciliation by runner name was not backed by the value. Also records the reframed guarantee a future implementer needs: device-without- launch is not the state that must be impossible; a VMM launched without an effect-bound, read-back credential device for this attempt is. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…oes and does not do The reviewer's finding, which is the one worth recording: a PR whose whole purpose was removing overstated commentary introduced three smaller overstatements of its own. 1. 'this verdict read' attributed host I/O the gate never performs -- attempt_staging_verdict RECEIVES both observations as caller-supplied values. It now says the receipt carries the planned paths the supplied outcomes were judged against. 2. 'it withholds nothing' was broader than the defect: sealing DOES withhold direct construction of AttemptStagingReceipt. What it does not withhold is the jailer capability. 3. 'nothing consumes this function' was false (a witness calls it); the absent consumer is a PRODUCTION one. And attempt identity alone is not the reconciliation subject -- it derives the runner name but does not identify the ORGANIZATION the registration must be queried in. 4. 'Closing it means withholding...' asserted the only possible construction; it is now one mechanically preventing repair. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Pushed the four substitutions from the review as The finding worth recording above the list: a PR whose entire purpose was removing overstated commentary introduced three smaller overstatements of its own. That is the repair-is-the-least-audited-code class.
Comment-only, re-confirmed against current main (the check re-run because main advanced past the old base): non-comment, non-blank ADDED lines = 0; removed = 0. Three files, +51/-13, all comment or blank. Per the review's landing treatment: no witness execution, no generated-artifact run, no substantive re-review. |
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.
Comment-only. No behaviour change, no new declarations, no witness changes — every changed line is a comment line (
git diff --numstattotals 57, all//). So it needs no re-take of evidence.Why
#11677 landed carrying three sentences stronger than its own source. The side-chat review that parked that PR named them; the PR was enqueued and merged before the correction could be pushed (its branch was frozen by the merge queue at the time), so the correction lands here.
The hazard is not runtime behaviour — nothing in production selects this code. It is that the prose asserts a structural law the types do not have, stated strongly enough that a later reader would treat it as the already-solved boundary and build on it without reopening the argument.
Verify in one read, on
mainbefore this PR:dag/gunbc/runner/runner_attempt_launch.dag—type AttemptLaunchPlanhas nosole_constructor(grep -c 'type AttemptLaunchPlan sole_constructor'→0), while the comment above it said no admitted credential means "no device AND no VMM process — the two cannot come apart".LaunchAuthorized.The three corrections
runner_attempt_launch, the plan arm. Now says what the arm actually establishes: a plan built by this planner carries a staging plan and a jailer together or neither. It states plainly that the type is notsole_constructor, thatLaunchAuthorizedis assemblable outside the module, and that its jailer field is readable by any holder — so the arm does not establish that the value came fromadmit_jit_credential, that staging ran, that the readbacks happened, or that the jailer cannot be obtained first. The reported rung (Mitigatable) was already honest; the prose was not.runner_attempt_launch, the sealed receipt. SealingAttemptStagingReceiptis real, but it proves less than claimed, twice: it withholds nothing while the same jailer command sits on the unsealed plan, and the gate accepts caller-authored positive outcomes — aJitDeviceStagedvalue is constructible withoutstage_jit_deviceever running, and this module's own witnesses construct one to drive the foreign-device case. So the receipt proves the gate was called with positive-shaped arguments, not that the host effects and their readbacks occurred.github_effect_perform, the commit-ambiguous arm. The classification is correct and stays — reporting an unanswered mutation POST as a refusal would assert a fact nobody observed. What is retracted is the promise beside it:JitMintStepNotSucceededcarries only the step and the performance, dropping the attempt, runner name, dispatch and organization, so it does not give an operator what reconciling a possibly-created registration needs.Also recorded
The reframed guarantee, because it is what a future implementer needs and it is not obvious: device-without-launch is not the state that must be impossible — a staged device with no VMM is safe provided teardown removes it. The dangerous state is a VMM launched without an effect-bound, read-back credential device for this attempt. Closing that means withholding the raw jailer capability until after effect-bound staging, the staging realization itself constructing the executable jailer from its own observations. That is a substantive redesign and is explicitly not attempted here.
🤖 Generated with Claude Code