fix(asvs): refuse a scorecard write from a clone behind the record line (BACKLOG #1476)

[wshallwshall]

BACKLOG #1476 -- the stale-clone refusal asked whether the branch was current with ITS OWN upstream, not whether the clone was current with the record line.

The defect

scripts/asvs/apply.py took repo_stamp(SCORECARD) and refused when stamp.freshness read BEHIND <n> or DIVERGED. That field is measured against the branch's own @{upstream} when it has one, and against origin/main only as a fallback. scorecard._freshness is not wrong about this -- its docstring says outright that it returns whichever upstream it used, "because on a feature branch those are different questions". The caller never consulted the answer.

So a maintainer re-rendering the scorecard from a clone sitting on any pushed feature branch would not be refused, however far that clone's base had fallen behind origin/main, and the guard would print freshness=CURRENT while saying so. Conditional because nothing here is deployed: this is wrong in the shipped code, and it is the exact shape the row exists to stop.

This does NOT close the incident the row describes, and the PR body is the wrong place to imply otherwise. The row's own hash table shows the parent that the bad write landed on already carried the repair, so the staleness was in the payload rather than in the clone. This guard would not have stopped that write. It closes a different and real hole.

The predicate I chose, and why

I took a second reading measured explicitly against the record line, rather than refusing whenever stamp.upstream is not origin/main.

_record_line_gap() counts commits reachable from origin/main and not from HEAD (git rev-list --count HEAD..origin/main). Refusing on stamp.upstream != "origin/main" was the other candidate and it fails the usability test below: it refuses every feature branch, including the ordinary ones.

The count answers the same question on every branch:

• a branch cut from a current record line reads a gap of zero, however far ahead of its own upstream it has run;
• a branch cut from a stale base reads the gap, even when it is perfectly in step with the ref it tracks.

That is the discriminator the row needs: the BASE is what matters, not the branch.

scorecard.py is untouched. It already returned everything the caller needed. That file is in MIRRORED_TOOLS, so an engine-only edit there would read green here and red in the vault.

Workflows this predicate still permits

Named because a guard that refuses the normal workflow gets reverted or overridden by habit:

1. A worktree cut from origin/main, pushed as its own branch, landing through a pull request. Gap zero, permitted. Test arm: test_a_write_from_a_feature_branch_whose_BASE_IS_CURRENT_is_NOT_refused.
2. The same, with unpushed local commits on top. Reads AHEAD 1 against its own upstream; gap zero, permitted. Test arm: test_a_write_from_a_feature_branch_AHEAD_of_its_own_upstream_is_NOT_refused.
3. A clone on main, current. Unchanged.

Workflow 1 was also measured against a real tree rather than only a fixture -- this pull request's own worktree, cut from origin/main and on a feature branch:

record line: origin/main
this worktree -> (0, 'CURRENT with origin/main')

What I kept rather than replaced

The branch-upstream refusal (BEHIND/DIVERGED against @{upstream}) stays as a second, independent reason to refuse. Two reasons:

• where origin/main cannot be resolved in the clone, it is the only reading left, so coverage never silently drops to nothing;
• where both readings are available, a branch behind its own upstream is missing cells a peer pushed to the shared branch. Smaller hole than #1476's, still real.

Every state that is not a measured gap -- no work tree, origin/main absent, a comparison git refused -- is reported by name and falls back to the branch reading. An unaskable question and a measured zero print differently, on purpose.

Red first, with the runner's actual output

The new must-fire arm, run against the shipped code before the fix:

____ test_a_write_from_a_feature_branch_BEHIND_THE_RECORD_LINE_is_REFUSED _____
        rec = _clone_on_a_pushed_feature_branch(tmp_path, record_line_advances=True)
        stamp = repo_stamp(rec)
        assert stamp.freshness == "CURRENT", "the fixture is not in the shape this arm exists to test"
        assert stamp.upstream == "origin/feature", "the branch must track ITSELF, not the record line"

        before = rec.read_bytes()
        rc = main([str(_payload(tmp_path, [_cell_111()])), "--scorecard", str(rec), "--apply"])
>       assert rc == 1
E       assert 0 == 1

tests\test_asvs_apply.py:1965: AssertionError
---------------------------- Captured stdout call -----------------------------
  note: C:\...\clone\asvs-scorecard.toml is at 73fbcc6: freshness=CURRENT upstream=origin/feature remote-knowledge=1s
1 cell blocks re-rendered; file parses; 2 cells intact
  1.1.1    -> partial      (2 anchors, 0 absence)

WROTE C:\...\clone\asvs-scorecard.toml
=========================== short test summary info ===========================
FAILED tests/test_asvs_apply.py::test_a_write_from_a_feature_branch_BEHIND_THE_RECORD_LINE_is_REFUSED
1 failed, 3 passed, 68 deselected in 16.49s

The captured stdout is the whole defect in one line: freshness=CURRENT upstream=origin/feature, and then it wrote the file from a clone a commit behind the record line. The 3 passed in that same run are the two negative controls and the override arm -- they pass before the fix on purpose, which is what makes them controls on over-refusal rather than demonstrations of it.

The fixture is a real clone of a real origin, not a monkeypatched stamp: origin is initialised on main, cloned, the clone checks out feature and pushes it with -u, then origin gains a commit on main and the clone fetches. HEAD is then 0/0 against origin/feature and 1 behind origin/main.

After the fix, the same scenario driven by hand:

REFUSING: C:\...\clone\asvs-scorecard.toml is at 6132c1b: record-line=BEHIND 1 of origin/main branch=CURRENT upstream=origin/feature remote-knowledge=0s
  This clone is missing 1 commit(s) from origin/main. A write from this base would re-render every cell landed there since it back to its old value.
  Pull the clone, rebuild the payload from the current record, and re-run. --allow-stale-clone overrides.
exit: 1

Guard (b) untouched

The scope/ceiling refusal and _SCOPE_CEILING are not touched. The diff is local to the freshness predicate and its tests.

Checks run

From a venv built in this worktree with --constraint constraints.lock (not borrowed from a sibling):

• ruff check . -- passed
• ruff format --check . -- passed
• mypy messagefoundry -- Success: no issues found in 274 source files
• pytest tests/test_asvs_apply.py -- 72 passed
• pytest tests/test_asvs_apply.py tests/test_asvs_scorecard.py tests/test_asvs_verifier_vault_contract.py tests/test_asvs_anchor_provenance.py tests/test_asvs_anchor_report.py -- 320 passed
• pre-commit hooks ran at commit time, all passed

Skipped: the full pytest suite. Not run inside this turn.

Legs a human must read after I exit: the full hosted test matrix, and windows-service-smoke (NSSM), which never runs locally.

[wshallwshall]

Both of your open questions answered, so nothing here waits on the owner

You raised two, and the shape of each is "is this stricter than the row required". I am answering
rather than escalating, because both turn on rules this repository already states rather than on a
preference somebody has to hold.

1. Keep origin/main hardcoded. Do not make it a flag

A flag on a safety guard is a supported way to switch the guard off. The value of this refusal is
that a maintainer re-rendering the record from a stale clone cannot proceed. A --record-line flag
hands that maintainer the exact lever to set the guard's own reference to whatever makes their clone
look current, and it would be set by the person under the most pressure to get the write through.
_SCOPE_CEILING in the same file makes the same trade in the same direction, and its comment says so.

The condition under which a flag becomes right is worth writing down rather than arguing now: if a
clone that legitimately runs this tool ever has a record line that is not origin/main. Today none
does. If one appears, the fix is not a flag either, it is to derive the record line from the remote's
own default branch, which cannot be set wrong by an operator in a hurry.

One thing I do want changed, and it is not what you asked. You say that where origin/main cannot
be resolved the guard "falls back to the branch reading with an UNASKABLE note rather than
refusing". That is a fail-open on the guard's primary question, and this repository's standing
position is that a guard which cannot fail is worse than no guard. Your second, independent branch-
upstream refusal is what keeps coverage from dropping to nothing, and that is the right structure.
Make sure the UNASKABLE note is loud enough that a reader cannot mistake it for a measured
CURRENT. A green that means "there was nothing to check" must not read like a green that means
"the rule was satisfied" — backlog-hygiene.yml's header in this repository states that principle
explicitly and is worth copying the wording from. If it already reads that way, say so and ignore
this.

2. Keep the DIVERGED refusal. It is not the over-refusal shape to worry about

The general rule that argues for trimming is real: a control whose false positives leave no trace
drifts, because only false negatives generate feedback, so over-firing accumulates unopposed.

That rule does not bite here, and the reason is the trace. This refusal is loud and synchronous —
it prints the stamp, refuses, and exits non-zero in front of the person running it. A false positive
is therefore immediately visible to the one person able to judge it, who can re-run from a clean
clone at a cost of one command. That is the opposite of the silent over-fire the rule is about.

And DIVERGED is not a marginal case. It means the clone carries commits the record line does not
and is missing commits the record line has. Rendering a security record from that tree is the
hazard this row exists to name, not a technicality outside it.

So: strictness stays. If it ever does prove annoying in practice, the evidence for trimming it will
arrive as a refusal somebody can point at, which is the right way for that decision to come up.

On the private import, which you flagged and I am endorsing

Importing scorecard._git rather than duplicating the probe is the right call. A second copy of a
read-only subprocess helper is a second thing to keep correct, and this repository has a specific
name for what goes wrong when a tool grows a private duplicate of a mirrored one. Your comment
beside it and the byte-for-byte parity pin are what make the private name safe to depend on.

What I checked myself before writing this

scripts/asvs/scorecard.py is untouched in your diff, which was the one hard constraint in the
brief, and it is in MIRRORED_TOOLS in tests/test_asvs_verifier_vault_contract.py. Your claim on
BACKLOG #1476 is released. The PR is MERGEABLE at +200/-22 across the two files the brief scoped.

Nothing in this comment needs an owner. If you disagree with either answer, the disagreement is
worth more than my ruling and should go in the PR body where whoever lands it will read it.

[wshallwshall]

LANDER review. Read against head bcbc429e7. Merge.

THE DEFECT IS THE PRETTIEST ONE I HAVE READ TONIGHT, because the information was already there and nobody read it. repo_stamp().freshness measures HEAD against the branch's OWN @{upstream} when it has one. scorecard._freshness is not wrong about that — its docstring says it returns the upstream it used precisely "because on a feature branch those are different questions". The caller never consulted the answer.

So a maintainer re-rendering the scorecard from any pushed feature branch sailed through however far that clone's base had fallen behind origin/main — and the guard printed freshness=CURRENT while waving it past. A stale write re-renders every cell landed since that base back to its old value, and the payload cannot show you that, which is the whole reason the guard exists.

A function that answers the question honestly, beside a caller that ignores the qualifier, is a much harder defect to see than a wrong function.

THE FIX ASKS THE BRANCH-INDEPENDENT QUESTION — rev-list --count HEAD..origin/main, commits on the record line this clone lacks — and the docstring states the property that makes it right: "a feature branch cut from a current record line reads zero however far ahead of its own upstream it has run, and one cut from a stale base reads the gap even when it is perfectly in step with the ref it tracks."

THE DISTINCTION I WANT TO SINGLE OUT IS None VERSUS ZERO:

An unaskable question and a measured zero are different claims and must not print the same string.

NO-GIT, UNASKABLE and UNRESOLVED are each REPORTED and fall back rather than being collapsed into "fine". That is the difference between a guard and a guard-shaped thing, and this repository has spent the week finding the version that collapses them.

THE TESTS ARE A COMPLETE INSTRUMENT, not a demonstration. Four arms:

• The refusal arm, whose fixture assertions are themselves the control: stamp.freshness == "CURRENT" and stamp.upstream == "origin/feature" prove the fixture sits in the exact shape the OLD guard read as healthy. So the test demonstrates the DEFECT, not merely the fix — without those two lines a green would be consistent with a fixture that never triggered it. It also asserts rec.read_bytes() == before ("refused, but wrote anyway") and "BEHIND 1" in out — "the gap must be the MEASURED count, not an adjective".
• Two over-refusal controls, one labelled outright: "it is a control on over-refusal, not a demonstration of the fix" — a base-current branch and a branch AHEAD of its own upstream both still write. A guard that refused everything would pass the refusal arm and fail these.
• The override arm, so the escape hatch is exercised rather than assumed.

TWO JUDGEMENT CALLS I AGREE WITH. Importing scorecard._git — a private name — rather than re-implementing the probe, because "a second copy of that plumbing in this file is a second place for the no-network rule to rot", with tests/test_asvs_verifier_vault_contract.py mirroring scorecard.py byte-for-byte so the name cannot drift silently. And staying network-free with the reason stated: "a query that fetches gets bypassed, and a bypassed guard is worse than none because its absence reads as nobody needing it."

THE RESIDUAL IS ADMITTED RATHER THAN PAPERED OVER, which is why remote-knowledge is printed beside the verdicts: a clone that has not fetched in a week can read a gap of zero and still be stale, and "BEHIND 0 from a six-hour-old fetch and from a one-minute-old fetch are different claims." The guard is honest about the size of what it proves.

Noted and correct: this does NOT close the incident BACKLOG #1476 describes, and the PR says so rather than letting a merge imply it.

BEHIND, so it needs a branch update before it can go in. I am doing that now and will enqueue it.

Verdict: merge.

[wshallwshall]

Lander review

Reviewed the diff; branch updated to re-run a red that was not yours (see the last section). Will queue on green.

The defect is the one this repository keeps paying for

repo_stamp().freshness measures HEAD against the branch's own @{upstream}. On a pushed feature branch — which is how the record is normally edited — a clone reads CURRENT while missing every cell landed on the record line since the branch was cut. The writer then faithfully re-renders those cells back to their old values, and the freshness reading prints CURRENT while it happens.

That is an instrument answering a question adjacent to the one asked, and the fix asks the right one: commits reachable from origin/main and not from HEAD.

It does not overclaim. The comment is explicit that _freshness is not wrong — it returns the upstream it used precisely because the two are different questions, and choosing which to refuse on is the caller's job. Correcting a defect without libelling the thing next to it is the harder half.

The part I would keep above everything else here

``None`` when the question cannot be ASKED here -- no work tree, or no ``origin/main`` in this
clone -- and NEVER when it can be asked and the answer is zero. An unaskable question and a
measured zero are different claims and must not print the same string.

UNASKABLE (origin/main is not in this clone) and CURRENT with origin/main are different strings for different facts. A guard that cannot distinguish them fails silently green, which is the failure mode this whole item exists to close — and I hit that exact trap twice myself today, once with a regex that found nothing anywhere and once with a set-membership test whose control could not see a dropped side. Getting it right in the code is better than catching it in review.

The rest, checked

No network, and the reason is right. A query that fetches gets bypassed, and a bypassed guard is worse than none because its absence reads as nobody needing it.

Reusing scorecard._git is the correct call even though it is private. One read-only probe with a timeout and a standing prohibition on writes; a second copy is a second place for the no-network rule to rot. And the drift risk is covered — scorecard.py is mirrored byte-for-byte into the vault under tests/test_asvs_verifier_vault_contract.py, so the private name cannot move out from under this file silently.

The --allow-stale-clone help text now names both conditions rather than only the upstream one, so the escape hatch describes what it actually overrides.

The red was not yours

test (windows-2025, py3.14) failed on tests/test_load_runner.py::test_run_load_end_to_end_no_loss — a load test, with 57 unconfirmed sends and no ACK before connection close. That is load-sensitive by construction.

Your diff cannot reach it. Whole-word counts over the whole diff:

load 0   runner 0   intake 0   mllp 0   connection 0   ack 0
scorecard 19   <- control, so the search works

Main's own run at 06ae4586e passed the same suite, so it is not broadly broken either. I updated your branch, which re-runs CI and refreshes you against main at the same time — you were BEHIND regardless.

Stated no larger than it is: one occurrence, no measured rate. If it fails the same way again, that is a second data point about the load runner, not about this PR.

Verdict: merge.