From c50cde348254f35a750758863c410e8506a68f42 Mon Sep 17 00:00:00 2001 From: Bonanza Date: Thu, 27 Aug 2026 06:30:24 -0700 Subject: [PATCH 01/48] =?UTF-8?q?fix(safety):=20bind=20the=20leaf=20where?= =?UTF-8?q?=20expecting:=20is=20nil=20=E2=80=94=20contents=20mode=20and=20?= =?UTF-8?q?the=20no-revalidator=20item=20arm=20(fn-4.21)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- Sources/Cacheout/Cleaner/CacheCleaner.swift | 271 ++++++++--- .../Cacheout/Cleaner/DepthSafeRemoval.swift | 79 +++- Tests/CacheoutTests/CacheCleanerTests.swift | 431 +++++++++++++++++- 3 files changed, 708 insertions(+), 73 deletions(-) diff --git a/Sources/Cacheout/Cleaner/CacheCleaner.swift b/Sources/Cacheout/Cleaner/CacheCleaner.swift index d9c7249..4cd10ba 100644 --- a/Sources/Cacheout/Cleaner/CacheCleaner.swift +++ b/Sources/Cacheout/Cleaner/CacheCleaner.swift @@ -798,11 +798,16 @@ actor CacheCleaner { // revalidator does not consider applicable (a sweep entry whose // classifier declined the clean promise, which reaches a deletion // ONLY through conscious per-item confirmation against DISPLAYED - // caution evidence). Neither has an inspection verdict to bind a - // disposal to, so neither gets one — the guards they rest on are the - // container admission, the containment chain, the deny list and the - // mount doctrine. This is not a way to skip the binding; it is the - // absence of anything to bind to. + // caution evidence). Neither has an inspection VERDICT to bind a + // disposal to, so neither gets one. The sentence that used to + // continue here — the guards they rest on are the container + // admission, the containment chain, the deny list and the mount + // doctrine, full stop — under-claimed since fn-4.21: a verdict is + // not the only bindable thing, and the deletion now also binds the + // OBJECT standing at the name when it first reads it + // (`removeGuardedItem`'s `boundTarget`; `deleteGuardedChild`'s + // `boundChild`). What `.unestablished` withholds is any promise + // about the object's CONTENT — only a revalidator can make one. guard let revalidator = preDeleteRevalidators[item.scannerID] else { return .proceed(inspected: .unestablished) } guard item.requiresPreDeleteRevalidation @@ -1127,11 +1132,40 @@ actor CacheCleaner { return .failed(error.localizedDescription) } - // Already gone = skip. Decided by OUR probe: an absent leaf yields an - // EMPTY SizeReport indistinguishable from an empty directory, so - // "already gone" must never be inferred from the report. - if provider.probeKind(of: child) == .absent { + // THE LEAF BINDING, TAKEN AT THE PIPELINE'S FIRST READ OF THE CHILD + // (fn-4.21). Contents mode runs no probe, so no VERDICT about this + // child exists — but an OBJECT does, and this is the moment the + // pipeline first reads it, so this is where what-is-measured and + // what-is-destroyed must be pinned to each other. The child's kind + // and inode are read by `TrashDisposal.boundLeaf` under the SAME + // admitted container the removal is later proved against (the r18 + // `BoundObject` shape, through the same function, so two readings + // cannot disagree), and the permanent arm below re-proves the pair + // on the far side of the queue hop. Measured before this binding + // existed, on the deterministic swap cell: a stranger renamed onto + // the child's own path after the measurement was destroyed with + // `expecting: nil` and the measured tree's 4096 bytes reported + // freed, `errors=[]` + // (`testContentsModeChildSwappedInsideTheWindowIsRefused`). + // + // It also answers "already gone = skip", and answers it from a + // descriptor-relative read rather than a path `lstat`: an absent + // leaf yields an EMPTY SizeReport indistinguishable from an empty + // directory, so "already gone" must never be inferred from the + // report — and an ENOENT here (the leaf, or the whole enumerated + // root, vanished since the listing) is the same skip the path probe + // used to produce. + let boundChild: FileSystemIdentityProvider.ChildFacts + do { + boundChild = try TrashDisposal.boundLeaf( + of: child, containedIn: admittedParent, provider: provider + ) + } catch let failure as DepthSafeRemoval.Failure + where failure.cause == .posix(ENOENT) + { return .skippedAlreadyGone + } catch { + return refusedChild(error, child: child, label: label) } // Measure → register (Phase 1) BEFORE deletion. Known inodes @@ -1181,6 +1215,21 @@ actor CacheCleaner { // container, binds the child under that descriptor, and // proves what the disposal actually took on the far side of // a call it cannot be given a descriptor for. + // + // RESIDUAL, STATED RATHER THAN IMPLIED (fn-4.21): this + // binding is taken at the DISPOSAL'S entry, not at the + // pipeline's first read of the child the way the permanent + // arm's now is — `boundChild` above is deliberately NOT + // handed to it. A child swapped between the measurement and + // this call is therefore admitted (`.whateverStandsThere`) + // and MOVED, with the measured bytes reported. What bounds + // that residual is the disposal itself: the move is into the + // user's REVERSIBLE Trash and the after-proof + rollback + // hold, so the cost is a recoverable wrong entry — never the + // unrecoverable destruction the permanent arm's binding + // exists to refuse. Feeding `boundChild` through would need + // a new `LeafAdmission` shape and its own cells; recorded at + // fn-4.21 as out of that task's five-site scope. try await TrashDisposal.dispose( child, containedIn: admittedParent, provider: provider, via: { try await self.trash($0, provingImmediatelyBefore: $1) } @@ -1189,58 +1238,87 @@ actor CacheCleaner { // NO INSPECTION VERDICT TO BIND TO, and the call site says // so. Contents mode runs no user-data probe (only the // orphaned-caches sweep carries a clean promise), so there - // is no inspected object here — which is precisely why the - // CONTAINER binding below is not optional for this arm: with - // `expecting: nil` there is nothing else that can notice the - // folder these children were enumerated from being swapped. + // is no inspected object here — `expecting: nil` is a + // statement of that absence, never a licence. The CONTAINER + // binding is not optional for this arm, and since fn-4.21 it + // is not alone: what `nil` leaves unproved — WHICH OBJECT + // stands at the child's name — is carried by `boundChild`, + // captured at this pipeline's first read of the child. + // + // The claim that stood here — "every proposition this arm + // carries is the container binding, and `DepthSafeRemoval` + // re-proves exactly that on the far side, so nothing further + // crosses the hop" — was the same reasoning PR #460 r18 + // measured FALSE for the worktree seam, and it was false + // here the same way: measured on the deterministic swap + // cell, a stranger renamed onto the child's path inside the + // window was destroyed with the measured tree's bytes + // reported. So this arm's OWN proposition — the leaf it + // measured is the leaf it destroys — rides across in the + // proof closure, re-read by the SAME `boundLeaf` under the + // SAME admitted container immediately before the removal. try await Self.removeItemConcurrently( at: child, expecting: nil, provider: provider, containedIn: admittedParent, - // NOTHING FURTHER TO PROVE PAST THE HOP, STATED RATHER - // THAN DEFAULTED (PR #460 codex r7, D1). Every - // proposition this arm carries is the container binding, - // and `DepthSafeRemoval` re-proves exactly that from a - // descriptor on the far side. The worktree arm passes a - // real closure because it carries three propositions — - // which checkout, the lock, HEAD — that live outside that - // file's vocabulary. - provingImmediatelyBefore: LastInstantProof.nothingFurther.run + provingImmediatelyBefore: Self.provedStillTheBoundLeaf( + boundChild, at: child, + containedIn: admittedParent, provider: provider + ) ) } } catch { - if error is PathGuardError { - logRefusal( - label: label, tag: Self.refusalTag(error), - detail: "\(child.path): \(error.localizedDescription)" - ) - } else if let failure = error as? DepthSafeRemoval.Failure, - failure.cause == .notTheAdmittedContainer { - // The category root these children were enumerated from was - // replaced under the loop. Same tag item mode uses for the - // same event, so the cleanup log has ONE word for it. - logRefusal( - label: label, tag: "container-drift", - detail: "\(child.path): \(error.localizedDescription)" - ) - } else if let failure = error as? TrashDisposal.Failure { - // The swap landed inside `trashItem`'s own resolution, so it - // was caught AFTER the move and undone. Same event as the one - // above, one disposal over — and the same tag item mode uses - // for it, INCLUDING the container case (r13). - logRefusal( - label: label, tag: Self.trashRefusalTag(failure), - detail: "\(child.path): \(error.localizedDescription)" - ) - } // Failed deletions never accept — their registrations remain for // siblings to transfer later (R8). - return .failed(error.localizedDescription) + return refusedChild(error, child: child, label: label) } // Phase 2: transfer canonical bytes exactly once, after success only. return .accepted(await registry.acceptSuccessful(token)) } + /// ONE place that turns a throw from the guarded-child pipeline into the + /// child's outcome (fn-4.21) — the binding's refusal and the deletion's + /// refusal must not be able to word or tag the same event differently. + private func refusedChild( + _ error: Error, child: URL, label: String + ) -> ChildOutcome { + if error is PathGuardError { + logRefusal( + label: label, tag: Self.refusalTag(error), + detail: "\(child.path): \(error.localizedDescription)" + ) + } else if let failure = error as? DepthSafeRemoval.Failure, + failure.cause == .notTheInspectedObject { + // The child at this name is not the object this pipeline bound + // when it first read it (fn-4.21's leaf binding, or a leaf + // binding one disposal down). Same tag item mode uses for the + // same event, so the log has ONE word for a swapped LEAF. + logRefusal( + label: label, tag: "content-drift", + detail: "\(child.path): \(error.localizedDescription)" + ) + } else if let failure = error as? DepthSafeRemoval.Failure, + failure.cause == .notTheAdmittedContainer { + // The category root these children were enumerated from was + // replaced under the loop. Same tag item mode uses for the + // same event, so the cleanup log has ONE word for it. + logRefusal( + label: label, tag: "container-drift", + detail: "\(child.path): \(error.localizedDescription)" + ) + } else if let failure = error as? TrashDisposal.Failure { + // The swap landed inside `trashItem`'s own resolution, so it + // was caught AFTER the move and undone. Same event as the one + // above, one disposal over — and the same tag item mode uses + // for it, INCLUDING the container case (r13). + logRefusal( + label: label, tag: Self.trashRefusalTag(failure), + detail: "\(child.path): \(error.localizedDescription)" + ) + } + return .failed(error.localizedDescription) + } + // MARK: - Item mode (.removeItem, R15) /// Does `target` STILL name the object the pre-delete probe inspected? @@ -1454,6 +1532,36 @@ actor CacheCleaner { directory: target.deletingLastPathComponent(), displayPath: target.path, provider: provider ) + // WITH NO LEAF VERDICT, BIND WHAT STANDS AT THE NAME (fn-4.21). + // When `probedObject` is nil no inspection ran, and until this + // binding existed the permanent arm's leaf open was proved + // against NOTHING — the same "the container binding is what + // covers the leaf" reasoning r18 measured false for the worktree + // seam, measured false here too: a stranger renamed onto the + // target's path after the rechecks below was destroyed with + // success reported + // (`testItemModeNilProbeTargetSwappedInsideTheWindowIsRefused`). + // So the object is read HERE — kind+inode under the container + // descriptor just proved, before the rechecks the binding must + // cover — and re-proved by the same read on the far side of the + // hop. What it pins is delete-time standing, not scan-time + // content: only a revalidator can promise content, and a scanner + // that registers none has nothing content-shaped to bind + // (`preDeleteOutcome`'s `.unestablished`). + // + // An ENOENT here is the ghost target's frozen behaviour, one + // read earlier: the removal's own leaf open would have raised + // the identical `Failure(.posix(ENOENT))` a moment later. + // When a verdict DOES exist, the verdict is the leaf binding — + // it travels in `expecting:` and is proved against the opened + // inode by the removal itself, so no second binding is taken. + let boundTarget: FileSystemIdentityProvider.ChildFacts? = + probedObject == nil + ? try TrashDisposal.boundLeaf( + of: target, containedIn: admittedParent, + provider: provider + ) + : nil // TOCTOU narrowing, immediately pre-delete: the SAME no-follow // + snapshot-identity admission re-runs (a container swapped // between the checks above and here is refused), then the @@ -1528,6 +1636,18 @@ actor CacheCleaner { // disposal binds the leaf under it. Once it does, this arm // rolls back like the other one — which is the second // reason the old note was wrong about itself. + // + // RESIDUAL, STATED (fn-4.21): `boundTarget`, captured + // before the rechecks, is deliberately NOT handed to + // this arm — the disposal re-binds at its own entry, so + // a target swapped between the capture and here is + // admitted and MOVED. Same bound as contents mode's + // Trash arm, for the same reason: the move is into the + // reversible Trash under the after-proof + rollback, so + // the residual's cost is a recoverable wrong entry, and + // closing it needs a new `LeafAdmission` shape with its + // own cells — recorded at fn-4.21 as out of that task's + // five-site scope. try await TrashDisposal.dispose( target, containedIn: admittedParent, provider: provider, @@ -1546,11 +1666,24 @@ actor CacheCleaner { try await Self.removeItemConcurrently( at: target, expecting: probedObject, provider: provider, containedIn: admittedParent, - // NOTHING FURTHER TO PROVE PAST THE HOP (PR #460 codex r7, - // D1): this arm's two propositions are the container - // binding and `probedObject`, and `DepthSafeRemoval` - // re-proves BOTH from descriptors on the far side. - provingImmediatelyBefore: LastInstantProof.nothingFurther.run + // WHAT CROSSES THE HOP DEPENDS ON WHICH BINDING THIS ARM + // HOLDS (fn-4.21; the claim that stood here — "this + // arm's two propositions are the container binding and + // `probedObject`, re-proved on the far side" — was true + // only when `probedObject` existed, and its nil case was + // the defect). With a verdict, the removal re-proves + // container AND leaf itself and nothing further rides + // across. With none, the leaf proposition is + // `boundTarget`, which only this caller can state — so + // it rides across and is re-proved immediately before + // the removal, the same way the worktree seam carries + // its own propositions. + provingImmediatelyBefore: boundTarget.map { + Self.provedStillTheBoundLeaf( + $0, at: target, + containedIn: admittedParent, provider: provider + ) + } ?? LastInstantProof.nothingFurther.run ) } } catch { @@ -1902,6 +2035,40 @@ actor CacheCleaner { } } + /// THE LEAF BINDING'S FAR SIDE, SPELLED ONCE (fn-4.21). For a deletion + /// with NO inspection verdict (`expecting: nil` — all of contents mode, + /// and every item whose scanner registers no revalidator), the caller + /// binds WHAT STANDS AT THE NAME when it first reads it + /// (`TrashDisposal.boundLeaf` under the proved container) and this + /// closure re-reads it THE SAME WAY, under the SAME `admittedParent`, + /// on the far side of the queue hop, immediately before the removal. + /// + /// This is the r18 mechanism (`WorktreeReclaimPerformer.BoundObject`, + /// re-proved in its `LastInstantProof`; `TrashDisposal.disposeBoundLeaf`, + /// step 2), reached through the same function — one reading on each side + /// of the hop, produced identically, so the two cannot disagree about + /// what the object IS. A difference is a swap inside the window and + /// nothing else, and it throws the SAME `.notTheInspectedObject` the + /// verdict-carrying arms throw for the same event, so the report and the + /// log word it identically (`content-drift`). + nonisolated private static func provedStillTheBoundLeaf( + _ bound: FileSystemIdentityProvider.ChildFacts, + at target: URL, + containedIn admittedParent: DepthSafeRemoval.AdmittedParent, + provider: FileSystemIdentityProvider + ) -> () throws -> Void { + { + let atTheInstant = try TrashDisposal.boundLeaf( + of: target, containedIn: admittedParent, provider: provider + ) + guard atTheInstant == bound else { + throw DepthSafeRemoval.Failure( + path: target.path, cause: .notTheInspectedObject, depth: 0 + ) + } + } + } + /// Move one URL to the Trash via the injectable seam (production: /// `FileManager.trashItem`, which requires the main actor), answering /// WHERE IT LANDED — `nil` when the disposal would not say. diff --git a/Sources/Cacheout/Cleaner/DepthSafeRemoval.swift b/Sources/Cacheout/Cleaner/DepthSafeRemoval.swift index 68b6fa9..6787245 100644 --- a/Sources/Cacheout/Cleaner/DepthSafeRemoval.swift +++ b/Sources/Cacheout/Cleaner/DepthSafeRemoval.swift @@ -320,26 +320,50 @@ enum DepthSafeRemoval { /// bind to; it is not a way to skip the check. /// /// THE PARAMETER HAS NO DEFAULT, AND EVERY CALL SITE STATES ITS `nil` - /// (PR #458 review; the enumeration RE-TAKEN at PR #460 codex r18). + /// (PR #458 review; the enumeration RE-TAKEN at PR #460 codex r18, and + /// AGAIN at fn-4.21, which found the r18 take listing three sites of + /// five). /// /// The r458 wording said "both call sites" and closed with "if a third /// call site appears, either it states its `nil` or this paragraph stops /// being true and must change with it". A third appeared at r7 — the /// worktree performer's `removeTree` seam — and the paragraph did not /// change with it, which is the shape it was written to prevent. The - /// standing enumeration, from `grep -rn "expecting:" Sources`: + /// standing enumeration, from `grep -rn "expecting:" Sources` (five call + /// sites; `TrashDisposal.dispose(_:expecting:…)` is the disposal's own + /// spelling of this parameter and binds its leaf itself, on both sides + /// of its own hop): /// - /// 1. `CacheCleaner.deleteGuardedChild` (contents mode) — a literal `nil` - /// under a paragraph saying contents mode runs no probe. SOUND: the - /// population is ENUMERATED CHILDREN of a folder this call opened and - /// proved, so no per-child verdict exists anywhere to bind to, and the - /// container binding (`containedIn`) is what covers the leaf. That is - /// why the container parameter is not optional for this arm. - /// 2. `CacheCleaner.removeGuardedItem` (item mode) — `probedObject`, - /// which is `nil` exactly when the item's scanner registers no - /// revalidator or the seam answered `.unestablished`. SOUND for the - /// same reason and under the same binding; when a verdict DOES exist - /// it is passed and proved here. + /// 1. `CacheCleaner.deleteGuardedChild` (contents mode) — a literal + /// `nil`: contents mode runs no probe, so no VERDICT exists for any + /// enumerated child. The claim that stood here — SOUND, because "the + /// container binding (`containedIn`) is what covers the leaf" — was + /// the same reasoning r18 measured FALSE for the worktree seam, and + /// fn-4.21 measured it false here the same way: a child renamed aside + /// at its own path inside the window, container untouched, left a + /// stranger that this removal destroyed with `expecting: nil` while + /// the measured tree's bytes were reported freed + /// (`testContentsModeChildSwappedInsideTheWindowIsRefused`, red + /// before the binding). No verdict exists, but an OBJECT does: the + /// caller now binds what stands at the name at its FIRST READ of the + /// child (`TrashDisposal.boundLeaf` under this same admitted + /// container) and re-proves it via `provingImmediatelyBefore` on the + /// far side of the hop (`CacheCleaner.provedStillTheBoundLeaf` — the + /// r18 `BoundObject` shape through the same function, so two readings + /// cannot disagree). + /// 2. `CacheCleaner.removeGuardedItem` (item mode, permanent arm) — + /// `probedObject`, which is `nil` exactly when the item's scanner + /// registers no revalidator or the seam answered `.unestablished`. + /// When a verdict exists it is passed and proved here; when it is + /// nil the caller takes the same fn-4.21 binding as contents mode — + /// captured with `admittedParent`, before the delete-time rechecks — + /// and re-proves it past the hop + /// (`testItemModeNilProbeTargetSwappedInsideTheWindowIsRefused`, red + /// before the binding). The item-mode TRASH arm's no-verdict case + /// does not pass through this parameter at all: it takes + /// `TrashDisposal.dispose(_:containedIn:…)`, whose leaf binding is + /// taken at the disposal's own entry and after-proved (a swap in the + /// window before that entry lands in the reversible Trash, not here). /// 3. `CacheCleaner`'s `removeTree` seam for `WorktreeReclaimPerformer` /// — a literal `nil`, because `git_worktrees` registers no /// revalidator either. SOUND ONLY SINCE r18, AND IT WAS NOT BEFORE. @@ -358,7 +382,23 @@ enum DepthSafeRemoval { /// rather than inside it, so what it leaves open is this function's /// own parent open — the identical residual the LOCKED and HEAD-MOVED /// propositions have carried since r7, for the identical reason: none - /// of the three is expressible in this file's vocabulary. + /// of the three is expressible in this file's vocabulary. Since + /// fn-4.21 the two `CacheCleaner` bindings above run in the same + /// place and carry the identical residual. + /// 4. `CacheCleaner.removeItemConcurrently` — the forwarder sites 1–3 + /// reach this file through. It states nothing of its own: it carries + /// its caller's `expecting:` and `provingImmediatelyBefore:` across + /// the queue hop unchanged, which is exactly why the no-verdict + /// bindings ride in the proof closure rather than in a second + /// parameter. + /// 5. `CacheCleaner.removeGuardedItem`'s Trash arm — + /// `TrashDisposal.dispose(_:expecting: probedObject, …)`, reached + /// only inside `if let probedObject`, so `expecting:` is never nil + /// there at runtime; the disposal proves the verdict on both sides of + /// its hop (the fn-4.21 spec's claim that this site passes nil "when + /// no revalidator applies" was retired against the source: the + /// no-revalidator case takes the `containedIn:` overload in the + /// `else` arm). /// /// `provider` answers the mount question, and it is the same object the /// scanner's walk asks, so the two cannot classify a boundary @@ -648,8 +688,15 @@ enum DepthSafeRemoval { provider: FileSystemIdentityProvider, displayPath: String ) throws { - // No inspection ran (contents mode): nothing to bind to. The caller - // states this explicitly. + // No inspection ran: no VERDICT to bind to, and the caller states + // that explicitly. It is not a proof holiday — each of the three + // nil-stating production callers carries its own leaf binding + // (fn-4.21's `boundLeaf` re-read for contents mode and the + // no-revalidator item arm; the worktree performer's `BoundObject`) + // in its `provingImmediatelyBefore` closure, run on this side of + // the hop immediately before `remove` — it cannot run HERE because + // what it binds is not expressible in this file's vocabulary (see + // the standing enumeration above). guard let inspected else { return } // Only a `.directory` verdict about THIS inode admits an opened // directory: `.noDirectoryTree`, `.nonDirectoryLeaf` and diff --git a/Tests/CacheoutTests/CacheCleanerTests.swift b/Tests/CacheoutTests/CacheCleanerTests.swift index fb9636f..b8eeeac 100644 --- a/Tests/CacheoutTests/CacheCleanerTests.swift +++ b/Tests/CacheoutTests/CacheCleanerTests.swift @@ -233,7 +233,10 @@ final class CacheCleanerTests: XCTestCase { var mountPointPaths: Set = [] /// Paths whose probe reports `.absent` even though they exist — /// hermetic stand-in for a child vanishing between enumeration and - /// the cleaner's already-gone probe (fn-2.3 ENOENT-asymmetry tests). + /// the cleaner's already-gone read (fn-2.3 ENOENT-asymmetry tests). + /// Honored by BOTH kind seams: `probeKind` (the path probe that used + /// to be the skip's decider) and `probeChild` (the + /// descriptor-relative binding read that decides it since fn-4.21). var absentPaths: Set = [] override func identity(of url: URL) -> Identity? { @@ -263,6 +266,20 @@ final class CacheCleanerTests: XCTestCase { } return super.probeKind(of: url) } + + override func probeChild( + inDirectory descriptor: Int32, named name: String, + logical: @autoclosure () -> URL + ) -> ChildProbe { + let url = logical() + if absentPaths.contains(url.path) + || absentPaths.contains(canonicalize(url).path) { + return .absent + } + return super.probeChild( + inDirectory: descriptor, named: name, logical: url + ) + } } /// Minimal protocol conformer for the runtime-derived-admission test @@ -4823,12 +4840,24 @@ final class CacheCleanerTests: XCTestCase { XCTAssertTrue(message.contains("nothing was reported freed"), message) } - /// Removes the child inside the ONE window between the container proof - /// and the leaf bind — the `probeChild` that reads the leaf under the + /// Removes the child inside the window between the container proof and + /// a chosen leaf bind — the Nth `probeChild` that reads the leaf under a /// held container descriptor. A real `removeItem`, no sleeps. + /// + /// WHY THE CALL IS COUNTED (fn-4.21): the guarded-child pipeline now + /// takes its own binding read at its FIRST read of the child, before the + /// measurement, so the DISPOSAL's bind is the SECOND `probeChild` of the + /// child in trash mode. A vanish at call 1 is "gone before anything was + /// measured" — the already-gone SKIP; a vanish at call 2 is the + /// disposal's own window, which is what + /// `testTrashModeRefusesAChildThatVanishedBeforeItCouldBeBound` is + /// about. private final class VanishTheChildAtTheBindProvider: FileSystemIdentityProvider, @unchecked Sendable { var child: URL! + /// 1-based index of the matching `probeChild` call to vanish on. + var vanishOnCall = 1 + private var matchingCalls = 0 private(set) var vanished = false override func probeChild( @@ -4839,8 +4868,11 @@ final class CacheCleanerTests: XCTestCase { if !vanished, url.standardizedFileURL.path == child.standardizedFileURL.path { - vanished = true - try? FileManager.default.removeItem(at: child) + matchingCalls += 1 + if matchingCalls >= vanishOnCall { + vanished = true + try? FileManager.default.removeItem(at: child) + } } return super.probeChild( inDirectory: descriptor, named: name, logical: url @@ -4875,6 +4907,10 @@ final class CacheCleanerTests: XCTestCase { let provider = VanishTheChildAtTheBindProvider() provider.child = child + // Call 1 is the pipeline's own fn-4.21 binding read (a vanish there + // is the already-gone skip); call 2 is the DISPOSAL's bind — the + // window this cell is about. + provider.vanishOnCall = 2 let recorder = TrashRecorder() let cleaner = CacheCleaner( @@ -4907,6 +4943,52 @@ final class CacheCleanerTests: XCTestCase { XCTAssertTrue(message.contains(child.path), message) } + /// THE OTHER SIDE OF THE COUNTED CALL (fn-4.21): a child that vanishes + /// at the PIPELINE's binding read — before anything was measured — is + /// the already-gone SKIP, exactly as a child absent at the old path + /// probe was: no entry, no error, and the Trash never consulted. + func testTrashModeChildVanishedAtThePipelineBindIsSkipped() async throws { + let home = try makeTempDir("home") + let base = try makeTempDir() + let trashDir = try makeTempDir("fake-trash") + defer { + try? FileManager.default.removeItem(at: home) + try? FileManager.default.removeItem(at: base) + try? FileManager.default.removeItem(at: trashDir) + } + let root = base.appendingPathComponent("cache-root") + let child = root.appendingPathComponent("entry") + try FileManager.default.createDirectory( + at: child, withIntermediateDirectories: true + ) + try writeFile(child.appendingPathComponent("ours.bin"), bytes: 4096) + + let provider = VanishTheChildAtTheBindProvider() + provider.child = child + provider.vanishOnCall = 1 + + let recorder = TrashRecorder() + let cleaner = CacheCleaner( + home: home, containerRoots: [], provider: provider, + trashHandler: makeTrashSeam(into: trashDir, recorder: recorder) + ) + let report = await cleaner.clean( + items: categoryItems( + [makeScanResult(category: makeCategory(at: root))], + home: home, provider: provider + ), + moveToTrash: true + ) + + XCTAssertTrue(provider.vanished, "the fixture never removed the child") + XCTAssertTrue(recorder.urls.isEmpty, "\(recorder.urls)") + XCTAssertTrue(report.entries.isEmpty, "\(report.entries)") + XCTAssertTrue( + report.errors.isEmpty, + "gone before anything was measured is a SKIP: \(report.errors)" + ) + } + // MARK: - The cleanup log's own open must not block (PR #459 review r4) /// A zero-record `.removeContents` item whose refusal is LOGGED — the @@ -5272,4 +5354,343 @@ extension CacheCleanerTests { XCTAssertTrue(FileManager.default.fileExists(atPath: payload.path), "and its target tree is untouched") } + + // MARK: - fn-4.21: the leaf binding where `expecting:` is nil + + /// Deterministic child-swap fixture for CONTENTS mode — the mainline + /// deletion path (`CategoryScanner` is default-selected). + /// + /// The window it aims at: the child is measured, then the deletion hops + /// to a background queue and opens the ADMITTED CONTAINER (proved) and + /// the LEAF (with `expecting: nil`, proved against nothing). The swap is + /// two real `rename(2)`-class mutations INSIDE the container — the child + /// renamed aside at its own path, a stranger created at the name — with + /// the container itself untouched, so the container binding passes by + /// construction and only a LEAF binding can refuse. + /// + /// Single-threaded and deterministic: the swap fires inside a provider + /// question the pipeline is known to ask at the right instant — the + /// first identity-of-descriptor question whose answer IS the container + /// (the deletion proving the folder it is about to resolve the leaf in), + /// gated on the measurement having already read the payload + /// (`identity(of:)` of the marker file, the sizer's claim read). No + /// sleeps, no races to win. + private final class ContentsChildSwapProvider: FileSystemIdentityProvider { + var container: URL! + var child: URL! + /// A file INSIDE the child whose claim read marks "the measurement + /// has happened" — the swap must land after it, never before. + var markerLeafName: String! + var stash: URL! + private var armed = false + private var measureSeen = false + private(set) var swapped = false + private var containerIdentity: Identity? + + func arm() { + containerIdentity = super.identity(of: container) + armed = true + } + + override func identity(of url: URL) -> Identity? { + if armed, url.lastPathComponent == markerLeafName { + measureSeen = true + } + return super.identity(of: url) + } + + override func identity(ofDescriptor fd: Int32) -> Identity? { + let answer = super.identity(ofDescriptor: fd) + if armed, measureSeen, !swapped, answer == containerIdentity { + swapped = true + try? FileManager.default.moveItem(at: child, to: stash) + try? FileManager.default.createDirectory( + at: child, withIntermediateDirectories: true + ) + try? Data("stranger".utf8).write( + to: child.appendingPathComponent("stranger.bin") + ) + } + return answer + } + } + + private func makeContentsSwapFixture( + _ label: String = #function + ) throws -> (home: URL, root: URL, victim: URL, marker: String) { + let home = try makeTempDir(label) + let root = home.appendingPathComponent("Library/Caches/fixture-cache") + try FileManager.default.createDirectory( + at: root, withIntermediateDirectories: true + ) + let victim = root.appendingPathComponent("victim-child") + try FileManager.default.createDirectory( + at: victim, withIntermediateDirectories: true + ) + let marker = "payload-\(UUID().uuidString.prefix(8)).bin" + try writeFile(victim.appendingPathComponent(marker), bytes: 4096) + return (home, root, victim, marker) + } + + /// **THE fn-4.21 P1, CONTENTS MODE, PERMANENT ARM** (PR #460 r18 + /// adversarial verification; pre-existing on `origin/main`). + /// + /// `deleteGuardedChild` passes `expecting: nil`, and + /// `DepthSafeRemoval.proveInspectedRoot` returns immediately on nil — so + /// before the fix NOTHING between the measurement and the `unlinkat` + /// examined the child: a stranger renamed onto the child's own path + /// inside the window was destroyed, and the report carried the byte + /// count of the tree the app had measured and then never touched. + /// + /// The binding demanded here is the r18 `BoundObject` shape: the child's + /// kind+inode read by `TrashDisposal.boundLeaf` under the SAME admitted + /// container the removal is proved against, captured at the pipeline's + /// FIRST READ of the child and re-proved by the same function on the far + /// side of the queue hop. + func testContentsModeChildSwappedInsideTheWindowIsRefused() async throws { + let (home, root, victim, marker) = try makeContentsSwapFixture() + defer { try? FileManager.default.removeItem(at: home) } + + let provider = ContentsChildSwapProvider() + provider.container = root + provider.child = victim + provider.markerLeafName = marker + provider.stash = home.appendingPathComponent("stash") + + let cleaner = CacheCleaner( + home: home, containerRoots: [], provider: provider + ) + provider.arm() + let report = await cleaner.clean( + items: categoryItems( + [makeScanResult(category: makeCategory(at: root))], + home: home, provider: provider + ), + moveToTrash: false + ) + + XCTAssertTrue(provider.swapped, "the fixture never armed the swap") + XCTAssertTrue( + FileManager.default.fileExists( + atPath: victim.appendingPathComponent("stranger.bin").path + ), + "the stranger renamed onto the child's path was DELETED without " + + "any check having examined it" + ) + XCTAssertTrue( + FileManager.default.fileExists( + atPath: provider.stash.appendingPathComponent(marker).path + ), + "the measured tree is intact at the stash path" + ) + XCTAssertTrue( + report.entries.isEmpty, + "reported bytes for a tree it never touched: \(report.entries)" + ) + XCTAssertEqual(report.errors.count, 1, "\(report.errors)") + let message = try XCTUnwrap(report.errors.first?.message) + // WHICH refusal fired, not just that one did: the leaf binding's + // (`.notTheInspectedObject`), never the container's. + XCTAssertTrue( + message.contains("no longer the one that was inspected"), message + ) + XCTAssertTrue( + logContents(home: home).contains("REFUSED [content-drift]"), + logContents(home: home) + ) + } + + /// CONTROL for the cell above: identical fixture, identical provider + /// double, never armed — the clean must SUCCEED, so the swap cell's + /// refusal is evidenced to come from the swap and not from the fixture + /// refusing for its own reasons. + func testContentsModeSwapFixtureUnarmedControlCleans() async throws { + let (home, root, victim, marker) = try makeContentsSwapFixture() + defer { try? FileManager.default.removeItem(at: home) } + + let provider = ContentsChildSwapProvider() + provider.container = root + provider.child = victim + provider.markerLeafName = marker + provider.stash = home.appendingPathComponent("stash") + // NOT armed. + + let cleaner = CacheCleaner( + home: home, containerRoots: [], provider: provider + ) + let report = await cleaner.clean( + items: categoryItems( + [makeScanResult(category: makeCategory(at: root))], + home: home, provider: provider + ), + moveToTrash: false + ) + + XCTAssertFalse(provider.swapped) + XCTAssertTrue(report.errors.isEmpty, "\(report.errors)") + XCTAssertEqual(report.entries.count, 1) + XCTAssertFalse( + FileManager.default.fileExists(atPath: victim.path), + "the child is deleted on the unarmed control" + ) + } + + /// Deterministic target-swap fixture for ITEM mode with NO revalidator + /// (`probedObject == nil` — `expecting:` nil at runtime). + /// + /// The swap fires inside the delete-time RECHECKS: the first + /// path-identity question about the target asked AFTER the container + /// binding was read from a descriptor + /// (`DepthSafeRemoval.admittedParent`, the recheck sequence's first + /// act). That instant is after the fn-4.21 binding captures the leaf and + /// before the removal, so an unbound deletion destroys the stranger and + /// a bound one refuses. + private final class ItemNilProbeSwapProvider: FileSystemIdentityProvider { + var target: URL! + var stash: URL! + private var armed = false + private var parentSeen = false + private(set) var swapped = false + private var parentIdentity: Identity? + + func arm() { + parentIdentity = super.identity( + of: target.deletingLastPathComponent() + ) + armed = true + } + + override func identity(ofDescriptor fd: Int32) -> Identity? { + let answer = super.identity(ofDescriptor: fd) + if armed, answer == parentIdentity { parentSeen = true } + return answer + } + + override func identity(of url: URL) -> Identity? { + guard armed, parentSeen, !swapped, + url.lastPathComponent == target.lastPathComponent + else { return super.identity(of: url) } + // Freeze the answer about the object that WAS there, then swap — + // every later path question answers about the stranger, which + // every path check accepts. + let frozen = super.identity(of: url) + swapped = true + try? FileManager.default.moveItem(at: target, to: stash) + try? FileManager.default.createDirectory( + at: target, withIntermediateDirectories: true + ) + try? Data("stranger".utf8).write( + to: target.appendingPathComponent("stranger.bin") + ) + return frozen + } + } + + /// **THE SAME DEFECT, ITEM MODE, PERMANENT ARM** — `removeGuardedItem` + /// passes `probedObject`, which is nil for every item whose scanner + /// registers no revalidator (`fixture_scanner` here, and every shipped + /// scanner without one). Before the fix the deletion's leaf open was + /// proved against NOTHING: a stranger renamed onto the target's path + /// after the container binding was captured (and after every path + /// recheck answered about the old object) was destroyed with success + /// reported. + func testItemModeNilProbeTargetSwappedInsideTheWindowIsRefused() + async throws + { + let base = try makeTempDir() + defer { try? FileManager.default.removeItem(at: base) } + let container = base.appendingPathComponent("fixture-container") + let project = container.appendingPathComponent("proj") + let target = project.appendingPathComponent("victim-item") + try FileManager.default.createDirectory( + at: target, withIntermediateDirectories: true + ) + try writeFile(target.appendingPathComponent("payload.bin"), bytes: 4096) + + let provider = ItemNilProbeSwapProvider() + provider.target = target + provider.stash = base.appendingPathComponent("stash") + + let cleaner = CacheCleaner( + home: base, containerRoots: [container], + containerSnapshot: sessionSnapshot( + of: [container], provider: provider + ), + provider: provider + ) + provider.arm() + let report = await cleaner.clean( + items: [makeRemoveItem(origin: container, target: target)], + moveToTrash: false + ) + + XCTAssertTrue(provider.swapped, "the fixture never armed the swap") + XCTAssertTrue( + FileManager.default.fileExists( + atPath: target.appendingPathComponent("stranger.bin").path + ), + "the stranger renamed onto the target's path was DELETED without " + + "any check having examined it" + ) + XCTAssertTrue( + FileManager.default.fileExists( + atPath: provider.stash + .appendingPathComponent("payload.bin").path + ), + "the measured tree is intact at the stash path" + ) + XCTAssertTrue( + report.entries.isEmpty, + "reported bytes for a tree it never touched: \(report.entries)" + ) + XCTAssertEqual(report.errors.count, 1, "\(report.errors)") + let message = try XCTUnwrap(report.errors.first?.message) + XCTAssertTrue( + message.contains("no longer the one that was inspected"), message + ) + XCTAssertTrue( + logContents(home: base).contains("REFUSED [content-drift]"), + logContents(home: base) + ) + } + + /// CONTROL for the item-mode cell: identical fixture and provider, + /// never armed — success, so the refusal above is the swap's and only + /// the swap's. + func testItemModeNilProbeSwapFixtureUnarmedControlCleans() async throws { + let base = try makeTempDir() + defer { try? FileManager.default.removeItem(at: base) } + let container = base.appendingPathComponent("fixture-container") + let project = container.appendingPathComponent("proj") + let target = project.appendingPathComponent("victim-item") + try FileManager.default.createDirectory( + at: target, withIntermediateDirectories: true + ) + try writeFile(target.appendingPathComponent("payload.bin"), bytes: 4096) + + let provider = ItemNilProbeSwapProvider() + provider.target = target + provider.stash = base.appendingPathComponent("stash") + // NOT armed. + + let cleaner = CacheCleaner( + home: base, containerRoots: [container], + containerSnapshot: sessionSnapshot( + of: [container], provider: provider + ), + provider: provider + ) + let report = await cleaner.clean( + items: [makeRemoveItem(origin: container, target: target)], + moveToTrash: false + ) + + XCTAssertFalse(provider.swapped) + XCTAssertTrue(report.errors.isEmpty, "\(report.errors)") + XCTAssertEqual(report.entries.count, 1) + XCTAssertFalse( + FileManager.default.fileExists(atPath: target.path), + "the target is deleted on the unarmed control" + ) + } } From 28eb58341f86b6c8a422fe5ae731e16f1f535029 Mon Sep 17 00:00:00 2001 From: Bonanza Date: Thu, 27 Aug 2026 06:58:14 -0700 Subject: [PATCH 02/48] fix(safety): a drain that died on a hard read error no longer ends like EOF (fn-4.24) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). --- .../Cacheout/Scanner/GitCommandRunner.swift | 106 +++++++++- .../CacheoutTests/GitCommandRunnerTests.swift | 190 ++++++++++++++++++ 2 files changed, 285 insertions(+), 11 deletions(-) diff --git a/Sources/Cacheout/Scanner/GitCommandRunner.swift b/Sources/Cacheout/Scanner/GitCommandRunner.swift index fb5d789..1f2b106 100644 --- a/Sources/Cacheout/Scanner/GitCommandRunner.swift +++ b/Sources/Cacheout/Scanner/GitCommandRunner.swift @@ -191,14 +191,16 @@ enum GitCommandOutcome: Equatable, Sendable { /// Non-zero exit, with git's own stderr (lossily decoded — stderr is a /// human message, never a path used as a deletion target). case failure(exitCode: Int32, stderr: String) - /// The invocation could not be COMPLETED within its bounds, and the full - /// termination protocol ran. Two ways in, and they are one answer on - /// purpose (PR #460 codex r18, C7): the per-invocation budget expired - /// before git exited, OR git exited but its output could not be read to - /// completion within the drain budget — which means something git spawned - /// still holds the inherited pipe, so the captured bytes may be short. - /// Both are retryable and neither may be reported as a `.success` whose - /// `stdout` a porcelain parser will count. + /// The invocation could not be COMPLETED within its bounds. Three ways + /// in, and they are one answer on purpose (PR #460 codex r18 C7; + /// fn-4.24): the per-invocation budget expired before git exited (the + /// full termination protocol ran), OR git exited but its output could + /// not be read to completion within the drain budget — something git + /// spawned still holds the inherited pipe, so the captured bytes may be + /// short — OR a drain DIED on a hard `read(2)` error, which also leaves + /// the captured bytes short. All three are retryable (a fresh invocation + /// opens fresh pipes and descriptors) and none may be reported as a + /// `.success` whose `stdout` a porcelain parser will count. case timeout /// `env` could not find git (exit 127), the launch itself failed, or the /// instance's cached availability probe already said no. @@ -299,6 +301,14 @@ final class GitCommandRunner: GitCommandRunning, @unchecked Sendable { let defaultTimeout: TimeInterval private let terminationGrace: TimeInterval private let drainJoinBudget: TimeInterval + /// How `execute` builds the drain over each pipe. TESTING SEAM + /// (fn-4.24): production always uses the default — a `PipeDrain` over + /// the pipe's own read end — and nothing in this file reassigns it. The + /// seam exists because a pipe cannot be made to fail `read(2)` HARD on + /// demand, and the died-on-error drain classes (EISDIR through a + /// directory descriptor) are only reachable through + /// `PipeDrain(readingFrom:)`. + private let drainFactory: (Pipe) -> PipeDrain // MARK: Mutable state (lock-guarded) @@ -317,13 +327,15 @@ final class GitCommandRunner: GitCommandRunning, @unchecked Sendable { executableURL: URL = GitCommandRunner.defaultExecutable, defaultTimeout: TimeInterval = GitCommandRunner.scanTimeout, terminationGrace: TimeInterval = GitCommandRunner.defaultTerminationGrace, - drainJoinBudget: TimeInterval = GitCommandRunner.defaultDrainJoinBudget + drainJoinBudget: TimeInterval = GitCommandRunner.defaultDrainJoinBudget, + drainFactory: @escaping (Pipe) -> PipeDrain = { PipeDrain(pipe: $0) } ) { self.baseEnvironment = environment self.executableURL = executableURL self.defaultTimeout = defaultTimeout self.terminationGrace = terminationGrace self.drainJoinBudget = drainJoinBudget + self.drainFactory = drainFactory } /// Production initializer — the cleaner-cloned environment. @@ -480,8 +492,8 @@ final class GitCommandRunner: GitCommandRunning, @unchecked Sendable { // Both drains start BEFORE the wait: either stream filling the // 64 KiB pipe buffer would otherwise block the child forever while // the parent waits for an exit that can never come. - let stdoutDrain = PipeDrain(pipe: stdoutPipe) - let stderrDrain = PipeDrain(pipe: stderrPipe) + let stdoutDrain = drainFactory(stdoutPipe) + let stderrDrain = drainFactory(stderrPipe) stdoutDrain.start() stderrDrain.start() @@ -559,6 +571,50 @@ final class GitCommandRunner: GitCommandRunning, @unchecked Sendable { outcome: .timeout ) } + // AND A DRAIN CAN FINISH BECAUSE `read(2)` DIED, NOT BECAUSE THE + // STREAM ENDED (fn-4.24, PR #460 round 18). A hard read error is + // terminal for the worker — it signals `finished` like EOF does, so + // the joins above answer true — and until the drain RECORDED that + // ending, this boundary had no way to tell the two apart: the + // partial buffer shipped as `.success`. This is the first read of + // that fact, on the one path that can still turn into a success. + // + // The answer is `.timeout` — the same class as the unjoined-drain + // arm above, for the same reason: the output could not be read to + // completion, and a porcelain parser must never count a short + // stdout. Ask the standing question: can a retry differ? Yes — a + // fresh invocation opens fresh pipes and fresh descriptors, so + // nothing deterministic pins the error, and every caller's `.timeout` + // handling (re-scan or refuse) is the right disposition. + // + // NO `terminate` in this arm, deliberately: the child has already + // exited (`waitForExit` succeeded) and both drains have returned, so + // unlike the arm above there is no evidence of a surviving holder — + // a kill whose removal no cell could redden would be an unevidenced + // guard by this repo's own rule. + // + // WOULD A DOWNSTREAM PARSER HAVE CAUGHT THE TRUNCATION? (asked by + // the task spec, answered from the parsers' source): only partially, + // so this gate is load-bearing. `GitWorktreePorcelainParser.parse` + // fails closed on a cut that lands mid-field or mid-record (its + // final `start == endIndex && current.isEmpty` guard), but a cut on + // a RECORD BOUNDARY — right after a record's closing empty field — + // parses cleanly as a repository with fewer worktrees. + // `WorktreeStalenessAssessor.verdict(for:)` catches nothing: a + // truncated `status --porcelain` has fewer entries and an empty one + // is `.clean` by definition. The first-line readers (`rev-parse`, + // `symbolic-ref`) accept whatever line survives. None of them can + // refuse what this boundary fails to mark. + if stdoutDrain.terminalReadFailure != nil + || stderrDrain.terminalReadFailure != nil + { + stdoutDrain.close() + stderrDrain.close() + return GitCommandInvocation( + profile: profile, argv: argv, environment: environment, + outcome: .timeout + ) + } let capturedStdout = stdoutDrain.closeAndCapture() let capturedStderr = stderrDrain.closeAndCapture() @@ -741,6 +797,10 @@ final class PipeDrain: @unchecked Sendable { private var buffer = Data() private var isClosed = false private var closeRequested = false + /// The errno of a HARD `read(2)` failure that ended the drain — `nil` + /// when the drain ended at EOF or on a requested close (fn-4.24). + /// Guarded by `lock`, like the buffer it qualifies. + private var terminalReadErrno: Int32? init(pipe: Pipe) { handle = pipe.fileHandleForReading @@ -785,6 +845,24 @@ final class PipeDrain: @unchecked Sendable { return buffer } + /// How the drain DIED, when it died on a hard `read(2)` error rather + /// than reaching EOF: the errno, or `nil` for a clean ending (EOF or a + /// requested close). fn-4.24 — before this existed, a died-on-error + /// drain signalled `finished` exactly like EOF, `join(within:)` answered + /// true, and `execute` shipped the partial buffer as `.success`; a + /// truncated porcelain listing does not look malformed, it looks like a + /// repository with fewer worktrees. + /// + /// Takes `lock` without announcing itself — the same shape `captured`'s + /// doc warns about — so production reads it ONLY after a successful + /// `join(within:)`, when the worker has provably returned and cannot + /// contend. + var terminalReadFailure: Int32? { + lock.lock() + defer { lock.unlock() } + return terminalReadErrno + } + /// END the drain and take everything it read — the one bounded spelling /// of that pair, and what production uses. /// @@ -917,6 +995,12 @@ final class PipeDrain: @unchecked Sendable { } // A hard read error is terminal: polling a broken // descriptor would spin, and there is nothing left to read. + // RECORDED, never equated with EOF (fn-4.24): until it was, + // this arm signalled `finished` exactly like end-of-stream, + // and `execute` shipped the partial buffer as `.success` — + // a truncated porcelain listing that reads as a repository + // with fewer worktrees. `lock` is held here. + terminalReadErrno = code isDone = true break readAvailable } diff --git a/Tests/CacheoutTests/GitCommandRunnerTests.swift b/Tests/CacheoutTests/GitCommandRunnerTests.swift index daefc66..0fa6438 100644 --- a/Tests/CacheoutTests/GitCommandRunnerTests.swift +++ b/Tests/CacheoutTests/GitCommandRunnerTests.swift @@ -551,6 +551,196 @@ final class GitCommandRunnerTests: XCTestCase { ) } + // MARK: - A hard read error is not EOF (fn-4.24) + + /// Serial call counter for the drain factory seam. `execute` builds its + /// drains on ONE thread in a pinned source order, and a runner's first + /// `run` makes exactly four: the availability probe's stdout (1) and + /// stderr (2), then the command's stdout (3) and stderr (4). Locked + /// anyway so the cell asserts the count without a data-race caveat. + private final class DrainBuildCounter: @unchecked Sendable { + private let lock = NSLock() + private var value = 0 + func next() -> Int { + lock.lock() + defer { lock.unlock() } + value += 1 + return value + } + var count: Int { + lock.lock() + defer { lock.unlock() } + return value + } + } + + /// A `PipeDrain` whose `read(2)` fails HARD on the first call: a + /// directory descriptor, `EISDIR` (errno 21) — the reproduction the task + /// spec measured (PR #460 round 18, runner scope). + private func drainOverADirectoryDescriptor(named name: String) throws -> PipeDrain { + let directory = base.appendingPathComponent(name) + try fm.createDirectory(at: directory, withIntermediateDirectories: true) + let descriptor = open(directory.path, O_RDONLY) + XCTAssertGreaterThanOrEqual( + descriptor, 0, "opening a directory read-only cannot fail here" + ) + return PipeDrain( + readingFrom: FileHandle(fileDescriptor: descriptor, closeOnDealloc: false) + ) + } + + /// The drain-level half of fn-4.24, with its EOF control. + /// + /// DEFECT (measured before the fix): a drain whose `read(2)` died hard + /// ended EXACTLY like EOF — `finished` signalled, `join(within:)` true, + /// nothing recorded — so `execute` had no fact to read and shipped the + /// partial buffer as `.success`. The cell asserts WHICH failure ended + /// the drain (EISDIR, not merely "some refusal"), and the control shows + /// a genuine EOF records nothing, so the two endings are distinguishable. + func testADrainThatDiedOnAHardReadErrorRecordsTheErrnoInsteadOfPosingAsEOF() throws { + let drain = try drainOverADirectoryDescriptor(named: "read-error-target") + drain.start() + XCTAssertTrue( + drain.join(within: 5), + "a hard read error must still END the drain — polling a broken " + + "descriptor would spin forever" + ) + XCTAssertEqual( + drain.terminalReadFailure, EISDIR, + "the drain died on EISDIR; a died-on-error ending that records " + + "nothing is indistinguishable from EOF, which is what let " + + "a partial buffer ship as `.success` (fn-4.24)" + ) + XCTAssertEqual( + drain.closeAndCapture(), Data(), + "no byte was ever readable from a directory descriptor" + ) + + // CONTROL: a drain that reaches genuine EOF records NO failure — + // otherwise the execute gate would refuse every healthy invocation + // and the cell above could be passing for the wrong reason. + let pipe = Pipe() + let eofDrain = PipeDrain(pipe: pipe) + eofDrain.start() + try pipe.fileHandleForWriting.write(contentsOf: Data("complete\n".utf8)) + try pipe.fileHandleForWriting.close() + XCTAssertTrue(eofDrain.join(within: 5), "EOF must end the control drain") + XCTAssertNil( + eofDrain.terminalReadFailure, + "a clean EOF is not a read failure; recording one here would " + + "turn every healthy run into a refusal" + ) + XCTAssertEqual(eofDrain.closeAndCapture(), Data("complete\n".utf8)) + } + + /// The execute-boundary half of fn-4.24, stdout arm: git exits 0, the + /// STDOUT drain dies on a hard read error (a real EISDIR descriptor via + /// the factory seam), and the invocation must be `.timeout` — never a + /// `.success` whose short stdout a porcelain parser will count as a + /// repository with fewer worktrees. + /// + /// CONTROL FIRST: the same stub through default drains is a plain + /// `.success` carrying the expected bytes, so the refusal below cannot + /// be the fixture refusing for reasons of its own. And the refusal is + /// pinned to the read-failure GATE, not the join guard: a died-on-error + /// drain joins within milliseconds, and the mutation run that deletes + /// the gate turns exactly this cell green-to-red via `.success`. + func testAHardStdoutReadErrorAfterANormalExitIsRefusedNotShippedAsSuccess() async throws { + let stubs = base.appendingPathComponent("stubs-stdout-read-error") + _ = try GitFixture.makeStubGit(in: stubs, body: """ + echo "worktree /tmp/x" + exit 0 + """) + + let control = GitCommandRunner( + environment: stubEnvironment(pathDirectories: [stubs]), + defaultTimeout: 30 + ) + let controlRun = await control.run(["worktree", "list"], timeout: 20) + guard case .success(let stdout) = controlRun.outcome else { + return XCTFail( + "CONTROL: the stub itself must succeed through default " + + "drains, got \(controlRun.outcome)" + ) + } + XCTAssertTrue( + String(decoding: stdout, as: UTF8.self).contains("worktree /tmp/x"), + "CONTROL: the stub's stdout must arrive intact" + ) + + let calls = DrainBuildCounter() + // Built OUTSIDE the factory closure: the closure cannot throw, and + // the strand fence rightly forbids `try!` in a test source. + let broken = try drainOverADirectoryDescriptor(named: "broken-stdout-fd") + let runner = GitCommandRunner( + environment: stubEnvironment(pathDirectories: [stubs]), + defaultTimeout: 30, + drainFactory: { pipe in + // Call 3 is the COMMAND'S stdout drain; the probe (1, 2) and + // the command's stderr (4) stay healthy. + if calls.next() == 3 { return broken } + return PipeDrain(pipe: pipe) + } + ) + let invocation = await runner.run(["worktree", "list"], timeout: 20) + XCTAssertEqual( + calls.count, 4, + "the factory must have built the probe's two drains and the " + + "command's two — a different count means the broken drain " + + "was not the command's stdout" + ) + XCTAssertEqual( + invocation.outcome, .timeout, + "a stdout drain that died on a hard read error must be refused " + + "at the execute boundary; before fn-4.24 it ended like EOF " + + "and the truncated buffer shipped as `.success` — a " + + "porcelain listing with fewer worktrees" + ) + } + + /// The stderr arm of the same gate: a truncated stderr is a truncated + /// answer too (on the failure path it is THE answer), so either drain + /// dying on a hard read error refuses the invocation. + func testAHardStderrReadErrorAfterANormalExitIsRefusedNotShippedAsSuccess() async throws { + let stubs = base.appendingPathComponent("stubs-stderr-read-error") + _ = try GitFixture.makeStubGit(in: stubs, body: """ + echo "worktree /tmp/x" + exit 0 + """) + + let control = GitCommandRunner( + environment: stubEnvironment(pathDirectories: [stubs]), + defaultTimeout: 30 + ) + let controlRun = await control.run(["worktree", "list"], timeout: 20) + guard case .success = controlRun.outcome else { + return XCTFail( + "CONTROL: the stub itself must succeed through default " + + "drains, got \(controlRun.outcome)" + ) + } + + let calls = DrainBuildCounter() + let broken = try drainOverADirectoryDescriptor(named: "broken-stderr-fd") + let runner = GitCommandRunner( + environment: stubEnvironment(pathDirectories: [stubs]), + defaultTimeout: 30, + drainFactory: { pipe in + // Call 4 is the COMMAND'S stderr drain. + if calls.next() == 4 { return broken } + return PipeDrain(pipe: pipe) + } + ) + let invocation = await runner.run(["worktree", "list"], timeout: 20) + XCTAssertEqual(calls.count, 4, "probe (2) + command (2) drains") + XCTAssertEqual( + invocation.outcome, .timeout, + "a stderr drain that died on a hard read error must refuse the " + + "invocation exactly like the stdout arm — fail closed on " + + "either stream" + ) + } + // MARK: - The drain's lock: bounded turns, and a closer nobody barges past /// A shell that starts `count` writers on the pipe's write end and then From 7f2ac6ec74c2a536e7ba79142714e6088653597d Mon Sep 17 00:00:00 2001 From: Bonanza Date: Thu, 27 Aug 2026 07:17:03 -0700 Subject: [PATCH 03/48] =?UTF-8?q?fix(safety):=20the=20gate=20answers=20bef?= =?UTF-8?q?ore=20realpath=20=E2=80=94=20pointer=20chase=20and=20predicate?= =?UTF-8?q?=20both=20dereferenced=20deferred=20paths=20(fn-4.26)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../Scanner/GitWorktreeInventory.swift | 48 +++++ .../Cacheout/Scanner/GitWorktreeScanner.swift | 37 ++-- .../Cacheout/Scanner/ProjectTreeWalker.swift | 46 ++++- .../GitWorktreeInventoryTests.swift | 186 ++++++++++++++++++ .../GitWorktreeScannerTests.swift | 84 +++++++- .../ProjectTreeWalkerTests.swift | 48 +++++ 6 files changed, 427 insertions(+), 22 deletions(-) diff --git a/Sources/Cacheout/Scanner/GitWorktreeInventory.swift b/Sources/Cacheout/Scanner/GitWorktreeInventory.swift index 3209de4..0d3e24d 100644 --- a/Sources/Cacheout/Scanner/GitWorktreeInventory.swift +++ b/Sources/Cacheout/Scanner/GitWorktreeInventory.swift @@ -318,6 +318,23 @@ struct GitWorktreeGitdirResolver { let pointer = pointerPath(inFileAt: dotGit, relativeTo: worktreePath) else { return nil } + // THE GATE READS THE POINTER FIRST, AS SPELLED (fn-4.26). On an + // `.automatic` scan the injected provider answers `.absent` for a + // deferred (TCC-protected) target — and its predicate classifies a + // directly-protected spelling lexically — so `canonicalize` below, + // which is `realpath(3)` and therefore itself a traversal of every + // component it resolves, runs only on a target the gate permitted. + // The previous order canonicalized first, which walked the protected + // path before the deferral could answer. No verdict changed: the + // kinds that pass here are exactly the ones the canonical probe + // below could still map onto a directory (a directory spelling, or a + // symlink to one); every other kind, absence, and probe failure + // produced nil AFTER the traversal — now before it. + switch identity.probeKind(of: pointer) { + case .kind(.directory), .kind(.symlink): break + default: return nil + } + let adminDirectory = identity.canonicalize(pointer) guard identity.probeKind(of: adminDirectory) == .kind(.directory), adminDirectory.deletingLastPathComponent().lastPathComponent @@ -326,9 +343,20 @@ struct GitWorktreeGitdirResolver { // BACK-LINK: the admin directory must point back at THIS worktree's // `.git` file. One-way, stale, and forged pointers all stop here. + // + // The back-link TARGET is data too, and `sameLocation`'s fallback + // canonicalizes BOTH sides when either identity is missing — which a + // deferred side always is — so the same gate answers for it before + // the comparison (fn-4.26). A deferred target could never have + // verified anyway: `dotGit` demonstrably exists, and a comparison of + // an existing file against an untouchable one proves nothing. + // Absence and probe failure fail exactly as they did before — the + // fallback comparison they used to reach could not answer true for a + // target that is not there while `dotGit` is. let backlinkFile = adminDirectory.appendingPathComponent("gitdir") guard identity.probeKind(of: backlinkFile) == .kind(.regularFile), let backlinkTarget = pathContents(of: backlinkFile, relativeTo: adminDirectory), + case .kind = identity.probeKind(of: backlinkTarget), identity.sameLocation(backlinkTarget, dotGit) else { return nil } @@ -343,6 +371,15 @@ struct GitWorktreeGitdirResolver { guard identity.probeKind(of: commonDirFile) == .kind(.regularFile), let target = pathContents(of: commonDirFile, relativeTo: adminDirectory) else { return nil } + // The same first-read gate as `adminDirectory(forWorktreeAt:)` + // (fn-4.26): a `commondir` is usually the relative `../..`, but the + // file's content is DATA — an absolute spelling into a deferred + // location must be answered by the gate before `realpath(3)` walks + // it. Verdicts are unchanged for every non-deferred shape. + switch identity.probeKind(of: target) { + case .kind(.directory), .kind(.symlink): break + default: return nil + } let resolved = identity.canonicalize(target) guard identity.probeKind(of: resolved) == .kind(.directory) else { return nil } return resolved @@ -392,6 +429,17 @@ struct GitWorktreeGitdirResolver { case .kind(.regularFile): guard let pointer = pointerPath(inFileAt: dotGit, relativeTo: mainRecord.path) else { return false } + // The same first-read gate (fn-4.26): `sameLocation`'s fallback + // canonicalizes BOTH sides when either identity is missing — and + // a deferred side always is — so the gate answers for the + // pointer target before the comparison. Worse than the + // traversal, the fallback's canonical PATH equality would have + // answered true for a deferred target, validating a repository + // the scan was told not to touch. A genuinely absent target + // could never compare equal to `parentGitDir`, which was probed + // a directory moments ago — failing closed changes no reachable + // verdict. + guard case .kind = identity.probeKind(of: pointer) else { return false } return identity.sameLocation(pointer, parentGitDir) default: return false diff --git a/Sources/Cacheout/Scanner/GitWorktreeScanner.swift b/Sources/Cacheout/Scanner/GitWorktreeScanner.swift index b526a21..67359df 100644 --- a/Sources/Cacheout/Scanner/GitWorktreeScanner.swift +++ b/Sources/Cacheout/Scanner/GitWorktreeScanner.swift @@ -132,15 +132,17 @@ import os /// can name a path under a TCC-protected ancestor. The path is unknowable /// before the pointer is read, so the gate cannot precede the resolution — it /// is applied INSIDE it, by making the deferred paths look like they are not -/// there. The resolver probes each pointer target before it opens anything, so -/// on an automatic scan nothing under a protected ancestor is ever opened, -/// enumerated or read. -/// -/// What DOES still happen on a deferred path is `canonicalize` — `realpath(3)`, -/// the same operation fn-4's pinned protected-root classification performs on a -/// protected root before skipping it (`ProjectTreeWalker.isProtectedRoot` -/// canonicalizes first, by design). The house doctrine already draws the line -/// there, and this wrapper does not move it. +/// there. The resolver probes each pointer target before it opens — and, +/// since fn-4.26, before it CANONICALIZES — anything: `realpath(3)` traverses +/// every component it resolves, so the probe-then-canonicalize order is what +/// keeps an automatic scan from walking a protected path while ruling it +/// untouchable. (This doc used to claim a pre-gate `canonicalize` was fine +/// because `ProjectTreeWalker.isProtectedRoot` "canonicalizes first, by +/// design" — that predicate now classifies a directly-protected spelling +/// LEXICALLY before it ever canonicalizes, so neither side dereferences such +/// a path, and the line the claim leaned on no longer exists.) On an +/// automatic scan nothing under a protected ancestor is opened, enumerated, +/// read, or realpath'd through. /// /// `.absent` rather than `.failed` deliberately: a policy deferral is not a /// problem to report (the walker skips a vanished entry quietly for the same @@ -182,7 +184,14 @@ private final class DeferringIdentityProvider: FileSystemIdentityProvider { // Path arithmetic is delegated UNCHANGED so an injected test provider's // aliasing still flows through (and so the deferral above is the only - // behavioural difference). + // behavioural difference). SAFE only because of ORDER (fn-4.26): the + // resolver asks `probeKind` — which the deferral intercepts — for every + // pointer-derived path BEFORE it canonicalizes it, and the deferral + // predicate itself classifies directly-protected spellings lexically. A + // caller that canonicalized first would traverse the protected path + // through this very pass-through; that ordering is pinned red by + // `testAutomaticScanNeverRealpathsThroughAProtectedAdminDirectory` and + // the resolver-level deferral cells. override func realPath(of path: String) -> String? { wrapped.realPath(of: path) } override func canonicalize(_ url: URL) -> URL { wrapped.canonicalize(url) } @@ -1768,9 +1777,11 @@ struct GitWorktreeScanner: @unchecked Sendable { /// /// So on `.automatic` the resolver runs on a provider that reports every /// deferred path as ABSENT. The resolver is fail-closed by construction — - /// it `probeKind`s each pointer target BEFORE reading it — so the deferral - /// lands before any `open`, and the worktree simply attributes nowhere - /// (silent, exactly like every other policy skip). + /// it `probeKind`s each pointer target BEFORE reading it, and (fn-4.26) + /// before CANONICALIZING it, `realpath(3)` being a traversal of its own — + /// so the deferral lands before any `open` and before any dereference, + /// and the worktree simply attributes nowhere (silent, exactly like every + /// other policy skip). private func identityProvider(for context: ScanContext) -> FileSystemIdentityProvider { guard !context.includeProtectedRoots else { return provider } let home = self.home diff --git a/Sources/Cacheout/Scanner/ProjectTreeWalker.swift b/Sources/Cacheout/Scanner/ProjectTreeWalker.swift index 4fae1ff..1d97052 100644 --- a/Sources/Cacheout/Scanner/ProjectTreeWalker.swift +++ b/Sources/Cacheout/Scanner/ProjectTreeWalker.swift @@ -152,17 +152,47 @@ struct ProjectTreeWalker { // MARK: - TCC-protected-root determination (R12) - /// Is `root` gated behind a macOS TCC consent prompt? True iff the - /// CANONICAL root path is equal to or under a canonical protected - /// ancestor (`home/Documents`, `home/Desktop`, `home/Downloads`) — - /// prefix by `pathComponents`, never string `hasPrefix`, never basename: - /// `~/Documents/GitHub` is protected because `Documents` is; a directory - /// merely NAMED `Documents` outside home is not; an alias spelling that - /// resolves INTO `~/Documents` through a symlinked ancestor is protected - /// as `~/Documents`. + /// Is `root` gated behind a macOS TCC consent prompt? True iff the root + /// is equal to or under a protected ancestor (`home/Documents`, + /// `home/Desktop`, `home/Downloads`) — prefix by `pathComponents`, never + /// string `hasPrefix`, never basename: `~/Documents/GitHub` is protected + /// because `Documents` is; a directory merely NAMED `Documents` outside + /// home is not; an alias spelling that resolves INTO `~/Documents` + /// through a symlinked ancestor is protected as `~/Documents`. + /// + /// TWO STAGES, and the ORDER is load-bearing (fn-4.26): this predicate + /// IS the TCC gates' classification, so it must answer for a + /// directly-protected spelling without dereferencing it — `realpath(3)` + /// traverses every component it resolves, and the previous + /// canonicalize-first body performed that traversal on exactly the paths + /// it was about to rule untouchable. + /// + /// 1. LEXICAL — the spelling as given, `.`/`..` folded with no + /// filesystem access, against the spelled ancestors. A match + /// classifies protected with NOTHING dereferenced. A spelling under a + /// protected ancestor that a symlink would resolve elsewhere now + /// classifies protected too — fail-closed, and every caller's + /// true-arm is a silent skip or deferral, never a mutation. + /// 2. CANONICAL — only for spellings stage 1 could not match: the alias + /// shapes (a symlinked ancestor, a case/NFD respelling) classify on + /// the canonical path exactly as before. The argument handed to + /// `canonicalize` here is never a directly-protected spelling; an + /// ALIAS argument's resolution does still traverse its target — the + /// disclosed residual, and not one the worktree resolver's pointer + /// chase can reach (its targets arrive spelled by git, and stage 1 + /// answers for those). static func isProtectedRoot( _ root: URL, home: URL, provider: FileSystemIdentityProvider ) -> Bool { + let spelled = root.standardizedFileURL.pathComponents + for name in tccProtectedAncestorNames { + let ancestor = home.appendingPathComponent(name) + .standardizedFileURL.pathComponents + if spelled.count >= ancestor.count, + Array(spelled.prefix(ancestor.count)) == ancestor { + return true + } + } let rootComponents = provider.canonicalize(root).pathComponents for name in tccProtectedAncestorNames { let ancestor = provider diff --git a/Tests/CacheoutTests/GitWorktreeInventoryTests.swift b/Tests/CacheoutTests/GitWorktreeInventoryTests.swift index d15f02f..735e9f0 100644 --- a/Tests/CacheoutTests/GitWorktreeInventoryTests.swift +++ b/Tests/CacheoutTests/GitWorktreeInventoryTests.swift @@ -43,6 +43,56 @@ private final class RedirectingIdentityProvider: FileSystemIdentityProvider { } } +/// The scanner's `.automatic`-scan provider, reduced to exactly the arms the +/// RESOLVER consumes (fn-4.26): a deferred path probes `.absent` and carries +/// no identity, and every `realpath(3)` ARGUMENT is recorded so a cell can +/// count the dereferences the gate must forestall. Deliberately NO more +/// capable than the production `DeferringIdentityProvider` — a double that +/// answered more would hide exactly the ordering bug under test. The deferral +/// predicate is a plain string prefix, dereferencing nothing, so every +/// recorded realpath is attributable to the RESOLVER's own ordering, never to +/// the predicate's. +private final class DeferralRecordingProvider: FileSystemIdentityProvider { + private let deferredPrefix: String? + private let lock = NSLock() + private var recorded: [String] = [] + + init(deferring prefix: String? = nil) { + self.deferredPrefix = prefix + super.init() + } + + var realPathArguments: [String] { + lock.lock() + defer { lock.unlock() } + return recorded + } + + func realPathArguments(under prefix: String) -> [String] { + realPathArguments.filter { $0.hasPrefix(prefix) } + } + + private func isDeferred(_ path: String) -> Bool { + guard let deferredPrefix else { return false } + return path.hasPrefix(deferredPrefix) + } + + override func probeKind(of url: URL) -> KindProbe { + isDeferred(url.path) ? .absent : super.probeKind(of: url) + } + + override func identity(of url: URL) -> Identity? { + isDeferred(url.path) ? nil : super.identity(of: url) + } + + override func realPath(of path: String) -> String? { + lock.lock() + recorded.append(path) + lock.unlock() + return super.realPath(of: path) + } +} + final class GitWorktreeInventoryTests: XCTestCase { private var base: URL! @@ -393,6 +443,142 @@ final class GitWorktreeInventoryTests: XCTestCase { XCTAssertNil(GitWorktreeGitdirResolver().adminDirectory(forWorktreeAt: main)) } + // MARK: - Resolver: the secondary TCC gate reads the pointer FIRST (fn-4.26) + + func testADeferredPointerTargetIsNeverRealpathedBeforeTheGateAnswers() throws { + let fixture = try makeHandBuiltPair() + + // CONTROL first: the same fixture over the same double WITHOUT a + // deferral resolves, and the resolution DOES realpath the pointer + // target — so whatever the deferring half refuses below, it refuses + // because of the deferral, not for the fixture's own reasons, and the + // zero below is measured on a live seam. + let control = DeferralRecordingProvider() + XCTAssertNotNil( + GitWorktreeGitdirResolver(identity: control) + .adminDirectory(forWorktreeAt: fixture.worktree) + ) + XCTAssertFalse( + control.realPathArguments(under: fixture.gitDir.path).isEmpty, + "an ungated resolution realpaths the pointer target — without " + + "this the deferring half's zero would be vacuous" + ) + + let deferring = DeferralRecordingProvider(deferring: fixture.gitDir.path) + XCTAssertNil( + GitWorktreeGitdirResolver(identity: deferring) + .adminDirectory(forWorktreeAt: fixture.worktree), + "a deferred pointer target attributes NOWHERE" + ) + XCTAssertEqual( + deferring.realPathArguments(under: fixture.gitDir.path), [], + "realpath(3) traverses every component it resolves — the gate " + + "must answer before it runs, not after" + ) + } + + func testADeferredCommondirTargetIsNeverRealpathedBeforeTheGateAnswers() throws { + // An absolute `commondir` spelling into deferred territory: the + // file's content is DATA, so the gate must answer for it before + // `canonicalize` walks it. + let outside = base.appendingPathComponent("hand/elsewhere-common") + try fm.createDirectory(at: outside, withIntermediateDirectories: true) + let fixture = try makeHandBuiltPair(commonDirSpelling: outside.path) + + let control = DeferralRecordingProvider() + XCTAssertNotNil( + GitWorktreeGitdirResolver(identity: control) + .commonGitDirectory(forAdminDirectory: fixture.adminDir) + ) + XCTAssertFalse( + control.realPathArguments(under: outside.path).isEmpty, + "an ungated resolution realpaths the commondir target" + ) + + let deferring = DeferralRecordingProvider(deferring: outside.path) + XCTAssertNil( + GitWorktreeGitdirResolver(identity: deferring) + .commonGitDirectory(forAdminDirectory: fixture.adminDir), + "a deferred commondir target resolves NOWHERE" + ) + XCTAssertEqual( + deferring.realPathArguments(under: outside.path), [], + "the gate answers before the dereference" + ) + } + + func testADeferredSeparateGitDirPointerFailsCrossValidationWithoutRealpath() throws { + // A non-bare first record whose `.git` FILE points into deferred + // territory. `sameLocation`'s fallback canonicalizes BOTH sides when + // either identity is missing — and a deferred path carries none — so + // without the pointer gate the COMPARISON itself would traverse the + // deferred target (and, worse, canonical path equality would answer + // true for a path the scan was told not to touch). + let record = base.appendingPathComponent("hand/sep-wd") + try fm.createDirectory(at: record, withIntermediateDirectories: true) + let external = base.appendingPathComponent("hand/sep-git") + try fm.createDirectory(at: external, withIntermediateDirectories: true) + try "gitdir: \(external.path)\n".write( + to: record.appendingPathComponent(".git"), atomically: true, encoding: .utf8 + ) + + // CONTROL: ungated, the shape cross-validates — the deferring half's + // false below is the deferral firing, not a broken fixture. + let control = DeferralRecordingProvider() + XCTAssertTrue( + GitWorktreeGitdirResolver(identity: control).crossValidate( + mainRecord: mainRecord(at: record), against: external + ) + ) + + let deferring = DeferralRecordingProvider(deferring: external.path) + XCTAssertFalse( + GitWorktreeGitdirResolver(identity: deferring).crossValidate( + mainRecord: mainRecord(at: record), against: external + ), + "a deferred pointer target fails cross-validation CLOSED" + ) + XCTAssertEqual( + deferring.realPathArguments(under: external.path), [], + "the comparison must not canonicalize a deferred side" + ) + } + + func testADeferredBacklinkTargetIsNeverRealpathedByTheComparison() throws { + // The back-link file lives in an UNdeferred admin directory, but its + // content is data and can spell a path under a deferred ancestor. The + // back-link can never verify against such a target (the worktree's + // own `.git` is reachable, or the resolution would have stopped + // sooner) — so the comparison must fail closed WITHOUT dereferencing + // the spelled target. + let elsewhere = base.appendingPathComponent("hand/protected-elsewhere") + try fm.createDirectory(at: elsewhere, withIntermediateDirectories: true) + let target = elsewhere.appendingPathComponent(".git") + try "gitdir: nothing\n".write(to: target, atomically: true, encoding: .utf8) + let fixture = try makeHandBuiltPair(backlinkTarget: target) + + // CONTROL: ungated this is the forged-backlink shape — refused, and + // refused by the COMPARISON (both sides carry identities, so the + // comparison runs and answers false). + let control = DeferralRecordingProvider() + XCTAssertNil( + GitWorktreeGitdirResolver(identity: control) + .adminDirectory(forWorktreeAt: fixture.worktree) + ) + + let deferring = DeferralRecordingProvider(deferring: elsewhere.path) + XCTAssertNil( + GitWorktreeGitdirResolver(identity: deferring) + .adminDirectory(forWorktreeAt: fixture.worktree), + "a deferred back-link target verifies NOTHING" + ) + XCTAssertEqual( + deferring.realPathArguments(under: elsewhere.path), [], + "a deferred side must fail the comparison closed, not be " + + "canonicalized by its fallback" + ) + } + // MARK: - Resolver: real git, ordinary parent func testMembershipOfARealLinkedWorktreeResolvesBothAuthorities() async throws { diff --git a/Tests/CacheoutTests/GitWorktreeScannerTests.swift b/Tests/CacheoutTests/GitWorktreeScannerTests.swift index 5cd2aa6..5197a83 100644 --- a/Tests/CacheoutTests/GitWorktreeScannerTests.swift +++ b/Tests/CacheoutTests/GitWorktreeScannerTests.swift @@ -198,10 +198,16 @@ private final class MountPointInjectingProvider: FileSystemIdentityProvider { /// Records every lstat PROBE (the operation that precedes every read the /// resolver and the mapper perform) so a test can prove nothing under a -/// protected ancestor was ever inspected. +/// protected ancestor was ever inspected — and every `realpath(3)` ARGUMENT +/// (fn-4.26), because `realpath` is not a probe: it traverses every component +/// it resolves, so canonicalizing a protected path is itself the access the +/// deferral exists to prevent, and counting probes alone left it invisible. +/// `canonicalize` funnels through `realPath(of:)`, so recording the one seam +/// counts both. private final class ProbeRecordingProvider: FileSystemIdentityProvider { private let lock = NSLock() private var probed: [String] = [] + private var realpathed: [String] = [] var probedPaths: [String] { lock.lock() @@ -209,6 +215,19 @@ private final class ProbeRecordingProvider: FileSystemIdentityProvider { return probed } + var realPathArguments: [String] { + lock.lock() + defer { lock.unlock() } + return realpathed + } + + override func realPath(of path: String) -> String? { + lock.lock() + realpathed.append(path) + lock.unlock() + return super.realPath(of: path) + } + override func probeKind(of url: URL) -> KindProbe { lock.lock() probed.append(url.path) @@ -3076,6 +3095,69 @@ final class GitWorktreeScannerTests: XCTestCase { try assertNonMalformed(user, from: userScanner) } + func testAutomaticScanNeverRealpathsThroughAProtectedAdminDirectory() + async throws + { + // The cell above proves nothing under the protected git directory was + // ever PROBED — but `realpath(3)` is not a probe, and the resolver + // used to `canonicalize` a worktree's `gitdir:` pointer target BEFORE + // its first gated `probeKind`, while the deferral predicate itself + // canonicalized the path it was classifying. So a background scan + // traversed the protected path with the probe cell green (fn-4.26, + // PR #460 codex). This cell counts the DEREFERENCE itself, on the + // injected provider. + let protectedHome = base.appendingPathComponent("protected-home") + let gitDirectory = protectedHome.appendingPathComponent("Documents/repo.git") + let workingTree = dev.appendingPathComponent("wd") + let worktree = dev.appendingPathComponent("wt") + try makeSplitRepository( + gitDirectory: gitDirectory, workingTree: workingTree, worktree: worktree + ) + let listing = Self.porcelain([ + ["worktree \(workingTree.path)", "HEAD \(String(repeating: "a", count: 40))", "branch refs/heads/main"], + ["worktree \(worktree.path)", "HEAD \(String(repeating: "a", count: 40))", "branch refs/heads/feature"], + ]) + + // Both spellings, exactly as the probe cell matches them: the `/var` + // alias the fixture rides and its canonical `/private/var` form. + let protectedPrefixes = [ + gitDirectory.path, + FileSystemIdentityProvider().canonicalize(gitDirectory).path, + ] + func protectedRealpaths(_ provider: ProbeRecordingProvider) -> [String] { + provider.realPathArguments.filter { path in + protectedPrefixes.contains { path.hasPrefix($0) } + } + } + + let automaticProvider = ProbeRecordingProvider() + let automaticRunner = ScriptedGitRunner(listing: listing) + let automatic = await makeScanner( + runner: automaticRunner, provider: automaticProvider, home: protectedHome + ).scan(context: ScanContext(trigger: .automatic)) + XCTAssertTrue(automatic.items.isEmpty) + XCTAssertTrue(automatic.errors.isEmpty, "a deferral is silent: \(automatic.errors)") + XCTAssertEqual( + protectedRealpaths(automaticProvider), [], + "an automatic scan realpath'd THROUGH the protected git directory " + + "— the gate must answer before any dereference" + ) + + // POSITIVE control: the very same fixture under a user-initiated scan + // DOES realpath through it (the deferral is policy, not a + // capability) — without this the zero above would be vacuous. + let userProvider = ProbeRecordingProvider() + let user = await makeScanner( + runner: ScriptedGitRunner(listing: listing), provider: userProvider, + home: protectedHome + ).scan(context: ScanContext(trigger: .userInitiated)) + XCTAssertFalse( + protectedRealpaths(userProvider).isEmpty, + "a user-initiated scan resolves the pointer — the counting seam is live" + ) + XCTAssertTrue(user.errors.contains { $0.kind == .containerRefused }) + } + func testFirstRecordAuthorityWinsWhenTheGitDirsParentIsNotTheWorkingTree() async throws { diff --git a/Tests/CacheoutTests/ProjectTreeWalkerTests.swift b/Tests/CacheoutTests/ProjectTreeWalkerTests.swift index 0b26003..6274e1b 100644 --- a/Tests/CacheoutTests/ProjectTreeWalkerTests.swift +++ b/Tests/CacheoutTests/ProjectTreeWalkerTests.swift @@ -1047,6 +1047,54 @@ final class ProjectTreeWalkerTests: XCTestCase { // MARK: - R12: TCC protection by canonical prefix, never basename + /// Counts `realpath(3)` arguments — `canonicalize` funnels through + /// `realPath(of:)`, so one seam counts every dereference the + /// classification performs (fn-4.26). + private final class RealpathRecordingProvider: FileSystemIdentityProvider { + private(set) var realPathArguments: [String] = [] + + override func realPath(of path: String) -> String? { + realPathArguments.append(path) + return super.realPath(of: path) + } + } + + func testDirectlyProtectedSpellingClassifiesWithoutDereferencing() throws { + // The predicate is the secondary TCC gate's classification, so it + // must answer for a spelling that ALREADY lies under a protected + // ancestor without dereferencing it — `realpath(3)` on `~/Documents/…` + // is itself a traversal of the protected path, and the previous + // canonicalize-first body performed it on exactly the paths it was + // about to rule untouchable (fn-4.26). + let documents = home.appendingPathComponent("Documents") + try mkdir(documents.appendingPathComponent("GitHub")) + let recorder = RealpathRecordingProvider() + + XCTAssertTrue(ProjectTreeWalker.isProtectedRoot( + documents.appendingPathComponent("GitHub"), home: home, provider: recorder + )) + XCTAssertEqual( + recorder.realPathArguments, [], + "classifying a directly-protected spelling must not traverse it" + ) + // Lexical `.`/`..` folds stay lexical too — no filesystem access. + XCTAssertTrue(ProjectTreeWalker.isProtectedRoot( + home.appendingPathComponent("Desktop/./x"), home: home, provider: recorder + )) + XCTAssertEqual(recorder.realPathArguments, []) + + // CONTROL: an unprotected spelling still reaches the CANONICAL stage + // (the alias shapes in the cell below depend on it) — so the zeros + // above are a property of the lexical match, not of a dead seam. + XCTAssertFalse(ProjectTreeWalker.isProtectedRoot( + home.appendingPathComponent("work"), home: home, provider: recorder + )) + XCTAssertFalse( + recorder.realPathArguments.isEmpty, + "the canonical stage ran for the unmatched spelling" + ) + } + func testProtectedRootDeterminationIsCanonicalPrefixNotBasename() throws { let documents = home.appendingPathComponent("Documents") try mkdir(documents.appendingPathComponent("GitHub")) From 820a0a7f79271b072b4db1180bf22d26eec24fdd Mon Sep 17 00:00:00 2001 From: Bonanza Date: Thu, 27 Aug 2026 07:24:27 -0700 Subject: [PATCH 04/48] fix(docs): repoint the mount-arm anchor the fn-4.26 insertion shifted (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. --- Sources/Cacheout/Scanner/BuildArtifactsScanner.swift | 2 +- Tests/CacheoutTests/SourceAnchorIntegrityTests.swift | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Sources/Cacheout/Scanner/BuildArtifactsScanner.swift b/Sources/Cacheout/Scanner/BuildArtifactsScanner.swift index 778d5ff..5000aac 100644 --- a/Sources/Cacheout/Scanner/BuildArtifactsScanner.swift +++ b/Sources/Cacheout/Scanner/BuildArtifactsScanner.swift @@ -723,7 +723,7 @@ struct BuildArtifactsScanner: @unchecked Sendable { /// invented: device-id change against the ANCESTOR, plus the `statfs` /// mount-root check that catches the same-`st_dev` firmlink mounts a /// device comparison is blind to (`DirectorySizer.swift:354-359`, - /// `ProjectTreeWalker.swift:529-532`, `ValuablesDetector.swift`). The sizer + /// `ProjectTreeWalker.swift:559-562`, `ValuablesDetector.swift`). The sizer /// records the boundary and skips its subtree uncounted; the cleaner /// refuses any tree containing one whole /// (`CacheCleaner.deleteGuardedChild`:1151 and `removeGuardedItem`:1371). diff --git a/Tests/CacheoutTests/SourceAnchorIntegrityTests.swift b/Tests/CacheoutTests/SourceAnchorIntegrityTests.swift index 8779842..11f152a 100644 --- a/Tests/CacheoutTests/SourceAnchorIntegrityTests.swift +++ b/Tests/CacheoutTests/SourceAnchorIntegrityTests.swift @@ -182,7 +182,7 @@ final class SourceAnchorIntegrityTests: XCTestCase { "compared as `pathComponents` arrays (never `hasP"), ("PathGuard.swift:462-469", "The filesystem root `/` is exempt from both: it"), - ("ProjectTreeWalker.swift:529-532", + ("ProjectTreeWalker.swift:559-562", "|| provider.isMountPoint(provider.canonicalize(c"), ("SpaceScanner.swift:1941-1952", "no-follow reality gate to THAT spelling and refu"), From 6bb24c020ea00d6a7a823c79b60daa6ee4d081af Mon Sep 17 00:00:00 2001 From: Bonanza Date: Thu, 27 Aug 2026 07:43:20 -0700 Subject: [PATCH 05/48] fix(docs): repoint the two CacheCleaner anchors fn-4.21's growth shifted (fn-4 r1 gate) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- Sources/Cacheout/Scanner/BuildArtifactsScanner.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/Cacheout/Scanner/BuildArtifactsScanner.swift b/Sources/Cacheout/Scanner/BuildArtifactsScanner.swift index 5000aac..3f2be62 100644 --- a/Sources/Cacheout/Scanner/BuildArtifactsScanner.swift +++ b/Sources/Cacheout/Scanner/BuildArtifactsScanner.swift @@ -726,7 +726,7 @@ struct BuildArtifactsScanner: @unchecked Sendable { /// `ProjectTreeWalker.swift:559-562`, `ValuablesDetector.swift`). The sizer /// records the boundary and skips its subtree uncounted; the cleaner /// refuses any tree containing one whole - /// (`CacheCleaner.deleteGuardedChild`:1151 and `removeGuardedItem`:1371). + /// (`CacheCleaner.deleteGuardedChild`:1187 and `removeGuardedItem`:1460). /// - **AN UNENUMERABLE DIRECTORY** on that same chain — the ancestor /// itself, or any directory strictly between it and the descendant /// (review r7). Mode `0111` is the field shape: SEARCHABLE, so a root From a8de2a23b444f4d8fa7dbc420764b7480bcdeda6 Mon Sep 17 00:00:00 2001 From: Bonanza Date: Thu, 27 Aug 2026 07:56:06 -0700 Subject: [PATCH 06/48] test(safety): evidence the non-ENOENT bind-failure branch of the pipeline capture (fn-4 r1 gate) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- Tests/CacheoutTests/CacheCleanerTests.swift | 118 ++++++++++++++++++++ 1 file changed, 118 insertions(+) diff --git a/Tests/CacheoutTests/CacheCleanerTests.swift b/Tests/CacheoutTests/CacheCleanerTests.swift index b8eeeac..a950a3f 100644 --- a/Tests/CacheoutTests/CacheCleanerTests.swift +++ b/Tests/CacheoutTests/CacheCleanerTests.swift @@ -5535,6 +5535,124 @@ extension CacheCleanerTests { ) } + /// A provider whose descriptor-relative child read fails with a + /// NON-ENOENT errno for the victim child — the bind-failure branch of + /// `deleteGuardedChild`'s pipeline capture that is neither "already + /// gone" nor a successful binding (`TrashDisposal.boundLeaf` → + /// `probeChild` answering `.failed`). Modeled on + /// `WorktreeReclaimPerformerTests.LockUnreadableProvider` (PR #460 + /// codex r19, R3): deny exactly one read, pass everything else through. + private final class ChildBindDeniedProvider: FileSystemIdentityProvider, + @unchecked Sendable + { + var deniedName: String! + var armed = false + private(set) var denied = false + + override func probeChild( + inDirectory descriptor: Int32, named name: String, + logical: @autoclosure () -> URL + ) -> ChildProbe { + if armed, name == deniedName { + denied = true + return .failed(errno: EACCES) + } + return super.probeChild( + inDirectory: descriptor, named: name, logical: logical() + ) + } + } + + /// **THE NON-ENOENT BIND-FAILURE BRANCH OF fn-4.21's PIPELINE CAPTURE** + /// (fn-4 round 1 gate): `deleteGuardedChild` catches `.posix(ENOENT)` + /// from `TrashDisposal.boundLeaf` as `skippedAlreadyGone`; every OTHER + /// bind failure must be the child's FAILURE — reported in the cleanup + /// report — and never a silent skip, because an EACCES leaf is still + /// STANDING THERE: "already gone" would be a lie the report repeats as + /// success-shaped silence, and the deletion must not proceed either + /// (nothing was bound to prove it against). + /// + /// WHICH refusal: the report's one error is the binding's own + /// `.posix(EACCES)` ("Permission denied" naming the child's path) — + /// distinguishable from a skip, which produces NO error row and NO + /// entry. + /// + /// MUTATION: widen the ENOENT catch in `deleteGuardedChild` to swallow + /// every `DepthSafeRemoval.Failure` as `skippedAlreadyGone` and this + /// cell goes red on the error-count assertion — the EACCES leaf is then + /// reported as if it were absent. + func testContentsModeChildWhoseBindReadFailsIsRefusedNotSkipped() + async throws + { + let (home, root, victim, marker) = try makeContentsSwapFixture() + defer { try? FileManager.default.removeItem(at: home) } + + let provider = ChildBindDeniedProvider() + provider.deniedName = victim.lastPathComponent + + let cleaner = CacheCleaner( + home: home, containerRoots: [], provider: provider + ) + provider.armed = true + let report = await cleaner.clean( + items: categoryItems( + [makeScanResult(category: makeCategory(at: root))], + home: home, provider: provider + ), + moveToTrash: false + ) + + XCTAssertTrue(provider.denied, "the fixture never denied the bind") + XCTAssertTrue( + FileManager.default.fileExists( + atPath: victim.appendingPathComponent(marker).path + ), + "a child whose bind read failed must survive untouched" + ) + XCTAssertTrue( + report.entries.isEmpty, + "reported bytes for a child it refused to bind: \(report.entries)" + ) + // NOT a skip: a skip is silent (zero errors); a non-ENOENT bind + // failure is the child's failure, with the binding's own errno. + XCTAssertEqual(report.errors.count, 1, "\(report.errors)") + let message = try XCTUnwrap(report.errors.first?.message) + XCTAssertTrue(message.contains("Permission denied"), message) + XCTAssertTrue(message.contains(victim.path), message) + } + + /// CONTROL for the cell above: identical fixture, identical provider + /// double, never armed — the clean must SUCCEED, so the refusal above + /// is evidenced to come from the denied bind and not from the fixture + /// refusing for its own reasons. + func testContentsModeBindDeniedFixtureUnarmedControlCleans() async throws { + let (home, root, victim, _) = try makeContentsSwapFixture() + defer { try? FileManager.default.removeItem(at: home) } + + let provider = ChildBindDeniedProvider() + provider.deniedName = victim.lastPathComponent + // NOT armed. + + let cleaner = CacheCleaner( + home: home, containerRoots: [], provider: provider + ) + let report = await cleaner.clean( + items: categoryItems( + [makeScanResult(category: makeCategory(at: root))], + home: home, provider: provider + ), + moveToTrash: false + ) + + XCTAssertFalse(provider.denied) + XCTAssertTrue(report.errors.isEmpty, "\(report.errors)") + XCTAssertEqual(report.entries.count, 1) + XCTAssertFalse( + FileManager.default.fileExists(atPath: victim.path), + "the child is deleted on the unarmed control" + ) + } + /// Deterministic target-swap fixture for ITEM mode with NO revalidator /// (`probedObject == nil` — `expecting:` nil at runtime). /// From 3c98463b09f26ebac995019cfb07dfd2fac7beee Mon Sep 17 00:00:00 2001 From: Bonanza Date: Thu, 27 Aug 2026 08:11:55 -0700 Subject: [PATCH 07/48] fix(safety): bound the container-identity capture off the calling thread (fn-4.19) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- Sources/Cacheout/CLIHandler.swift | 2 +- Sources/Cacheout/Cleaner/PathGuard.swift | 76 +++++++- Sources/Cacheout/Models/DiskInfo.swift | 47 +---- .../Cacheout/Models/FirstWinsRendezvous.swift | 55 ++++++ Sources/Cacheout/Scanner/SpaceScanner.swift | 179 ++++++++++++++---- .../ViewModels/CacheoutViewModel.swift | 2 +- .../BuildArtifactsScannerTests.swift | 2 +- .../CacheoutViewModelTests.swift | 174 ++++++++++++++++- .../CacheoutTests/CategoryScannerTests.swift | 2 +- .../EphemeralTempRegistrationTests.swift | 14 +- .../EphemeralTempScannerTests.swift | 2 +- .../OrphanedCachesScannerTests.swift | 6 +- .../SpaceScannerIntegrationTests.swift | 2 +- 13 files changed, 463 insertions(+), 100 deletions(-) create mode 100644 Sources/Cacheout/Models/FirstWinsRendezvous.swift diff --git a/Sources/Cacheout/CLIHandler.swift b/Sources/Cacheout/CLIHandler.swift index d1ed1ab..5b84d29 100644 --- a/Sources/Cacheout/CLIHandler.swift +++ b/Sources/Cacheout/CLIHandler.swift @@ -474,7 +474,7 @@ struct CLIHandler { context: ScanContext ) async -> CollectedScanEvents { var collected = CollectedScanEvents() - let session = runtime.scanValidatedSession( + let session = await runtime.scanValidatedSession( scannerIDs: scannerIDs, context: context ) collected.snapshot = session.snapshot diff --git a/Sources/Cacheout/Cleaner/PathGuard.swift b/Sources/Cacheout/Cleaner/PathGuard.swift index 0025fef..9e07ec4 100644 --- a/Sources/Cacheout/Cleaner/PathGuard.swift +++ b/Sources/Cacheout/Cleaner/PathGuard.swift @@ -151,7 +151,12 @@ struct CategoryAdmissionPolicy { /// a visible refusal naming the unmount remedy. A mount landing between /// this table read and a capture that already passed is the accepted /// racing residual — the capture's own lstat can then block; no table -/// re-read closes it. +/// re-read closes it, but since fn-4.19 that block costs the session its +/// CAPTURE DEADLINE rather than parking it: `captureBounded` runs the +/// whole loop off the calling thread under a wall-clock budget, and an +/// expiry is reported by the session, never swallowed (a root INSIDE a +/// hung mount — which the table preflight cannot see — is covered by the +/// same budget). struct ContainerSnapshot: Sendable { private let identities: [String: FileSystemIdentityProvider.Identity] @@ -182,6 +187,75 @@ struct ContainerSnapshot: Sendable { return ContainerSnapshot(identities: identities) } + /// A snapshot that captured NOTHING. Admits no container — every + /// delete-time lookup misses, which is the same fail-closed refusal an + /// absent root gets — and exists so the session a timed-out capture + /// produces (`scanValidatedSession`, fn-4.19) can still carry the + /// non-optional snapshot its shape requires without inventing an + /// identity nobody read. + static let empty = ContainerSnapshot(identities: [:]) + + /// The bounded capture's result. `.captured` and `.timedOut` are kept + /// apart — the `BoundedDiskInfo.Outcome` discipline — so a cell cannot + /// pass one while asserting the other. + enum BoundedCapture: Sendable { + case captured(ContainerSnapshot) + case timedOut + } + + /// `capture(roots:provider:)` under a wall-clock budget, OFF the calling + /// thread — the `BoundedDiskInfo.current(within:)` shape (PR #460 codex + /// r14, V2-1) applied to the capture fn-4.19 measured freezing the app: + /// the synchronous loop ran on the MainActor's thread and each root's + /// `lstat` was first contact with whatever answers for that path, so a + /// hung network mount or unresponsive FUSE volume under ANY session root + /// — including a root INSIDE a mount, which the `mountPointPaths()` + /// preflight cannot see — froze the app unbounded and unreported + /// (measured: a 6 s blocking `identity(of:)` gave a 6.03 s `scan` with + /// `isMainThread == true`). + /// + /// The loop now runs in a detached task racing a `ScanSessionClock` + /// timer — off the cooperative pool, because a `Task.sleep` deadline + /// cannot resume while the pool is the thing that is starved — and the + /// caller resumes on its own executor with whichever arrives first. The + /// detached band is `.utility`, the SAME band-separation decision the + /// session producer takes and for the same reason (PR #460 codex r13, + /// B): this is scan work, and cooperative-pool width is per-band, so a + /// saturated consumer band (the shape + /// `testScanIsNotParkedByItsOwnDiskInfoPreambleWhenTheBandIsSaturated` + /// drives) cannot stop the capture from even STARTING — measured in that + /// cell: with the band unspecified here, the capture queued behind the + /// holders and the scan rode this budget instead of finishing. A + /// saturated `.utility` band (concurrent sessions' own walks) is the + /// residual the timer still covers: “cannot start” reports exactly like + /// “started and hung”. + /// + /// WHAT IS NOT CLOSED, stated rather than glossed: a losing capture is + /// ABANDONED, not cancelled — `lstat` takes no deadline, so its thread + /// stays parked until the volume answers, exactly as `BoundedDiskInfo` + /// leaks its losing fetch. The bound converts the hang into a report; it + /// cannot cure the hang. CAN A RETRY DIFFER? Yes — a mount answers or + /// is unmounted, a saturated band frees — which is what makes reporting + /// the expiry as retryable honest (`scanValidatedSession` says how it is + /// reported). + static func captureBounded( + roots: [URL], provider: FileSystemIdentityProvider, + within budget: Duration + ) async -> BoundedCapture { + let rendezvous = FirstWinsRendezvous() + let timer = ScanSessionClock.schedule(after: budget) { + rendezvous.settle(.timedOut) + } + Task.detached(priority: .utility) { + rendezvous.settle( + .captured(capture(roots: roots, provider: provider)) + ) + } + let outcome = await rendezvous.wait() + timer.cancel() + return outcome + } + /// The captured identity for a registered root's declared path spelling; /// nil when the root was absent at capture (refused downstream). func identity( diff --git a/Sources/Cacheout/Models/DiskInfo.swift b/Sources/Cacheout/Models/DiskInfo.swift index 0c96152..a0c95c6 100644 --- a/Sources/Cacheout/Models/DiskInfo.swift +++ b/Sources/Cacheout/Models/DiskInfo.swift @@ -71,8 +71,10 @@ struct DiskInfo { /// `scanValidatedSession` creates the stream, the producer, the watchdog and /// the grace timer. So it was covered by NO bound and could produce NO /// `.scanDidNotFinish` — the twelfth strand mechanism on this branch, and the -/// second one (after `ContainerSnapshot.capture`) that sits in front of the -/// session bound rather than inside it. +/// second one (after `ContainerSnapshot.capture`) that sat in front of the +/// session bound rather than inside it. (The capture has since been bounded +/// on this type's own shape — `ContainerSnapshot.captureBounded`, fn-4.19 — +/// so neither pre-session wait survives unbounded.) /// /// `Task.detached` with no stated priority runs on the Swift cooperative /// pool, in the unspecified band, so it needs a free worker to START. MEASURED @@ -149,7 +151,7 @@ enum BoundedDiskInfo { within budget: Duration, fetch: @escaping @Sendable () -> DiskInfo? = { DiskInfo.current() } ) async -> Outcome { - let rendezvous = Rendezvous() + let rendezvous = FirstWinsRendezvous() // OFF THE POOL, deliberately: see the type comment. A `Task.sleep` // here would need the very worker the fetch is waiting for. let timer = ScanSessionClock.schedule(after: budget) { @@ -166,40 +168,7 @@ enum BoundedDiskInfo { return outcome } - /// One-shot, first-writer-wins, lock-guarded — the two settlers are a - /// Dispatch timer body and a detached task, neither of which may suspend. - /// The wait is a plain `withCheckedContinuation` (the spelling that - /// ignores the caller's cancellation) for the same reason `OneShotGate` - /// uses it: the timer ALWAYS settles, so an uncancellable wait cannot - /// become an unbounded one, and a cancellation-aware wait would collapse - /// the budget to zero in a cancelled caller. - private final class Rendezvous: @unchecked Sendable { - private let lock = NSLock() - private var settled: Outcome? - private var waiter: CheckedContinuation? - - func settle(_ outcome: Outcome) { - lock.lock() - guard settled == nil else { lock.unlock(); return } - settled = outcome - let waiter = self.waiter - self.waiter = nil - lock.unlock() - waiter?.resume(returning: outcome) - } - - func wait() async -> Outcome { - await withCheckedContinuation { - (continuation: CheckedContinuation) in - lock.lock() - if let settled { - lock.unlock() - continuation.resume(returning: settled) - return - } - waiter = continuation - lock.unlock() - } - } - } + // The rendezvous itself is `FirstWinsRendezvous` — born here as a + // private class, extracted when `ContainerSnapshot.captureBounded` + // (fn-4.19) and `dockerPrune` (fn-4.20) needed the identical shape. } diff --git a/Sources/Cacheout/Models/FirstWinsRendezvous.swift b/Sources/Cacheout/Models/FirstWinsRendezvous.swift new file mode 100644 index 0000000..4dfc8ad --- /dev/null +++ b/Sources/Cacheout/Models/FirstWinsRendezvous.swift @@ -0,0 +1,55 @@ +/// # FirstWinsRendezvous — the bounded-await primitive +/// +/// One-shot, first-writer-wins, lock-guarded: two settlers race — typically +/// a `ScanSessionClock` Dispatch timer body and a detached task, neither of +/// which may suspend — and whichever arrives first decides the outcome; the +/// loser's settle is a no-op. The caller resumes on its own executor, so a +/// MainActor caller needs no cooperative worker to observe the result. +/// +/// Born as `BoundedDiskInfo`'s private `Rendezvous` (PR #460 codex r14, +/// V2-1 — the header refresh's bound) and extracted verbatim when +/// `ContainerSnapshot.captureBounded` (fn-4.19) and +/// `CacheoutViewModel.dockerPrune` (fn-4.20) needed the identical shape: +/// one spelling, so the three bounds cannot drift in their settle/wait +/// semantics. +/// +/// The wait is a plain `withCheckedContinuation` (the spelling that ignores +/// the caller's cancellation) for the same reason `OneShotGate` uses it: the +/// timer ALWAYS settles, so an uncancellable wait cannot become an unbounded +/// one, and a cancellation-aware wait would collapse the budget to zero in a +/// cancelled caller. +/// +/// ONE waiter per instance, by contract: every use in this repo creates the +/// rendezvous, arms the timer, launches the work, and awaits once. + +import Foundation + +final class FirstWinsRendezvous: @unchecked Sendable { + private let lock = NSLock() + private var settled: Outcome? + private var waiter: CheckedContinuation? + + func settle(_ outcome: Outcome) { + lock.lock() + guard settled == nil else { lock.unlock(); return } + settled = outcome + let waiter = self.waiter + self.waiter = nil + lock.unlock() + waiter?.resume(returning: outcome) + } + + func wait() async -> Outcome { + await withCheckedContinuation { + (continuation: CheckedContinuation) in + lock.lock() + if let settled { + lock.unlock() + continuation.resume(returning: settled) + return + } + waiter = continuation + lock.unlock() + } + } +} diff --git a/Sources/Cacheout/Scanner/SpaceScanner.swift b/Sources/Cacheout/Scanner/SpaceScanner.swift index 010854e..2ee72fc 100644 --- a/Sources/Cacheout/Scanner/SpaceScanner.swift +++ b/Sources/Cacheout/Scanner/SpaceScanner.swift @@ -1375,14 +1375,43 @@ struct ScanSessionBounds: Sendable { /// wind down before `untilProducerFinishes()` stops waiting for it. See /// that method for what is given up when this one expires. let producerWindDownGrace: Duration + /// How long the PRE-SESSION container-identity capture + /// (`ContainerSnapshot.captureBounded`, fn-4.19) may take before the + /// session is concluded `.boundFired` with NOTHING scanned. Its own + /// bound and not `eventDeadline`'s, because the two cover different + /// work: the capture is a handful of `lstat`s (microseconds when every + /// volume answers — a hung network mount or unresponsive FUSE + /// filesystem under a session root is what spends it), while the event + /// deadline covers whole filesystem walks. See `scanValidatedSession` + /// for what a session whose capture timed out reports. + let captureDeadline: Duration + + /// `captureDeadline` defaults here so the FIXTURE constructions across + /// the suite — none of which wedge the capture unless they inject a + /// bound of their own — keep their two-argument spelling. Ten seconds is + /// the `default` philosophy applied to a microseconds-scale operation: + /// it never fails a capture for being SLOW, only for being ABANDONED. + init( + eventDeadline: Duration, + producerWindDownGrace: Duration, + captureDeadline: Duration = .seconds(10) + ) { + self.eventDeadline = eventDeadline + self.producerWindDownGrace = producerWindDownGrace + self.captureDeadline = captureDeadline + } /// What the SHIPPED composition runs under — `production(…)` is the /// only construction in the repo that names it. Ten minutes is far above /// any measured scan of a real machine; it exists to convert "never" /// into "reported", and a scan that legitimately needs longer than this - /// has a different problem. + /// has a different problem. The capture deadline is thirty seconds by + /// the same rule: four orders of magnitude above a healthy capture, + /// short enough that a scan stalled on a dead mount reports within the + /// time a user will actually wait for a spinner. static let production = ScanSessionBounds( - eventDeadline: .seconds(600), producerWindDownGrace: .seconds(30) + eventDeadline: .seconds(600), producerWindDownGrace: .seconds(30), + captureDeadline: .seconds(30) ) /// WHAT A COMPOSITION THAT DID NOT NAME A BOUND GETS, and why it is not @@ -3087,8 +3116,10 @@ struct SpaceScannerRuntime { func scanValidated( scannerIDs: Set? = nil, context: ScanContext - ) -> AsyncStream { - scanValidatedSession(scannerIDs: scannerIDs, context: context).events + ) async -> AsyncStream { + await scanValidatedSession( + scannerIDs: scannerIDs, context: context + ).events } /// `scanValidated` plus the producer's REAL completion (additive over @@ -3099,10 +3130,18 @@ struct SpaceScannerRuntime { /// keeps its "scanning" guard honest until the walk has actually /// stopped, instead of releasing it while an orphaned traversal is /// still reading the same trees. + /// ASYNC SINCE fn-4.19, and the await is the fix: the container-identity + /// capture used to run synchronously inside this call, which + /// `CacheoutViewModel.scan` reaches on the MainActor — so every session + /// root's `lstat` ran ON THE MAIN THREAD, before any bound existed. The + /// method is nonisolated, so an async caller hops off its actor to run + /// it, and the capture itself is bounded (`captureDeadline`) with its + /// expiry reported through the session's own `.scanDidNotFinish` + /// vocabulary — see below. func scanValidatedSession( scannerIDs: Set? = nil, context: ScanContext - ) -> ValidatedScanSession { + ) async -> ValidatedScanSession { // TWO independent filters, and the second is the PROTOCOL's rather // than the caller's (PR #459 review r2). `scannerIDs` is what the // caller asked for; `participates(in:)` is what the scanner will @@ -3131,50 +3170,57 @@ struct SpaceScannerRuntime { // lstat (PR #459 review r6 codex C2 — that lstat is first contact // with the mounted filesystem). // - // RECORDED, NOT FIXED, AND NOT COVERED BY THE BOUND BELOW (PR #460 - // codex r13). This capture is SYNCHRONOUS — `ContainerSnapshot - // .capture` reads the mount table and then runs `provider - // .identity(of:)`, an `lstat`, for every remaining session container - // root — and `CacheoutViewModel.scan` calls this method without an - // `await`, on the MainActor. So it runs ON THE MAIN THREAD, and it - // runs BEFORE the stream, the producer, the watchdog and the grace - // timer exist: a hung network mount or unresponsive FUSE volume - // freezes the app here, unbounded and unreported, with no - // `.scanDidNotFinish` possible because nothing is armed yet. r12's - // verifier measured a 6.03 s `scan` with `isMainThread == true` from - // a 6 s blocking `identity(of:)`; verified here as a code path - // rather than re-measured — the loop is straight-line synchronous - // and this call site has no suspension point before it. The - // `mountPointPaths()` preflight does NOT close it: it skips roots - // that ARE mount points, and a root INSIDE a hung mount is still - // lstat'ed. Pre-existing on origin/main; its own task. + // AND IT IS BOUNDED, OFF THE CALLING THREAD (fn-4.19; through fn-4 + // round 1 this was RECORDED, NOT FIXED). The capture is synchronous + // filesystem work — the mount-table read plus one `lstat` per + // remaining session container root — and it ran inline in this + // then-synchronous method, which `CacheoutViewModel.scan` reaches + // from the MainActor: a hung network mount or unresponsive FUSE + // volume under ANY session root froze the app here, unbounded and + // unreported, with no `.scanDidNotFinish` possible because nothing + // was armed yet (r12's verifier measured a 6.03 s `scan` with + // `isMainThread == true` from a 6 s blocking `identity(of:)`; the + // `mountPointPaths()` preflight never covered a root INSIDE a hung + // mount). `captureBounded` runs the loop in a detached task racing a + // `ScanSessionClock` timer — the `BoundedDiskInfo` shape, and the + // same off-the-pool clock every session bound uses. // - // IT IS NOT THE ONLY PRE-SESSION WAIT, and reading it as "the one - // place the bound cannot reach" is what let a second one sit - // undisclosed for two rounds (PR #460 codex r14, V2-1). - // `CacheoutViewModel.scan`'s header refresh also runs after the - // in-progress guard and before this method is called; it is now - // bounded on its own clock (`BoundedDiskInfo`), which is the shape a - // fix for THIS site would take too — a wall-clock budget of its own, - // since the session's cannot be armed yet. What stops that here and - // not there is the disposition: an abandoned header fetch costs a - // stale figure, while an abandoned container snapshot would leave the - // session with no identity baseline to admit deletes against, so - // giving up on it needs a product decision about what the scan then - // reports. - let snapshot = ContainerSnapshot.capture( - roots: sessionContainerRoots(for: selected), provider: provider + // WHAT AN EXPIRED CAPTURE REPORTS — the product decision the r13 + // record said this fix would owe. An abandoned capture leaves the + // session with NO identity baseline to admit deletes against, so + // proceeding to scan would publish items that delete-time admission + // must refuse wholesale — a scan-shaped success whose cleaning + // silently fails is the erasure class this project refuses. The + // session therefore runs NO scanner and reports every selected one + // through the vocabulary consumers already fail closed on: one + // `.scanDidNotFinish` per scanner, the ledger concluded + // `.boundFired` (so `didExceedBounds` is true and the GUI declines + // adoption; the CLI's target-scoped refusal reads the same rows), + // and a detail that names the capture and the retryable causes. CAN + // A RETRY DIFFER? Yes — the causes are a stalled volume or a + // starved band, both transient — so the re-scan remedy the label + // names is real, and this is a bound, not a deterministic strand. + // THE SESSION'S WALL-CLOCK BOUNDS (PR #460 codex r12, D2) — see + // `ScanSessionBounds` for the mechanism and the decision. Everything + // the watchdog needs is captured HERE, before any task starts — and + // since fn-4.19 the capture below spends the first of them. + let bounds = sessionBounds + let selectedIDs = selected.map(\.id) + let capture = await ContainerSnapshot.captureBounded( + roots: sessionContainerRoots(for: selected), provider: provider, + within: bounds.captureDeadline ) + guard case .captured(let snapshot) = capture else { + return Self.captureTimedOutSession( + selectedIDs: selectedIDs, captureDeadline: bounds.captureDeadline, + windDownGrace: bounds.producerWindDownGrace + ) + } let registeredCategories = self.registeredCategories let declaredContainerRoots = self.declaredContainerRoots let preDeleteRevalidators = self.preDeleteRevalidators let (events, continuation) = AsyncStream.makeStream() - // THE SESSION'S WALL-CLOCK BOUNDS (PR #460 codex r12, D2) — see - // `ScanSessionBounds` for the mechanism and the decision. Everything - // the watchdog needs is captured HERE, before any task starts. - let bounds = sessionBounds - let selectedIDs = selected.map(\.id) let ledger = ScanSessionLedger() let woundDown = OneShotGate() // THE PRODUCER RUNS IN A LOWER PRIORITY BAND THAN ITS CONSUMER, and @@ -3303,6 +3349,55 @@ struct SpaceScannerRuntime { ) } + /// The session a TIMED-OUT container-identity capture produces + /// (fn-4.19): nothing scanned, everything said so. One + /// `.scanDidNotFinish` per selected scanner — the kind's contract holds + /// exactly (nothing was rejected: nothing ARRIVED, and a retry can + /// genuinely differ) — with a detail naming the capture rather than the + /// walk, the ledger concluded `.boundFired` so `didExceedBounds` reads + /// true, and the EMPTY snapshot, which admits no container (the same + /// fail-closed refusal an omitted root has always produced). + /// + /// The stream is fully buffered before it is returned (`makeStream`'s + /// unbounded default), the producer is a completed no-op, and the + /// wind-down gate opens immediately — `untilProducerFinishes()` returns + /// on the spot, because there is no walk to wind down. + private static func captureTimedOutSession( + selectedIDs: [String], captureDeadline: Duration, + windDownGrace: Duration + ) -> ValidatedScanSession { + let (events, continuation) = + AsyncStream.makeStream() + let ledger = ScanSessionLedger() + let woundDown = OneShotGate() + // The same atomic step the watchdog takes, for the same reason: the + // conclusion and the reported set are decided together. This is the + // first touch of a fresh ledger, so the missing set is every + // selected scanner. + let missing = ledger.conclude(.boundFired, selected: selectedIDs) ?? [] + for id in missing { + continuation.yield(.malformed( + scannerID: id, + ScanIssue( + url: nil, kind: .scanDidNotFinish, + detail: "the container-identity capture did not finish " + + "within \(captureDeadline); no scanner ran and " + + "nothing from this session was used — a volume " + + "that stopped answering (a hung network mount or " + + "unresponsive FUSE filesystem under a scanned " + + "location) can cause this, and a re-scan after it " + + "answers or is unmounted can succeed" + ) + )) + } + continuation.finish() + woundDown.open() + return ValidatedScanSession( + snapshot: .empty, events: events, producer: Task {}, + ledger: ledger, woundDown: woundDown, windDownGrace: windDownGrace + ) + } + // MARK: Slug & item-id syntax /// `[a-z0-9_]+` — the address grammar's slug alphabet (no colon, so the diff --git a/Sources/Cacheout/ViewModels/CacheoutViewModel.swift b/Sources/Cacheout/ViewModels/CacheoutViewModel.swift index c61c1c7..98d4341 100644 --- a/Sources/Cacheout/ViewModels/CacheoutViewModel.swift +++ b/Sources/Cacheout/ViewModels/CacheoutViewModel.swift @@ -1527,7 +1527,7 @@ class CacheoutViewModel: ObservableObject { diskInfo = fetched } - let session = sessionRuntime.scanValidatedSession( + let session = await sessionRuntime.scanValidatedSession( scannerIDs: participating, context: context ) diff --git a/Tests/CacheoutTests/BuildArtifactsScannerTests.swift b/Tests/CacheoutTests/BuildArtifactsScannerTests.swift index fec174d..444ad8c 100644 --- a/Tests/CacheoutTests/BuildArtifactsScannerTests.swift +++ b/Tests/CacheoutTests/BuildArtifactsScannerTests.swift @@ -5266,7 +5266,7 @@ final class BuildArtifactsScannerTests: XCTestCase { categories: [], home: fixtureHome, provider: FileSystemIdentityProvider() ) - let session = runtime.scanValidatedSession( + let session = await runtime.scanValidatedSession( context: ScanContext(trigger: .userInitiated) ) var items: [ReclaimableItem] = [] diff --git a/Tests/CacheoutTests/CacheoutViewModelTests.swift b/Tests/CacheoutTests/CacheoutViewModelTests.swift index b40e92e..86d8c2a 100644 --- a/Tests/CacheoutTests/CacheoutViewModelTests.swift +++ b/Tests/CacheoutTests/CacheoutViewModelTests.swift @@ -1418,6 +1418,176 @@ final class CacheoutViewModelTests: XCTestCase { XCTAssertEqual(issue.kind, .scanDidNotFinish) } + /// A provider whose `identity(of:)` for ONE root blocks the way an + /// `lstat` against a hung mount does — the fn-4.19 fixture. The root it + /// wedges is a plain directory, NOT a mount point, so the + /// `mountPointPaths()` preflight passes it through to the lstat: this is + /// the "root INSIDE a hung mount" shape, the one the kernel-table skip + /// can never see. It records the thread the block landed on, so the cell + /// can assert the capture left the main thread as well as the bound. + private final class HungRootIdentityProvider: FileSystemIdentityProvider, + @unchecked Sendable + { + private let lock = NSLock() + /// The wedge is RELEASABLE, not a fixed sleep: the losing capture's + /// thread stays parked past the cell's assertions by design (the + /// disclosed abandonment residual), and a fixed multi-second sleep + /// left that parked worker leaking into whichever cell ran next — + /// observed as `testScanIsNotParkedByItsOwnDiskInfoPreamble…` going + /// red only when scheduled inside this cell's leftover window. The + /// cell releases it on the way out; the 10 s timeout is a fallback + /// so no mutation can turn this cell into a suite hang. + private let wedge = DispatchSemaphore(value: 0) + private var hungPath: String? + private var recordedMainThread: Bool? + + func arm(root: URL) { + lock.lock() + defer { lock.unlock() } + hungPath = root.path + } + + func disarm() { + lock.lock() + defer { lock.unlock() } + hungPath = nil + } + + func releaseWedge() { + for _ in 0..<8 { wedge.signal() } + } + + /// `true`/`false` once the wedge fired; nil if it never did. + var blockedOnMainThread: Bool? { + lock.lock() + defer { lock.unlock() } + return recordedMainThread + } + + override func identity(of url: URL) -> Identity? { + lock.lock() + let isHung = url.path == hungPath + if isHung { recordedMainThread = Thread.isMainThread } + lock.unlock() + if isHung { + // NOT `Task.sleep`: an lstat against a dead mount blocks the + // THREAD, and so must the simulation. + _ = wedge.wait(timeout: .now() + 10) + } + return super.identity(of: url) + } + } + + /// **fn-4.19, END TO END THROUGH PRODUCTION.** The container-identity + /// capture used to run synchronously on the main thread before any + /// bound existed: a hung mount under a session root froze the app, + /// unbounded and unreported (measured at PR #460 r12: a 6 s blocking + /// `identity(of:)` gave a 6.03 s `scan` with `isMainThread == true`). + /// + /// Now: the scan returns on `captureDeadline` (not the wedge); EVERY + /// selected scanner — the one whose root wedged AND the healthy one, + /// because nothing ran — gets a `.scanDidNotFinish` row whose detail + /// names the CAPTURE (which refusal, not just that one fired); the block + /// lands OFF the main thread; the in-progress guard is released; nothing + /// is adopted; and a re-scan after the volume answers succeeds — the + /// remedy the label names is real, so this is a bound, not a + /// deterministic strand. + /// + /// MUTATION (proved red, fn-4 round 2): (a) restore the synchronous + /// unbounded `ContainerSnapshot.capture` call in `scanValidatedSession` + /// and this cell reds on the elapsed-time assertion (the scan takes the + /// wedge); (b) make the timed-out branch finish the stream WITHOUT + /// yielding the per-scanner rows — the silent-skip erasure — and it reds + /// on the missing-issue assertion. + @MainActor + func testAHungMountUnderASessionRootIsBoundedReportedAndOffMain() + async throws + { + let hungRoot = base + .appendingPathComponent("inside-hung-mount") + .appendingPathComponent("cache-root") + try fm.createDirectory(at: hungRoot, withIntermediateDirectories: true) + let okOutcome = ScanOutcome( + items: [perItem(scanner: "ok", id: "o1", bytes: 5000)], errors: [] + ) + let provider = HungRootIdentityProvider() + let runtime = try makeRuntime( + [ + fixtureScanner("ok") { okOutcome }, + FixtureScanner( + id: "victim", trustedContainerRoots: [hungRoot] + ) { + ScanOutcome(items: [], errors: []) + }, + ], + provider: provider, + sessionBounds: ScanSessionBounds( + eventDeadline: .milliseconds(500), + producerWindDownGrace: .milliseconds(50), + captureDeadline: .milliseconds(200) + ) + ) + let viewModel = CacheoutViewModel(runtime: runtime) + // Armed AFTER construction: registration-time reads are not the + // capture, and must not be what the cell wedges. Released on every + // exit so the abandoned capture thread cannot leak into the next + // cell. + provider.arm(root: hungRoot) + defer { provider.releaseWedge() } + + let started = Date() + await viewModel.scan(trigger: .automatic) + let elapsed = Date().timeIntervalSince(started) + + // (1) BOUNDED: the scan returned on the capture deadline (200 ms), + // not the wedge (a semaphore this cell has not yet released, capped + // at 10 s so a mutated run fails rather than hangs). + XCTAssertLessThan( + elapsed, 2.0, + "the scan must return on captureDeadline, not on the hung " + + "lstat: \(elapsed) s" + ) + // (2) OFF THE MAIN THREAD: the wedge fired, and not on main. + XCTAssertEqual( + provider.blockedOnMainThread, false, + "the capture's lstat must have left the main thread " + + "(nil = never blocked, true = still on main)" + ) + // (3) REPORTED, NOT SWALLOWED — for EVERY selected scanner, since + // nothing ran; a silent skip of a container root is the erasure + // class this project refuses. And WHICH refusal: the capture's, not + // the walk's. + for id in ["ok", "victim"] { + let issue = try XCTUnwrap( + viewModel.malformedIssuesByScannerID[id], + "scanner \(id) must carry the capture-expiry row" + ) + XCTAssertEqual(issue.kind, .scanDidNotFinish) + XCTAssertNil(issue.url, "a NON-filesystem kind never invents a path") + XCTAssertTrue( + issue.detail.contains("container-identity capture"), + issue.detail + ) + } + // (4) NOTHING PUBLISHED, NOTHING ADOPTED, GUARD RELEASED. + XCTAssertEqual(viewModel.items(forScanner: "ok"), []) + XCTAssertFalse(viewModel.hasScanned, + "a capture-expired first scan is not a completed scan") + XCTAssertFalse(viewModel.isAnyScanInProgress, + "the scan guard must not survive the capture bound") + + // (5) A RETRY CAN DIFFER — the volume "answers" (disarm) and the + // next scan runs to completion. This is also the cell's CONTROL: + // were the fixture refusing for its own reasons, this scan would + // still carry the rows and fail here. + provider.disarm() + await viewModel.scan(trigger: .automatic) + XCTAssertNil(viewModel.malformedIssuesByScannerID["ok"]) + XCTAssertNil(viewModel.malformedIssuesByScannerID["victim"]) + XCTAssertEqual(viewModel.items(forScanner: "ok").map(\.id), ["o1"]) + XCTAssertTrue(viewModel.hasScanned) + } + /// THE BOUND MUST FIRE WHEN THE COOPERATIVE POOL IS STARVED — i.e. in /// exactly the wedge class it exists for (PR #460 codex r13, B). /// @@ -1611,7 +1781,7 @@ final class CacheoutViewModelTests: XCTestCase { // `.utility` rather than this cell's, and so the starvation below // cannot reach the MainActor the test itself runs on. let consumer = Task.detached(priority: .utility) { - let session = runtime.scanValidatedSession( + let session = await runtime.scanValidatedSession( context: ScanContext(trigger: .automatic) ) for await _ in session.events {} @@ -1992,7 +2162,7 @@ final class CacheoutViewModelTests: XCTestCase { producerWindDownGrace: .milliseconds(50) ) ) - let session = runtime.scanValidatedSession( + let session = await runtime.scanValidatedSession( context: ScanContext(trigger: .automatic) ) // THE RAW STREAM, not the view model's reduction of it: this is diff --git a/Tests/CacheoutTests/CategoryScannerTests.swift b/Tests/CacheoutTests/CategoryScannerTests.swift index 887ec93..451d5df 100644 --- a/Tests/CacheoutTests/CategoryScannerTests.swift +++ b/Tests/CacheoutTests/CategoryScannerTests.swift @@ -2037,7 +2037,7 @@ final class CategoryScannerTests: XCTestCase { let runtime = try makeRuntime(scanners: [fast, gated], home: home) var events: [ValidatedScannerEvent] = [] - for await event in runtime.scanValidated( + for await event in await runtime.scanValidated( context: ScanContext(trigger: .automatic) ) { events.append(event) diff --git a/Tests/CacheoutTests/EphemeralTempRegistrationTests.swift b/Tests/CacheoutTests/EphemeralTempRegistrationTests.swift index 0528dde..daae375 100644 --- a/Tests/CacheoutTests/EphemeralTempRegistrationTests.swift +++ b/Tests/CacheoutTests/EphemeralTempRegistrationTests.swift @@ -989,7 +989,7 @@ final class EphemeralTempRegistrationTests: XCTestCase { ) async -> (items: [String: [ReclaimableItem]], snapshot: ContainerSnapshot) { let everyScanner: Set? = nil - let session = runtime.scanValidatedSession( + let session = await runtime.scanValidatedSession( scannerIDs: everyScanner, context: ScanContext(trigger: .userInitiated) ) @@ -1019,7 +1019,7 @@ final class EphemeralTempRegistrationTests: XCTestCase { _ runtime: SpaceScannerRuntime, path: String ) async throws -> (scanners: [String], bytes: Set) { let everyScanner: Set? = nil - let session = runtime.scanValidatedSession( + let session = await runtime.scanValidatedSession( scannerIDs: everyScanner, context: ScanContext(trigger: .userInitiated) ) @@ -1517,7 +1517,7 @@ final class EphemeralTempRegistrationTests: XCTestCase { for trigger in [ScanTrigger.userInitiated, .automatic] { let everyScanner: Set? = nil - let session = runtime.scanValidatedSession( + let session = await runtime.scanValidatedSession( scannerIDs: everyScanner, context: ScanContext(trigger: trigger) ) @@ -1689,7 +1689,7 @@ final class EphemeralTempRegistrationTests: XCTestCase { let runtime = try makeRuntime([gated, companion], provider: provider) provider.arm() - let deferredSession = runtime.scanValidatedSession( + let deferredSession = await runtime.scanValidatedSession( context: ScanContext(trigger: .automatic) ) for await _ in deferredSession.events {} @@ -1708,7 +1708,7 @@ final class EphemeralTempRegistrationTests: XCTestCase { ) provider.arm() - let liveSession = runtime.scanValidatedSession( + let liveSession = await runtime.scanValidatedSession( context: ScanContext(trigger: .userInitiated) ) for await _ in liveSession.events {} @@ -1739,7 +1739,7 @@ final class EphemeralTempRegistrationTests: XCTestCase { let runtime = try makeRuntime([outside, inside], provider: provider) provider.arm() - let session = runtime.scanValidatedSession( + let session = await runtime.scanValidatedSession( scannerIDs: ["fixture_inside"], context: ScanContext(trigger: .userInitiated) ) @@ -1798,7 +1798,7 @@ final class EphemeralTempRegistrationTests: XCTestCase { "both spellings must survive into the union, in this order" ) - let session = runtime.scanValidatedSession( + let session = await runtime.scanValidatedSession( context: ScanContext(trigger: .automatic) ) for await _ in session.events {} diff --git a/Tests/CacheoutTests/EphemeralTempScannerTests.swift b/Tests/CacheoutTests/EphemeralTempScannerTests.swift index 7c25441..6c2e6c2 100644 --- a/Tests/CacheoutTests/EphemeralTempScannerTests.swift +++ b/Tests/CacheoutTests/EphemeralTempScannerTests.swift @@ -3072,7 +3072,7 @@ final class EphemeralTempScannerTests: XCTestCase { scanners: [scanner], categories: [], home: home, provider: FileSystemIdentityProvider() ) - let session = runtime.scanValidatedSession( + let session = await runtime.scanValidatedSession( context: ScanContext(trigger: .userInitiated) ) var outcome: ScanOutcome? diff --git a/Tests/CacheoutTests/OrphanedCachesScannerTests.swift b/Tests/CacheoutTests/OrphanedCachesScannerTests.swift index 724872a..493ecca 100644 --- a/Tests/CacheoutTests/OrphanedCachesScannerTests.swift +++ b/Tests/CacheoutTests/OrphanedCachesScannerTests.swift @@ -742,7 +742,7 @@ final class OrphanedCachesScannerTests: XCTestCase { let scanner = makeScanner() let runtime = try makeRuntime([scanner]) - let session = runtime.scanValidatedSession( + let session = await runtime.scanValidatedSession( context: ScanContext(trigger: .userInitiated) ) var items: [ReclaimableItem] = [] @@ -1070,7 +1070,7 @@ final class OrphanedCachesScannerTests: XCTestCase { private func scanSession( _ runtime: SpaceScannerRuntime ) async -> (items: [ReclaimableItem], snapshot: ContainerSnapshot) { - let session = runtime.scanValidatedSession( + let session = await runtime.scanValidatedSession( context: ScanContext(trigger: .userInitiated) ) var items: [ReclaimableItem] = [] @@ -1171,7 +1171,7 @@ final class OrphanedCachesScannerTests: XCTestCase { let runtime = try makeRuntime([DelegatingGatedSweepScanner( inner: makeScanner(), gate: gate )]) - let session = runtime.scanValidatedSession( + let session = await runtime.scanValidatedSession( context: ScanContext(trigger: .userInitiated) ) try fm.removeItem(at: cachesRoot) diff --git a/Tests/CacheoutTests/SpaceScannerIntegrationTests.swift b/Tests/CacheoutTests/SpaceScannerIntegrationTests.swift index cb51d2d..27d99cf 100644 --- a/Tests/CacheoutTests/SpaceScannerIntegrationTests.swift +++ b/Tests/CacheoutTests/SpaceScannerIntegrationTests.swift @@ -534,7 +534,7 @@ final class SpaceScannerIntegrationTests: XCTestCase { _ runtime: SpaceScannerRuntime, scannerIDs: Set?, file: StaticString = #filePath, line: UInt = #line ) async -> (outcomes: [String: ScanOutcome], snapshot: ContainerSnapshot) { - let session = runtime.scanValidatedSession( + let session = await runtime.scanValidatedSession( scannerIDs: scannerIDs, context: ScanContext(trigger: .userInitiated) ) var outcomes: [String: ScanOutcome] = [:] From 2e210916324f28932386233a295a7e27a75a8b9e Mon Sep 17 00:00:00 2001 From: Bonanza Date: Thu, 27 Aug 2026 08:16:35 -0700 Subject: [PATCH 08/48] fix(safety): bound dockerPrune's whole child interaction; retire the last bare waitUntilExit calls (fn-4.20) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../Intervention/Tier2Interventions.swift | 22 ++- .../ViewModels/CacheoutViewModel.swift | 166 +++++++++++++----- .../CacheoutViewModelTests.swift | 90 ++++++++++ .../DocumentedContractTests.swift | 64 +++++++ 4 files changed, 291 insertions(+), 51 deletions(-) diff --git a/Sources/Cacheout/Intervention/Tier2Interventions.swift b/Sources/Cacheout/Intervention/Tier2Interventions.swift index ac393c3..496086a 100644 --- a/Sources/Cacheout/Intervention/Tier2Interventions.swift +++ b/Sources/Cacheout/Intervention/Tier2Interventions.swift @@ -814,8 +814,15 @@ public final class SnapshotCleanup: Intervention { try? await Task.sleep(nanoseconds: 2_000_000_000) if process.isRunning { kill(process.processIdentifier, SIGKILL) - // Wait for child to be reaped before resuming. - process.waitUntilExit() + // Wait for the child to be reaped before resuming — + // BOUNDED (fn-4.20): `waitUntilExit()` is the primitive + // this repo retired after measuring it miss its + // termination wakeup under concurrent reaping, and a + // SIGKILLed child is reaped by the kernel promptly, so + // five seconds is orders of magnitude of headroom. If + // even that expires, the timeout below is resumed + // anyway — a leaked zombie beats a strand. + _ = process.waitForExit(within: 5) } resumer.resume(with: .failure(SnapshotError.timeout)) } @@ -864,8 +871,15 @@ public final class SnapshotCleanup: Intervention { try? await Task.sleep(nanoseconds: 2_000_000_000) if process.isRunning { kill(process.processIdentifier, SIGKILL) - // Wait for child to be reaped before resuming. - process.waitUntilExit() + // Wait for the child to be reaped before resuming — + // BOUNDED (fn-4.20): `waitUntilExit()` is the primitive + // this repo retired after measuring it miss its + // termination wakeup under concurrent reaping, and a + // SIGKILLed child is reaped by the kernel promptly, so + // five seconds is orders of magnitude of headroom. If + // even that expires, the timeout below is resumed + // anyway — a leaked zombie beats a strand. + _ = process.waitForExit(within: 5) } resumer.resume(with: .failure(SnapshotError.timeout)) } diff --git a/Sources/Cacheout/ViewModels/CacheoutViewModel.swift b/Sources/Cacheout/ViewModels/CacheoutViewModel.swift index 98d4341..f969d17 100644 --- a/Sources/Cacheout/ViewModels/CacheoutViewModel.swift +++ b/Sources/Cacheout/ViewModels/CacheoutViewModel.swift @@ -1818,14 +1818,42 @@ class CacheoutViewModel: ObservableObject { @Published var isDockerPruning = false @Published var lastDockerPruneResult: String? + /// THE STATED BOUND on one prune attempt (fn-4.20). Ten minutes is the + /// `ScanSessionBounds.production` philosophy applied to a subprocess: a + /// `docker system prune` over a large image store legitimately runs for + /// minutes, so the bound exists to convert "never" into "reported", + /// not to hurry a big prune — and a prune that legitimately needs + /// longer than this has a different problem. + static let dockerPruneDefaultBudget: Duration = .seconds(600) + + /// TEST SEAMS — production reads the defaults. The budget so a cell can + /// prove the expiry path in milliseconds; the command so a cell can + /// substitute a wedged or scripted child for the real docker CLI. + var dockerPruneBudget: Duration = CacheoutViewModel.dockerPruneDefaultBudget + var dockerPruneCommand: [String] = ["docker", "system", "prune", "-f"] + + /// What one prune attempt produced. `.finished` carries the child's own + /// exit status INCLUDING failures — a completed failure and a timeout + /// are kept apart (the `BoundedDiskInfo.Outcome` discipline) so a cell + /// cannot pass one while asserting the other. + private enum DockerPruneOutcome: Sendable { + case finished(status: Int32, output: String) + case launchFailed + case timedOut + } + func dockerPrune() async { isDockerPruning = true + // Released on EVERY path out of this method — and since fn-4.20 + // every path RETURNS: the child interaction below is raced against + // its budget, so the defer can no longer be postponed forever by a + // docker CLI that never exits. defer { isDockerPruning = false } let process = Process() let pipe = Pipe() process.executableURL = URL(fileURLWithPath: "/usr/bin/env") - process.arguments = ["docker", "system", "prune", "-f"] + process.arguments = dockerPruneCommand process.standardOutput = pipe process.standardError = pipe // Real home is correct here: the view model has no injected-home @@ -1837,50 +1865,94 @@ class CacheoutViewModel: ObservableObject { "HOME": FileManager.default.homeDirectoryForCurrentUser.path ] - do { - // RECORDED, NOT FIXED (PR #460 codex r13). This wait is - // UNBOUNDED, and it is the one primitive this repo replaced - // everywhere else: `Process.waitUntilExit()` can miss its - // termination wakeup under concurrent reaping (see - // `Process.waitForExit(within:)` in CacheCategory.swift, which - // exists for that and is used by CacheCleaner and the category - // subprocesses). Here it sits behind a `readToEnd()` on a - // cooperative worker, so a docker CLI that never exits holds the - // worker AND latches `isDockerPruning` true for the life of the - // app — the button never re-enables. Pre-existing on - // origin/main, unrelated to this PR's scanners, and left to its - // own change: the fix is `waitForExit(within:)` plus a - // termination policy, which is a product decision about how long - // a prune may take. - let result = try await Task.detached { () -> (Int32, String) in + // BOUNDED, AND BOUNDED AS A WHOLE (fn-4.20; through fn-4 round 1 + // this was RECORDED, NOT FIXED at PR #460 codex r13). The old body + // did `readToEnd()` then a bare `process.waitUntilExit()` in a + // detached task and awaited its value: unbounded, on a cooperative + // worker, and the one surviving production call site of the + // primitive this repo retired everywhere else after measuring it + // miss its termination wakeup under concurrent reaping + // (`Process.waitForExit(within:)`, CacheCategory.swift). A docker + // CLI that never exited held the worker AND latched + // `isDockerPruning` true for the life of the app. + // + // AND THE WAIT WAS NOT THE ONLY PARK — the task-spec question + // "can `readToEnd()` park too?" answers YES: it blocks until EOF, + // and EOF needs every write end of the pipe closed, so a wedged + // child that keeps its descriptors open parks the read BEFORE any + // wait is reached. Bounding only the wait would have moved the + // strand one line up. That is why the budget races the WHOLE child + // interaction (spawn → drain → bounded wait) through + // `FirstWinsRendezvous` on `ScanSessionClock` — off the cooperative + // pool, where a `Task.sleep` deadline cannot be starved — while the + // interaction itself still ends in `waitForExit(within:)`, never + // the retired primitive. + let budget = dockerPruneBudget + let waitSeconds = Double(budget.components.seconds) + + Double(budget.components.attoseconds) / 1e18 + let rendezvous = FirstWinsRendezvous() + let timer = ScanSessionClock.schedule(after: budget) { + rendezvous.settle(.timedOut) + } + Task.detached { + do { try process.run() - let data = try pipe.fileHandleForReading.readToEnd() ?? Data() - process.waitUntilExit() - let output = String(data: data, encoding: .utf8) ?? "" - return (process.terminationStatus, output) - }.value - - if result.0 == 0 { - // Extract "Total reclaimed space:" line - if let line = result.1.components(separatedBy: "\n") - .first(where: { $0.contains("reclaimed") }) { - lastDockerPruneResult = line.trimmingCharacters(in: .whitespaces) - } else { - lastDockerPruneResult = "Docker pruned successfully" - } + } catch { + rendezvous.settle(.launchFailed) + return + } + let data = (try? pipe.fileHandleForReading.readToEnd()) ?? Data() + // EOF does not prove exit (a child can close its descriptors + // and live on), so the exit is still awaited — bounded, and by + // the SAME figure: the outer timer started first, so on a + // wedged child it is the timer that settles, and this poll can + // never outlive the budget by more than its own scheduling. + guard process.waitForExit(within: waitSeconds) else { + rendezvous.settle(.timedOut) + return + } + rendezvous.settle(.finished( + status: process.terminationStatus, + output: String(data: data, encoding: .utf8) ?? "" + )) + } + let outcome = await rendezvous.wait() + timer.cancel() + + switch outcome { + case .finished(let status, let output) where status == 0: + // Extract "Total reclaimed space:" line + if let line = output.components(separatedBy: "\n") + .first(where: { $0.contains("reclaimed") }) { + lastDockerPruneResult = line.trimmingCharacters(in: .whitespaces) } else { - let lowerOutput = result.1.lowercased() - if lowerOutput.contains("cannot connect") || - lowerOutput.contains("is the docker daemon running") || - lowerOutput.contains("connection refused") || - lowerOutput.contains("no such file or directory") { - lastDockerPruneResult = "Docker must be running to prune" - } else { - lastDockerPruneResult = "Docker prune failed — is Docker running?" - } + lastDockerPruneResult = "Docker pruned successfully" + } + case .finished(_, let output): + let lowerOutput = output.lowercased() + if lowerOutput.contains("cannot connect") || + lowerOutput.contains("is the docker daemon running") || + lowerOutput.contains("connection refused") || + lowerOutput.contains("no such file or directory") { + lastDockerPruneResult = "Docker must be running to prune" + } else { + lastDockerPruneResult = "Docker prune failed — is Docker running?" } - } catch { + case .launchFailed: lastDockerPruneResult = "Docker not found" + case .timedOut: + // REPORTED, NOT SWALLOWED — and the abandonment is disclosed + // rather than dressed as a kill: SIGTERM is best-effort (docker + // forwards it; a child that ignores it keeps running, and its + // reader thread stays parked until the pipe closes — the same + // abandonment residual `BoundedDiskInfo` carries). CAN A RETRY + // DIFFER? Yes: the causes — a daemon mid-restart, a huge layer + // delete, a wedged Docker Desktop — are all transient, so + // "check Docker and retry" is a real remedy, not a strand + // dressed as one. + if process.isRunning { process.terminate() } + lastDockerPruneResult = "Docker prune did not finish within " + + "\(budget) — asked it to stop; check Docker and retry" } // Refresh disk info after prune — THE TWIN OF `scan`'s fetch, and @@ -1888,10 +1960,10 @@ class CacheoutViewModel: ObservableObject { // bounds this one either: `isDockerPruning` is released by the // `defer` at the top of this method, which does not run until this // await returns, so an unstarted detached fetch latched the button - // disabled exactly the way the prune's own unbounded - // `waitUntilExit()` does. On `.timedOut` the header keeps the - // figures it had, `lastDockerPruneResult` (already set above) still - // reaches the user, and the next scan refreshes. + // disabled exactly the way the prune's own then-unbounded + // `waitUntilExit()` did before fn-4.20. On `.timedOut` the header + // keeps the figures it had, `lastDockerPruneResult` (already set + // above) still reaches the user, and the next scan refreshes. if case .fetched(let fetched) = await BoundedDiskInfo.current( within: diskInfoBudget ) { @@ -1963,8 +2035,8 @@ class CacheoutViewModel: ObservableObject { // below — COUNTED, `grep -n 'isCleaning ='`). No watchdog, no // timeout and no view ever clears it. So a clean that never returns // latches BOTH the clean path and the scan path shut for the life of - // the app, exactly as `dockerPrune`'s unbounded `waitUntilExit()` - // latches its own button. + // the app, exactly as `dockerPrune`'s formerly unbounded wait + // latched its own button before fn-4.20. // // NOT BOUNDED, deliberately, and this is the product decision: a // deletion cannot be abandoned. `removefile`/`trashItem` keep running diff --git a/Tests/CacheoutTests/CacheoutViewModelTests.swift b/Tests/CacheoutTests/CacheoutViewModelTests.swift index 86d8c2a..0c9099d 100644 --- a/Tests/CacheoutTests/CacheoutViewModelTests.swift +++ b/Tests/CacheoutTests/CacheoutViewModelTests.swift @@ -2072,6 +2072,96 @@ final class CacheoutViewModelTests: XCTestCase { XCTAssertGreaterThan(disk.totalSpace, 0) } + // MARK: - Docker prune bound (fn-4.20) + + /// **fn-4.20: A WEDGED DOCKER CLI MUST NOT LATCH THE BUTTON.** The old + /// body awaited `readToEnd()` + a bare `waitUntilExit()` unbounded, so + /// a child that neither exited nor closed its pipe held a cooperative + /// worker and left `isDockerPruning` true for the life of the app. + /// + /// The fixture child (`sleep 30`) is exactly that shape: it keeps its + /// stdout open, so `readToEnd()` parks — the task-spec's "can the read + /// park too?" answered by construction, not only in a comment — and it + /// ignores no signals, so the expiry's SIGTERM also cleans the fixture + /// up. With a 300 ms budget the prune must return promptly, release the + /// button, and REPORT the expiry, not swallow it. + /// + /// MUTATION (proved red, fn-4 round 2): drop the `ScanSessionClock` + /// timer from `dockerPrune` (never settle `.timedOut`) and this cell + /// reds on the elapsed assertion — the await rides the child's full 30 s + /// instead of the budget. Restoring the bare `waitUntilExit()` INSIDE + /// the raced task is caught by the OTHER named cell, the + /// `DocumentedContractTests` grep gate: the outer race would still + /// bound it, which is precisely why the gate exists as its own cell. + @MainActor + func testDockerPruneExpiresReportsAndReleasesTheButton() async throws { + let runtime = try makeRuntime([]) + let viewModel = CacheoutViewModel(runtime: runtime) + viewModel.dockerPruneBudget = .milliseconds(300) + viewModel.dockerPruneCommand = ["sh", "-c", "sleep 30"] + + let started = Date() + await viewModel.dockerPrune() + let elapsed = Date().timeIntervalSince(started) + + XCTAssertLessThan( + elapsed, 5.0, + "dockerPrune must return on its budget, not on the wedged " + + "child: \(elapsed) s" + ) + XCTAssertFalse( + viewModel.isDockerPruning, + "the button must be released on the expiry path" + ) + let result = try XCTUnwrap( + viewModel.lastDockerPruneResult, + "the expiry must be reported, not swallowed" + ) + XCTAssertTrue(result.contains("did not finish"), result) + } + + /// CONTROL for the cell above, and the success path's parser: a child + /// that prints docker's reclaimed line and exits must be read to EOF + /// and reported through the same seams — so the expiry cell's refusal + /// is evidenced to come from the wedge, not from the seams themselves. + @MainActor + func testDockerPruneSuccessStillParsesTheReclaimedLine() async throws { + let runtime = try makeRuntime([]) + let viewModel = CacheoutViewModel(runtime: runtime) + viewModel.dockerPruneBudget = .seconds(10) + viewModel.dockerPruneCommand = [ + "sh", "-c", "echo 'Total reclaimed space: 1.234GB'", + ] + + await viewModel.dockerPrune() + + XCTAssertFalse(viewModel.isDockerPruning) + XCTAssertEqual( + viewModel.lastDockerPruneResult, "Total reclaimed space: 1.234GB" + ) + } + + /// AND THE FAILURE PATH IS STILL A COMPLETED FAILURE, NOT A TIMEOUT — + /// the two travel different arms of `DockerPruneOutcome` and must not + /// be able to trade places: a child that exits non-zero within the + /// budget reports the daemon guidance, never the expiry sentence. + @MainActor + func testDockerPruneCompletedFailureIsNotReportedAsExpiry() async throws { + let runtime = try makeRuntime([]) + let viewModel = CacheoutViewModel(runtime: runtime) + viewModel.dockerPruneBudget = .seconds(10) + viewModel.dockerPruneCommand = [ + "sh", "-c", "echo 'Cannot connect to the Docker daemon' >&2; exit 1", + ] + + await viewModel.dockerPrune() + + XCTAssertFalse(viewModel.isDockerPruning) + XCTAssertEqual( + viewModel.lastDockerPruneResult, "Docker must be running to prune" + ) + } + /// EXACTLY ONE EVENT PER SCANNER, EVER — the watchdog's report and the /// real outcome must be exclusive in BOTH directions (PR #460 codex r13, /// C). diff --git a/Tests/CacheoutTests/DocumentedContractTests.swift b/Tests/CacheoutTests/DocumentedContractTests.swift index 933079f..c01c5b1 100644 --- a/Tests/CacheoutTests/DocumentedContractTests.swift +++ b/Tests/CacheoutTests/DocumentedContractTests.swift @@ -1295,6 +1295,69 @@ final class DocumentedContractTests: XCTestCase { requiresPreDeleteRevalidation: true ) } + // MARK: - Retired-primitive fence (fn-4.20) + + /// **NO BARE `waitUntilExit()` IN PRODUCTION** — the grep gate the + /// fn-4.20 acceptance demands, as a cell rather than a review habit. + /// + /// `Process.waitUntilExit()` was retired from this codebase after being + /// measured missing its termination wakeup under concurrent reaping + /// (macOS 26, ~12-25% of spawns under load — the comment on + /// `Process.waitForExit(within:)` in CacheCategory.swift carries the + /// measurement). Every production wait is the bounded poll; the LAST + /// surviving call sites (dockerPrune, and the two post-SIGKILL reaps in + /// Tier2Interventions the fn-4.20 spec's "last one" claim had not + /// counted) were converted at fn-4.20. + /// + /// TWO-LAYER SHAPE: the narrow layer scans every non-comment line of + /// every `Sources/**.swift` for the call and must find ZERO; comment + /// lines are deliberately allowed to mention it, because the retirement + /// is documented BY those comments and a fence that banned the name + /// outright would erase its own rationale. TEST sources are OUT OF + /// SCOPE by the task spec (their ~13 sites reap fixture children whose + /// exit already happened; filed separately if the gate is extended). + /// + /// MUTATION (proved red, fn-4 round 2): restoring the bare + /// `process.waitUntilExit()` in `dockerPrune`'s raced task reds THIS + /// cell — deliberately, because the behavioral expiry cell alone would + /// stay green there (the outer race still bounds the whole + /// interaction), which is exactly why the fence is its own cell. + func testNoBareWaitUntilExitRemainsInProductionSources() throws { + let sourcesRoot = repoRoot.appendingPathComponent("Sources") + let enumerator = try XCTUnwrap(FileManager.default.enumerator( + at: sourcesRoot, includingPropertiesForKeys: nil + )) + var scanned = 0 + var offenders: [String] = [] + for case let url as URL in enumerator + where url.pathExtension == "swift" { + scanned += 1 + let text = try String(contentsOf: url, encoding: .utf8) + for (index, line) in text + .components(separatedBy: "\n").enumerated() + { + guard line.contains("waitUntilExit()") else { continue } + let trimmed = line.trimmingCharacters(in: CharacterSet.whitespaces) + // Comment lines may NAME the retired primitive — they are + // where its retirement is documented. + guard !trimmed.hasPrefix("//") else { continue } + offenders.append( + "\(url.lastPathComponent):\(index + 1): \(trimmed)" + ) + } + } + XCTAssertGreaterThan( + scanned, 20, + "the walk found implausibly few Swift files — the fence is " + + "scanning the wrong root, not proving absence" + ) + XCTAssertEqual( + offenders, [], + "bare waitUntilExit() calls in production Sources — use " + + "Process.waitForExit(within:) (CacheCategory.swift) " + + "instead: \(offenders)" + ) + } } /// The documented recipe, hashed. The INDEPENDENCE that matters is the @@ -1308,6 +1371,7 @@ private enum DocumentedToken { .map { String(format: "%02x", $0) } .joined() } + } // MARK: - The worked retry example, executed From 80e5d6131dfa3878897d0a6dbc7b8b55c01cef03 Mon Sep 17 00:00:00 2001 From: Bonanza Date: Thu, 27 Aug 2026 08:37:24 -0700 Subject: [PATCH 09/48] fix(docs): repoint the 11 anchors fn-4.19's growth shifted (fn-4.19) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../Cacheout/Scanner/EphemeralTempRoots.swift | 28 +++++++++---------- Sources/Cacheout/Scanner/SpaceScanner.swift | 6 ++-- .../EphemeralTempRootsTests.swift | 2 +- .../EphemeralTempScannerTests.swift | 2 +- .../SourceAnchorIntegrityTests.swift | 22 +++++++-------- 5 files changed, 30 insertions(+), 30 deletions(-) diff --git a/Sources/Cacheout/Scanner/EphemeralTempRoots.swift b/Sources/Cacheout/Scanner/EphemeralTempRoots.swift index b4578a1..0af6dce 100644 --- a/Sources/Cacheout/Scanner/EphemeralTempRoots.swift +++ b/Sources/Cacheout/Scanner/EphemeralTempRoots.swift @@ -51,7 +51,7 @@ /// ROOT, not a comparison value. `realpath(3)` resolves the leaf too, so a /// symlink standing where `C`/`T` should be would silently register its /// DESTINATION as a temp root — and the container-root policy only refuses -/// `/`, volume roots and `$HOME` itself (`PathGuard.swift:370` says so in +/// `/`, volume roots and `$HOME` itself (`PathGuard.swift:444` says so in /// as many words: "`~/Documents` can be a container while `admitDeletionRoot` /// refuses it"), so an arbitrary directory would be admitted, walked, listed /// as cache-container payload and deleted. Keeping the leaf means the @@ -110,7 +110,7 @@ /// thread while building its `@StateObject` /// (`CacheoutApp.swift:58` → `CacheoutViewModel.production()` → /// `CacheoutViewModel.swift:563` → `SpaceScannerRuntime.production`, -/// `SpaceScanner.swift:2129`), long +/// `SpaceScanner.swift:2158`), long /// before any trigger or `participates(in:)` gate exists to consult. The main /// thread is not an inference: `CacheoutViewModel` is `@MainActor` /// (`CacheoutViewModel.swift:264`), so its `production()` factory cannot be @@ -137,7 +137,7 @@ /// This is where the file DIVERGES from the two dev-root precedents it /// otherwise follows: `DevRootsStore.swift:322` and /// `SpaceScannerRuntime.suppressingAliasShadows`' probe pair -/// (`SpaceScanner.swift:1992-1996`) +/// (`SpaceScanner.swift:2021-2025`) /// both still build their comparison key with `provider.canonicalize`, on /// every root including non-directory ones, at the same construction time. /// Neither has been changed here. @@ -147,7 +147,7 @@ /// It is not a claim about `production()` as a whole, and the difference is /// measured. A symlink root this resolution cannot place is KEPT, so it /// reaches the runtime's cross-scanner union and -/// `suppressingAliasShadows`' probe pair (`SpaceScanner.swift:1992-1996`) +/// `suppressingAliasShadows`' probe pair (`SpaceScanner.swift:2021-2025`) /// canonicalizes it there — one leaf-following /// `realpath(3)` on the destination, still during construction. Measured /// through the shipped `??` arm with the same fixture, before and after this @@ -157,7 +157,7 @@ /// root).path` with `root.path` takes both to 0 and 0.0026 s, which is how /// the surviving contact was attributed — NOT a proposed fix: that key is /// what suppresses a shadowing alias ACROSS scanners -/// (`suppressingAliasShadows`' doc, `SpaceScanner.swift:1941-1952`), and +/// (`suppressingAliasShadows`' doc, `SpaceScanner.swift:1970-1981`), and /// weakening it trades one hazard for /// another. Closing it needs its own change, on fn-4.5's contract. /// @@ -188,7 +188,7 @@ /// fix and a relocation: the 2 of those 5 that `resolve` never made were /// `suppressingAliasShadows` canonicalizing and probing the same root in the /// cross-scanner union (`suppressingAliasShadows`' probe pair, -/// `SpaceScanner.swift:1992-1996`), which every KEPT +/// `SpaceScanner.swift:2021-2025`), which every KEPT /// root reaches. /// /// ### RESIDUAL, at measured scope: three cases this does not cover @@ -217,7 +217,7 @@ /// One value is probed per declared root: whether the DECLARED spelling is /// itself a real directory (`lstat` leaf, no follow), which is the /// `isDirectory` half of the probe pair at `DevRootsStore.swift:320-324` and -/// `suppressingAliasShadows`' probe pair (`SpaceScanner.swift:1992-1996`). +/// `suppressingAliasShadows`' probe pair (`SpaceScanner.swift:2021-2025`). /// The `key:` half of that pair is deliberately /// NOT taken (see above). The two halves that consume the probe have /// different precedents — do not read this as one pattern copied whole from @@ -228,7 +228,7 @@ /// alone (:361-364, `seenCanonicalKeys.insert`). /// `SpaceScannerRuntime.suppressingAliasShadows` does NOT do this half — it /// deliberately DECLINES it, and `suppressingAliasShadows` -/// (`SpaceScanner.swift:2002-2005`) says so: +/// (`SpaceScanner.swift:2031-2034`) says so: /// "Two real-directory spellings of one location are NOT touched: both pass /// the reality gate, so neither shadows the other, and dropping either would /// change which declared spelling the identity binding keys off for no @@ -264,18 +264,18 @@ /// returns the FIRST configured root that matches and `admitContainer` /// refuses THAT spelling without trying the real one behind it. /// `DevRootsStore.swift:326-332` names that shape "ACTIVELY HARMFUL"; -/// `suppressingAliasShadows`' doc (`SpaceScanner.swift:1941-1952`) +/// `suppressingAliasShadows`' doc (`SpaceScanner.swift:1970-1981`) /// records the breakage it caused when the /// shadowed root came from another scanner. /// /// BOTH files do this half — `DevRootsStore.swift:333-335` + :341-357 and -/// `suppressingAliasShadows` (`SpaceScanner.swift:1964-2004`) — but only +/// `suppressingAliasShadows` (`SpaceScanner.swift:1993-2033`) — but only /// `DevRootsStore` classifies the /// drop. `suppressingAliasShadows` returns roots plus their canonical keys -/// and NO issue channel of its own (`SpaceScanner.swift:1987-1989`; the +/// and NO issue channel of its own (`SpaceScanner.swift:2016-2018`; the /// "bare `[URL]`" this sentence used to say stopped being true when the /// keys were carried out of the same probe, PR #460 codex r4); -/// `suppressingAliasShadows`' doc (`SpaceScanner.swift:1981-1986`) +/// `suppressingAliasShadows`' doc (`SpaceScanner.swift:2010-2015`) /// records what /// reports its drops instead. The `.symlinkRoot` issue raised here follows /// `DevRootsStore.swift:349-355`, not that function. @@ -572,7 +572,7 @@ enum EphemeralTempRoots { // syscall blocks. And this is CONSTRUCTION, not scan time: // `EphemeralTempRoots.resolve` runs inside // `SpaceScannerRuntime.production` - // (`SpaceScannerRuntime.production` (`SpaceScanner.swift:2129`)), which + // (`SpaceScannerRuntime.production` (`SpaceScanner.swift:2158`)), which // the GUI calls from `CacheoutViewModel.production` // (`CacheoutViewModel.swift:555-574`) at the `@MainActor` view // model's construction (`CacheoutApp.swift:58`), so the block lands @@ -586,7 +586,7 @@ enum EphemeralTempRoots { // The root is DROPPED, not kept-and-skipped, and that difference is // the fix: a kept root reaches the runtime's cross-scanner union, // where `suppressingAliasShadows`' probe pair - // (`SpaceScanner.swift:1992-1996`) canonicalizes and probes it — + // (`SpaceScanner.swift:2021-2025`) canonicalizes and probes it — // the remaining 2 of those 5 — still during construction. Dropping // is also fail-CLOSED in the same shape as alias suppression: the // root could not have been scanned (fn-6.2's own arm refuses it) and diff --git a/Sources/Cacheout/Scanner/SpaceScanner.swift b/Sources/Cacheout/Scanner/SpaceScanner.swift index 2ee72fc..6c71dd0 100644 --- a/Sources/Cacheout/Scanner/SpaceScanner.swift +++ b/Sources/Cacheout/Scanner/SpaceScanner.swift @@ -2040,7 +2040,7 @@ struct SpaceScannerRuntime { // #459 codex r16). `sessionContainerRoots` needs to know which // union entries a participating scanner's declared root can // MATCH, and matching is by canonical identity - // (`PathGuard.matchConfiguredRoot`, PathGuard.swift:462-469), not + // (`PathGuard.matchConfiguredRoot`, PathGuard.swift:536-543), not // by spelling. Re-canonicalizing at session time would pay this // construction's realpath bill again — per session, per trigger, // on exactly the roots the participation gate exists to leave @@ -3047,7 +3047,7 @@ struct SpaceScannerRuntime { /// /// WHY IT CANNOT STRAND A LATER CLEAN. Omission from the snapshot is /// fail-closed: `PathGuard.admitContainer` refuses a root it cannot find - /// there (PathGuard.swift:400-403). Both consumers already refuse the + /// there (PathGuard.swift:474-477). Both consumers already refuse the /// same items for an independent reason: /// /// - the ViewModel gates every destructive path on the scanner's @@ -3069,7 +3069,7 @@ struct SpaceScannerRuntime { /// /// WHY CANONICAL KEYS AND NOT JUST PATHS. Delete-time root matching is by /// canonical identity over the whole union, returning the FIRST match - /// (PathGuard.swift:462-469), and the snapshot is keyed by THAT root's + /// (PathGuard.swift:536-543), and the snapshot is keyed by THAT root's /// declared spelling. So a participating scanner's claim can legitimately /// key off a union entry only a NON-participating scanner declared — an /// alias spelling of the same location, including the case where the diff --git a/Tests/CacheoutTests/EphemeralTempRootsTests.swift b/Tests/CacheoutTests/EphemeralTempRootsTests.swift index dd067dc..4e07068 100644 --- a/Tests/CacheoutTests/EphemeralTempRootsTests.swift +++ b/Tests/CacheoutTests/EphemeralTempRootsTests.swift @@ -720,7 +720,7 @@ final class EphemeralTempRootsTests: XCTestCase { /// turns on. A root KEPT here would reach the runtime's cross-scanner /// union, where `SpaceScannerRuntime.suppressingAliasShadows` /// (`suppressingAliasShadows`' probe pair, - /// `SpaceScanner.swift:1992-1996`) canonicalizes and probes every root + /// `SpaceScanner.swift:2021-2025`) canonicalizes and probes every root /// it is given, still during construction — so the block would simply /// move one function along. Measured against this fixture before the /// preflight existed: `production()` made 5 calls naming the mounted diff --git a/Tests/CacheoutTests/EphemeralTempScannerTests.swift b/Tests/CacheoutTests/EphemeralTempScannerTests.swift index 6c2e6c2..dd40eac 100644 --- a/Tests/CacheoutTests/EphemeralTempScannerTests.swift +++ b/Tests/CacheoutTests/EphemeralTempScannerTests.swift @@ -3033,7 +3033,7 @@ final class EphemeralTempScannerTests: XCTestCase { /// payload; the delete-time revalidator allowed them; and the cleaner /// deleted them. No later gate refused, because the container-root policy /// refuses only `/`, volume roots and `$HOME` itself — `~/Documents` is a - /// legal container by design (`PathGuard.swift:350-361`). + /// legal container by design (`PathGuard.swift:424-435`). /// /// The victim is deliberately `/Documents`, the production shape, /// and the disposal is the PERMANENT arm so the assertion observes a real diff --git a/Tests/CacheoutTests/SourceAnchorIntegrityTests.swift b/Tests/CacheoutTests/SourceAnchorIntegrityTests.swift index 11f152a..0eb7220 100644 --- a/Tests/CacheoutTests/SourceAnchorIntegrityTests.swift +++ b/Tests/CacheoutTests/SourceAnchorIntegrityTests.swift @@ -172,33 +172,33 @@ final class SourceAnchorIntegrityTests: XCTestCase { "A failed read mid-directory: the rest is unprove"), ("PathGuard.swift:165-176", "refuses non-directory containers, so a link iden"), - ("PathGuard.swift:350-361", + ("PathGuard.swift:424-435", "throw PathGuardError.outsideCategoryPolicy(path:"), - ("PathGuard.swift:370", + ("PathGuard.swift:444", "`~/Documents` can be a container while `admitDel"), - ("PathGuard.swift:400-403", + ("PathGuard.swift:474-477", "(2) No-follow reality gate on BOTH spellings: th"), ("PathGuard.swift:45", "compared as `pathComponents` arrays (never `hasP"), - ("PathGuard.swift:462-469", + ("PathGuard.swift:536-543", "The filesystem root `/` is exempt from both: it"), ("ProjectTreeWalker.swift:559-562", "|| provider.isMountPoint(provider.canonicalize(c"), - ("SpaceScanner.swift:1941-1952", + ("SpaceScanner.swift:1970-1981", "no-follow reality gate to THAT spelling and refu"), ("SpaceScanner.swift:143", "`[\"git\", \"-C\", , \"worktree"), - ("SpaceScanner.swift:1981-1986", + ("SpaceScanner.swift:2010-2015", "Nothing is silently lost: a dropped root is unus"), - ("SpaceScanner.swift:1987-1989", + ("SpaceScanner.swift:2016-2018", "private static func suppressingAliasShadows("), - ("SpaceScanner.swift:1964-2004", + ("SpaceScanner.swift:1993-2033", "private static func suppressingAliasShadows("), - ("SpaceScanner.swift:1992-1996", + ("SpaceScanner.swift:2021-2025", "let probed = roots.map { root in"), - ("SpaceScanner.swift:2002-2005", + ("SpaceScanner.swift:2031-2034", "Two real-directory spellings of one location are"), - ("SpaceScanner.swift:2129", + ("SpaceScanner.swift:2158", "static func production("), ("SpaceScanner.swift:40-51", "(Documents, Desktop, …) are enumerated ONLY for"), From 9ce6b1de8a71f05b5e7707416e30ed439c56ee6c Mon Sep 17 00:00:00 2001 From: Bonanza Date: Thu, 27 Aug 2026 09:12:36 -0700 Subject: [PATCH 10/48] fix(safety): the sizing walk answers cancellation and bounds its entries (fn-4.15) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- Sources/Cacheout/Cleaner/CacheCleaner.swift | 38 ++- .../Cleaner/WorktreeReclaimPerformer.swift | 24 ++ .../Scanner/BuildArtifactsScanner.swift | 2 +- Sources/Cacheout/Scanner/DirectorySizer.swift | 95 +++++- .../Scanner/EphemeralTempScanner.swift | 14 +- .../Scanner/OrphanedCachesScanner.swift | 11 +- .../Cacheout/Scanner/ProjectTreeWalker.swift | 4 + .../Cacheout/Scanner/ValuablesDetector.swift | 4 +- .../BuildArtifactsScannerTests.swift | 2 +- Tests/CacheoutTests/CacheCleanerTests.swift | 100 ++++++ Tests/CacheoutTests/DirectorySizerTests.swift | 286 +++++++++++++++++- .../SourceAnchorIntegrityTests.swift | 8 +- Tests/CacheoutTests/StrandFenceTests.swift | 2 +- Tests/CacheoutTests/TestElementAccess.swift | 2 +- .../WorktreeReclaimPerformerTests.swift | 84 +++++ 15 files changed, 657 insertions(+), 19 deletions(-) diff --git a/Sources/Cacheout/Cleaner/CacheCleaner.swift b/Sources/Cacheout/Cleaner/CacheCleaner.swift index 4cd10ba..72e4768 100644 --- a/Sources/Cacheout/Cleaner/CacheCleaner.swift +++ b/Sources/Cacheout/Cleaner/CacheCleaner.swift @@ -343,7 +343,16 @@ actor CacheCleaner { ) { self.home = home self.provider = provider - self.sizer = DirectorySizer(provider: provider) + // UNCAPPED on purpose (fn-4.15): this is the DELETE-TIME verification + // sizer, and the mount-doctrine consumers below read the report's + // `mountBoundaries` as "the whole tree was swept" — a capped walk + // could not claim that, so a cap here would have to fail closed, and + // a deterministic cap that blocks deletion is a permanent strand + // (the same tree re-measures to the same cap forever). The pass + // stays bounded by the tree the caller is about to delete over the + // same entries anyway, and it is cancellable per entry; a CANCELLED + // (partial) report is refused by every consumer, not consumed. + self.sizer = DirectorySizer(provider: provider, entryCap: nil) self.pathGuard = PathGuard( home: home, containerRoots: containerRoots, provider: provider ) @@ -1176,6 +1185,20 @@ actor CacheCleaner { knownInodes: await registry.knownIdentities ) + // A CANCELLED measurement is a PARTIAL one (fn-4.15): the mount + // check below is only sound over a report that swept the whole tree, + // and a partial claims registration would under-account. Fail closed + // — and say honestly that a retry CAN differ (cancellation is a + // caller act, never a property of this tree). Bound where the fact + // is first read: this is the first consumer of `report`. + if report.cancelled { + let detail = "\(child.path): measurement was cancelled before " + + "the tree was fully inspected — refused, not deleted " + + "(cleaning again re-measures; this refusal is not permanent)" + logRefusal(label: label, tag: "measurement_cancelled", detail: detail) + return .failed(detail) + } + // ANY mount boundary in the measured tree — the child itself, or a // mounted subtree nested anywhere beneath it — refuses the deletion. // The sizer records-and-skips boundaries for SIZING, but `removeItem` @@ -1452,6 +1475,19 @@ actor CacheCleaner { knownInodes: await registry.knownIdentities ) + // A CANCELLED measurement is a PARTIAL one (fn-4.15) — same + // fail-closed rule as the category-child arm above, for the same two + // reasons (an unswept mount check, an under-registered claim set), + // and the same honest retryability: a new clean re-measures. + if report.cancelled { + let detail = "\(target.path): measurement was cancelled before " + + "the tree was fully inspected — refused, not deleted " + + "(cleaning again re-measures; this refusal is not permanent)" + logRefusal(label: item.displayName, tag: "measurement_cancelled", + detail: detail) + return (nil, [Self.itemError(item, detail)]) + } + // Same mount doctrine as category children (R15): a boundary // anywhere in the measured tree refuses the deletion — // `validateRemovableItem` catches the target ITSELF being a mount diff --git a/Sources/Cacheout/Cleaner/WorktreeReclaimPerformer.swift b/Sources/Cacheout/Cleaner/WorktreeReclaimPerformer.swift index 68b1da8..883e41b 100644 --- a/Sources/Cacheout/Cleaner/WorktreeReclaimPerformer.swift +++ b/Sources/Cacheout/Cleaner/WorktreeReclaimPerformer.swift @@ -695,6 +695,18 @@ struct WorktreeReclaimPerformer { let report = measure( worktreePath, .deletionTarget, await registry.knownIdentities ) + // (3b) A CANCELLED measurement is a PARTIAL one (fn-4.15): the + // mount doctrine below is only sound over a report that swept the + // whole tree. Fail closed, first consumer of the report — and the + // refusal is honestly retryable (a new clean re-measures). + if report.cancelled { + let detail = "\(worktreePath.path): measurement was cancelled " + + "before the tree was fully inspected — refused, not " + + "deleted (cleaning again re-measures; this refusal is not " + + "permanent)" + logRefusal("measurement_cancelled", detail) + return failure(item, detail, tag: nil) + } // (4) Mount doctrine (`removeGuardedItem` parity): a boundary at the // target or nested anywhere beneath it refuses the deletion — the // removal would recurse straight through an inner mount. @@ -1198,6 +1210,18 @@ struct WorktreeReclaimPerformer { let report = measure( directory, .deletionTarget, await registry.knownIdentities ) + // (4b) A CANCELLED measurement is a PARTIAL one (fn-4.15) — + // same fail-closed rule as the worktree arm, before any claim + // is registered; honestly retryable. + if report.cancelled { + let detail = "refused: measurement of affected admin " + + "directory \(directory.path) was cancelled before the " + + "tree was fully inspected — nothing was pruned " + + "(cleaning again re-measures; this refusal is not " + + "permanent)" + logRefusal("measurement_cancelled", detail) + return failure(item, detail, tag: nil) + } // (5) MOUNT DOCTRINE (epic round 9): the removal is a // RECURSIVE filesystem mutation over these directories, so the // boundary-bearing-recursive-delete rule applies exactly as it diff --git a/Sources/Cacheout/Scanner/BuildArtifactsScanner.swift b/Sources/Cacheout/Scanner/BuildArtifactsScanner.swift index 3f2be62..b45ef5b 100644 --- a/Sources/Cacheout/Scanner/BuildArtifactsScanner.swift +++ b/Sources/Cacheout/Scanner/BuildArtifactsScanner.swift @@ -722,7 +722,7 @@ struct BuildArtifactsScanner: @unchecked Sendable { /// descendant (review r3). The house rule VERBATIM, no third notion /// invented: device-id change against the ANCESTOR, plus the `statfs` /// mount-root check that catches the same-`st_dev` firmlink mounts a - /// device comparison is blind to (`DirectorySizer.swift:354-359`, + /// device comparison is blind to (`DirectorySizer.swift:443-448`, /// `ProjectTreeWalker.swift:559-562`, `ValuablesDetector.swift`). The sizer /// records the boundary and skips its subtree uncounted; the cleaner /// refuses any tree containing one whole diff --git a/Sources/Cacheout/Scanner/DirectorySizer.swift b/Sources/Cacheout/Scanner/DirectorySizer.swift index 2323275..6b39c93 100644 --- a/Sources/Cacheout/Scanner/DirectorySizer.swift +++ b/Sources/Cacheout/Scanner/DirectorySizer.swift @@ -116,6 +116,13 @@ struct SizeDenial: Equatable { /// distinguished. What it is NOT any more is a reason the item /// cannot be DELETED: `DepthSafeRemoval` addresses this tree. case unaddressablePath + /// The walk hit this sizer's entry cap with entries still unread + /// (fn-4.15). DETERMINISTIC: the same tree re-measures to the same + /// cap, so no re-scan can ever clear this — the denial's `detail` + /// must state permanence (the only clearing act is shrinking the + /// tree) and must never promise a retry. Every byte/count figure in + /// a capped report is a FLOOR over the first `entryCap` entries. + case enumerationCapped /// Anything else — recorded, never swallowed. case other } @@ -185,6 +192,17 @@ struct SizeReport { /// child churn and lie about content age. `nil` when no regular file's /// date could be read. var newestContentDate: Date? + /// The walk observed task cancellation and STOPPED with entries still + /// unread (fn-4.15). Every figure in the report is then a floor over + /// whatever was measured before the stop — a caller may not consume a + /// cancelled report as a complete measurement (the delete-time mount + /// doctrine reads `mountBoundaries` as "the whole tree was swept", which + /// a cancelled walk cannot claim). Deliberately NOT a denial: nothing + /// refused the read, and a retry CAN differ — cancellation is a caller + /// act, not a property of the tree. Never set by a walk that ran out of + /// entries before it observed the cancellation: such a report IS + /// complete. + var cancelled: Bool = false /// The two byte components summed — what a scan row displays today. var measuredBytes: Int64 { exactAllocatedBytes + estimatedUpToBytes } @@ -200,11 +218,50 @@ struct DirectorySizer { case deletionTarget } + /// The default entry cap a sizing walk spends before it stops and + /// records `.enumerationCapped` (fn-4.15). THE VALUE IS A DESIGN + /// DECISION, sized against measurement, not folklore: this walk runs at + /// ~111k entries/s on a warm APFS volume (measured over a 20k-entry + /// fixture; the figure is printed by + /// `DirectorySizerTests.testAMidWalkCancelReturnsPromptly` on every + /// run), and the largest real trees this project has evidence for are + /// far below the cap — a full `.build` here holds 5,917 entries for + /// 481 MB, which extrapolates the 23G field-evidence worktrees tree + /// (`FIELD-EVIDENCE-2026-08-06.md`, scenario 2) to roughly 300k + /// entries. So 2,000,000 clears every evidenced real tree by better + /// than 6x while bounding a pathological one at ~18 s of walk, and + /// cancellation (checked every entry, 25 µs cancel-to-return measured) + /// covers the user who will not wait even that long. + /// + /// COMPOSITION (the product rule): this cap is PER MEASURE CALL. A + /// scanner that sizes N candidates spends at most N × cap entries per + /// scan — bounded per scan because every scanner's candidate list is + /// itself bounded (categories: fixed root list; orphaned-caches sweep: + /// one directory's children; build artifacts / worktrees: the walker's + /// own entry budget) — and the per-entry cancellation check is what + /// bounds the wait a user actually experiences. + static let defaultEntryCap = 2_000_000 + private let provider: FileSystemIdentityProvider + /// Entries one `enumerateTree` walk may read before stopping with + /// `.enumerationCapped`; `nil` = uncapped. `nil` is reserved for the + /// DELETE-TIME verification sizer (`CacheCleaner`'s): the mount doctrine + /// consumes `mountBoundaries` as proof the WHOLE tree was swept, so a + /// capped delete-time walk would have to fail closed — and a + /// deterministic cap that blocks deletion is a permanent strand (the + /// tree can never measure differently). Delete-time work stays bounded + /// anyway: the sizing pass is at most proportional to the deletion the + /// caller is about to perform over the same tree, and it is cancellable + /// per entry. + private let entryCap: Int? private let fileManager = FileManager.default - init(provider: FileSystemIdentityProvider = FileSystemIdentityProvider()) { + init( + provider: FileSystemIdentityProvider = FileSystemIdentityProvider(), + entryCap: Int? = DirectorySizer.defaultEntryCap + ) { self.provider = provider + self.entryCap = entryCap } /// Measure the tree (or leaf) at `url`. Pure function: never mutates @@ -321,7 +378,37 @@ struct DirectorySizer { let rootDevice = provider.deviceID(of: root) while let next = enumerator.nextObject() { + // CANCELLATION, between entries (fn-4.15) — the walk was + // unabortable before this: a scanner cancelled mid-measure kept + // walking to the end of the tree. Checked BEFORE the pulled + // entry is processed, so a cancelled walk never half-counts an + // entry; the granularity is one entry's processing (measured in + // `DirectorySizerTests.testAMidWalkCancelReturnsPromptly`). + // Reached only when the enumerator HANDED BACK another entry, so + // an exhausted walk that never observed the flag stays complete + // (`cancelled == false`) — see `SizeReport.cancelled`. + if Task.isCancelled { + report.cancelled = true + break + } guard let itemURL = next as? URL else { continue } + // THE ENTRY CAP (fn-4.15), spent only when a FURTHER entry + // actually exists: an exact-fit tree ends the loop above with no + // truncation claim (the `PrefilterBudget.wasCutShort` rule). The + // cap is deterministic over a static tree, so the disclosure + // states permanence and offers no retry — the + // deterministic-bound doctrine's one honest wording. + if let cap = entryCap, report.enumeratedEntries >= cap { + report.denials.append(SizeDenial( + url: root, kind: .enumerationCapped, + detail: "this folder holds more than \(cap) entries — " + + "more than a sizing walk will ever read, so every " + + "figure shown for it is a floor over the first " + + "\(cap); measuring again cannot reach further " + + "(only removing entries from the folder can)" + )) + break + } // The CENSUS, counted before any classification: every entry the // enumerator yielded, whatever it turns out to be and whether or // not it survives to contribute bytes. @@ -561,7 +648,11 @@ extension SizeDenial.Kind { switch self { case .tcc: return .tccDenied case .permission: return .permissionDenied - case .metadata, .other, .unaddressablePath: return .other + // The cap is neither a TCC nor a BSD denial and no grant lifts it; + // `.other` here, and the truncation-specific `ScanIssue` kind where + // an issue surface exists (`OrphanedCachesScanner.rootIssueKind`). + case .metadata, .other, .unaddressablePath, .enumerationCapped: + return .other } } } diff --git a/Sources/Cacheout/Scanner/EphemeralTempScanner.swift b/Sources/Cacheout/Scanner/EphemeralTempScanner.swift index d458c05..885a044 100644 --- a/Sources/Cacheout/Scanner/EphemeralTempScanner.swift +++ b/Sources/Cacheout/Scanner/EphemeralTempScanner.swift @@ -159,7 +159,7 @@ /// `.tcc` ⇒ NEUTRAL `.other`-kind `ScanError` with the detail preserved, /// because `SizeDenial.Kind.tcc` CONFLATES chain-proven denials /// (`classifyDenial`'s `case .some(Int(EPERM))` arm, -/// `DirectorySizer.swift:483`) with raw-probe guesses +/// `DirectorySizer.swift:570`) with raw-probe guesses /// (`denial(forFailedProbe:errno:)`'s `case EPERM`, :545); the only /// surviving discriminator is a detail STRING, and classification derived /// from message text is forbidden house doctrine (`CacheCleaner.refusalTag` @@ -1876,6 +1876,11 @@ struct EphemeralTempScanner: @unchecked Sendable { // descriptor-relative and handles such trees whole. case .metadata, .other, .unaddressablePath: return (.unreadable, "\(url.path): \(denial.detail)") + // Unreachable from `classifyDenial` (errno classification never + // yields the cap); mapped rather than defaulted so the switch + // stays a reviewer's inventory (fn-4.15). + case .enumerationCapped: + return (.enumerationTruncated, "\(url.path): \(denial.detail)") } case .metadataUnavailable: return (.unreadable, "\(url.path): metadata unavailable") @@ -1890,6 +1895,13 @@ struct EphemeralTempScanner: @unchecked Sendable { "\(denial.url.path): \(denial.detail)") case .tcc, .metadata, .other, .unaddressablePath: return (.unreadable, "\(denial.url.path): \(denial.detail)") + // The sizer's entry cap (fn-4.15): the taxonomy's truncation + // kind, whose GUI label ("too many entries — partially + // inspected") is exactly true; the detail carries the sizer's + // own no-retry sentence verbatim. + case .enumerationCapped: + return (.enumerationTruncated, + "\(denial.url.path): \(denial.detail)") } } } diff --git a/Sources/Cacheout/Scanner/OrphanedCachesScanner.swift b/Sources/Cacheout/Scanner/OrphanedCachesScanner.swift index c824b10..0457d1c 100644 --- a/Sources/Cacheout/Scanner/OrphanedCachesScanner.swift +++ b/Sources/Cacheout/Scanner/OrphanedCachesScanner.swift @@ -1048,8 +1048,8 @@ struct OrphanedCachesScanner: @unchecked Sendable { /// decision is taken on this spelling. /// /// NO-CROSS rule (safety — PR #458 review, matching the sizer's own - /// root check at `DirectorySizer.swift:261-272` and its within-walk check at - /// `DirectorySizer.swift:354-359`): mount boundaries are never crossed, at + /// root check at `DirectorySizer.swift:317-332` and its within-walk check at + /// `DirectorySizer.swift:443-448`): mount boundaries are never crossed, at /// the root or anywhere beneath it, and an uncrossed boundary makes the /// probe INCOMPLETE. Both signals are now read from DESCRIPTORS — /// `f_fsid` plus `st_dev` (see `crossesMountBoundary`) — which is @@ -1440,7 +1440,7 @@ struct OrphanedCachesScanner: @unchecked Sendable { } // MOUNT BOUNDARY AT THE ROOT, before anything below it is read — - // the same stance `DirectorySizer.swift:261-272` takes on its own root. + // the same stance `DirectorySizer.swift:317-332` takes on its own root. // An entry that IS a mount is not enumerated at all: not one entry // of the foreign filesystem is read, and the verdict is INCOMPLETE // precisely because we did not look. @@ -2948,6 +2948,11 @@ extension OrphanedCachesScanner: SpaceScanner { case .tcc: return .tccDenied case .permission: return .permissionDenied case .metadata, .other, .unaddressablePath: return .unreadable + // The sizer's entry cap (fn-4.15) is the condition the GUI already + // words for this issue kind ("too many entries — partially + // inspected") — deterministic, so the denial's own detail carries + // the no-retry wording. + case .enumerationCapped: return .enumerationTruncated } } diff --git a/Sources/Cacheout/Scanner/ProjectTreeWalker.swift b/Sources/Cacheout/Scanner/ProjectTreeWalker.swift index 1d97052..788f1f1 100644 --- a/Sources/Cacheout/Scanner/ProjectTreeWalker.swift +++ b/Sources/Cacheout/Scanner/ProjectTreeWalker.swift @@ -656,6 +656,10 @@ struct ProjectTreeWalker { // `overlongDescendantPathBytes` now feeds the row's SIZE CAVEAT only // — the refusal it used to drive was retired with its premise. case .metadata, .other, .unaddressablePath: kind = .unreadable + // The sizer's entry cap (fn-4.15). This walker never runs the sizer's + // enumeration, so the arm is unreachable today; it maps to the + // taxonomy's truncation kind so a future sizer-fed path stays honest. + case .enumerationCapped: kind = .enumerationTruncated } return ScanIssue(url: denial.url, kind: kind, detail: denial.detail) } diff --git a/Sources/Cacheout/Scanner/ValuablesDetector.swift b/Sources/Cacheout/Scanner/ValuablesDetector.swift index b9b56ef..0a0da25 100644 --- a/Sources/Cacheout/Scanner/ValuablesDetector.swift +++ b/Sources/Cacheout/Scanner/ValuablesDetector.swift @@ -140,7 +140,7 @@ /// this machine, `st_dev` is identical for literally every path INCLUDING /// `/` and `/System/Volumes/Data`, so the device comparison is blind to /// exactly the APFS firmlink split it was partly meant to catch. The two -/// path-based signals the sizer (`DirectorySizer.swift:261-272,354-359`) and the +/// path-based signals the sizer (`DirectorySizer.swift:317-332,443-448`) and the /// project walker already use are retained beside it — they are the seam /// hermetic tests inject through, and they can only ever push the answer /// toward refusal. There is still exactly ONE notion of "mount boundary" in @@ -939,7 +939,7 @@ enum ValuablesDetector { descriptorWindow: Int? = nil ) -> ValuablesDisclosure { // MOUNT BOUNDARY AT THE ROOT. The sizer applies these signals to its - // OWN root (`DirectorySizer.swift:261-272`) and declines to enumerate; the + // OWN root (`DirectorySizer.swift:317-332`) and declines to enumerate; the // probe must decline identically, or the delete-time face — which has // no size report to consult — would read a whole mounted volume. // Nothing beneath is opened: not one entry of a foreign filesystem is diff --git a/Tests/CacheoutTests/BuildArtifactsScannerTests.swift b/Tests/CacheoutTests/BuildArtifactsScannerTests.swift index 444ad8c..095aca6 100644 --- a/Tests/CacheoutTests/BuildArtifactsScannerTests.swift +++ b/Tests/CacheoutTests/BuildArtifactsScannerTests.swift @@ -3770,7 +3770,7 @@ final class BuildArtifactsScannerTests: XCTestCase { async throws { // The ROOT cell. The sizer declines to enumerate its own root when - // that root is a mount (`DirectorySizer.swift:261-272`); the probe must + // that root is a mount (`DirectorySizer.swift:317-332`); the probe must // decline identically, or it reads a whole foreign volume that the // caller has already denied. let artifact = try makeProject( diff --git a/Tests/CacheoutTests/CacheCleanerTests.swift b/Tests/CacheoutTests/CacheCleanerTests.swift index a950a3f..bf3e85f 100644 --- a/Tests/CacheoutTests/CacheCleanerTests.swift +++ b/Tests/CacheoutTests/CacheCleanerTests.swift @@ -553,6 +553,106 @@ final class CacheCleanerTests: XCTestCase { XCTAssertFalse(FileManager.default.fileExists(atPath: goodB.path)) } + // MARK: - fn-4.15: a CANCELLED (partial) measurement fails closed + + /// Runs `body` inside a task that is PROVABLY cancelled before `body` + /// starts (bounded spin on `Task.isCancelled`, then the already-cancelled + /// task runs `body`); cancellation's stickiness makes every + /// `Task.isCancelled` read inside answer true — deterministic, no race. + private func inPreCancelledTask( + _ body: @escaping @Sendable () async -> T + ) async -> T? { + let task = Task.detached { () -> T? in + var spins = 0 + while !Task.isCancelled { + spins += 1 + if spins > 1_000_000 { return nil } // bounded, never a park + await Task.yield() + } + return await body() + } + task.cancel() + return await task.value + } + + /// The CATEGORY-CHILD arm: the delete-time sizer is uncapped but + /// cancellable, and a walk that stopped on cancellation swept only part + /// of the tree — its mount check and its claims are both partial, so the + /// child is refused, not deleted. REAL path end to end: the production + /// sizer marks the report inside an already-cancelled task. + /// + /// MUTATION: delete the `report.cancelled` guard in the category-child + /// arm — RED here (the child is deleted despite the partial sweep). + func testACancelledMeasurementRefusesTheCategoryChildNotDeletes() async throws { + let root = try makeTempDir("cancelled-category-root") + defer { try? FileManager.default.removeItem(at: root) } + let child = root.appendingPathComponent("payload-dir") + try FileManager.default.createDirectory(at: child, withIntermediateDirectories: true) + try writeFile(child.appendingPathComponent("payload.bin")) + + let category = makeCategory(at: root, name: "cancelled-cat") + let cleaner = CacheCleaner(containerRoots: []) + let items = categoryItems([makeScanResult(category: category)]) + + let cleaned = await inPreCancelledTask { + await cleaner.clean(items: items, moveToTrash: false) + } + let report = try XCTUnwrap(cleaned) + + XCTAssertTrue(FileManager.default.fileExists(atPath: child.path), + "a partially-measured child must not be deleted") + XCTAssertEqual(report.errors.count, 1) + let message = try XCTUnwrap(report.errors.first?.message) + // WHICH refusal fired: cancellation, not the mount arm and not a + // denial — and honestly retryable (a new clean re-measures). + XCTAssertTrue(message.contains("cancelled"), message) + XCTAssertTrue(message.contains("not permanent"), message) + + // CONTROL: the identical fixture, uncancelled, deletes cleanly — so + // the refusal above is attributable to cancellation alone. + let control = await cleaner.clean(items: items, moveToTrash: false) + XCTAssertTrue(control.errors.isEmpty, + "\(control.errors.map(\.message))") + XCTAssertFalse(FileManager.default.fileExists(atPath: child.path)) + } + + /// The ITEM-TARGET arm: same rule through `clean(items:)`'s removeItem + /// pipeline. + /// + /// MUTATION: delete the `report.cancelled` guard in the item arm — RED + /// here (the target is deleted despite the partial sweep). + func testACancelledMeasurementRefusesTheItemTargetNotDeletes() async throws { + let root = try makeTempDir("cancelled-item-root") + defer { try? FileManager.default.removeItem(at: root) } + let target = root.appendingPathComponent("proj/artifacts") + try FileManager.default.createDirectory(at: target, withIntermediateDirectories: true) + try writeFile(target.appendingPathComponent("a.json")) + + let items = [removableItem(at: target, originContainer: root)] + let cleaner = CacheCleaner( + containerRoots: [root], containerSnapshot: sessionSnapshot(of: [root]) + ) + + let cleaned = await inPreCancelledTask { + await cleaner.clean(items: items, moveToTrash: false) + } + let report = try XCTUnwrap(cleaned) + + XCTAssertTrue(FileManager.default.fileExists(atPath: target.path), + "a partially-measured item must not be deleted") + XCTAssertEqual(report.errors.count, 1) + XCTAssertTrue(report.entries.isEmpty, "nothing was freed") + let message = try XCTUnwrap(report.errors.first?.message) + XCTAssertTrue(message.contains("cancelled"), message) + XCTAssertTrue(message.contains("not permanent"), message) + + // CONTROL: uncancelled, the same item deletes. + let control = await cleaner.clean(items: items, moveToTrash: false) + XCTAssertTrue(control.errors.isEmpty, + "\(control.errors.map(\.message))") + XCTAssertFalse(FileManager.default.fileExists(atPath: target.path)) + } + // MARK: - Honest freed bytes (R1/R16) func testCategoryFreedEqualsMeasuredDeletedBytesAcrossTwoPaths() async throws { diff --git a/Tests/CacheoutTests/DirectorySizerTests.swift b/Tests/CacheoutTests/DirectorySizerTests.swift index e01a283..17e5745 100644 --- a/Tests/CacheoutTests/DirectorySizerTests.swift +++ b/Tests/CacheoutTests/DirectorySizerTests.swift @@ -78,9 +78,23 @@ final class DirectorySizerTests: XCTestCase { } private func makeSizer( - provider: FileSystemIdentityProvider = FileSystemIdentityProvider() + provider: FileSystemIdentityProvider = FileSystemIdentityProvider(), + entryCap: Int? = DirectorySizer.defaultEntryCap ) -> DirectorySizer { - DirectorySizer(provider: provider) + DirectorySizer(provider: provider, entryCap: entryCap) + } + + /// `count` EMPTY files under `root` — creation speed matters more than + /// bytes for the cancellation/cap fixtures (the walk stats each entry + /// either way). + private func makeManyEmptyFiles(_ root: URL, count: Int) throws { + try mkdir(root) + for index in 0..