Skip to content

fn-4 hardening: the 18-defect backlog, three gated rounds - #461

Open
acebytes wants to merge 48 commits into
mainfrom
fn-4-hardening
Open

fn-4 hardening: the 18-defect backlog, three gated rounds#461
acebytes wants to merge 48 commits into
mainfrom
fn-4-hardening

Conversation

@acebytes

@acebytes acebytes commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Closes out the fn-4 hardening backlog — the 18 defects filed against main during the fn-5/fn-6 review campaigns, worked in three gated rounds on this branch. Every fix was reproduced red-first, every guard arm is mutation-evidenced (red rate over ≥8 runs where timing-dependent), and each round ended with an independent read-only gate. Round 3's gate additionally re-ran the named signature cells from rounds 1–2 and PR #460 — the disposal bindings, drain fences, capture bound, walker clamp, sizer arms — all green.

Suite: 1665 executed / 3 skipped / 0 failures, exit 0 (swift test, at commit 2bb2596, 214 s). 29 commits, 60 files, +7,221 / −857. Forked from main at 8a38e3f.

What a user gets

  • Deletion binds what it deletes, on the mainline path. Contents-mode and no-revalidator item deletions bound only their container; a child renamed at its own path inside the window was destroyed unexamined. Both arms now bind kind+inode through the same function on both sides of the hop (the Stale git-worktree scanner (fn-5) #460 r18 mechanism, reused rather than duplicated).
  • A truncated git answer is never a success. A drain that died on a hard read error looked like EOF, so a partial porcelain listing parsed as "a repository with fewer worktrees". Refused as .timeout at the execute boundary — the only place it could be caught; measured, no downstream parser can see the truncation.
  • Background scans keep out of TCC-protected paths. The resolver canonicalized pointers before the deferral gate could answer, so an automatic scan could realpath through ~/Documents. Order inverted at every pointer-derived path — plus three sibling arms the spec didn't name, one where canonical equality answered true for a deferred target.
  • No more app-freezing waits. The container-identity capture is bounded and off-main (a hung mount reported per-scanner, never a spinner); dockerPrune's whole child interaction is raced (the read parks before the wait — bounding only the wait would have moved the strand one line); zero bare waitUntilExit() remains in production, enforced by a grep-gate cell.
  • The sizing walk answers cancellation (25 µs cancel-to-return, measured at 111k entries/s) and caps at 2M entries — the cap disclosed as permanent, never dressed as retryable, and every cleaner arm refuses on a cancelled measurement.
  • The walker cannot crash the cooperative pool. Depth clamped at 128; the crash band was measured at 257–288 on the real executor (reverting the clamp dies on signal 10). Production passes 8 everywhere — zero behaviour change.
  • Launch performs no unbounded I/O on configured roots; a symlink root is answered from its link content, destination-free.
  • Issue labels state their producer's true condition. Full kind→producer matrix audited; bare EPERM is never labeled TCC (decision recorded where both producers see it); one new extensible wire kind mutation_scope_refused; no wire string renamed.
  • Bare repositories whose checkouts are all gone are now discovered — the prune tier's own case. The discovery proves the shape git itself requires, TCC-deferral answers first, and the fn-5 witness discipline (discovery identities, delete-time re-proof, detached-HEAD preservation, one-to-one mapping) applies to the new path with cells. Permanent residual disclosed: a config spelling bareness other than git's writer form stays undiscovered.
  • A git child's process group is established at spawn (posix_spawn + POSIX_SPAWN_SETPGROUP), never discovered from a live leader — the exited-leader race is gone and descendants are reaped by PID.

Test integrity

  • The three trapping shapes (as! / fatalError / preconditionFailure) are statement-trap-fenced across all test sources; the reproduction aborted the process with 38 cells unrun and a tally reading 0 failures.
  • The one genuinely flaky cell was root-caused, not tolerated: a process-wide descriptor sweep asserted against a walk-local census — the exclusivity assumption was wrong, not the census or the kernel. Rewritten as a set-based classifier (leak fails at once; ambient churn retries bounded; two clean disagreements convict). An unprovable arm was written and then deleted per doctrine.
  • All 33 performer refusal tags are pinned by a two-layer source census — swapping any two reds naming position and line; a smuggled new spelling reds at its line.
  • The Trash message fence now pins the vocabulary byte-exact and derives its own coverage: Remedy carries no free text, retirement is a production declaration, and a new Established case fails a named cell without anyone editing the test's lists. Attack round: 0 of 11 fresh false wordings pass.

Measured, deliberately not built

The dev-root "double walk" (Codex P2) is 2.2% duplicated probes, not doubling — 249 of 11,544 on an artifact-bearing 5,772-entry tree, because the two walks are asymmetric (one prunes matched dirs, the other must not). Buying 2.2% would rewrite the scanner I/O contract. The measurement cell ships executable (DevTreeWalkMeasurementTests) so the decision can be re-derived when tree shapes change.

Review round 1 (Codex, on efc2760)

Three findings, all on code this PR added — the pattern the campaign documents: each round's findings attack the last round's fix. All fixed and mutation-proven.

  • P1 — a losing task could launch the prune it was already reported not to have run. FirstWinsRendezvous decides which outcome is reported, not whether the work started; a queued detached task could run() after the timeout branch truthfully reported nothing running and re-enabled the button. The starvation the off-pool timer exists to survive is exactly what keeps the task queued. New LaunchClaim decides the pair under one lock. Mutation: 504 red, evidenced by 500 concurrent begin/abandon pairs requiring exactly one winner each.
  • P2 — MOUNT FIRST said since fn-4.12, while the probe ran above it. On an unresponsive mount that lstat never returns, so the promised issue is never emitted and the session leaves a blocked worker. The kernel table costs no filesystem call; asking it first is free. The cell counts probes rather than hanging to prove a hang.
  • P2 — unchecked posix_spawn setup returns. A dropped adddup2 spawns a child whose stdout goes elsewhere: git exits 0 and the empty buffer is accepted as complete — fn-4.24's class re-entering through fn-4.27's spawn. Now try require(...)d. Negative result recorded: the failure cannot be staged behaviourally, and launch was not reshaped to make it reachable; a source fence with a vacuity guard asserts it instead.

Known residuals (each disclosed at its site)

  • A losing bounded-capture worker stays parked on a .utility thread until its volume answers — one per timed-out attempt.
  • Appear-and-stay descriptor churn from a sibling cell would convict the census cell as a leak (the measured flake shape was vanish-churn; the retry ladder covers that).
  • Third-spelling aliases can shadow a real dev root — availability only, never destruction; pinned at all three sites.
  • The fence forces every wording change into its own diff but cannot judge truth — a coordinated wording+pin edit remains reviewer judgement.
  • pid-recycle on group SIGKILL unchanged in class from Stale git-worktree scanner (fn-5) #460 r18.
  • 29 of 33 refusal tags' user-facing detail prose has no behavioral cell (the tag routing is pinned; 4 details asserted).

…d the no-revalidator item arm (fn-4.21)

remove(at:expecting: nil) proves the container and nothing else; a child
renamed aside at its own path inside the window was destroyed unexamined
with the measured tree's bytes reported (reproduced deterministically on
both arms before the change — contents mode and item mode with no
revalidator, moveToTrash: false).

The binding is the r18 BoundObject shape through the same function:
TrashDisposal.boundLeaf under the proved admitted container at the
pipeline's first read of the leaf, re-proved by the identical read in
provingImmediatelyBefore on the far side of the queue hop
(provedStillTheBoundLeaf, shared by both arms so two readings cannot
disagree). Refusal is the same .notTheInspectedObject the verdict arms
throw, tagged content-drift in contents mode too.

The standing enumeration at DepthSafeRemoval is re-derived from grep:
five sites, not three; the dispose(expecting:) site is never nil at
runtime (inside if-let — the task spec's claim retired against source);
the Trash arms' disposal-entry binding residual is stated where each arm
ignores the early capture. Vanish-at-bind fixtures now count the
matching probeChild call: call 1 is the pipeline bind (already-gone
skip, newly pinned), call 2 the disposal's own window.
…ke EOF (fn-4.24)

PR #460 r18 adversarial verification (runner scope), MEASURED: on any
read(2) error that is not EINTR/EAGAIN/EWOULDBLOCK, PipeDrain.drain()
set isDone and returned, signalling `finished` exactly like EOF —
join(within:) answered true, execute's success gate could not tell the
two endings apart, and the partial buffer shipped as .success(stdout:).
A truncated porcelain listing does not look malformed; it looks like a
repository with fewer worktrees, or a clean tree.

Reproduced RED before the fix with a real failing descriptor (EISDIR
via a directory fd), at both levels:
- drain: terminalReadFailure nil where EISDIR died the worker
- execute: .success(stdout: 0 bytes) [stdout arm] and .success(stdout:
  16 bytes) [stderr arm] where the drain had died mid-stream, with a
  CONTROL run proving the stub succeeds through healthy drains

Fix: the drain RECORDS the terminal errno under its own lock where the
error fires; execute reads it right after the successful joins — the
first read of that fact on the only path that can still become a
success — and answers .timeout, the same class as the unjoined-drain
arm (C7) and for the same reason. Retry can differ: fresh invocation,
fresh pipes, fresh descriptors. No terminate in the new arm: the child
has provably exited and both drains returned, so a kill there would be
an unevidenced guard.

Downstream-parser question (asked by the spec), answered from source
and RECORDED at the gate: GitWorktreePorcelainParser fails closed on a
mid-field/mid-record cut but accepts a record-boundary cut as fewer
worktrees; WorktreeStalenessAssessor.verdict counts a truncated status
as cleaner and an empty one as .clean; first-line readers accept
whatever line survives. The boundary gate is load-bearing.

Mutations (deterministic cells, filtered runs, target rebuilt each):
- m1 restore EOF-equivalence (drop the errno recording): RED 3/3 cells
- m2 delete the execute gate: RED both execute cells
- m3 drop the stderr operand: RED stderr cell only
- m4 drop the stdout operand: RED stdout cell only

PR #460 drain bounds re-run green after the change:
testCapturingOutputIsNotStarvedByAContinuouslyWrittenWriteEnd,
testCloseIsNotStarvedByAContinuouslyWrittenWriteEnd,
testAReadTurnThatNeverRunsDryStillEndsOnItsOwnBound,
testTheDrainIsEndedOnlyThroughTheBoundedSpelling,
testAnUnfinishedDrainOnANormalExitIsNotReportedAsSuccess.

Full suite: 1600 executed / 2 skipped / 0 failures (baseline 1597/2/0).
…dicate both dereferenced deferred paths (fn-4.26)

GitWorktreeGitdirResolver canonicalized a worktree's gitdir: pointer BEFORE
its first gated probeKind, so an .automatic scan realpath(3)'d through a
TCC-protected admin directory before DeferringIdentityProvider could answer
.absent — and the deferral predicate (ProjectTreeWalker.isProtectedRoot)
itself canonicalized the very path it was classifying. Reproduced RED first
in six cells (scan-level realpath count, resolver pointer/commondir/backlink/
cross-validation, predicate direct-spelling); the cross-validation cell also
showed the fallback's canonical path equality answering TRUE for a deferred
target.

The fix is ORDER, not removal: the resolver probes every pointer-derived
path AS SPELLED before canonicalizing or comparing it, and isProtectedRoot
classifies lexically first, canonicalizing only spellings the lexical stage
could not match (aliases). The pass-through delegation injected test
providers rely on is untouched. Retired the 'house doctrine draws the line
at canonicalize' claim where it was written.
… (fn-4.26)

ProjectTreeWalker.isProtectedRoot grew 30 lines, moving the cited
device-compare/isMountPoint arm from 529-532 to 559-562; the anchor
integrity cell caught the drift in the full-suite run.
…ted (fn-4 r1 gate)

The gate found BuildArtifactsScanner.swift:729 citing the mount-refusal arms
at :1151/:1371 — lines that now hold a comment and an unrelated identity
check after fn-4.21's ~200-line insertion. Repointed to :1187/:1460, verified
by grep against the 'mount boundary' refusal strings themselves.
SourceAnchorIntegrityTests does not cover doc-comment anchors in THIS file's
prose (it pins its own expectation table), which is why the gate had to catch
it by hand — same class as the r19 shift, different detector.
…line capture (fn-4 r1 gate)

fn-4.21's refusedChild path had no cell for a bind read that fails with
anything but ENOENT: an EACCES at TrashDisposal.boundLeaf during the
pipeline capture must be the child's reported FAILURE, never the silent
skippedAlreadyGone skip — the leaf is still standing there.

Provider double denies exactly one probeChild read (the
LockUnreadableProvider pattern), with an unarmed control. Mutation
proved: widening the ENOENT catch to swallow every
DepthSafeRemoval.Failure as a skip turns the cell red on the
error-count assertion (0 != 1), reverted.
…ead (fn-4.19)

ContainerSnapshot.capture ran synchronously inside scanValidatedSession,
which CacheoutViewModel.scan reaches on the MainActor: every session
root's lstat ran on the main thread before any bound existed, so a hung
mount under a root — including a root INSIDE a mount, which the
mountPointPaths() preflight cannot see — froze the app unbounded and
unreported (r12 measured 6.03 s with isMainThread=true from a 6 s
blocking identity(of:)).

captureBounded races the capture loop, detached at .utility (the
producer's band-separation decision, for the same reason — an
unspecified-band capture queued behind saturation-cell holders and rode
its own deadline), against a ScanSessionClock timer via
FirstWinsRendezvous — BoundedDiskInfo's rendezvous, extracted so the
three bounds share one spelling. scanValidatedSession is async since
this change; both production consumers and 16 test call sites await it.

An expired capture runs NO scanner and says so: one .scanDidNotFinish
per selected scanner whose detail names the capture, ledger concluded
.boundFired (GUI declines adoption, CLI target-scoped refusal reads the
rows), snapshot .empty (admits nothing — the same fail-closed refusal an
omitted root always had). A retry can differ — stalled volume, starved
band, both transient — so the re-scan remedy is real, not a strand.

New captureDeadline on ScanSessionBounds: production 30 s, fixture
default 10 s. The cell's wedge is a releasable semaphore: a fixed
5 s sleep leaked its abandoned-capture worker into the saturation
cell's window (red only in close pairings; measured, fixed, 8/8 green
paired). Mutation A proved: restoring the sync unbounded capture reds
the cell 8/8 on the elapsed assertion.
…last bare waitUntilExit calls (fn-4.20)

dockerPrune did readToEnd() then a bare Process.waitUntilExit() in a
detached task, awaited unbounded: the retired primitive's last
production call site, on a cooperative worker, latching isDockerPruning
for the life of the app if docker never exited. And the wait was not
the only park — the task-spec question answered: readToEnd() blocks
until EOF, so a wedged child that keeps its pipe open parks the read
BEFORE any wait is reached; bounding only the wait would have moved the
strand one line up.

The budget (stated: 600 s production, seams for tests) therefore races
the WHOLE interaction (spawn -> drain -> waitForExit(within:)) via
FirstWinsRendezvous on ScanSessionClock. Expiry is reported ('did not
finish within ...'), SIGTERM is best-effort, the button releases on
every path, and a completed failure cannot trade places with a timeout
(three cells: expiry, success control, completed-failure control).

The spec's 'last surviving call site' claim was stale against source:
Tier2Interventions carried two more bare waits (post-SIGKILL reaps);
both converted to waitForExit(within: 5) so the gate can hold. The gate
itself is a cell (DocumentedContractTests): zero non-comment
waitUntilExit() lines across Sources/**.swift, comment mentions allowed
(they document the retirement), test sources out of scope per spec.
Verifier finding: 3c98463 grew PathGuard.swift by a uniform +74 lines and
SpaceScanner.swift by +29 below its insertions, and
SourceAnchorIntegrityTests.testEverySourceAnchorStillPointsAtWhatItCites
went red on exactly the 11 anchors citing below those points (green at
a8de2a2, red at 3c98463 — attributed by running the cell at both). The
cited text itself is byte-identical (every pinned excerpt found at the
old offset + the file's uniform delta); each citing sentence was re-read
against its shifted target before repointing, per this check's own rule.

21 citing sites + the 11 anchorExpectations rows updated; every
replacement is digit-for-digit the same width, so no line in any citing
file moved and no further anchors drift from this commit.
…ies (fn-4.15)

DirectorySizer.measure had no cancellation point and no entry cap
(measured: Task.isCancelled occurred 0 times in the file) — a scanner
cancelled mid-measure kept walking, and nothing bounded the walk.

CANCELLATION: checked between entries, before the pulled entry is
processed. A cancelled walk returns what it has with the report MARKED
partial (SizeReport.cancelled) — deliberately not a denial: nothing
refused the read, and a retry CAN differ (cancellation is a caller act).
Measured: 111,437 entries/s full-walk rate; cancel-to-return latency
25 us after 5,765 entries (figure to beat was r16's 46.3 ms). Red-first:
testAPreCancelledMeasureStopsBeforeTheFirstEntry failed at 12/12 entries
enumerated before the check existed.

ENTRY CAP (the design decision): 2,000,000 entries per measure call,
DISCLOSED as .enumerationCapped at the walk root, spent only when a
further entry actually exists (an exact-fit tree makes no truncation
claim). The cap is DETERMINISTIC over a static tree, so the disclosure
states permanence and offers no retry — no re-scan wording anywhere,
pinned by testTheCapDisclosureNamesAPermanentConditionNeverARetry.
Value sized against measurement: ~111k entries/s puts the cap at ~18 s
of walk; the largest evidenced real tree (23G worktrees, scenario 2 of
FIELD-EVIDENCE) extrapolates to ~300k entries at this repo's measured
entries-per-byte (.build: 5,917 entries / 481 MB).

PER-CALLER VERDICTS (the C6 check, stated per scanner):
- CacheScanner / sweep / worktree items: capped figures are floors; the
  denial rides the existing channels (scanErrorKind .other;
  rootIssueKind -> .enumerationTruncated, whose GUI label is already
  true of it). No deletion verdict consumes the cap: delete-time
  re-measurement is UNCAPPED.
- BuildArtifactsScanner census: a capped census is a FLOOR, and the
  probe's doubling already grows past an undercounting census by
  documented design.
- GitWorktreeScanner admin-prune suppression (denials.first) would
  suppress on a cap denial; unreachable there in practice (fixed-shape
  admin dirs, ~10 entries) and delete-time still re-verifies uncapped.
- CacheCleaner (delete time): UNCAPPED sizer by construction — the
  mount doctrine reads mountBoundaries as 'the whole tree was swept',
  so a capped walk would either permanently strand deletion (the C6
  pattern) or delete mount-blind. The pass stays proportional to the
  deletion it precedes, and is cancellable per entry.

FAIL-CLOSED CONSUMERS (a partial report is never consumed as complete):
CacheCleaner's category-child and item arms and WorktreeReclaimPerformer's
worktree and admin-prune arms each refuse a cancelled report before the
mount check and before any claim registration, tag
'measurement_cancelled', wording explicitly retryable ('not permanent').
Scan-time consumers are covered by the session-completion discard
(CacheoutViewModel: completed = !Task.isCancelled && !didExceedBounds;
nothing a cut-off session saw becomes deletable).

Anchors: 4 repointed (DirectorySizer 261-272 -> 317-332, 354-359 ->
443-448, 483 -> 570; CacheCleaner 514 -> 523) across expectations and
citing sites.

Suite: 1625 executed / 2 skipped / 0 failures in 208.6 s wall (exit 0,
total line printed). Mutation matrix runs next; results recorded in the
task report.
…mp the walker's depth budget (fn-4.13)

THE SWEEP (every self-calling function over a filesystem tree in
Sources/, traced to actor reachability):

ONE true recursion found: ProjectTreeWalker.visit — one frame and one
anchor per level, reached from the cooperative pool by BuildArtifacts
and GitWorktree scans. Its budget guard (childDepth <= maxDepth, before
the self-call) bounds production at defaultMaxDepth = 8 — no production
caller passes anything else (grepped: both scanner inits default it) —
but maxDepth was an unclamped parameter, so a test seam or future
caller could turn it into a crash.

MEASURED, red-first, through the real visit on the real executor
(Task.detached, mkdirat-chain fixture, env-gated exploratory cell):
depth 128 survived, 192 survived, 256 survived, 288 and 320 died with
signal 10 — a guard-page hit on the cooperative pool's small stack.
The crash band (257..288) matches PR #459 r14's freshContentBelow
measurement (~250-260) — the class this task exists to hunt.

FIX: walk() clamps maxDepth to stackSafeMaxDepthCeiling = 128 (2x
margin under the measured crash floor, 16x above the production
default; zero shipped behavior change). No runtime refusal exists to
word: the clamp is deterministic and disclosed at the constant.
Mutation: reverting the clamp kills
testAWalkAskedForACrashBandDepthSurvivesTheCooperativePool with
signal 10 (the r14 precedent: a signal death on revert is a
legitimate red); with the clamp it returns with the deepest event
exactly at the ceiling.

NEGATIVE RESULTS, recorded:
- DirectorySizer.enumerateTree: a while-loop over Foundation's deep
  enumerator (heap-backed, no per-level frame) — MEASURED at depth 350
  on the cooperative pool, walks to completion and reaches the leaf
  (testADeepChainMeasuresOnTheCooperativePoolWithoutAStackCrash).
- DepthSafeRemoval: iterative on purpose (its own doc/code) — the
  in-repo re-anchoring pattern this task treats as the replacement.
- EphemeralTempScanner.walkForFreshContent: iterative [URL] stack
  (fixed in PR #459 r14; pinned there, out of scope here).
- ValuablesDetector's ValuablesProbeWalk: an explicit frame machine
  (descend pushes frames; no self-call).
- Single-level listings only: CacheCleaner (category children),
  GitWorktreeInventory (admin container), InstalledAppResolver.
- Every other self-call hit in the sweep is overload dispatch, not
  recursion (CacheoutViewModel.handle/clean, TrashDisposal.look ->
  look(named:), FileSystemIdentityProvider.leafMetadata,
  GitWorktreeScanner.respell, GitCommandRunner.run, DevRootsStore
  .resolve, ValuablesDetector.acknowledgementToken/probe,
  EphemeralTempScanner.boundedFirstLevelNames,
  SpaceScanner.validatedOutcome, ProjectTreeWalker.issue).
- Mutual-recursion pass: manual read of the walker files; visit's
  self-cycle is the only cycle. Sources/CacheoutHelper has no tree
  walk; Watchdog/ sits outside Sources/ and the task's scope.

The exploratory env-gated cell (WALKER_DEPTH=n) stays, skipped by
default, for future re-measurement.
ProjectTreeWalker.swift:559-562 -> 578-581 (the +22-line
stackSafeMaxDepthCeiling doc moved the mount-check excerpt); caught by
testEverySourceAnchorStillPointsAtWhatItCites on the full run.
…ion — policy, dev-root resolution and union answer from the link's own content (fn-4.11)

Reproduced RED first in 8 cells: `PathGuard.validateContainerRoot`
canonicalized every configured root — full realpath(3), leaf included —
at runtime construction on the main thread, so a persisted dev root a
same-UID process re-aimed at an unresponsive mounted volume froze launch
before any window existed; `DevRootsStore.resolve`'s probe pass and
`SpaceScannerRuntime.suppressingAliasShadows` then paid the same
resolution again (and a dangling alias sent realpath's fallback walking
the destination's parent chain).

The fix is the fn-4.26 order applied at all three sites, with fn-6's
readlink technique as the spec directs: probe AS SPELLED first (lstat
no-follow), canonicalize only a spelling proven a real directory (the
resolved leaf then IS the object the lstat touched), and answer for a
symlink leaf from the link's OWN content — one readlink(2) plus the
lexical fold hoisted from EphemeralTempRoots to
FileSystemIdentityProvider.lexicalTargetPath, shared by all four
resolvers. The policy's deny verdicts survive destination-free: "/" and
..-escapes from the folded content, volume roots from the getfsstat
kernel table (never a path syscall), $HOME by string against both
spellings; the r15 kernel-table preflight now also refuses an
over-mounted DECLARED root at the policy and skips it in the union's
probe. Cross-scanner alias shadowing still suppressed, by NAME —
residual (third spelling: second hop, case variant, /var-style alias)
disclosed at all three sites and pinned as kept-both.

Mutation evidence, one arm at a time (all restored): policy preflight →
over-mounted cell red (contact + no refusal); canonicalize-first
restored → all five no-contact cells red at first contact; mount-table
arm → alias-of-mount cell red; "/" arm → both root-alias cells red;
home arm → both home-alias cells red; DevRoots drop arm → drop cell +
delete-time shadow cell red; DevRoots/union symlink canonicalize → the
production/resolution contact cells red; union drop arm → both shadow
cells red; union preflight → union-probe cell red. The dropped-alias
covering-key link in canonicalKeys was mutation-tested GREEN (no cell
reached it — a dropped spelling supports no admissible claim), so it is
retired rather than kept unevidenced, and the r16 sentence claiming it
is rewritten at both sites.

Also retired on the same evidence: EphemeralTempRoots' recorded r12
"one surviving contact" residual and its r15 symlink-to-mount residual
bullet (both closed by this change, history kept), and the
"canonicalize-before-check" wording wherever the proposition survived.
All shifted anchors repointed and re-verified by
SourceAnchorIntegrityTests.

The union probe's kind switch spells every KindProbe case explicitly —
the R4 no-`default:` fence over SpaceScanner.swift caught the first
draft (testNoExhaustiveReclaimActionSwitchGainedADefaultArm), which is
that fence doing its job.

Full suite: 1636 executed, 3 skipped, 0 failures (exit 0, 187 s).
…e — respell, repoint the population, and pin (fn-4 r2 gate)

Third anchor-rot occurrence in two rounds, and the root cause was a SPELLING:
the citations of CacheCleaner's two mount-refusal arms were written
`deleteGuardedChild`:1151 — a form SourceAnchorIntegrityTests' pattern cannot
see — so the suite could never redden on them and each gate caught the drift
by hand. Worse, the r1 fix repointed ONE citing site while ValuablesDetector
and OrphanedCachesScanner carried the identical stale pair: the
sweep-the-phrasing-not-the-claim failure, applied to anchors.

All three citing sites respelled to the canonical CacheCleaner.swift:1210 /
:1496 form, which the gate's DEFAULT-DENY rule then forces into the pin table
— three rows added, so the next insertion that shifts those arms is a red
cell, not a reviewer finding. The pre-existing stale :571 citation of
preDeleteUserDataProbe (actual :816, stale at base 8a38e3f) is repointed and
pinned the same way. The mechanism promptly proved itself: the +2 lines of
respelled citations shifted the pre-existing BuildArtifactsScanner
:1405-1406 pin, and the gate went red before this was committed; repointed to
:1407-1408 in both the pin table and the citing test comment.

CORRECTION OF RECORD for 9ce6b1d: its commit message claims "Anchors: 4
repointed", which was false against its own tree — it re-broke the
BuildArtifactsScanner:729 anchor the r1-gate commit 6bb24c0 had just fixed
(arms moved :1187->:1210, :1460->:1496 under fn-4.15's insertions). Corrected
forward here per the project's standing practice; the history is not
rewritten.

Suite: 1636 executed / 3 skipped / 0 failures, exit 0, 183 s, at this commit.
…itionFailure out of test sources (fn-4.14)

The PR #459 r15 strand diagnosed: a SHORT READ — both test socket clients
did one unframed read(2) on a newline-framed SOCK_STREAM reply, and the
truncated JSON fed a trapping cast. Reproduced red-as-crash (signal 6,
38 later cells never ran, last tally read 0 failures) and red-as-failure
after the XCTUnwrap shape; the casts themselves were converted on main in
8fa8ad3, so this closes what was left open:

- TestSocketClient.readNewlineTerminatedReply: loop to the newline, EOF,
  or a full buffer; both helpers now call it. Evidence cell drives a
  server that writes the reply in two spaced segments — single-read
  mutation red 10/10, framed loop green 10/10.
- StrandFenceTests: as!, preconditionFailure and fatalError promoted from
  'counted, not scanned' to statement traps (all zero occurrences, no
  allowance to rot). Mutation: one cell carrying all three shapes redded
  each arm by name at its line.
- CONTRIBUTING.md: a green TALLY is not a green RUN — trust exit code and
  executed count, never a greppable '0 failures' line.
…sserted a per-walk property (fn-4.16)

DIAGNOSED, not retried away. The suite is one process; a sibling cell's
deferred cleanup (StatusSocket accept handlers' deferred closes, Process
pipe teardown) can close descriptors mid-walk, so heldDescriptorCount()
minus a baseline is NOT the walk's descriptor count. That is exactly the
observed shape — 'census 2+1 vs measured -8', kernel count DOWN, which no
walk that closes only what it opened can produce. Alone the old cell was
green 20/20 (recorded negative result): the failure needed a sibling.
Neither the census nor the kernel was wrong; the cell's exclusivity
assumption was.

The cell now snapshots the fd SET around each attempt and classifies:
appeared-and-stayed = leak, fail at once; vanished = ambient churn, the
cross-check is unattributable, retry (bounded at 5 — a retry CAN differ,
the churn is transient sibling cleanup, not a deterministic limit);
set-clean disagreement = suspect, and a second one convicts the census.
Walk-internal invariants (completion, forced climb, observed mid-climb
peak, window bound) assert on every attempt. A new cell manufactures the
churn deterministically — closes eight held fds inside the first census
instant — and pins the classifier while reproducing the original
disagreement evidence.

Mutations: census transient over-counted by one → red 8/8 (two-suspect
conviction); classifier blinded to vanished fds → churn cell red; a
descriptor leaked inside the attempt → red on the leak arm. Fixed cells
green 20/20.
…ag gate, decision recorded (fn-4.23)

DECISION (of the task's two designs): the FULL per-tag gate, implemented
as a two-layer source census rather than 29 new behavioral cells — the
set-gate alternative cannot meet the task's own mutation bar, because
swapping two tags between arms leaves the set unchanged. Layer 1 pins the
ORDERED sequence of the 42 tag literals (33 distinct) at their definition
sites: any swap, insertion, removal, or rename changes the sequence, so
sequence equality subsumes the set gate (hence no second gate, per the
spec's 'do not add both'). Layer 2 sweeps the whole file for tag-shaped
literals and requires each to be censused or pinned as a non-tag, closing
the new-spelling hole a position grammar alone would leave. User-reaching
DETAILS keep their behavioral assertions in WorktreeReclaimPerformerTests
(the r17 lesson: 4 tags asserted there today); this gate protects the
routing discriminator underneath them.

Mutations, each red with the position and source line named: swapping
worktree-deregistered with worktree-not-linked (position 20, line 2290);
swapping the prunedAdminBinding pair (position 30); smuggling
'brand-new-tag' through a let binding (layer 2, line 2695).
…ion; bare EPERM is never TCC (fn-4.12)

The GUI derives the whole visible row from ScanIssue.Kind alone
(ScanIssueRowPresentation.label(for:)), so a kind shared with a different
condition prints a false diagnosis to the user and to wire consumers
(PROTOCOL.md scanner_errors[].kind). Audit of the full kind -> producer
matrix, r17 message-truth method (derive from the producing path, then
check the label):

  containerRefused "not a configured search root"
    - GitWorktreeScanner outside-every-root arm  TRUE, kept + pinned
    - GitWorktreeScanner scope arms (worktree inside a root)  FALSE
        -> NEW mutationScopeRefused (wire mutation_scope_refused)
    - DevRootsStore validateContainerRoot catch (detail says
      "configured dev root refused")  FALSE -> policyRefusedRoot
    - ProjectTreeWalker admitSearchRoot catch  FALSE for policy clauses
        -> policyRefusedRoot; mount standing at the root -> the
        mountedVolumeRoot ephemeral shape (table re-read per walk, so the
        label's "then re-scan" is true); notAConfiguredContainer keeps
        containerRefused (roots: is a parameter; that label IS the truth)
  symlinkRoot "symlinked - not searched"
    - ProjectTreeWalker root gate (any non-directory)  FALSE for
      file/FIFO/socket/device -> nonDirectoryRoot, split by probed kind
    - OrphanedCachesScanner rootNotADirectory  same split
    - DevRootsStore + EphemeralTempRoots alias arms  TRUE by construction
      (readlink-gated), pinned
  tccDenied "access denied by macOS privacy settings" + Grant-access link
    - bare-errno EPERM producers (ProjectTreeWalker forFailedOpen;
      DirectorySizer.denial(forFailedProbe:) feeding walker, sweep and
      sizer walks)  FALSE: a raw errno has no provenance (TCC, SIP,
      immutable flags indistinguishable) -> neutral unreadable/.metadata
      with the cause-not-established detail; the rule the ephemeral
      scanner already measured and recorded, now decided ONCE at the
      shared classifier. Chain-proven EPERM (classifyDenial via
      NSUnderlyingErrorKey) keeps tccDenied - the one assertable arm.
  unreadable / enumerationTruncated / config/tool/malformed/didNotFinish
    - audited, labels true; GitWorktreeScanner validation failures stay
      unreadable (state could not be faithfully read; recorded)

Wire: mutation_scope_refused is an ADDITION on the extensible enum
(schema stays 4, no string renamed); PROTOCOL.md, API-REFERENCE.md and
CHANGELOG call out every condition whose kind moved. cacheout-mcp needs
no update (measured in the task spec: forwards rows opaquely).

Residual, recorded in EphemeralTempScanner header (c): its sizing-path
.tcc neutrality is now over-conservative (the conflation that mandated
it died at the source) but asserts nothing false; out of fn-4.12 bounds.

Anchors repointed in the same commit (SpaceScanner +25 tail,
DevRootsStore +6 tail, DirectorySizer +5 tail, ProjectTreeWalker
578-581 -> 629-632, CLIHandler 2123 -> 2125, CacheoutViewModel
1487-1488 -> 1488-1489); SourceAnchorIntegrityTests green.
…ever discovered from a live leader (fn-4.27)

r18's ownProcessGroup read getpgid(pid) after run(), so the group fact
was contingent on observing a LIVE leader: a git that spawned a helper
and exited inside that window left group == nil, and terminate() then
signalled only the corpse's pid while the descendants -- the case the
group protocol exists for -- ran on holding the inherited pipe. The r18
comment called nil "never worse than before"; for the exited-leader case
it was exactly as bad as before. Comment updated to what is now true.

Fix: the runner spawns through its own SpawnedProcess -- posix_spawn
with POSIX_SPAWN_SETPGROUP (pgroup 0), so the kernel makes the child the
leader of a new group (id == pid) atomically at creation; pid IS the
group id for the process's whole life and nothing about signalling is
conditional on leader liveness. ownProcessGroup is deleted.

WHY posix_spawn over the trampoline option (spec asked): it keeps the
argv fence byte-identical (/usr/bin/env + ["git", ...], argv-only,
never a shell -- testTheNewGitFilesNeverConstructAShellString) and
ships no new binary (scripts/bundle.sh copies nothing it is not told
to -- the v2.1.0 lesson); a shell trampoline is banned by the fence, a
compiled one is a bundling liability.

Parity with Process, each stated in SpawnedProcess's doc: /dev/null
stdin via file actions, dup2'd pipe write ends, CLOEXEC_DEFAULT for
everything else, parent write-end close at launch (Process did this
implicitly; forgetting it starves the drains of EOF), and a bounded
pid-targeted waitpid(WNOHANG) poll in Process.waitForExit's own
deadline/backoff shape (waitUntilExit misses wakeups under concurrent
reaping -- house doctrine). terminationStatus maps signal deaths to
-(signal) so no real exit code is impersonated; callers test 0/127.

Cell (fn-4.27 acceptance):
testALeaderThatExitsImmediatelyStillHasItsDescendantReapedByPid --
leader records pid, spawns a TERM-immune descendant holding stdout,
exits 0; asserts .timeout, that the LEADER was already unsignallable
when the runner answered, and that the descendant dies BY PID.
Green 8/8 unmutated.

Mutation (acceptance): restoring post-launch discovery -- signalTree
requiring getpgid(pid) == pid before group-signalling, r18's
semantics -- RED 8/8 on that cell (the leader is provably reaped before
terminate, so discovery always fails and the descendant survives).
Negative result, recorded: restoring discovery at the LAUNCH spot
(r18's literal line) cannot be reddened deterministically -- the leader
is microseconds old there and always observable -- which is why the
mutation targets the semantics (group contingent on leader liveness)
at the point of use.

Full GitCommandRunnerTests: 32 tests, 0 failures on the new seam.
… the bare-EPERM rule; repoint the two anchors the fn-4.12/4.27 growth shifted (fn-4.12)

The full-suite gate caught what the targeted batches missed:

1. BuildArtifactsScanner.obstruction(at:errno:detail:) was a FOURTH raw
   bare-EPERM -> .tcc producer (a failed openat on the containment
   descent) the audit's grep of ScanIssue producers could not see - it
   classifies into an ITEM's SizeDenial, not a ScanIssue - and its .tcc
   becomes the item row's .tccDenied grant link. Same rule now: bare
   EPERM -> neutral .metadata with the cause-not-established caveat.
   Cell testDescentOpenEPERMClassifiesNeutrallyEACCESAsPermission (EPERM
   neutral + EACCES control); mutation restoring the .tcc arm reddens it.
   The walker's child-probe comment stating the retired proposition is
   updated with it.

2. ProjectTreeWalker.swift:629-632 -> :637-640 (the admission-catch
   rewrite grew the walker after the anchor gate had last run) and
   BuildArtifactsScanner.swift:1407-1408 -> :1416-1417 (the obstruction
   fix above), repointed with their citing sites in the same commit;
   SourceAnchorIntegrityTests green.
…ne — the prune tier's own case (fn-4.28)

Discovery keyed entirely on an entry named .git, and a bare repository
has none — so once its linked checkouts were deleted, no repository
group formed and the prune tier never ran for exactly the
all-checkouts-gone case it exists to reclaim. REPRODUCED first: bare
clone + worktree added + checkout rm'd -> scan returned zero items and
zero issues, silently.

The fix is a discovery kind, not a tier: GitWorktreeGitdirResolver
gains the bare-shape proof (HEAD a regular file whose content git's
validate_headref would accept, an objects directory, a refs or
reftable backend, and a config that declares 'bare = true' the way
git's writer spells it — which is what keeps a --separate-git-dir
git directory, same shape with bare = false, from being claimed).
Every probe runs through the injected identity provider, so the TCC
deferral answers first, and HEAD/config are read only behind their
probeKind gates (fn-4.26 ordering). The walk consumer admits the shape
only when the event's own lstat'd entries already show it — no .git
entry of any kind, HEAD/objects/refs kinds right — so unproved
directories cost no probe and provably reach no git subprocess.
Grouping canonicalizes the bare directory exactly as the main-checkout
arm canonicalizes <dir>/.git, so a bare parent reached both ways
(its own shape and a live checkout's gitdir: pointer) names ONE group
and pays for ONE listing (cell-pinned). Downstream is untouched: the
porcelain first record must still declare itself bare and
cross-validate against the same directory before anything acts.

Cells: the named discovery/prune cell (the task's mutation gate), the
looks-bare refusal (no item, no issue, no subprocess), per-requirement
forgery refusals with an intact-shape control, the deferral cell with
its own control, one-listing dedupe, detached-HEAD preservation on the
bare path, and the end-to-end cell that prunes through the production
composition and then asks git itself: clean registry, quiet fsck, and
a fresh 'git worktree add' that succeeds.

Residual, disclosed at the proof: a bare repository whose config
spells bareness any other way stays undiscovered — the same silent
non-discovery it had before, never dressed retryable.
…ication, not doubling; fan-in recorded, not built (fn-4.18)

MEASURE FIRST, per the task's own acceptance bar. On an
artifact-bearing tree (two populated node_modules, one Rust target,
one repo+worktree; 5772 entries):

  build WALK    249 probes  0.004s  (consumer prunes matched dirs)
  build SCAN   5772 probes  0.230s  (walk 249 + sizing census 5523)
  git   SCAN   5772 probes  0.212s  (walk prunes nothing, by design)
  union WALK   5772 probes  0.132s  (zero consumers = fused reach)

Codex's 'nearly double filesystem I/O and latency' is CORRECTED, not
inherited: the walks are asymmetric, the duplicated enumeration is
their intersection (the pruned build walk), and a fused walk must
carry the git walk's unpruned reach while the build scanner pays its
sizing census either way — so the fan-in buys 249 of 11544 entry
probes (2.2%) on the tree class these scanners exist for. The claim's
true half is a dev root with no artifacts and no repository: both
walks enumerate everything (496 probes each, ~7ms) and fusing halves
milliseconds.

DISPOSITION — scoped, not fixed, with the numbers: a shared walk must
be per scan session (the walk-instant witness the r16/r17 re-proof
chain hangs off can never come from an earlier session's walk),
scanners run concurrently AND individually, and nothing at the
SpaceScanner boundary names a session for two scan(context:) calls to
rendezvous on — coalescing on accidental concurrency would make the
walk count timing-dependent in the safety path. Buying the measured
2% means rewriting the protocol's 'a scanner does its own I/O'
contract, which the task's Boundaries forbid: record and stop.

The measurement is KEPT EXECUTABLE (DevTreeWalkMeasurementTests) with
the two load-bearing facts pinned: the pruned build walk is strictly
smaller than the git walk, and the git walk equals the zero-consumer
union reach. The disposition note sits at the walk step of
GitWorktreeScanner.scan, where the finding was anchored.
… text, retirement is a production declaration, extension fails closed (fn-4.22)

The r18 fact-set assembly made a false clause unrepresentable at any
call site; two surfaces remained, measured at 11/11 unanticipated
false wordings passing:

1. Remedy.text was free prose whose only check compared the rendered
   clause against the enum's OWN text — a tautology any prose
   satisfies. The enum now carries NO text: the remedy wordings live
   in sentence(for:), the one production wording table, and the fence
   pins them byte-exact per Remedy case instead of asking production
   to agree with itself.

2. A new Established case carried its own false sentence through
   every structural check, because each check is relative to tables
   the same author edits and the test-side neverEstablished list
   could not know about a case added after it. Three moves close it:
   - the unspoken set is DERIVED in the fence from established(for:)
     over every cause and required to equal a new production
     declaration, Failure.retired — a case claimed by no cause must
     be visibly retired, and a retired case must render nil;
   - the whole vocabulary, every wording, both remedy wordings and
     the placesTheItem verdicts are pinned BYTE-EXACT in the named
     cell testTheVocabularyAndEveryWordingArePinnedSoExtensionFailsClosed
     (default-deny, the SourceAnchorIntegrityTests shape): a new case
     or a reworded/grafted sentence is a red cell the moment it
     exists, with no test edit needed for the failure to fire —
     making it green again forces the sentence verbatim into the
     fence's own diff;
   - the r18-measured GREEN residual (a false clause grafted into an
     existing wording) reddens under the same pins.

What a pin deliberately does not claim: that a pinned sentence is
TRUE. The residual is disclosed BY MECHANISM at
TrashDisposal.Failure.sentence: (1) a coordinated wording+pin edit in
one commit — the fence forces the sentence in front of a reviewer,
it cannot judge it; (2) established(for:) admitting a proposition
its path does not prove — narrowed by placesTheItem and the measured
fixture cells, unnarrowed for non-placing facts.

Attack round (>= 10 fresh false wordings) follows in the next commit
with per-attack results.
…dings pass the pinned fence (fn-4.22)

Eleven unanticipated false wordings, each applied to production alone
and run through the full TrashDisposalHopProofTests filter, then
reverted: both remedy prose grafts (including the task's measured
passer), the r18-measured GREEN graft, a net-effect rewording, a
hedge strip, a non-placing proposition reworded into a placing claim,
three whole-new-Established-case rides (claimed, unclaimed, and
smuggled through retired with a sentence), and two derivation edits.
All eleven RED; baseline re-ran green between attacks as the control.
The per-attack table lives on the pin cell's doc.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: efc27604d7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread Sources/Cacheout/ViewModels/CacheoutViewModel.swift Outdated
Comment thread Sources/Cacheout/Scanner/ProjectTreeWalker.swift
Comment thread Sources/Cacheout/Scanner/GitCommandRunner.swift
… reported not to have run (PR #461 codex r1, P1)

FirstWinsRendezvous decides which OUTCOME is reported. It cannot decide
whether the work STARTED, and for a destructive child those are different
questions. A detached task still queued when the off-pool timer wins observes
nothing; the timeout branch sees `process.isRunning == false`, truthfully
reports that nothing is running, re-enables the button — and the task is then
scheduled and calls `run()`, launching an unowned `docker system prune -f`
after the operation was reported timed out, free to overlap the retry the user
was just invited to make.

The starvation this off-pool timer exists to survive is exactly the condition
that keeps the task queued, so the window is widest when it matters most.

New LaunchClaim: both sides claim through one lock, so the pair is decided
once. `begin()` false means DO NOT START; `abandon()` false means it already
started and the caller owns stopping it. The timeout branch now reads
`launch.didStart, process.isRunning` — stating which case happened rather than
inferring it from a Process that answers false when it never launched.

MUTATION (begin ignores a prior abandon): 504 assertion failures. The
evidence is not a sequenced imitation of the race — testUnderContention...
runs 500 concurrent begin/abandon pairs and requires EXACTLY ONE side of each
to win: both true is a prune launched after being reported stopped, both false
is a prune neither run nor accounted for.

Suite: 1665 executed / 3 skipped / 0 failures, exit 0, 214 s, at this commit.
The paragraph has said MOUNT FIRST since fn-4.12 while the root probe ran
above it. On an unresponsive mounted volume that `lstat` never returns: the
walk hangs, the `.mountedVolumeRoot` issue the paragraph promises is never
emitted, and the session reaches its watchdog leaving a blocked worker behind.
The comment described the contract; the order did not honour it — this
branch's most-retired defect class, found in the fix that wrote the comment.

The kernel table is memory and costs no filesystem call, so asking it first is
free. Reordered; the absent-root probe now follows, which is what makes the
hang unreachable.

EVIDENCE, and why it counts rather than measures: a hanging provider cannot
prove this — it would hang the suite, which IS the defect. So the new cell
counts instead. A provider that records every probe must record NONE for the
mounted root, and recording none is exactly what makes the hang impossible.
MUTATION (probe restored above the table): red, naming the recorded probe of
the mounted root.

Anchor: this round's insertions shifted ProjectTreeWalker.swift:637-640 ->
:650-653; repointed in both the citing prose (BuildArtifactsScanner.swift:735)
and anchorExpectations. Fourth payout of that gate this campaign.
…ns a different child (PR #461 codex r1, P2)

Every `posix_spawn_file_actions_*` and `posix_spawnattr_*` return value was
discarded. These calls allocate, so under transient pressure they answer
ENOMEM — and discarding that is not a lost message. A dropped `adddup2` leaves
git's stdout attached to whatever descriptor 1 already was: git exits 0, the
drain sees an immediate EOF, and the EMPTY buffer is accepted as a complete
answer. That is precisely the class fn-4.24 closed at the execute boundary,
re-entering through the spawn fn-4.27 introduced. A dropped attribute call
defeats fn-4.27's group isolation just as quietly.

Every setup call is now `try require(...)`d, throwing SpawnFailure. The caller
maps it to `.gitUnavailable`, which fn-4.25 retries rather than caches — and
ENOMEM is transient, so that retry can genuinely differ.

NEGATIVE RESULT, recorded in the cell rather than worked around: no
behavioural cell can stage this failure. `launch` takes Pipes and builds its
own descriptors; `adddup2` does not validate that a descriptor is OPEN at
setup time, so a closed pipe throws from `fileDescriptor` one layer earlier
and never reaches the guard. Staging real ENOMEM is not available to a test,
and reshaping `launch` to take raw descriptors purely to make the failure
reachable would widen production API for evidence — which this project
declines. A visibility widening tried for that purpose was reverted.

So the property is asserted where it lives, in the same shape as the
waitUntilExit gate (fn-4.20) and the refusal-tag census (fn-4.23): a source
fence over the spawn path, with a vacuity guard (>= 6 setup calls must be
found, or the fence has gone blind).

MUTATIONS: dropping one `try require(` reds naming GitCommandRunner.swift:785;
renaming the calls so the scan matches nothing reds the vacuity guard.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2bb25961bd

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines 1909 to 1911
guard launch.begin() else { return }
do {
try process.run()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Hold the launch claim through process startup

When the timer fires after launch.begin() returns but before process.run() starts, abandon() loses and the timeout result is published, yet process.isRunning is still false so the timeout arm does not terminate anything; the detached task can then resume and launch the destructive prune after the UI has re-enabled retries. Fresh evidence in the current code is that begin() marks didStart before the separate process.run() call, leaving this interval unsynchronized; startup must be atomic with the timeout decision.

Useful? React with 👍 / 👎.

Comment on lines +804 to +809
var argv: [UnsafeMutablePointer<CChar>?] =
([executablePath] + arguments).map { strdup($0) }
argv.append(nil)
defer { argv.forEach { free($0) } }
var envp: [UnsafeMutablePointer<CChar>?] =
environment.map { strdup("\($0.key)=\($0.value)") }

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject failed C-string allocations before spawning

Under the same memory pressure these new setup checks are intended to handle, strdup may return nil, but nil is also the argv/envp terminator and every allocation result here is accepted unchecked. A failure while duplicating "git", for example, makes /usr/bin/env run with no utility, print its environment, and exit 0, so the runner can report successful but unrelated output as a completed git command; validate every duplicated string and throw before posix_spawn if any allocation fails.

Useful? React with 👍 / 👎.

Comment on lines +438 to +444
configContents.split(whereSeparator: \.isNewline).contains { line in
let trimmed = line.trimmingCharacters(in: .whitespaces).lowercased()
guard let equals = trimmed.firstIndex(of: "=") else { return false }
let key = trimmed[..<equals].trimmingCharacters(in: .whitespaces)
let value = trimmed[trimmed.index(after: equals)...]
.trimmingCharacters(in: .whitespaces)
return key == "bare" && value == "true"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Restrict the bare key to Git's core section

When a traversed separate git directory has the normal core.bare = false plus an unrelated section containing bare = true (or an earlier core.bare = true overridden later by false), this line treats it as a bare repository because section context and Git's last-value semantics are ignored. The new discovery then runs worktree list against that healthy non-bare admin directory and publishes a cross-validation unreadable issue instead of leaving the deliberately unsupported separate-git-dir repository undiscovered; parse the effective core.bare value rather than matching any line named bare.

Useful? React with 👍 / 👎.

Comment on lines +402 to +404
guard identity.probeKind(of: head) == .kind(.regularFile),
let headContents = try? String(contentsOf: head, encoding: .utf8),
Self.isAcceptableHeadContent(headContents)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Bound metadata reads during bare-repository discovery

When any directory under a configured dev root has the inexpensive bare-shape names but an oversized regular file at HEAD (and likewise at config below), String(contentsOf:) loads and decodes the entire file before deciding whether this is a repository. A malformed or adversarial multi-gigabyte file can therefore cause a large memory spike or terminate the app, and the scan deadline cannot cancel this synchronous read; read only a bounded HEAD value and parse config incrementally or under a size limit.

Useful? React with 👍 / 👎.

…ndow, it did not close it (PR #461 merge gate)

83e2fd7 gave LaunchClaim a bare `begin()` and left the caller to start the
work on the next statement. That moved the window from "task never scheduled"
to "task scheduled, between two adjacent statements", and the window is the
same defect: with the timer firing in between, `abandon()` answers false
("already started"), the timeout branch reads `didStart == true` with
`isRunning == false` — because `run()` has not executed yet — terminates
NOTHING, reports the prune stopped, re-enables the button, and the detached
task then launches an unowned `docker system prune -f`.

Worse, 83e2fd7's comment asserted `didStart` "states which case happened
rather than inferring it". In that window it states the wrong case and the
conjunction silently selects do-nothing: a claim about the code that the code
did not have, which is the class this branch exists to retire.

`begin(_ body:)` now runs the work while holding the lock, so deciding and
starting are one act. After `abandon()` answers false, the work has PROVABLY
run. A throwing start spends the claim but leaves `didStart` false — nothing
is running to terminate.

THE HEADLINE EVIDENCE IN 83e2fd7 WAS FALSE, and this corrects the record. It
claimed "MUTATION: 504 assertion failures" and presented a 500-pair contention
cell as primary proof. Re-running the stated mutation yields THREE failures,
not 504 — and that contention cell PASSES under it. Instrumenting the same
shape shows why: beginWins=500, abandonWins=0. The two GCD blocks never
raced; 500 iterations exercised one ordering, and the abandon-first ordering —
the queued-task case the whole type exists for — was never taken. The
"deliberately not a sequenced imitation" claim was exactly backwards: the
sequenced cell was carrying the evidence.

The cell now races on a barrier and ASSERTS BOTH ORDERINGS OCCURRED, so a
cell that stops contending fails instead of passing quietly; and the invariant
it checks is the one that matters — a winning `begin` has already run the
work. MUTATION (claim taken, work merely scheduled): red.

The never-launched timeout also stopped lying: it said "asked it to stop" for
a child that was never started. It now says "did not start ... nothing was
run", with the same real remedy (the cause — a starved pool, a busy daemon —
is transient).

Suite: 1666 executed / 3 skipped / 0 failures, exit 0, 233 s.
… that said otherwise was blind (PR #461 merge gate)

150fe01 moved `probeKind` below the mount table and asserted the hang was
unreachable. It was not. `isProtectedRoot`'s stage 2 canonicalizes, and
`realpath(3)` on a hung hard mount blocks uninterruptibly in the kernel — so
the TCC gate, which ran ABOVE the table, was the first contact with the root.
On `includeProtectedRoots: false` — what every non-userInitiated trigger
passes, i.e. the automatic/background scan — the walk hung there, the
`.mountedVolumeRoot` issue was never emitted, and the session reached its
watchdog with a blocked worker: the exact outcome 150fe01 claimed to have made
impossible. Stage 1's lexical match only rescues roots spelled under
~/Documents|Desktop|Downloads; a /Volumes-style dev root falls to stage 2.

The table is memory (getfsstat with a private buffer, no filesystem contact),
so it now answers before ANY code that can touch the filesystem. Order is
table -> TCC gate -> probe.

AND THE CELL WAS BLIND BY CONSTRUCTION. It recorded `probeKind` alone, so it
could only ever prove something about `probeKind` — while `canonicalize` and
`realPath` walked past it. A guard that names one syscall proves one syscall.
The double now records probeKind, canonicalize, realPath and identity, and the
cell runs on the background arm that was actually broken. MUTATION (TCC gate
restored above the table): red, naming canonicalize(<volume>) and realPath on
the mounted root — the contact the old cell could not see.

Anchor: 150fe01's repoint was off by two in the very gate it called its
"fourth payout" — :650-653 began mid-`||` and ended on a blank line, invisible
to SourceAnchorIntegrityTests because it only requires the excerpt to appear
somewhere inside the range. Repointed to :661-664, the actual four-line
condition.
…build it as a property (PR #461 merge gate)

2bb2596's fence was written in the same hour as a commit criticising
blocklists, and was one. Measured against the real spawn path: four unchecked
setup calls inserted after the last `try require` all escaped while the
vacuity guard stayed satisfied — an assignment prefix, a nested-func wrapper,
a local alias, and `posix_spawn_file_actions_addclose`, a genuine setup call
whose only defect was being absent from a six-name alternation. Six more
escaped by inspection, including a bare call whose trailing COMMENT contained
the literal text `try require(`, because checkedness was
`line.contains("try require(")` and a comment laundered it. The regex was
anchored per line, so ANY token before the symbol made a call invisible rather
than flagged: the failure was silent, not loud.

It also FALSE-POSITIVED: re-wrapping the existing, correct
`try require(posix_spawnattr_setpgroup(...))` across three lines reddened the
fence, so a routine reformat broke the suite on correct code.

Rebuilt as a property:
  - symbols matched BY PREFIX (`posix_spawn*`), not an enumerated list, so a
    setup call cannot escape by being newly spelled;
  - comments stripped before matching, so nothing launders itself;
  - statements rather than lines, so wrapping neither hides nor false-flags;
  - checkedness is STRUCTURAL — the call must be the direct operand of
    `try require(` — so an assignment or alias in front fails loudly;
  - bounded to `launch`'s body, with teardown and the spawn itself exempted
    BY NAME rather than excused by a pattern.

VERIFIED against every escape the gate found: assignment prefix, `_ =`,
addclose, setsigmask and comment-laundering are now all CAUGHT (5/5), and the
re-wrapped correct call is GREEN — the false positive is gone.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 28a8f15cbc

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +402 to +403
guard identity.probeKind(of: head) == .kind(.regularFile),
let headContents = try? String(contentsOf: head, encoding: .utf8),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Open bare-repository metadata without following replacements

When another process can modify a candidate directory during an automatic scan, it can replace HEAD after this no-follow probe with a symlink before String(contentsOf:) opens it; the same window exists for config. Foundation then follows the replacement, so the new bare-repository discovery can read or block on a TCC-protected or unresponsive target despite requiring these entries to be regular files. Read each file through a no-follow descriptor and verify that descriptor's identity/kind, rather than separating the probe from the path-based read.

Useful? React with 👍 / 👎.

Comment on lines +1594 to +1599
let boundTarget: FileSystemIdentityProvider.ChildFacts? =
probedObject == nil
? try TrashDisposal.boundLeaf(
of: target, containedIn: admittedParent,
provider: provider
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Bind item identity before measuring it

For a permanently deleted item whose scanner supplies no revalidator, this binding is taken only after sizer.measure(at:) has finished. If the target is renamed away and a different object is installed at the same name between the measurement and this line, boundTarget records the replacement and the far-side proof succeeds, so the replacement is destroyed while the report credits the original tree's bytes. Capture the leaf identity before the measurement (as the contents-mode path does) and carry that same identity through the final removal.

Useful? React with 👍 / 👎.

…2 P1)

The merge gate reopened the exact window 36dff04 claims to close —
`decided = true; unlock(); try body()` — and the FULL 1666-cell suite
stayed green. Every cell read the witness after `begin` returned or after
`group.wait()`: instants at which the body has run under either shape.
`abandon()` answering false is a claim about the PAST, and it is only
falsifiable while `begin` is still in flight.

Two cells now observe inside the window:

- testAbandonNeverAnswersFalseBeforeTheWorkHasRun manufactures the instant
  deterministically — the timer thread is released from inside the body and
  reads the witness immediately after abandon() answers. RED 12/12 mutated,
  GREEN 12/12 pristine.
- the contention cell reads the witness in the abandon thread rather than
  after the join, and its body now takes spawn-like time (200us) because the
  real body is a fork/exec. RED 12/12 mutated, GREEN 12/12 pristine. With a
  nanosecond body the same cell caught it 1 run in 20 — recorded, because a
  sampler is only as good as the window it samples.

Corrected on the record: 36dff04's cited mutation (dispatching the start
asynchronously) is strictly STRONGER than the defect it was cited for, so
the synchronous near-miss was invisible to it.

testAbandonAnsweringFalseMeansTheWorkHasAlreadyRun renamed to
testBeginPerformsTheWorkRatherThanSchedulingIt — it asserts the sequential
contract and cannot see the window; its old name said otherwise. The
throwing-start cell is annotated with what it does and does not cover.

Measurement note: a first attempt scored the contention cell 0/12 by
grepping for "with 1 failure" — the mutated run fails many rounds at once.
The predicate is now the absence of "with 0 failures".
…(gate r2 P2, P3)

P2 — the fence matched `posix_spawn…\s*\(` and required `try require(`
immediately in front. Keyed on a CALL, it could not see a function VALUE:

    let addclose = posix_spawn_file_actions_addclose
    _ = addclose(&fileActions, 5)

The symbol appears with no paren after it, and `addclose(` is not a
posix_spawn spelling — fence green. The gate compiled and ran that to prove
it is working Swift. The property is now on the SYMBOL: every appearance of
a `posix_spawn*` identifier in the spawn path must be the direct operand of
`try require(`. A function value cannot be taken without naming the symbol,
so aliasing is caught at the alias. Exemptions are by property (`_t` is a
type) or by name (two `defer` destroys, the guarded `posix_spawn` itself) —
never by pattern. Acknowledged limit, recorded at the site: a symbol reached
through `dlsym` by string is outside what a source fence can see.

Retired the false comment that claimed "an assignment or alias in front of
it fails loudly instead of vanishing" — it vanished. And corrected the
record: 28a8f15's "5/5 escapes" list silently substituted `setsigmask` and
comment-laundering for the two alias cases round 1 actually found.

P3 — `\s+` -> " " collapsed whitespace BEFORE `components(separatedBy: "  ")`
split on two spaces, so the "statements, not lines" split never fired and
every failure quoted the first 90 characters of the whole function. Matching
is now done over the body with line numbers computed from newline counts,
and a failure reads `GitCommandRunner.swift:802  posix_spawnattr_setpgroup
—  let setpgroup = posix_spawnattr_setpgroup`. The rationale doc block, which
had detached onto `repositoryRoot`, is back on the fence.

Also closed while rebuilding (gate residuals P8, P9): the region is the
balanced-brace extent of `launch` rather than `range(of: "\n    }\n")`, which
was the first four-space-indented `}` INSIDE the function — a pure
truncation escape with all seven checked calls left intact is now caught,
naming :807. Block comments are blanked as well as line comments. The
vacuity floor is 7, the actual count, and `posix_spawn` is now a reachable
exemption rather than dead configuration.

VERIFIED, 15 variants applied to GitCommandRunner.swift, built and run:
  RED   E1 bare call · E2 assignment prefix · E3 `_ =` prefix
        E4 function-value alias · E5 alias via array element
        E6 line-comment launder · E7 block-comment launder
        E8 four-space-brace truncation · E9 unlisted setsigmask
  GREEN R1 wrapper split across lines · R2 `try require (` with a space
        R3 comment between wrapper and operand · R4 deep argument wrapping
        R5 semicolon-preceded statement · R6 one-line `if`
15/15 as expected, 0 mismatches. Full GitCommandRunnerTests: 33/33 green.
… P6, P7)

A residual this project has decided to keep is recorded where it can be
reopened, not left for the next round to rediscover.

P5, at the docker-prune claim: `begin` performs the launch UNDER the claim's
lock — that is the mechanism — so the timer body blocks on it for the
spawn's duration, and that body runs on ScanSessionClock's single shared
serial queue. No deadlock cycle, but every other bound scheduled there is
delayed by a fork/exec, and a `.timedOut` settle overshoots by the same
amount. Not fixable by releasing the lock earlier: releasing it is the
window LaunchClaim exists to close.

P6, at the timeout branch: which arm is taken is pinned by LaunchClaimTests
through `didStart`; the two wordings are not. Pinning them would mean
hoisting the strings into production API for a test to read, and a source
fence over the spellings would be a blocklist wearing a fence's name — the
class this branch has retired eleven times. Recorded as uncovered.

P7, at the walker's mount-table gate: the table-before-TCC order changes
what a root that is BOTH protected and mounted reports on a background scan
— silence before, `.mountedVolumeRoot` now. Honest, but user-visible, and
the printed remedy is half true for such a root: eject it and the next scan
reaches the TCC gate and skips it silently again, with nothing to explain
the disappearance.

Anchor `ProjectTreeWalker.swift:661-664` repointed to `:671-674` (both
citing sites) — the ten disclosed lines shifted it, and the anchor gate
caught it before the suite did.
… line (codex r2)

`declaresBare` matched any line whose key was `bare`, anywhere in the file,
and shipped with no test of any kind. git reads two common shapes the other
way:

  [core]                      [core]
  	bare = false            	bare = true
  [remote "origin"]           [core]
  	bare = true             	bare = false

Both were read as bare. That is not harmless over-discovery: the scanner
then runs `worktree list` against a healthy non-bare admin directory and
publishes a cross-validation `unreadable` issue on EVERY scan, for a
`--separate-git-dir` repository shape it deliberately does not cover.

Now resolved as git resolves it: section context honoured (`[core "sub"]` is
core.sub.bare, a different key), comments stripped outside quotes, a
variable on the section header's own line read, quoted values unquoted, and
the LAST value winning — so an override can turn bareness OFF rather than
merely failing to turn it on.

The narrow VALUE spelling is unchanged and still disclosed: `bare = yes`,
`bare = 1` and a valueless `bare` key leave the repository undiscovered.
That residual is now PINNED by a cell, so it cannot widen or narrow in
silence.

12 new cells, 12/12 green. MUTATION, measured both directions:
  - the any-line matcher reds 7 cells / 8 assertions, including both cells
    written for the finding;
  - first-wins instead of last-wins reds exactly 2, and nothing else.

GitWorktreeInventoryTests 97/97, WorktreeReclaimPerformerTests 153/153,
SourceAnchorIntegrityTests 6/6 green.
… site (gate r3 P5, P6)

P5 — the r2 note at dockerPrune's timeout branch was false in BOTH halves.
It claimed the two wordings had no cell and that pinning them would mean
hoisting the strings into production API. In fact
`testDockerPruneExpiresReportsAndReleasesTheButton` already pinned the
`didStart` arm by reading `lastDockerPruneResult` — published state three
cells in that file already consume. The other arm is pinned the same way
now, by a new cell, with nothing hoisted.

A wrong residual is worse than none: that one would have sent a future round
hunting for a cell thirty lines away in a file it already reads, and licensed
a swap of the two messages as "uncovered".

  MUTATION: give the unstarted arm the started arm's wording ("did not
  finish … asked it to stop"). RED 4/4; GREEN 4/4 unmutated. The new cell
  carries a vacuity floor — a zero budget must actually reach the
  never-started arm, or it pins nothing.

P6 — `ScanSessionClock`'s own doc block still read "nothing blocking may
ever be scheduled on it", absolutely and with an enumerated list, while
since 36dff04 `dockerPrune` schedules `abandon()` there and `abandon()`
blocks on a lock held across a fork/exec. d0a0b66 disclosed that at the
SCHEDULING site only; the site a future round actually reads to answer "may
I block in this body?" still said no. Both ends now say the same thing, and
the exception is named as an exception rather than a precedent.

Anchor drift: the 14-line insertion shifted six SpaceScanner anchors;
repointed at both citing sites (measured +14, not the +15 the edit implied).
…gate r3 P7)

`mountTable.contains(root.path)` is EXACT membership against mount POINTS,
so a dev root INSIDE a mounted volume — `<volume>/dev`, the ordinary way
people put a dev root on an external or network disk — never matches it.
The first filesystem contact for such a root is `isProtectedRoot` stage 2's
`realpath(3)`, on a path on that volume: precisely the call f4575bf
identifies as blocking uninterruptibly on a hung hard mount. Two comments
stated the opposite without qualification ("the only ordering in which its
own canonicalize cannot be the call that hangs", "what makes the hang
impossible"), and the cell that guarded them exercises only
root == mount point.

The guard was put where the finding pointed rather than at the earliest
point the root is read.

No policy change, because the project already has a deliberate answer for
this shape and it is not the table: the SESSION's wall-clock bound. Both
walker callers are `SpaceScanner` conformers, so a blocked `realpath`
becomes `.scanDidNotFinish` with the ledger concluded `.boundFired` — the
same answer `captureBounded` gives for a container root inside a hung mount,
whose own note already says the table preflight never covered it. The same
residual carries verbatim: the bound converts the hang into a REPORT, it
does not cure it; the thread stays parked until the volume answers.
Widening the gate from membership to containment would refuse every root
under /System/Volumes/Data, which is a policy decision nobody has taken.

New cell `testARootInsideAMountedVolumeIsNotCaughtByTheTableAndIsProbed`
pins the honest behaviour — the root IS contacted, no `.mountedVolumeRoot`
is emitted — and reds if the gate is ever widened to containment, so that
change has to come here and say so.

Anchor ProjectTreeWalker.swift:671-674 -> :695-698 at both citing sites.
ProjectTreeWalkerTests 36/36, SourceAnchorIntegrityTests 6/6.
…odex r2, gate r3 P4)

strdup (codex r2) — `map { strdup($0) }` wrote a failed allocation's nil
straight into argv, where nil is the TERMINATOR. Under the same pressure the
setup checks exist for, a failed copy of "git" leaves argv
`[/usr/bin/env, nil]`: env runs with no utility, prints its environment and
exits 0, so the runner accepts unrelated output as a completed git command.
The same silently-different-child class as a dropped adddup2, and quieter,
because this one succeeds. Every copy now goes through a throwing
`duplicate`, and each vector's `defer` is registered BEFORE it is filled so
a throw part-way frees what was already copied.

P4 — the fence exempts `posix_spawn` because it "is checked by its own
`guard`", and nothing tested that guard. Nothing COULD: `executableURL`
defaults to `/usr/bin/env`, which always exists, and no test had ever passed
`executableURL:`. It is also the one call whose unchecked failure is
catastrophic rather than wrong — `var pid: pid_t = 0` means a swallowed
failure hands back pid 0 and `signalTree` then runs `kill(-0, SIGTERM)`,
signalling the caller's own process group.

A nonexistent executable makes posix_spawn answer ENOENT on Darwin, so the
throw path is reachable without staging memory pressure.

  MUTATION: guard deleted -> RED 3/3. Measured with `signalTree` given a
  `pid > 0` precondition in the SAME mutation, deliberately: without that
  second edit the mutant signals the harness's own process group, which
  destroys the run rather than measuring it. Recorded at the cell.

GitCommandRunnerTests 33/33 before the new cell, 34 after.
…sted past it (gate r3 P2, P3, P8)

Fourth rebuild. Rebuild 3 keyed on a CALL and a local alias walked past;
rebuild 4 keyed on the SYMBOL but scanned only `launch`'s BODY, so the gate
hoisted the same alias one scope up and walked past again:

    private static let addclose = posix_spawn_file_actions_addclose
    _ = Self.addclose(&fileActions, 5)

Both escapes compiled and ran. The region is now the WHOLE FILE — there is
no scope left to hoist to.

P2 — `commentsBlanked` modelled only plain string literals. One
`private static let rawMarker = #"\"#` anywhere earlier in the file left
`inString` true for the entire remainder (the backslash sets `escaped`, the
closing quote is swallowed), so no comment after it was ever blanked and
comment-laundering came back with the fence green. The file is now REFUSED
if it contains a raw or multiline literal, rather than mis-scanned. String
CONTENTS are blanked too, which also retires `let note = "try require("`
in front of a bare call.

P8 — `try require(Darwin.posix_spawnattr_setpgroup(...))` is correct code
that rebuild 4 reported as unchecked. The wrapper patterns now tolerate an
optional module qualifier. Renaming a wrapper still reds, which is correct
and is stated as an acknowledged limit: the fence cannot know a new name is
a checker.

The fence is now a table of rules, so the new `strdup` invariant rides the
same mechanism: every `strdup` must be the operand of a `guard let`.

VERIFIED, 15 variants applied to GitCommandRunner.swift, each built and run:
  RED   bare call · `_ =` prefix · local function-value alias
        type-scope static func · type-scope static alias (P3, both)
        raw literal + laundered call (P2) · line-comment launder
        string-literal launder · unlisted setsigmask · unchecked strdup
  GREEN wrapper split across lines · `try require (` with a space
        module-qualified (P8) · comment between wrapper and operand
        semicolon-preceded statement
15/15, 0 mismatches. Several escapes red on the vacuity floor as well as the
unchecked list; the floors are per-rule now (7 spawn symbols, 1 strdup).

Destroy-exemption residual disclosed at the exemption site: it is by name
and does not check the call is in a `defer`, so a destroy before the spawn
is green. Kept — the alternative is a `defer`-proximity spelling test.
…its one call site (gate r3 P1)

`LaunchClaim` is correct and `LaunchClaimTests` proves it — the gate
independently re-measured RED 12/12 on the near-miss. But every one of those
cells builds its own claim and calls `begin` itself. Nothing held
`dockerPrune`, the ONLY production user, to keeping the launch inside the
body: the claim takes a closure, so `begin({})` is writable and the launch is
free to drift back to the next statement, which is the original defect rather
than a variant of it. The gate restored that two-statement shape here and all
1667 cells stayed green.

A test cannot hold this boundary. The damage needs the timer to land in a
fork/exec-wide window, so any cell for it samples rather than proves — I
wrote one, it went green 3/3, and the vacuity floor I added then showed
16/16 rounds reported the arm with ZERO children ever observed, because the
correct shape kills the child before it can write. That green was hollow.

So the boundary moves into the type. `ClaimedProcess` owns the `Process`,
builds it from executable/arguments/environment/pipes, and never hands it
out; `start()` is the only launcher and decides under the claim's lock.
There is no `process` in scope at the call site any more.

  VERIFIED: the gate's mutation applied verbatim now fails to build with
  `cannot find 'process' in scope`.

  LIMIT, measured and disclosed at the type: a caller can still construct
  its OWN Process and run it (that compiles). But that starts a DIFFERENT
  child than the one the claim guards — a visible act, not silent drift —
  and `didStart` would contradict it. The boundary is "the claimed child
  cannot start unclaimed", not "nothing may ever be spawned here".

CacheoutViewModelTests 56/56, LaunchClaimTests 6/6, GitCommandRunnerTests
34/34.
…ad (codex r2)

Codex named `HEAD` and `config` in the bare-repository probe. The population
is EIGHT: seven `String(contentsOf:)` and one `Data(contentsOf:)`, across
`GitWorktreeInventory`, `pathContents`, and `WorktreeReclaimPerformer` — a
DELETION path. Every one asked `probeKind` about a PATH and then handed that
same PATH to a reader that resolves it again and FOLLOWS symlinks. Fixing
only the two named would have left six with the identical shape.

Three defects, one primitive:

1. TOCTOU. A `HEAD` or `config` replaced by a symlink between the probe and
   the read was opened through the replacement, so an automatic scan of a
   dev root can be steered into a TCC-protected or unresponsive target
   despite a check that just said "regular file". A path is not an identity,
   and asking twice is asking about two objects.

2. UNBOUNDED READS. Any directory under a dev root with the cheap
   bare-repository shape but a multi-gigabyte `HEAD` was loaded and UTF-8
   decoded in full before anything decided it was not a repository. The size
   is now checked against the DESCRIPTOR and the file REFUSED, never
   truncated — truncating would let a huge file whose first bytes spell
   `ref: refs/…` pass as a valid HEAD.

3. `WorktreeReclaimPerformer.HeadWitness` resolved its file THREE times
   (probeKind, identity, bytes), so the witness could pair one object's
   inode with another's bytes — and that witness is what the reclaim proves
   the far side against. Kind, identity and bytes now come from one
   descriptor.

`O_NONBLOCK` is not incidental: `O_NOFOLLOW` does not save the open of a
FIFO, and a named pipe left at one of these names parks the opening thread
until a writer appears.

9 new cells, 9/9 green. MUTATION, each alone and measured:
  - drop O_NOFOLLOW      -> reds the symlink cell, nothing else
  - drop BOTH size checks -> reds the oversize cell, nothing else
                             (dropping one is caught by the other; the file
                              can grow between them, so both stay)
  - drop S_IFREG         -> reds the FIFO cell
  - drop O_NONBLOCK      -> THE SUITE HANGS, killed at 300 s with no tally.
                             That is the guard's point, and why the FIFO cell
                             asserts wall-clock: a reader that parks does not
                             fail, it stops.

Limits named once (`gitPointerByteLimit` 64 KiB, `gitConfigByteLimit` 1 MiB).
A real repository whose config exceeds the limit stays UNDISCOVERED — the
same silence every bare repository had before fn-4.28, disclosed at the
constant, never a refusal dressed as retryable.

GitWorktreeInventoryTests 97/97, WorktreeReclaimPerformerTests 153/153,
GitConfigBarenessTests 12/12, SmallRegularFileReadTests 9/9.
…ys done it the other way (codex r2)

For a permanently deleted item whose scanner registers no revalidator —
every shipped scanner without one — the leaf binding was taken AFTER
`sizer.measure`. That left a window the whole measurement wide: rename the
target away, install a stranger at the same name, and the binding recorded
the STRANGER. The far-side proof then succeeded (it proved the stranger
against itself), the stranger was destroyed, and the report credited the
ORIGINAL tree's bytes. Contents mode binds first and always has; only this
arm was backwards, 300 lines apart in the same file.

The binding and its `admittedParent` capture move above the measurement,
which is where contents mode has them.

  NEW CELL `testItemModeTargetSwappedDuringTheMeasurementIsRefused`: swaps
  the target when the SIZER reaches the payload. MUTATION (bind after the
  measure, as before) -> RED 3/3, and the mutant reproduces the report
  exactly: "the stranger was DELETED" plus an entry crediting 4096 bytes of
  a tree it never touched. Unmutated GREEN.

THE PRE-MEASURE BINDING IS `try?`, WITH A FALLBACK, and that is not
incidental. A hard `try` re-tagged the absent-target arms: two
OrphanedCaches cells refuse a directory that appears at a name the probe
found ABSENT, and they assert the refusal MESSAGE on purpose — "had the
replacement landed BEFORE the probe, the probe would have walked it and
refused with user-data-shaped content instead, so the fixture cannot
silently degrade into testing the other arm". A ghost leaf now yields nil
here and the original read still stands exactly where it always did, same
call, same point, same failure. The new binding only ever ADDS a refusal;
it never moves one.

Three fixtures keyed on provider CALL ORDER needed re-arming, and the
production order is why:
  - SwapAtTheDisposalContainerProofProvider counted descriptor-identity
    questions after the revalidator gate. The admitted-parent capture used
    to be #1 and the disposal's container proof #2; the capture now happens
    before the gate, so the proof is #1. What the swap targets is unchanged.
  - TempRaceWonAtTheFinalCheckProvider opened its window on ANY
    descriptor-identity call. Two now happen before the revalidation, so it
    opened too early and the swap landed ahead of the revalidator's own path
    check — which caught it, making the cell prove a different guard than
    its name. It now gates on `ownerUID(ofDescriptor:)`, the revalidator's
    own accessor, as its sibling fixture already did.

`token` and the success tail moved inside the widened `do`; a failure on any
path still reaches the catch with the token abandoned.

Anchor CacheCleaner.swift:1496 -> :1543 at all four citing sites.
Full suite: 1692 executed, 3 skipped, 0 failures, exit 0.
…eported as success (gate r4 P4)

LIVE DATA DESTRUCTION, reproduced on 70f4376 before the fix: item mode, a
scanner with no revalidator, a target already unlinked when cleaning starts.
The pre-measure bind answers absent, so `preMeasureLeaf` is nil; the `??`
fallback then re-reads the leaf at the original point, and if an object has
appeared in between it binds THAT one. The far-side proof succeeds — it
proves the newcomer against itself — the newcomer is destroyed, and the item
reports SUCCESS with `errors: []`.

The r2 comment said that fallback was "the original read … same call, same
point, same failure". The third clause was false: the re-read can SUCCEED,
on a different object. Nothing inspected it and nothing measured it, so the
report has no bytes to be wrong about; it simply deletes a stranger and
calls it a success.

Pre-existing rather than introduced by the r2 hoist — the hoist closed this
window for leaves that bind pre-measure and left it open for ghosts — and it
was disclosed nowhere. The population with the hole is exactly the one the
r2 commit said it was fixing.

The re-read still happens, because a still-absent leaf must raise its ENOENT
at the original point with the original identity: the absent-target arms pin
that message on purpose, so a fixture "cannot silently degrade into testing
the other arm". What changes is what a SUCCESSFUL re-read means — a drift
event, refused as `.notTheInspectedObject` (tag `content-drift`), not a
target.

  NEW CELL `testAStrangerArrivingAtAGhostTargetIsNeverDeleted`, written
  BEFORE the fix and RED on it 3/3: "a stranger that arrived at a GHOST
  target's name was DELETED", plus a success entry and zero errors. GREEN
  after. It carries a vacuity floor (`provider.planted`).

Deletion-path suites after the fix: CacheCleanerTests, OrphanedCaches 120,
EphemeralTemp, WorktreeReclaimPerformer — 436/436 green.
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Aug 29, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-08-29T08:54:03.445480Z 21c48b5 New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

…nce read one file (gate r4 P1, P2, P3)

P1 — the unchecked `map { strdup($0) }` that 2d5b351 fixed in the spawn path
was ALSO in `CLIHandler`'s `execv` re-exec, on the shipped Homebrew install
route. Nil is argv's TERMINATOR: a failed copy of element k truncates the
command, so `cacheout install-helper` through the documented symlink
re-execs into a no-subcommand `cacheout` — and the process image is already
replaced, so nothing can report it. Every copy now goes through a nil-check
and a failure declines the re-exec instead of performing a different one.

The fence could not see it because it read ONE FILE. That is the third time
this fence has been beaten by moving one scope out: rebuild 3 keyed on a
CALL (a local alias walked past), rebuild 4 on the SYMBOL but within one
function BODY (the alias hoisted to type scope), rebuild 5 on one FILE (the
defect moved to a sibling file). The scope is now every Swift file the
repository tracks, via `git ls-files` — the repository's own list, not the
machine's. There is nowhere left to move.

P3 — the allocation rule matched `\bstrdup\b` alone under a heading reading
"EVERY ALLOCATION", so `strndup`, `malloc` and `calloc` walked past a cell
whose name promised otherwise. It is now the family, and the wrapper is
`guard let` / `if let` rather than one spelling.

P2 — the scanner blanked string CONTENTS, which also blanked interpolated
EXPRESSIONS, so `_ = "\(posix_spawn_file_actions_addclose(&a, 5))"` compiled,
ran, and was invisible to every rule. Blanking strings was itself rebuild
5's mitigation for a different escape: the fix introduced the hole.

`scannableSource` is now a real Swift lexer — nested block comments, plain /
raw (`#"…"#`, any hash count) / multiline strings, correct escape and
interpolation handling per hash count — because three of five rebuilds were
defeated THROUGH the scanner rather than around it, and because refusing
files with raw or multiline literals is not available: the fence must cover
`CLIHandler.swift`, which has both. It carries 8 cells of its own, each
asserting newline count so a scanner that eats a newline cannot send a
reader to the wrong line.

VERIFIED, 10 variants built and run across two files:
  RED   unchecked strdup in CLIHandler (P1) · call inside an interpolation
        (P2) · unchecked strndup · unchecked malloc (P3, both)
        alias at type scope · raw literal + laundered call · bare setpgroup
  GREEN module-qualified · wrapper split across lines · `if let` allocation
10/10, 0 mismatches. Per-rule vacuity floors: 7 spawn symbols, 2 allocations,
plus a file-count floor and a required-membership check for the two files
that actually hold a guarded allocation.

GitCommandRunnerTests + CLIHandlerTests 42/42.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c57a0d7a14

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

event.entries.contains(where: {
($0.name == "refs" || $0.name == "reftable") && $0.kind == .directory
}),
resolver.bareRepositoryGitDirectory(at: event.directory) != nil {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Validate bare repositories through the walked directory

When the visited directory or one of its ancestors is renamed and replaced with a symlink after the walker produced event.entries, this call reopens event.directory by absolute path instead of inspecting the directory descriptor that supplied those entries. bareRepositoryGitDirectory consequently probes and reads HEAD/config from the replacement (its O_NOFOLLOW applies only to those leaf files), and the scanner can then run git worktree list against a repository outside the configured root or block on an unresponsive replacement. Bind the bare-shape validation to the walk's open directory, or re-establish descriptor-relative containment and identity before reading metadata.

Useful? React with 👍 / 👎.

Comment on lines 491 to 493
} catch {
// `env` itself is missing/unrunnable — the same class of answer
// as "git is not on PATH", never a silent zero.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve transient spawn failures as retryable

When spawn setup or posix_spawn fails transiently with errors such as ENOMEM or EAGAIN, this catch collapses the failure into .gitUnavailable. Downstream, GitWorktreeScanner withdraws every result and emits its .toolUnavailable issue, while clean-time paths tell the user to retry once Git is installed, even though Git was never proven absent; the new unavailabilityIsDefinitive distinction is not consulted there. Keep transient launch failures in a retryable execution-failure outcome and reserve .gitUnavailable for the definitive /usr/bin/env exit-127 case.

Useful? React with 👍 / 👎.

…e claims that do not reproduce (gate r4 P6, P7, P8)

P7 — the SpaceScanner probe anchor shipped ROTTED through 1a3bb64, the very
commit whose job was repointing anchors. That commit measured the +14 shift
correctly and repointed six anchors; it missed this one. The integrity cell
stayed green because it asks only whether the excerpt lies SOMEWHERE inside
the cited range, and 19 lines of range absorbed 14 lines of drift — so six
citing sites pointed at a doc-comment continuation instead of the probe.
Repointed to :2102-2120 at all six.

The tolerance that hid it is now disclosed precisely, with the known fix
named: carry each excerpt's expected OFFSET within its range and assert it
exactly. Measured across the 58 rows, offsets legitimately run 0 to 58, so
no fixed tolerance works and the offsets must be recorded per row — a schema
migration that belongs in its own increment rather than bolted onto a
review-fix round. (Writing the historical range into that disclosure made
the gate red on it, correctly: default-deny treats any `File.swift:NNN`
spelling as a live pin. Respelled as prose.)

P6 — the byte-limit disclosure said a file past the limit "stays
UNDISCOVERED … never a refusal dressed as retryable". True of
`gitConfigByteLimit`, whose one path treats nil as "not a bare repository".
FALSE of `gitPointerByteLimit` at two sites: a `.git` pointer past it yields
`ambiguous` ("Re-scan once that path is settled") and a `gitdir` back-link
past it yields `.incomplete` and then "the prunable set is not provably
complete". The limit is a FIXED CONSTANT, so for that cause a retry can
never differ — a permanent strand wearing a retryable message, the class
this project refuses everywhere else. Kept (the messages are shared with
genuinely transient causes, and no git writes a 64 KiB pointer file) and now
disclosed at the constant AND at both refusal sites.

P8 — two evidence claims in 70f4376 do not reproduce, both mine:
  (a) "three fixtures keyed on provider CALL ORDER needed re-arming" — the
      diff contains exactly TWO, and the commit then enumerates two.
  (b) "the mutant … crediting 4096 bytes of a tree it never touched" —
      measured: the entry is `exactBytes: 0, estimatedUpToBytes: 0`, because
      the sizer's size read lands after the rename. The defect is unchanged
      and no smaller — a stranger nobody inspected is destroyed and reported
      as SUCCESS — but the bytes half was wrong, and a wrong detail in an
      evidence note is how the next round is sent looking in the wrong place.
Corrected at the cell, where a future round reads, not only here.

CacheCleanerTests 110/110, SourceAnchorIntegrityTests 6/6,
GitWorktreeInventoryTests + anchors 56/56.
…4 P5)

`smallRegularFile` shipped with nine cells and they proved nothing about
whether anything USES it: the gate reverted two of the eight converted sites
to their pre-PR `String(contentsOf:)` / `Data(contentsOf:)` shape and the
full 1692-cell suite stayed green.

The steady state is not the gap, which is why this was easy to miss. Every
one of these sites still asks `probeKind` first, so a symlink merely SITTING
at the name is refused under either shape. What the descriptor buys is the
RACE — the probe answers about a path and a path-based read then resolves
that path AGAIN — so a cell has to stage the window. These fixtures plant the
symlink on the way out of the probe, truthfully answering `.regularFile`
about a file that IS one, and swapping only the timing.

  MUTATION, measured: reverting the `HEAD` read reds
  `…ABareRepoProbeDoesNotFollowASymlinkPlantedAfterTheProbe` and nothing
  else; reverting `pointerPath`'s read reds
  `…AWorktreePointerReadDoesNotFollowALatePlantedSymlink` and nothing else.

COVERAGE IS TWO OF EIGHT, disclosed at the file rather than implied away.
The other six — including `WorktreeReclaimPerformer.headWitness`, which its
own converting commit called "what the reclaim proves the far side against"
— still have no call-site cell. The two here are the two distinct mechanisms;
the rest being the same shape is a reason to expect them correct, not
evidence that they are.

Both cells carry a control that must resolve with nothing swapped, and the
second one earned it twice over: its first version was GREEN AND VACUOUS,
answering nil because the fixture wrote a `gitdir:` prefix into the admin
back-link where real git writes a bare path. Without the control it would
have reported the read as guarded when nothing had been read at all — the
same failure shape as the hollow cell in gate r3.

GitMetadataReadCallSiteTests + GitWorktreeInventoryTests +
SmallRegularFileReadTests 61/61.
…onger indexes

`StrandFenceTests.testNoLoopBoundIndexSubscriptCanStrandTheRun` flagged seven
integer subscripts in the scanner and tally added for gate r4 — correctly: a
loop-bound index traps and kills the PROCESS, not the cell, which is how 985
and 493 cells were skipped at earlier rounds.

`XCTUnwrapElement` is the sanctioned idiom but it is an assertion helper, and
a lexer's inner loop is the wrong place for one. The scanner now consumes an
`ArraySlice` from the front — `first`, `starts(with:)`, `prefix`, `dropFirst`
— so it cannot read past the end AT ALL. Bounds safety is now a property of
the operations rather than of my arithmetic, which is a better answer than
either subscripting carefully or asserting per character. The per-rule tally
is keyed by rule name instead of indexed for the same reason.

Re-verified after the rewrite, because a scanner rewrite invalidates every
result that depended on it: the same 10 fence variants behave identically
(9 measured here, the `if let` reformat measured separately as GREEN), the
scanner's own 8 cells pass, StrandFenceTests 14/14, GitCommandRunnerTests
41/41.

Full suite: exit 0, 1702 executed, 3 skipped, 0 failures, "All tests passed"
printed once, 0 error lines — whole log captured and grepped, not tailed.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 21c48b523a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +462 to +464
for rawLine in configContents.split(
whereSeparator: \.isNewline
) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Honor included overrides before declaring a repository bare

When the primary config contains core.bare = true followed by an [include] path = ... whose included config sets core.bare = false, Git treats the repository as non-bare, but this loop examines only the primary configContents and returns true. Fresh evidence after the core-section/last-value fix is that include directives remain ignored: the directory is added as .bareRepository, then the non-bare first record from worktree list fails crossValidate in GitWorktreeScanner and produces a recurring .unreadable issue for a valid repository the scanner intends to leave undiscovered. Either resolve includes safely or fail closed when an include could override the local value.

Useful? React with 👍 / 👎.

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