fix(chat): follow claude attach panes with options, an id prefix or a job name by devswha · Pull Request #1897 · devswha/ovis

devswha · 2026-10-24T15:16:51Z

Fixes #1645. The parts of #1645 this does not cover are now #1876.

Summary

A pane running claude attach showed the background job's conversation only for claude attach <8-hex id> (#1813). Claude Code 2.1.308 also opens a job with root options before attach, by the start of its id, and by part of its name. Those panes still showed terminal text.

server/claude-store.ts now reads the arguments and finds the job the way Claude 2.1.308 does. The rules come from its own code (backgroundSessionVerbIndex and the claude attach resolver). A real claude 2.1.308, run in a throwaway HOME/CLAUDE_CONFIG_DIR, agreed with each form below.

claudeAttachArgs(argv):
- attach counts when it is the first argument that is neither an option nor an option's value. The option tables are Claude's: -r and --debug take the next argument unless it is an option, and --add-dir and the other list options take every argument after them. So claude --debug attach x and claude --add-dir a attach x are no attach.
- Behind options, each option must be one Claude allows there (-p, --bg, --version, -cp, --exec and unknown options are not), and the job must be a whole 8-hex id.
claudeAttachedJob:
- A whole id is that job. Otherwise it opens the one job whose id starts with the argument.
- Otherwise, when attach comes first, nothing follows the argument and it cannot be the start of an id, it opens the one job whose name holds it (case-insensitive), among the job states Claude's schema loads. Another job whose task text holds the argument counts against that unless the job is over: done, failed or stopped and not active, and not a routine, self-wake, cron or /loop.
- Claude finds the job once, as the client starts. Jobs made after the client started (createdAt after the process's start) are left out, so a job renamed or made later never takes the pane.
- The job each client found is remembered by store, pid, argument and process start until the client is gone. Ambiguous or unreadable lookups are retried after 10 s. A pid that another process takes over finds its own job.
claudeAttachSession: with options before attach, the client counts only when its environment can be read and is not a background session's. Claude reads no subcommand behind options when CLAUDE_CODE_SESSION_KIND=bg, and runs an interactive Claude instead.
It fails closed. In each of these cases the pane keeps its terminal:
- an ambiguous prefix or name;
- a job state that cannot be read while names are matched;
- a client whose start time cannot be read, for a prefix or a name;
- an environment that cannot be read, for options before attach (Windows lets no one read it);
- a shell-command job, which still has no session.
server/conversation.ts: claudeTranscriptPath asks claudeAttachSession instead of the 8-hex-only attachedJob. An attach pane still reports no start time.

Remote bundle: both server files are in the remote PC bundle. REMOTE_BUNDLE_VERSION is not raised here; the release PR raises it (docs/development.md, as in #1813).

Not in this PR (#1876):

following "Session moved to …";
caching the job's parsed state;
choosing among Windows stores;
queued-input pinning (claudeForegroundSession);
the unbounded read in backgroundJobTranscript;
a name followed by debug options (Claude reads it as a name; this keeps the terminal).

Design precedent

None applies, since there is no new UI. The pane shows the existing Claude transcript view of the job's session.

Verification

Claude's own rules: the option tables and the attach resolver were read from the Claude Code 2.1.308 binary. A real claude 2.1.308, run in a throwaway HOME/CLAUDE_CONFIG_DIR with three fake jobs, agreed on each case below. Any daemon it started was stopped afterwards.
- attach aaaa with two matching ids: "Ambiguous prefix".
- abc, ABC and aaaa1: no match, and "not read as a name".
- intent: task text only, no match.
- release extra: "A name must be the only argument".
- aaaa1: opens aaaa1111.
- release: opens the job named "Release notes draft".
- readme logo: opens the job named "Check README logo".
- --model opus attach <id>, --debug --verbose attach <id> and --add-dir /tmp --verbose attach <id>: the job opens.
- A finished job's task text does not count against a name; a running one's, or a done /loop's, does.
Contract (server/claude-session.contract.test.ts, Linux, isolated test herdr): 16 pass, 0 fail on this commit. New cases, each with no PID record and the job's state on disk:
- claude -r ./resumed.js attach <id>: root options before attach. The stand-in is the renamed Node, which also takes -r as its own option, so the arguments stay as typed.
- claude attach 3a9b: a unique id prefix.
- claude attach "release notes": part of a job's name.
Each answers claude-transcript with that job's turns. claude attach 8e9f, a prefix of two jobs, in a reused pane whose earlier Claude reported a session through the hook, answers scrollback. fix(chat): follow a claude attach pane to its background job's conversation #1813's three attach cases still pass.
Unit (server/claude-store.test.ts): the argument rules, including the misreads; prefix and name lookup; schema-invalid states; jobs made after the client started; a client whose start is unknown; unreadable, oversized, link, folder and FIFO states; the job a client opened staying put after renames, other clients and a reused pid; live clients kept past 256 others; and CLAUDE_CODE_SESSION_KIND=bg read from a real child process's environment, rerun 30 times.
- Each test written for a review finding failed on the code before its fix.
- Mutating the cache, the fail-closed check, the over-job rule, the options check or the prefix uniqueness each fails a named test.
Related tests: HERDR_TEST_MODE=unit bun test server/claude-store.test.ts server/conversation.test.ts server/claude-subagents.test.ts gives 285 pass, 1 skip, 0 fail.
- conversation.test.ts "Devin pane identity > falls back when the explicit store is inaccessible or malformed" timed out once at a load average of about 100. It times out on origin/main under the same load, and it passes on a quieter machine.
bun run check fast: passed on 2a28ffb3e: typecheck, build, and 6408 unit tests with 0 failing. Since then only test files changed (the options-before contract case and one parser case), and the branch was rebased on main.
- The machine's dori heavy queue could not run another check fast in time, so CI's Fast checks are the gate for this push, as the lane lead decided.
- server/claude-store.test.ts and the contract file pass on the pushed commit.
Reviews: omo-native-code-reviewer and Codex (gpt-6.1-sol, xhigh), each over three rounds.
- Findings fixed: jobs made after the client started could be picked; cache eviction could move a live pane; CLAUDE_CODE_SESSION_KIND=bg was not read; schema-invalid states were matched; an environment test raced exec; an unreadable environment fell back to the hook's session.
- Final verdict: both APPROVE. The one point left, validating Claude's other optional job-state fields, is Claude Code attach panes: follow-ups from #1645 #1876 item 7.
Not run natively on macOS or Windows. The contract cases are Linux-only, like fix(chat): follow a claude attach pane to its background job's conversation #1813's. Windows cannot read another process's environment, so options before attach keep the terminal there.

… job name A pane running `claude attach` showed the background job's conversation only for `claude attach <8-hex id>` (#1813). Claude Code 2.1.308 also takes root options before `attach`, the start of a job's id, and part of a job's name; such panes kept their terminal text. claude-store.ts reads the arguments and finds the job the way Claude does (its backgroundSessionVerbIndex and attach resolver): `attach` as the first operand, behind its root options only with a whole id and outside a background session's environment; a unique id prefix; else a name, where another running job's task text counts against it, over the job states Claude loads. Jobs made after the client started are left out. It fails closed: an ambiguous or unreadable job state, or a client whose start or environment cannot be read, keeps the terminal. Each client's job is remembered by store, pid and start until the client is gone, since Claude resolves once as it starts. Fixes #1645. Remaining items: #1876.

coderabbitai · 2026-10-24T15:17:01Z

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 37 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

Run configuration

Source mode: Auto
Repository: devswha/ovis
Branch: fix/claude-attach-variants
File: .coderabbit.yaml
Commit: 3e1012d
Layers: Repository YAML
Review profile: CHILL
Plan: Advanced
Run ID: 67d02fa3-4dd4-41f5-a4ec-7f3914d8fdc4

Commits

Reviewing files that changed from the base of the PR and between 4c8ce30 and 3e1012d.

Files selected for processing: 5

changelog.d/claude-attach-variants.md
server/claude-session.contract.test.ts
server/claude-store.test.ts
server/claude-store.ts
server/conversation.ts

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

devswha · 2026-10-24T15:17:55Z

CodeRabbit's review hit its rate limit on this PR, so no CodeRabbit review ran. For the merge pass, here are the reviews that did run before the push, with each finding and how it was resolved. Every reproducible finding got a test that failed on the code before its fix.

Reviewers: omo-native-code-reviewer and Codex (gpt-6.1-sol, xhigh). Each reviewed the full diff and then every fix delta.

Finding	Severity	Resolution
A job made, or renamed to match, after the client started could be picked when the server found the job later (cold cache, restart)	HIGH	Jobs whose `createdAt` is after the client process's start are left out. A prefix or name lookup opens nothing when the start cannot be read.
A reused pid could inherit another client's remembered job	HIGH	The cache keeps the process start and finds the job again when it moves.
FIFO eviction after 256 clients could move a live pane to another job	HIGH	No eviction. Entries are pruned only on a confirmed exit (no signal reaches the pid) or a known moved start; with 256 live clients a new one keeps its terminal. The prune never deletes an entry another lookup just replaced.
`CLAUDE_CODE_SESSION_KIND=bg` with options before `attach` runs an interactive Claude, but a torn PID record let it count as an attach	HIGH	The variable is read from the process's environment. An environment that cannot be read keeps the terminal and never falls back to the hook's session.
Job states Claude's schema rejects could still match a name	HIGH	The required fields (including `template`) and the types of every field the lookup reads are checked. Claude's other optional fields are not: issue #1876, item 7 (LOW in the final review).
Unbounded parallel state reads, and EMFILE rethrown	MEDIUM	States are read 8 at a time, and EMFILE/ENFILE count as unreadable.
The environment test raced `exec` (empty `/proc/<pid>/environ`)	HIGH (test)	The child signals readiness and then waits on its input without `exec`; 30/30 reruns pass.

Final verdicts: both APPROVE.

devswha · 2026-10-24T20:20:41Z

Merge pass review-work report. Overall: PASSED.

Lane	Verdict	Confidence
Manual QA (artifacts kept)	PASS	-
Gate review (another model; programming + remove-ai-slops applied directly)	APPROVE	HIGH

QA

claude-store and conventions: 89 pass.
The typecheck passes.
git merge-tree is clean against main and against fix(chat): follow-ups from the agent command forms review #1893, which shares server/conversation.ts.

Gate

88 unit and conventions tests pass, with 1 platform skip. All 16 isolated conversation contracts pass.
An ambiguous id prefix or job name fails closed. A reused pane gets terminal scrollback, never the previous conversation.
The four non-attach forms all return null: --debug attach x, --add-dir a attach x, -r attach, and an unknown option before attach.
Malformed and very long argv neither throw nor hang.
A mutant that drops prefix uniqueness fails the ambiguity test: expected null, got dc5643bd.

Follow-up (LOW): each job-state read is bounded, but the total lookup work grows with the number of jobs.

All required checks are green and no review thread is open.

CodeRabbit did not review this PR (rate-limited or skipped). The reviews above stand in for it.

…e pinned, and pi sessions over 16 MiB (#1936) Fixes #1544 Fixes #1549 Fixes #1487 Follow-ups to the command outcomes (#1543, #1450) that Chat shows under a command it sent. Seven commits: one per issue, the changelog fragment, two rounds of review fixes, and a Hermes test brought up to date with the shared snapshot. ## What changes for the user - **#1487: pi sessions over 16 MiB.** A command sent from Chat to a pi session larger than the 16 MiB window now shows its result. Before, it stayed "Accepted" until "No result seen" whenever pi kept writing. The first entry in a cut window was counted as a branch point, so almost every append changed the transcript's identity, and each send bound to the old identity was skipped from then on. On an append, the branch read before is now kept while the new path runs on from its last entry with no fork after it. That last entry may sit on the path or just before the window's first entry. Any other append, such as a `/tree` switch or a fork made since, names the branch afresh, as before. - **#1487: cheaper changed polls.** A changed poll still checks every byte the cached and new reads share; `validates the full overlapping tail within budget when the window slides` pins that check against in-place rewrites. Three things changed: - The check no longer sha256-hashes both sides. - The window starts on a 64 KiB boundary, and the cache keeps an xxHash3 digest of each chunk instead of up to four raw 16 MiB windows. - The read buffer is no longer zeroed first; a short read still gives up. With four 33 MiB pi transcripts cached, the memory held went from about 190 MiB to 65-95 MiB. On 33 MiB with 16 KiB appends, a changed poll's median went from 34-49 ms on origin/main to 10-20 ms (busy host, 3 runs each). Reading only the appended bytes would drop that pinned full-overlap check, so it is left for #1879, with the rest of #1487's low items. - **#1544: herdr calls per poll.** While a command waits for its result, each `GET /api/pane/prompt` poll of a Claude pane asked herdr for `pane.process_info` up to four times and for `session.snapshot` twice. The poll now reads the processes once and hands that answer and its snapshot to the transcript lookup (Claude, pi, devin, unlabelled panes, and the OmO check). A read that fails is no answer, so the lookup then asks herdr itself as before; a `claude attach` client still leads to its job's session. The Codex, omp, gjc and OmO store lookups still read on their own, noted in #1879. - **#1549**: - Tests at 59 s (row kept) and 61 s (row hidden) after a session change typed in the terminal now pin `STALE_SESSION_LAG_MS`. The old test jumped to 11 minutes, so a 5-minute bound also passed. A mutation to 300 s or 30 s fails one of them. - A send whose agent process could not be matched (native Windows lists only the shell) now also adopts a session herdr reports under the label herdr gave the pane at the send. An OmO send therefore keeps its row when OmO's session appears under herdr's `pi` label. - `docs/agent-commands.md` now states which session a row belongs to, plus four limits: the minute is counted when Chat next asks, a terminal-typed change in a resumed session within it, pid reuse outside Linux, and repeat ownership trusting herdr's history rows (the F4 items). ## How it was verified - Each new test fails before its fix and passes after: - the method-count test (4 `pane.process_info` calls before); - the 61 s test, under a 5-minute bound (and the 59 s test under a 30 s one); - the OmO adoption test, without the label; - the sliding-window, branch-switch, later-fork, leaf-cut and fork-slide pi tests, and the failed-read `claude attach` test, each against the commit before its fix. With the chunk-digest check forced to always match, five rewrite tests fail, including the pinned one. - **Real surface:** `server/outcome.contract.test.ts` starts the server, uses the isolated `ovis-test` herdr and runs a stand-in `claude` in a real pty. It sends `/context` from Chat over the WebSocket and reads `GET /api/pane/prompt`. A copy of it counts herdr RPCs during the first poll after the send: origin/main made 3 `pane.process_info`, 2 `session.snapshot` and 1 `agent.get` calls; this branch makes 1, 1 and 1. The outcome is `result-observed` in both. - `server/outcome*`, `prompt*`, `conversation*` and `pi*`: 752 pass. The one exception is `conversation.test.ts` "Devin pane identity > falls back when the explicit store is inaccessible or malformed", which timed out at 5.9 s on this busy host. It runs 4.1 s alone on origin/main and does not touch this change. `bun run check fast` passes. - **Reviews:** omo-native-code-reviewer and Codex (gpt-6.1-sol, xhigh) reviewed this. - Codex's two reproduced MEDIUMs, a branch switch at the cut and a failed process read, are fixed in "keep a pi branch while the window slides, and ask again after a failed process read". - Its re-review found two more MEDIUMs, a fork after the cached leaf and an append cutting the leaf out. Both are fixed in "carry a pi branch only past no new fork, and keep chunk digests instead of the window" and covered by tests. - A branch that switches away and comes back still loses its send. Codex reproduced that on the parent commit too, so it is not new here; it is noted in #1879. - omo's LOW stays: an OmO pane labelled `claude` while its Claude child runs could adopt the child's session, which hides the row and never shows a wrong result. - **After the rebase onto main** (#1897's `claude attach` options and #1899's pi agent directory meet this change in `server/conversation.ts`): Codex reviewed the rebase delta and found nothing. Its 19 scratch tests confirm that an attach client found through the shared process read still leads to its job's session and that pi's store and agent directory resolve the same with and without the shared read. The live contract QA above was rerun and still counts 1 `pane.process_info`, 1 `session.snapshot` and 1 `agent.get`, `result-observed`. `bun run check fast` passes on the rebased head. No HTTP or WebSocket contract, auth or workflow changes. --------- Co-authored-by: mp <mp@local>

devswha merged commit 73acbaa into main Oct 24, 2026
18 checks passed

devswha deleted the fix/claude-attach-variants branch October 24, 2026 20:20

Navigation Menu

Repository navigation

fix(chat): follow claude attach panes with options, an id prefix or a job name - #1897

fix(chat): follow claude attach panes with options, an id prefix or a job name#1897
devswha merged 1 commit into
mainfrom
fix/claude-attach-variants

devswha commented Oct 24, 2026

Uh oh!

coderabbitai Bot commented Oct 24, 2026

Review limit reached

Uh oh!

devswha commented Oct 24, 2026

Uh oh!

devswha commented Oct 24, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

devswha commented Oct 24, 2026

Summary

Design precedent

Verification

Uh oh!

coderabbitai Bot commented Oct 24, 2026

Review limit reached

Uh oh!

devswha commented Oct 24, 2026

Uh oh!

devswha commented Oct 24, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant