Repository navigation
docs(guide): Command Code chat needs the session herdr reports, and none does yet - #1647
Conversation
…one does yet The Supported agents table said Command Code's session file is "resolved through herdr". No released herdr reports a cmd session: herdr keeps the session report only of the agents listed in is_official_agent_source (src/agent_resume.rs) and Command Code is not among them, so agent_session stays absent for the pane while its status, title and model arrive from the same reporter. The bridge reads the conversation only from the file herdr names, so a cmd pane opens in Terminal, which is what a user sees and the row did not say. Write that limit where the row points, as the Devin row states its own, and note that the permission prompt cards come off the screen rather than the session file, so they still answer from the chat.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Summary
Merge Risk: ⚪ Minimal · up to
|
devswha
left a comment
There was a problem hiding this comment.
Looks good to merge. Checked against server/cmd.ts: cmdTranscriptPath binds only a session herdr reports with source === "herdr:cmd", so a herdr that drops Command Code's report leaves the pane in Terminal, as the new paragraph says. Docs-only, no conflicts with the other open PRs touching docs/guide.md.
Last pass over the Ovis rebrand leftovers. `git grep -I -i -n -E "herdr[- ]?web"` still finds 1404 hits on main (1094 in CHANGELOG.md, 310 elsewhere). I audited each one. Only one could be fixed in a PR: - `docs/app-updates.md`: "herdr-web-ui is MIT" -> "Ovis is MIT". What the rest are: - **Kept on purpose:** links to the old repo and notes in CHANGELOG.md / release-summaries.json, herdrweb.dev installer/demo forwarding, legacy plugin id / state dir / localStorage / cache migrations and their tests, the remote bridge registry and runtime paths (`~/.config/herdr-web-ui/bridges`, `%LOCALAPPDATA%\herdr-web-ui`, a cross-version contract), deployed telemetry Worker/D1 names, ovis-site's deliberate "formerly herdr web UI" and quoted testimonials, and fixtures captured verbatim. - **Left for the maintainer, user-facing but blocked:** - `CHANGELOG.md:3` intro: CHANGELOG.md is not edited in PRs. - `telemetry/worker.ts:2` doc comment: any change fails `scripts/telemetry-deployed.test.ts` until the Worker is redeployed. - `docs/guide.md:56` stale Node path: touched by open PRs #1706, #1652, #1650, #1647, #1542 and #1442. - `server/api.contract.test.ts:36` stale comment: touched by open PRs #1706, #1598 and #1427. - **Unclear:** `AGENTS.md` names the real `herdr-web-main.service` unit and `~/.herdr/worktrees/herdr-web-ui*` dirs, which still exist under those names. There are also internal names: the `/herdr-web-ui/demo/app/` test subpath in about 37 regression scripts, the film artifact names, install.ps1's `HerdrWebUi.GhJob` type, and test fixture strings. No behaviour change, so no changelog fragment. `bun run check fast` passes: typecheck, build, and 6161 unit tests pass with 0 failures.
## Change
Renames the remaining pre-rename product names in the docs and the
installer that nothing depends on:
- `docs/guide.md`: new installs put Node in `~/.local/share/ovis/node`
(`install.sh` `NODE_DIR`), not `~/.local/share/herdr-web-ui/node`. The
pre-rename path is still documented further down for older installs.
- README, INSTALL, `docs/development.md` and AGENTS.md called the app
"the web UI"; they now say Ovis.
- The film renders to `site/media/ovis-film.{mp4,jpg}` instead of
`herdr-web-ui-film` (`scripts/film/render.ts` and the doc).
`site/media/` is not committed and nothing else names the file.
- `install.ps1`: the installer's internal Add-Type job type is now
`Ovis.GhJob` instead of `HerdrWebUi.GhJob`, mirrored in
`scripts/windows-install.test.ps1`, which splices the `Assign` call out
of the installer's text. The type lives only in the PowerShell session,
so nothing persisted changes.
- `telemetry/README.md`: names `OVIS_TELEMETRY_URL` as the variable that
does not exist on purpose, and says why the Worker and D1 database keep
the `herdr-web-ui-telemetry` name.
Kept on purpose:
- herdrweb.dev redirects and installer/demo URLs.
- Legacy plugin id `devswha.herdr-web-ui`, `HERDR_WEB_*` env names and
pre-rename paths, documented for upgrades.
- Remote bundle and bridge paths
(`~/.local/share/herdr-web-ui/remote-vN`, `%LOCALAPPDATA%\herdr-web-ui`,
`~/.config/herdr-web-ui/bridges`), a cross-version contract.
- The deployed telemetry Worker and D1 names, and `telemetry/worker.ts`,
where any edit needs a redeploy.
- ovis.sh's "formerly herdr web UI" notice with its translations, the
JSON-LD `alternateName` (`scripts/ovis-site.test.ts`), verbatim
testimonials and the `llms.txt` rename note.
- Release notes.
- AGENTS.md's real `herdr-web-main.service` unit and
`~/.herdr/worktrees/herdr-web-ui*` dirs.
Not touched to avoid conflicts: `docs/guide.md` lines 161 and 178 ("the
web UI's GJC extension") sit inside hunks of open PRs #1652, #1650,
#1647 and #1442.
No `src/` or `server/` file changes, and nothing changes behaviour, so
there is no changelog fragment.
## Validation
- `bun run check fast` passes: workflow syntax, generated types and
themes, typecheck, build, and 6253 unit tests with 0 failures.
- `git grep -i -E 'HerdrWebUi|herdr-web-ui-film'` finds nothing.
- The `install.ps1` type rename is covered by
`scripts/windows-install.test.ps1` in the required Native Windows
install job.
…andidate (#1790) ## Change Refs #896. When a Codex TUI was resumed through the `codex resume` picker (`codex resume`, `codex resume --all`) from another folder, Codex Chat stayed on "Conversation unavailable". **Root cause, reproduced on Linux.** I used real Codex 0.163.0 with an isolated herdr session and an isolated `CODEX_HOME`. A thread created in folder B was picker-resumed from a pane in folder A. `codexTranscriptPath()` returned `null` for that pane, while a control pane in A running `codex resume <thread-id>` resolved the same rollout. Every signal the lookup trusts is missing for the picker pane: - herdr reports no `agent_session`. - Since Codex 0.157 the shared app-server daemon holds the rollout open, not the TUI, so the pane's processes hold no rollout. - The picker puts no thread id on argv, so `resumedThread` has nothing. - The screen fallback only considers threads of the pane's own cwd (A). The resumed thread's cwd is B. The daemon's logs tie threads to app-server connection ids, but nothing ties a connection to a TUI pid. `thread/loaded/list` lists loaded thread ids without their clients (codex-rs rust-v0.163.0). **Fix: the title offers a candidate, and the screen decides.** This follows the maintainer's call on the issue: exact signals only, no matching by name, cwd or recency. #717 was held for guessing among other folders' threads. Codex's opt-in `thread-id` terminal-title item ("Current thread identifier", in `[tui].terminal_title`) shows the current thread's id, and herdr reports the title. Codex 0.163.0 shortens it to 29 characters, as in `... | b | 01a16b42-3c88-7a30-91b3-772fb...`. - `titledThread()` takes the id from herdr's `terminal_title_stripped` (or `terminal_title`) only when exactly one of Codex's `" | "`-separated title items is exactly a whole id or Codex's shortened form (29 characters + `...`). An id inside other text, or shown twice, is not taken. - `titledCandidate()` turns it into the rollout of the one unarchived thread whose id starts with it. Three conditions apply: - a Codex process runs in the pane; - the same text appears neither in the pane's `cwd`/`foreground_cwd` nor in any stored thread's `cwd`, `name` or `title`, which a project-name or thread-name item could show (R1, merge pass); - the rollout passes `codexRolloutPath`, which keeps it inside the store and interactive. - `codexTranscriptPath()` adds that rollout to the screen-match candidates. **It is never taken on the title alone**, so a folder or thread name that only looks like an id cannot pick a conversation. - Three guards keep the new candidate from making matching less safe: - It counts only on an answer that no other candidate ever gave, in all of their parsed history, not just their last eight answers (`matchCodexTranscript`'s new `scrutinized` set). - When every answer it shows was given by another candidate too, the choice stays open (`null`), so a folder conversation cannot win by elimination either. - While it is offered, the cwd-only short-first-exchange fallback (`shortThreadFromProcess`) is skipped, because it would leave that competitor out. - Once the screen matches, the existing binding keeps the chat there. A binding made through the title holds only while the title still names that thread, in both `codexTranscriptPath` and `codexOpenRollout`. With Codex's default title (`activity`, `project-name` since 0.161), nothing changes. **Limits, also noted on #896.** - **Default title:** a picker resume from another folder still shows "Conversation unavailable". Users can add the item: ```toml [tui] terminal_title = ["activity", "thread-name", "project-name", "thread-id"] ``` - **Answer on screen:** the chat binds only once the screen shows one of that conversation's answers. - **After binding:** later `/new` detection still scans the pane's own folder only, as for every Codex binding today. - **Known residual (maintainer decision: document, not fix).** Competing conversations are read as their last 1 MiB, as every Codex candidate is today. A wrong pick needs all three of these: - the title carries a token that is not Codex's thread-id item but names another conversation, such as a folder or thread named with that conversation's UUID; - that conversation shares an answer with the pane's real one; - the real one's copy of that answer has been pushed out of its last 1 MiB, for example by a large inline image, while the screen still shows it. The fifth gate review reproduced this case with synthetic inputs. The changelog fragment explains the setting. `docs/guide.md` is edited by four open PRs (#1442, #1647, #1650, #1652), so its Codex note is left for a follow-up. **Design precedent:** none in the UI; only the server's transcript lookup changes. It keeps the bridge's rule for every agent: a chat binds only on positive evidence (herdr's session, a held file, an id on the command line, or the pane's own screen), otherwise the pane stays Terminal. ## Validation - **Unit tests:** 11 new cases in `server/codex-processes.test.ts`, run against the real SQLite store and rollouts behind the RPC fake. - **Resolves:** the title's thread from another folder, once the screen shows its answer; `codexOpenRollout` then returns the binding. It is also followed over the thread the command line resumed, as after `/new`. - **Stays `null`:** a UUID-valued name with nothing on screen, for both functions (with the pane's own answer on screen, its own thread wins); an answer this folder's conversation also gave, past its last eight answers and past 100 turns; an answer both conversations gave lately; a short first exchange the other folder's conversation shares; an answer another conversation split into parts; R1 (the title's id shown through a folder named after it while the pane runs a third conversation); a title binding after the title names another thread; no id in the title, an id inside other text, a non-Codex cut or an id shown twice; a `bash` or empty foreground; an id the store does not hold. - **Before each fix:** each case that guards a fix failed before it. - `HERDR_TEST_MODE=unit bun test server/codex-processes.test.ts server/codex.test.ts` passes; 274 tests across `codex-processes`, `codex` and `conversation`. - `bun run check fast` passes: workflow syntax, generated types and themes, typecheck, build, and 6281 unit tests with 0 failures. - **Live QA**, real Codex 0.163.0 with an isolated herdr session and `CODEX_HOME`, one prompt. The B session's answer was visible after each resume, and only the daemon held the rollout: - Picker resume from A with the default title: `codexTranscriptPath` returned `null`. - With `thread-id` in `config.toml`: it returned B's rollout, and `codexOpenRollout` then did too. - This ran before the last three guards. They only change ambiguous cases, and in this run folder A had no conversation to compete. - **Gate review** (a different model) took five rounds. It is not approved: the fifth round's only finding is the 1 MiB residual above, left documented at the maintainer's decision. Every earlier blocker is fixed and covered by a test above: - trusting the title without provenance; - a UUID-valued name picking another thread; - an older shared answer, past eight answers and past 100 turns; - a shared recent answer won by elimination; - the cwd-only short fallback. - **Follow-up b7c710b** (merge-pass R1 and three CodeRabbit findings): approved by a gate review on a different model, which probed R1 variants (folder equal to the id, shortened id as the project name, a thread renamed to the id, the id in a parent folder) and the live title format. - **Follow-up f6a9e70** (merge-pass R2): names and folders are compared with the title as Codex writes them: its dropped characters, whitespace collapse, GNU Screen mapping and 240-character cap. A gate review approved it after two re-review rounds. - **Not run:** Windows and macOS.
|
Verified against main
Looks good for the maintainer's merge pass. Review prepared with a code-review agent and checked against the code. The merge decision stays with the maintainer's merge pass. |
|
Merge pass review-work report. This is an outside-contributor PR, and the maintainer approved merging it in this pass. The ext-pr-review lane verified it, and its comment is above. Overall: PASSED (docs only).
The guide's statement matches the code: LOW follow-up: "no released herdr accepts it yet" will go stale once herdr changes. |
## Change Follow-ups from #1399's review (#1445), for Command Code (`cmd`) panes. Per item: - **2, the footer check was strict** (`server/dialogs/cmd.ts`, `server/dialogs/live.ts`): a permission prompt now gets its card when its footer wraps (split hint, word or character wrap) or uses the ctrl+e / ctrl+y variants. These are the shapes the published `command-code` 1.88.0 renderer draws (read statically from the npm package; the CLI was never run or logged into). A stale or incomplete footer, or anything drawn below it, still gives no card. Normal status chrome is hidden while a permission dialog is open, so it cannot sit under the footer. - **LOW, an ordinary numbered menu was labelled an approval card**: a menu is an approval card only when its rows are Command Code's own Yes … No choices, so a theme picker with the same footer is not one. - **4, herdr's model stuck in the transcript cache** (`server/conversation.ts`, cmd metadata branch only): the served model follows the pane's current model, and the ETag changes with it. The response is a new object, so the cached page is never mutated. herdr's live model now also replaces a model recorded in the transcript, which is what the item asks for. This branch is only reached where herdr reports the pane's Command Code session, which no released herdr does yet (#1647), so the changelog entry says so. - **5, store validation copied from `server/pi.ts`**: both now use `server/transcript-store.ts`. Behaviour is identical, and the old names are re-exported. - **6, test gaps**: tests for the cmd metadata branch, for the keys each answer sends from every cursor position, and for refusing a stale card over HTTP (409 `prompt_changed`, no keys sent). Session resolution and the skills source were already covered. - **LOW**: the unused `CmdTranscriptOptions.home` is removed. **Not fixed** - **1, the card id covers everything above the rows**, so a card goes stale when output above an open dialog redraws, as before. Narrowing the id to the dialog was tried and reverted: two review rounds showed that command text (a heredoc) can imitate the dialog's frame, including its real layout (rule, blank line, heading, subtitle). Then two different commands got the same id, and a stale card could be answered. Doing this safely needs Command Code to expose the dialog, or a capture proving a boundary that command text cannot imitate. A test pins the safe behaviour. - **LOW, a multi-line command's title**: left as main draws it, for the same reason (it would need the same boundary guess). - **3, the shell's cwd won over the agent's**: already fixed on main by #1745. - **LOW, the comment saying the body is part of the id**: it is accurate with main's id, which covers everything above the rows, the body included. Fixes #1445 ## Verification - New tests fail on origin/main and pass here: footer shapes, the Yes/No menu check, the model and ETag refresh without a transcript append, and the answer keys. Tests from the reviews are kept as regressions. Heredoc and indentation changes give different ids, a quoted dialog frame in the real layout gives different ids, and file-permission card bodies equal main's. The stale-card route test gets 409 with no keys sent. - Single-file runs after the rebase onto main: `server/cmd.test.ts` 56 pass, `server/prompt.test.ts` 357 pass, `server/cmd-conversation.test.ts` 2 pass, `server/pi.test.ts` 31 pass, `server/dialogs/registry.test.ts` 55 pass, `server/dialogs/interaction-plan.test.ts` 23 pass, `server/dialogs/claude.test.ts` 13 pass. - Live check over HTTP, against a real headless herdr session of its own (not a fake socket) and the server's `/api/pane/prompt` routes. A pane reported as `cmd` drew Command Code's shell permission dialog at 80 columns, where the terminal wraps the footer mid-word (`… bypass all perm` / `issions (Docs ↗)`): - this branch: an approval card with Yes / Yes, don't ask again / No. origin/main: only the generic fallback card (↑ ↓ Enter Esc). - a theme picker with the same footer: this branch gives the generic menu card; origin/main labels it an approval card titled "Theme". - a line redrawn above the open dialog gives a new id; answering the old id gets 409 `prompt_changed` and no byte reaches the pane; answering the new id sends only Enter. - Reviews: - Round 1: omo-native-code-reviewer and Codex (gpt-6.1-sol, xhigh) each found a HIGH in the narrowed card id (a heredoc hiding the command prefix). - Round 2: both found more ways around the narrowing, and file cards showing the whole screen. That led to restoring main's id. - Round 3: both APPROVE with no HIGH or MEDIUM. - After the rebase (one import conflict in `server/pi.ts`, where main's #1899 added `fileURLToPath`): Codex (gpt-6.1-sol, xhigh) reviewed the rebase against the new main (#1899's store and agent directory code, #1893's cache ordering, #1890's dialog reader order) and APPROVED with no findings. - `bun run check fast` on this head: workflow syntax, generated types and themes, typecheck, build and unit tests ok (6800 pass, 0 fail, 13 skip). --------- Co-authored-by: mp <mp@local>
Change
docs/guide.mdlists Command Code's chat as a native session file"resolved through herdr". No released herdr resolves it.
cmdTranscriptPathreads the session herdr reports for the pane, as pidoes (
server/cmd.ts,session.source === "herdr:cmd"). herdr keeps areported session only for the agents in
is_official_agent_source(
src/agent_resume.rs), and Command Code is not among them, so thereport Command Code sends is dropped and
agent_sessionstays absent forthe pane. Measured on herdr 0.9.5 + Command Code 1.87.1:
Status, title and model arrive from the same reporter over the same
socket, so the pane looks fully integrated while its conversation stays
out of reach, and the row promised a chat that cannot appear. The bridge
reads the conversation only from the file herdr names — the alternatives
(newest file in the project directory, joining herdr's pane title to
<id>.meta.json) pick the wrong conversation when twocmdpanes sharea folder — so the pane opens in Terminal.
This writes that limit where the row points, in the shape the Devin row
already uses for its own case. It also notes the part that does work: the
permission prompt cards are read off the screen
(
server/dialogs/cmd.ts), not from the session file, so they answer fromthe chat either way.
No code change.
server/cmd.tsis right; the herdr half was never built.Upstream ask with the exact contract gap: herdrdev/herdr#3333
(herdrdev/herdr#3333 (comment))
Validation
Docs only — no
src/,server/orshared/change, so perAGENTS.md this PR asks for no CodeRabbit review.
bun test scripts/changelog-fold.test.ts scripts/conventions.test.ts— 21 pass, 0 fail.docs-only commits have none (
302b9eae,4555859d).agent chips list Command Code as an agent the app works with, which
stays true (terminal, status, alerts and prompt cards all work); only
the guide asserted the chat resolves.