Skip to content

Add the git stage and mechanical verify checks, and fix four defects in them - #179

Merged
VeryComplexAndLongName merged 2 commits into
mainfrom
feat/git-stage-and-mechanical-checks
Sep 2, 2026
Merged

Add the git stage and mechanical verify checks, and fix four defects in them#179
VeryComplexAndLongName merged 2 commits into
mainfrom
feat/git-stage-and-mechanical-checks

Conversation

@VeryComplexAndLongName

Copy link
Copy Markdown
Owner

Three changes land together because they were implemented into one working tree and touch the same files: load-sensitive-test-timeouts (10/10), harness-mechanical-checks (29/31), agentic-harness-git-stage (20/23).

load-sensitive-test-timeouts — clean

Each multi-step vi.waitFor is sized against measured cost — ~453 ms isolated, ~1823 ms under deliberate co-load, ceiling 5000 ms — and the single-event waits are left alone, since an unnecessary ceiling is noise and noise is what makes the necessary ones unreadable. The full local suite is green for the first time this week.

Also removed two .tmp-load-sensitive-*.log files the run left in the repository root. Evidence belongs beside the number it justifies, not one git add -A from the history.

The other two arrived reported as finished, and were not

npm run typecheck failed with nine errors, none in core

Narrowing HarnessStepAgents to exclude archive is right — the stage is mechanical, an entry there configures nothing. But every consumer holding a plain HarnessStage still indexed the record: commands.ts, ai-panel.ts, websocket.ts, and five sites in HarnessSettingsView.tsx. core compiled while the other three packages did not, which is precisely what a green vitest run cannot tell you — vitest does not typecheck.

Fixed with isHarnessStepAgentStage and stepAgentFor in the type's own module. Keeping the archive case in one place is what stops the removal from becoming six subtly different guards.

HarnessSettingsView keeps archive in its stage list, as the change's own task 4.4 requires, and renders it as a row saying it runs mechanically — rather than offering controls that write a setting nothing reads. Same treatment commands.ts already gave it.

The git stage carried three defects — all behind the one unchecked task

Task 4.4, the live smoke test, was the only one left unchecked, and all three sit behind it. Not a coincidence, and not a formality.

1. gh pr create does not accept --json. Verified against the installed binary:

$ gh pr create --json number,url --head x --base main --fill
unknown flag: --json

createPullRequest threw every time; the stage could never open a pull request. It now reads the URL gh prints, taking the last line that is one, and treats no URL as an error rather than a guess.

2. SKIPPED counted as a failing check. Anything outside two hard-coded sets was read as a failure. From this repository's own PR #178:

[{"name":"Tag and release VS Code extension","state":"SKIPPED"},
 {"name":"Typecheck, lint, test, and build","state":"SUCCESS"},
 {"name":"Version pending changesets","state":"SKIPPED"}, ]

Two of seven, both conditional jobs that skip on every pull request by design. The gate would have refused every pull request this repository can produce. skipped, neutral and stale are now their own set — ran and decided nothing — and a failure names the state as well as the check.

A case the original had no answer for is now decided explicitly: every check skipped is a refusal, not a pass. Nothing exercised the change, and ADR 0014 already treats an absent result that way. waitForChecks returns it immediately rather than polling a decided answer into a timeout that would report something else.

3. The allowlist gated an invocation that was not the one executed. The stage built git push <remote> <branch>, checked that against the allowlist, audited that — then called GitWrapper.push(), a bare git push resolving both from the branch's upstream. Possibly a different target, and absent entirely on a branch that has never been pushed, which is every branch the stage creates.

The security model's premise is that the checked invocation is the executed one. push(remote, branch) now takes its target, and no --set-upstream either — an extra flag would reopen the same gap from the other side. The integration test no longer establishes an upstream first: it pushes a branch that has none, which is what the stage actually faces.

Also

The changeset named only @openspec-ui/core at patch, for the git stage alone, while both changes moved the public surface of three packages. Corrected to minor across core, webui and the extension.

Left open, deliberately

  • harness-mechanical-checks 6.3 — every existing tasks.md asserted against the real openspec/changes/ tree rather than a fixture. Not done, and worth doing: the parser is the one change here whose blast radius is every change in the repository, and a fixture cannot show that. Same shape as harness-config-strictness task 6.4, which passed for days while resolving a path one level above the repository.
  • harness-mechanical-checks 6.6 and agentic-harness-git-stage 4.4 — human-only.

Test plan

  • npm run typecheck0 (was 9 errors)
  • npm run lint — clean
  • npm run test — green across all four workspaces: core 44/490, extension 17/218, server 3/61, webui 35/230
  • openspec change validate --strict — all three valid
  • gh pr create --json and gh pr checks --json name,state verified against the real binary
  • CI green

🤖 Generated with Claude Code

…in them

Three changes land together because they were implemented into one
working tree and touch the same files: load-sensitive-test-timeouts,
harness-mechanical-checks, and agentic-harness-git-stage.

The first is clean. It sizes each multi-step vi.waitFor against measured
cost - about 453 ms isolated and 1823 ms under deliberate co-load, with a
5000 ms ceiling - and leaves the single-event waits alone, since an
unnecessary ceiling is noise and noise is what makes the necessary ones
unreadable. The full local suite is now green for the first time this
week. It also removed two .tmp log files the run had left in the
repository root; evidence belongs beside the number it justifies.

The other two arrived reported as finished and were not.

npm run typecheck failed with nine errors, none of them in core.
Narrowing HarnessStepAgents to exclude archive was right - the stage is
mechanical and an entry there configures nothing - but every consumer
holding a plain HarnessStage still indexed the record, and core compiling
while the other three packages did not is exactly what a green vitest run
cannot tell you. Fixed with isHarnessStepAgentStage and stepAgentFor in
the type's own module: keeping the archive case in one place is what stops
the removal from becoming six subtly different guards. The settings view
keeps archive in the stage list, as its own task 4.4 requires, and renders
it as a row saying it runs mechanically rather than offering controls the
validator would reject.

The git stage carried three defects, all of them behind task 4.4, the
live smoke test, the only task left unchecked. That was not a coincidence
and the check was not decoration.

gh pr create does not accept --json. The real binary answers "unknown
flag: --json" and exits non-zero, so createPullRequest threw every time
and the stage could never open a pull request. It now reads the URL gh
prints, taking the last line that is one, and treats no URL as an error
rather than a guess.

Every check state outside two hard-coded sets counted as a failure, and
SKIPPED was outside them. Two of the seven checks on this repository's own
PR #178 are SKIPPED - conditional jobs that skip on every pull request by
design - so the gate would have refused every pull request this repository
can produce. Skipped, neutral and stale are now their own set: ran and
decided nothing. A case the original had no answer for is now decided
explicitly - every check skipped is a refusal, not a pass, because nothing
exercised the change, and ADR 0014 already treats an absent result that
way.

The allowlist gated an invocation that was not the one executed. The
stage built git push <remote> <branch>, checked and audited that, then
called a bare git push, which resolves both from the branch's upstream -
possibly a different target, and absent entirely on a branch that has
never been pushed. GitWrapper.push now takes its remote and branch, and no
--set-upstream either, since an extra flag would reopen the same gap from
the other side. The integration test no longer establishes an upstream
first: it pushes a branch that has none, which is what the stage faces.

Also corrected the changeset, which named only core at patch, for the git
stage alone, while both changes moved the public surface of three
packages.

Two tasks stay open by intent. harness-mechanical-checks 6.3 wants every
existing tasks.md asserted against the real openspec/changes/ tree rather
than a fixture, which is not done and is worth doing - the parser is the
one change here whose blast radius is every change in the repository.
And 6.6 and the git stage's 4.4 are human-only.

Typecheck, lint and test green across all four workspaces.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`path-unchanged`'s escape test used `..\outside-the-workspace`. A
backslash is a separator only on Windows; on Linux it is an ordinary
filename character, so the path stayed inside the workspace, nothing was
refused, and the assertion that nothing spawned failed on the one runner
that could see it. The implementation was always correct - `path.resolve`
plus `checkCwdSandbox` care about neither platform's spelling.

Now `../outside-the-workspace`, which escapes on both, plus a second case
for a rooted path. `/etc/passwd` was picked over a drive-letter path
because `path.resolve` leaves it alone on POSIX and rewrites it to
`<drive>:\etc\passwd` on Windows, and both land outside a workspace under
a temporary directory. A `C:\...` fixture would have reproduced the
original mistake with the platforms exchanged: an ordinary relative name
on Linux, inside the workspace, legitimately reaching `git diff`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@VeryComplexAndLongName
VeryComplexAndLongName merged commit 366bb77 into main Sep 2, 2026
7 checks passed
VeryComplexAndLongName added a commit that referenced this pull request Sep 2, 2026
…l keys (#186)

* Add the harness guides, widen ACP capabilities, refuse stray top-level keys

Three changes land together, implemented into one working tree.

HARNESS.md and LIMITS.md exist now, and README.md contains the word
harness for the first time. The reference table has a row per agent id
read out of HARNESS_AGENT_CAPABILITIES rather than recalled, LIMITS.md
separates the chain-wide budget evaluated between stages from the
per-stage cap passed to one invocation, and says outright that no
wall-clock limit exists - the question that prompted the document assumed
one did. The standalone screenshots are generated by a Playwright spec
now, so the settings image can no longer show a screen that stopped
existing three commits ago, which is what the committed one did.

acp-agent-capabilities gives copilot-cli-acp and claude-cli-acp the
capabilities their plain counterparts have, because they run the same
binaries with the same flags. codex-cli-acp and gemini-cli-acp get
explicit empty entries, so an omission stops being spelled the same way
as a decision.

harness-config-top-level-keys refuses a top-level key the schema does not
define, which is what would have caught the harness.json that sat in this
repository doing nothing.

Two human-only tasks are now done rather than deferred.

The top-level-key check was verified by writing the deleted file
byte-for-byte and reading it through the real loader: it now answers
"unrecognized top-level key apply ... Did you mean stepAgents.apply?"
where it used to return silently.

The ACP capabilities claim was verified along the whole path rather than
at its ends. The entry resolves through a real config file, builds the
argv copilot --acp --effort high --max-ai-credits 30, passes the
allowlist, and the real binary spawned through cross-spawn with that argv
accepts the flags and waits on stdin. An unknown flag exits immediately -
which is how gh pr create --json was caught.

The standalone half of the documentation check is done against the
generated images: every control the document places is where it says,
archive shows no picker, and the checkpoint offers Continue and Cancel
and nothing else. The VS Code half cannot be done here and stays open.

Two new proposals come out of doing that check.

harness-git-stage-no-agent: stepAgents.git is accepted and offered by
both settings surfaces, and nothing reads it. HARNESS.md already
documents this honestly, but a defect described in prose with no tracked
change is how it becomes permanent. It is the same defect
harness-mechanical-checks removed for archive, missed because git was not
in CHAIN_STAGES when that change narrowed the type - both landed in #179.

core-test-worker-contention: git.push.test.ts takes 2.6 s alone and hangs
past 20 s beside one other worker, and both pass in 7.7 s under a single
fork. The cause is contention over real git subprocesses and temp
directories, not duration. Raising the ceiling to 20000 ms was tried and
produced a 20 s failure instead of a 5 s one. Two task notes had recorded
this as timeout flakiness and one had worked around it with a pool flag
without recognising the flag as the diagnosis.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Correct HARNESS.md: only one of the two hosts offers the git picker

Task 6.7 asks for the document to be read beside the running surface.
Doing that for VS Code, against the implementation rather than a screen,
found the document overstating a defect it had otherwise described well.

HARNESS.md said "both UIs offer an agent picker" for stepAgents.git. Only
the standalone one does. HARNESS_TEMPLATE_STAGES in commands.ts lists
propose, review, apply, verify and archive, and the wizard never asks
about git, so VS Code already behaves the way harness-git-stage-no-agent
proposes to make both hosts behave.

It is right by accident rather than by decision: git is absent from that
list because the list predates the git stage. That change's task 5.4 -
every stage in CHAIN_STAGES without an entry in CHAIN_STAGE_COMMAND is
excluded from HarnessStepAgentStage - is what turns the coincidence into
a property, and its task 3.2 now says to confirm the wizard rather than
edit it.

The rest of the VS Code column checks out: both command titles match
package.json exactly, and the wizard is the sequential Quick Pick the
document describes, offering effort and budget only where
HARNESS_AGENT_CAPABILITIES accepts them.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Scope the override screenshot to the section it is about

The second settings image was a full-page capture taken after loading the
per-change override, so it contained the whole global section as well -
a strict superset of the first image, presented in HARNESS.md as a
separate illustration of a different thing, and 2131 pixels tall for a
reader who only needed the half that differs. It is now scoped to the
override section: 36 KB instead of 394 KB.

`.last()` on the locator because the view nests one section inside
another and the filter matches the outer wrapper too.

Two further problems the images surfaced are left alone here, because
this change's own task 6.5 forbids touching non-test source, and both are
webui strings rather than screenshots.

The autonomy select reads "semi-autonomous (not yet implemented)" and
"autonomous (not yet implemented)". Both are implemented -
HarnessChainRunner has a test suite for each level, and the very next
screenshot in the document shows a semi-autonomous run paused at a
checkpoint. The document now carries a label contradicting the image
below it.

The panel's own description points at openspec/changes/agentic-harness/,
which was archived and no longer exists at that path.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Reconcile the guides with what shipped beside them

The ACP section said copilot-cli-acp and claude-cli-acp refuse effort and
budget, and pointed at acp-agent-capabilities as the change that would fix
it. That change is implemented in this same branch, so the section
described a state that no longer exists by the time anyone could read it.
HARNESS_AGENT_CAPABILITIES now carries both rows, and the document says so
instead. This is the reconciliation both changes asked whichever landed
second to perform.

The guide also claimed VS Code screenshots below were captured by hand and
labelled with their date. There are none - no image under
docs/images/extension is referenced anywhere in the file. A sentence
describing the provenance of images that do not exist is worse than
silence, because it reads as a guarantee. Replaced with the plain fact
that no VS Code capture exists yet and that adding one is the open
human-only task.

Both images are now links to their own full-resolution PNG with a caption
saying what to look at, and a navigation table sits at the top. The
per-change caption names the (not yet implemented) suffix visible in the
screenshot as stale UI copy and points at the section describing the
semi-autonomous behaviour that suffix denies - the label is wrong in the
product, and a caption is the only place this change is permitted to say
so, its own task 6.5 forbidding non-test source edits.

Verified before committing: no extension image is referenced anywhere in
the file, the two ACP rows exist in harness-step-agent.ts, and every
internal anchor the new navigation table introduces resolves to a heading
that exists.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@VeryComplexAndLongName
VeryComplexAndLongName deleted the feat/git-stage-and-mechanical-checks branch September 3, 2026 11:15
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