chore: remove temp design documents
This commit is contained in:
Bohdan Triapitsyn
@@ -1,150 +0,0 @@
|
||||
# Settings Item Search Plan
|
||||
|
||||
## Goal
|
||||
|
||||
Add Settings search that finds individual settings items, not only top-level pages.
|
||||
|
||||
The search should behave like this:
|
||||
|
||||
- User types a query in the Settings navigation area.
|
||||
- Results show matching concrete settings, grouped or labeled by their Settings page.
|
||||
- Each result shows the item title and, when available, its description.
|
||||
- Clicking a result opens the correct Settings page.
|
||||
- After the page renders, the matching row/card/section scrolls into view.
|
||||
- The matched item gets a short visual highlight so the user can see where they landed.
|
||||
|
||||
## Current Architecture Notes
|
||||
|
||||
- Settings shell lives in `packages/ui/src/components/views/SettingsView.tsx`.
|
||||
- Page metadata and slugs live in `packages/ui/src/lib/settings/metadata.ts`.
|
||||
- Settings localization lives in `packages/ui/src/lib/i18n/messages/*.settings.ts`.
|
||||
- Settings UI text is read through `useI18n()` and `t(key)`.
|
||||
- Standard page wrappers live in `packages/ui/src/components/sections/shared/`.
|
||||
|
||||
## Proposed Architecture
|
||||
|
||||
Use an explicit searchable item registry instead of scraping React or the DOM.
|
||||
|
||||
Each searchable item should contain:
|
||||
|
||||
- `id`: stable item id, for example `appearance.language`.
|
||||
- `page`: target `SettingsPageSlug`, for example `appearance`.
|
||||
- `titleKey`: localized title key.
|
||||
- `descriptionKey`: optional localized description key.
|
||||
- `keywords`: optional non-visible search helpers.
|
||||
- `isAvailable`: optional runtime/mobile guard for item-level availability.
|
||||
|
||||
Example:
|
||||
|
||||
```ts
|
||||
{
|
||||
id: 'appearance.language',
|
||||
page: 'appearance',
|
||||
titleKey: 'settings.appearance.language.label',
|
||||
descriptionKey: 'settings.appearance.language.description',
|
||||
keywords: ['locale', 'translation', 'ui language'],
|
||||
}
|
||||
```
|
||||
|
||||
## Implementation Steps
|
||||
|
||||
1. Create `packages/ui/src/lib/settings/search.ts`.
|
||||
- Export `SETTINGS_SEARCH_ITEMS`.
|
||||
- Export a helper to build localized search results from `t()`.
|
||||
- Filter by page availability and `visiblePageSlugs`.
|
||||
|
||||
2. Add search UI to `SettingsView.tsx`.
|
||||
- Search input should live in the left Settings navigation area on desktop.
|
||||
- On mobile, keep behavior simple: show results in the nav stage and open target page on select.
|
||||
- When query is empty, keep the existing navigation list.
|
||||
- When query has text, replace the normal nav list with concrete search results.
|
||||
|
||||
3. Add click behavior for a search result.
|
||||
- Set `settingsPage` to the result page.
|
||||
- Store pending target item id in component state/ref.
|
||||
- After content renders, find `[data-settings-item="<id>"]`.
|
||||
- Scroll it into view.
|
||||
- Add a temporary highlight using a data attribute or CSS class.
|
||||
|
||||
4. Add a tiny shared anchor/highlight pattern.
|
||||
- Prefer adding `data-settings-item="..."` to existing row/card containers.
|
||||
- Avoid wrappers that change layout.
|
||||
- Keep highlight styling generic, for example a short ring/background transition.
|
||||
|
||||
5. Add initial searchable coverage.
|
||||
- Start with high-value pages that already use many localized strings:
|
||||
- `appearance`
|
||||
- `chat`
|
||||
- `sessions`
|
||||
- `notifications`
|
||||
- `git`
|
||||
- `providers`
|
||||
- `agents`
|
||||
- Add more pages incrementally.
|
||||
|
||||
6. Validation.
|
||||
- Run `bun run type-check`.
|
||||
- Run `bun run lint`.
|
||||
- Manually verify search result navigation for at least one single page and one split page.
|
||||
|
||||
## Current Implementation Status
|
||||
|
||||
Done:
|
||||
|
||||
- `packages/ui/src/lib/settings/search.ts` exists and exports the explicit registry plus localized result builder.
|
||||
- Search input is wired into `SettingsView.tsx`.
|
||||
- Results are grouped by page header.
|
||||
- ArrowUp, ArrowDown, Enter, and Escape work while the search input is focused.
|
||||
- Result click opens the target page and scrolls to `[data-settings-item="..."]`.
|
||||
- Matching target gets a temporary highlight via `data-settings-search-highlight`.
|
||||
- Search respects page availability, `visiblePageSlugs`, and item-level platform/runtime/mobile guards.
|
||||
- Initial anchors exist for `appearance`, `chat`, `sessions`, `notifications`, `git`, and `usage`.
|
||||
|
||||
Covered pages/items so far:
|
||||
|
||||
- `appearance`: themes, localization, PWA/mobile-only controls, layout controls, navigation controls, usage reports.
|
||||
- `chat`: render mode, transport, reasoning, layout/message toggles, mobile status bar, dotfiles, queue/draft/spellcheck.
|
||||
- `sessions`: defaults, retention, desktop network controls, OpenCode CLI controls.
|
||||
- `notifications`: delivery, events, background push.
|
||||
- `git`: GitHub account, identities, changes view, Gitmoji, gitignored files.
|
||||
- `usage`: header menu visibility, model quotas section.
|
||||
- `agents`: create action plus static editor fields for name, mode, model, temperature, Top P, system prompt, and permissions.
|
||||
- `commands`: create action plus static editor fields for name, agent, model, and template.
|
||||
- `mcp`: create action plus static editor sections for server, command/URL, environment variables, and advanced remote options.
|
||||
- `plugins`: add action plus static editor fields for spec, options JSON, and file content.
|
||||
- `snippets`: create action plus snippet content editor.
|
||||
- `providers`: connect action plus auth, connection details, and models sections.
|
||||
- `skills.installed`: create action plus basic information, instructions, and supporting files sections.
|
||||
- `behavior`: global AGENTS.md and response style sections.
|
||||
- `projects`: static project metadata fields and worktree section, excluding individual projects.
|
||||
- `skills.catalog`: source repository, catalog search, and add catalog action, excluding individual catalog skills/sources.
|
||||
- `magic-prompts`: visible prompt, instructions, and reset-all action, excluding individual prompt result generation beyond the selected editor page.
|
||||
- `shortcuts`: keyboard shortcut editor section.
|
||||
- `voice`: voice setup, speech recognition, and playback sections.
|
||||
- `tunnel`: provider, tunnel type, TTLs, managed remote/local configuration, and start/connect link sections.
|
||||
- `remote-instances`: client auth/pairing and desktop direct-host sections; SSH instance dialog fields stay out of search because they require selected-instance state.
|
||||
|
||||
Still pending:
|
||||
|
||||
- Add state-aware filtering for settings that are hidden based on current settings values, not just platform. Examples: `chat.activity-default-mode`, `chat.collapsible-reasoning`.
|
||||
- Add focused tests for `buildSettingsSearchResults`, especially runtime/mobile filtering.
|
||||
|
||||
Out of scope by decision:
|
||||
|
||||
- Do not generate search results from dynamic store entities such as individual agents, commands, MCP servers, snippets, plugins, skills, providers, or projects.
|
||||
- For split pages, search should cover predictable static create actions, editor fields, and sections only.
|
||||
|
||||
## Important Constraints
|
||||
|
||||
- Do not rely on localized key naming alone for navigation. The registry is the source of truth.
|
||||
- Do not parse JSX or scrape the DOM to discover settings automatically.
|
||||
- Search should use current locale strings, with English fallback already handled by i18n.
|
||||
- Do not introduce broad Zustand state for transient search query/highlight state. Keep it local to `SettingsView` unless another surface needs it.
|
||||
- Keep page behavior unchanged when the query is empty.
|
||||
- If a page is unavailable in the current runtime, its search items must not appear.
|
||||
|
||||
## Future Improvements
|
||||
|
||||
- Add fuzzy ranking instead of simple substring matching.
|
||||
- Support deep-linking to settings items from URLs or app commands.
|
||||
- Add complete registry coverage for all Settings pages.
|
||||
@@ -1,326 +0,0 @@
|
||||
# Preview — Remote-host relay (design)
|
||||
|
||||
Status: design only, no implementation.
|
||||
Owner: TBD.
|
||||
Audience: contributors planning the next phase of the embedded preview feature.
|
||||
|
||||
## Problem
|
||||
|
||||
The current preview implementation (`packages/web/server/lib/preview/proxy-runtime.js`,
|
||||
`packages/ui/src/components/layout/ContextPanel.tsx`) terminates inside the
|
||||
OpenChamber server process and forwards requests to a **loopback** target
|
||||
(`localhost`, `127.0.0.1`, `::1`, `0.0.0.0`). It works for these topologies:
|
||||
|
||||
| Topology | Works today? |
|
||||
| ------------------------------------------------------------------------ | ------------ |
|
||||
| Web UI in browser, OpenChamber server on same host as dev server | yes |
|
||||
| Electron desktop, dev server on same host | yes |
|
||||
| VS Code extension, dev server on same host | yes |
|
||||
| Mobile/tablet hitting OpenChamber over LAN, dev server on host | yes |
|
||||
| **Remote OpenChamber** (cloud / shared / tunneled), dev server on user's local machine | **no** |
|
||||
|
||||
The blocked case is real: a user runs `openchamber serve` on a remote box (or a
|
||||
hosted OpenChamber instance) but their dev server (`vite`, `next dev`, etc.)
|
||||
runs on their laptop. The proxy correctly refuses to talk to non-loopback
|
||||
targets — that is a deliberate SSRF gate, not a bug. We need a separate path
|
||||
that tunnels traffic from the remote OpenChamber back to the user's laptop
|
||||
without weakening that gate.
|
||||
|
||||
## Non-goals
|
||||
|
||||
- Replacing the existing loopback proxy. The local-loopback path is the common
|
||||
case and stays unchanged.
|
||||
- Acting as a generic public ingress for arbitrary local services. We only
|
||||
expose dev servers selected through the preview UI, scoped to the active
|
||||
user's session.
|
||||
- Providing a hosted relay service. The relay is something the user runs;
|
||||
OpenChamber provides the agent + the server endpoints.
|
||||
|
||||
## Constraints (carried forward from the loopback proxy)
|
||||
|
||||
- Same-origin in the browser. The iframe must load from the OpenChamber
|
||||
origin so HTTPS, cookies, and CSP behave predictably.
|
||||
- Per-target cookie auth. A target id must not be guessable, and the cookie
|
||||
must be HttpOnly + scoped to that target's path.
|
||||
- WebSocket upgrade support (HMR is a hard requirement; without it the
|
||||
feature is uninteresting).
|
||||
- Strip frame-busting headers on the response.
|
||||
- Strip OpenChamber credentials before forwarding to the dev server.
|
||||
- Survive partial failure cleanly: if the agent disconnects, the iframe
|
||||
should land on the existing "dev server is not responding" overlay, not a
|
||||
zombie hang.
|
||||
|
||||
## Architecture
|
||||
|
||||
Three components, in order of where they run.
|
||||
|
||||
### 1. Local agent (runs on the user's laptop)
|
||||
|
||||
A small process the user starts on the same machine as the dev server. Two
|
||||
shipping options:
|
||||
|
||||
- A subcommand of the existing CLI: `openchamber preview-agent`.
|
||||
- A standalone single-binary build for users who do not have the full UI
|
||||
installed locally.
|
||||
|
||||
Responsibilities:
|
||||
|
||||
- Open exactly one outbound, authenticated WebSocket to the remote
|
||||
OpenChamber server (`wss://<host>/api/preview/agent`). Outbound-only — no
|
||||
inbound port on the user's machine, so it works behind NAT, VPN,
|
||||
corporate firewall, etc.
|
||||
- Authenticate with a short-lived enrollment token issued by the remote
|
||||
OpenChamber server (see "Pairing flow").
|
||||
- Advertise the set of dev servers the user has authorised. Scope is
|
||||
loopback-only on the agent side (same allowlist as the existing proxy:
|
||||
`localhost`, `127.0.0.1`, `::1`, `0.0.0.0`). The agent never proxies to
|
||||
arbitrary hosts on the user's network.
|
||||
- Multiplex per-request streams over the single control WebSocket
|
||||
(frame protocol below). Each browser request becomes one logical stream.
|
||||
- Forward HTTP and upgraded WebSocket connections to the local dev server.
|
||||
- Send authoritative `agent-disconnected` notifications so the server can
|
||||
evict targets immediately rather than waiting for TTL.
|
||||
|
||||
Deliberately out of scope for the agent:
|
||||
|
||||
- TLS termination. The agent only talks to loopback over plain HTTP; the
|
||||
outbound link to OpenChamber is TLS via the server's existing cert.
|
||||
- Anything that mutates the user's filesystem.
|
||||
- Acting as a general SOCKS/HTTP proxy. It is dev-server-scoped.
|
||||
|
||||
### 2. Remote OpenChamber server (extends `proxy-runtime.js`)
|
||||
|
||||
Adds two new surfaces alongside the existing loopback proxy:
|
||||
|
||||
- `GET /api/preview/agent` (WebSocket): the single control channel an agent
|
||||
connects to after enrollment. Authenticated by the enrollment token + the
|
||||
user's UI session.
|
||||
- `POST /api/preview/targets/remote`: same shape as the existing
|
||||
`POST /api/preview/targets`, but the URL is interpreted **relative to a
|
||||
connected agent**. The body becomes
|
||||
`{ agentId, url, ttlMs? }` (or the existing endpoint accepts an optional
|
||||
`agentId` and dispatches to the right path). The response keeps the same
|
||||
contract: `{ id, proxyBasePath, expiresAt }`. The browser does not learn
|
||||
it is talking to a remote agent — that is a server-side detail.
|
||||
|
||||
The existing `/api/preview/proxy/:id/*` route is reused unchanged from the
|
||||
browser's perspective. Internally it now dispatches based on the registered
|
||||
target type:
|
||||
|
||||
- `kind: 'loopback'` (existing) → `http-proxy-middleware` to a local origin.
|
||||
- `kind: 'agent'` (new) → encode the request into a frame, push it onto the
|
||||
matching agent's WebSocket, await the response frames, stream them back
|
||||
to the browser.
|
||||
|
||||
This dispatch boundary is the only invasive change to the existing runtime.
|
||||
The factory stays `createPreviewProxyRuntime`; the agent registry, frame
|
||||
codec, and response streaming live in a sibling module
|
||||
(`packages/web/server/lib/preview/agent-runtime.js`) so the loopback path
|
||||
remains readable and individually testable.
|
||||
|
||||
### 3. Browser (UI layer)
|
||||
|
||||
Almost no change. `PreviewPane` already POSTs to `/api/preview/targets` and
|
||||
loads the iframe at the returned `proxyBasePath`. The remote case adds:
|
||||
|
||||
- A small "no agent connected" empty state when the user's profile has no
|
||||
active agent but tries to preview a non-public URL. Gives them the exact
|
||||
command to run and a one-click copy of the enrollment token.
|
||||
- The existing 502 / dev-server-down overlay handles agent disconnects too
|
||||
— the proxy returns 502 if the agent vanishes mid-request.
|
||||
|
||||
## Pairing / enrollment flow
|
||||
|
||||
The agent must prove it is acting on behalf of a specific UI user, and the
|
||||
server must be able to revoke that proof.
|
||||
|
||||
1. User opens Settings → Preview → "Connect a local dev-server agent".
|
||||
2. Server mints a short-lived (5 min) enrollment token bound to the user's
|
||||
UI session id, with a single allowed scope: `preview-agent.connect`. UI
|
||||
shows the command:
|
||||
```
|
||||
openchamber preview-agent --server https://<host> --token <enrollment-token>
|
||||
```
|
||||
3. Agent posts the enrollment token to `POST /api/preview/agent/enroll` and
|
||||
receives a long-lived `agentId` + `agentSecret`. Stored in the agent's
|
||||
config dir (`$XDG_CONFIG_HOME/openchamber/agent.json` or platform
|
||||
equivalent).
|
||||
4. Agent opens the control WebSocket, authenticating with `agentId` +
|
||||
`agentSecret`. The server verifies and registers the agent against the
|
||||
owning user.
|
||||
5. Agent sends an initial `hello` frame with: agent version, OS, hostname
|
||||
hint (display only — never used for routing), and a list of dev-server
|
||||
URLs the user has explicitly approved on the agent side.
|
||||
|
||||
Revocation:
|
||||
|
||||
- User can revoke an agent from Settings; the server invalidates the
|
||||
`agentSecret` and closes any open WebSocket.
|
||||
- The agent honours `disconnect` frames from the server with a clean
|
||||
shutdown.
|
||||
- Enrollment tokens are single-use and expire after 5 min.
|
||||
|
||||
## Wire protocol (control WebSocket)
|
||||
|
||||
Binary frames, little-endian, one frame = one logical operation. JSON metadata
|
||||
header followed by an opaque body. Designed to be implementable in Node and
|
||||
Bun without exotic deps.
|
||||
|
||||
```
|
||||
+--------+--------+--------+----------------------+----------------------+
|
||||
| u8 ver | u8 op | u32 len| metadata (JSON, len) | body (remaining) |
|
||||
+--------+--------+--------+----------------------+----------------------+
|
||||
```
|
||||
|
||||
Operations:
|
||||
|
||||
| op | name | direction | metadata | body |
|
||||
| ---- | ----------------- | -------------- | ------------------------------------------------------------------- | ----------------------------------- |
|
||||
| 0x01 | hello | agent → server | `{ agentVersion, hostnameHint, allowedTargets: [{origin}] }` | empty |
|
||||
| 0x02 | hello-ack | server → agent | `{ ok, serverVersion }` or `{ ok: false, reason }` | empty |
|
||||
| 0x10 | http-request | server → agent | `{ streamId, method, path, headers, originHint }` | request body bytes |
|
||||
| 0x11 | http-response-head| agent → server | `{ streamId, status, headers }` | empty |
|
||||
| 0x12 | http-response-data| agent → server | `{ streamId, fin: bool }` | response body chunk |
|
||||
| 0x13 | http-error | agent → server | `{ streamId, code, message }` | empty |
|
||||
| 0x20 | ws-open | server → agent | `{ streamId, path, headers, subprotocols }` | empty |
|
||||
| 0x21 | ws-open-ack | agent → server | `{ streamId, ok, status?, subprotocol? }` | empty |
|
||||
| 0x22 | ws-frame | both | `{ streamId, opcode: 'text'|'binary', fin: bool }` | frame payload |
|
||||
| 0x23 | ws-close | both | `{ streamId, code?, reason? }` | empty |
|
||||
| 0x30 | cancel | server → agent | `{ streamId }` | empty |
|
||||
| 0xFE | ping | both | `{ ts }` | empty |
|
||||
| 0xFF | disconnect | server → agent | `{ reason }` | empty |
|
||||
|
||||
Notes:
|
||||
|
||||
- `streamId` is server-assigned for `http-request` and `ws-open`. It scopes
|
||||
ordering and back-pressure per logical request.
|
||||
- Body chunks for HTTP responses are streamed (`fin: false` until the last
|
||||
chunk). The server proxies them to the browser without buffering, so
|
||||
large downloads do not balloon memory on either side.
|
||||
- The `originHint` lets the agent log which approved target a request was
|
||||
routed to; routing itself is determined by the registered target's
|
||||
`agentId` + origin, not by anything the browser sends.
|
||||
- Back-pressure: if the server's downstream socket is paused, it stops
|
||||
reading from the agent's WebSocket. WebSocket flow control then applies
|
||||
end-to-end. We do not implement an additional credit scheme until
|
||||
measurement shows we need one.
|
||||
|
||||
## Security model
|
||||
|
||||
Every guarantee the loopback proxy gives must hold here too. Checked
|
||||
against the same threat model:
|
||||
|
||||
- **Server-side SSRF**: target URLs are still validated against the loopback
|
||||
allowlist — but on the agent, not the server. The server never makes a
|
||||
network call on behalf of a target.
|
||||
- **Cross-user target access**: a target id is owned by the user that
|
||||
registered it. Cookie + path scope unchanged.
|
||||
- **Cross-agent leakage**: a target id is also bound to the specific
|
||||
`agentId` it was registered against. Even if two users somehow share a
|
||||
target id (they cannot — ids are 128-bit random), dispatch only reaches
|
||||
the agent the target was bound to.
|
||||
- **Agent impersonation**: `agentSecret` is per-agent, stored only on the
|
||||
user's machine, transported only over TLS during enrollment + connect.
|
||||
Revocable from Settings.
|
||||
- **Frame-busting headers**: stripped server-side after the agent returns
|
||||
the response, identical to the loopback path. Same code path
|
||||
(`stripFrameBustingHeaders`) — keep it as a single point of truth.
|
||||
- **Dev-server credentials**: the agent strips `cookie`, `authorization`,
|
||||
and `x-openchamber-ui-session` before forwarding to the local dev
|
||||
server, mirroring the existing `proxyReq` handler.
|
||||
- **Public-internet exposure**: no inbound port opens on the user's
|
||||
machine; no egress to non-loopback addresses; the agent process refuses
|
||||
to start with `0.0.0.0` upstream targets that resolve off-loopback.
|
||||
- **Connection pinning**: when the agent's WebSocket disconnects, all of
|
||||
its targets are evicted immediately and any in-flight streams are
|
||||
aborted with 502. The cached entry on the browser side (see
|
||||
`previewProxyTargetCache` in `ContextPanel.tsx`) will then re-register
|
||||
on the next attempt and surface the "no agent connected" empty state.
|
||||
|
||||
Out-of-scope hardening to revisit later:
|
||||
|
||||
- mTLS for the agent ↔ server link (current proposal: TLS + agentSecret;
|
||||
mTLS is a future option for self-hosters who want it).
|
||||
- Audit logging of every proxied request (today the loopback path doesn't
|
||||
do this; the remote path should not become an exception without a UX
|
||||
for inspecting the log).
|
||||
|
||||
## Failure modes
|
||||
|
||||
| Failure | Behaviour |
|
||||
| -------------------------------------- | -------------------------------------------------------------------------------------------------------------- |
|
||||
| Agent never connected | `POST /api/preview/targets/remote` returns 409 with `{ error: 'No agent connected' }`. UI shows empty state. |
|
||||
| Agent disconnected mid-request | Server cancels the stream, returns 502 to the browser, evicts the target. Existing overlay handles it. |
|
||||
| Dev server down on user's laptop | Agent forwards the connection refusal as `http-error`; server emits 502. Existing overlay handles it. |
|
||||
| Slow agent / dev server | Streamed response keeps flowing; no buffering on the server. WebSocket flow control gates the data rate. |
|
||||
| Server restarted | Agent reconnects with stored `agentSecret`. Browser-side cache 404s on next request and re-registers. |
|
||||
| Enrollment token expired | `POST /api/preview/agent/enroll` returns 401 with a clear error; UI prompts to mint a new one. |
|
||||
| Two agents registered for same user | Allowed. The browser-side flow always picks the most recently active agent for a given upstream URL. |
|
||||
|
||||
## Open questions
|
||||
|
||||
These need a decision before implementation, not before the doc lands.
|
||||
|
||||
1. **CLI surface.** Is `openchamber preview-agent` the right verb, or should
|
||||
it live under `openchamber agent preview`? Bias: the former; only one
|
||||
agent today, and we can rename without breaking anything if we ever ship
|
||||
a second.
|
||||
2. **Multi-agent UX.** When a user has two agents online (laptop + desktop)
|
||||
and registers a `localhost:3000` preview, which one wins? Most-recent
|
||||
activity is a sensible default but we should also let the user pin a
|
||||
target to an agent.
|
||||
3. **Browser-side detection of remote vs loopback.** Today the UI has no
|
||||
reason to know. If the empty state needs the user's enrolled agents,
|
||||
that becomes a new `GET /api/preview/agents` endpoint. Acceptable.
|
||||
4. **Storage of `agentSecret`.** Plain file under the agent config dir is
|
||||
simplest. OS keychain integration is nicer but a much larger surface.
|
||||
Bias: file first, keychain later.
|
||||
5. **Frame protocol vs. full HTTP/2 / gRPC.** The custom frame protocol is
|
||||
maybe 200 lines in each runtime. gRPC would handle streaming and back
|
||||
pressure for us but adds a heavy dep. Bias: custom frames; revisit only
|
||||
if we hit a back-pressure or multiplexing bug we cannot solve cleanly.
|
||||
6. **Compression.** The current loopback path forces `accept-encoding:
|
||||
identity` to keep the proxy simple. The remote path probably wants
|
||||
gzip/br between the agent and the server to save bandwidth on slow
|
||||
links — but the dev server may not be configured for it. Decide once we
|
||||
measure.
|
||||
|
||||
## Implementation milestones
|
||||
|
||||
Each milestone is independently shippable and reviewable. Numbers are
|
||||
sequence, not effort.
|
||||
|
||||
1. Agent registry + enrollment endpoints on the server. No proxying yet.
|
||||
Settings UI to mint and revoke enrollment tokens.
|
||||
2. Standalone agent that connects, says hello, and stays connected with
|
||||
ping/pong. No proxying yet. Validates the auth + reconnect story.
|
||||
3. HTTP-only proxying through the agent (`http-request` /
|
||||
`http-response-*`). Browser can register a remote target and load
|
||||
static pages. No HMR yet.
|
||||
4. WebSocket proxying through the agent (`ws-open` / `ws-frame` /
|
||||
`ws-close`). HMR works.
|
||||
5. Failure-mode polish: 502 on disconnect, target eviction, browser-side
|
||||
empty state, "agent connected" indicator in Settings.
|
||||
6. Documentation + tutorial for the remote-host scenario; update
|
||||
`docs/REVERSE_PROXY.md` cross-link.
|
||||
|
||||
## Why not …?
|
||||
|
||||
- **A reverse SSH tunnel from the agent.** Works but requires SSH server
|
||||
on the OpenChamber host, exposes a port, and breaks the same-origin
|
||||
guarantee unless we also reverse-proxy that port through the
|
||||
OpenChamber HTTP server. The control-WebSocket design avoids all of
|
||||
that and keeps a single TLS endpoint.
|
||||
- **Cloudflare/ngrok-style hosted relay.** Would work but turns
|
||||
OpenChamber into a service that depends on a third party (or on us
|
||||
hosting a relay). The agent design lets users run entirely
|
||||
self-hosted.
|
||||
- **WebRTC data channels.** Lower latency in theory, much harder to debug
|
||||
and to reason about behind corporate NATs. Not worth the complexity
|
||||
for HTTP + WS forwarding.
|
||||
|
||||
## Cross-references
|
||||
|
||||
- Loopback runtime: `packages/web/server/lib/preview/proxy-runtime.js`
|
||||
- Browser PreviewPane + cache: `packages/ui/src/components/layout/ContextPanel.tsx`
|
||||
- Reverse-proxy deployment notes: `docs/REVERSE_PROXY.md`
|
||||
@@ -1,766 +0,0 @@
|
||||
# Review Flow Implementation Plan
|
||||
|
||||
## Goal
|
||||
|
||||
Build an end-to-end OpenChamber review handoff flow that lets one session implement changes and another normal session review them, with OpenChamber metadata connecting the two sessions invisibly.
|
||||
|
||||
The agents must not see session IDs, metadata, linked-session wording, or routing details. They should only receive natural prompts:
|
||||
|
||||
- Review session initial prompt: a handoff plus an instruction to review it.
|
||||
- Review-to-implementer prompt: another agent reviewed the changes and left feedback; resolve relevant issues.
|
||||
- Implementer-to-review prompt: the agent implementing changes responded to previous feedback; review latest state again.
|
||||
|
||||
The review session is not a child/subsession. It is a normal session in the same directory as the original session.
|
||||
|
||||
## Proposed Command Name
|
||||
|
||||
Do not use `/review`, because that overlaps with OpenCode's default review command semantics.
|
||||
|
||||
Use `/handoff-review` unless we choose a shorter name before implementation.
|
||||
|
||||
Other acceptable names if `/handoff-review` feels too long:
|
||||
|
||||
- `/review-handoff`
|
||||
- `/ask-review`
|
||||
- `/start-review`
|
||||
|
||||
This plan assumes `/handoff-review`.
|
||||
|
||||
## Existing Code Paths To Reuse
|
||||
|
||||
### Slash Command Routing
|
||||
|
||||
Relevant files:
|
||||
|
||||
- `packages/ui/src/sync/session-ui-store.ts`
|
||||
- `packages/ui/src/lib/opencode/client.ts`
|
||||
- `packages/ui/src/lib/magicPrompts.ts`
|
||||
|
||||
Current behavior:
|
||||
|
||||
- `routeMessage(...)` detects messages starting with `/`.
|
||||
- It checks command metadata from `getDirectoryState(requestDirectory)?.command` and `useCommandsStore.getState().commands`.
|
||||
- If the command exists, it uses `optimisticSend(...)` and calls `opencodeClient.sendCommand(...)`.
|
||||
- `opencodeClient.sendCommand(...)` calls SDK `client.session.command(...)` with `sessionID`, `command`, `arguments`, selected model, selected agent, variant, files, and client-generated `messageID`.
|
||||
|
||||
Reuse this for invoking the handoff-generation command. The review flow should not invent a separate command transport.
|
||||
|
||||
### Magic Prompt Registry
|
||||
|
||||
Relevant file:
|
||||
|
||||
- `packages/ui/src/lib/magicPrompts.ts`
|
||||
|
||||
Current examples:
|
||||
|
||||
- `session.summary.visible`
|
||||
- `session.summary.instructions`
|
||||
- `session.review.visible`
|
||||
- `session.review.instructions`
|
||||
- `git.commit.generate.visible`
|
||||
- `git.commit.generate.instructions`
|
||||
|
||||
Add new prompt entries for this flow instead of hardcoding long prompts in components/actions.
|
||||
|
||||
Needed prompt entries:
|
||||
|
||||
- `session.reviewHandoff.visible`
|
||||
- `session.reviewHandoff.instructions`
|
||||
- `session.reviewSession.visible`
|
||||
- `session.reviewSession.instructions` if we need hidden instructions for the review session starter prompt
|
||||
- `session.reviewFeedbackToImplementer.visible`
|
||||
- `session.implementationResponseToReviewer.visible`
|
||||
|
||||
The two cross-session prompts must match the agreed wording closely.
|
||||
|
||||
Review feedback sent back to the original session:
|
||||
|
||||
```md
|
||||
Another agent reviewed your changes and left the feedback below.
|
||||
|
||||
Please review the feedback, resolve the relevant issues, and explain what you changed.
|
||||
|
||||
<review feedback>
|
||||
```
|
||||
|
||||
Implementation response sent back to the review session:
|
||||
|
||||
```md
|
||||
The agent implementing the changes has responded to the previous review feedback.
|
||||
|
||||
Please review the latest state again and report any remaining issues.
|
||||
|
||||
<implementation response / latest assistant message>
|
||||
```
|
||||
|
||||
Initial review session prompt should be similar to:
|
||||
|
||||
```md
|
||||
Please review the changes described in this handoff.
|
||||
|
||||
Focus on correctness, regressions, missing implementation, missing tests, and whether the implementation satisfies the stated intent. Provide concise, actionable feedback for the agent implementing the changes.
|
||||
|
||||
<handoff>
|
||||
```
|
||||
|
||||
The handoff-generation prompt should be based on the summary command, but explicitly include the user's intent and enough implementation context for another agent to review.
|
||||
|
||||
### Handoff Generation Concept
|
||||
|
||||
Relevant existing prompt:
|
||||
|
||||
- `session.summary.instructions` in `packages/ui/src/lib/magicPrompts.ts`
|
||||
|
||||
Current summary instructions already include:
|
||||
|
||||
- completed work
|
||||
- in-progress work
|
||||
- modified files and why
|
||||
- open questions and next steps
|
||||
- user requests, constraints, preferences
|
||||
- technical decisions and rationale
|
||||
|
||||
The new handoff prompt should keep those ideas and make intent explicit:
|
||||
|
||||
- What the user wanted and why
|
||||
- What was implemented
|
||||
- What files changed and why
|
||||
- Important design choices
|
||||
- Known limitations or uncertainty
|
||||
- Validation/test status if known from the session
|
||||
- Anything the reviewer should pay special attention to
|
||||
|
||||
This is an implementation detail, not a risk. The prompt should be specific enough that the review agent can judge intent and implementation without needing private OpenChamber routing context.
|
||||
|
||||
### Active Session Generation Concept
|
||||
|
||||
Relevant files:
|
||||
|
||||
- `packages/ui/src/lib/gitApi.ts`
|
||||
- `packages/ui/src/components/views/GitView.tsx`
|
||||
|
||||
Current commit-message generation uses:
|
||||
|
||||
- `resolveSessionGenerationContext()` to find current session, model, agent, and variant.
|
||||
- `runStructuredGenerationInActiveSession(...)` to send a visible prompt plus hidden synthetic instructions to the active session.
|
||||
- `extractAssistantText(...)` and JSON parsing to get output from the assistant response.
|
||||
|
||||
Important difference for review flow:
|
||||
|
||||
- Commit generation uses `client.session.prompt(...)` and receives the response directly.
|
||||
- Slash commands use `client.session.command(...)`, are effectively fire-and-forget from the UI path, and rely on SSE to populate messages/status.
|
||||
|
||||
For `/handoff-review`, prefer the visible slash command path so the original session contains the generated handoff. Then wait for the resulting assistant output through sync state. Reuse the commit-generation concepts for:
|
||||
|
||||
- selected model/agent/variant resolution
|
||||
- extracting text from assistant message parts
|
||||
- forcing chat scroll if useful
|
||||
- timeout/error handling style
|
||||
|
||||
Create a small reusable helper for waiting for the next completed assistant text after a known user command message ID.
|
||||
|
||||
### Session Create/Update/Delete
|
||||
|
||||
Relevant files:
|
||||
|
||||
- `packages/ui/src/lib/opencode/client.ts`
|
||||
- `packages/ui/src/sync/session-actions.ts`
|
||||
- `packages/ui/src/sync/event-reducer.ts`
|
||||
- `packages/ui/src/stores/useGlobalSessionsStore.ts`
|
||||
|
||||
Current behavior:
|
||||
|
||||
- `opencodeClient.createSession(...)` calls `client.session.create(...)` using the legacy OpenCode session API.
|
||||
- OpenCode supports `metadata` on that API, but OpenChamber currently only forwards `parentID` and `title`.
|
||||
- `opencodeClient.updateSession(...)` currently only forwards `title` and `time.archived`.
|
||||
- OpenCode `metadata` update replaces the whole metadata object. It does not deep-merge.
|
||||
- `deleteSession(...)` and `deleteSessionInDirectory(...)` optimistically remove the session, then call `opencodeClient.deleteSession(...)`, and restore snapshots on failure.
|
||||
- `event-reducer.ts` replaces session objects from `session.created` and `session.updated` events.
|
||||
|
||||
Add metadata support here first. The review flow depends on it.
|
||||
|
||||
### Context Panel Session Tabs
|
||||
|
||||
Relevant files:
|
||||
|
||||
- `packages/ui/src/stores/useUIStore.ts`
|
||||
- `packages/ui/src/components/session/sidebar/SessionNodeItem.tsx`
|
||||
- `packages/ui/src/components/layout/ContextPanel.tsx`
|
||||
- `packages/ui/src/components/chat/message/MessageBody.tsx`
|
||||
- `packages/ui/src/components/chat/message/parts/ToolPart.tsx`
|
||||
|
||||
Current behavior:
|
||||
|
||||
- `useUIStore.openContextPanelTab(directory, tab)` opens or upserts context panel tabs.
|
||||
- Chat tabs use `mode: 'chat'`.
|
||||
- Existing dedupe key convention for session chat tabs is `session:<sessionID>`.
|
||||
- Sidebar already opens a session in the side panel with:
|
||||
|
||||
```ts
|
||||
openContextPanelTab(sessionDirectory, {
|
||||
mode: 'chat',
|
||||
dedupeKey: `session:${session.id}`,
|
||||
label: sessionTitle,
|
||||
})
|
||||
```
|
||||
|
||||
Reuse the same convention for opening the review session in the context panel.
|
||||
|
||||
### Assistant Message Action Buttons
|
||||
|
||||
Relevant file:
|
||||
|
||||
- `packages/ui/src/components/chat/message/MessageBody.tsx`
|
||||
|
||||
Current behavior:
|
||||
|
||||
- `AssistantMessageActionButtons` renders icon-only buttons for copy, save image, and TTS.
|
||||
- The buttons use shared `Button`, `Tooltip`, and `Icon` components.
|
||||
- The shared icon sprite already contains `arrow-left-right`.
|
||||
|
||||
Extend this action area with an optional review-transfer action. Do not import icons directly from Remixicon.
|
||||
|
||||
## Metadata Contract
|
||||
|
||||
Use a namespaced metadata object so we do not collide with user or upstream metadata.
|
||||
|
||||
Original session metadata:
|
||||
|
||||
```ts
|
||||
{
|
||||
openchamber: {
|
||||
reviewSessionID: string
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
Review session metadata:
|
||||
|
||||
```ts
|
||||
{
|
||||
openchamber: {
|
||||
kind: 'review'
|
||||
originalSessionID: string
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
Rules:
|
||||
|
||||
- Only one review session per original session.
|
||||
- If original metadata already has `openchamber.reviewSessionID`, reuse that session instead of creating a new review session.
|
||||
- The review session must not have `parentID` set to the original session.
|
||||
- Both sessions must stay in the same directory.
|
||||
- Metadata is internal routing state only. Never include it in prompts.
|
||||
- Metadata updates must preserve unrelated metadata keys.
|
||||
|
||||
Recommended helpers:
|
||||
|
||||
```ts
|
||||
type OpenChamberSessionMetadata = {
|
||||
openchamber?: {
|
||||
kind?: 'review'
|
||||
originalSessionID?: string
|
||||
reviewSessionID?: string
|
||||
}
|
||||
[key: string]: unknown
|
||||
}
|
||||
```
|
||||
|
||||
Helper functions should live in a focused module, for example:
|
||||
|
||||
- `packages/ui/src/lib/sessionReviewMetadata.ts`
|
||||
|
||||
Functions:
|
||||
|
||||
- `getOpenChamberMetadata(session)`
|
||||
- `isReviewSession(session)`
|
||||
- `getOriginalSessionID(session)`
|
||||
- `getReviewSessionID(session)`
|
||||
- `withReviewSessionLink(metadata, reviewSessionID)`
|
||||
- `withReviewSessionMarker(metadata, originalSessionID)`
|
||||
- `withoutReviewSessionLink(metadata, reviewSessionID)`
|
||||
|
||||
The helpers should clone only the metadata branch they change and preserve all unrelated metadata.
|
||||
|
||||
## Implementation Steps
|
||||
|
||||
### 1. Load Required Skills Before Editing
|
||||
|
||||
When implementing this plan, load these skills before changing code:
|
||||
|
||||
- `ui-api-decoupling` because the work changes SDK data access and session API wrapper behavior.
|
||||
- `theme-system` because the work adds a UI button/icon.
|
||||
- `locale-ui-patterns` because the work adds tooltips, aria labels, toasts, and command text.
|
||||
|
||||
If the final implementation touches Settings magic prompt UI, also load:
|
||||
|
||||
- `settings-ui-patterns`
|
||||
|
||||
### 2. Add Metadata Support To OpenCode Client Wrapper
|
||||
|
||||
File:
|
||||
|
||||
- `packages/ui/src/lib/opencode/client.ts`
|
||||
|
||||
Change `createSession` signature from:
|
||||
|
||||
```ts
|
||||
async createSession(params?: { parentID?: string; title?: string }, directory?: string | null): Promise<Session>
|
||||
```
|
||||
|
||||
to:
|
||||
|
||||
```ts
|
||||
async createSession(
|
||||
params?: {
|
||||
parentID?: string
|
||||
title?: string
|
||||
metadata?: Record<string, unknown>
|
||||
},
|
||||
directory?: string | null,
|
||||
): Promise<Session>
|
||||
```
|
||||
|
||||
Forward `metadata: params?.metadata` only when it is defined.
|
||||
|
||||
Change `updateSession` patch type from:
|
||||
|
||||
```ts
|
||||
patch: { title?: string; time?: { archived?: number | null } }
|
||||
```
|
||||
|
||||
to:
|
||||
|
||||
```ts
|
||||
patch: {
|
||||
title?: string
|
||||
metadata?: Record<string, unknown>
|
||||
time?: { archived?: number | null }
|
||||
}
|
||||
```
|
||||
|
||||
Forward `metadata` when defined.
|
||||
|
||||
Important: this method should still replace metadata because the upstream API replaces metadata. Do not hide this with an implicit merge here. Add merge behavior in a separate helper so call sites are explicit.
|
||||
|
||||
### 3. Make Session Types Metadata-Aware In OpenChamber
|
||||
|
||||
OpenCode SDK response types should include metadata in the current v2 SDK legacy `Session`, but verify local imports and generated types used by OpenChamber.
|
||||
|
||||
Files to inspect/update:
|
||||
|
||||
- `packages/ui/src/stores/types/sessionTypes.ts`
|
||||
- Any local `Session` wrapper/normalizer if present
|
||||
- `packages/ui/src/sync/sanitize.ts`
|
||||
- `packages/ui/src/sync/event-reducer.ts`
|
||||
- `packages/ui/src/stores/useGlobalSessionsStore.ts`
|
||||
|
||||
Goal:
|
||||
|
||||
- `session.metadata` should survive list, get, create, update, SSE event replacement, global sessions, and reconnect recovery.
|
||||
- Do not strip `metadata` in sanitation helpers.
|
||||
- Do not create new broad store subscriptions. Use leaf selectors where UI only needs review metadata for one session.
|
||||
|
||||
### 4. Add Explicit Metadata Merge Helpers
|
||||
|
||||
Add a helper, likely in `packages/ui/src/sync/session-actions.ts` or a small module imported by it:
|
||||
|
||||
```ts
|
||||
async function patchSessionMetadata(
|
||||
sessionId: string,
|
||||
directory: string | null | undefined,
|
||||
updater: (metadata: Record<string, unknown>) => Record<string, unknown>,
|
||||
): Promise<Session>
|
||||
```
|
||||
|
||||
Behavior:
|
||||
|
||||
1. Read the current session with `opencodeClient.getSession(sessionId)` using the correct directory.
|
||||
2. Read `current.metadata ?? {}`.
|
||||
3. Apply updater.
|
||||
4. Call `opencodeClient.updateSession(sessionId, { metadata: nextMetadata }, directory)`.
|
||||
5. Upsert the returned session into `useGlobalSessionsStore` and the relevant child store if needed.
|
||||
|
||||
Do not swallow fetch/update errors. Callers need to know if metadata linkage failed.
|
||||
|
||||
### 5. Add Review Flow Magic Prompts
|
||||
|
||||
File:
|
||||
|
||||
- `packages/ui/src/lib/magicPrompts.ts`
|
||||
|
||||
Add these prompt records:
|
||||
|
||||
1. `session.reviewHandoff.visible`
|
||||
|
||||
Suggested template:
|
||||
|
||||
```txt
|
||||
Prepare a handoff for another agent to review this work.
|
||||
```
|
||||
|
||||
2. `session.reviewHandoff.instructions`
|
||||
|
||||
Suggested template:
|
||||
|
||||
```txt
|
||||
Produce a review handoff for another agent. Do not compact or mutate session history. Your output is an assistant message that OpenChamber will send to a separate reviewer agent.
|
||||
|
||||
Include:
|
||||
- The user's original intent and any later clarifications that changed the intent
|
||||
- What was implemented and why
|
||||
- Files changed, with brief purpose per file
|
||||
- Important design decisions and tradeoffs
|
||||
- Validation/tests run, if known
|
||||
- Known gaps, uncertainty, or areas the reviewer should inspect closely
|
||||
|
||||
Formatting:
|
||||
- Concise markdown with clear sections
|
||||
- No preamble like "Here is a handoff"
|
||||
- Do not mention OpenChamber metadata, linked sessions, session IDs, or routing
|
||||
- Respond in the same language the user used most in the conversation
|
||||
```
|
||||
|
||||
3. `session.reviewSession.visible`
|
||||
|
||||
Suggested template with `{{handoff}}` placeholder:
|
||||
|
||||
```txt
|
||||
Please review the changes described in this handoff.
|
||||
|
||||
Focus on correctness, regressions, missing implementation, missing tests, and whether the implementation satisfies the stated intent. Provide concise, actionable feedback for the agent implementing the changes.
|
||||
|
||||
{{handoff}}
|
||||
```
|
||||
|
||||
4. `session.reviewFeedbackToImplementer.visible`
|
||||
|
||||
Suggested template with `{{review_feedback}}` placeholder:
|
||||
|
||||
```txt
|
||||
Another agent reviewed your changes and left the feedback below.
|
||||
|
||||
Please review the feedback, resolve the relevant issues, and explain what you changed.
|
||||
|
||||
{{review_feedback}}
|
||||
```
|
||||
|
||||
5. `session.implementationResponseToReviewer.visible`
|
||||
|
||||
Suggested template with `{{implementation_response}}` placeholder:
|
||||
|
||||
```txt
|
||||
The agent implementing the changes has responded to the previous review feedback.
|
||||
|
||||
Please review the latest state again and report any remaining issues.
|
||||
|
||||
{{implementation_response}}
|
||||
```
|
||||
|
||||
### 6. Add Localized UI Strings
|
||||
|
||||
Files:
|
||||
|
||||
- `packages/ui/src/lib/i18n/messages/en.ts`
|
||||
- Other locale files as required by the project pattern
|
||||
|
||||
Add strings for:
|
||||
|
||||
- Command autocomplete description for `/handoff-review`.
|
||||
- Review flow button aria label on review session: “Send review feedback to implementing agent”.
|
||||
- Review flow button aria label on original session: “Send implementation response to reviewing agent”.
|
||||
- Tooltip text for both directions.
|
||||
- Toasts for starting handoff generation, review session creation/reuse, transfer success, transfer failure, missing linked session, missing assistant text.
|
||||
|
||||
Follow locale-ui-patterns. Do not hardcode user-facing text inside components.
|
||||
|
||||
### 7. Register The New OpenChamber Slash Command
|
||||
|
||||
There are two possible implementation paths. Pick the one matching how OpenChamber-owned commands are currently registered.
|
||||
|
||||
Likely locations:
|
||||
|
||||
- `packages/ui/src/lib/magicPrompts.ts`
|
||||
- command autocomplete/store code around `useCommandsStore`
|
||||
- command rendering in `ChatInput` / command autocomplete components
|
||||
|
||||
The command should appear as `/handoff-review` in OpenChamber command autocomplete.
|
||||
|
||||
It should be treated as an OpenChamber flow command, not only a raw OpenCode command, because after the handoff assistant output completes OpenChamber must create/reuse/open/send to the review session.
|
||||
|
||||
Implementation options:
|
||||
|
||||
1. Intercept `/handoff-review` in `routeMessage(...)` before normal OpenCode command lookup.
|
||||
2. Add it to the command store as an OpenChamber-owned command with a handler.
|
||||
|
||||
Prefer the smallest approach consistent with existing command architecture.
|
||||
|
||||
### 8. Implement Handoff Generation And Wait Helper
|
||||
|
||||
Add a helper that starts the handoff command in the original session and resolves with the assistant handoff text.
|
||||
|
||||
Possible module:
|
||||
|
||||
- `packages/ui/src/lib/reviewFlow.ts`
|
||||
|
||||
Inputs:
|
||||
|
||||
```ts
|
||||
{
|
||||
originalSessionID: string
|
||||
directory: string
|
||||
providerID: string
|
||||
modelID: string
|
||||
agent?: string
|
||||
variant?: string
|
||||
}
|
||||
```
|
||||
|
||||
Flow:
|
||||
|
||||
1. Render `session.reviewHandoff.visible` and `session.reviewHandoff.instructions`.
|
||||
2. Send a user message to the original session using `opencodeClient.sendMessage(...)` or the existing command route, depending on final command integration.
|
||||
3. Include the visible handoff request as the visible user text.
|
||||
4. Include hidden instructions as synthetic additional part if using `sendMessage(...)`.
|
||||
5. Capture the generated user message ID.
|
||||
6. Wait until a later assistant message for the same session is complete and has text.
|
||||
7. Extract text with the same idea as `flattenAssistantTextParts(...)` / `extractAssistantText(...)`.
|
||||
8. Timeout with a clear failure if no handoff arrives.
|
||||
|
||||
Waiting rules:
|
||||
|
||||
- Prefer sync store state over polling the server repeatedly.
|
||||
- Use existing `getSyncMessages(sessionID)` and `getSyncParts(sessionID)` from `sync-refs` if they expose enough data.
|
||||
- If a subscription-based wait is not easy, use a bounded interval that reads sync refs and stops on timeout or completion.
|
||||
- Ensure it waits for assistant completion, not just first streaming text.
|
||||
- Avoid broad store subscriptions in React components.
|
||||
|
||||
### 9. Create Or Reuse The Review Session
|
||||
|
||||
After handoff text is available:
|
||||
|
||||
1. Read original session with `opencodeClient.getSession(originalSessionID)`.
|
||||
2. Read `original.metadata.openchamber.reviewSessionID`.
|
||||
3. If it exists:
|
||||
- Try to get that review session in the same directory.
|
||||
- If it exists and has `metadata.openchamber.kind === 'review'`, reuse it.
|
||||
- If it is missing/deleted, clear the stale link and create a new review session.
|
||||
4. If it does not exist, create a new normal session in the same directory with metadata:
|
||||
|
||||
```ts
|
||||
{
|
||||
openchamber: {
|
||||
kind: 'review',
|
||||
originalSessionID,
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
5. Patch original session metadata with:
|
||||
|
||||
```ts
|
||||
{
|
||||
openchamber: {
|
||||
reviewSessionID: reviewSession.id,
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
Preserve unrelated metadata on both sessions.
|
||||
|
||||
If metadata patching original fails after creating review session, report failure clearly. Do not silently proceed with an unlinked session.
|
||||
|
||||
### 10. Send Initial Prompt To Review Session
|
||||
|
||||
After create/reuse:
|
||||
|
||||
1. Render `session.reviewSession.visible` with `handoff`.
|
||||
2. Send it to the review session as a normal user message.
|
||||
3. Use the same provider/model/agent/variant policy as the current session unless product decision says otherwise.
|
||||
4. Do not mention session IDs or linked sessions.
|
||||
|
||||
Important:
|
||||
|
||||
- If reusing an existing review session, still send the new handoff prompt into it.
|
||||
- Reuse does not mean “do nothing”; it means continue the same review conversation.
|
||||
|
||||
### 11. Open Review Session In Context Panel
|
||||
|
||||
Use:
|
||||
|
||||
```ts
|
||||
useUIStore.getState().openContextPanelTab(directory, {
|
||||
mode: 'chat',
|
||||
dedupeKey: `session:${reviewSession.id}`,
|
||||
label: reviewSession.title,
|
||||
})
|
||||
```
|
||||
|
||||
This should happen after the review session exists and the initial prompt has been sent, or immediately after creation if sending happens asynchronously but errors are still surfaced.
|
||||
|
||||
### 12. Add Cross-Session Transfer Button On Assistant Messages
|
||||
|
||||
File:
|
||||
|
||||
- `packages/ui/src/components/chat/message/MessageBody.tsx`
|
||||
|
||||
Add optional props to `AssistantMessageActionButtons`:
|
||||
|
||||
```ts
|
||||
reviewTransferAction?: {
|
||||
ariaLabel: string
|
||||
tooltip: string
|
||||
disabled?: boolean
|
||||
onClick: () => Promise<void> | void
|
||||
}
|
||||
```
|
||||
|
||||
Render an icon-only button with:
|
||||
|
||||
```tsx
|
||||
<Icon name="arrow-left-right" ... />
|
||||
```
|
||||
|
||||
Visibility rules:
|
||||
|
||||
- Only assistant messages.
|
||||
- Only messages with copyable text.
|
||||
- In a review session: show button to send review feedback to the original session.
|
||||
- In an original session with `metadata.openchamber.reviewSessionID`: show button to send implementation response to the review session.
|
||||
- Do not show in mini-chat if that surface should avoid extra controls; follow current action-button surface rules.
|
||||
|
||||
To avoid button spam:
|
||||
|
||||
- Preferred first implementation: show on assistant messages where normal assistant action buttons already show.
|
||||
- Do not add the button to user messages.
|
||||
- If this feels too noisy in testing, narrow to latest completed assistant message per session as a follow-up, but not required for initial end-to-end implementation.
|
||||
|
||||
### 13. Implement Review Feedback Transfer
|
||||
|
||||
When clicking the button in a review session:
|
||||
|
||||
1. Get current review session metadata.
|
||||
2. Resolve `originalSessionID`.
|
||||
3. Extract the clicked assistant message text.
|
||||
4. Render `session.reviewFeedbackToImplementer.visible` with `review_feedback`.
|
||||
5. Send it as a normal user message into the original session.
|
||||
6. Use original session directory.
|
||||
7. Optionally open/focus the original session or leave context panel as-is. The agreed behavior only requires sending.
|
||||
8. Show success/failure toast.
|
||||
|
||||
Message sent to the agent must be exactly natural-language feedback, not routing data.
|
||||
|
||||
### 14. Implement Implementation Response Transfer
|
||||
|
||||
When clicking the button in the original session:
|
||||
|
||||
1. Get original session metadata.
|
||||
2. Resolve `reviewSessionID`.
|
||||
3. Extract the clicked assistant message text.
|
||||
4. Render `session.implementationResponseToReviewer.visible` with `implementation_response`.
|
||||
5. Send it as a normal user message into the review session.
|
||||
6. Use same directory.
|
||||
7. Open/focus the review session context panel tab, because review continuation happens there.
|
||||
8. Show success/failure toast.
|
||||
|
||||
### 15. Cleanup Metadata When Deleting Review Session
|
||||
|
||||
Files:
|
||||
|
||||
- `packages/ui/src/sync/session-actions.ts`
|
||||
- `packages/ui/src/lib/opencode/client.ts`
|
||||
|
||||
Before deleting a session:
|
||||
|
||||
1. Read the session being deleted.
|
||||
2. If it is a review session and has `originalSessionID`, read the original session.
|
||||
3. If original metadata has `reviewSessionID` equal to the deleted review session ID, patch original metadata to remove it.
|
||||
4. Then delete the review session.
|
||||
|
||||
Failure behavior:
|
||||
|
||||
- If metadata cleanup fails, do not delete silently. Return failure and show the existing delete failure path/toast.
|
||||
- If original session no longer exists, continue deleting the review session; there is nothing to clean.
|
||||
- If delete fails after metadata cleanup succeeded, restore the original metadata link as part of rollback if possible. At minimum, log and surface the delete failure.
|
||||
|
||||
Also apply this to `deleteSessionInDirectory(...)`.
|
||||
|
||||
### 16. Cleanup Stale Link When Reusing Review Session
|
||||
|
||||
If original metadata points to a review session that no longer exists:
|
||||
|
||||
1. Patch original metadata to remove stale `reviewSessionID`.
|
||||
2. Create a fresh review session.
|
||||
3. Patch original metadata with the fresh review session ID.
|
||||
|
||||
This is not a background reconciler. It only happens when the user starts the review flow.
|
||||
|
||||
### 17. Tests
|
||||
|
||||
Add focused tests for helpers and flow boundaries.
|
||||
|
||||
Likely files:
|
||||
|
||||
- New `packages/ui/src/lib/sessionReviewMetadata.test.ts`
|
||||
- Existing `packages/ui/src/sync/session-actions.test.ts`
|
||||
- Component test around `MessageBody` only if nearby test patterns exist
|
||||
|
||||
Test cases:
|
||||
|
||||
1. Metadata helper marks review session without removing unrelated metadata.
|
||||
2. Metadata helper links original session without removing unrelated metadata.
|
||||
3. Metadata helper removes review link only when it matches the deleted review session ID.
|
||||
4. `createSession` forwards metadata to SDK client.
|
||||
5. `updateSession` forwards metadata to SDK client.
|
||||
6. Review flow reuses existing review session ID instead of creating another.
|
||||
7. Review flow clears stale review session ID when referenced review session is missing.
|
||||
8. Delete review session cleans original metadata before deleting.
|
||||
9. Transfer prompt for review-to-implementer contains no session ID / metadata / linked-session wording.
|
||||
10. Transfer prompt for implementer-to-reviewer contains no session ID / metadata / linked-session wording.
|
||||
|
||||
### 18. Validation
|
||||
|
||||
Run:
|
||||
|
||||
```sh
|
||||
bun run type-check
|
||||
bun run lint
|
||||
```
|
||||
|
||||
Manual validation checklist:
|
||||
|
||||
1. Start `/handoff-review` in a normal session.
|
||||
2. Confirm a handoff assistant message appears in original session.
|
||||
3. Confirm a normal review session is created in the same directory, not as a child.
|
||||
4. Confirm original metadata has `openchamber.reviewSessionID`.
|
||||
5. Confirm review metadata has `openchamber.kind === 'review'` and `openchamber.originalSessionID`.
|
||||
6. Confirm review session opens in context panel.
|
||||
7. Confirm review session receives the initial handoff review prompt.
|
||||
8. Confirm arrow-left-right appears on review assistant message actions.
|
||||
9. Click it and confirm original session receives the agreed review feedback prompt.
|
||||
10. Confirm arrow-left-right appears on original assistant message actions when original has review metadata.
|
||||
11. Click it and confirm review session receives the agreed implementation response prompt.
|
||||
12. Run `/handoff-review` again on the same original session and confirm it reuses the existing review session.
|
||||
13. Delete the review session and confirm original metadata link is removed.
|
||||
14. Delete original session and confirm no review cleanup crash occurs.
|
||||
|
||||
## Non-Goals
|
||||
|
||||
- Do not add a review status state machine.
|
||||
- Do not allow multiple review sessions for one original session in this first implementation.
|
||||
- Do not expose metadata, linked sessions, or session IDs to agents.
|
||||
- Do not use parent/child session relationships for this feature.
|
||||
- Do not change OpenCode core or SDK unless OpenChamber cannot access metadata from the existing SDK types.
|
||||
- Do not make a background metadata reconciler.
|
||||
|
||||
## Main Implementation Risks To Watch While Coding
|
||||
|
||||
These are coding concerns, not product blockers:
|
||||
|
||||
- Metadata replacement must not drop unrelated metadata.
|
||||
- Event/store sanitation must not strip `metadata` from session records.
|
||||
- The handoff wait helper must wait for completed assistant output, not first streaming text.
|
||||
- Cross-session sends must use the correct directory dynamically, not cached closure values.
|
||||
- Message action buttons must not subscribe broad chat rows to global session collections.
|
||||
- Delete cleanup should not delete the review session if cleanup fails in a way that would leave confusing metadata behind.
|
||||
Reference in New Issue
Block a user