Skip to content

Negative control for T-1502 — do not merge - #763

Closed
MongLong0214 wants to merge 4 commits into
mainfrom
nc-canonical-tamper
Closed

Negative control for T-1502 — do not merge#763
MongLong0214 wants to merge 4 commits into
mainfrom
nc-canonical-tamper

Conversation

@MongLong0214

Copy link
Copy Markdown
Owner

Not for merge. This is T-1502's negative control, and it is closed once it has gone red.

The control the ticket originally named — skipping artifact:manifest — cannot be performed from a pull request. That step is hard-coded in canonical-merge.yml, which is loaded from the default branch, and the workflow's source-only filter refuses any change under .github/workflows/. A negative control nobody can run is the defect #722 named, so the ticket now names one that can be run: this.

This branch is #762's canonical rebuild with dist/commitlore.mjs edited by hand and src/ left alone. The bundle digest moved 6e69cc37…879e17c8… with no source change behind it.

CI rebuilds from source and then runs git diff --exit-code -- dist/ installer/canonical-artifact.json. The check jobs must fail. If they pass, the bytes that land match the source that landed with them is not a property this repository actually checks, and #762 proves nothing.

MongLong0214 and others added 4 commits August 18, 2026 15:35
Pushing the v1.1.3 tag printed `notes mirror (origin) failed: spawnSync git
ETIMEDOUT`. Nothing was wrong with the transport, and nothing needed doing that
the rest of the line did not already say -- but the sentence names the call that
returned rather than the decision that was made, so it reads as git having
failed and sends whoever gets it to look at a remote that is fine.

The two seconds are this hook's. A notes mirror is auxiliary to a branch push,
so the budget exists to stop an offline push feeling stuck, and declining to
wait is the hook working. `spawnSync` had no way to say that, and the raw code
reached the operator unchanged.

The line now says whose budget ran out, and interpolates the value from
`PRE_PUSH_NOTES_SYNC_TIMEOUT_MS` so the sentence cannot drift from the constant
it describes. Every other failure detail is untouched -- a refused connection
still reports as a refused connection, because that one is about the transport.

This is #746 in a second hook: a message accurate about the mechanism and wrong
about the situation costs more than a vague one.

Limit: only ETIMEDOUT is rewritten, so a transport that hangs without setting that code still surfaces its raw detail
Blast: local
Undo: easy
Certainty: firm
Record-Id: r-etimeout
Provenance: authored
Verified: with dist unrebuilt the extended timeout test fails naming the real string it still matched, and passes after the rebuild with all six pre-push tests green; the assertions were first added to the wrong test in the same file and a deliberately impossible expectation still passed, which is how that was caught
CommitLore-Version: 2.0.0
`build:canonical` on the merged tree, so the commit that lands matches the source it lands with. The pull request carried source only, which is what a contributor on a host that cannot run a linux/amd64 Docker build can produce (#720).

Limit: this proves the bundle matches this tree; whether this tree is what a reviewer wants is what the pull request is for
Blast: system
Undo: easy
Certainty: firm
Record-Id: r-canonmerge761
Provenance: authored
Verified: artifact:verify passed against the regenerated manifest in the same job, before any credential was available to it
CommitLore-Version: 2.0.0
Not for merge. This exists to make T-1502's negative control performable, and
it is deleted once it has gone red.

The control the ticket first named -- skipping `artifact:manifest` -- cannot be
performed from a pull request: that step is hard-coded in a workflow loaded from
the default branch, and the source-only filter refuses changes under
`.github/workflows/`. A negative control nobody can run is the defect #722 named,
so it was replaced with this one.

`dist/commitlore.mjs` is edited by hand on top of the canonical rebuild and `src/`
is untouched. CI rebuilds from source and diffs, so the edit has to surface as a
failure. If it does not, "the bytes that land match the source that landed with
them" is not a property this repository checks.

Limit: this proves the diff catches a tampered bundle, not that every path into dist is covered
Blast: local
Undo: easy
Certainty: firm
Record-Id: r-ncontrol
Provenance: authored
Verified: the bundle digest moved from 6e69cc379584928a to 879e17c82c47fb14 with no change under src/
CommitLore-Version: 2.0.0
@github-actions

Copy link
Copy Markdown

CommitLore — record lint

Trailers: clean — 4 commits in origin/main..cfae0046ec4755fe5cccd7285a21e5be3a93682c
Active constraints: not read — commitlore: git log --follow accepts exactly one pathspec, so renames are not followed for 6 paths; query one path at a time to follow its rename chain (6 changed paths)

Trailer violations fail this check. Active constraints are informational — they are what the repository already decided, not a verdict on this PR.

@MongLong0214

Copy link
Copy Markdown
Owner Author

Went red, in the right place.

canonical artifact verified: fcd0832fed28ef33c18591a93d743c918b3af2eda7d311e7bc321d34b5921e8a
diff --git a/dist/commitlore.mjs b/dist/commitlore.mjs
index 1396641..714b9b4 100755
Binary files a/dist/commitlore.mjs and b/dist/commitlore.mjs differ
##[error]Process completed with exit code 1.

Both check (22.23.2) and check (24) failed: https://github.com/MongLong0214/commitlore/actions/runs/32108141891

artifact:verify passed, and that is the part worth recording. The manifest binds src/, and src/ was not touched — so a hand-edited bundle satisfies the manifest check completely and is caught only by the rebuild-and-diff after it. Without that diff, the bytes that land match the source that landed with them would not be a property this repository checks, and #762 would prove nothing.

Closing. T-1502's second acceptance criterion is observed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant