Skip to content

fix: ACP session history visibility diagnostics#1

Open
jeftekhari wants to merge 6 commits into
mainfrom
fix/acp-history-visibility-diagnostics
Open

fix: ACP session history visibility diagnostics#1
jeftekhari wants to merge 6 commits into
mainfrom
fix/acp-history-visibility-diagnostics

Conversation

@jeftekhari

@jeftekhari jeftekhari commented Mar 2, 2026

Copy link
Copy Markdown

Summary

  • Problem: ACP session history can return forbidden or messages: [] across contexts despite existing session metadata and thread output. Operators cannot distinguish "hidden" vs "empty" vs "not persisted."
  • Why it matters: Without a status field, callers (tools, UI, debugging) have to guess why history is empty, leading to confusion and misdiagnosis.
  • What changed: Added readSessionMessagesWithStatus (returns { messages, fileFound }) and a historyStatus field ("ok" | "not_found" | "not_persisted" | "empty") to the chat.history gateway response. Added unit +
    integration tests.
  • What did NOT change (scope boundary): readSessionMessages signature preserved (delegates internally). No changes to sessions.preview, sessions.resolve, SessionEntry schema, or existing status value names.

Change Type (select all)

  • Bug fix
  • Feature
  • Refactor

Scope (select all touched areas)

  • Gateway / orchestration
  • API / contracts

Linked Issue/PR

  • Related: ACP session history visibility investigation

User-visible / Behavior Changes

  • chat.history response now includes historyStatus field. Existing callers that don't read this field are unaffected (additive, backwards-compatible).

Security Impact (required)

  • New permissions/capabilities? No
  • Secrets/tokens handling changed? No
  • New/changed network calls? No
  • Command/tool execution surface changed? No
  • Data access scope changed? No

Repro + Verification

Environment

  • OS: Linux (WSL2)
  • Runtime/container: Node 22+ / Bun
  • Model/provider: N/A (gateway infra)
  • Integration/channel (if any): N/A

Steps

  1. Start gateway, create a session
  2. Call chat.history with the session key
  3. Inspect historyStatus in the response

Expected

  • historyStatus reflects transcript state: "ok", "empty", "not_persisted", or "not_found"

Actual

  • Confirmed via test suite (38 unit tests, 11 integration tests — all passing)

Evidence

  • Failing test/log before + passing after
    • 4 new readSessionMessagesWithStatus unit tests (all pass)
    • 5 new chat.history historyStatus integration tests (all pass)
    • All 34 pre-existing unit tests and 6 pre-existing integration tests unaffected

Human Verification (required)

  • Verified scenarios: all 5 historyStatus states via integration tests against real gateway harness
  • Edge cases checked: missing store entry, entry without sessionId, missing transcript file, empty transcript, transcript with messages
  • What you did not verify:

Compatibility / Migration

  • Backward compatible? Yes
  • Config/env changes? No
  • Migration needed? No

Failure Recovery (if this breaks)

  • How to disable/revert this change quickly: revert the 5 commits; readSessionMessages preserved as-is
  • Files/config to restore: src/gateway/session-utils.fs.ts, src/gateway/server-methods/chat.ts, src/gateway/session-utils.ts
  • Known bad symptoms reviewers should watch for: chat.history response missing expected fields (unlikely — additive change)

Risks and Mitigations

  • Risk: So many commits happening that we might conflict or overlap with other changes
    • Mitigation: Thorough testing and we keep rebasing off main

@seslly
seslly force-pushed the fix/acp-history-visibility-diagnostics branch from 7575bf8 to a8a94a7 Compare March 3, 2026 05:34
@carl-shipstuff

Copy link
Copy Markdown

Live repro + fix validation from Carl

Repro on current runtime (before PR)

  • Queried session history for ACP key:
    • agent:claude:acp:8194e78d-52bb-474d-b036-0273f2949f2c
  • Result was ambiguous (no diagnostic state):
{
  "sessionKey": "agent:claude:acp:8194e78d-52bb-474d-b036-0273f2949f2c",
  "messages": [],
  "bytes": 2
}

Validate fix on PR branch

  • Checked out PR head a8a94a71 and built.
  • Ran a live dev gateway instance from PR build on port 18891.
  • Called chat.history against that gateway for the same ACP key:
node dist/entry.js gateway call chat.history \
  --url ws://127.0.0.1:18891 \
  --token devtoken123 \
  --json \
  --params '{"sessionKey":"agent:claude:acp:8194e78d-52bb-474d-b036-0273f2949f2c","limit":5}'
  • Actual response includes the new status:
{
  "sessionKey": "agent:claude:acp:8194e78d-52bb-474d-b036-0273f2949f2c",
  "sessionId": "9ae6ac96-7286-4a1a-b1b3-419c8b78ec70",
  "messages": [],
  "thinkingLevel": "low",
  "historyStatus": "not_persisted"
}

✅ Confirmed end-to-end: repro before PR, then diagnostic historyStatus after PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants