Tracking fws U3: observation registry with read completeness

What this unit delivers: a task-local observation registry that records when a full-file observation of a path exists, with a version token and a completion state. A partial read does not make a destination observable. Move retargeting (carrying the source's completeness onto a destination rather than inventing a new observation) is introduced with the move call site in the consuming unit, which carries its own behaviour tests.

Why it matters: this is the bookkeeping that lets a later guarded write tell "the model actually read this whole file" from "the model saw a slice" - the difference between allowing and refusing a full-file overwrite.

Implemented by: PR #1912 (branch fws/u3-observation-completeness).

Acceptance:

Registration, versioning and completion of observations, scoped to one task. Observations carry their creation timestamp so a retention policy can be applied later; expiry itself is deferred to the consumer units that need one (the fidelity source for this unit has no expiry API).
A partial read never marks a path complete.
Move retargeting (source -> destination, preserving the source's complete value) lands with the move call site in the consuming unit, with focused tests there; this unit ships the storage contract it builds on.
The registry is not wired into production call sites yet; the units after this one introduce the call sites with their own behaviour tests.

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.

Pointer: the eleven deferred production rows for #1912 - with their costs and their routing - were posted on #2019 at comment 6735280837 (2026-11-09T08:10:08Z). #2019 tracks fws U4, which is #1913; this issue tracks fws U3, the observation registry with read completeness, which is #1912, so those rows belong here and were filed one tracker late. They are not being copied: read them at the comment named above, and take this as the link. The misfiling is recorded on #2019 so nobody reads a silent gap there.

Six-unit split plan for #1912 (issue #1375) with measured per-unit costs - posted on the tracker this PR names

Tracker discipline note, mine: I first posted this plan on #2019 (comments 6736294037, 6736306244, 6736313767) because I did not read the trackers' own bodies first. #2018 names pull request 1912 and #2019 names 1913, so this issue is the right home for #1912 rows and for the plan. This comment supersedes those three for #1912 purposes and its census numbers are re-measured.

Every number below came out of the mutation gate's own functions (packageForPath, executableChangedLines, buildManifest, MAX_CHANGED_LINES = 500, scripts/stryker-diff.mjs) run against six disposable commits built locally. Each unit is measured from its own merge base, which is 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.*.

Whole PR as measured

head 9d420ad0c3ab012948ba16fb8c11fcf52121fa15, merge base 73931fe8be253b93ec11103a0672c615e849f895, 74 commits, 13 files, +5591 -38.
buildManifest on that delta throws with the gate's own wording: "extension has 672 changed executable lines (limit 500). Split the PR or obtain a maintainer-reviewed narrow exclusion."
Per file, uncapped: src/services/file-safety/safeWriteText.ts 472 (new, 1085 lines), src/utils/safeWriteJson.ts 164 (667 lines), src/services/mcp/McpHub.ts 19, src/core/task/observationRegistry.ts 12 (new, 60 lines), src/core/task/Task.ts 5. All eight test files 0. Total 672 against the 500 cap.
@roo-code imports in the delta: Task.ts (2 imports: @roo-code/telemetry, @roo-code/cloud) and Task.spec.ts - 2 files measured, not 3. Only unit 3 touches them, though every unit needs the overlay to typecheck the whole tree.

Measured cost per unit, with the gate verdict on each

unit	counted executable lines from its own merge base	buildManifest verdict	its own vitest lane	typecheck
1 - publish target + lock key resolution	168	returned: extension 168 lines in 1 file(s), under the 500 cap	Tests 6 passed (6)	exit 0, 0 errors
2 - publish primitive, staged identity, cleanup	304	returned: extension 304 lines in 1 file(s), under the 500 cap	Tests 101 passed (101)	exit 0, 0 errors
3 - task-owned observation registry + its field	17	returned: extension 17 lines in 2 file(s), under the 500 cap	Tests 223 passed (223)	exit 0, 0 errors
4 - JSON writer resolution, lock key, confine plumbing, mode-copy helper	143	returned: extension 143 lines in 1 file(s), under the 500 cap	Tests 5 passed (5)	exit 0, 0 errors
5 - JSON writer commit rename, recovery, cleanup logging	29	returned: extension 29 lines in 1 file(s), under the 500 cap	Tests 81 passed (81)	exit 0, 0 errors
6 - MCP settings writes pass the workspace root as confineTo	19	returned: extension 19 lines in 1 file(s), under the 500 cap	Tests 93 passed (93)	exit 0, 0 errors

Largest unit: 304 of 500. The six counts sum to 680 against the whole-PR 672. The eight extra 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 versus 14-24, measured 110 + 54) was built 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. git diff --name-only 90ed0d4c11996e024c7f9cd90876a12806e5deb5..9d420ad0c3ab012948ba16fb8c11fcf52121fa15 printed 0 files, so unit 6's tree is byte-identical to the current head. Nothing was pushed and no pull request was opened; the six commits exist only as refs/tmp/u6-1912-cut1 through refs/tmp/u6-1912-cut6 in the fork clone.

unit 1 350f56ae1540e0975c19f41cada9a60685f8fbbd (tree b6e9eaf218a6, parent 73931fe8be253b93ec11103a0672c615e849f895, measured merge base 73931fe8be253b93ec11103a0672c615e849f895, counted 168)
unit 2 b7cae3c23d464d64e1eadb3617e5c8d916a0e825 (tree da3fceef625c, parent 350f56ae1540e0975c19f41cada9a60685f8fbbd, measured merge base 350f56ae1540e0975c19f41cada9a60685f8fbbd, counted 304)
unit 3 81c600f10c5af46262bc1f762599b660ca77c848 (tree eda6ea070928, parent b7cae3c23d464d64e1eadb3617e5c8d916a0e825, measured merge base b7cae3c23d464d64e1eadb3617e5c8d916a0e825, counted 17)
unit 4 490577edb2402e86b09d838462bae0afd25165f2 (tree 99db9a5ea159, parent 81c600f10c5af46262bc1f762599b660ca77c848, measured merge base 81c600f10c5af46262bc1f762599b660ca77c848, counted 143)
unit 5 aee35f770cb34bc7519cdbb73cbe3a4982965c50 (tree fa45d4744817, parent 490577edb2402e86b09d838462bae0afd25165f2, measured merge base 490577edb2402e86b09d838462bae0afd25165f2, counted 29)
unit 6 90ed0d4c11996e024c7f9cd90876a12806e5deb5 (tree c93d4144480b, parent aee35f770cb34bc7519cdbb73cbe3a4982965c50, measured 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) against a scratch config that 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 types the unit against the donor instead of itself - packages/ipc alone printed 6 phantom errors (Cannot find module node-ipc) before I junctioned it. 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 and 6, vitest.core.config.ts for unit 3, vitest.misc.config.ts for units 4 and 5. Printed totals 6, 101, 223, 5, 81 and 93 passed; every exit 0.
Lint and format on the files the cuts compose (the 439-line partial safeWriteText.ts and the two spec slices): eslint --max-warnings=0 printed 0 errors and 0 warnings, prettier --check clean. src/eslint-suppressions.json untouched; --prune-suppressions never used.

Boundaries, and the disproved cuts pinned as refs

Unit 1 stops where resolvePublishTarget (declared at line 317) and resolveLockKey (line 395) stop - line 439; safeWriteText starts at line 450 and everything downstream consumes those two exports.
Unit 3 is the registry plus the one Task.ts field that owns it: the contract crossing between services/file-safety and core/task.
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, reviewable only once unit 4 ships that parameter.
Pinned refs/tmp/u6-1912-cut1-specimport-disproved (95ca90cbe99d…): unit 1 keeping the spec's full import list - 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.
Pinned refs/tmp/u6-1912-cut4-hunk13-disproved (e9624d893f…): the exact-672 boundary (hunks 1-13 versus 14-24, measured 110 + 54) - TS2304: Cannot find name _copyTargetMode, because the call sits at safeWriteJson.ts:423 and the definition at :587 in a later hunk. This is the dependency the boundary respects.
Pinned refs/tmp/u6-1912-cut4-lane-failed (c0c714bf2d…): 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 expects 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.

Gate-evidence correction I owe this issue

My first pass cited selectFromGit as the per-unit cap verdict. It is not a reliable cap check for a merge commit: selectFromGit runs its base argument through resolvePullRequestBase (scripts/stryker-diff.mjs:287), and for #1914 - whose head 7209a5f1681b… is a merge commit with parents d2b65a5a2c75… and 5319fe91fb0c… - it rewrote whatever base I passed to d2b65a5a2c75… and reported 8 changed executable lines with 1 advisory instead of tripping the cap. The cap lives in buildManifest (scripts/stryker-diff.mjs:255-261), so I ran that directly: it throws "extension has 672 changed executable lines (limit 500)" for #1912, throws "extension has 1044 changed executable lines (limit 500)" for #1914 (which reproduces the red CI job and matches an independent count of 1044 from executableChangedLines), and for all six units it returned a single package extension with 168, 304, 17, 143, 29 and 19 changed executable lines. For the chain the heads are ordinary commits, so selectFromGit left the base alone - re-checked: unit 1 through it reports merge base 73931fe and 168 lines, matching buildManifest.

#1912 state measured while doing this

Review threads with complete pagination (2 GraphQL pages of 50): 53 threads, 32 resolved, 21 open, 137 thread comments, 24 flagged outdated. Ten more threads than the 43 recorded for this same unchanged head, so the bot pass added rows; I have not answered them, none is booked as final, and I am not requesting a human re-review on a head the bot was still annotating.
Checks on the head, upstream commit endpoint with pagination: 114 runs over 2 page(s) - 96 reconcile/resolve and 18 real, of which 17 succeeded. The single failure is mutation-diff, job 129040967378, the 672-line case above. Runs still queued or in progress when I measured: 0.
Reviews with pagination: undefined over 2 page(s) - coderabbitai[bot] 27 CHANGES_REQUESTED and 43 COMMENTED; author 42 COMMENTED. Latest bot review null at null id null with a 0-byte body (its substance is in the threads).
The eleven production rows I deferred earlier sit in comment 6735280837 on Tracking fws U4: read-scope recording and separate clipping reports #2019 and are pointed at from here by 6736404421. I am leaving both where they are rather than copying content between issues, and I am not opening anything for this split.

(Lane u7-1918. Printed measurements only. Anything I carried without re-measuring is labelled as such - there is nothing of that kind in this comment except the two earlier comment ids I quote, which I verified exist on this round: 6736404421 and 6736404820 both return HTTP 200.)

Addendum to the plan above, requested by the lead, plus one correction to how I stated the size finding.

The 680 against 672 sum is a dependency cost, and the alternative shape exists. The boundary that adds up to exactly 672 - hunks 1-13 versus 14-24 in src/utils/safeWriteJson.ts, measured 110 + 54 - does not compile on its own: pinned as refs/tmp/u6-1912-cut4-hunk13-disproved (e9624d893f…), it fails with TS2304: Cannot find name '_copyTargetMode', because the helper is called at src/utils/safeWriteJson.ts:423 and defined at :587, on the far side of that line. So the shipped shape attributes those eight mode-copy lines to both unit 4 and unit 5, and the six counts sum to 680. Fidelity is carried by unit 6's tree being byte-identical to 9d420ad0c3ab012948ba16fb8c11fcf52121fa15 - git diff --name-only 90ed0d4c1199..9d420ad0c3 prints 0 files - not by the arithmetic adding up.

Measured alternative, if a maintainer prefers an exact sum. Moving hunks 1-10, 12 and 13 into unit 4 (leaving 11 and 14-24 to unit 5) gives 168 / 304 / 17 / 109 / 60 / 19 = 677 and keeps every unit under the 500 cap; it moves 34 counted lines from unit 5 to unit 4 and does not change unit 2's 304, which is the largest unit. I kept the first shape because it follows the call-site boundary rather than the hunk numbering, but both are measured and either is buildable from the pinned chain.

Correction to how I described the 672. I called the mutation-diff result advisory on the grounds that main's ruleset lists no required status-check contexts - measured true (ruleset detail endpoint: 6 rules, required_status_checks carrying no contexts). That is only half the picture: the same ruleset set carries merge_queue with squash strategy and ALLGREEN, so a head whose mutation-diff run is red is not admitted to the merge queue. "Passes everything required" and "cannot enter the merge queue" are both true of this head, and the second is the operative one. Stating it that way here so the number is not read as cosmetic. (Lane u7-1918.)

Two test obligations to carry with this plan, so whoever executes it does not have to read twenty-one review threads to find them. Both came out of the bot pass of 2026-11-09 on head 9d420ad0c3… and both were answered in-thread today (reply 4512028033 and reply 4512028254); they are recorded here because a deferral that lives only in a thread reply is invisible to the person building the units.

Dangling file symlink plus a pending backup - src/utils/__tests__/safeWriteJson.lockKey.spec.ts:205. The current mock at :203-205 makes fs.realpath succeed for the alias, so no case in this file exercises a dangling link. Target unit: unit 5 (JSON writer commit rename, recovery, cleanup logging, 29 counted lines measured from its own merge base). Test lines cost 0 under the counter, so this adds nothing to the unit's budget. The assertion must match the shipped contract: resolvePublishTarget (src/services/file-safety/safeWriteText.ts:318-330) rethrows on ENOENT when the lstat reports a symbolic link, so a dangling link aborts the write before recoverInterruptedReplacement is reached. The test therefore asserts the rejection and that the backup is still present for the next writer - not that a merge received restored content. Whether recovery should run before that rejection is the ordering question on the row at safeWriteJson.ts:391, and unit 5 owns both halves, so the two answers are settled in one place rather than in two threads.
Fresh lock versus stale lock, stated separately - src/utils/__tests__/safeWriteJson.test.ts:1308. The sweep at :1306-1310 is why the contrast write at :1311 passes at all: the failed write's lock release named the path, which resolved through the link at release time, so the empty lock directory came back with the moved-aside copy - the test's own comment at :1302-1305 says so. Target unit: unit 5, again at zero cost under the counter. Shape asked for: one writer retrying while its own lock is still held (expect ELOCKED), one retrying after the lock is gone (expect the write to succeed), with the cleanup no longer load-bearing for the second assertion.

Not carried here, because they are not #1912 work: the row at safeWriteJson.ts:189 belongs to the shared lock protocol in src/utils/fileLock.ts, which is not in this delta (git diff --name-only 73931fe8be25..9d420ad0c3 lists 13 paths and does not include it), and the Windows DACL row at safeWriteJson.ts:558 is filed deferred and unbooked against #2020, where JSON publishing moves onto the primitive that already has the dump-and-restore machinery (icacls 7 hits and dacl/DACL 31 in safeWriteText.ts, 0 and 0 in safeWriteJson.ts). (Lane u7-1918.)

Correction to comment 6738643978 in the paragraph about rows that are not #1912 work: the shared-lock-protocol row is at src/utils/safeWriteJson.ts:185, not :189. The other reference in that paragraph (safeWriteJson.ts:558, the Windows DACL row filed deferred and unbooked against #2020) is correct as written. (Lane u7-1918.)

One contract item to carry here, from #1914 measurement, recorded rather than opened elsewhere.

The question is what permissions a published file receives, on POSIX and on Windows, and three rows on two PRs describe three parts of it:

feat(tools): guarded write core under the shared lock (U5, #1375) #1914 row at safeWriteJson.ts:644 (thread root 4507403701). Measured: at this PR's merge base 5319fe9 the staged stream was created with no explicit mode (src/utils/safeWriteJson.ts line 247 there: fsSync.createWriteStream(targetPath, { encoding: "utf8" })). At the head 7209a5f it is created with mode 0o600 (:644) and _copyTargetMode returns without a mode when the target stat fails ENOENT (:671-682), so a newly created target publishes 0600. The permissions change is introduced by this delta.
feat(task): observation registry with read completeness (U3, #1375) #1912 row at safeWriteJson.ts:597 at that head: _copyTargetMode groups ENOTDIR with ENOENT, so a swapped-in non-directory is treated as absence.
feat(task): observation registry with read completeness (U3, #1375) #1912 row at safeWriteJson.ts:558 at that head: Windows ACL inheritance, where the dump-and-restore machinery lives in the sibling primitive - icacls appears 7 times and dacl/DACL 31 times in src/services/file-safety/safeWriteText.ts and 0 times and 0 times in src/utils/safeWriteJson.ts.

I am recording them as one item rather than settling permissions three times in three threads, and I opened no issue for it. The two heads are independent branches (measured on #1914 row 1 today: neither is an ancestor of the other, and all 26 of #1914's delta paths also differ at #1912's head), so whichever generation of the publish primitive lands first has to own the answer for both. (lane u7-1918.)

Navigation Menu

Repository navigation

Tracking fws U3: observation registry with read completeness #2018

Six-unit split plan for #1912 (issue #1375) with measured per-unit costs - posted on the tracker this PR names

Whole PR as measured

Measured cost per unit, with the gate verdict on each

Chain, merge order, fidelity

Verification run on each unit alone

Boundaries, and the disproved cuts pinned as refs

Gate-evidence correction I owe this issue

#1912 state measured while doing this

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Tracking fws U3: observation registry with read completeness #2018

Description

Activity

easonLiangWorldedtech commented on Nov 9, 2026

easonLiangWorldedtech commented on Nov 9, 2026

Six-unit split plan for #1912 (issue #1375) with measured per-unit costs - posted on the tracker this PR names

Whole PR as measured

Measured cost per unit, with the gate verdict on each

Chain, merge order, fidelity

Verification run on each unit alone

Boundaries, and the disproved cuts pinned as refs

Gate-evidence correction I owe this issue

#1912 state measured while doing this

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