[AI-assisted] Add session history family lookup#90239
Conversation
|
Codex review: found issues before merge. Reviewed July 10, 2026, 7:37 AM ET / 11:37 UTC. Summary PR surface: Source +617, Tests +764, Docs -1, Other +85. Total +1465 across 37 files. Reproducibility: yes. at source level: current history is active-transcript oriented, and the supplied real Gateway and embedded runs demonstrate the opt-in family path, bounds, ordering, collision rejection, and current-tail preservation. Review metrics: 1 noteworthy metric.
Stored data model Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review findings
Review detailsBest possible solution: Rebuild the feature as a narrow current-main patch with one shared family-target resolver, only the required contract, docs, generated outputs, and tests, then obtain explicit owner approval for the family-scoped API. Do we have a high-confidence way to reproduce the issue? Yes at source level: current history is active-transcript oriented, and the supplied real Gateway and embedded runs demonstrate the opt-in family path, bounds, ordering, collision rejection, and current-tail preservation. Is this the best way to solve the issue? No in the current branch shape; the family-scoped design is reasonable, but unrelated changes must be removed and maintainers must choose this contract over the broader archived-history alternative. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against c8655be91514. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +617, Tests +764, Docs -1, Other +85. Total +1465 across 37 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (31 earlier review cycles; latest 8 shown)
|
34ea846 to
22d3a25
Compare
60588c9 to
f2cf517
Compare
…amily # Conflicts: # test/fixtures/agents/prompt-snapshots/codex-runtime-happy-path/discord-group-codex-message-tool.md # test/fixtures/agents/prompt-snapshots/codex-runtime-happy-path/telegram-direct-codex-message-tool.md # test/fixtures/agents/prompt-snapshots/codex-runtime-happy-path/telegram-heartbeat-codex-tool.md
…amily # Conflicts: # src/skills/workshop/curator.ts
…amily # Conflicts: # test/fixtures/agents/prompt-snapshots/codex-runtime-happy-path/discord-group-codex-message-tool.md # test/fixtures/agents/prompt-snapshots/codex-runtime-happy-path/telegram-direct-codex-message-tool.md # test/fixtures/agents/prompt-snapshots/codex-runtime-happy-path/telegram-heartbeat-codex-tool.md
…amily # Conflicts: # test/scripts/native-app-i18n.test.ts
…amily # Conflicts: # test/fixtures/agents/prompt-snapshots/codex-runtime-happy-path/discord-group-codex-message-tool.md # test/fixtures/agents/prompt-snapshots/codex-runtime-happy-path/telegram-direct-codex-message-tool.md # test/fixtures/agents/prompt-snapshots/codex-runtime-happy-path/telegram-heartbeat-codex-tool.md
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
…amily # Conflicts: # scripts/protocol-gen-swift.ts
…amily # Conflicts: # apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift # scripts/protocol-gen-swift.ts
Problem
When a threaded conversation has already been split across reset transcripts,
chat.historyand thesessions_historytool only read the currently active transcript. The session store can already retain the family throughusageFamilySessionIds, but history readers do not use that chain and do not discover archived*.jsonl.reset.<timestamp>topic transcripts.That means an agent/tool can be looking at the correct logical Telegram/Slack/Discord thread and still miss earlier turns after session rollover.
Fix
Add an opt-in family history path:
chat.historyacceptsincludeFamily: true.sessions_historyacceptsincludeFamily.chat.historynow share the bounded/header-checked reset archive resolver instead of using separate prefix scans.<sessionId>-topic-456.jsonl.reset.<timestamp>only when the archive header matches the requested session id.chat.historybehavior remains opt-in: UI startup does not load ancestor history.Validation
Current head:
1aa4e9cbb94f6841803bd55559aa0ea6de17dc67.CI=true node scripts/run-vitest.mjs run src/gateway/server.chat.gateway-server-chat-b.test.ts src/agents/tools/embedded-gateway-stub.test.tsCI=true node scripts/run-vitest.mjs run src/agents/openclaw-tools.sessions.test.tsCI=true node --import tsx scripts/generate-prompt-snapshots.ts --checkPrompt snapshots are current (7 files).CI=true node scripts/run-tsgo.mjs -p test/tsconfig/tsconfig.core.test.json --incremental --tsBuildInfoFile .artifacts/tsgo-cache/core-test.tsbuildinfoCI=true node scripts/run-tsgo.mjs -p test/tsconfig/tsconfig.extensions.test.json --incremental --tsBuildInfoFile .artifacts/tsgo-cache/extensions-test.tsbuildinfogit diff --checkReal Behavior Proof
usageFamilySessionIdsshould be able to fetch earlier reset transcript turns when a caller explicitly requests family history, including owned topic-suffixed reset archives, without reading colliding or headerless topic archives. The current active transcript must also be preserved when a retained reset family has more entries than the bounded 32 read targets.a12e1bbcf0, using temporary on-disk session directories, the real Gatewaychat.historyRPC integration harness, embedded-modechat.history, and the same reset archive filename/header patterns that OpenClaw creates for topic transcripts.usageFamilySessionIdslinking an ancestor session and the current session.type: "session"headers.chat.historyRPC path with and withoutincludeFamily: true.usageFamilySessionIds.chat.history(includeFamily)through the sessions tool stub, including the same >32-family-target cap case.chat.historyexcludes ancestor reset archive content, whilechat.history(includeFamily)includes the owned plain ancestor archive, includes the owned topic-suffixed ancestor archive, includes current-session content, and rejects the colliding/headerless topic reset archives. In the >32-target Gateway/RPC regression,chat.history(includeFamily)containscurrent survives family target cap, contains early ancestor content, and does not read the ancestor entries beyond the target cap. Embeddedchat.history(includeFamily)now reads the current active transcript first, then current archives, then ancestor archives/active transcripts; its >32-target regression also keeps the current transcript and drops over-cap ancestors.Current head addendum:
8c164eeaa3ClawSweeper's
a12e1bbcf0review finding is addressed in this head:limittrimming keeps the active session tail.chat.history(includeFamily)mirrors the Gateway order.includeFamilywith a small final limit keeps current active history.Standalone redacted embedded
chat.historysmoke from current head8c164eeaa3545a9f05dc507908f9dec0236d2c47:Validation on
8c164eeaa3545a9f05dc507908f9dec0236d2c47:Current head addendum:
0d37b42702Maintainer readiness note addressed on current head
0d37b42702688ac9c4bf0edf949f99b00f096cf6:resetAncestorssessions_history alias so the model-facing selector is onlyincludeFamily.chat.historyandsessions_historyaligned on the singleincludeFamilyflag.6050d0bbfb.0d37b42702; at refresh time there were no failed checks and the remaining required/security checks were still running.Validation on
0d37b42702688ac9c4bf0edf949f99b00f096cf6:Current head addendum:
1aa4e9cbb9ClawSweeper's generated-snapshot blocker is addressed on current head
1aa4e9cbb94f6841803bd55559aa0ea6de17dc67:resetAncestors.resetAncestorsno longer appears in source, docs, protocol/tool snapshots, or prompt snapshot counters.includeFamilyas the only model-facing family-history selector.Validation on
1aa4e9cbb94f6841803bd55559aa0ea6de17dc67:Current head addendum:
64281c2720ClawSweeper's latest exact-head proof and offset-semantics request is addressed on
64281c2720dce8a9dbe35bf2db7c37f5c64f8f6e.The supported contract is now explicit:
chat.history/sessions_historyreads active historyincludeFamily: truereads the newest bounded reset-family tailoffsetreads active-history pagesincludeFamily: truewithoffsetis rejected at protocol/tool/Gateway/embedded validation instead of silently returning active-only pagesExact-head protocol proof:
Exact-head validation:
Current head addendum:
578af36109Follow-up head
578af36109ebdd666d0159fc5287cdfda69d9ea1keeps the explicitincludeFamily + offsetrejection while preserving generated protocol compatibility.Important correction to the previous addendum: protocol schema generation currently needs
ChatHistoryParamsto remain a simple generated model, so the public schema still describes the fields. The unsupported combination is rejected consistently at the executable request boundaries instead of silently returning active-only pages:sessions_historytool rejects before callingchat.historychat.historyrejects before reading session fileschat.historyrejects withincludeFamily cannot be combined with offsetExact-head validation:
The broader exact-head validation from
64281c2720remains applicable for lint, test-types, and prompt snapshots; this head only restores generated protocol compatibility while keeping runtime rejection behavior.Current head addendum: `99106083df`
ClawSweeper's latest review items are addressed on current head `99106083dfc91bd188d2b4abd2d5aced2c7981ae`:
Exact-head validation:
```text
$ git rev-parse HEAD
9910608
$ CI=true node scripts/run-vitest.mjs run packages/gateway-protocol/src/index.test.ts
Test Files 1 passed (1)
Tests 45 passed (45)
$ CI=true node scripts/run-vitest.mjs run src/agents/tools/sessions-history-tool.test.ts src/agents/openclaw-tools.sessions.test.ts -t 'includeFamily|offset'
Test Files 4 passed (4)
Tests 12 passed | 84 skipped (96)
$ CI=true node scripts/run-vitest.mjs run src/agents/tools/embedded-gateway-stub.test.ts src/gateway/server.chat.gateway-server-chat-b.test.ts -t 'family|includeFamily|subagent_announce|sessionStartedAt|offset|cap'
Test Files 3 passed (3)
Tests 30 passed | 58 skipped (88)
$ pnpm protocol:check
passed; generated protocol schema and Swift models are current
$ pnpm lint --threads=8
passed
$ CI=true pnpm check:test-types
passed
$ git diff --check
no output
```
Current head addendum:
e9b75be554Current head
e9b75be554c409dbf49769417bd6ca5b3b2b7b30keeps the runtime family-history behavior from99106083dfand only adds the latestorigin/mainmerge, a docs note forsessions_history(includeFamily), and the generated docs map refresh.Compatibility / migration proof for the persistent-state surfaces ClawSweeper flagged:
includeFamilyselector is opt-in; omitted orfalsekeeps the existing active-transcript-only behavior forchat.historyandsessions_history.includeFamily: truewithoffsetis rejected at the executable Gateway/embedded/tool validation boundaries instead of changing existing offset pagination semantics.e9b75be554c4is green: watchdog saw 105 checks, 94 evaluated required/relevant checks, 0 failed, 0 running; GitHub check runs include successful Real behavior proof, docs, lint, test-types, protocol/bundled checks, gateway/channel/agent critical quality, and security-high shards.