Tracking fws U4: read-scope recording and separate clipping reports

What this unit delivers: the read side of the same story. Each read records what it actually saw - a bigint stat pair brackets the read, the target is observed only when both tokens match, and the record says whether the view was complete - and "lines clipped in this view" and "lines omitted from this view" become two separate notices.

Why it matters: a slice, a truncated read, or a lossy indentation view must never authorize a full-file replacement later, and the model needs to be told about clipping and omission separately.

Implemented by: PR #1913 (branch fws/u4-read-scope-recording).

Acceptance:

Observation only when both stat tokens match; a stat failure leaves the target unobserved and never fails the read.
complete recorded so slice/truncated/lossy views never authorize a full-file replacement.
Task.disposeOnce() retires the registry; post-abort reads skip the observation work.
Project-scoped MCP settings writes confine to the provider cwd; global writes stay unconstrained by design.

Part of the file-write-safety epic #1375, landed as a nine-PR chain tracked on #1991. Merge order: U1 -> U2 -> U3 -> U4 -> U5 -> U8 -> U6 -> U7 -> U9.

Merge upstream/main (73931fe) into fws/u4-read-scope-recording

Reconcile the carried lock-key spec with the replacement protocol

Mark safeWriteText internal now that nothing in production calls it

Add the notice-separator row the review asked for on the clipped full…

Stage the confined write's bytes after staging, and copy the target m…

Read the staged bytes after staging, where the escape undo can use them

Copy the target's mode onto the staged file so the publish cannot wid…

Compare the interruption injection against identity so main's rows fi…

Planned-unit list, added by lane u7-1918 on 2026-11-09 (unit U4, PR #1913, head `4132582f4de`)

CodeRabbit offered follow-up issues on five threads. The lead declined all five: an issue does not create mutation headroom, a split unit does. They are carried here instead, each with the cost measured against the gate own counter (selectFromGit in scripts/stryker-diff.mjs, base 73931fe8be, throwaway commits parented on 4132582f4de, deleted afterwards).

The arithmetic that makes this absolute: safeWriteText.ts and safeWriteJson.ts arrive in this branch as new or wholly rewritten files, so every one of their executable lines is already inside the counted 500. Replacing a line costs 0 against the cap; adding one costs 1. The gate throws above 500 (scripts/stryker-diff.mjs:265) and CI printed extension (500 lines) for this head, so headroom is 0 and only a split unit changes it.

Free in cap terms - need a decision plus a lane run, not budget:

6098w- staging directory compared lexically (safeWriteText.ts:317): +0 measured, one-line replacement reusing canonicalDirKey, which already canonicalises the nearest existing ancestor. Open question for the split unit: a tempPath in a missing directory then compares against a canonicalised parent instead of the literal parent.
6098ah flush opens the mode-narrowed staged file r+ (safeWriteJson.ts:610): a replacement of an already-counted line, same class.

Charged - need new budget from the split:

60-CvM resolveLockKey swallows resolution errors (safeWriteText.ts:261-276): +3 (classify the primary error), +4 (classify the walk readlink errors), +7 (both), all measured; reviewer wants unexpected errors propagated rather than terminating the walk.
6098aj undo runs although the commit rename failed: +3 estimated for the commit flag (not prototyped).
6098aU ancestor directories never fsynced: about +5 estimated.
6098af target bound to the held lock (A -> B -> A): +3..4 estimated.
6098ac identity window between the final stat and the unlink: restructure of 188-207, cost depends on shape.
6098ad dangling link rejected before recovery: reviewer contract - recover under the referent lock only after the confinement check, then re-resolve, keep the ENOENT rejection when no backup matches; test both directions plus confinement-fails-in-recovery.
6098ak teardown skipped when revalidation throws: reviewer contract - escape error stays primary, rollback failure and backup location reported separately, preserve the backup, never skip teardown because revalidation failed.
6098ai, 6098aX, 6098aa Windows ACL and descriptor work: several lines plus a platform split; libuv chmod only toggles the write bit on this box, so I am not putting numbers on them.
6098ab lock key not revalidated after acquisition: design choice between the helper and every caller; jsonFileRecovery.ts reads through withFileLock with no revalidation of its own.
6098xJ cleanup cannot read a 0o200 escaped file: +1..2.

Routed, not owned here: 6098xN (backup named for the referent while a scan searches the alias). The publishing path resolves before recovery (safeWriteJson.ts:343); the alias-shaped scan is src/utils/jsonFileRecovery.ts:30/33/43 against :48. The files endpoint shows that file in 2357 26-file delta and 2359 28-file delta and in no other unit of this chain; declared merge order puts 2357 first, so the fix belongs in 2357 and ports forward to 2359.

Standing note for whoever clears these threads: at 4132582f4de there is no Zoo Code / PR review gate check run - 0 of 72 check runs carry that name; 15 reconcile PR review state (1913) runs and 39 resolve PRs to reconcile runs are what exists. An absent check run is not a passing one.

Rows carried from PR #1912 (unit U5, branch fws/u3-observation-completeness, head 9d420ad0c3ab012948ba16fb8c11fcf52121fa15).

The reviewer pass of 2026-11-09T06:37Z opened eight further rows on that PR. Eleven threads are open; one (a mode-preservation test) was fixed in 9d420ad0c3. The ten deferrals below are
deferred with a number rather than a mood: the mutation/size arm on that branch is red on
headroom alone - CI job 129040967378 prints "Mutation gate failed: extension has 672 changed
executable lines (limit 500). Split the PR or obtain a maintainer-reviewed narrow exclusion.", and
I reproduce 672 locally with scripts/stryker-diff.mjs selectFromGit against merge base
73931fe8be253b93ec11103a0672c615e849f895. Adding a counted line costs 1 and replacing one costs
0, so no production line lands there until the split lands; in each case the regression test the
reviewer asks for is red without the production line, so test and fix move together.

4510045535 - src/utils/safeWriteJson.ts:362 (Major, Security): the canonical target is
recomputed at :360 and its parent is pinned at :362 without re-running the scope test that
:345 runs for the supplied path, and _ancestorIdentities (:203-212) degenerates the pin to
[root] when the relative path escapes. One executable line. Reviewer reply 4510480809, my
answer 4510477160 and 4510523797.
4510045541 - :367 (Minor, Functional Correctness): reachable TargetMovedError. I first
argued the comparison sides agree; that was wrong - resolvePublishTarget
(src/services/file-safety/safeWriteText.ts:317-332) returns the supplied, lexical path at
:330 when realpath hits ENOENT and lstat confirms the path absent, so the pinned
expectedResolvedPath is lexical while the file is missing and canonical after recovery
restores it at :378. Fix plus a row that swaps the alias directory and a missing target
through it. Reviewer reply 4510502880, my answer 4510496600 and 4510516119.
4510045547 - :441 (Major, Data Integrity): safeWriteJson performs no fstat/lstat of
the staged file (0 matches for either in the file) before its commit rename at :441, while
safeWriteText.ts captures staged identity from the descriptor (:659, :698) and re-verifies
the name at :873-900. Mirror that block here. Reviewer reply 4510480496, my answer
4510477660 and 4510524563.
4504888149 - :441 (Major, Security): three re-verification points exist (:367, :405,
:437) and all throw rather than follow; what remains is the interval between the last check
and fs.rename, which no path-based Node API closes (the reviewer agrees - 4510483157). A
handle-bound rename or a fail-closed path is the shape. My answers 4510478090, 4510541075.
4504888130 - orig :294, now line: null (Minor, Security): confinement-before-recovery is
in place (:367 before :378, comment :363-366); what is unprotected is recovery's own
renames and unlinks after that check. The leftover-scan half (scan the referent directory) is in
src/utils/jsonFileRecovery.ts, which is in feat(file-safety): guard machinery in the writer, with the identity and rights refusals (U5-B, split 1/2 of #1914, #1375) #2357's and feat(tools): the guardedWrite tool on top of the writer's guard API (U5-A, #1375) #2359's deltas and not in this unit's
13-file delta, so it routes there. Reviewer reply 4510508067, my answers 4510497945,
4510517774.
4504888151 - :457 (Minor, Stability): corrected by the reviewer and by me in turn -
recovery already receives the referent's lock key, and the defect is the order: :335 rejects a
dangling link (safeWriteText.ts:329) before recovery is reached at :378, so an exit between
the fallback renames at :455/:457 leaves the symlink dangling and the next writer never
reaches recovery. This one is fixable inside safeWriteJson.ts. Reviewer reply 4510502764, my
answers 4510497074, 4510517411.
4510045497 - src/services/file-safety/safeWriteText.ts:429 (Minor, Data Integrity): the
realpath(path.dirname(key)).catch(() => undefined) fallback is deliberate (comment
:425-426); the exposure is lock granularity turning into a lost update, because
safeWriteJson.ts imports resolveLockKey at :11 and calls it at :305 and does not go
through safeWriteText publish, so two writers can hold different locks over the same JSON and
the path comparison at :860-865 cannot see it. Reviewer reply 4510486701, my answers
4510479409, 4510516554.
4510045502 - :430 (Minor, Data Integrity): path.resolve folds .. lexically before the
target's own components are resolved, so sub/../file.json with sub a directory symlink lands
on a wrong lock key. A component-wise walk is the fix, and it belongs with row 7 and row 9 in the
same function. Reviewer reply 4510483384, my answers 4510479880, 4510540193.
4510045504 - :436 (Minor, Stability): the throw at the eight-link bound is unconditional,
and there is no lstat between the walk (:413 onward) and that throw (the file's lstat sites
are :325, :531, :546, :874), so a legitimate depth-8 chain whose last referent is
transiently missing fails. I argued the primitive has no caller; true of safeWriteText publish
only - resolveLockKey is called by JSON writes today. Reviewer reply 4510486748, my answers
4510480346, 4510516985.
4510045513 - :879 (Minor, Data Integrity): the ENOENT branch throws without setting
stagedPathIsForeign (declared :590, set on the identity branch :895, consumed by the
cleanup guard at :1040, unlink at :1044), so cleanup can unlink a file that took the name.
One line plus its test, and the reviewer confirmed the mechanism independently (4510505469). My
answers 4510478955, 4510540619.
4504888118 - :903 (Minor, Data Integrity): the rename is already preceded by the
re-resolution at :860-865 and the identity re-verification at :873-900; the irreducible part
is :874 to :903. Thread stays open by agreement (4510505469). My answers 4510497560,
4510518246.

Fixed rather than deferred, recorded here so the split does not re-do it: 4510045532, the
mode-preservation test, shipped in 9d420ad0c3 at src/utils/__tests__/safeWriteJson.test.ts :1577-1612 (+36 test lines, 0 counted executable lines - the gate reads 672 before and after). It
pins the direction on platforms that store a mode; on Windows the pre-existing 0o600 row already
fails a hardcoded-0o600 implementation, which I measured under a control.

Measured six-unit pre-cut of #1912 (issue #1375) - nothing opened yet

Per the standing ruling the split plan lives here rather than in a new issue. Every number below came out of the gate's own functions (packageForPath, executableChangedLines, selectFromGit from scripts/stryker-diff.mjs) run against six disposable commits built locally, each measured from its own merge base (the previous unit's commit), not from the whole-PR diff. Test and spec lines cost 0 because the gate excludes __tests__/, *.spec.* and *.test.*.

Measured cost per unit

unit	counted executable lines	files carrying them	its own vitest lane	typecheck
1 - publish target + lock key resolution	168	safeWriteText.ts partial (lines 1-439)	6 passed (1 file)	exit 0, 0 errors
2 - publish primitive, staged identity, cleanup	304	safeWriteText.ts full	101 passed (2 files)	exit 0, 0 errors
3 - task-owned observation registry + its field	17	observationRegistry.ts 12, Task.ts 5	223 passed (2 files)	exit 0, 0 errors
4 - JSON writer resolution, lock key, confine plumbing, mode-copy helper	143	safeWriteJson.ts	5 passed (1 file)	exit 0, 0 errors
5 - JSON writer commit rename, recovery, cleanup logging	29	safeWriteJson.ts	81 passed (3 files)	exit 0, 0 errors
6 - MCP settings writes pass the workspace root as confineTo	19	McpHub.ts	93 passed (1 file)	exit 0, 0 errors

Cap verdict per unit from its own merge base: selectFromGit returned without throwing for all six, so none trips MAX_CHANGED_LINES = 500. The whole PR measures 672 counted lines (172 over), which is the reason for the split. Largest unit: 304.

The six counts sum to 680, eight more than the whole-PR 672. Those eight are the mode-copy helper lines unit 4 carries while its later call-site neighbours stay in unit 5: the lines between them shift position, so both diffs mark them changed and they get attributed twice. The boundary that sums to exactly 672 (hunks 1-13 vs 14-24, measured 110 + 54) was built, measured and failed typecheck, so those eight lines are the price of a unit that compiles alone.

Chain, merge order, fidelity

Merge order 1, 2, 3, 4, 5, 6; each commit's parent is the unit before it. After unit 6 the tree is byte-identical to the current head: git diff --name-only 229556893a2d1e5ae040a5f4d87a26a20b14da44..9d420ad0c3ab012948ba16fb8c11fcf52121fa15 printed 0 files. Nothing was pushed and no pull request opened; the commits live only as refs/tmp/u6-1912-cut1 .. refs/tmp/u6-1912-cut6 in the fork clone.

Shas with measured merge bases:

unit 1 350f56ae1540e0975c19f41cada9a60685f8fbbd (parent 73931fe8be253b93ec11103a0672c615e849f895, merge base 73931fe8be253b93ec11103a0672c615e849f895, counted 168)
unit 2 b7cae3c23d464d64e1eadb3617e5c8d916a0e825 (parent 350f56ae1540e0975c19f41cada9a60685f8fbbd, merge base 350f56ae1540e0975c19f41cada9a60685f8fbbd, counted 304)
unit 3 81c600f10c5af46262bc1f762599b660ca77c848 (parent b7cae3c23d464d64e1eadb3617e5c8d916a0e825, merge base b7cae3c23d464d64e1eadb3617e5c8d916a0e825, counted 17)
unit 4 490577edb2402e86b09d838462bae0afd25165f2 (parent 81c600f10c5af46262bc1f762599b660ca77c848, merge base 81c600f10c5af46262bc1f762599b660ca77c848, counted 143)
unit 5 aee35f770cb34bc7519cdbb73cbe3a4982965c50 (parent 490577edb2402e86b09d838462bae0afd25165f2, merge base 490577edb2402e86b09d838462bae0afd25165f2, counted 29)
unit 6 90ed0d4c11996e024c7f9cd90876a12806e5deb5 (parent aee35f770cb34bc7519cdbb73cbe3a4982965c50, merge base aee35f770cb34bc7519cdbb73cbe3a4982965c50, counted 19)

Verification run on each unit alone

Typecheck per cut: node node_modules/typescript/bin/tsc --noEmit -p src/tsconfig.pcmap.json from the scratch worktree root (interpreter node v22.23.4). The scratch config extends src/tsconfig.json and maps all 9 workspace @roo-code/* packages into the unit's own packages/*/src (18 path entries) - a plain donor junction would type the unit against the donor instead of itself. Printed: 14-20 s per cut, exit 0, 0 errors, six for six.
Lane: vitest v4.1.12 from src, --globals, only that unit's own test files - vitest.services.config.ts for units 1, 2, 6; vitest.core.config.ts for unit 3; vitest.misc.config.ts for units 4, 5. Printed test totals: 6, 101, 223, 5, 81, 93; every exit 0.
Lint and format on the files the cuts compose (the 439-line partial safeWriteText.ts plus the two spec slices): eslint --max-warnings=0 0 errors / 0 warnings, prettier --check clean. src/eslint-suppressions.json untouched, --prune-suppressions never used.

Boundaries, and the cuts I disproved

Unit 1 stops where resolvePublishTarget and resolveLockKey stop (line 439 of the new module); safeWriteText starts at line 450. Everything downstream consumes those two exports - that is the contract crossing.
Unit 3 is the registry plus the one Task.ts field that owns it. Measured: Task.ts is the only production file in this delta importing @roo-code (@roo-code/telemetry, @roo-code/cloud) and Task.spec.ts the only spec that does - two files, not three. Only unit 3 needs the overlay for its own code, although every unit needs it to typecheck the whole tree.
Unit 4 stops after the hunks carrying resolution, lock key, confineTo plumbing and the mode-copy helper; unit 5 carries the commit rename, recovery and cleanup-logging hunks; unit 6 is the three McpHub.ts call sites that pass the workspace root as confineTo and is only reviewable after unit 4 ships that parameter.
Disproved, kept as refs/tmp/u6-1912-cut1-specimport-disproved (95ca90cbe99d…): the first unit-1 draft kept the spec's full import list, which names safeWriteText before it exists - TS2305: Module '../safeWriteText' has no exported member 'safeWriteText'. Six unused imports and one unused fakeChild binding also had to come out before eslint printed 0.
Disproved, kept as refs/tmp/u6-1912-cut4-hunk13-disproved (e9624d893f…): the exact-672 boundary (hunks 1-13 vs 14-24) fails because the call to _copyTargetMode sits at line 423 while the definition is at line 587 in a later hunk - TS2304: Cannot find name '_copyTargetMode'. That dependency is what the boundary must respect.
Disproved, kept as refs/tmp/u6-1912-cut4-lane-failed (c0c714bf2d…): shipping the whole safeWriteJson.lockKey.spec.ts with unit 4 fails its own lane - does not log a cleanup error when the safety net finds the temp file already gone asserts one console.error and got two, at line 255, because the suppression of the second log ships with unit 5. The test now travels with unit 5, where the behaviour lives, and unit 5's lane prints 81 passed.

#1912 state measured while doing this (not part of the plan)

Review threads and checks, complete pagination: 2 GraphQL pages of 50, 53 threads, 32 resolved, 21 open, 137 thread comments, 24 outdated, root authors {"coderabbitai":53}; check runs fetched 0 of 0 over 1 page(s); queued or in progress: none; latest CodeRabbit review state null submitted null id null.
The mutation-diff run against the 672-line head is still the red check. These six units are the measured route back under the cap; nothing is opened, and the open rows are not booked as final while a bot pass is still moving.

(Lane u7-1918. Everything above is a printed measurement from this round; nothing here is estimated or heard.)

#1912 state - correction to the measurement line in my previous comment

The check-run and CodeRabbit lines in that comment were under-fetched, not zero: I queried the fork commit (easonLiangWorldedtech/Zoo-Code/commits/9d420ad0c3…/check-runs, which genuinely reports 0) instead of the upstream commit, and I stopped the reviews query at page 1. Re-run with complete pagination and printed HTTP statuses:

Upstream commit Zoo-Code-Org/Zoo-Code/commits/9d420ad0c3…/check-runs: HTTP 200, total_count 114, 114 runs fetched over 2 page(s); 96 reconcile/resolve runs and 18 real runs, 17 of the real ones success. Names and states: {"Zoo Code / reconcile PR review state (1912)":"completed/success , completed/success , completed/success , completed/success , completed/success , completed/success , completed/success , completed/success , completed/success , completed/success , completed/success , completed/success , completed/success , completed/success , completed/success , completed/success , completed/success , completed/success , completed/success , completed/success , completed/success , completed/success , completed/success , completed/success , completed/success , completed/success , completed/success , completed/success , completed/success , completed/success , completed/success , completed/success , completed/su.
Failures: [{"name":"mutation-diff","id":129040967378,"job":"https://github.com/Zoo-Code-Org/Zoo-Code/actions/runs/42581476200/job/129040967378"}]. Queued or in progress: none.
Reviews, paginated: HTTP 200, 112 reviews over 2 page(s); by author and state {"coderabbitai[bot]":{"CHANGES_REQUESTED":27,"COMMENTED":43},"easonLiangWorldedtech":{"COMMENTED":42}}. Latest CodeRabbit review: state null, submitted null, id null.
The thread census in that comment was already correct: 2 GraphQL pages of 50, 53 threads, 32 resolved, 21 open, 137 thread comments, 24 flagged outdated. Ten of those threads are new since the head commit I fixed earlier today, so the open rows are not final and I am not pressing for a human re-review on a head the bot is still annotating.

Everything else in the earlier comment (unit costs, shas, lanes, typecheck, lint, disproved cuts) was measured directly and stands as printed. (Lane u7-1918.)

#1912 state - second correction, the CodeRabbit line only

My correction comment printed "Latest CodeRabbit review: state null". That was my filter, not the API: I matched login coderabbitai and the API reports coderabbitai[bot], so the filter matched 0 of the 112 reviews. Re-measured with the paginated list and a prefix match:

Logins present across 112 reviews on 2 page(s): ["coderabbitai[bot]","easonLiangWorldedtech"]. CodeRabbit entries: 70 (27 CHANGES_REQUESTED, 43 COMMENTED).
Latest CodeRabbit review by submitted_at: state COMMENTED, submitted 2026-11-09T08:21:02Z, id 5823254898, body 0 bytes.
The head has not moved: it is still 9d420ad0c3ab012948ba16fb8c11fcf52121fa15. The thread count grew from 43 to 53 on that same head - the bot pass finished and added 10 rows, 24 threads now flagged outdated, 32 resolved, 21 open, 137 thread comments. Checks on the head: 114 total, 96 reconcile and 18 real (17 success, 1 failure: mutation-diff, job 129040967378); 0 runs queued or in progress, so the bot is no longer annotating this head.
Therefore the 21 open rows are new rows on an unchanged head and I have not answered them yet; they are this lane's next work, not part of the split plan above. (Lane u7-1918.)

Note: comment 6735280837 (the eleven deferred production rows for #1912) was filed on this tracker by mistake - this issue tracks fws U4 = #1913, while #1912 is tracked by #2018, where the pointer now stands. Nothing was deleted here; the rows that do belong to U4 were posted earlier (6734682909) and stay here.

Pointer so this issue does not read as a gap: the six-unit plan for #1912 I posted here (6736294037, plus its two measurement corrections 6736306244 and 6736313767) belongs on #2018, whose body names pull request 1912 - #2019 names 1913. I posted it here because I did not read the trackers' bodies first. The plan, with re-measured census numbers and the gate-evidence correction, is now #2018 comment 6736488173, and that comment supersedes these three for #1912 purposes. The eleven deferred rows in 6735280837 stay where they are. Nothing deleted, nothing edited in place. (Lane u7-1918.)

Navigation Menu

Repository navigation

Tracking fws U4: read-scope recording and separate clipping reports #2019

Planned-unit list, added by lane u7-1918 on 2026-11-09 (unit U4, PR #1913, head `4132582f4de`)

Measured six-unit pre-cut of #1912 (issue #1375) - nothing opened yet

Measured cost per unit

Chain, merge order, fidelity

Verification run on each unit alone

Boundaries, and the cuts I disproved

#1912 state measured while doing this (not part of the plan)

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Tracking fws U4: read-scope recording and separate clipping reports #2019

Description

Activity

easonLiangWorldedtech commented on Nov 9, 2026

Planned-unit list, added by lane u7-1918 on 2026-11-09 (unit U4, PR #1913, head 4132582f4de)

easonLiangWorldedtech commented on Nov 9, 2026

easonLiangWorldedtech commented on Nov 9, 2026

Measured six-unit pre-cut of #1912 (issue #1375) - nothing opened yet

Measured cost per unit

Chain, merge order, fidelity

Verification run on each unit alone

Boundaries, and the cuts I disproved

#1912 state measured while doing this (not part of the plan)

easonLiangWorldedtech commented on Nov 9, 2026

easonLiangWorldedtech commented on Nov 9, 2026

easonLiangWorldedtech commented on Nov 9, 2026

easonLiangWorldedtech commented on Nov 9, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

Planned-unit list, added by lane u7-1918 on 2026-11-09 (unit U4, PR #1913, head `4132582f4de`)