Fix Windows host resolution and batch spawning (#716) - #720
Conversation
|
The evidence here is the kind this issue asked for and did not have: a real machine, agents actually installed, before and after, disk readback, and an MCP Three things to settle before this merges. The first two are mechanical; the third is a finding. 1.
|
Next, while you still have the machineThe machine is the scarce part. Everything below needs it; nothing below can be settled from a Linux runner, and that is the whole reason this work is yours. Work on this branch. Update it from 1. Hermes — the finding that mattersBoth of your after-runs still show: {"host":"hermes","requested":true,"outcome":"failed","healthy":false,"detail":"Hermes setup failed"}#716 attributes that to the same cause you just fixed: "
Check the first from the source — it is a five-minute read. If that is it, fix it here. If it is the second, stop and report rather than expanding this PR. 2. claude-code detectionStill Say what you observe: does the enumeration check for an executable only, or is there a config fallback that did not fire? Do not change the behaviour on a guess — record the question with the evidence and it becomes decidable. 3. Re-run so the receipt says itAfter merging instead of the unattributable form. Your 0-byte finding is what closed that question; this makes the output say it without a comment thread. Paste the new JSON. Also worth one line: your first run had 4. Your PR body closes the wrong thingGitHub's parser reads Not yours
AcceptanceSame as before, and the first line still holds: a green CI run is not acceptance. For each item above the evidence is the machine's own output — the enumeration JSON, the config on disk, the process's stderr — pasted verbatim. A partial result with an honest list of what you could not settle is worth more than a claim that covers what you did not test. |
Correction to my own comment above, and what changed on the branchMy first hypothesis about Hermes was wrong, and I should have checked before offering it. I wrote that the Hermes step might have its own const status = commandStatus(options.wrapper, ['hermes', 'install', …], 30_000);
So the second branch is the one that holds: this is a third cause, unnamed, and cause 2 never covered it. #716's body attributing The next step is not a fix. Also unexplained: the gemini-cli first-run timeoutSame wiring, dead on the first attempt and alive on the second. This branch does not explain it. Whether a cold start genuinely exceeds 15s on that machine, or something is true only of the first run, decides whether the budget is wrong or something else is. "Run it twice" is not an answer a user can be given, so this needs either an explanation or an explicit statement that it is unexplained. What I changed here
Nothing about the Windows behaviour was judged by any of that. The live evidence on this pull request is still the only evidence for that path. Release orderThis is going out as 1.1.1, after 1.1.0 tags. It repairs behaviour that was broken rather than adding anything, so it is a patch. 1.1.0's notes state plainly that Windows host wiring does not work, which is true of 1.1.0 and stops being true when this lands — that is the release which gets to say so. |
Blocking: two trailer violations in
|
Review findings. Three, and the first one is the defect class this work exists to removeAn independent review of the spawn path came back. I checked each against the merged source before repeating it here — every code fact below is quoted from the branch, not relayed. 1. Blocker — the installer can report
|
Addendum: finding 2 is a regression on the platform CI runs, and nothing here can fail on itI checked whether any existing test would catch the missing So a non-executable shadow earlier in That matters more than the other two findings, because this one is not Windows-only. It regresses macOS and Linux, which CI does run, and CI still cannot see it. The green Which also means it is the one finding a POSIX runner can be made to fail on. Concretely: Before the fix that test fails, because the resolver returns the first file it finds. After it passes. That is the shape #717 used for the temporary-filename defect and the reason it holds: a guard that cannot fail where the code runs is decoration.
Adding to the punchlist: Items 1 and 3 still need the machine to confirm the behaviour end to end. Item 2 does not — and it is the one currently breaking something that works. |
* Release 1.1.0 A machine can differ from the committed capture policy without modifying it. `.commitlore-policy.local.json` wins per key over `.commitlore-policy.json`, and `commitlore auto on|off --local` writes it, so a contributor who needs a different answer no longer has to leave a tracked file permanently modified -- the failure #709 reported was a release script refusing to tag a worktree dirtied exactly that way. Also here: doctor stops telling a repository that a record was lost when nothing ever reached staging (#710); both installers carry only the code that runs, 845 lines of unreachable shell and PowerShell removed (#691); and one of the two Windows host-wiring defects is fixed, with every host failure now naming the file it read (#716). Nineteen version surfaces move together: three manifests, five install pins in each of four READMEs, and three header examples in each installer. Two lines in README.md do not move -- "One installed before v1.0.2" and "Hooks installed from v1.0.2 onward follow upgrades on their own" are statements about a release boundary, and bumping them makes the README say something false. `test/readme.test.ts` reads only the install shapes, so nothing would have caught that. The comment on REQUIRED_CHECKS gains its second half. It said why `lint` is excluded from the tag gate and stopped there, which reads as a hole: a reader on 2026-08-17 asked whether main could be pushed unlinted. It cannot -- `lint` is one of the eleven required status checks on main's branch protection, evaluated on the pull request head. The exclusion is about which contexts survive a squash onto main, not about whether the job ran. Included in this commit because it is documentation of the gate this commit is about to be judged by. Limit: this release does not make host wiring work on Windows -- detection still cannot see a .cmd and spawn still cannot run one (#716) Limit: 1.0.0 through 1.0.2 have no CHANGELOG entries; a pointer to the releases page stands in rather than reconstructing them Ruled-out: fold the second Windows cause into this release | it arrived as #720 with real Windows evidence and needs its own judgement, and holding this back would make the note about what is still broken false in both directions Blast: system Undo: easy Certainty: firm Record-Id: r-rel110 Provenance: authored Verified: 95 cases pass across manifest, readme and release-publish-prerequisites -- the last two caught install.sh and install.ps1 documenting themselves at 1.0.2, which the README bump alone had missed; the bump was dry-run on a scratch copy first and left both README.md prose lines untouched; dist needs no rebuild because the CLI reads its version at runtime CommitLore-Version: 2.0.0 * Regenerate the canonical manifest the version bump moved `artifact:verify` rejected the release commit with `source checksum does not match this checkout`. The manifest binds four source inputs -- `package-lock.json`, `package.json`, `tsconfig.json`, `src` -- and the version bump changes one of them, so a release commit always moves the digest even when nothing about the build does. `dist/commitlore.mjs` is byte-identical, and that is the distinction worth keeping. The bundle comes from `src/`, which this release does not touch, and the CLI reads its version at runtime -- so "the release needs no dist rebuild" is true of the artifact and false of the manifest that describes it. The two get conflated because they live in the same commit and are regenerated by the same command. Caught by CI rather than by the release checklist, which is the wrong way round for a step every release has to take. Limit: this is the release commit's own repair, not a fix -- the next release will need the same regeneration for the same reason Blast: module Undo: easy Certainty: firm Record-Id: r-relmanifest110 Provenance: authored Verified: build:canonical then artifact:verify -> 2dcf0c4aeca193839f284c1b4fa5da57db8e29764ee02a36dad40d6768910101, and `git status` after the rebuild shows installer/canonical-artifact.json alone -- dist/ is unchanged by the bump CommitLore-Version: 2.0.0 * Bump the fourth manifest the release gate reads `check-release-version` refused the tag: `package-lock.json` carries the version twice -- `.version` and `.packages[""].version` -- and the release commit moved the other three manifests without it. The gate names both disagreements, which is how this was found rather than at the tag. The bump is structural, not textual. A `"version": "1.0.2"` replacement across the file matches five times, and three of those are dependencies genuinely at 1.0.2 -- `call-bind-apply-helpers`, `side-channel-weakmap`, `wrappy`. Declaring them 1.1.0 corrupts what `npm ci` resolves, and the version tests would still have passed, because they only read the package's own two fields. Parsing the JSON and setting exactly those two is the operation that cannot reach the others. `package-lock.json` is also one of the four inputs to the canonical source checksum, so the manifest moves again with it. `dist/commitlore.mjs` is still byte-identical: the lockfile records the same dependency tree it did before. Limit: nineteen version surfaces was already wrong before this -- the lockfile makes it twenty-one, and the count is only ever known after the gate says so Ruled-out: replace the version string throughout the lockfile | it matches four dependencies that are really at 1.0.2, and nothing in the suite would have caught it Blast: module Undo: easy Certainty: firm Record-Id: r-rellock110 Provenance: authored Verified: the diff is exactly two lines, both under the root package entry, with no node_modules path touched; npm ci resolves the tree unchanged; 104 cases pass across check-release-version, manifest, readme and release-publish-prerequisites; build:canonical then artifact:verify -> 2dcf0c4aeca193839f284c1b4fa5da57db8e29764ee02a36dad40d6768910101, unchanged from before the bump CommitLore-Version: 2.0.0
Item 2's guard belongs in the fixture helper, not beside itSharpening what I wrote above. The gap is not that this one case is unwritten — it is that the helper cannot express it: // test/installer-hosts.test.ts:28
const wrapper = (root: string, name = 'commitlore'): string => {
…
chmodSync(path, 0o755); // hard-coded, no way to ask for anything else
return path;
};Every fixture this produces is executable. So no defect involving executability can ever be caught by this suite, and the missing If the fix adds only the one fixture — const wrapper = (root: string, name = 'commitlore', mode = 0o755): string => …Then the assertion is the ordinary shape rather than an exception to it. This is the same question as the rest of the punchlist, asked about the tests instead of the code: what is the green a statement about? For this branch it is not a statement about executable resolution, because nothing in the suite can produce a file that is not executable. |
Windows lookup examined only bare PATH entries, so it missed .cmd host shims, while shell-disabled spawning could not execute those shims. The installer now resolves one PATHEXT candidate for both detection and launching, and invokes cmd.exe with an explicit, checked argv body for batch files. This preserves shell:false and prevents wrapper or configured paths from becoming an unchecked shell command line. Limit: Does not change MCP probing or rebuild generated artifacts. Ruled-out: shell: true | wrapper and config paths must not become unchecked shell input. Blast: module Undo: easy Certainty: firm Record-Id: r-winexec716 Provenance: authored Verified: npx tsc --noEmit; installer tests 24 passed; real Windows installer readback reported a healthy Codex registration. CommitLore-Version: 2.0.0
The branch deliberately shipped `src/` and `test/` only. CI binds the committed bundle to source by rebuilding twice in `docker linux/amd64 node:24-bookworm` and comparing bytes, and a Windows machine cannot produce that build -- so the artifact had to be added by a host that can, rather than guessed at or skipped. Also brings `main` in. `installer-hosts.ts` merged without conflict: the path-naming change and the execution-path change touch different sites, and the merged file carries both plus the temporary-filename fix underneath. Limit: this adds no judgement about the Windows behaviour -- it makes the branch buildable, and the live evidence on the pull request is still the only evidence for that path Blast: module Undo: easy Certainty: firm Record-Id: r-canon720 Provenance: authored Verified: 26 cases pass across the five installer suites; npx tsc --noEmit clean; the merged source carries atomicTemporaryName, the path-leading failure messages and the PATHEXT resolver together; build:canonical then artifact:verify -> 173d5ca2100676300bb5b94f8c600b482a93313b2e4a4e8ed5a1dd4fb95218c4 CommitLore-Version: 2.0.0
The first resolver fixed .cmd discovery but embedded every value in the cmd.exe program text. A data root ending in a backslash merged the final --verify token, and percent-bearing paths were refused, so verification could be skipped or a legal user path could fail. The same resolver treated any regular POSIX file as executable, letting a stale shadow mask the real host CLI. Carry user values through one-use environment references, double trailing backslashes for the downstream argv parser, require X_OK, and propagate the first command failure line. Windows tests now capture the exact batch argv and Hermes stderr, while the Hermes path assertion parses YAML semantically on Windows. Limit: Does not change executable-only Claude detection or the pre-existing MCP probe shell path; a new canonical dist rebuild is not included from Windows Ruled-out: shell: true | wrapper and user paths would become a shell injection surface Blast: module Undo: easy Certainty: firm Record-Id: r-winargv716 Provenance: authored Verified: npx tsc --noEmit clean; cca98cc merged a trailing data root with --verify; current Windows argv and Hermes E2E passed; 44 installer and Hermes cases passed across the current source bundle and source suites; real enumeration reported Hermes installed healthy and preserved the Cursor failure CommitLore-Version: 2.0.0
3ba609e to
6a98a70
Compare
|
Addressed the full updated punchlist and force-pushed with a lease check. Commit records
Independent review findings
The same E2E also delivered Hermes findingThere was no second missed spawn site. The previous temporary test package omitted shipped Adding only that shipped directory made the identical command exit 0. Correct-layout current source now reports Hermes installed/healthy and writes the real config entry. Final real-machine receipt{"schema":"commitlore_installer_hosts.v1","runtimeIdentity":{"version":"1.0.2","entrypoint":"C:\\Users\\u\\AppData\\Local\\Temp\\commitlore-716-e2e-final\\dist\\commitlore.mjs","packageRoot":"C:\\Users\\u\\AppData\\Local\\Temp\\commitlore-716-e2e-final","indexSchemaVersion":4},"ok":false,"hosts":[{"host":"codex","requested":true,"outcome":"custom-preserved","healthy":true,"detail":"healthy custom registration preserved; plugin installed"},{"host":"gemini-cli","requested":true,"outcome":"custom-preserved","healthy":true,"detail":"healthy custom registration preserved"},{"host":"cursor","requested":true,"outcome":"failed","healthy":false,"detail":"C:\\Users\\u\\.cursor\\mcp.json is not parseable JSON: Unexpected end of JSON input"},{"host":"hermes","requested":true,"outcome":"installed","healthy":true,"detail":"Hermes setup verified"}],"notDetected":["windsurf","opencode","claude-code"]}Disk readback agrees:
Gemini's initial 15-second timeout did not reproduce in four corrected-layout runs: 4/4 healthy. It remains a one-off observation, not an explained failure. Verification: |
# Conflicts: # installer/canonical-artifact.json
|
Follow-up after main advanced with #721:
I am monitoring new comments and checks. |
|
Post-approval CI result:
No new code or history failure surfaced. The remaining merge gate is the owner/Linux canonical rebuild followed by the check rerun. |
The branch carries the review fixes -- trailing backslashes doubled, executable resolution gated on X_OK, the percent handling that survives one-pass expansion -- and the committed bundle still described the source before them. CI compares the committed dist against two `docker linux/amd64 node:24-bookworm` builds and against the manifest, so the branch could not go green without a build from a host that can produce one. The 1.1.0 release moved through here in between: `package.json` is one of the four canonical source inputs, so the manifest digest moved for the version bump as well as for the source change. Resolving that conflict to main's copy on the Windows side was right -- the alternative is generating a Linux artifact on a machine that cannot, which is a guess wearing a checksum. Limit: this rebuilds, it does not judge -- the Windows behaviour still rests on the live evidence recorded on the pull request, and Hermes and the first-run probe timeout are still open on MongLong0214#716 Blast: module Undo: easy Certainty: firm Record-Id: r-canon720b Provenance: authored Verified: build:canonical then artifact:verify -> 14ff61683da747382e03b026db252dd06d4f20820c3ab6f6a75282837458429d; 27 cases pass across the five installer suites, including the POSIX-only case that places a mode-0644 shadow ahead of a mode-0755 CLI and requires the latter CommitLore-Version: 2.0.0
Canonical rebuild is in, and CI is approved and running
I also approved the two fork-workflow runs. Read this next part before your next push, because it will keep costing you round trips otherwise: a fork PR's runs land as Resolving the manifest conflict to main's 1.1.0 copy was the right call. Generating a Linux artifact on Windows would have been a guess wearing a checksum, and the check that compares it would have caught it — but only after costing another cycle. Your five fixes, checked against the sourceI read each in the branch rather than taking the summary:
Two of these are worth naming for what they did rather than what they fixed. The percent handling. You did not implement the suggestion. You tested it, found
And the X_OK test is the one that had to exist: a mode-0644 shadow ahead of a mode-0755 CLI, running on POSIX. That regression would have been invisible to every runner this project has — the fixtures are all executable — so without it a future revert would be silent. Still open on #716, and not blocking this PR
Both need the machine, and both are worth a re-run once this is green — against v1.1.0, which is now tagged, so the observation is quotable rather than being about a moving One small thing left from my earlier note, not blocking: |
What closing this out actually requiresFour issues are open. Two of them can only be closed from your machine, and this comment is the place we coordinate that — so here is each one with the evidence that closes it, rather than a list of tasks. #716 — two of its threads are answered, two are notAnswered and not worth re-litigating:
Open, and both need the machine: 1. Hermes — a third cause, currently unnamed. It runs through What closes it: one run on v1.1.1 with the stderr pasted verbatim. Then either it is ours and we fix it, or it is a Hermes-side condition and #716 records that and closes. Both outcomes close the thread; only silence does not. 2. gemini-cli first-run timeout. Same wiring, dead on the first attempt and alive on the second. What closes it: does it reproduce on a fresh install? If yes, the 15s budget is wrong for a cold start on that hardware and we have a number to raise it to. If it does not reproduce, say so and it is recorded as a one-off — which is a worse answer but an honest one. "Run it twice" is not something we can tell a user, so the one thing that cannot happen is leaving it undescribed. #714 — closes on one readback, not on a fixThat issue exists because host wiring had never been observed working on Windows. It is currently titled Verified, and it fails. When a run on v1.1.1 shows at least one host #722 — the one that stops this recurringFiled just now: plant a fake Not yours unless you want it: it is a workflow change, and #719 — stays open on purposeThe committed SequenceRun against the tag, not I will approve the fork workflow runs after each push — they land as |
Green on all eleven required checks — and
|
|
@MongLong0214 I addressed the remaining fixture-helper note in
Please approve the new fork workflow run when it appears. On the two machine-only follow-ups: I agree the quotable closure must be the released v1.1.1 tag. For clarity before that run, the corrected-layout current-source run already showed Hermes |
|
@MongLong0214 blocking release observation: the merge did not use the merge-commit path described in your last comment. Runtime/API evidence: So the state is the failure your comment intended to avoid: the six branch commits were collapsed, and the four records that preserve reconstructed were discarded with the runner. This is not caused by the Windows fix, but it is real repository state, not just a red optional check. The source branch history is still recoverable from Also, |
|
Recovery confirmed from the upstream ref, so my release block above is cleared: I fetched the upstream notes ref independently and read the target note. It contains all four inherited blocks with their source SHAs, so the runner loss has been repaired and the pre-release record state is now sound. v1.1.1 is not visible yet; I am ready to start the tag-pinned Windows install as soon as it appears. |
|
Released-tag closure receipt: Full JSON and paired disk evidence: #716 (comment) |
|
Correcting the merge commit for this PR.
That is wrong, and #726 has the right account: hermes works. The trailing backslash was the hermes failure — How I got it wrong: the PR body carries three JSON captures and hermes reads A merge commit cannot be edited, so this comment is the correction of record — anyone reading |
* Let a fork's records survive their own squash (#723) `preserve` ran on `pull_request`, which hands a fork's workflow a read-only token. So the job read the branch's commits, built the inherited note correctly, and could not publish it: merge-type=squash records=4 pushed=false → the record was attached on the runner and is about to be discarded with it That is what happened to #720. Four records were lost at the merge and recovered by hand afterwards, in the repository whose whole claim is that records survive a squash. `contents: write` was already declared; it cannot grant what the event does not carry. `pull_request_target` carries it. That event is better known for handing an attacker a write token, and the mechanism is specific: it checks out the base by default, and the hole opens when a workflow points the checkout at the fork's head and then runs it. This job never has. It checks out `base.ref` -- for an unrelated reason, because `refs/pull/<n>/merge` is gone once the pull request closes -- installs and builds from that tree, and brings the fork's commits in as `refs/commitlore/pr-head`, which is read for trailer blocks and nothing else. Data, never scripts. The change is one line; what makes it safe was already true. Being already true is not the same as staying true, so the two rules are asserted rather than requested: the checkout must name the base branch, and every command mentioning the fork ref must be a `git fetch`. Reverting the event, pointing the checkout at the head, or running anything out of that ref each fail a named case. Limit: this runs on close, so it publishes rather than gates -- a merge that could not carry its records still happens first, and the report is the only thing standing there Limit: the fetched ref is trusted as data by every consumer downstream; a future step that treats a commit message as a command would reopen this by a different door Ruled-out: make preserve a required check | it runs on the close event, so it cannot be a merge gate at all Ruled-out: a maintainer runs squash-preserve after each fork merge | it is what I did for #720, and a manual step nobody is reminded of fails on the fifth occurrence rather than the first Blast: system Undo: easy Certainty: firm Record-Id: r-forkpreserve723 Provenance: authored Verified: three mutations, each caught by the case whose name claims it -- reverting to `pull_request` fails the trigger case; pointing the checkout at `head.sha` fails the base-branch case; adding `git checkout refs/commitlore/pr-head -- .` before `npm ci` fails three, including the one asserting every mention of that ref is a fetch. Six cases pass on the file as it stands; the assertions read the workflow with comment lines stripped, so a comment cannot satisfy them. CommitLore-Version: 2.0.0 * Let the dogfooding case read the trigger that is there `action-preserve` asserted `on.pull_request.types` and #723 moved the workflow to `pull_request_target`, so the case died on `Cannot read properties of undefined` rather than on anything it was checking. What it was checking is unchanged: the job fires on the closed event and nothing else. It now reads whichever key carries that. Which key it should be, and the two rules that keep the stronger event safe, are asserted in `test/preserve-workflow-safety.test.ts` -- a property belongs in one place, and that place is the file named after it. Limit: this is the third guard this week whose subject moved out from under it; the pattern is a case pinned to a spelling rather than to the thing spelled Blast: local Undo: easy Certainty: firm Record-Id: r-trigkey723 Provenance: authored Verified: 37 cases pass across action-preserve and preserve-workflow-safety together; before this the check jobs failed with `Cannot read properties of undefined (reading 'types')`, which named the spelling and not the property CommitLore-Version: 2.0.0
…731) * Plant a host in Windows CI so the wiring branch actually runs (#722) A GitHub runner has no coding agents, so detection short-circuits and the Windows job has printed `"hosts":[]` on every run it has ever made. Every branch that wires a host is unexecuted there -- not under-tested, unreachable -- and an assertion reading that list passes whether the wiring code works, is broken, or has been deleted. That vacancy is why #714 existed as a standing record rather than a task, and why both Windows defects in #716 were found by a person on a real machine instead of by any job here. The response until now was to make the guards falsifiable somewhere the code does run: #717 pinned a temporary-filename property as a pure function, #720 added a mode-0644 shadow for executable resolution. That works and it is second best, because it proves properties of helpers rather than executing the path. A planted `cursor.cmd` removes the vacancy instead of working around it. Detection looks for the command on PATH, and nothing here executes Cursor -- Cursor plays no part in wiring, since what gets written is its config and what gets probed is CommitLore's own wrapper. So the fixture is a file, and the branch it unlocks is the real one, end to end: detection, the atomic config write, and a live MCP initialize. Two cases, and the second is what makes the first mean anything. A fixture that can only succeed proves the installer reports what the fixture told it, which is the empty runner again with more steps. So the second gives the same detected host a config it cannot parse and requires `outcome: "failed"`, `ok: false`, the file named in the reason, and the unusable config unmodified afterwards. The suggestion and its framing -- so this class cannot hide behind an empty runner again -- came from kantorcodes1 in a public discussion. It is a better answer than the one I was giving. Limit: one host of seven, on one platform. gemini-cli, windsurf and opencode take the same JSON path and are not planted; codex, hermes and claude-code take different ones and are not covered at all Limit: this executes wiring, not a real agent -- a planted shim is a file the job wrote, and it says nothing about what Cursor itself does with the registration Ruled-out: assert on the existing `"hosts":[]` output | it is the same on a working installer and a deleted one, which is the defect this replaces Ruled-out: a stub that always registers cleanly | it proves the installer reports what the stub told it, and cannot fail, so it restores the vacancy under a new name Blast: system Undo: easy Certainty: firm Verified: the workflow parses as YAML and the job gains one step; `EXPECTED_CI_WORKFLOW_SHA256` is re-locked to the edited file, which is the mechanism that would otherwise let a workflow body change unreviewed, and 29 release-prerequisite cases pass against it. The step itself cannot be run locally -- it needs windows-latest, which is the whole point -- so its first execution is this pull request's own `install-ps1` job. Record-Id: r-plantedhost722 Provenance: authored CommitLore-Version: 2.0.0 * Assert the exit code the planted host was already producing The step's first run did everything it claimed. On a GitHub runner, with no coding agent installed, the enumeration wired a host and live-verified it: ok:true cursor installed healthy "registration created and live-verified" ok:false cursor failed "D:\a\_temp\cl-host-bad\.cursor\mcp.json is not parseable JSON..." The job still failed, and the reason is worth keeping rather than papering over. GitHub's pwsh shell exits with the last native command's status, and the second case deliberately produces a failing one -- so the step was reporting, accurately, that a host had failed. That is a property nothing asserted. `install.ps1` returns this command's status, so a host that failed must make it non-zero or an install reports success over a repository with nothing wired -- the exact claim the installer is for. Both codes are now checked: zero when the host wired, non-zero when it did not, and an explicit `exit 0` at the end because every assertion throws before reaching it. Limit: the second case's non-zero code is asserted, not its value -- a change from 1 to 2 passes here Blast: module Undo: easy Certainty: firm Record-Id: r-plantedexit722 Provenance: authored Verified: the run this repairs is the evidence -- both enumerations are in its log with the outcomes the step requires, and the only thing that failed was the shell's exit status; the workflow parses, the digest is re-locked to it, and 29 release-prerequisite cases pass CommitLore-Version: 2.0.0
The first shape of this let a wrong commit reach `main` and opened a second pull request to fix it. That does not meet what T-1502 asks for -- "a source-only pull request merges and the commit that lands on `main` passes `artifact:verify` and `git diff --exit-code -- dist/` without anyone rebuilding by hand" -- because the commit that lands is the wrong one, briefly, every time. It was also building a file T-1502 owns while T-1501 puts "any workflow file" out of scope. So it builds the landing commit instead: `main` plus the pull request's source plus a canonical rebuild, on one branch, opened as one pull request. All eleven required contexts run on the tree that actually merges rather than on one that resembles it, and a contributor who cannot produce a `linux/amd64` Docker build waits for nothing (#720). Three things the review of the first shape found, each fixed here rather than argued with. The concurrency group orders jobs, not merges. My own comment claimed it removed a race and then described that race: a rebuild is of `main` as it stood when the job started, and nothing stops another pull request landing while it runs. There is now a check between the rebuild and the push that refuses when `main` moved. A rebuild branch cannot be updated, only replaced. `main`'s protection is `strict`, and `.gitattributes` marks `dist/**` as `-merge`, so GitHub's "Update branch" conflicts on the one file the job exists to produce. The branch is keyed to the pull request rather than to a commit, and a rerun force-pushes it. And the guard's own reading was wrong on merge commits. `git show --name-only` prints a merge's combined diff -- only paths differing from every parent -- so a clean merge reports nothing at all. `git show --first-parent` reports seven paths for `5dcc02b` where the plain form reports zero. The suggested replacement, `diff-tree --first-parent`, also reports zero; it was measured rather than adopted. The safety property with no precedent here is the order of two steps. The rebuild runs `npm ci` and `build:canonical` on the merged tree, which executes whatever the pull request put in `package.json`. The App token is minted after that, and is not in that step's environment: a rebuild of somebody else's change must not be able to read the credential that lets it push. `test/canonical-merge-workflow.test.ts` asserts the ordering, with comments stripped so the explanation cannot satisfy it. Limit: this leaves pull requests still carrying `dist/` when they choose to -- T-1503 is what stops requiring it, and until then a source-only pull request is a contributor's option rather than the rule Blast: system Undo: easy Certainty: firm Record-Id: r-canonmergewf Provenance: authored Verified: two negative controls fail the intended assertion and pass after restore -- moving the token mint above the rebuild, and deleting the source-only refusal; 79 tests across the five workflow suites pass, and the guard's `--first-parent` reading was checked against this repository's real merge commit rather than a constructed one CommitLore-Version: 2.0.0
…anual T-1502 asks that "a source-only pull request merges and the commit that lands on `main` passes ... without anyone rebuilding by hand". This job opens a second pull request, so the first half looked unmet -- and it is met by what lands rather than by what is clicked. The branch merges the contributor's head with `--no-ff`, so their commit is an ancestor of it. Merged with a merge commit, that commit lands on `main` and GitHub closes their pull request as merged. Squashed, new bytes land instead and their pull request stays open pointing at nothing. This repository allows both, so the method is part of the contract rather than a preference, and the bot now says so in the body it writes. A test pins both halves: the `--no-ff` and the instruction. The reason this is dispatched by hand also changed, and saying so matters more than the trigger does. It was security -- an automatic trigger would let a fork's push decide when the App token sits in an environment building that fork's code. The job split removed that: the token is never in the job that runs contributor code. What is left is cost, one Docker `npm ci` and a canonical build per dispatch, which a pull-request trigger would run on every push from anybody. Leaving the old reason in place would have been the worse outcome. A guard whose stated reason has been fixed elsewhere is one somebody removes later on the grounds that the reason no longer holds, without noticing it acquired a different one. And `without a maintainer` in PRD-F15's success line means without a maintainer *rebuilding* -- what #720 waited on twice. Nobody rebuilds here. Limit: the merge method is stated and asserted in the body, not enforced -- a squash merge of a canonical pull request still works, it just leaves the contributor's pull request open, and nothing fails when somebody does it Blast: system Undo: easy Certainty: firm Record-Id: r-mergemethod Provenance: authored Verified: `allow_merge_commit` is true on this repository, so the method is a choice rather than a constraint; replacing `--no-ff` with `--squash` fails the new assertion, and 15 of 15 pass restored CommitLore-Version: 2.0.0
Summary
Refs #716. This addresses cause 2 and the independent-review blockers without enabling
shell: true:PATHEXTon Windows and requireX_OKon POSIX, so a non-executable shadow cannot mask a later real CLI;.cmd/.batthrough explicitcmd.exeargv whose/ctext contains only random one-use environment references, never wrapper or user paths;%,&,|,^,!, spaces, and trailing backslashes, including a final--verifytoken after a data-root ending in\\;The branch includes merged #717, #718, and the 1.1.0 release from #721. The prior owner-built
dist/remains untouched and the latest main manifest/version metadata wins the merge. This Windows follow-up changessrc/andtest/only, so the Linux canonical builder must refreshdist/andinstaller/canonical-artifact.jsonagain before merge.Real Windows evidence
Environment: Windows
10.0.19045.0, PowerShell5.1.19041.6456, Nodev24.12.0.Machine scope actually observed:
codex.exe,codex.cmd,codex.ps1;codex-cli 0.147.0.%USERPROFILE%\\.claude.jsonexists, but noclaudeexecutable exists. Current enumeration is executable-only, so it remainsnotDetectedby design pending a product decision.Before: owner observation on v1.0.2
{"schema":"commitlore_installer_hosts.v1","runtimeIdentity":{"version":"1.0.2","entrypoint":"C:\\Users\\u\\AppData\\Local\\commitlore\\v1.0.2\\dist\\commitlore.mjs","packageRoot":"C:\\Users\\u\\AppData\\Local\\commitlore\\v1.0.2","indexSchemaVersion":4},"ok":false,"hosts":[{"host":"codex","requested":true,"outcome":"failed","healthy":false,"detail":"codex mcp add failed"},{"host":"gemini-cli","requested":true,"outcome":"failed","healthy":false,"detail":"atomic config write failed: ENOENT: no such file or directory, open 'C:\\Users\\u\\.gemini\\.C:\\Users\\u\\.gemini\\settings.json.commitlore-41756-3e1a1692-8aec-445c-be59-5e90a8be2cd0.tmp'"},{"host":"cursor","requested":true,"outcome":"failed","healthy":false,"detail":"config is not parseable JSON: Unexpected end of JSON input"},{"host":"hermes","requested":true,"outcome":"failed","healthy":false,"detail":"Hermes setup failed"}],"notDetected":["windsurf","opencode","claude-code"]}After: current source, correct package layout
Command:
Exit:
1, solely because the real Cursor target is still an empty, invalid JSON file.{"schema":"commitlore_installer_hosts.v1","runtimeIdentity":{"version":"1.0.2","entrypoint":"C:\\Users\\u\\AppData\\Local\\Temp\\commitlore-716-e2e-final\\dist\\commitlore.mjs","packageRoot":"C:\\Users\\u\\AppData\\Local\\Temp\\commitlore-716-e2e-final","indexSchemaVersion":4},"ok":false,"hosts":[{"host":"codex","requested":true,"outcome":"custom-preserved","healthy":true,"detail":"healthy custom registration preserved; plugin installed"},{"host":"gemini-cli","requested":true,"outcome":"custom-preserved","healthy":true,"detail":"healthy custom registration preserved"},{"host":"cursor","requested":true,"outcome":"failed","healthy":false,"detail":"C:\\Users\\u\\.cursor\\mcp.json is not parseable JSON: Unexpected end of JSON input"},{"host":"hermes","requested":true,"outcome":"installed","healthy":true,"detail":"Hermes setup verified"}],"notDetected":["windsurf","opencode","claude-code"]}This is intentionally not presented as a seven-host green run. Cursor stays
failed; Windsurf, OpenCode, and Claude Code stay innotDetected.Direct disk readback after the final run
custom-preserved, healthy%USERPROFILE%\\.codex\\config.toml:commitlore.cmd,args = ["mcp"]custom-preserved, healthy%USERPROFILE%\\.gemini\\settings.json: same registeredcommitlore.cmd,["mcp"]failed%USERPROFILE%\\.cursor\\mcp.jsonremained 0 bytes, SHA-256E3B0...B855; no entryinstalled, healthy%USERPROFILE%\\.hermes\\config.yaml: current stagedcommitlore.cmd,[mcp], enabled, and shipped skills directorynotDetected%USERPROFILE%\\.claude.jsonremained byte-identical; no entryCodex, Gemini, Cursor, Hermes, and Claude config SHA-256 values were identical immediately before and after this final idempotent run. Hermes had already been written by the preceding correct-layout run; this final run verified it without another mutation.
Independent-review red ? green evidence
cca98cc, expected["...\\data root\\", "--verify"]; the child actually received one argument:"...\\data root\\\" --verify". Current code captures the two exact arguments separately.cmd.exe /cproved naive%%is not exact:%%NAME%%became%EXPANDED%. One-use environment expansion is one-pass; the real batch child received literal100% Readyand%PATH%.A&B,A|B,A^B,!PATH!, and a space-bearing path exactly.missing Hermes skills sentinelon stderr now producesHermes setup failed: missing Hermes skills sentinel.claudebefore mode-0755claude; the resolver must select the latter.codex.CMD; its name no longer claims a condition the fixture did not establish.Hermes and Gemini findings
The earlier Hermes failure was not another missed spawn site. The temporary source stage omitted the shipped
hermes/skillsdirectory. With that directory absent, the same current bundle exited 2 with:Adding only the shipped directory made the identical command exit 0 and write the verified Hermes entry. The corrected package layout reports Hermes
installed,healthy:trueabove.Gemini's initial 15-second MCP timeout did not reproduce in four consecutive corrected-layout enumerations: 4/4 were healthy. That establishes it as a one-off observation on this machine, not an explained or reproducible failure.
Deliberately unchanged
%USERPROFILE%\\.cursor\\mcp.json.src/core/mcp-probe.tsremains outside this PR.Verification
npx tsc --noEmitclean.cca98ccWindows argv test: 1 failed, showing--verifymerged into the trailing-backslash path.validate --range origin/main..HEAD --json:examined:5,violations:[].