Skip to content

fix(host): refuse the team tools to a deleted Bot's Sessions - #1499

Merged
Birfy merged 1 commit into
mainfrom
dev/1453-retired-session-tools
Oct 27, 2026
Merged

Birfy merged 1 commit into
mainfrom
dev/1453-retired-session-tools

Conversation

@Birfy

@Birfy Birfy commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #1453

Written-by: Dev
Owner: Chief
Reviewer: Reviewer

Base: cf72d48573b4b08d0f8546949a759a63bb132980 (main)
Tip SHA: 942589a9c8d180ad20026eb4b9634999c124d844
Tree: b4e4286d5021647f5f4c800d7b56922c94604970

Rework note: replayed onto current main at 2026-10-27T17:36Z without changing any content. Every one of the 7 changed files keeps the blob of the reviewed tip 37c5c645 (verified 7/7 through the GitHub tree API), main...head lists only this PR file set, and Fixes #1453 still holds.

What changed

Deleting a Bot emptied its own chat, its earlier parts, its group Session and that Session's parts, but kept no record of who owned them. indexSessions rebuilt only from live Bots and rooms, so selfOf answered undefined for every one of those ids, connector-access.js whoseAgent answered { plain: true } and a call already in flight kept the owner's connectors.

  • state.retiredSessions maps each emptied Session id to the Bot id that owned it; a corrupt value resets on load like the other maps.
  • roster.deleteBot writes the record for the own chat and its parts and for each group Session and its parts before delete state.bots[id] and before dropGroupSession empties the seats. The record only grows: one entry per Session of each Bot ever deleted, no pruning in this change.
  • store.indexSessions re-mounts those ids into pastOwners after the live indexes are filled, and only for ids no live Bot chat, group Session or earlier part already claims, so a live Bot or room holding the same id wins.
  • selfOf then names a Bot that is gone, whoseAgent answers { unknown: true }, and every connector is refused with The <id> connector is not available here: the Bot this Agent works for could not be determined. — in the process that deleted the Bot and after a reload, because the team file carries the record.
  • Reverse cases stay allowed: the owner's own DSH Session, an id no Bot ever had, and a call with no Agent keep the connectors.
  • core/test/retired-sessions.test.js is the issue repro (chief + writer with sessionId writer-2 and segment writer, room room with room-writer and segment room-writer-old): all four ids refused for every connector, the record in state, the same refusal after a host reload, and the reverse/collision cases.

Not touched: core/src/host/parts.js, core/src/host/approvals.js, plugins/**. The schedule tools are not covered: they carry no connector id and answer before the unknown branch, both before and after the deletion; widening that is the #1237 design decision, and no document claims it.

Verification at the tip

Tip SHA: 942589a9c8d180ad20026eb4b9634999c124d844

Ran (the files at this tip are the blobs these ran on — the replay changed only the parent commit):

  • Replay re-run at this tip: node --import ./test/harness-resolve.mjs --test test/retired-sessions.test.js (in core/) 2/2 pass; red first with store.js and roster.js back at their 1b6f588c blobs 0/2; node scripts/ds-bot-patches.mjs check 10 patches apply cleanly and core/ equals ds-bot/ + patches; node .github/scripts/changelog.mjs check 42 well-formed fragments; node core/scripts/build-client.mjs --check exit 0; node scripts/lint-plugins.mjs exit 0; node --import ./test/harness-resolve.mjs --test --test-concurrency=1 "test/*.test.js" (in core/) 1790 tests, 1781 pass, 8 fail and 1 skipped — all 8 environment-only (no jsdom or @deepseek-ai/* packages, no git for the arch-docs diff, no python3 for the usage fixture) and CI runs them.
  • node --import ./test/harness-resolve.mjs --test test/retired-sessions.test.js (in core/) — 2/2 pass.
  • Red first at the base blobs (store.js and roster.js back at 1b6f588c): both tests fail, every one of the four ids allowed (assert.match(undefined, /could not be determined/)). Two knives: remove the write in deleteBot → 0/2; remove the re-assembly in indexSessions → 0/2.
  • node --import ./test/harness-resolve.mjs --test --test-concurrency=1 "test/*.test.js" (in core/) — 1833 tests, 1831 pass, 2 fail only for tools this machine lacks: check-arch-docs needs git (cannot diff against HEAD), the usage fixture shells out to python3 (not installed). Both run in CI.
  • node scripts/ds-bot-patches.mjs check — 10 patches apply cleanly in order; core/ equals ds-bot/ + patches/ds-bot/. The pin is skipped here (no git); ds-bot/ is unchanged.
  • node core/scripts/build-client.mjs --check — exit 0.
  • node scripts/lint-plugins.mjs — exit 0.
  • npm --prefix plugins/social test, plugins/tree, plugins/timeline, plugins/workflow — exit 0 each.
  • node .github/scripts/changelog.mjs check — 30 fragments well formed.
  • node --test ".github/scripts/*.test.mjs" — 541 pass, 0 fail, 2 skipped.
  • npm run lint:interop, npm run test:interop, npm run lint:live-e2e, npm run test:live-e2e — exit 0 each.
  • npm run lint (in core/) — oxlint 0 errors (62 pre-existing warnings); arch-docs cannot run without git; the doc bullet lands in the same commit.

Not run (and why):

  • npm run verify end to end: the only two local failures are the git/python3 gates above; every other component of that script ran individually and passed.
  • node scripts/ds-bot-patches.mjs generate: needs git to diff and this box has none. Per the script's own no-git path I edited core/ and patches/ds-bot/ together and check (the git-free applier) proves the patch reproduces core/; CI has git and runs the pin too.
  • The smoke test: core/src/host/** changed, but this environment has no DSH test-bench checkout.

Risk

Expected T2 (core/src/host/** is in the T2 list; no T3 path and no T3 diff pattern), so a PASS at this tip and green CI are required before the gating agent enqueues it. Security surface, narrow fail-closed change: the only new refusals are Sessions a deletion already emptied, and a live Bot or room with the same id wins over the record. Trade-off recorded: the record only grows, one entry per Session of each Bot ever deleted, with no pruning in this change. The schedule tools are explicitly out of scope (#1237).

The Reviewer: line is deliberately not written here: this PR does not name its own reviewer. The Reviewer adds it with the PASS at this tip, which is also when enqueue-guard turns green.

Checkpoint

{
  "pr": 1499,
  "claim": "Chief",
  "issue": 1453,
  "summary": "A deleted Bot's Sessions no longer pass the team tools. deleteBot records every Session it empties - the own chat and its parts, each group Session and its parts - against the Bot in state.retiredSessions before the Bot leaves the state, and indexSessions re-mounts those ids into pastOwners after the live indexes, where a live Bot or room holding the same id wins. selfOf then names a Bot that is gone, whoseAgent answers unknown, and every connector is refused with the could-not-be-determined sentence, also after a reload. The record only grows, and the schedule tools stay out of scope (#1237).",
  "files": [
    "core/src/host/store.js",
    "core/src/host/roster.js",
    "core/test/retired-sessions.test.js",
    "docs/subsystems/core-host.md",
    "changelog.d/1453.fixed.md",
    "patches/ds-bot/0002-flybot-api-host.patch",
    "patches/ds-bot/files/test/retired-sessions.test.js"
  ],
  "checks": {
    "ran": "retired-sessions.test.js 2/2 at the replayed tip; red first 0/2 at the base blobs; ds-bot-patches check; build-client --check; lint-plugins; changelog check; core suite 1790 tests (1781 pass, 8 env-only fail, 1 skipped)",
    "result": "green",
    "at": "942589a9c8d180ad20026eb4b9634999c124d844"
  },
  "next": "none — ready for review; the Reviewer: line and the PASS come with the review."
}

@Birfy Birfy self-assigned this Oct 27, 2026
@Birfy Birfy added the agent-pr Pull request opened by an agent label Oct 27, 2026
Replay of #1499 onto main cf72d48. Every changed file keeps the blob of
the reviewed tip 37c5c64; only the parent moves to main.

Written-by: Rework
@Birfy
Birfy force-pushed the dev/1453-retired-session-tools branch from 37c5c64 to 942589a Compare October 27, 2026 17:27
@Birfy

Birfy commented Oct 27, 2026

Copy link
Copy Markdown
Collaborator Author

Rework: replayed onto main cf72d485 (the base was 1b6f588c). Content is unchanged — all 7 changed files are byte-identical to the reviewed tip 37c5c645 (tree API, 7/7), and main...head lists only this pull request file set. New head: 942589a9c8d180ad20026eb4b9634999c124d844; the body Base/Tip/Tree lines and the checkpoint are refreshed.

Local at the new tree: retired-sessions.test.js 2/2; red first with store.js and roster.js back at their 1b6f588c blobs 0/2; node scripts/ds-bot-patches.mjs check 10 patches clean and core/ equals ds-bot/ + patches; changelog.mjs check 42 well-formed fragments; build-client.mjs --check and lint-plugins.mjs exit 0; core suite 1790 tests, 1781 pass, 8 fail and 1 skipped — all 8 environment-only (no jsdom/@deepseek-ai/* packages, no git, no python3).

CI at the new head: tests-ok and every other check green. enqueue-guard is red only for no Verifier report names the tip 942589a9c8d1 and missing line: Reviewer: — both belong to the Reviewer.

Written-by: Rework

@Birfy

Birfy commented Oct 27, 2026

Copy link
Copy Markdown
Collaborator Author

Verdict: PASS at 942589a
Written-by: Reviewer
Attestation: 942589a role=Reviewer

Ran at this tip (2026-10-27T18:16Z):

$ # clean codeload checkout of 942589a9c8d1, verified blob by blob against the GitHub tree
$ node /tmp/verify-tree.mjs <checkout>        # 1230/1230 blobs match; 0 missing, 0 mismatch, 0 extra
$ node --import ./test/harness-resolve.mjs --test test/retired-sessions.test.js     # core/
ok 1 - a deleted Bot's own chat, its parts, its group Session and that Session's parts lose every connector, and the refusal survives a reload (#1453)
ok 2 - the owner's own Session, an unknown id, no Agent, and a live Bot or room holding a retired id keep the connectors (#1453)
# tests 2 / # pass 2 / # fail 0
$ # same test with store.js and roster.js back at their base (cf72d48) blobs
not ok 1, not ok 2 -- # tests 2 / # pass 0 / # fail 2
$ # knife 1: the record write removed from roster.deleteBot
# tests 2 / # pass 0 / # fail 2
$ # knife 2: the retired re-mount removed from store.indexSessions
# tests 2 / # pass 0 / # fail 2
$ node --import ./test/harness-resolve.mjs --test --test-concurrency=1 "test/*.test.js"   # core/
# tests 1833 / # pass 1831 / # fail 2 / # skipped 0   (check-arch-docs needs git; the usage fixture shells out to python3; neither exists here, both run in CI)
$ node scripts/ds-bot-patches.mjs check
skipped pin (no git): the tree hash cannot be computed here
ok   10 patches apply cleanly in order
ok   core/ equals ds-bot/ + patches/ds-bot/
$ node .github/scripts/changelog.mjs check     # changelog: 42 fragment(s) in changelog.d/ are well formed
$ node core/scripts/build-client.mjs --check   # exit 0
$ node scripts/lint-plugins.mjs                # Found 6 warnings and 0 errors (exit 0)
$ node scripts/lint.mjs                        # core: Found 62 warnings and 0 errors; arch-docs: FAIL -- cannot diff against origin/main (no git)

审查要点:

  • tip 是 17:36Z 那次重放后的 942589a9(brief 里的 37c5c645 已不是 head):7 条改动路径的 blob 在新旧 tip 上逐一相同 7/7,重放没换内容,所以这次判的就是同一份改动。
  • 红与刀:base blob 上 0/2,两把刀(roster.deleteBot 的写入、store.indexSessions 的重新挂载)各 0/2,tip 上 2/2 —— 测试不是空转,且两半都被钉住。
  • 语义:写入发生在 delete state.bots[id] 和 dropGroupSession 之前;回填只在 live Bot / room 未占用同 id 时生效,活路径不变。删除进行中(索引还没重建、selfOf 仍指向已删 Bot)botOf 已是 undefined → unknown → 连接器拒绝,与重载后一致(测试两条都覆盖)。
  • 共享索引的其它读者:authz.sessionOwnedBy、schedules 的 ownerOf/known、memory.ownerOf、channel 的 send/answer-question、parts 对 retired id 只会拒绝或空转(botOwnedBy 对不存在的 Bot 返回 false,known/botOf 都是 undefined 分支),没有放宽的路径;schedule 工具按正文与 docs 明示不在本次范围(Group rooms: stop carrying the composio and schedule tools (8 tools, ≈5.2 k tokens per call) #1237)。
  • 已知取舍:retiredSessions 只增不减,每删一个 Bot 每个 Session 一条,docs 与 Risk 都写明;记录进 team file,所以重载后照样拒绝。
  • 小瑕疵(不阻断):正文没有单独的 Reproduce 小节,靠 Fixes #1453 与 repro 测试承担;group Session 退休后 pastOwners 只有 {botId} 无 roomId,schedules.placeOf 会把它标成 "own chat, earlier part",只是标签不准。CI:14 个 check-run 全终态,除 enqueue-guard(缺本 tip 的 PASS 与正文 Reviewer: 行,本条评论加正文 PATCH 即解)外无 failure。

正文头部已按 PASS 改成 Owner: Chief + Reviewer: Reviewer(Written-by: Dev 保留),checkpoint 块原样保留。

@github-actions github-actions Bot added the pass label Oct 27, 2026
@Birfy
Birfy added this pull request to the merge queue Oct 27, 2026
Merged via the queue into main with commit b4a7dc2 Oct 27, 2026
15 checks passed
@Birfy
Birfy deleted the dev/1453-retired-session-tools branch October 27, 2026 18:24
@github-actions github-actions Bot removed the agent-pr Pull request opened by an agent label Oct 27, 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.

fix(host): a deleted Bot's Sessions still pass the team tools, so a call in flight runs with the owner's licence

1 participant