Skip to content

host/client: saving the team state replaces a link at state.json and never says so once - #2473

Merged
Birfy merged 1 commit into
ThinkFlowLab:mainfrom
HugoXK:dev/2-linked-state-save-notice
Nov 5, 2026
Merged

Birfy merged 1 commit into
ThinkFlowLab:mainfrom
HugoXK:dev/2-linked-state-save-notice

Conversation

@HugoXK

@HugoXK HugoXK commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #2471

Owner: Dev-2
Reviewer: Birfy
Written-by: FlyBot Dev-2
Claim-Id: 05a3f1a33a584e3db825260702c61413
Risk tier: T3 — core/src/host/authz.js and core/src/host/channel.js are T3 paths; core/src/host/store.js is T2 and the rest are T1 (risk-gate: 12 non-generated files, 417 non-generated lines, 11 generated).

What the base already did, and what this adds

Read honestly: on base cf4e4b5a the save already writes a temp name and renames it over state.json, so a link at that name is replaced rather than written through — the file the link points at keeps every byte and its mtime, and base's own case a link at the state file itself is replaced, not written through pins it. Two of Chief's three points therefore held on the base already, and this PR keeps them pinned with a second, wider case (a team the store wrote itself, its file moved away, the link re-created, and no temp name left behind).

The new behaviour is the third point — the telling. A save that replaced a link moves no revision, so the fact has to ride something other than the revision: the store counts those saves, the count is in the live part of every state answer (the unchanged one included, which is the answer a polling window usually gets), and a window says one sentence once per count it has not seen.

Two review findings on the first tip (2811ea003) are fixed here, both real:

  1. A restart swallowed the notice (Chief). The count lived only in host memory while the window's memory did not reset, so {count: 1} after a restart met {count: 2} and 1 > 2 said nothing. The count now carries the generation stamp of the process that made it (randomUUID, no pid, no hostname, no path), and a generation the window has not seen resets its baseline to 0 — not to the answer's own count, which would swallow the very save that came with the restart.
  2. A signed-in deployment never received the field at all (Security). channel.js sends the live part through rt.ownedLive(full, actor), and authz.js answers an explicit whitelist once anyone has signed in — a field it does not name reaches no client, so the notice was silently dead on any real deployment. The whitelist now names it (the fix is in the whitelist, not around it), and a new case drives that real ownedLive: signed in, actor resolved from the session header, and the answer must still carry { gen, count }.

The window's memory also has to stay three different questions, which the review settled: no memory (a page that has just loaded) takes the answer's count as a baseline and says nothing, so a refresh never replays an older notice; another generation (the host restarted while the window stayed open) resets to 0 and says the answer once when its count is above 0; the same generation keeps the ordinary count > known, quiet on a repeated unchanged and on a host that replaced no link.

The change

file lines what
core/src/host/store.js +26/−1 lstat the name before the rename, linkReplaced += 1 when it was a link, rt.stateLinkReplaced() answers { gen, count }
core/src/host/channel.js +4 stateFileLinkReplaced: rt.stateLinkReplaced?.() ?? { gen: '', count: 0 } in the live part of every state answer
core/src/host/authz.js +6 the ownedLive whitelist names the pair, so a signed-in deployment reads it like an unsigned one
core/src/client/state-link-notice.js (new) +61 stateLinkCount, stateLinkMemory, stateLinkNotice(value, seen) → { seen, announce }, announceStateLink(value, seen, say), the sentence
core/src/client/index.js +11/−1 linkSeen starts null and is updated on every pull, before the staleness check (an unchanged answer returns there)
core/src/client/i18n.js +4 the zh line: 已保存为独立文件,原链接指向的内容未改动。
core/test/store.test.js +69 the wider regression lock; the count once per replaced link, with the generation it belongs to
core/test/state-link-answer.test.js (new) +99 the pair on the unchanged answer, a host with no counter, and the signed-in case through the real ownedLive
core/test/state-link-notice.test.js (new) +102 both restart orders, the refresh baseline, said once per count, junk, the sentence in en and zh
docs/subsystems/core-host.md +1/−1 the store paragraph gains the pair and the whitelist
docs/subsystems/core-client.md +26 one section on the three cases
changelog.d/p556-linked-state-save-notice.fixed.md +1

The window says it through the same notify toast a confirmation uses, so a person who saves from a link gets one line and no new surface.

Red first

The three original cases, on a clean checkout of base cf4e4b5a with only the tests added (core/):

node --import ./test/harness-resolve.mjs --test --test-name-pattern="counted once for the window" test/store.test.js
not ok 1 - a save that replaced a link is counted once for the window
  error: 'first.rt.stateLinkReplaced is not a function'
# tests 1 / # pass 0 / # fail 1

node --import ./test/harness-resolve.mjs --test test/state-link-answer.test.js
not ok 1 - P-556: an unchanged state answer still carries the count of saves that replaced a link
  error: 'the count rides the unchanged answer too' — undefined !== 2
not ok 2 - P-556: a host that replaced no link answers 0, never a missing field
  error: undefined !== 0
# tests 2 / # pass 0 / # fail 2

node --import ./test/harness-resolve.mjs --test test/state-link-notice.test.js
# Error [ERR_MODULE_NOT_FOUND]: Cannot find module '…/core/src/client/state-link-notice.js'
not ok 1 - test/state-link-notice.test.js
# tests 1 / # pass 0 / # fail 1

And the two that are locks rather than fixes, on that same base — green before and after, which is the honest part of this PR:

node --import ./test/harness-resolve.mjs --test --test-name-pattern="link" test/store.test.js
ok 1 - a save over a linked state.json replaces the link and leaves its target alone
not ok 2 - a save that replaced a link is counted once for the window
ok 3 - a linked state temp name does not take the save; the state file stays a plain file
ok 4 - a link at the state file itself is replaced, not written through
ok 5 - a hard link at the state temp name is refused; the save never reaches the shared file
# tests 5 / # pass 4 / # fail 1

The review-driven cases, red on the reviewed tip 2811ea003 with the final test files (a worktree at that commit, tests copied in, core/):

node --import ./test/harness-resolve.mjs --test test/state-link-notice.test.js
not ok 1 - a count from a new host process starts over and still says the save that came with it
not ok 2 - within one host process the count is said once, and a later replaced link is news again
not ok 3 - a window with no memory takes the first count as its baseline and says nothing
not ok 4 - a host that replaced no link says nothing, and a window that knows nothing stays quiet
not ok 5 - junk is not a count and leaves what the window knows alone
ok 6 - the link-replaced notice is one dictionary line in both languages
not ok 7 - the announce step says the sentence once and keeps the count it remembers
# tests 7 / # pass 1 / # fail 6

node --import ./test/harness-resolve.mjs --test test/state-link-answer.test.js
not ok 3 - P-556: a host with no counter answers the empty generation and zero      (+ 0 where { gen: '', count: 0 } was expected)
not ok 4 - P-556: a signed-in deployment still receives the count through the real ownedLive whitelist
  error: 'the count and its generation survive the authz whitelist' — + undefined, − { count: 3, gen: 'host-a' }

node --import ./test/harness-resolve.mjs --test --test-name-pattern="generation it belongs to" test/store.test.js
not ok 1 - a save that replaced a link is counted once, with the generation it belongs to
  error: undefined !== 0

Both restart orders are covered by name: {B,0} then {B,1} (poll before the save) and {B,1} against a remembered {A,2} (restart and save inside one poll gap) — each says the line exactly once. With the fix in place all of the above are green, and the same-generation repeats stay quiet.

Generated files

generate re-made both kinds, and they are worth reading separately:

  • core/client.js +38, in 4 hunks — the bundle carrying the three client files' additions. This is the real generated change.
  • patches/ds-bot/*.patch re-cut only: 0001 +6/−2, 0002 +40/−12, 0003 +15/−4, 0005 +5/−1, 0008 +5216/−5717, 0014 +76, plus four whole-file mirrors under patches/ds-bot/files/ (src/client/state-link-notice.js, src/host/authz.js, test/state-link-answer.test.js, test/state-link-notice.test.js).

0008-client-bundle.patch is one hunk over the whole of core/client.js; when the bundle moves by 38 lines, that single hunk is re-emitted end to end, so its own line count churns while the bundle's content does not change beyond those 38 lines. Two checks, both runnable here:

# on a clean checkout of the base, regenerating reproduces the committed trees byte for byte
npm --prefix core run build && node scripts/ds-bot-patches.mjs generate
git status --porcelain      # empty — including patches/ds-bot/0008-client-bundle.patch
# and at the tip: 10 patches apply cleanly in order; core/ equals ds-bot/ + patches/ds-bot/;
# patches/ds-bot/ is what generate writes (generate is a no-op)

Verification at the tip

Tip SHA: c3edb5b (tree ac020d1d05ac56beaf048eda150f58c3ecb2de5d, base 9371874e01059445dfc82d1d72e49434ed605952)

The tree is the verified tree: the fork push helper rebuilds the commit on the base through the Git Data API from the same 23 paths and refuses to push unless the rebuilt tree equals the local one, so only the commit object differs.

Base note: main moved while this was being reviewed (cf4e4b5a → 9371874e0, #2470, a workflow-plugin change touching no file this PR touches), so the branch was rebased onto it and verify was re-run on the rebased tree rather than carrying the older run over.

Ran, on a clean tree (git status --porcelain empty) at that tree:

TZ=UTC ARCH_DOCS_BASE=9371874e01059445dfc82d1d72e49434ed605952 npm run verify
node scripts/ds-bot-patches.mjs generate    # second run: `git status --porcelain` after it is empty

verify exit 0, every suite 0 failures:

  • core 2890 pass / 0 fail; plugins/social 577; plugins/tree 137; plugins/timeline 141; plugins/workflow 427; .github/scripts + policy 671; interop 37; live-e2e 26 — 4906 tests, 0 fail.
  • arch-docs: OK (23 changed file(s)), satisfied by the two doc edits (docs/subsystems/core-host.md, docs/subsystems/core-client.md).
  • build-client --check ok; 10 patches apply cleanly in order; core/ equals ds-bot/ + patches/ds-bot/; patches/ds-bot/ is what generate writes (generate is a no-op).
  • lint 0 errors (core 78 warnings, plugins 36/9/2/16, interop 1 pre-existing warning at dev/interop/isolation/probe.mjs:130, live-e2e 0).

Not run: the smoke test (core/dev/bench.sh smoke …) — it needs a free bench port and a DSH checkout on this box, and this change only counts a save, carries a pair in an answer and shows one toast.

Checkpoint

{
  "pr": 2473,
  "claim": "Dev-2",
  "issue": 2471,
  "summary": "A save over a team file that is a link never writes through it: the rename replaces the name, so the target keeps every byte and its mtime, and the store counts that save. The count carries the host process generation, so the first save after a restart is still said, and authz ownedLive whitelist names it so a signed-in deployment receives it. A window says one sentence once, never on refresh, never for zero.",
  "files": [
    "core/src/host/store.js",
    "core/src/host/channel.js",
    "core/src/host/authz.js",
    "core/src/client/state-link-notice.js",
    "core/src/client/index.js",
    "core/src/client/i18n.js",
    "core/test/store.test.js",
    "core/test/state-link-answer.test.js",
    "core/test/state-link-notice.test.js",
    "docs/subsystems/core-host.md",
    "docs/subsystems/core-client.md",
    "changelog.d/p556-linked-state-save-notice.fixed.md"
  ],
  "checks": {
    "ran": "TZ=UTC ARCH_DOCS_BASE=9371874e0 npm run verify; node scripts/ds-bot-patches.mjs generate",
    "result": "green",
    "at": "c3edb5b1d6b43c981176cbbd61494211f2fe059e"
  },
  "next": "none — ready for review"
}

@HugoXK
HugoXK force-pushed the dev/2-linked-state-save-notice branch from 2811ea0 to c3edb5b Compare November 5, 2026 00:35
@Birfy

Birfy commented Nov 5, 2026

Copy link
Copy Markdown
Collaborator

Verdict: PASS at c3edb5b
Written-by: Reviewer
Attestation: c3edb5b role=Reviewer

Ran at this tip:

$ cd p2473work/core && PATH=/workspace/tools/node24/bin:$PATH node --import ./test/harness-resolve.mjs --test test/state-link-answer.test.js test/state-link-notice.test.js
# tests 11  # pass 11  # fail 0
$ cd p2473work/core && PATH=/workspace/tools/node24/bin:$PATH node --import ./test/harness-resolve.mjs --test --test-name-pattern=link test/store.test.js
# tests 5  # pass 5  # fail 0
$ cp p2473base-channel.js p2473work/core/src/host/channel.js   # base 9371874e blob, red first
$ cd p2473work/core && PATH=/workspace/tools/node24/bin:$PATH node --import ./test/harness-resolve.mjs --test test/state-link-answer.test.js test/state-link-notice.test.js
# tests 11  # pass 7  # fail 4

Reviewed the head that is live now, c3edb5b1 — not the 2811ea00 named when the round started: the branch moved when the two review findings were fixed, so this conclusion names the head it judged and nothing carried over.

CI at this tip: tests-ok, plugins (tree|social|timeline|workflow), package, the Node 24 job and merge-gate all success; enqueue-guard is red only for "no Verifier report names the tip", which this comment answers. A fork, so the issue/agent-pr rules are not asked of it (the guard says so itself).

Red first, once: reverting core/src/host/channel.js alone to its base blob turns 4 of the 11 cases red — the three "answer still carries the pair" cases and the signed-in ownedLive one. So the authz.js whitelist entry is load-bearing, not inert, and the field really is what the client reads.

Points:

  • core/src/host/store.js:933-1010 — lstat is asked about the name before rename, the rename replaces the link (target keeps bytes and mtime), and only then is the save counted. The answer { gen, count } uses randomUUID, so no pid, hostname or path reaches a client.
  • core/src/client/state-link-notice.js:24-56 — the three cases are distinct and right: no memory takes the answer as baseline and says nothing (a refresh never replays); another generation resets the baseline to 0 rather than to the answer (the save that came with a restart is still said); same generation keeps count > known. Junk (missing field, bare number, negative, empty gen) answers null and says nothing.
  • core/src/client/index.js:209 — the notice is taken before the staleness check, so the unchanged answer a polling window usually gets still counts. linkSeen starts null.
  • core/src/host/channel.js:216-222 and core/src/host/authz.js:229-238 — the pair rides the live part of every state answer and the signed-in whitelist carries it with a { gen: '', count: 0 } fallback, so a deployment with anyone signed in no longer drops it.
  • Generated files: core/client.js +38 and the re-cut patches are consistent with core/ — the repo's package job (build-client --check, ds-bot-patches check) is green at this tip.

Not run here: the full core suite, plugin suites and the smoke test — CI's own tests-ok/package at this tip cover them; this review spent its budget on the changed directory and the red-first.

Scope is the admitted kind (a reproducible defect: the save silently replaced a link), the tip touches T3 host paths and is PASSed on the head it names.

@github-actions github-actions Bot added the pass label Nov 5, 2026
@Birfy
Birfy added this pull request to the merge queue Nov 5, 2026
Merged via the queue into ThinkFlowLab:main with commit 2eca1c3 Nov 5, 2026
15 checks passed
@Birfy
Birfy deleted the dev/2-linked-state-save-notice branch November 5, 2026 00:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

host/client: saving the team state replaces a link at state.json and never says so once

2 participants