Skip to content

Read the git-stage gate before archive moves the file it lives in - #188

Merged
VeryComplexAndLongName merged 1 commit into
mainfrom
fix/git-stage-gate-before-archive
Sep 3, 2026
Merged

Read the git-stage gate before archive moves the file it lives in#188
VeryComplexAndLongName merged 1 commit into
mainfrom
fix/git-stage-gate-before-archive

Conversation

@VeryComplexAndLongName

Copy link
Copy Markdown
Owner

The defect

shouldRunGitStage reads openspec/changes/<name>/harness.json, and it ran after the archive stage had moved that directory to openspec/changes/archive/.

So the read always found nothing, always resolved to "not configured", and the git stage was always skipped. It could never have run at the end of a real chain, under any configuration.

The unit tests could not see it because mockArchiveSucceeds only returned success — nothing moved, so the file was still there when the gate read it. A mock that reports success without doing what success does certifies the wrong thing.

The fix

Read the gate immediately before archive runs — the last moment the file exists — and make the archive mock perform the move.

Verified by reverting the source fix and re-running:

FAIL  under agent-sufficient plus allowlist, runs git push -> pr create -> merge
AssertionError: expected +0 to be 1

Zero push calls without the fix; passes with it. Checked deliberately, because two tests found earlier today passed no matter what the code did.

A side effect worth naming: a malformed harness.json now fails the chain before archive rather than after — the better order for an irreversible step.

This sat exactly inside the boundary drawn in task 4.4a. The live run recorded under 4.4 verified push, pull request, checks and merge through the real git and gh; it explicitly did not verify the chain's own wiring, and that is where the defect was.

Two corrections to things I wrote

harness-config.test.ts matched fenced JSON blocks with a bare \n. On Windows HARNESS.md is checked out CRLF — 500 pairs, no bare LF — so the pattern found zero blocks and the assertion failed, while CI's Linux checkout found both and passed.

Green in CI and red on the machine of whoever edits the document is the worst of the two combinations, and this is the third line-ending or path-separator assumption found in tests today.

core-test-worker-contention said a single fork makes git.push.test.ts pass. A full packages/core run with that flag still fails it at the 5000 ms default. So "parallel workers contend" is too narrow — something accumulates over a long run regardless of a second worker. The two-file experiment behind that claim was too small to see it; the proposal now says so, and its first task is to name the cause rather than apply the workaround.

Also

harness-mechanical-checks 6.6 is closed by the repository owner's own live run: a chain on a change whose task declared `check(path-unchanged, README.md)` with a deliberate uncommitted edit reported the failing check by name, made zero agent calls, and cleared the checkbox; restoring the file and re-running marked it with no agent having claimed it.

agentic-harness-documentation/tasks.md is deliberately not in this PR — 6.7 and 3.4 are being worked on separately as this lands.

Test plan

  • harness-chain-runner.test.ts — 36/36
  • harness-config.test.ts — 87/87 (was 86/87 on Windows)
  • Fix reverted → the new test fails; restored → passes
  • npm run typecheck, npm run lint — clean
  • openspec change validate --strict — valid
  • CI green

🤖 Generated with Claude Code

shouldRunGitStage reads openspec/changes/<name>/harness.json, and it ran
after the archive stage had moved that directory to
openspec/changes/archive/. The read therefore always found nothing,
always resolved to "not configured", and the git stage was always
skipped. It could never have run at the end of a real chain, under any
configuration.

The unit tests could not see it because mockArchiveSucceeds only returned
success - nothing moved, so the file was still there when the gate read
it. A mock that reports success without doing what success does certifies
the wrong thing. The mock now performs the move, and the gate is read
immediately before archive runs, which is the last moment the file
exists.

Verified by reverting the source fix and re-running: the test fails with
zero push calls and passes with the fix. Checked deliberately, because
two tests found earlier today passed no matter what the code did.

A side effect worth naming: a malformed harness.json now fails the chain
before archive rather than after, which is the better order for an
irreversible step.

This sat exactly inside the boundary drawn yesterday in task 4.4a. The
live run recorded under 4.4 verified push, pull request, checks and merge
through the real git and gh; it did not verify the chain's own wiring,
and that is where the defect was.

Also in this commit, two corrections to things I wrote.

harness-config.test.ts matched fenced JSON blocks with a bare \n. On
Windows HARNESS.md is checked out CRLF - 500 pairs, no bare LF - so the
pattern found zero blocks and the assertion failed, while CI's Linux
checkout found both and passed. Green in CI and red on the machine of
whoever edits the document is the worst of the two, and it is the third
line-ending or path-separator assumption found in tests today.

core-test-worker-contention said a single fork makes git.push.test.ts
pass. A full packages/core run with that flag still fails it at the 5000
ms default, so "parallel workers contend" is too narrow: something
accumulates over a long run regardless. The two-file experiment behind
that claim was too small to see it, and the proposal now says so - its
first task is to name the cause rather than to apply the workaround.

harness-mechanical-checks 6.6 is closed by the repository owner's own
live run, recorded in that change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@VeryComplexAndLongName
VeryComplexAndLongName merged commit d161b50 into main Sep 3, 2026
7 checks passed
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