Skip to content

refactor: route session status through accessors#96460

Merged
jalehman merged 1 commit into
openclaw:mainfrom
jalehman:josh/path3-session-status-accessor
Jun 24, 2026
Merged

refactor: route session status through accessors#96460
jalehman merged 1 commit into
openclaw:mainfrom
jalehman:josh/path3-session-status-accessor

Conversation

@jalehman

@jalehman jalehman commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

Path 3 is moving session metadata and transcript access behind storage-neutral seams before the SQLite-backed session store lands. session_status still had direct session-store reads/writes for visibility-aware session resolution and model-selection persistence, which kept one gateway-visible tool coupled to the JSON store shape.

Fixes part of #88838.

Why This Change Was Made

session_status is the user-facing “what session am I in and what model is it using?” surface. It has to preserve semantic current, literal main, session-id lookup, requester visibility, agent-to-agent policy, and model override behavior while moving storage access through the permanent session accessor boundary. A simple loadSessionEntry swap is not enough because the tool resolves ordered candidate keys and can materialize implicit current-session fallbacks before a model mutation.

User Impact

Users should see the same /status-style output and model pin/reset behavior. The change is internal: session-status reads and model override writes now go through named session accessor/query operations, so the tool is ready for the SQLite session backend without adding runtime fallback readers or broad compatibility shims.

Changes

  • Added an ordered session-entry candidate accessor.
  • Routed session_status lookup through accessor queries.
  • Routed model persistence through keyed patch accessors.
  • Kept status-specific resolution local to the tool.
  • Added regression coverage for implicit fallback writes.
  • Extended the session accessor boundary ratchet.

Evidence

Final rebased head: e61837e665befd7cf1e1cab998de9bdd6107138a
Base: acc2a0ee7297f178693ea18d06baf194aeab736d

Post-rebase local proof on the final head:

node scripts/run-vitest.mjs src/config/sessions/session-accessor.test.ts src/agents/openclaw-tools.session-status.test.ts test/scripts/check-session-accessor-boundary.test.ts
node scripts/check-session-accessor-boundary.mjs && git diff --check upstream/main...HEAD
pnpm exec oxfmt --check --threads=1 scripts/check-session-accessor-boundary.mjs src/agents/openclaw-tools.session-status.test.ts src/agents/tools/session-status-tool.ts src/agents/tools/session-status-session-resolve.ts src/config/sessions.ts src/config/sessions/session-accessor.test.ts src/config/sessions/session-accessor.ts test/scripts/check-session-accessor-boundary.test.ts
node scripts/run-oxlint.mjs scripts/check-session-accessor-boundary.mjs src/agents/openclaw-tools.session-status.test.ts src/agents/tools/session-status-tool.ts src/agents/tools/session-status-session-resolve.ts src/config/sessions.ts src/config/sessions/session-accessor.test.ts src/config/sessions/session-accessor.ts test/scripts/check-session-accessor-boundary.test.ts
node scripts/run-tsgo.mjs -p tsconfig.core.json --incremental --tsBuildInfoFile .artifacts/tsgo-cache/core.tsbuildinfo
node scripts/run-tsgo.mjs -p test/tsconfig/tsconfig.test.src.json --incremental --tsBuildInfoFile .artifacts/tsgo-cache/test-src.tsbuildinfo
.agents/skills/autoreview/scripts/autoreview --mode local

Result: focused Vitest passed 28 + 56 + 64 tests; boundary guard, diff check, oxfmt, oxlint, both tsgo probes, and autoreview all passed.

Real behavior proof

Behavior addressed: session_status resolves semantic current through the gateway-visible path and persists a per-session model override without changing the existing session row identity.

Real environment tested: local live gateway from ~/Projects/clawdbot, OpenClaw 2026.6.9, temporary proof branch with this patch applied. Live proof was run before the final conflict-free rebase; the final head above has fresh focused proof and autoreview.

Exact steps or command run after this patch:

pnpm build
pnpm openclaw gateway restart
pnpm openclaw --version
curl -fsS http://127.0.0.1:18789/readyz
curl -fsS http://127.0.0.1:18789/healthz
pnpm openclaw gateway call health --json
pnpm openclaw gateway call tools.effective --params '{"sessionKey":"main"}' --json
pnpm openclaw gateway call tools.invoke --params '{"name":"session_status","args":{"sessionKey":"current","model":"anthropic/claude-sonnet-4-6"},"sessionKey":"main","idempotencyKey":"path3-status-proof-override-20260624"}' --json

Evidence after fix: tools.invoke returned ok=true, toolName=session_status, details.sessionKey=agent:main:main, changedModel=true, modelProvider=anthropic, and modelOverride=anthropic/claude-sonnet-4-6. The persisted row kept the same sessionId (538d6092-6323-4fcc-a7d0-d4f92a8b9bf1), set providerOverride=anthropic, modelOverride=claude-sonnet-4-6, liveModelSwitchPending=true, and did not create a bare main row. The session store was restored to the pre-proof SHA afterward, and the live gateway was rebuilt/restarted back on clean main with RPC OK.

Observed result after fix: the live gateway command path resolved current to the canonical main session and wrote model selection through the accessor-backed status path while preserving existing session identity and route metadata.

What was not tested: the future SQLite adapter/runtime flip, doctor migration behavior, plugin SDK compatibility, and CI rollup on this PR head.

@openclaw-barnacle openclaw-barnacle Bot added scripts Repository scripts agents Agent runtime and tooling size: L maintainer Maintainer-authored PR labels Jun 24, 2026
@jalehman
jalehman marked this pull request as ready for review June 24, 2026 14:14
@clawsweeper

clawsweeper Bot commented Jun 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 24, 2026, 10:29 AM ET / 14:29 UTC.

Summary
The branch moves session_status session resolution and model override persistence from direct session-store access to session accessor helpers, with regression tests and boundary ratchet updates.

PR surface: Source +138, Tests +168, Other +2. Total +308 across 8 files.

Reproducibility: not applicable. this is a behavior-neutral refactor rather than a standalone bug report. The relevant behavior path is covered by source inspection, regression tests, current checks, and the PR body's live gateway proof.

Review metrics: none identified.

Stored data model
Persistent data-model change detected: serialized state: src/agents/tools/session-status-session-resolve.ts, serialized state: src/agents/tools/session-status-tool.ts, serialized state: src/config/sessions/session-accessor.test.ts, serialized state: src/config/sessions/session-accessor.ts, unknown-data-model-change: src/agents/tools/session-status-session-resolve.ts, unknown-data-model-change: src/agents/tools/session-status-tool.ts, and 2 more. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #88838
Summary: This PR is a candidate implementation slice for the active session/transcript SQLite migration tracker.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • none.

Risk before merge

  • [P1] The diff changes session status lookup and persisted model-override writes, so a wrong key-resolution edge case could mis-associate session state even though the focused tests, live proof, and current checks are clean.
  • [P1] The future SQLite adapter/runtime flip is explicitly out of scope and still needs separate proof on the canonical migration tracker.

Maintainer options:

  1. Land with session-state awareness (recommended)
    Accept the remaining future-SQLite proof gap as intentionally out of scope for this behavior-neutral accessor slice and rely on the tracker to prove the later storage flip.
  2. Request broader pre-merge proof
    Ask for an additional maintainer-run gateway proof only if maintainers want same-head live validation after the final rebase before landing.
  3. Pause for the SQLite adapter
    Hold this PR only if maintainers decide session-status accessor adoption should not land until the actual SQLite backend implementation is ready.

Next step before merge

  • Protected maintainer label and session-state-sensitive merge risk call for normal maintainer review/landing rather than cleanup close or automated repair.

Security
Cleared: No concrete security or supply-chain concern was found; the diff changes internal TypeScript session accessors, tests, and a local boundary script without new dependencies, workflows, secrets, or permissions.

Review details

Best possible solution:

Land or continue maintainer review of this accessor-adoption slice, while keeping the SQLite storage flip and adapter proof on the canonical tracker.

Do we have a high-confidence way to reproduce the issue?

Not applicable: this is a behavior-neutral refactor rather than a standalone bug report. The relevant behavior path is covered by source inspection, regression tests, current checks, and the PR body's live gateway proof.

Is this the best way to solve the issue?

Yes: moving session_status reads and writes behind the existing session accessor boundary is the maintainable owner-boundary fix for this migration slice. The safer alternative would be to leave direct JSON-store access in place until the SQLite flip, but that conflicts with the staged accessor plan in the canonical tracker.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 374076b5a8c5.

Label changes

Label changes:

  • add P2: This is a normal-priority internal session-state migration slice with limited blast radius and focused proof, not an emergency runtime outage.
  • add merge-risk: 🚨 session-state: The PR changes how session_status resolves and persists per-session model state, so a merge regression could write or display state for the wrong session row.
  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes live gateway command output showing session_status resolving current and persisting a model override through the changed path, with focused final-head tests after the rebase.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes live gateway command output showing session_status resolving current and persisting a model override through the changed path, with focused final-head tests after the rebase.

Label justifications:

  • P2: This is a normal-priority internal session-state migration slice with limited blast radius and focused proof, not an emergency runtime outage.
  • merge-risk: 🚨 session-state: The PR changes how session_status resolves and persists per-session model state, so a merge regression could write or display state for the wrong session row.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes live gateway command output showing session_status resolving current and persisting a model override through the changed path, with focused final-head tests after the rebase.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes live gateway command output showing session_status resolving current and persisting a model override through the changed path, with focused final-head tests after the rebase.
Evidence reviewed

PR surface:

Source +138, Tests +168, Other +2. Total +308 across 8 files.

View PR surface stats
Area Files Added Removed Net
Source 4 301 163 +138
Tests 3 170 2 +168
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 1 2 0 +2
Total 8 473 165 +308

What I checked:

Likely related people:

  • jalehman: Authored the current PR and recent merged accessor-migration slices for boot session mapping and checkpoint mutations that touch the same session-accessor boundary. (role: recent area contributor; confidence: high; commits: e61837e665be, acc2a0ee7297, c588606a9bf3; files: src/agents/tools/session-status-tool.ts, src/config/sessions/session-accessor.ts, scripts/check-session-accessor-boundary.mjs)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. labels Jun 24, 2026
@jalehman
jalehman merged commit b58e6e0 into openclaw:main Jun 24, 2026
207 of 215 checks passed
@clawsweeper clawsweeper Bot mentioned this pull request Jun 24, 2026
Closed
@github-actions github-actions Bot mentioned this pull request Jun 24, 2026
Open
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jun 25, 2026
@jalehman
refactor: route session status through session accessors (openclaw#96460
480fb7f 
)
QiuYuang pushed a commit to QiuYuang/openclaw that referenced this pull request Jul 1, 2026
@jalehman @QiuYuang
refactor: route session status through session accessors (openclaw#96460
a8f1bf6 
)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

agents Agent runtime and tooling maintainer Maintainer-authored PR merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. scripts Repository scripts size: L status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@jalehman