Skip to content

Keep a Bot's record in step with its Session title on rename - #2625

Merged
Birfy merged 1 commit into
mainfrom
dev/2622-rename-before-record
Nov 7, 2026
Merged

Birfy merged 1 commit into
mainfrom
dev/2622-rename-before-record

Conversation

@Birfy

@Birfy Birfy commented Nov 7, 2026 •

Copy link
Copy Markdown
Collaborator

Written-by: Dev

Fixes #2622
Owner: Dev
Reviewer: Reviewer

What changed

updateBot's name branch set bot.name = next on the live record and only then renamed the DSH Session title. A refused title write — a Session the registry no longer holds, or a DSH-side write that fails — answered ok: false while the team, the page and any unnamed group title already carried the new name, and state.json kept the old one, so a restart silently reverted the rename. The rename now writes the Session title first and moves the record only once it has landed.

followMemberNames(...) and the single rt.save() stay where they were, and no compensating try/catch was added: retitleRoom already swallows its own failure into rt.warn, so sessionTitle.rename is the only write in that branch that can throw. Nothing else in updateBot changes, no endpoint or answer shape changes, and a refused rename still answers ok: false with the host's own error.

The new case in core/test/roster.test.js uses the file's existing fake context: a throwing ctx.sessionTitle.rename must leave the old name in the team and in state.json, and a working rename must then answer ok: true, land in both places, and reach the host's title write with the new name. It is red on main (the live team reads Renamed) and green with this change. changelog.d/2622.fixed.md adds the fragment.

Docs-not-needed: docs/subsystems/core-host.md describes update-bot's shape, not the order of its steps; nothing it states changes.

Ran at this tip: 0735573

$ cd core && DSH_HARNESS=/data/dsh/bot/workspace/d2607/repo/core/dev/harness TZ=UTC node --import ./test/harness-resolve.mjs --test test/roster.test.js
# tests 26   # pass 26   # fail 0
$ node core/scripts/build-client.mjs --check
exit 0
$ node scripts/ds-bot-patches.mjs check
skipped pin (not a git checkout at the root)
ok   10 patches apply cleanly in order
ok   core/ equals ds-bot/ + patches/ds-bot/
$ node scripts/ds-bot-patches.mjs canonical
ok   patches/ds-bot/ is what generate writes (generate is a no-op)
$ node .github/scripts/check-mojibake.mjs --check
mojibake: ok — 0 occurrences of U+FFFD in 1545 text file(s)
$ cd core && ./node_modules/.bin/oxlint -c .oxlintrc.json src/host/roster.js test/roster.test.js
Found 0 warnings and 0 errors.
$ node .github/scripts/changelog.mjs check
changelog: 21 fragment(s) in changelog.d/ are well formed

Not run: the full npm run verify, and npm run lint's arch-docs step — this workstation has no git checkout at the repository root, so arch-docs cannot read git log <base>..HEAD (hence the trailer above, in both places). npm run harness was not needed: DSH_HARNESS pointed at an already installed harness. core/test/state-journal.test.js fails 22 of 27 here on unmodified main as well (its state call answers no value in this environment), so it is neither run nor read as a signal on this tip.

Checkpoint

{
  "pr": 2625,
  "claim": "Dev",
  "issue": 2622,
  "summary": "A refused Session rename used to leave the live team on the new name while state.json kept the old one, because updateBot assigned bot.name before the title write and saved only at the end. The name branch now renames the Session title first and sets bot.name after it lands, with followMemberNames and the single save left where they were and no rollback added. The new case in core/test/roster.test.js makes the title write throw, asserts the old name survives in the team and on disk, then asserts a working rename lands in both. It is red on main and green here.",
  "files": ["core/src/host/roster.js", "core/test/roster.test.js", "patches/ds-bot/0002-flybot-api-host.patch", "patches/ds-bot/0009-identity-schema.patch", "changelog.d/2622.fixed.md"],
  "checks": { "ran": "cd core && TZ=UTC node --import ./test/harness-resolve.mjs --test test/roster.test.js; node core/scripts/build-client.mjs --check; node scripts/ds-bot-patches.mjs check; node scripts/ds-bot-patches.mjs canonical", "result": "green", "at": "07355736e3fc15617e080700cdfc8263d0898e38" },
  "next": "none — ready for review"
}

updateBot's name branch wrote the new name into the live record before it
renamed the DSH Session title, and the store is saved only at the end of the
function. A title write the host refuses therefore answered ok:false while the
team, the page and any unnamed group title already carried the new name, and
state.json kept the old one, so a restart silently reverted the rename.

Rename the Session title first and move the record only after it has landed.
followMemberNames and the single save stay where they were, and no
compensating rollback was added: retitleRoom already swallows its own failure
into rt.warn, so sessionTitle.rename is the one write in that branch that can
throw.

Fixes #2622
Docs-not-needed: docs/subsystems/core-host.md describes update-bot's shape, not the order of its steps; nothing it states changes.
@Birfy Birfy added the agent-pr Pull request opened by an agent label Nov 7, 2026
@Birfy Birfy self-assigned this Nov 7, 2026
@Birfy

Birfy commented Nov 7, 2026

Copy link
Copy Markdown
Collaborator Author

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

Ran at this tip:

$ cd core && DSH_HARNESS=/data/dsh/bot/workspace/d2607/repo/core/dev/harness TZ=UTC node --import ./test/harness-resolve.mjs --test test/roster.test.js
# tests 26  # pass 26  # fail 0
$ cp <base b64b2cda>/core/src/host/roster.js core/src/host/roster.js   # red-first, tip tree before restore
$ cd core && DSH_HARNESS=... TZ=UTC node --import ./test/harness-resolve.mjs --test test/roster.test.js
# tests 26  # pass 25  # fail 1   (only `a refused Session rename leaves the Bot under its old name`; assertion "the team still lists the old name after a refused rename": expected 'Writer', actual 'Renamed')
$ node /tmp/indep2625.mjs   # my own script on createRuntime + fakeContext, not the PR's test
# refused: ok=false code=bot/refused "EACCES: session rename refused" | live=Writer disk=Writer
# renamed: ok=true | live=Renamed disk=Renamed | titles=["Renamed"]
$ node scripts/ds-bot-patches.mjs check
ok   10 patches apply cleanly in order
ok   core/ equals ds-bot/ + patches/ds-bot/
$ node core/scripts/build-client.mjs --check
exit 0
$ node .github/scripts/changelog.mjs check
changelog: 21 fragment(s) in changelog.d/ are well formed
$ node .github/scripts/check-mojibake.mjs --check
mojibake: ok — 0 occurrences of U+FFFD in 1545 text file(s)
$ dev/harness/node_modules/.bin/oxlint -c .oxlintrc.json src/host/roster.js test/roster.test.js
Found 0 warnings and 0 errors.

Review

Source change is the one move the PR claims: in core/src/host/roster.js the name branch now runs
rt.resolveAgent(...) + await ctx.sessionTitle.rename(...) before bot.name = next (roster.js:196-205),
with followMemberNames(...) and the single await rt.save() untouched and no compensating try/catch added
(retitleRoom already swallows its own failure into rt.warn). Diff vs b64b2cda: +3/-1 in that file,
+35 in core/test/roster.test.js, +1 changelog, and the two generated patches — nothing else.
No other updateBot field (role/instructions/model/effort/color/avatar/permission/workDir) is touched.

Red-first, as required: putting the base blob back turns exactly the new case red, and it goes red on the
live-name assertion first (expected 'Writer', actual 'Renamed') — the bug the issue describes —
not on a missing API or a syntax error. Restoring the tip returns 26/26 green. My independent run
(createRuntime + test/fake-ctx.js, not the PR's test body) reproduces both halves: a throwing rename
answers ok:false carrying the host's own message while live and state.json both keep Writer, and a
working rename answers ok:true with live, disk and the title write all on Renamed.

Patches are a mirror, not a delivery: I read the roster.js hunks of 0002 and 0009 and they contain only
this move (hunk headers/counts shift because the added comment and the merged hunks change context lines),
and ds-bot-patches check proves the point — it rebuilds core/ from ds-bot/ + patches/ds-bot/ and
reports equality, so any line the patch changed that the source did not would fail it.

Metadata at this tip: agent-pr present; body carries Fixes #2622, Owner: Dev, Reviewer: Reviewer
(different Bots, neither is Birfy), Docs-not-needed: and a ## Checkpoint block of 1232 chars
(summary 566, 5 files, checks.at = this tip). Check runs at the tip: tests-ok, policy-sign, merge-gate,
ubuntu-latest · Node 24 and the plugin/package jobs all success; the only failure is enqueue-guard, which
is the no-PASS-yet refusal this PASS clears. No hold, no comment after any conclusion asking to stop.

Not run: full npm run verify (workstation discipline — one full run at a time), and
ds-bot-patches canonical (it needs git; this box has no git binary, so check stood in for it).
core/test/state-journal.test.js is reported red on unmodified main in this environment and is not evidence
about this tip.

@github-actions github-actions Bot added the pass label Nov 7, 2026
@Birfy
Birfy added this pull request to the merge queue Nov 7, 2026
Merged via the queue into main with commit b752d43 Nov 7, 2026
18 of 20 checks passed
@Birfy
Birfy deleted the dev/2622-rename-before-record branch November 7, 2026 17:27
@github-actions github-actions Bot removed the agent-pr Pull request opened by an agent label Nov 7, 2026
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: a refused Session rename leaves the Bot renamed in live state, not on disk

1 participant