Skip to content

herd: chat session file for daemon-signed-in sessions (RT-326) - #504

Merged
m4ttheweric merged 2 commits into
mainfrom
herd-chat-session-file
Sep 26, 2026
Merged

m4ttheweric merged 2 commits into
mainfrom
herd-chat-session-file

Conversation

@m4ttheweric

@m4ttheweric m4ttheweric commented Sep 26, 2026 •

Copy link
Copy Markdown
Collaborator

Herd workers and shepherds that the daemon signs in to chat had no local chat session file, so every chat_* MCP tool refused them ("no signed-in chat session") while Bash rt chat still worked through the CLI's derived-handle fallback.

Herd handlers (lib/daemon/handlers/herd.ts)

  • recordChatSession writes the session file after the daemon's chat:sign-in for a spawned worker, and for the shepherd on herd:start and herd:resume
  • Keeps an existing file that names the same handle (the CLI's own sign-in)
  • Rewrites a stale file that names another handle
  • Logs a warning on a write failure

Verification: new tests in herd-handlers.test.ts for spawn, start, resume, stale handle and cleanup (RED first: no file after spawn); bun test lib/daemon lib/mcp 3055 pass; tsc --noEmit clean.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Herd sessions now save chat-session details when starting or resuming, and when workers sign in. This keeps session information available across those actions.
    • Matching session details are preserved; outdated handle information is refreshed without losing room details. A failure to save these details does not interrupt the herd operation.

Herd workers and shepherds signed in by the daemon had no session file, so every chat_* MCP tool refused them with 'no signed-in chat session' while Bash rt chat worked. An existing CLI-written file is kept.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: f8d5791d-5654-4f22-8cc7-bc7eb66beb08

📥 Commits

Reviewing files that changed from the base of the PR and between bf0c55f and 3587b93.

📒 Files selected for processing (2)
  • lib/daemon/__tests__/herd-handlers.test.ts
  • lib/daemon/handlers/herd.ts

Included review availability: This review used your included allowance. 5 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 10 reviews per hour.


📝 Walkthrough

Walkthrough

Herd start and resume now record shepherd session identity. Successful worker sign-in records worker session identity. Existing session files for the same handle remain unchanged; files for a different handle are updated while retaining their room. Read and write errors are logged as warnings.

Changes

Herd session recording

Layer / File(s) Summary
Record session identity
lib/daemon/handlers/herd.ts, lib/daemon/__tests__/herd-handlers.test.ts
The handlers record session identity during shepherd start and resume, and after successful worker sign-in. Tests verify the session data, preserve files for the same handle, and check that updating a different handle retains the room. Test cleanup removes the session files.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 3587b

Daemon-signed-in Herd sessions will now have chat identity files, while persistence failures remain non-fatal. No concrete release-impacting regression is established, so the change appears ready to merge subject to normal checks.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to 3587b

Recording chat identity enables the intended tools, but the new files may remain usable for handle resolution after a session is signed out through the daemon. The downstream effect needs confirmation.

Retained concerns

  • Medium · security · inferred: Daemon-created session files can continue to resolve a chat handle after daemon chat sign-out, because that sign-out does not remove the file and the MCP handle resolver checks the file rather than current presence. Downstream authorization impact is unverified.
Security review details

Security Blast Radius

  • inferred — The directly affected scope is the local session IDs of shepherds and spawned workers for which herd handlers record chat handles. Evidence does not establish a cross-tenant, network, or infrastructure privilege change.

Security Findings and Attack Paths

  • inferred — If a daemon-signed-in session is subsequently signed out through the daemon, its newly written file can still supply a handle to the MCP resolver. Whether a caller can use that handle for a chat operation after sign-out is not established.

Trust Boundaries and Controls

  • observed — The MCP resolver takes its session ID from the process environment and accepts only a readable file with the same embedded ID and a string handle. It does not check the current presence row at handle-resolution time.

Resilience and Maintainability Implications

  • inferred — A failed file write degrades MCP handle resolution without failing the herd operation. Existing tests cover matching and stale handles, but the available evidence does not establish write-failure or competing-writer outcomes end to end.

Hardening Proposals

  • proposed — Define which component removes daemon-created session files at sign-out, and verify MCP behavior after daemon sign-out and handle reassignment before treating the file as a durable identity credential.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: adding chat session files for daemon-signed-in Herd sessions. It matches the pull request objectives and affected files.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Comment @coderabbitai help to get the list of available commands.

…st start, resume and cleanup

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@m4ttheweric
m4ttheweric merged commit e9f21c7 into main Sep 26, 2026
15 checks passed
@m4ttheweric
m4ttheweric deleted the herd-chat-session-file branch September 26, 2026 19:49
@m4ttheweric m4ttheweric mentioned this pull request Sep 26, 2026
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.

1 participant