Skip to content

Allocate the ticket T-1124 was told to file, and give it the measurements - #325

Merged
MongLong0214 merged 1 commit into
devfrom
feat-t1127-ticket
Jul 31, 2026
Merged

Allocate the ticket T-1124 was told to file, and give it the measurements#325
MongLong0214 merged 1 commit into
devfrom
feat-t1127-ticket

Conversation

@MongLong0214

Copy link
Copy Markdown
Owner

Allocates T-1127 for #321. Ticket-doc change only — no source is touched here.

Why a new ticket rather than widening T-1124

T-1124 (#283) set out to verify #71's install-root containment on Windows. The measurement found the hook broken, not merely unverified, and its stop condition is explicit:

if the attacks fail on Windows for any reason, stop. Do not weaken the job, do not skip the assertion, do not mark Windows supported. Record the failure as its own issue.

None of the three files that need to change belongs to T-1124 — it owns a CI job and a unit test, not the hook, the recorder, or doctor's mirror. Amending its Owns after the fact would void the stop condition to let a measurement ticket absorb a platform fix.

The ownership map row and the order note are updated in the same commit, and T-1124 is marked blocked on this.

The defect, from the runner

commitlore.root = C:\Users\...\commitlore\v9.9.9      <- written by node, win32
dirname+pwd -P  = /c/Users/.../commitlore/v9.9.9/dist  <- read under MSYS sh, POSIX
containment case: NO MATCH -- the stub falls through
test -x node: yes     test -L bin: no
command -v commitlore: NOT found (the shim is commitlore.cmd)
real commit: HUNG -- killed at 90s, the hook never returned

hook PWD    = C:/Users/RUNNER~1/AppData/Local/Temp/.../probe
hook pwd -P = /c/Users/runneradmin/AppData/Local/Temp/.../probe
walk: NON-TERMINATING, stuck at [C:]

Both guards on the containment line pass. Only the comparison fails — for every value, the installer's own bundle included — so the check neither contains nor permits, and that is what lets control reach a walk ${dir%/*} cannot terminate once no / remains.

Severity

P0 availability. A Windows user who runs commitlore init cannot commit again in that repository. The hook does not refuse; it does not return. Sequenced ahead of the remaining documentation work.

Six acceptance conditions, not "fix the hang"

The hang and the dead comparison are one causal chain, and the two obvious shortcuts are the ones that must not be taken.

  1. Two representations of one location compare equal — the writer in hooks.ts and the reader in the stub agree on the form. Normalising one side only relocates the mismatch.
  2. The ancestor walk terminates at a drive root and at a filesystem root.
  3. The legitimate recorded bundle executes. This is the baseline; until it passes, the containment attacks cannot run at all.
  4. A target outside the install root, and one with an unallowed extension, are still refused. Loosening the match to make (3) pass trades Security: commitlore.bin and COMMITLORE_BIN are executed, and the env path lacks the guard the config path has #71's property for a working hook.
  5. doctor and the hook agree on one state. isInsidePackage uses node:pathpath.win32 on Windows — so it handles the backslashed pair correctly and reports no problem in exactly the state where the hook is dead. A stub-only fix leaves the check that should have warned still reporting healthy.
  6. An already-installed repository recovers. A wedged repository cannot commit, so "new installs work" is not a fix for anyone already affected. Whether installing the corrected version suffices, or commitlore hooks install must be re-run, needs an executed answer and belongs in the release note.

The RED already exists

PR #320 is draft and red on windows-latest: its baseline step fails and the two attack steps behind it never execute. It stays that way as the platform gate this fix must clear — not merged green, not skipped, not made continue-on-error. T-1127 adds POSIX-level coverage for conditions 1, 2 and 5 that must fail before the fix.

Related: #321 (defect), #283 (blocked on this), #320 (the gate).

…ents

T-1124 set out to verify a containment property on Windows and found the hook
broken instead: the recorded install root is written by node as a win32 path, the
stub reads it under Git for Windows' sh where `pwd -P` answers in POSIX form, and
the comparison therefore fails for every value including the installer's own
bundle. Control falls through to a walk that `${dir%/*}` cannot terminate once no
`/` remains, and a real commit does not return.

Its stop condition says to record that as its own issue and stop, which is #321.
This allocates the ticket that #321 needs, because none of the three files
involved belongs to T-1124: it owns a CI job and a unit test, not the hook, the
recorder, or doctor's mirror.

Six acceptance conditions rather than "fix the hang". The hang and the dead
comparison are one causal chain, so a change that ends the hang without
reconciling the path forms leaves containment unenforced -- and the two obvious
shortcuts are exactly the ones that must not be taken: loosening the match to make
the baseline pass trades #71's property for a working hook, and normalising one
side only moves the mismatch. Condition 5 exists because doctor's mirror uses
node:path, which is path.win32 on Windows and gets the backslashed pair right, so
it reports no problem in precisely the state where the hook is dead; a stub-only
fix leaves the check that should have warned still reporting healthy.

Condition 6 is the one this ticket would have shipped without. A wedged repository
cannot commit at all, so "new installs work" is not a fix for the people already
affected. Whether installing the corrected version is enough, or whether
`commitlore hooks install` has to be re-run, is a question with an executed answer
and it belongs in the release note.

P0, and sequenced ahead of the remaining documentation work. T-1124 is marked
blocked on it; #320 stays draft and red as the platform gate this has to clear.

Record-Id: r-winfix1127
Limit: the fix has to reconcile the writer in hooks.ts with the reader in the stub; normalising either side alone relocates the mismatch rather than removing it
Limit: doctor's mirror is path.win32 and the stub is MSYS sh, so a stub-only fix leaves the mirror green while the hook is dead
Ruled-out: Amending T-1124 to own the three source files | its stop condition exists for this fork, and voiding it to let a measurement ticket absorb a platform fix teaches that stop conditions are optional when the failure is interesting
Ruled-out: Merging #320's measurement step alone to keep CI green | it preserves the observation and discards the executable definition of done, and the hang would remain
Ruled-out: Marking the containment assertions skipped until the fix lands | a skip looks like coverage and encodes surrender; the honest place for a red assertion is a draft pull request, not the mainline
Ruled-out: Treating "new installs work" as the fix | a wedged repository cannot commit, so the recovery path is the part that matters to anyone already affected
Certainty: firm
Blast: local
Undo: easy
Verified: every measurement quoted in the ticket is from a windows-latest runner and reproduced across two runs -- the recorded triple, the containment case not matching, both guards passing, commitlore not resolvable by bare name, the commit killed at a 90s bound with an orphan sh surviving into cleanup, and a probe hook reporting $PWD as C:/... while pwd -P answers /c/... in the same invocation
Unverified: everything the six conditions ask for; no fix is written here, and this commit allocates the ticket rather than executing it
@github-actions

Copy link
Copy Markdown

CommitLore — record lint

Trailers: clean — 1 commit in origin/dev..bd6a7fb09b887730f0391c08e195e77baa2e0522
Active constraints: 4 limits · 31 ruled-out · 3 warnings — from 10 records over 1 changed path

Active constraints for the paths this PR touches

Limits (4)

  • r-winfix1127 bd6a7fb — the fix has to reconcile the writer in hooks.ts with the reader in the stub; normalising either side alone relocates the mismatch rather than removing it
  • r-winfix1127 bd6a7fb — doctor's mirror is path.win32 and the stub is MSYS sh, so a stub-only fix leaves the mirror green while the hook is dead
  • r-muslbullet1126 04ac181 — this ticket owns four bullets and not the tests that read the section around them, so a check that breaks here means a region was taken that was not allocated
  • r-f14owner281 f149ff9 — the shipped install.sh still fetches SHA256SUMS at this base, so no document may describe the installer as Node-only until T-1120 changes both together

Ruled out (31)

  • r-winfix1127 bd6a7fb — Amending T-1124 to own the three source files | its stop condition exists for this fork, and voiding it to let a measurement ticket absorb a platform fix teaches that stop conditions are optional when the failure is interesting
  • r-winfix1127 bd6a7fb — Merging Execute #71's containment on Windows, with the baseline that gives it meaning #320's measurement step alone to keep CI green | it preserves the observation and discards the executable definition of done, and the hang would remain
  • r-winfix1127 bd6a7fb — Marking the containment assertions skipped until the fix lands | a skip looks like coverage and encodes surrender; the honest place for a red assertion is a draft pull request, not the mainline
  • r-winfix1127 bd6a7fb — Treating "new installs work" as the fix | a wedged repository cannot commit, so the recovery path is the part that matters to anyone already affected
  • r-muslbullet1126 04ac181 — Replacing the bullet with "musl is supported" | T-1122 owns the compatibility statement, and a second summary in the README is the duplication the ownership map exists to prevent
  • r-muslbullet1126 04ac181 — Removing the Windows bullet beside it | it is still true, test/readme.test.ts uses its exact wording as a mutation oracle, and T-1124 owns that claim
  • r-muslbullet1126 04ac181 — Folding this into T-1122's pull request | that ticket's forbidden scope is one pointer line per README and nothing else; widening it to fix an adjacent owner's debris is how single-writer discipline decays
  • r-muslbullet1126 04ac181 — Merging T-1122 first and cleaning up afterwards | two ordinary merges are not transactional, and the window where dev contradicts itself has no upper bound
  • r-wrapfallback 5a78bcc — Adding a node version check to the wrapper now | requirement 7 makes it thin deliberately and the cost lands on the hook hot path, so that trade belongs to an approved ticket rather than a documentation pass
  • r-wrapfallback 5a78bcc — Recording that an older Node breaks the bundle | none was available to run it, and asserting a failure nobody observed is the overclaim this project refuses
  • r-wrapfallback 5a78bcc — Leaving the fallback undocumented because it works | it works by using an unchecked interpreter, and the check it silently drops is one the installer advertises
  • r-t1122measured 1267579 — Marking musl supported because the platform gate is gone | removing an obstacle is not the same as running the thing, and claiming a platform without executing it is the specific error the distribution ADRs record
  • r-t1122measured 1267579 — Listing Node once as a prerequisite for both install paths | only one of them checks it, and a document that implies otherwise tells a user their setup was validated when it was not
  • r-t1122measured 1267579 — Fixing the stale musl bullet here | it is a residual compiled-binary reference, which the ownership map assigns to T-1125, and two tickets editing one region is the defect this file was corrected for
  • r-t1123inv 7472bc1 — Matching config entries by the substring commitlore | it would also match an unrelated server a user named commitlore-something, and removing someone else's entry is the failure this ticket exists to prevent
  • r-t1123inv 7472bc1 — Having uninstall remove the Claude Code plugin cache too | 6948 files keyed by plugin version belong to the CLI that wrote them, and reaching into another tool's cache is not this command's job
  • r-t1123inv 7472bc1 — Leaving the counts out and describing the split in prose | the split is only obviously right once the numbers are beside each other
  • r-containevid 3cc3f22 — Leaving both clauses as written and relying on the issue comments | a ticket read at implementation time is the contract, and a comment on an issue is not where a forbidden-scope clause belongs
  • r-containevid 3cc3f22 — Concluding from this that Windows containment is already safe | the measurement is macOS only, which is the whole reason T-1124 exists, and inferring the platform is the specific error the ADR forbids
  • r-f14postt1120 e427225 — Deleting the install.sh inventory row outright | a struck-through row that says which ticket completed it and what remains is a record; a deleted row leaves a later reader unable to tell whether it was done or forgotten
  • r-f14postt1120 e427225 — Re-anchoring every 8b0c9fa reference to the merged head | some of them are RED justifications about what was true before T-1120 shipped, and rewriting those would make the tickets claim their tests fail against a state that no longer exists
  • r-f14postt1120 e427225 — Leaving T-1121 to re-derive T-1120's contract at implementation time | it is a fact now, and an implementer inferring it can infer it differently, which is how two installers stop implementing one contract
  • r-f14owner281 f149ff9 — Leaving the README to T-1122 and letting requirement 29 slide | the requirement exists because a README that describes an installer it does not ship beside is the failure this project treats as most serious, and deferring it guarantees a window in which that is exactly what ships
  • r-f14owner281 f149ff9 — Moving requirement 29 out of the removal section into T-1120's own section | the requirement text is cited by number elsewhere, and renumbering to tidy placement would break those citations for a cosmetic gain
  • r-f14owner281 f149ff9 — Splitting the README region between T-1120 and T-1122 by language or by block | two owners on one region is the defect being corrected, and any split reintroduces it in a smaller form
  • r-f14owner281 f149ff9 — Declaring one owner per file and calling ci.yml an exception without enumerating it | an exception a reader has to interpret is the kind that rots; the four jobs are listed so a crude scan and a human reach the same answer
  • r-gateb4replan 5a35062 — Writing the Gate B acceptance rows for these tickets now | a row with no approved ticket behind it is exactly the dangling authority the Gate A matrix was written to stop, so each row lands with its ticket
  • r-gateb4replan 5a35062 — Shipping the compatibility statement before the installers | the document would describe an installer that does not exist, which is the failure mode this project treats as most serious
  • r-gateb4replan 5a35062 — Removing the compiled-binary code in the same wave as the new installers | it would open a window in which neither install path works
  • r-gateb4replan 5a35062 — Treating Windows as supported once install.ps1 lands | reachability through an installer is not a verified containment property, and conflating them would ship a platform where the property is unestablished
  • r-gateb4replan 5a35062 — Reusing the F12 ticket numbers for this work | those numbers name withdrawn tickets whose issues are closed as not planned, and reusing them would make the history unreadable

Warnings (3)

  • r-wrapfallback 5a78bcc (claim) — the wrapper enforces no Node version at run time -- the installer's Node 22 check holds only until the recorded interpreter path changes
  • r-f14owner281 f149ff9 (claim) — T-1120 must land the installer and all four README shell-install regions in one commit -- a two-commit split puts a false README on the branch, which is the window this correction closes
  • r-gateb4replan 5a35062 (claim) — T-1125 must preserve install-root containment for the wrapper case -- a diff that deletes the check together with the compiled-binary arm removes a verified security property

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

@MongLong0214
MongLong0214 merged commit 77e5ca5 into dev Jul 31, 2026
8 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