Skip to content

fix: last-response viewer follows /clear to the new Claude conversation#76

Merged
Ark0N merged 1 commit intoArk0N:masterfrom
TeigenZhang:fix/eye-icon-follows-clear
Apr 28, 2026
Merged

fix: last-response viewer follows /clear to the new Claude conversation#76
Ark0N merged 1 commit intoArk0N:masterfrom
TeigenZhang:fix/eye-icon-follows-clear

Conversation

@TeigenZhang
Copy link
Copy Markdown
Contributor

Summary

The last-response viewer (eye icon) keeps showing the pre-/clear transcript after a user runs /clear in the Claude CLI. This is because the Session's _claudeSessionId is only refreshed when Claude emits JSON on stdout, which never happens in the interactive PTY mode Codeman spawns.

This PR adds two complementary update paths:

  • Session.adoptClaudeSessionId() — public setter mirroring the existing no-op-if-same guard. Called from POST /api/hook-event whenever Claude Code hooks carry data.session_id (works automatically once hooks are configured in the target case).

  • /api/sessions/:id/last-response now resolves the active conversation id from ~/.claude/history.jsonl before reading the transcript. This is the only source-of-truth that does not require hooks, which matters because Codeman intentionally does not write hook config into arbitrary user repos (see the comment at session-routes.ts POST /api/sessions).

The history scan:

  • Skips entries held by other Codeman sessions in the same cwd so concurrent tabs don't shadow each other.
  • Requires the candidate's jsonl mtime to exceed the initial <session.id>.jsonl mtime, so a stale id inherited from a prior dead session is never adopted.

Why history.jsonl

Claude Code writes one line per user prompt to ~/.claude/history.jsonl with {sessionId, project, timestamp}. After /clear the new conversation uuid shows up there on the first prompt. ~/.claude/sessions/<pid>.json would be nicer but older Claude CLI versions don't update it after /clear, so it isn't reliable across versions.

Test plan

  • Verified against a session that had been /cleared: claudeSessionId adopts the post-/clear uuid on the next eye-icon request, and the returned transcript reflects the new conversation.
  • Verified non-cleared sessions are unaffected (claudeSessionId stays equal to session.id).
  • Synthetic hook POST with a fake session_id flips claudeSessionId and a follow-up POST with the original id restores it.
  • tsc --noEmit clean, npm run build succeeds.

Known limitation

If two concurrent Codeman sessions in the same cwd both run /clear before either has had its id re-resolved, the history scan can't disambiguate them (history.jsonl doesn't record which Codeman session typed). Single-session /clear and single-tab-per-cwd setups are unaffected.

Interactive Claude CLI never emits session_id on stdout, so the
Session's _claudeSessionId stayed pinned to the pre-/clear jsonl and
the last-response viewer kept showing the old conversation.

Two complementary update paths:

- Session.adoptClaudeSessionId() — public setter mirroring the existing
  no-op-if-same guard. Called from POST /api/hook-event when Claude Code
  hooks carry data.session_id (works once hooks are configured).

- /api/sessions/:id/last-response now resolves the active id from
  ~/.claude/history.jsonl before reading the transcript. This is the
  only source-of-truth that does not require hooks, and we intentionally
  don't write hooks into arbitrary user repos.

History scan filters out sessionIds held by other Codeman sessions in
the same cwd, and validates via jsonl mtime to avoid inheriting a dead
prior session's id.
@aakhter
Copy link
Copy Markdown
Contributor

aakhter commented Apr 26, 2026

Nice fix for the /clear session tracking issue! One security concern I noticed:

Path traversal via candidateSid -- In resolveActiveClaudeSessionIdFromHistory, the sessionId parsed from ~/.claude/history.jsonl is used unsanitized in path.join:

const cs = await fs.stat(join(projectsDir, projDir, `${candidateSid}.jsonl`));

path.join does not neutralize .. segments, so a crafted sessionId like ../../etc/passwd would cause the server to stat arbitrary filesystem paths. The same value also flows into adoptClaudeSessionId() which stores it as trusted session state with no format validation.

Suggested fix -- Validate that session IDs are UUIDs before use. This can be enforced in adoptClaudeSessionId as a defensive invariant, plus at the point of extraction from history.jsonl:

// In Session class:
private static readonly SESSION_ID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i;

adoptClaudeSessionId(newId: string): void {
    if (!newId || newId === this._claudeSessionId) return;
    if (!Session.SESSION_ID_RE.test(newId)) return;
    this._claudeSessionId = newId;
}

// In resolveActiveClaudeSessionIdFromHistory, after extracting candidateSid:
const SESSION_ID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i;
if (!candidateSid || !SESSION_ID_RE.test(candidateSid)) return null;

This protects both the stat calls and any downstream consumers of _claudeSessionId.

@Ark0N
Copy link
Copy Markdown
Owner

Ark0N commented Apr 28, 2026

Thank you, @TeigenZhang! Reviewed the PR and verified locally — typecheck, lint, and format all pass.

The two-pronged design is well thought out:

  • Hook path (adoptClaudeSessionId from data.session_id) is the clean route once hooks exist.
  • history.jsonl fallback correctly handles the "no hooks installed in user repo" case, and the mtime safety check + sibling-tab dedup are good guards against stale-id adoption.

The known limitation (concurrent /clear across same-cwd tabs) is honestly disclosed and an acceptable trade-off — single-session-per-cwd, the common case, works correctly.

Merging now.

@Ark0N Ark0N merged commit f21df2a into Ark0N:master Apr 28, 2026
1 check passed
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