Repository navigation
fix(agents): offer installed Copilot in new-session menus - #1650
Merged
Merged
Conversation
Keep the deployed telemetry wire format unchanged: Copilot continues to use the existing other category. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1fad73aa-cbc5-42dd-916b-64e5198bc512
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the
 |
| Layer / File(s) | Summary |
|---|---|
Agent registration and usage shared/supported-agents.ts, shared/skill-syntax.ts, src/lib/api.test.ts, src/lib/agentLaunchMode.test.ts, DESIGN.md, docs/guide.md, changelog.d/copilot-agent-menu.md |
The supported agent list and skill syntax include Copilot. Tests cover agent-menu filtering and launch prompt handling. Documentation and the changelog describe Copilot availability in workspace and tab flows. |
Telemetry categorization server/telemetry.test.ts |
Tests map Copilot to the other harness category and verify that category is sent for a Copilot visit. |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix
Suggested reviewers: devswha
Merge Risk: 
c5167
-
No actionable regression remains in the reviewed Copilot registration, picker behavior, launch preparation, or telemetry mapping.
Pre-merge checks: 
-
Check name Status Explanation Title check The title clearly identifies the main change: offering installed GitHub Copilot in new-session menus. Docstring Coverage No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 5… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Description check The description includes the required Change and Validation sections. It clearly explains the behavior change, scope, compatibility considerations, related work, and validation results. It also identi… Full details: Docstring Coverage
-
Explanation
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 5 files. (3 skipped: 3 unsupported.)
Full details: Description check
-
Explanation
The description includes the required Change and Validation sections. It clearly explains the behavior change, scope, compatibility considerations, related work, and validation results. It also identifies unrun Windows and upstream CI checks.
-
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- 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.
Comment @coderabbitai help to get the list of available commands.
This was referenced Oct 22, 2026
devswha
added a commit
that referenced
this pull request
Oct 23, 2026
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.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1fad73aa-cbc5-42dd-916b-64e5198bc512
devswha
added a commit
that referenced
this pull request
Oct 23, 2026
## 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.
devswha
added a commit
that referenced
this pull request
Oct 24, 2026
…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.
devswha
added a commit
that referenced
this pull request
Oct 24, 2026
…#1800) ## Change Fixes #1628. This picks up #1629, which was closed only to wait for the terminal refactor; that refactor moved the code to `src/lib/useTerminalMount.ts`. In Direct typing, a long IME composition (Gboard's voice typing sends one, growing with every word) made the browser scroll the terminal host sideways to keep xterm's helper caret in view. The whole grid slid off to the left and stayed there. On main at 412 px wide, `.pane-terminal.scrollLeft` became 181. #1534 added `setTextAreaViewportBounds`, which confines the helper textarea and the preedit to the visible mount. `syncInputViewport` applied it only to mirrored, observed and followed grids, and a grid this browser drives kept stock xterm. It now applies the same bounds to every grid. A driven grid fits its mount, so for ordinary typing the bounds change nothing. They only stop the sideways scroll and keep the preedit inside the visible terminal. **Tradeoff:** a preedit longer than the room to the right edge is now clipped at that edge, using #1534's own clipping. Before, the grid scrolled to show it whole. The committed text still reaches the pane in full. **Design precedent:** this follows #1534's rule in DESIGN.md ("its input caret and IME preedit stay inside the visible mount"), which already rests on the comparison with Zed's terminal, now for every grid. DESIGN.md is changed by four open PRs (#1427, #1650, #1714, #1795), so its paragraph, which still names only mirrored and observed grids, is left for a follow-up instead of being edited here. ## Validation `scripts/terminal-composition-regression.ts` is #1629's script, run by `scripts/ui-contexts-checks.ts`, with its storage key and temp names moved to the Ovis names the conventions test requires. It covers 412 and 720 px wide, Input line and Direct typing, and the default font and the bundled "JetBrains Mono Web". After a long `Input.imeSetComposition`, it checks that: - the page and `.pane-terminal` stay at scroll 0; - the composition box ends inside the grid; - the grid is the pane's width, less padding and less than one cell. Results: - On main it fails with `412 direct default font: the terminal is not pushed sideways (181)`. With this change it passes all 8 cases. - These pass under one `bun run check run --build` (isolated herdr): `ui-contexts-regression.ts` (98 PASS, the new check included), `ui-regression.ts` (72), `terminal-input-regression.ts` (3, with its composition checks) and `mirror-scroll-demo-regression.ts` (3). - `bun run check fast` passes: typecheck, build, and 6308 unit tests with 0 failures. - A gate review on a different model approved it. Its notes: the check emulates a phone at both widths and does not wait for the bundled font to load, and layout cost was not measured. - **Not run:** a physical phone keyboard; this was exercised through CDP IME events only.
devswha
added a commit
that referenced
this pull request
Oct 24, 2026
## Change Fixes #1559. It picks up #1560, which was closed only to wait for the terminal refactor, and follows the design the maintainer set on the issue. The desktop terminal ignored herdr's `[ui] copy_on_select`, and with it off a selection could not be kept: - Letting go of a drag always copied. - herdr's attach stream turns on any-motion mouse reporting. xterm.js clears its selection on every report, so the first hover after the release dropped the highlight and the **Quote in message** offer. The next Ctrl+C then sent `^C` to the pane. **Behaviour**, as herdr's TUI does (`mouse.rs` and `input.rs`, quoted on the issue): - **Copy on (herdr's default):** unchanged. Letting go copies, and the first pointer move clears the copied highlight, so Ctrl+C interrupts again and the next click reaches the program. - **Copy off:** letting go only selects, and the clipboard is not touched. While a selection exists, buttonless moves over the grid stop before xterm, so the highlight and the quote offer stay until a press, a scroll or a key. Ctrl+C or Copy then copies herdr's full range, including rows scrolled out of view, as before. - **An explicit copy made before herdr has sent the drag's text** (Ctrl+C, Cmd+C or context-menu Copy) reserves a clipboard write for that text in the same gesture. Before, the copy's own clearing of the selection dropped herdr's answer and left only the visible rows. Native copy still writes the visible rows at once as the fallback. This also covers Ctrl+C right after a release in copy-on mode, where the release's reserved write was cancelled the same way. The gate review found this with copy off. As the design on the issue says, leaving the pane before herdr answers cancels that explicit copy, and the clipboard keeps the visible rows. Leaving counts any change of the terminal's pane, or of whether it is active, presented, shown or the chat view, even when the user comes back. The hook counts these on each commit, so `PaneTerminal.tsx` is unchanged. The copy made on release still finishes as on main. - **Until the preference is known,** letting go copies nothing. A failed read is tried again when the terminal next mounts, when the browser comes back online, and on the first frame after a reconnect. Only an older server, which answers 404 `not_found` for the route, means herdr's default (on). **Server** (the small contract additions this PR owns): - `GET /api/herdr-ui` returns only `{ copy_on_select: boolean }` (`HerdrUiConfig` in `shared/protocol.ts`), with `no-store`. It is read again on each request, behind the token gate. A watching device may read it, because read-only devices select too. Remote PCs need nothing: the preference is the web UI host's, for every pane, like the sidebar config. - `server/herdr-ui.ts` reads it through `readHerdrConfig()`, which is now exported from `server/herdr-sidebar.ts` and shared with the sidebar rows. It is the sidebar's existing safe read: - regular file only, opened non-blocking (a FIFO can't block the event loop); - 1 MiB cap; - herdr's line-start BOM stripping. Anything missing, unreadable, not TOML, or not a boolean gives herdr's default. - The demo transport answers the route. **Not handled** (from the second review on the issue): - When another setting is invalid, herdr ignores the whole config, so herdr copies (all defaults) while this reads `false`. herdr shows its own parse error then. - A `HERDR_CONFIG_PATH` set only in the user's shell is not seen by the service, the same as for the sidebar rows. **Design precedent:** herdr's own TUI, as traced on the issue: copy on release only with `copy_on_select` on, no hover moves to a pane that holds a selection, and Ctrl+C copies a kept selection. #1560 cited Zed's configurable terminal copy-on-select. This also corrects #1560's regression the maintainer found: in copy-on mode the selection is no longer kept through hover. DESIGN.md is changed by four open PRs (#1427, #1650, #1714, #1795), so the behaviour is written in `docs/guide.md` and the changelog fragment instead. ## Validation - **New `scripts/terminal-copy-off-regression.ts`,** run by `ui-contexts-checks.ts`. It runs its own server with a temp config set to `copy_on_select = false`, and the pane runs on the isolated test herdr. - **Checks:** the route answers `false`; a drag shows **Quote in message** but leaves the clipboard unchanged with no "copied" note; four hover moves keep the selection; Ctrl+C copies it. - **Second case (event-controlled):** it holds herdr's `/api/pane/selection` answer until Ctrl+C has been pressed, then answers and waits until no clipboard write is pending. The clipboard must hold herdr's text. Without the explicit-copy fix it held only the visible rows. - **Third case:** the same, but Ctrl+C is followed by a click on another pane's sidebar row, and herdr answers only once that pane's text shows. The clipboard must keep the visible rows. Without the cancellation it held herdr's text. - **Failed before the fix** in both pre-fix states: - main's terminal: there is no preference read; - a variant that reads the preference but keeps main's copy behaviour: `with copy_on_select off, letting go of a drag copies nothing`, because the clipboard held the dragged text. - **`scripts/terminal-copy-regression.ts`** (copy on) now waits for the preference answer (`true` with no config) and still passes every case: plain drag copy, Ctrl+C, scrolled drag over herdr's range, denied clipboard, and stale or empty responses. - **Under one `bun run check run --build`:** - `ui-contexts-regression.ts` passes (100 PASS, both copy checks included); - `terminal-arrows-clicks-regression.ts` passes; - the route's contract test passes: 401 without a token, 200 with `no-store`, re-read after a config edit, 400 for POST; - the watch-device contract passes, and a watch device reads the route. - **Unit tests:** - `server/herdr-ui.test.ts` covers values, CRLF, line-start BOMs, invalid values, other tables, missing and directory paths, an oversize file and a FIFO; - `src/lib/herdrUi.test.ts` covers copying off until known, the 404 fallback, failed reads staying unknown, kept values and one read in flight; - `server/herdr-sidebar.test.ts` still passes after the shared read was extracted. - **`bun run check fast`:** 6331 pass, 0 fail.
devswha
reviewed
Oct 24, 2026
devswha
left a comment
devswha
left a comment
Owner
There was a problem hiding this comment.
Small and correct as code. Copilot starts through Herdr’s generic launch path as a bare copilot, with no Claude or Codex permission, model, or effort flags. Keeping telemetry as other is correct because the deployed Worker does not list a Copilot category. Fast checks and focused tests pass on this head.
A few points before merge:
- This changes a documented rule; it does not fix a bug. #994 narrowed the picker because most Herdr agents show only terminal text here. Every kind currently in
SUPPORTED_AGENT_KINDShas a native Chat reader and command catalog; Copilot would be the first terminal-only entry. Gemini CLI, Cursor, Kimi, Amp and Droid would qualify by the same rationale. That is a product-scope decision. If Copilot remains in this list, please clarify the constant’s comment and file the changelog under### Added. - Keep the telemetry test’s instruction. Its revised assertion loses the prior actionable message telling contributors to add a new harness and deploy the Worker. With another kind such as Kimi added, the test now fails without explaining the fix. Please keep the prior message for unlisted harnesses and name the Copilot exception.
- DESIGN.md placement. The merge placed the new sentences in the login-shell PATH paragraph before “The picker itself is unchanged,” making that sentence read as if it referred to Copilot. Nothing in the dialog’s design changes; please remove those sentences or move a concise list explanation to the end of the paragraph that describes the picker.
- Guide row. The Copilot Chat cell includes launch steps that no other row has. “The terminal’s text” matches the existing “Anything else” row. The same list feeds New worktree, so the changelog should mention that dialog too, or simply say “the agent picker.”
The new test additions in agentLaunchMode.test.ts and telemetry test (i) pass without the change and are redundant; they are not blockers. The DEFAULT_SKILL_SYNTAX entry has no effect until a Copilot command catalog exists, but is consistent with the current map contract.
I did not verify a real first message sent by the dialog to Copilot. Herdr’s own Copilot prompt path sends focus-gained first, while this bridge’s pending paste path does not; the newly created pane should still be focused, but an end-to-end check would settle it. The CI run itself is safe to approve: this PR adds only unit tests and shared constants, and does not change workflows, scripts, dependencies, runner routing or permissions.
devswha
reviewed
Oct 24, 2026
devswha
left a comment
devswha
left a comment
Owner
There was a problem hiding this comment.
The code is small and correct. Copilot starts through herdr's own agent.start as a bare copilot with no arguments, so no Claude or Codex mode, model or permission flag reaches it. Keeping it as other in telemetry is right, because the Worker refuses a harness it does not list. bun run check fast passes on this head (6248 pass, 0 fail), and it merges cleanly with current main.
Before merge:
- This is a scope change, not a bug fix (
shared/supported-agents.ts). #994 limited the picker on purpose to the agents in the guide's Supported agents table. Every one of them has a native Chat reader, anddocs/agent-commands.mdcalls the list "the agents the app treats as its own". Copilot would be the first agent there whose Chat is terminal text only. Gemini CLI, Cursor, Kimi, Amp and Droid would then qualify by the same argument. That needs a maintainer decision before this merges, separate from #1652. If it goes in, please file the fragment under### Added, and update the constant's comment to say that the list can hold an agent whose Chat is terminal text. - Keep the telemetry guard's instruction (
server/telemetry.test.ts, test (h)). The rewrite drops the message that told the next contributor what to do. Suppose a later PR addskimito the list: (h) then fails witherror: kimi/Received: false, where it used to say "add it to TELEMETRY_HARNESSES and deploy the Worker". Please name the exception, for exampleconst COUNTED_AS_OTHER = new Set(["copilot"]), and keep the old message for every other kind. - DESIGN.md placement. The merge with main put the two new sentences inside the login-shell PATH paragraph, just before "The picker itself is unchanged.", so that sentence now reads as if it were about Copilot. Nothing in the dialog's design changes, so please drop both, or keep one sentence after "shell-only is always available".
- Guide row (
docs/guide.md). The Chat cell has launch steps that no other row has. "The terminal's text" is enough, as for Anything else. The same list also feeds New worktree (WorktreeDialog.tsx), so the changelog's "new workspaces and tabs" leaves one out.
Nits:
agentLaunchMode.test.tslines 27-28 and telemetry test (i) pass on main without this change, and (i) repeats what (h) checks.copilot: DEFAULT_SKILL_SYNTAXis fine. It has no effect until Copilot has a command catalog.
Not verified: a first message typed in the dialog reaching a real Copilot. For Copilot, herdr's own agent.prompt sends focus-gained before Enter, and the bridge's paste and Enter does not. A pane created with focus: false has never lost focus, so it should work, but one manual run would settle it.
CI: the PR touches no workflow, script, package.json or lockfile, so approving the fork run is safe.
Review prepared with a code-review agent and checked against the code. The merge decision stays with the maintainer's merge pass.
devswha
added a commit
that referenced
this pull request
Oct 24, 2026
## Change herdr 0.9.5 already detects Letta Code (agent kind `letta`, from its `letta.toml` detection manifest), and `GET /api/agents` lists it. The app's supported-agent list left it out, though, so **New workspace**, **New tab with agent…** and **New worktree** hid an installed `letta`. This PR adds `letta` to `SUPPORTED_AGENT_KINDS` along with: - the label **Letta Code** in `/api/agents` (`server/agents-routes.ts`); - a skill-syntax row. Letta's slash menu opens only on a leading `/` and lists skills as bare `/<skill-id>` rows (letta-code 0.34.23, `src/cli/components/SlashCommandAutocomplete.tsx`); - a Supported agents row in the guide and a changelog fragment. There is no new UI: the existing picker lists one more agent, with the fallback letter mark (lobe-icons has no Letta mark). The launch uses the existing generic path and adds no permission flags. A first message waits for the first ready turn, because Letta takes a prompt on its command line only with `-p` (its headless mode). **Telemetry:** the deployed Worker refuses a harness it does not list (`telemetryHarnesses`, `telemetry/deployed.json`), so Letta counts as `other`. Test (h) in `server/telemetry.test.ts` lists it as not yet deployed instead of adding it to `TELEMETRY_HARNESSES`, so no Worker deploy is needed. **No native chat reader yet.** In its default mode, Letta keeps the conversation on the Letta server. Locally, `~/.letta/sessions.jsonl` holds only session metadata. Chat shows the terminal's text until a follow-up reads it. The storage findings are in #1730 (comment). Part of #1730 (the picker; the issue stays open for the reader, prompts and session history). **Scope notes** - Untouched: `server/conversation.ts`, the claude stores and `shared/protocol.ts` (open #1813), and `server/dialogs/` (open #1814). - The outside Copilot PR #1650 edits the same lines in `shared/supported-agents.ts`, `shared/skill-syntax.ts`, `docs/guide.md`, `server/telemetry.test.ts`, `src/lib/agentLaunchMode.test.ts` and `src/lib/api.test.ts`. Whichever PR lands second needs a small rebase that keeps both entries; in the telemetry test that means `undeployed = new Set(["copilot", "letta"])`, or #1650's mapping. ## Validation - `bun run check fast`: passed on the committed tree (workflow syntax, generated types and themes, typecheck, build, unit tests with 0 failures). - Focused tests (the touched files, `scripts/changelog-fold.test.ts`, every `conventions.test.ts`): 115 passed, 0 failed. `server/agents-routes.test.ts` checks the label. `src/lib/api.test.ts` checks that Letta is offered where installed, hidden where `installed: false`, and offered from an older bridge that sends no installation metadata. - Browser check on the demo build over its fixture transport (no herdr session; `/api/agents` and the create requests intercepted, so nothing launched), 4 of 4 passed: - **New workspace** offers Letta Code and not an unsupported kind, and Start sends `{"agent":{"kind":"letta"}}`; - **New tab with agent…** does the same through `POST /api/tab/create`; - with `installed: false`, Letta Code is not offered; - the picker at 1440×900 and 390×844 (dark) shows the Letta Code row unclipped. - A separate code-review agent found no blockers. Its two fixes are applied: a test assertion that passed without the change was dropped, and the guide and changelog now name New worktree too. Not run: launching a real `letta` (it is not installed here), so a first message sent to a live Letta pane has not been tried end to end. The generic pending path holds such a message until herdr reports the pane ready, and the user can always send or discard it.
Owner
|
Heads-up from the merge pass: #1815 (Letta Code in the new-session menus) just merged to main, and it now conflicts with this PR in two places. Both conflicts are entries added on the same lines.
export const SUPPORTED_AGENT_KINDS: ReadonlySet<string> = new Set(["claude", "codex", "copilot", "omp", "omo", "gjc", "pi", "opencode", "devin", "grok", "hermes", "agy", "letta"]);
Nothing else in #1815 overlaps your changes. |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1fad73aa-cbc5-42dd-916b-64e5198bc512
Contributor
Author
|
Addressed the launcher review in the latest commit:
Validation: focused launcher/telemetry suites (52 passed), full I agree this is a product-scope addition, not a regression fix: approval to offer a terminal-only agent in the picker remains your decision. This PR is still independent of native Chat support in #1652. |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1fad73aa-cbc5-42dd-916b-64e5198bc512 # Conflicts: # docs/guide.md # server/telemetry.test.ts # shared/supported-agents.ts
Owner
|
Re-checked the follow-up at
The branch merges cleanly with main The code is ready. What remains is the product question: Copilot would be the first agent in the picker whose Chat is terminal text only. That decision is the maintainer's, so this stays with them. Review prepared with a code-review agent and checked against the code. The merge decision stays with the maintainer's merge pass. |
Owner
|
The maintainer agreed to add Copilot CLI to the picker as a terminal-only agent, the same way Letta Code went in (#1815). I finished it on your branch, keeping your commits:
Verification on
Thanks @YuuuXie. This goes to the merge pass after the current release PRs. Review prepared with a code-review agent and checked against the code. The merge decision stays with the maintainer's merge pass. |
Owner
|
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.
Copilot CLI joins the new-session menu as terminal-only, the same way Letta does. The CodeRabbit did not review this PR (rate-limited or skipped). The reviews above stand in for it. |
devswha
added a commit
that referenced
this pull request
Oct 25, 2026
## Change Native Copilot CLI Chat reader for #1574. **Maintainer agreement on the feature and the broader process-log binding design is still pending.** This PR is open for review, not a claim of approval. **Launcher #1650 is now merged upstream** as `c952eb58`. This PR is synchronized directly with `main` at `e430be78` and inherits the launcher there; no separate launcher integration or stack is needed. The guide updates the existing upstream **GitHub Copilot CLI** row instead of showing duplicate native/terminal-only rows. On Linux, the pane's verified foreground Copilot process selects its native `events.jsonl` from its own configuration home. Its canonical open CLI log can identify the foreground session through lifecycle registrations, including in-process resume/fork/switch-back. A trusted Herdr report plus a unique process-held session marker is a restricted fallback when no process log has been observed, or the complete initial log has no lifecycle events. Invalidated, deleted, unreadable, partial or discontinuous log evidence does not unlock that fallback: a fresh registration is required. File recency and launch `--resume` arguments never select history by themselves. Text, tools, recorded model/effort, recorded `ask_user` choices and root task-completion summaries are displayed. Interactive questions and approvals remain in Terminal. No guessed selection keys, automatic hook installation or user configuration changes. macOS and Windows retain Terminal. See `docs/copilot-chat.md` for setup and limitations. ## Review revisions and consolidation - Checkpoint complete lifecycle-log reads even when no selection is available; recover from ambiguous unregister records on a later registration without enabling stale fallback. - Skip vanished unrelated descriptors; retain observed-but-invalid evidence after an incomplete log closes. Bound that state by verified process lifetimes rather than evicting live denials. - Deduplicate a completion summary matching the last unphased/commentary reply, promoting that text to a final answer. - Resolve foreground process evidence once **per validation pass**, rather than rescanning through separate logged-session, transcript and unwritten-session wrappers. The typed result distinguishes a lifecycle selection, with its required generation, from a held-marker fallback. - Gather peer claims once per resolution. Both conversation and tool-output reads deliberately perform a fresh ownership and peer check after reading. - Consolidate tool-part construction, Linux log-fixture processes and HTTP fixture startup: **106 net lines removed** in `bee51d62`, retaining ownership, rewrite/gap, ambiguous unregister, incomplete/closed log, peer isolation and stale cursor/tool-reference scenarios. The completion-summary test covers both request and execution-start records. - Extract setup and limits into `docs/copilot-chat.md`, correct fallback promises, use one Added changelog fragment and remove the unrelated Tailscale documentation edit. - Remove launch-only bookkeeping and the synthetic launch placeholder. Verified process-owned unwritten sessions still show an empty Chat; unavailable sessions avoid the redundant launch-placeholder process query. - Preserve upstream conversation-module extractions and the pane-specific PI agent-directory handling when integrating current main. The reusable production fixes (`fc3ca004` continuity and `260cfc0a` lifecycle-free fallback) are ancestors of this PR. Personal deployment tooling and one-off runtime metadata repairs are not imported. ## Transient recovery After a temporary unavailable response, Chat uses its existing 2 s polling cadence instead of retaining the 10 s native-push backstop. A verified native response restores the ordinary cadence without switching lenses. Unverified history is still hidden immediately; no stale-session grace period is introduced. The owned sandboxed browser regression failed at the 4 s automatic-recovery assertion before `6573986d` and passed afterward. This establishes a frontend recovery delay, **not** the backend trigger for every reported live occurrence, and does not solve missing lifecycle evidence. UI reference: T3 Code's [connection runtime](https://github.com/pingdotgg/t3code/blob/main/docs/internals/connection-runtime.md) separates transport health from data freshness. Ovis uses its existing conversation scheduler rather than a new reconnect loop; no code or UI controls were borrowed. The difference is documented in `DESIGN.md`. ## Validation Owned fictional fixtures only; no live-session prompts or private transcripts/logs were published. - Full Fast checks at `038ded8c`: **6,798 passed, 0 failed, 13 skipped**, plus workflow syntax, fresh generated types/themes, typecheck and build. - Isolated HTTP contract: **3 passed, 61 assertions**, including verified unwritten sessions, resumed history, peer isolation and cursor/tool-output invalidation. - Sandboxed built React/HTTP browser fixture passed: tool details, appends, desktop/mobile, PTY input and automatic recovery without switching lenses. - Independent reviews found no significant issues in the recovery fix, consolidation and upstream integration. Local browser success is not a claim that the hosted browser lane passed. Correction retained: hosted run `39829635346` included a browser failure; hosted results for each new head must be assessed separately. ## Still open for discussion - Approval of lifecycle-log selection beyond the original hook-only proposal. - Browser/environment compatibility with sandboxing enabled. The requested `--no-sandbox` change is not implemented. - Literal once-per-HTTP-request peer gathering is not implemented: fresh post-read ownership checks deliberately repeat the peer sweep. - Partial trailing lifecycle lines remain conservatively unavailable rather than serving a potentially stale complete prefix. - A redacted real-format lifecycle/background fixture and broader native subagent-format evidence remain outstanding; current fixtures do not establish universal marker availability. Observed CLI logging behavior is not a compatibility guarantee for every release. The current log-based design still falls back to Terminal when log rewriting removes foreground evidence; these revisions do not claim to solve that underlying limitation. --------- Co-authored-by: Yu Xie <yuxie1@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: mp <mp@local> Co-authored-by: devswha <devswha@gmail.com> Copilot-Session: 1fad73aa-cbc5-42dd-916b-64e5198bc512 Copilot-Session: 064cd7db-0ee7-4c99-ab55-74aad409d4b1
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Change
Installed GitHub Copilot CLI is detected by
/api/agents, but the frontend's supported-agent allowlist omits it. Consequently, New workspace and New tab with agent... hide an installed executable that herdr can already launch.This PR adds Copilot to the existing picker and retains the existing generic launch path and permission defaults. It includes an explicit default skill-syntax entry and keeps Copilot in the deployed telemetry receiver's existing
othercategory; no telemetry schema or Worker deployment is needed.This is an availability fix to the existing selected-PC picker, not a new UI design: no new controls, settings, copied third-party code, or chat transcript reader are introduced. Local and remote installation filtering keep their existing behavior, including compatibility with older bridges without installation metadata.
Native Copilot chat support is deliberately separate in #1652, linked to the scope discussion in #1574 and explicitly submitted without prior maintainer agreement. This launcher fix does not depend on that work. Without a native reader, Copilot continues using the existing terminal-text fallback.
Based on upstream
mainatd5a35b7a.Validation
UNIT_TEST_WORKERS=2 bun run check fast): passed, including workflow syntax, generated files, typecheck, production build, and 6,068 unit tests (0 failed, 12 skipped).agent.kind: "copilot". Creation requests were intercepted so this check did not launch a real agent or contact a model.Validation was performed on Linux/WSL with Bun 1.4.3 and herdr 0.9.5, using an isolated test session. Native Windows install checks and maintainer-approved upstream CI have not been run locally.