Skip to content

fix(chat): hold pending input after process replacement - #1069

Merged
devswha merged 1 commit into
mainfrom
fix/pending-identity-process-fingerprint
Oct 14, 2026
Merged

devswha merged 1 commit into
mainfrom
fix/pending-identity-process-fingerprint

Conversation

@devswha

@devswha devswha commented Oct 14, 2026

Copy link
Copy Markdown
Owner

Refs #540

Root cause

pendingContext in server/index.ts (origin/main L787-788) judged whether a queued message still targets the same agent by comparing only agent, terminalId and the native session reference. An agent that reports no native session (hookless) and is killed and restarted in the same pane keeps the same terminal and agent name, so the replacement process received the message typed for the old one. That comparison was also strict on session, so a session that was unknown at enqueue and became known later was refused as if the target had changed.

What changed

  • server/pending-input.ts: PendingIdentity gains optional weak parts processPid, processBirth and omoSession. The new samePendingIdentity() keeps strict equality for agent and terminalId. Every other part is compared only when both reads know it, so unknown to known is an upgrade, not a replacement, while known to different is refused. Process birth follows the existing gjc-runtime.ts rule: exact on Linux, ±1000 ms on macOS, where ps etime has whole-second precision.
  • server/index.ts pendingContext: the identity is enriched with the foreground pid from the typed paneProcessInfo wrapper, its birth from the existing processStartedAt helper, and OmoStatus.sessionOf(paneId). It is captured at enqueue and compared on every delivery check. A mismatch raises pending_target_changed, so the message is held for review. Parts learned later are kept on the stored identity, so a replacement between paste and Enter is checked against them too.
  • No protocol, frame or UI string changes. Changelog fragment: changelog.d/pending-process-identity.md.

Residual (why Refs, not Fixes)

A /clear or resume inside the same process, by an agent that reports neither a native session nor an OmO session, is still indistinguishable, so it is out of scope. On Windows panes where herdr reports no foreground process, the fingerprint stays weak (pid/birth unknown) and the previous behavior applies.

Evidence

Each new assertion is labelled regression or control in the tests.

  • Before (product code from origin/main, new tests): 4 contract tests fail. These are: the hookless process replacement (the message is not held), the 1500 ms same-pid birth drift on darwin, the null to known native session upgrade (wrongly held on main), and the different OmO session (not held). Both controls pass: same pid and birth delivers exactly once, and a 600 ms darwin drift delivers. The new unit file cannot load on main because samePendingIdentity doesn't exist there.
  • After: all 6 Pending messages: a session replaced inside the same pane is not noticed when the agent reports no native session #540 contract cases pass. In the replacement case, SESSION2's recorded bytes contain nothing of the queued message, and the WS client receives held with pending_target_changed.
  • A live probe on herdr 0.9.3 showed that foreground_processes[0] stays the agent while it runs a child in its own foreground process group. A tool subprocess therefore does not flip the fingerprint.

Checks run

  • bun run typecheck: exit 0
  • HERDR_TEST_MODE=unit bun test ./server/pending-input.test.ts: 29 pass, 0 fail
  • bun test ./server/pending-input.contract.test.ts (isolated herdr session): 56 pass, 0 fail
  • bun run test:unit: 3518 pass, 0 fail, 8 skipped
  • git diff --check: clean

Capture the agent process PID and birth time, plus OmO session identity, when pending input is queued and before it is delivered. Compare only identity parts known on both reads, with the existing macOS process-birth tolerance.

Refs #540
@coderabbitai

coderabbitai Bot commented Oct 14, 2026

Copy link
Copy Markdown

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 1 minute.

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
  • Configuration used: Repository: devswha/herdr-web-ui/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 7fab728e-db8b-44cd-bf84-ab6f7a6012c3
📥 Commits

Reviewing files that changed from the base of the PR and between 0de23fc and 0b21926.

📒 Files selected for processing: 5
  • changelog.d/pending-process-identity.md
  • server/index.ts
  • server/pending-input.contract.test.ts
  • server/pending-input.test.ts
  • server/pending-input.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
devswha merged commit 38d6863 into main Oct 14, 2026
23 of 25 checks passed
@devswha
devswha deleted the fix/pending-identity-process-fingerprint branch October 14, 2026 11:05
devswha added a commit that referenced this pull request Oct 17, 2026
…process (#1310)

Refs #540

## Root cause
`pendingContext` (`server/index.ts`) checks a queued message's target
with `samePendingIdentity`: agent, terminal, herdr's native session,
foreground pid and birth, and OmO's session (#1069). For a Claude Code
pane without herdr's hook, `agent_session` is null, and `/clear` or
`/resume` switches the conversation inside the same process, so every
part stays equal and the message is typed into the new conversation.

## What changed
- Claude Code rewrites its own PID record,
`<config>/sessions/<pid>.json`, when it switches session (checked in the
2.1.302 bundle: the concurrent-sessions writer updates `sessionId` on a
session switch). `claudeForegroundSession` (`server/claude-store.ts`)
reads it for the pane's one Claude process, through the existing
`processClaudeConfigDir` and `claudeProcessSession` (which already
verifies pid and process start).
- `PendingIdentity` gains `claudeSession`; `samePendingIdentity`
compares it only when both reads know it, as the other weak parts, so a
session learned after enqueue is not a replacement and an unreadable
record falls back to today's behavior. It is read for every Claude pane,
hook or not.
- No protocol or UI change. Changelog fragment
`changelog.d/pending-claude-clear.md`.

## What is left on #540
- Other agents that report neither a native nor an OmO session and can
switch session in one process (pi/omp, gjc, grok, hermes, OpenCode,
Devin, Antigravity) are not covered: each would need its own session
reading.
- Windows panes where herdr reports no foreground process: no pid, so no
PID record to read. That needs herdr to report the process.
- A hooked Claude relies on herdr updating `agent_session` after
`/clear`; `claudeSession` now covers it as well where the record is
readable.

## Evidence
- `server/pending-input.test.ts`: a known-and-different `claudeSession`
is refused; equal, learned-later and unreadable-now are permitted (the
refusal fails on origin/main, where the part is ignored).
- `server/claude-store.test.ts`: `claudeForegroundSession` returns the
recorded session, follows a rewrite of the record, and is null for no or
two Claude processes.
- `server/pending-input.contract.test.ts`: on an isolated herdr with a
recorder named `claude` (no `agent_session`), a queued message is held
with `pending_target_changed` and nothing is typed when the record names
another session at the turn's end; the control delivers once when it
keeps its session. The record itself is stubbed, so no real Claude store
is written.

## Checks run
- `bun run check fast`: all ok.
- `bun run check run bun test --timeout 30000
./server/pending-input.contract.test.ts` (isolated herdr): 63 pass, 0
fail.
- Browser lane not run locally; CI runs it.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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