Replies: 2 comments
Correcting this post: the framing was wrongI opened this asking which of two guarantees to keep, and asked for a security-model opinion on the trade. There is no trade. Two things settle it, and one of them was measurable from inside this repository the entire time. 1. This repository is squash-only, so the stronger option was never available hereI wrote the configuration block above as a property of an adopting repository. It is also a property of this one: Re-measured against this repository's own archives just now: #689 recorded 19 of 178 yesterday. The numerator has not moved; only the denominator has. So the table row labelled commit ancestry / survives squash: no is not a stronger guarantee we would be conceding. It describes a property this project has held one time in nine across its whole history, and never once by design. The choice is not weaker versus stronger. It is weaker versus none. I also had that 11% figure before writing this post, and used it to justify the #677 fix, and still framed the same cause as an adopter's configuration when it appeared on 2. The readiness half was not a semantics decision eitherI wrote that "the fix is a semantics decision, not a bug fix, and it should not be made by whoever happens to be implementing." Also wrong.
An adversarial survey run alongside that fix also established that The fix is merged in #695, on
Rebase and merge-commit are the control, not the fix. They show that nothing which worked was broken. What is still open is narrower, and it is #694Options 1 to 3 in the post above should now be read as applying only to that issue, because the check it concerns is genuinely different from the one I was writing about.
let allowed_prefix = git_repo_relative_path(root, &format!("{CHANGES_PATH}/{}/", record.id))?;So it proves that between the review and The defect is not "which guarantee do we keep". A squash destroys Measured here: 0 of 107 passing (2 reachable) archived reviews cite an implementation commit reachable from It also wedges. The question I should have askedNot what should The open question is what a check should report when it cannot run. Three candidates on #694, none chosen:
If you run spec-sync anywhere, the useful data point is no longer "does this match what you see". It is whether option 1's disclosure would be enough for you, or whether you rely on the guarantee itself and would accept option 3's cost to keep it. Correction (later). Two things above have been overtaken by measurement. The scoped-review walk does not merely fail sometimes on a squash repository — it passes 0 of 107 archived reviews, because archiving relocates the workspace out from under the walk's own path allowlist, so every change that ships fails its own walk forever after. And the walk never reads a blob: its whole vocabulary is |
|
The readiness half is no longer "on Nothing in the correction comment above has been un-corrected. What remains is still #694, and the options in the original post should not be used to decide it. The issue's correction comment ruled out options 2 and 3 against measurement:
The option that is actually left was not in this thread: bind the delta bodies into the review digest (option 4 on #694). That is the one property the walk uniquely protected, the walk does not actually protect it, and a digest over those bodies survives every merge strategy without claiming a topology it cannot see. If you run spec-sync on a squash-merging repo and you were waiting on this thread to tell you whether 6.0 is safe to adopt: the readiness trap this post opened with does not fire on 6.0.0. Archive on the PR before you merge; that rule did not go away. The remaining question is whether option 4 is enough for the review guarantee, and that question belongs on #694 rather than here. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Note
Resolved for
verification.commit. This is answered onmain:verifiedmeans content agreement — the recorded plan and tree still match what was verified — and commit ancestry was never load-bearing on the readiness path. Fixed in #689, merged in #695, onmainand not yet in a tagged release candidate.Two claims in the original text below are now refuted, not merely stale: "there is currently no configuration that avoids it", and "the fix is a semantics decision, not a bug fix".
ship_status_reportwas simply the last caller still asking a history question that every other currency path had already stopped asking.What remains open is #694 — what a check should report when it cannot run. The options listed further down apply to #694 only, and two of the three have since been ruled out. That issue's own framing has also been substantially corrected against measurement; see the correction comment there.
The question
verification.commitis checked withmerge-base --is-ancestor. A squash-merge rewrites that hash, so a change reads as unverified the moment its own PR lands.Measured on an adopting repository: three invalidations of one change in a single day (
CorvidLabs/site#244,#246,#247— that repository's numbering, not this one's). Each one forced a full re-verify and a fresh independent review — the one step in the lifecycle that structurally requires a human.There is currently no configuration that avoids it. On that repo:
Rebase-merge is disabled at the repository level, and
gh pr merge --rebasesilently falls back to squash — no error, no warning. So an adopter can follow a documented "use rebase-merge" instruction exactly, believe they complied, and still land a squash.Squash-only is a common posture for GitHub organisations. This is not an exotic configuration.
Tracked as #689, since fixed. It was framed here as a semantics decision rather than a bug fix; that framing was wrong, and the note at the top records why.
What has to be decided
verification.jsonalready records content-addressed evidence alongside the hash:{ "commit": "f127446a...", <- destroyed by squash "workspace_digest": "1bbcc8e1...", <- survives "acceptance_input_digest": "2b71cbcf...", <- survives "acceptance_manifest": { "entries": [ { "path": "specs/validator/context.md", "payload_digest": "c6b7498a..." } ]} }So the material for a content-based answer is already on disk. Feasibility is not the question. The question is what
verifiedshould mean.These are not equivalent guarantees. An attacker who can rewrite history cannot forge the first. The second is weaker.
But on a squash-only repository the first is unobtainable, so the real choice is weaker guarantee vs. no guarantee — not weaker vs. stronger.
Three options
workspace_digest/ manifest agreement as sufficient freshness whenever ancestry is unavailable. Simpler, concedes the same thing more broadly.commitas an informational correlation key. Cleanest model, largest change, concedes the most.Precedent, and why this is not hypothetical
#677 corrected the archive anchor predicate for exactly this cause: it looked for
.specsync/changes/<id>/state.jsonon the default branch, which a squash-merged workflow-v2 change never has, becausefinalizearchives inside the same PR.Measured across this repository's own history: 83 of 178 archives had the active path; 178 of 178 had the archive path. The fix asked the question that could actually be answered.
verification.commitancestry is a different check with the same flaw from the same cause, and it has not been touched.What would help
Interested in the security-model view most of all. The trade is small in code and large in meaning.
All reactions