Repository navigation
fix(spec-review): an unvouchable pin marker narrows the window to n (FAFF-1324) - #1087
Merged
Merged
Conversation
Contributor
|
Ready to land. Run this locally in a real terminal: |
…position Signed-off-by: Alec Hill <[email protected]>
…FAFF-1324) The decision this spike was opened to settle: when pin-capture.json cannot vouch for which round established the pin, govern() now takes the conservative end of the unknown range. The else arm of the marker ternary sets capRound = n instead of 1, and the late-pin rule reports unpinnable-reset when it fires with an anomaly attached. Exit codes are unchanged, no field is added to GovernanceResult, and no anomaly or action token is minted. Two grounds, in order of weight. First, the fail-safe idiom for this exact predicament is already settled twelve lines above: the pin === null arm narrows to n and its comment calls that a fail-safe the governor MUST take. Four of the six anomaly arms narrow to n, and prep narrows at both of its adjacent seams. The marker arms were the only ones that widened by omission, and nothing documented that asymmetry as intentional. Second, window_start is published as the span the loop can vouch for; with capRound = 1 it was not. That is a correctness claim about the artifact rather than about outcomes. Be precise about the trade, because it is not a safer outcome. A wider window is biased toward over-parking, never over-yielding: converging requires strictly_decreasing across every consecutive pair and no_churn across every step, both harder to satisfy as the window widens. Both dispositions therefore terminate at needs-human. What this buys is a comparison the governor can vouch for. What it costs is the one case where a correct yield was still reachable, every round genuinely served by one reviewer, plus the remaining rounds of the appetite cap. The status quo did not yield false convergence. FAFF-1052 deferred this question rather than deciding it: the window-narrowing disposition sits in its Proposed direction, under "Not settled, for prep to shape", not in its Acceptance, and it shipped with capRound = 1 in a line it wrote. This change does not claim to repair an acceptance breach. The decision covers pin-marker-missing identically. The two are one ternary with one severity and one fallback, and readCaptureOutcome returns null for an absent and a torn marker alike, by documented design. Tests: the two committed marker tests now pin action and window_start; three new cases cover window_start already at n (never lowered), round 1 (inert), and an unvouchable marker alongside a genuine swap (swap-reset, anomaly retained). Both selftest twins assert the same, and the contradiction twin needed a window reset because it shares a scratch dir with its sibling. The healthy late-pin-advance path, the occupant-served-dispatch, atomic-write and pin suites all pass unmodified. Deferred, with extension points recorded in the spec: repairing the marker so the anomaly stops latching, and the unguarded --backends-json failure write that is a second live cause of a contradiction. Signed-off-by: Alec Hill <[email protected]>
…ts served backend (FAFF-1324) Review observation: the helper hardcoded winner_index 0 while served_identity came from its servedBackend argument, so the swap fixture wrote BACKENDS[1]'s identity beside index 0 and disagreed with its own data. govern() reads only served_identity, so no test was weakened, but the record was internally inconsistent. Signed-off-by: Alec Hill <[email protected]>
Signed-off-by: Alec Hill <[email protected]>
alechill
force-pushed
the
FAFF-1324-pin-marker-contradiction-disposition
branch
from
October 17, 2026 13:42
933caef to
b6a1578
Compare
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
1 participant
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.
Warning: Ignoring extra certs from
/Users/shftwst/.claude/.cage-relay/ca-certs.pem, load failed: error:80000002:system library::No such file or directoryCloses FAFF‑1324.
A spike, so the deliverable is the decision plus the localised change it implies. The change is three lines of
govern(); the decision is the substance.The decision
When
pin-capture.jsoncannot vouch for which round established the pin,govern()now takes the conservative end of the unknown range. Theelsearm of the marker ternary setscapRound = ninstead of1, and the late-pin rule reportsunpinnable-resetwhen it fires with an anomaly attached (late-pin-advancewhen the anomaly is null). Exit codes unchanged, no field added toGovernanceResult, no anomaly or action token minted.Two grounds, in order of weight.
pin === nullarm narrows tonand its comment calls that a fail-safe the governor MUST take. Four of the six anomaly arms narrow ton, and prep narrows at both of its adjacent seams. The marker arms were the only ones that widened by omission, and nothing documented that asymmetry as intentional.window_startis published as the span the loop can vouch for, and withcapRound = 1it was not. That is a correctness claim about the artifact rather than about outcomes, and it is the claim the spike was opened to settle.The decision covers
pin-marker-missingidentically. The two are one ternary with one severity and one fallback, andreadCaptureOutcomereturnsnullfor an absent and a torn marker alike, by documented design.Be precise about the trade
This does not buy a safer outcome, and the PR should not be read as claiming it does.
A wider window is biased toward over-parking, never over-yielding:
convergingrequiresstrictly_decreasingacross every consecutive pair in the window andno_churnacross every step, both strictly harder to satisfy as the window widens.spec-review-window.js:18-21says so outright. Both dispositions therefore terminate atneeds-human.What this buys is a comparison the governor can vouch for. What it costs is the one case where a correct yield was still reachable, every round genuinely served by one reviewer, plus the remaining rounds of the appetite cap. The status quo did not yield false convergence.
FAFF‑1052 deferred this question rather than deciding it: the window-narrowing disposition sits in its Proposed direction, under "Not settled, for prep to shape", not in its Acceptance, and it shipped with
capRound = 1in a line it wrote. This change makes no claim to repair an acceptance breach.The accepted latch, and its owner
Nothing repairs an unvouchable marker, so every later governed round also narrows to
nand the convergence window becomes[n..n]for the rest of the loop.convergingis then deterministically false, the loop-cap yield never fires, and the loop runs to the appetite ceiling and parks. That is a real interim cost, accepted deliberately, and it is reachable rather than theoretical: two live causes of an unvouchable marker remain.Both now have owners, which is what makes the deferral honest rather than silent:
capturePin's existing-pin arm. Its open question is sharper than the spec assumed:pinned-reviewer.jsonstores no round, so the capture round may not be recoverable at all, and guessing it would restore a window the loop still cannot vouch for.--backends-jsonfailure write atspec-review-pin.js:382. FAFF‑1052 added that write deliberately for its never-silent property, so dropping it is the wrong remedy; the ticket records that constraint.Verification
occupant-served-dispatch,atomic-write,pin) 46/46, none edited.faff spec-review-window --selftestPASS against the modified lib. Both marker twins now assertactionandwindow_start; the contradiction twin needed a window reset because it shares a scratch dir with its sibling, whose earliergovern()call had already persistedwindow_start = 3. Without it the twin could never reach the late-pin rule. That reset is an addition beyond the spec's DoD text and was disclosed rather than quietly made.window_startalready atn(held, never lowered), round 1 (inert), and an unvouchable marker alongside a genuine swap (swap-reset, anomaly retained).window_start: 3,action: "unpinnable-reset",anomaly: "pin-marker-contradiction",governance-3.jsonmatching stdout,window.jsonre-read as{"window_start":3}.passon round 2. Round 1's major was the unfiled follow-ups above; its observation (a fixture's hardcodedwinner_index) is fixed.The local gate ladder reports
failon its UNIT rung for a pre-existing reason, not this change: a baseline run on an unmodified checkout had 46 failures against this worktree's 45, and a sorted diff of the failing-test lists showed zero unique to this change. LINT, TYPECHECK and STATIC_ANALYSIS rungs all pass.Plain JavaScript, no TypeScript sibling, so there is no emit to rebuild.
Acceptance Criteria
faff-1324#done:1A governed round with an unvouchable marker andwindow_start < npersistswindow_start: n, so the convergence window no longer spans rounds whose shared reviewer is unproven.Verified: Integration smoke test run: govern round 3 over a pin with a {state:failed,round:1} marker and window_start 1 persisted window_start 3, action unpinnable-reset, anomaly pin-marker-contradiction; window.json re-read as {"window_start":3}.
Method: command
faff spec-review-window --govern --dir <scratch> --round 3 --any-servedfaff-1324#done:2The commit message records the decision and rests it on the fail-safe idiom and onwindow_start's published meaning, states the trade in the direction Failure modes establishes (a wide window is biased toward over-parking, so this change buys a vouchable comparison rather than a safer outcome), and notes that FAFF‑1052 deferred this question rather than deciding it. It must not claim the status quo yielded false convergence, and must not claim it breached FAFF‑1052's acceptance.Verified: Commit 02a45a9's message rests the decision on the fail-safe idiom and on window_start's published meaning, states the over-parking direction and that the change buys a comparison the governor can vouch for rather than a safer outcome, and records that FAFF‑1052 deferred the question. Neither forbidden claim appears: no false-convergence claim, no acceptance-breach claim.
Method: command
git log -1 --format=%Bfaff-1324#done:3GovernanceResultcarries exactlyround,window_start,action,anomaly,served_identity,capture_state,capture_round,ts;test/spec-review-window.test.mjs:418-432passes with its key list unmodified.Verified: Key-set test passes with its list unmodified; no field added to GovernanceResult.
Method: test
test/spec-review-window.test.mjs::govern: every non-arg-error path writes governance-<n>.json with the full result shapefaff-1324#done:4NoAnomalyTokenorActionTokenvalue is added, renamed, or removed;grep -c '"unpinnable-reset"' plugin/skills/faff/bin/lib/spec-review-window.jsshows the token reused, not replaced.Verified: grep -c '"unpinnable-reset"' returns 6 — the token is reused, not replaced; no token added, renamed or removed.
Method: command
grep -c '"unpinnable-reset"' plugin/skills/faff/bin/lib/spec-review-window.jsfaff-1324#done:5spec-review-window.js:245— theelsearm — setscapRound = n; the literalcapRound = 1no longer appears ingovern(). Do not edit:244: that is the cleancap.state === "captured"arm, and settingcapRound = nthere would break late-pin-advance and failtest/spec-review-window.test.mjs:308.Verified: The else arm sets capRound = n and the literal capRound = 1 no longer appears anywhere in the file (0 occurrences). The arm now sits at :247 rather than :245 because the explanatory comment added above it shifted the line; the clean captured arm was not touched.
Method: command
grep -n 'capRound = n' plugin/skills/faff/bin/lib/spec-review-window.js; grep -c 'capRound = 1' plugin/skills/faff/bin/lib/spec-review-window.jsfaff-1324#done:6The late-pin rule reportsunpinnable-resetwhen it fires with a non-null anomaly andlate-pin-advancewhen it fires withanomaly == null.Verified: Rule (L) reads action = anomaly === null ? "late-pin-advance" : "unpinnable-reset" at :251; both branches are covered by the suite.
Method: test
test/spec-review-window.test.mjs::govern: late-pin-advance — the pin is first captured at round 2 (the FAFF‑996 recovery case)faff-1324#done:7test/spec-review-window.test.mjs:347(govern: soft pin-marker-missing ... full GovernanceResult asserted) is updated:actionchanges from"unchanged"to"unpinnable-reset", and a newassert.equal(result.window_start, 3)is added.code: 0,anomaly: "pin-marker-missing",capture_state: "absent",capture_round: nullare all asserted unchanged.Verified: The pin-marker-missing test now asserts action unpinnable-reset and window_start 3; its code, anomaly, capture_state and capture_round assertions are unchanged.
Method: test
test/spec-review-window.test.mjs::govern: soft pin-marker-missing (legacy pin, no capture marker) — full GovernanceResult assertedfaff-1324#done:8test/spec-review-window.test.mjs:362(thepin-marker-contradictiontwin) gainsassert.equal(result.action, "unpinnable-reset")andassert.equal(result.window_start, 3). Its existingcode: 0,anomaly, andcapture_state: "failed"assertions pass unmodified; this test asserts noactionorwindow_starttoday, so it does not fail before the edit. The assertions are added so the new behaviour is pinned rather than merely tolerated.Verified: The contradiction twin gained the action and window_start assertions; its existing assertions are untouched.
Method: test
test/spec-review-window.test.mjs::govern: soft pin-marker-contradiction (pin present, capture marker state != captured) — full GovernanceResult assertedfaff-1324#done:9The selftest twin atspec-review-window.js:556(govern pin-marker-missing (soft, non-fatal)) assertsr.result.action === "unpinnable-reset"andr.result.window_start === 3in place ofaction === "unchanged", keepingr.code === 0and the anomaly check.Verified: Selftest twin asserts action unpinnable-reset and window_start 3, run via the worktree's own binary (the modified lib), not the main checkout's.
Method: command
./plugin/skills/faff/bin/faff spec-review-window --selftestfaff-1324#done:10The selftest twin atspec-review-window.js:560(govern pin-marker-contradiction (soft, non-fatal)) is extended from code-and-anomaly only to also assertaction === "unpinnable-reset"andwindow_start === 3, closing the asymmetry with its sibling.Verified: The contradiction selftest twin now asserts action and window_start too, closing the asymmetry with its sibling. It needed a writeWindowStart(tmp, 1) reset, because it shares a scratch dir with the sibling whose govern call had already persisted window_start 3; without it the twin could not reach the late-pin rule at all. This fixture reset is an addition beyond the spec's literal DoD text.
Method: command
./plugin/skills/faff/bin/faff spec-review-window --selftestfaff-1324#done:11faff spec-review-window --selftestreports PASS and exits 0, sotest/spec-review-window.test.mjs:250,256pass.Verified: RESULT: PASS (spec-review-window, 0 failed), exit 0, against the modified lib.
Method: command
./plugin/skills/faff/bin/faff spec-review-window --selftestfaff-1324#done:12A governed round with an unvouchable marker andwindow_startalready equal tonreportsaction: "unchanged",window_start: n, anomaly recorded; covered by a new test case.Verified: New case: window_start already equal to the round reports unchanged with window_start held, anomaly recorded, exit 0 — never lowered.
Method: test
test/spec-review-window.test.mjs::govern: unvouchable marker, window_start already n — unchanged, never loweredfaff-1324#done:13Round 1 with an unvouchable marker is unchanged from today:window_start: 1,action: "unchanged"; covered by a new test case.Verified: New case: round 1 is inert — window_start 1, action unchanged, exit 0.
Method: test
test/spec-review-window.test.mjs::govern: unvouchable marker at round 1 — window_start 1, unchangedfaff-1324#done:14An unvouchable marker plus a served identity differing from the pin still reportsaction: "swap-reset"with the anomaly attached; covered by a new test case.Verified: New case: an unvouchable marker plus a differing served identity still reports swap-reset with the anomaly attached.
Method: test
test/spec-review-window.test.mjs::govern: unvouchable marker plus a swapped served identity — swap-reset with the anomaly attachedfaff-1324#done:15test/spec-review-window.test.mjs:308(the healthylate-pin-advancepath) and theunchangedandswap-resettests pass unmodified, proving the clean branch is untouched.Verified: Whole window suite green at 35/35, including the healthy late-pin-advance path and the unchanged and swap-reset tests, none of which were edited.
Method: test
test/spec-review-window.test.mjs::govern: late-pin-advance — the pin is first captured at round 2 (the FAFF‑996 recovery case)Method: test
test/spec-review-window.test.mjs::govern: unchanged when round 2 is served by the SAME backend as the pinMethod: test
test/spec-review-window.test.mjs::govern: swap-reset fires from served-<n>.json even though the pin is still chain[0]faff-1324#done:16test/spec-review-occupant-served-dispatch.test.mjs:76-80passes unmodified, asserting exit 0 andaction: "swap-reset"through the realfaffbinary.Verified: Passes unmodified; git diff --name-only HEAD~1 HEAD lists no sibling test file.
Method: test
test/spec-review-occupant-served-dispatch.test.mjs::occupant dispatch (extracted verbatim from SKILL.md): round >= 2 idempotent no-op capture STILL writes served-<n>.jsonfaff-1324#done:17test/spec-review-atomic-write.test.mjs:69,88andtest/spec-review-pin.test.mjs:262,290pass unmodified.Verified: Both suites pass unmodified; 46/46 across the three sibling suites, and the diff touches none of them.
Method: test
test/spec-review-atomic-write.test.mjs::govern: a rename fault on governance-<n>.json throws and leaves no orphaned tmp fileMethod: test
test/spec-review-pin.test.mjs::captureNoLensServed: a no-lens round after a served round PRESERVES the captured markerMethod: test
test/spec-review-pin.test.mjs::captureNoLensServed: a non-captured prior marker is replaced, not preservedfaff-1324#done:18test/spec-review-window.test.mjs:268(the prep prose lint) passes unmodified;git diff --statshows no change toplugin/skills/faff-prep/SKILL.mdand no change toSKILL_LINE_BASELINE.Verified: The prose lint passes and the diff touches neither plugin/skills/faff-prep/SKILL.md nor validate-adapters.js, so SKILL_LINE_BASELINE needs no bump.
Method: test
test/spec-review-window.test.mjs::faff-prep/SKILL.md: the loop resolves the window + round number via the new CLIfaff-1324#done:19No change tospec-review-pin.js,spec-review-convergence.js,judge-trail.js, or anySKILL.md.git diff --name-onlylists onlyspec-review-window.jsandtest/spec-review-window.test.mjs.Verified: git diff --name-only HEAD~1 HEAD lists exactly spec-review-window.js and its test; no change to spec-review-pin.js, spec-review-convergence.js, judge-trail.js or any SKILL.md.
Method: command
git diff --name-only HEAD~1 HEADfaff-1324#done:20The marker self-heal and the unguarded--backends-jsonfailure write are filed as separate issues, each citing its extension point from OUT OF SCOPE.Verified: Both filed: FAFF‑1331 (the marker self-heal, citing capturePin's existing-pin arm at spec-review-pin.js:214-224 and the [n..n] latch consequence from Failure modes) and FAFF‑1332 (the unguarded --backends-json failure write, citing the atomicWriteJSON at spec-review-pin.js:382 inside the :378-385 catch block, and noting FAFF‑1052's never-silent property the fix must preserve). Each cites its OUT OF SCOPE extension point. Intake recorded for both.
Method: human
Closes FAFF-1324