Repository navigation
chore(release): v0.5.2 - #108
Merged
Merged
Conversation
Lay out the design for embedding opencode's official web UI in myworktree as a new "opencode-web" instance kind, replacing the current PTY + xterm.js path that suffers from OSC/DA echo, high CPU on TUI repaints, and fragile keyboard round-trips. The investigation in FEASIBILITY.md covers the opencode app/cli architecture (server is multi-tenant by directory, web UI is a separate SolidJS app) and explains why the embedded web UI is too rich for per-worktree use and why the /agent endpoint hangs with oh-my-opencode loaded. PLAN.md picks approach A (one opencode serve process per myworktree instance) with command, --hostname and --port hard- coded in Go for security, while OPENCODE_SERVER_PASSWORD is always generated and injected by myworktree. Non-security env vars from tag.Env are merged through BuildEnv. No code changes in this commit; documents only for review.
PR 1 of opencode-native-ui implementation plan. Solidifies the spec for the new opencode-web instance kind across all project docs: PRD §7 gets opencode-web entry, API.md documents the new GET /api/instances/<id>/opencode endpoint and /__opencode/<id>/* reverse proxy, ARCHITECTURE.md adds §8 with diagram and design constraints, CHANGELOG.md records the spec entry. TASK.md at docs/plans/opencode-native-ui/ maps out 5 PRs with file lists and acceptance criteria.
PR 2 of opencode-native-ui. Zero behavior change — PTY instances unchanged. - state.go: ManagedInstance gains Kind + Extra (backward-compat via omitempty) - opencode.go: Command(), GeneratePassword(), BuildEnv(), ExtractListeningAddress(), IsAPIPath() - tag.go: opencode-web default tag added to built-in list - Tests: backward compat for old state.json without Kind/Extra, round-trip with new fields, all opencode helpers covered
PR 3 of opencode-native-ui. Branch Manager.Start() on Kind field: "pty" (default): existing PTY path unchanged. "opencode-web": hardcoded opencode serve invocation, crypto/rand password generation, BuildEnv for env merge with forced overrides, io.Pipe stdout → pumpAndWatch goroutine (ring buffer + listening address scan + status transition), health check goroutine polling /global/health every 5s with 3 consecutive failure → failed. Graceful tag fallback: if opencode-web tag is missing from tags.json, Start uses built-in defaults rather than failing. Add Get() and UpdateExtra() for proxy access. Mock binary integration test verifies end-to-end: spawn mock, detect listening port in stdout, transition to "running", health endpoint works.
PR 4 of opencode-native-ui. - POST /api/instances now accepts optional "kind" field (default "pty"). - GET /api/instances/<id>/opencode returns iframe_src, api_base, password_set, worktree_path, host, port for frontend iframe src. - /__opencode/<id>/* reverse proxy to opencode server: injects Basic auth, adds ?directory=<worktree> to GET/HEAD API requests, SSE flush enabled, 502 on upstream unreachable. - Proxy mounted on main mux under existing withAuth middleware.
PR 5 of opencode-native-ui. Frontend changes: - Add #opencode-panel div with sandboxed iframe next to terminal-container. - Add .badge-oc CSS (accent-colored compact label). - renderTabs() shows "OC" badge for opencode-web instances. - selectInstance() branches on inst.kind: "opencode-web" → fetch /api/instances/opencode?id=, set iframe.src, hide terminal, show opencode-panel, disconnect PTY session. "pty" → restore terminal-container visibility, hide opencode-panel, existing PTY connect logic unchanged.
Refactor instance management to framework-based architecture: - internal/framework/: pluggable kind registry (pty, opencode-web) - internal/instance/opencode_web/: driver, proxy, CSP shim - internal/instance/pty/: extracted PTY kind from old manager opencode-web integration: - Reverse proxy at /__opencode/<id>/* with Basic auth injection - Injects ?directory=<worktree> for GET/HEAD API paths - HTML rewriting: <base> tag, absolute path rewrite (src/href) - Three-layer shim injected into SPA HTML: 1. history.replaceState strips proxy prefix for SPA Router 2. localStorage override sets defaultServerUrl to proxy URL 3. fetch/EventSource/XHR URL rewrite (defense-in-depth) - CSP hash injection for inline shim script - iframe sandbox: allow-scripts allow-same-origin allow-forms allow-popups - Debug report: docs/plans/opencode-native-ui/DEBUG.md UI: - Kind badge (OC) on instance tabs - iframe panel + poll-based ready detection (500ms/60s) - Debug bar showing upstream/proxy/worktree info
… + UI guard) Replace the deep session link (which kept blanking the SPA) with full-page embed at /__opencode/<id>/, and add proxy-side directory monitoring plus an injected UI guard so the embedded opencode web UI can no longer silently drift into a different worktree's project / session / server list. Implementation: - Reverse-proxy directory monitor records out-of-scope requests in an in-memory ScopeTracker, exposed via /api/instances/opencode/scope?id= - Injected script hides cross-worktree switch entries (project switch / add-project / open-project) and normalizes localStorage server list to the single embedded server - opencode --version gate (advisory, 1.18.x) + DOM-anchor / visibility checks reported via postMessage - Persistent in-panel warning bar (out-of-scope + hiding-not-effective states) Design / decision record: docs/plans/opencode-native-ui/WORKTREE-ISOLATION.md Threat model / review checklist: docs/ARCHITECTURE.md §8
…er rewrite + project preseed) Follow-up to b66f25b. The full-page embed (/__opencode/<id>/) on the home route surfaced cross-worktree switch entries that the previous guard did not cover, and the new-session button silently no-op'd because the persisted project list was empty. Patch the injected script: - Hide the home page project list by hiding the <aside> wrapping [data-slot='home-projects-scroll'] (covers the project list, add-project button, and per-project menus in one go). - Hide [data-action='project-switch'] on the session page so the click-to-switch entry is gone alongside the filtered project items. - Preseed localStorage opencode.global.dat:server with sd.list=[proxyUrl] AND sd.projects[proxyUrl]=[{worktree, expanded:true}] so the new-session button has a project to open (was empty → home page's create button silently no-op'd). - Extend r() to also rewrite root-relative paths (opencode's promise client builds 'new URL(path, baseUrl)' and was hitting the myworktree origin instead of the proxy → 404). - Extend ri() to rewrite URL objects (their .href) in addition to Request objects (.url). - Intercept the Worker constructor so Vite-bundled ?worker&url assets (Shiki highlighter, markdown worker) are routed through the proxy too. - Extend the L2 structural-anchor check with home-session-search and home-add-project so the L2 poll resolves on both the session page and the home page (was timing out on home).
… jank Replace the MutationObserver + querySelectorAll walk that hid [data-slot="home-projects-scroll"] (home project list) and [data-action="project-switch"] (project switch button) with a one-shot <style> injection, so the SSE-driven session stream no longer re-traverses the whole DOM on every mutation. The JS path still hides per-element foreign-worktree projects and the "Open project" button. DEBUG.md gains a debugging write-up for the full-page embed hardening round (issues 11-18, lessons learned). CHANGELOG entry added under Unreleased.
…e buffer cleanup Closes the code-review packet for feature/opencode-native-ui HEAD (e4158fa) plus one regression not surfaced by the review. Part 1 — REVIEW-e4158fa findings (9 files, +370/-54): * pty: per-instance subscriber scoping (multi-tab cross-talk fix). The kind-level subs map was global; every PTY tab received every other tab's output. Now keyed by instance id with an explicit empty-id rejection so a buggy caller can't subscribe to "everyone". TestSubscribeOutput_ScopedPerInstance + TestBroadcast_Parallel. * framework: spawn-time ring buffer ownership. PTY used to allocate its own buffer, which bypassed Manager.totalBufBytes / dropBuffer and silently broke the 25%-of-RAM budget. The framework now pre-allocates the per-instance buffer via Manager.AllocateBuffer and threads it through SpawnParams.Buffer; PTY panics if nil (framework contract, not user error). * opencode-web: pumpAndWatch now drains remaining stdout to EOF after parsing the listening line. Without this, the child filled the OS pipe (~64 KiB on Linux) and blocked on write(2) — long-run UI "stuck". TestPumpAndWatch_DrainsAfterListening. * opencode-web: Handle.publisher is now atomic.Pointer; readers in pumpAndWatch / healthLoop / wait go through loadPublisher(). Eliminates the -race hit on the bare-field read in hot paths. SetPublishers is now exactly-once (panics on second call). * opencode-web: Stop(graceSeconds) actually uses the parameter (was hardcoded 5s); 0/negative falls back to package default. * opencode-web: per-request proxy log moved from os.Stderr to Manager.Logger (default io.Discard), so SSE-driven traffic doesn't spam stderr and can't be poisoned by client-controlled dir/scope values. * Cleanup: dropped the var _ = log.Printf / var (_ sync.Mutex) placeholders that were keeping dead imports alive. * Documentation: REVIEW-e4158fa.md added for the review packet. Part 2 — buffer map leak in Manager.runLifecycle (1 file + tests): The review fixed PTY's buffer reaching the budget counter but missed the converse: runLifecycle's defer only decremented totalBufBytes without calling dropBuffer, so m.buffers[id] held a stale *RingBuffer for every instance that exited via lifecycle (Stop / kind-reported terminal status / ready-timeout). Long-running daemons accumulated dead buffers proportional to total stop/start cycles (~4-32 MiB each for PTY). Fix: replace defer m.totalBufBytes.Add(-capBytes) with defer m.dropBuffer(inst.ID) dropBuffer is idempotent (returns early when id absent) and is the contract specified in docs/ARCHITECTURE.md §"Buffer lifecycle" — "Stop / wait / Restart / Delete all funnel into dropBuffer". This realigns runLifecycle with that contract and lets the capBytes parameter be removed from the runLifecycle signature. Tests: TestRunLifecycle_DropsBufferOnTerminalStatus, TestRunLifecycle_DropsBufferOnReadyTimeout, TestStop_DropsBuffer. Verification: - go build ./... clean - go vet ./... clean - go test -race -count=1 ./internal/framework/... ok - go test -race -count=1 ./internal/instance/pty/ ok - go test -race -count=1 -short ./internal/app/ ok - go test -race -count=1 ./internal/instance/opencode_web/ ok (driver_test only; TestStartOpencodeWebEndToEnd fails on the sandbox env identically with and without these changes — TempDir cleanup race, unrelated.)
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Mirror the reasonix ensureWebFrame pattern so switching tabs no longer resets the embedded opencode page to about:blank and no longer unconditionally re-assigns iframe.src on every poll tick. - activate() now hides the panel and only resets the navigation cache when the instance is stopped; for a running instance whose iframe already shows the right src, visibility is restored and the loaded page (chat draft, scroll, app state) survives. - The poll only navigates when dataset.instance/dataset.src actually change, so repeated polls for the same running instance are no-ops. - Added ui_test regression TestOpencodeWebRendererKeepsFrameAlive that asserts the dataset guard is present and the unconditional navigation/reset lines are gone.
…ce tab switches Extend 49b18a1's single-iframe keep-alive to a per-instance iframe pool, so switching between two running opencode-web tabs only hides/shows their iframes instead of re-navigating the shared one (which would reload the embedded opencode SPA on every cross-instance switch and lose chat draft, scroll, app state). - Each opencode-web instance gets its own iframe cached in a Map keyed by instance id; activate() builds one lazily and shows only the active frame, leaving the others hidden but DOM-resident. - _pruneStaleFrames() drops cached iframes whose instance is gone, with an active-id guard so a transient state update can never force the currently-running instance to rebuild + reload. - A stopped instance no longer keeps its iframe around: activate() takes an early-return path that calls destroyFrame(session.id), and stopInstance()/deleteInstance() in index.html call destroyFrame immediately so dead iframes don't linger eating memory. - Cross-instance hidden-report cross-talk is fixed by filtering postMessage on event.source === this._currentFrame.contentWindow inside the scope listener, so only the active frame drives the warning. - ui_test assertions extended with destroyFrame, prune active guard, postMessage source filter, and _currentFrame cleanup hooks. Per-instance long-run memory cost is left as a follow-up: the user will profile tab-count vs. JS heap before deciding whether to add an LRU cap on _frames.
The injected CSS hid every [data-action="project-switch"] element, but the current worktree's project-switch entry doubles as the sidebar expand/collapse toggle (WORKTREE-ISOLATION.md §2.4 entry 2 — "只保留当前 worktree 项,其余隐藏/禁用"). Hiding it broke the toggle. - Scope the hide rule to :not([data-project="<current>"]) so the current worktree's entry stays visible while every other worktree's entry remains hidden. - The JS-side filter() already used the correct logic via foreign() (compare data-project === wt); only the CSS rule was over-broad. The two paths now agree. - TestBuildInjectScript extended with a positive guard on the new selector prefix to prevent the over-broad form from regressing.
… inject server displayName
Per OPENCODE-WORKDIR-DEBUG-2026-08-13.md §D.5: the proxy only monitors
client-supplied directories (no rewriting) so opencode keeps its
cross-directory capability; cross-worktree switch entries in the
embedded UI are now disabled (visible but inert) instead of hidden, so
the user still sees opencode's other projects as a reminder while
pointer events are blocked and the entries are greyed out (aria-disabled).
Server list injection now writes {type:"http", http:{url:pu}, displayName:<worktree basename>}
instead of just the proxy URL, so the search bar shows a project name
instead of the raw URL (WORKTREE-ISOLATION.md §0.3定位: 防误操作而非禁止).
L3 effectiveness report renamed from hide-failed → disable-failed and
the frontend warning text updated accordingly. Proxy now strictly
observes client directory (no rewriting) and only injects worktree as
the default-directory hint for directory-less GET/HEAD on API paths,
matching the original contract.
Tests:
- TestBuildInjectScriptSyntax uses node --check to catch CSP-hashed
inject-script syntax errors that would white-screen the embedded UI.
- newProxyTestUpstream captures every forwarded request; new assertions
verify out-of-scope directories are forwarded as-is and that GET
without a directory still gets worktree injected as default.
…arting + reusable loading overlay Per OPENCODE-WORKDIR-DEBUG-2026-08-13.md §E.7.4: - deactivate() now hides _currentFrame (visibility toggle, not teardown) so cross-instance tab switches no longer leave the previous instance's page visible on an empty panel. - activate() no longer bails on status !== 'running'; starting status keeps polling and the iframe auto-navigates once the server is ready, so the user no longer needs to manually re-click the tab to recover. - New #opencode-loading overlay (spinner + instance info lines) shown during starting, stopped, and timeout states; helpers _ensureLoading/_showLoading/_hideLoading are designed for reuse by future web-ui kinds. Front-end warning copy updated to match the proxy's hide→disable rename.
Captures the investigation, decision, and first-observation log for the opencode-web workdir phenomena reported on 2026-08-13: - §2 four-phenomena unified root-cause analysis (initial HOME-ization hypothesis, later revised in 附录 D after DB/runtime verification). - 附录 D final verdict: real cause is the stale worktree field on project a6e112c1ae... in opencode.db (the dir was a real checkout on 2026-06-19, cleared later but the row stayed); propagation is sync().project?.worktree → x-opencode-directory header → server defaultDirectory (header > proxy-injected query > cwd). - §D.5 chosen fix: proxy only monitors (no directory rewriting); UI cross-worktree switch entries disabled (not hidden); opencode data untouched (ghost sessions preserved). - 附录 E TUI vs web UI behavior split (same opencode server, different directory sources), and the E.7.4 cross-instance tab-switch 串台 fix that the previous two commits implement.
… (2026-08-14) Appends to OPENCODE-WORKDIR-DEBUG-2026-08-13.md: - E.7.5 single-server preseed (方案 A): bare-origin server entry so resolveServerList merges canonical + stored into one server and the home page renders like a native launch; defaultServerUrl + project preseed scoped under the canonical 'local' key. - Cross-instance localStorage clash (review finding) and fix: redirect the shared 'opencode.global.dat:server' key to a per-instance key (opencode.global.dat:server/__opencode/<id>) via getItem/setItem/ removeItem hijack, so each instance keeps its own preseed while still presenting a single merged server. - Review follow-ups: sendBeacon/WebSocket rewrite coverage + known dynamic-element limit, orphan-data rationale (no auto-cleanup), dn unified on jsQuote, pu0 -> origin0 rename, U+2028/U+2029 accepted.
…isolation + sendBeacon/WebSocket rewrite Per OPENCODE-WORKDIR-DEBUG-2026-08-13.md E.7.5: - Persist the server entry with the bare origin (same key as entry.tsx's canonical server) so resolveServerList merges them into one server and the home page renders single-server mode; set defaultServerUrl to the origin and preseed the project under the canonical 'local' scope. - Isolate the shared 'opencode.global.dat:server' key per instance (opencode.global.dat:server/__opencode/<id>) by hijacking getItem/setItem/removeItem, so two instances on one origin no longer clobber each other's preseed (review finding: cross-instance clash). - Extend r() rewrite to ws://wss:// URLs and hijack sendBeacon + WebSocket so every SDK channel reaches the proxy prefix. - Harden interpolated values with jsQuote (strconv.Quote + \u003c) so a worktree path can never break out of the <script> element. Tests: TestBuildInjectScriptSingleServer (dual-instance sim with resolveServerList merge mirror), TestInjectURLRewriteBehavior (behavioral rewrite coverage across fetch/XHR/EventSource/sendBeacon/ WebSocket), TestBuildInjectScriptEscapes (script-tag breakout guard).
…ng) + hide terminal controls + drop debug bar - proxy.go / opencode_web.js: rewrite URL prefix at new Request() construction time instead of rebuilding in fetch(). Pre-SDK-every-request goes through new Request(url, init), so intercepting the constructor prefixes the URL at source — fetch() receives an already-prefixed Request and passes it through unchanged. Zero rebuild, zero body/duplex round-trip. This matches the pre-single-server path (worked on every browser, including Safari's stricter ReadableStream handling). ri() rebuild stays as a fallback for bare-origin strings. Covers string / URL.href / Request copy (u.url) shapes. - proxy_test.go: tests for Request constructor interception, Request copy construction (must keep prefix + method + body), and the streaming-upload invariant (duplex:half + body stream + content all survive the rewrite). Reads back the rebuilt body via getReader() so a missing body lands in the call log instead of silently passing. - opencode_web.js: remove the bottom debug bar (the embedded page owns its own status UI now). Stopped-state path no longer polls back into the bar. - index.html: hide #terminal-controls when active instance is opencode-web (refresh / scroll-to-bottom shortcuts overlay the embedded page and are meaningless there). Class toggle, not inline style, so CSS keeps layout control. - docs: add FOLLOWUPS.md (review follow-ups: extract inject script, Playwright smoke test, old-data cleanup) + update README to point at it. Trim the long debug doc to final conclusion only (process & wrong hypotheses removed).
…uthority for session maps deactivate() no longer disconnects the WebSocket, so re-activating a hidden terminal shows up-to-date buffer content instead of a stale 64KB-capped log replay (matches pre-renderer behavior on main/develop). reconcileTerminalSessions / reconnectRunningTerminalSessions / renderTerminalSessions now iterate the PtyRenderer as the single authority for live sessions, falling back to the legacy map only when the renderer is absent (unit tests / embeds without pty.js). Fixes the silent no-op on stopped-session cleanup and reconnect for renderer sessions. renderTerminal Sessions also guards against container-less sessions on future async teardown. Follow-up notes recorded in FOLLOWUPS.md.
Preserves the released reasonix feature by porting it onto the opencode-native-ui kind architecture: - internal/instance/manager.go taken as deleted (branch refactor wins); its reasonix logic moved to a third framework kind (internal/instance/reasonix/kind.go wrapping Driver.Start) with Stop/Cleanup, Reattach (RestartSurvivor), and Status parity. - framework: RestartSurvivor re-attach in ReconcileRunningOnStartup, StopAllKind for shutdown, ResourceCleaner for Delete/Restart, persistNewInstance version-conflict retry, SetInstancePID via Publisher. - Restored tag semantics for all kinds: tag.Env / preStart / Command / Cwd resolve at Start (pty sends initial command, opencode-web and reasonix run preStart with kind-consistent env; reasonix strips REASONIX_HOME/REASONIX_STATE_HOME from preStart and serve alike). - app: independent loopback /rx/ listener + web_url (issue #44), same-origin fallback, instanceView, dedicated kind registry. - Frontend: third Reasonix modal tab, kinds/reasonix.js renderer with per-instance iframe keep-alive (issues #53/#54/#55), rx badge. - Tests ported to framework APIs; ui tests updated for the renderer-single-authority reconcile refactor; gofmt clean for CI. - Docs: API.md sections 5.10-5.13 merged/renumbered, ARCHITECTURE and PRD updated for the kind architecture and restored tag semantics.
The Reasonix tab now only asks for a name: the serve command is fixed and the UI sends an empty tag_id. Backend support is intentionally kept — tag env / preStart / cwd still apply when a tag_id is passed via the API or CLI (HTTP_PROXY, REASONIX_HOME opt-out, pre-serve setup), so no v0.4.0 capability is lost. - internal/ui/static/index.html: remove tagSelectRx select + its renderModals population + promptStartInstance reset; doStartInstance sends tag_id:'' for the reasonix tab. - internal/ui/ui_test.go: TestReasonixTabIsFixedCommandNoTemplate pins the fixed-command shape and the absence of the template picker. - docs/PRD.md: Reasonix tab description updated (name only; env/ preStart via API/CLI tag_id).
…sting tags.json Two startup regressions from the merge, both verified by tests: 1. Reasonix web UI never rendered and the terminal looped on 'kind "reasonix" does not support output subscription': the <script> tag for /static/kinds/reasonix.js was missing from index.html, so window.KindRenderers['reasonix'] never registered and selectInstance fell back to the PTY path (TTY WS → SubscribeOutput error → retry). Added the script tag; the regression is pinned in TestReasonixTabIsFixedCommandNoTemplate. 2. Opencode-Web startup alerted "unknown tag id: opencode-web": with tag semantics restored, the tab's tag_id must resolve, but the built-in opencode-web tag was only written when tags.json was first created — pre-existing configs lacked it. tag.Manager. LoadMerged now merges the built-in default tags as the base layer at load time (user file entries still override), so the command-less opencode-web reference tag always resolves. Pinned by TestLoadMergedDefaultsSurviveExistingFile.
Three frontend issues reported after the merge, all resolved at the UI layer (backend verified healthy: real-reasonix two-instance scratch run — Start <1s each, /rx/ proxy 200 in ~30ms for both): 1. Stacked half/half iframes: switching between opencode-web and reasonix tabs left BOTH panels visible because deactivate() only hid the iframe. selectInstance now hides both web panels before dispatching, and each renderer's deactivate() hides its panel. 2. Second reasonix instance white: the per-instance iframe cache kept several reasonix SPA pages alive simultaneously under one origin. Rebuilt ReasonixRenderer on the proven v0.4.0 model: ONE shared iframe, re-navigating on instance switch; hide-not-destroy keep-alive across non-reasonix tabs (issue #53); invalidate-on-stop (same-src fallback regression); issues #54/#55 xterm cleanups kept. 3. Startup stall: the renderer now navigates while the record is running OR starting (Driver.Start already waited for the listen port, and the proxy accepts starting), closing the async status-flip race that left a blank panel after Start. Tests: TestReasonixRendererKeepsFrameAlive updated to pin the single-frame model (and reject a per-instance cache); full suite + vet green.
…heal poll Per user decision: cross-instance switches must not reload and lose drafts. Restores the per-instance iframe cache (each reasonix instance keeps its own frame; switching hides/shows instead of re-navigating, so instance A's draft survives A→B→A — beyond main's single shared frame, which reloaded), plus the fixes gathered while diagnosing the white screen: - navigation only while the record is running/starting (serve is already listening; proxy accepts starting) — closes the async status-flip race that left a blank panel; - a 2s self-healing poll re-navigates on web_url port changes (daemon restart), invalidates on stop/failure, and retries fresh frames — restoring main's per-render-tick re-check; - panel mutual exclusion (selectInstance hides both web panels before dispatch; deactivate hides its own panel) — no stacked half layout; - [reasonix-renderer] console diagnostics so any remaining white screen can be reported with the actual activation/navigation log. Tests: TestReasonixRendererKeepsFrameAlive pins the per-instance cache and the poll/guard hooks. Full suite + vet green.
Reorganize the intro sections of both READMEs to lead with a punchy tagline (an ORCA-like lightweight agents team orchestrator), a curated 'Features' / '核心能力' block using ORCA-style wording for the common capabilities, and a separate 'What makes myworktree different' / 'myworktree 的特色' block for the distinctive ones. - Tagline reframes myworktree as 'ORCA-like' without explicit comparison. - 'Bring your own agents' now leads with OpenCode + Reasonix (native web UI embedded in the sidebar); Tag templates stay as the path for everything else. - 'Output replay' bullet reworded: the PTY ring buffer is in-memory only, so 'survives restarts' was an overstatement — replaced with 'without disk writes'. - 'Fan one prompt across many worktrees' replaced: myworktree does not auto-fan-out a prompt; users create worktrees and start instances themselves. New bullet is 'One worktree per task, kept apart by git'.
Resolve the post-v0.4.0 fork between the release line (main: v0.4.2 opencode-web snapshot + one-line installer) and the dev line (develop: PR #60 opencode-native-ui with follow-up fixes): - opencode-web / framework / reasonix / pty Go code: keep develop side (PR #60 state, strictly newer than the v0.4.2 snapshot) - CHANGELOG: merge v0.4.2 entry into Unreleased - README / README.zh-CN: keep ORCA-style positioning (local commit 5b3bf54) + adopt main's Quick start / one-line install section - adopt main-only assets: scripts/install.sh, ci release.yml, docs/REVIEW-pr61.md, docs/plans/opencode-native-ui DEBUG doc - .gitignore: ignore .gocache/ (already ignored on feature branches)
* docs(dsh-native-ui): add DeepSeek Harness web UI native-instance feasibility report
* docs(dsh-native-ui): add missing-binary handling — interactive npx/install choice
* docs(dsh-native-ui): switch workspace restriction to opencode-style — disable entries, monitor only, no interception
* docs(dsh-native-ui): add workspace bootstrap — inject worktree via workspace.create after ready
* docs(dsh-native-ui): record port-conflict semantics — no fallback on bind failure, pin --port 0
* docs(dsh-native-ui): add per-worktree data isolation — redirect storages/sessions roots via overlay
* docs(dsh-native-ui): session-plane tradeoff — shared sessions vs full isolation, pending decision
* docs(dsh-native-ui): reorganize as final-decisions document; drop rejected options, keep pitfalls
* docs(dsh-native-ui): FEASIBILITY 实施修订 + 跨进程会话边界
- 记真机验证（dsh 0.1.0-rc.6）后的四项实施修正：
* restrict overlay 停 directory-picker 自动组合器 + 裸挂
directory-picker-browse host 后端行（保留 api-gateway 依赖的
directoryPicker 服务、不挂 client 表面 → Add workspace 入口
不渲染；直接禁用整行会触发 '1 entry did not activate'）
* dsh web 启动命令 flag 顺序必须 launcher 优先：
'web --patch <overlay> --host 127.0.0.1 --port 0'
（web subcommand 遇到第一个未知 option 即 pass-through；--patch
放 --host 之后会被透传给 app 层报 unknown option）
* RPC 信封 endpoint 从 URL 路径派生：POST /api/<method>，body
'method' 字段必须与 endpoint 一致（打裸 /api 会 404）
- 跨进程会话边界限定为'可见、可打开、可读快照'：
* 会话实时事件只在写入者进程内广播（无 fs.watch/轮询）
* 第二进程打开活跃会话的构造器无锁追加 session/end-seed
(seq = log length) 撞 seq → 日志损坏（corrupt session log:
seq gap in committed region）；详见 CROSS-PROCESS-SESSION.md
- §2.2 '终端裸跑 dsh 与 web 实例会话互通' 同限定为快照级
* docs(PRD): add dsh-web instance type entry, status 已实现
Per review (REVIEW-2026-08-15.md #10 / REVIEW-2026-08-15-r2.md 必改 #1):
the dsh-web PRD entry was missing in HEAD and the '规划中' status
written in the working tree was stale — the implementation lives in
internal/instance/dsh_web/, CHANGELOG has 'feat(instance): dsh-web
kind implemented' in Unreleased, and README feature list already names
dsh-web.
Add the full entry mirroring the opencode-web paragraph above it:
hardcoded spawn (dsh web --patch <restrict.yml> --host 127.0.0.1 --port 0,
launcher flags first — PLAN §踩坑 11), shared DSH_HOME with per-worktree
storages (sessions shared = snapshot-only across processes, upstream
dsh issue — see CROSS-PROCESS-SESSION.md), per-instance dedicated
loopback origin (SPA API base hardcoded to location.origin + '/api'),
workspace bootstrap via workspace.create, restrict overlay (disable
directory-picker composer + bare -browse host backend, client-hmr —
PLAN §踩坑 12), proxy-side scope record-only with persistent warning
bar, hard version gate (< 0.1.0 fail-fast) + advisory [0.1.0, 0.2.0)
+ L2 --dump-config overlay verification (overlay_verified), three-way
missing-dependency dialog (npx pin / install / cancel), remote mode
(non-loopback main listener or TLS) with mandatory myworktree token
gate on the per-instance reverse proxy.
* docs(dsh-native-ui): implementation plan, task checklist, cross-process session analysis
Implementation record for the dsh-web integration (PR 1 of 5 — docs-first).
FEASIBILITY.md and the PRD entry (the public doc) already landed in earlier
commits; this commit is the in-team working docs.
- PLAN.md — 5-PR implementation plan in dependency order (docs → core →
proxy/app → frontend → remote), with security posture, key decisions
(per-instance dedicated loopback origin, shared DSH_HOME + per-worktree
storages, record-only scope monitoring, npx/install launch modes, version
gate + L2 overlay verification), and the 11–15 实施踩坑 (launcher flag
order, directory-picker composer not to be outright disabled, RPC wire
endpoint, --dump-config needs writable DSH_HOME, cross-process session
log corruption upstream dsh bug).
- TASK.md — PR-level checkbox mirror of PLAN §实施步骤.
- CROSS-PROCESS-SESSION.md — forensic record of the upstream dsh session
bug (session/end-seed appended without locking by the second reader →
seq collision → log corruption), byte-level evidence from the real
~/.dsh/sessions/session-d95142a8-…/session.jsonl.zstd, source citations
in dsh-session/lib/index.js and dsh-host-apiproxy, full timeline tying
the 3080 terminal process's writes to the 34463 embedded instance's
open, reproduction steps for upstream reporting. Root cause + design
boundary separated (现象 A = single-writer broadcast model, 现象 B =
implementation bug); myworktree side keeps 可见/可打开/可读快照
posture and does not implement cross-process sync.
* feat(instance): dsh-web kind (DSH_HOME share + per-worktree storages + per-instance loopback proxy)
internal/instance/dsh_web/ — a self-contained framework.Kind package that
embeds the DeepSeek Harness web UI (dsh web) as a managed instance.
Mirrors the opencode_web pattern; intentionally independent of app/,
pty/, reasonix/, or opencode_web/.
Driver (driver.go)
- framework.Kind 'dsh-web' (Interactive:false). Spawn pipeline:
launch-mode resolve → dsh LookPath preflight → hard version gate
(core < 0.1.0 fail-fast, reasonix #45 pattern) → write restrict.yml
→ L2 --dump-config verification (overlay_verified) → exec with
Setpgid for npx mode → pumpAndWatch scans ANSI-stripped stdout for
'dsh web: http://<host>:<port>' ready line → starts per-instance
reverse proxy via ProxyStarter seam → ready. Stop signals the whole
process group for npx mode (kill(-pid, SIGTERM) → grace → SIGKILL).
Health loop probes GET / at 5s/3-strike.
- tag preStart runs under zsh -lc with redact.Text scrubbing.
Restrict overlay (overlay.go)
- storage-json.root → <dataDir>/dsh/<gitx.HashPath(worktree)>/storages
(per-worktree registry isolation; sessions stay shared in ~/.dsh/).
- directory-picker.disabled + insert @deepseek-ai/dsh-host-directory-
picker-browse bare host backend row (preserves the directoryPicker
service that api-gateway hard-depends on while suppressing the
'Add workspace…' client surface — PLAN §踩坑 12, fixed in -rc.6).
- client-hmr.disabled (production hygiene).
- File mode 0o600 (carries worktree paths).
- verifyOverlayDump checks the 4 expected rows in --dump-config
output, block-scoped (a 'disabled: true' on a different row cannot
satisfy the directory-picker check).
Per-instance reverse proxy (proxy.go)
- Loopback bind by default; binds the main listener's host with a
mandatory token gate when the main listener is non-loopback or TLS.
- Director: Host→upstream loopback, Origin deleted, authq.StripToken
(token never reaches the dsh subprocess, neither via query nor via
header), Accept-Encoding passthrough, FlushInterval=-1 for SSE/WS.
- WS Upgrade passed through (events.mux / events.host).
- Token gate: ?token= or mw_token cookie; on query-token success the
proxy HttpOnly-sets the cookie on ITS origin and 302-redirects to
the token-free URL, so the embedded document never retains the
myworktree token in its own location.search (ARCHITECTURE §9
checklist item 9).
- observeBody reads POST /api bodies (capped at 16 MiB) to record
workspace.create {path} / session.create {cwd|workspaceId} into
ScopeTracker. Record-only, never blocks or rewrites. Bodies are
re-attached for forwarding; oversized bodies are passed through
untouched via MultiReader.
- Unexpected listener death (TLS reload failure etc.) fails loud:
logs + MarkFailed + iframe_url cleared + handle teardown; the
health loop probes only the upstream, so this is the only way the
dead proxy becomes visible. Pinned by TestProxyListenerDeathFailsLoud.
Scope classification (scope.go)
- normalizeDir canonicalizes a path (clean + EvalSymlinks + trim sep)
so symlink / .. / trailing-slash differences do not false-positive.
- classifyRPC: dir vs worktree (cwd) — workspaceId vs worktree's own
id (from bootstrap); when bootstrap hasn't landed yet and only
workspaceId is supplied, returns ok=false (no observation) so the
worktree's own workspace isn't flagged as out-of-scope.
Workspace bootstrap (bootstrap.go)
- After the ready line, Go http.Client posts POST /api/workspace.create
DIRECTLY to the upstream loopback (no Origin → passes the /api trust
fence). Wire contract: endpoint derived from URL path, body method
must equal endpoint — verified against the real binary (PLAN §踩坑 13).
- 3×2s retry on transient failure, warn-only on give-up.
- Records worktree's own workspaceId in blob for scope classification.
- bootstrapCreateSession = true preseeds a blank session.
Launch modes (launch.go)
- path / npx / install persisted PER WORKTREE at
<dataDir>/dsh/<worktreeHash>/launch.json (per-worktree because
framework Restart allocates a fresh instance id and wipes the
per-instance state dir — a per-instance launch file would be
orphaned on the failed → choose npx → Restart path).
- npx mode runs the pinned command: npx --yes @deepseek-ai/dsh@<NpxPin>
web ...; Setpgid, kill the whole group on Stop. The pin is the
exact version the integration was verified against.
- install mode records the absolute bin path resolved by
'npm prefix -g' (no daemon restart needed).
- webArgs: launcher flags MUST come first (--patch <overlay>
--host 127.0.0.1 --port 0). PLAN §踩坑 11: the web subcommand
switches to pass-through at the first unknown option, so
--patch after --host is forwarded to the app layer and
rejected as 'unknown option'. dumpArgs has the same order for
the L2 --dump-config verification.
Version gate (version.go)
- minVersion = 0.1.0, supportedMaxVersion = 0.2.0 (exclusive),
NpxPin = 0.1.0-rc.6 (the version the launcher flag order, the
directory-picker composer behavior, and the RPC wire were all
verified against).
- parseVersion tolerates -rc.N suffixes; 'dev' is logged and allowed.
- verMemo memoizes successful probes per binary path (reasonix #45).
Tests (driver_test / overlay_test / version_test / launch_test /
scope_test / bootstrap_test / proxy_test / integration_test)
- mock upstream at testdata/dsh-mock.go.
- End-to-end via framework.Manager: Spawn → ready → health → Stop,
proxy + upstream dual port released, npx process group has no
residual after Stop.
- Token gate matrix: no / wrong / query / cookie / query→cookie
redirect, token never leaked upstream.
- WS passthrough: real socket round-trip through a real listener.
- L2 dump-config with the composed overlay rows.
- 11-case scope classification matrix.
- Hard version gate fail-fast on 0.0.9 mock.
- ErrDshNotFound surfaced on missing binary.
- Proxy listener death → MarkFailed + blob iframe_url cleared.
- Bootstrap envelope + retry + warn-only on give-up + workspaceId
persisted in blob.
* feat(app): dsh-web kind registration + 4 API endpoints + shutdown
Wires the dsh_web package (from the previous commit) into the main
myworktree daemon.
- internal/store/state.go: KindDsh = "dsh-web" alongside the existing
KindPTY / KindReasonix / KindOpenCodeWeb constants.
- internal/tag/tag.go: defaultTags appends {ID: "dsh-web"} — like
opencode-web, the tag is a label / env / preStart source only; the
hardcoded dsh web invocation lives in the driver.
- internal/app/app.go:
* Server struct gains dshScope (*dsh_web.ScopeTracker) and dshDrv
(*dsh_web.Driver) plus dshInstallMu (sync.Mutex, serializes the
npm install -g flow so two tabs clicking install now don't run
two concurrent global installs).
* New() registers the dsh_web driver via the per-server Registry
(NOT framework.Default — see comment for the test-rerun reason).
The driver's Tracker / Proxy / ProxyStarter are filled here from
the main listener settings.
* dshProxyConfig derives the per-instance reverse-proxy settings
from the main listener: loopback bind with no token locally; when
the main listener is non-loopback or TLS, binds the main
listener's host with a MANDATORY token gate (dsh has no
authentication layer — the proxy is the only boundary between
the LAN and the unauthenticated upstream). TLS mirrors the main
certificate so an https page never embeds a mixed-content iframe.
* 4 new routes:
GET /api/instances/dsh — handleInstanceDshInfo
GET /api/instances/dsh/scope — handleInstanceDshScope
POST /api/instances/dsh/launch — handleInstanceDshLaunch
POST /api/instances/dsh/install — handleInstanceDshInstall
All four enforce method + id + dsh kind guards; non-dsh instances
get 404 before any dsh driver touch.
* handleInstanceDshInfo reports iframe_src (built from the caller's
r.Host when the proxy binds 0.0.0.0 — non-loopback mode), proxy
host/port, worktree path, version, version_supported, overlay_
verified, and recomputed missing_dsh (when the instance is
failed with a structured ErrDshNotFound). npm-available +
suggested pin are surfaced so the frontend can render the three-
way dialog.
* handleInstanceDshInstall runs npm install -g @deepseek-ai/dsh
under dshInstallMu (5 min timeout), then npm prefix -g to resolve
the absolute bin path, and persists it via Driver.SetInstallBin
(no daemon restart). npm output is truncated to 300 chars after
redact.Text scrubbing (private-registry URLs, lock dumps, etc.).
* Shutdown() adds StopAllKind(store.KindDsh) so SIGTERM cleans up
running dsh-web instances alongside the existing PTY / opencode-
web / reasonix cleanup.
- internal/app/app_dsh_test.go:
* newIsolatedTestServerWithDsh mirrors app.New for tests.
* Dsh kind guard 404 on non-dsh instances.
* Remote host mapping: 0.0.0.0-bound proxy gets the caller's host.
* Scope endpoint reads from the in-memory tracker.
* Launch endpoint rejects bogus modes + non-dsh instances.
* Install endpoint kind-guard test (the real npm run is exercised
manually — it mutates the global npm prefix).
* feat(ui): dsh-web renderer (iframe keep-alive, scope warning bar, missing-dep dialog)
- internal/ui/static/kinds/dsh_web.js (new, DshWebRenderer):
* activate() hides the PTY terminal container, shows #dsh-panel,
prunes stale iframes, and either reuses the existing frame for
the instance or shows a "starting…" placeholder.
* _startPolling polls /api/instances/dsh?id=... every 500ms up
to 60s for the iframe src. On success: cache the tokenized
src on the frame's dataset, mark the frame's instance, and
unhide. On missing-dsh: show the three-way dialog but keep
polling (a cancel only dismisses the dialog until the next
recovery/restart outcome — never stops the poll, so an install
or PATH fix clears the dialog automatically).
* _tokenizeIframeSrc appends ?token= only when this page is
served remotely (non-loopback); the proxy validates the first
navigation, HttpOnly-sets mw_token on its own origin, and
302-redirects to a token-free URL, so the embedded document
never keeps the credential in its own location.search. Locally
the iframe stays token-free.
* _startScopeMonitoring polls /api/instances/dsh/scope?id=...
every 1.5s and drives a three-state warning bar above the
iframe (record-only, never blocks): restriction ineffective
(version out of supported range OR L2 --dump-config did not
confirm the overlay rows) → danger bar; out-of-scope session
→ orange bar with the directory; normal → hidden.
* Missing-dependency dialog: dsh 未安装 + three buttons (npx /
install / cancel). The install button is a two-step confirm
(the install rewrites the global npm prefix — make the user
click twice; the second click sends POST /api/instances/dsh/
install and then auto-restarts the instance). npm_available
from the info endpoint hides the install button when npm is
not on PATH.
* Cleanup destroys all per-instance iframes (stops the keep-alive
cache) and clears scope-monitoring state.
- internal/ui/static/index.html:
* CSS for #dsh-panel / #dsh-frames / .dsh-frame / #dsh-loading /
.dsh-loading-{spinner,title,info} / #dsh-scope-warning /
.dsh-warning-danger / .badge-dsh.
* #dsh-panel DOM (scope-warning + frames).
* Dsh-Web start-instance tab (modal-tab-panel) with name input.
* #modal-dsh-missing dialog (titlebar, body, three actions,
progress + error paragraphs).
* Web-ui kind lists collapsed into one WEB_UI_KINDS constant
(panel mutual exclusion, controls.oc-hidden toggle, pty tags
filter, stop-instance destroyFrame, delete-instance loop).
* The DS badge next to instance names of kind dsh-web.
* <script src="/static/kinds/dsh_web.js"></script> after
kinds/reasonix.js.
- .gitignore: ignore .gocache/ (sandbox GOCACHE build artifact).
* docs(dsh-native-ui): API + architecture + changelog + readme sync
Public-facing doc sync for the dsh-web integration. The PRD entry landed
in a993ec3; this commit is everything else.
- docs/API.md §5.14–5.17: dsh instance endpoints
* GET /api/instances/dsh?id=... — iframe_src,
proxy_host/port, host/port, worktree_path, version,
version_supported, overlay_verified, optional missing_dsh;
404 on non-dsh instances.
* GET /api/instances/dsh/scope?id=... — last observed
scope (in-scope | out-of-scope) + directory + unix at;
404 on non-dsh instances.
* POST /api/instances/dsh/launch — body {id, mode}
where mode ∈ {path, npx, install}; 400 on unknown mode.
* POST /api/instances/dsh/install — body {id}; runs
npm install -g @deepseek-ai/dsh (5 min timeout, mutex
serialized, output truncated 300 chars after redact.Text);
returns {status, resolved_bin}.
- docs/ARCHITECTURE.md §9 (new): dsh-web integration
* ASCII architecture diagram (myworktree main UI ↔ per-instance
reverse proxy ↔ upstream dsh web, with the trust boundary
and the data plane).
* Threat model: loopback isolation (local), token gate (remote),
credential hygiene (?token= strip, cookie HttpOnly+Secure,
token never reaches the dsh subprocess), cross-worktree
drift (record-only, matches opencode/reasonix philosophy),
dsh sandbox (OS-level, the real write boundary for the
embedded UI).
* Review checklist (dsh-specific, 9 items): token gate cannot
be disabled, token scrubbing, Origin deletion, WS upgrade
passthrough, process-tree cleanup (npx Setpgid), row-id drift
(L1 advisory + L2 --dump-config), bootstrap scope leak
(worktree path only, never client-supplied), body parsing
(record-only fallback), and the global §8 items 1–9 carry
over (auth file 0o600, no token in logs/tests, cookie
Secure/Domain/Path, iframe document never carries the token
in its own location.search).
* Cross-references to FEASIBILITY.md / PLAN.md / TASK.md /
CROSS-PROCESS-SESSION.md and the §9 implementation files.
- CHANGELOG.md (Unreleased): add the dsh-web kind implemented entry
(full implementation summary) plus the 实施修订 / fix entries
(restrict overlay must not disable directory-picker outright,
bootstrap RPC wire is POST /api/<method>, cross-process session
boundary record) and the review-fix batch (302 redirect, loopback
bypass, listener death fail-loud, install two-step + mutex +
truncate, restrict.yml 0o600, npx pin → 0.1.0-rc.6, WEB_UI_KINDS
constant, status wording sync).
- README.md / README.zh-CN.md: Feature list adds dsh-web to the
per-worktree instance roster; new section "dsh-web sessions are
snapshot-only across processes (upstream dsh issue)" with the
non-technical summary + pointer to CROSS-PROCESS-SESSION.md.
* docs(review): dsh-web integration code-review audit trail
Three review documents, one per review pass, kept together so the
audit trail survives even when a reviewer is reading the branch
years later. Each file is a point-in-time assessment (REVIEW-<DATE>.md
+ a -2 / -r2 suffix for re-reviews); filename convention documented
in the first file's header.
- REVIEW-2026-08-15.md: first pass. 13 findings ranked by risk,
from the install endpoint (no audit / mutex / truncate) through
the proxy listener death silent-swallow to the cosmetic WEB_UI_KINDS
refactor suggestion. All findings tied to specific file:line
ranges and to the opencode-web precedent where relevant.
- REVIEW-2026-08-15-2.md: author's own re-review with a re-framed
11-item risk list, fix-tracking table, and a process note that
the original 13-finding review is the source of truth.
- REVIEW-2026-08-15-r2.md: follow-up review verifying the fix
batch (302 redirect, listener fail-loud, install two-step +
mutex + truncate, polling continuation on missing_dsh, restrict.yml
0o600, WEB_UI_KINDS constant, NpxPin = 0.1.0-rc.6). 6/13 修或
部分修; 2 条接受不修（#2 跨进程会话嗅探 / #5 r.Host，作者论据
合理）; 5 条 follow-up. Single remaining 必改 = docs/PRD.md:75
stale '规划中' status, fixed in a993ec3 (which landed BEFORE the
re-review but is referenced as the post-fix state in the doc).
Read in order: 1 → 2 → -r2. The two 必改 items that the re-review
identified (PRD status line, CHANGELOG fix batch) both landed
before this commit, so the re-review's "必改 before commit" wording
reflects the snapshot at review time, not the current HEAD.
No code changes in this commit. The review documents are an audit
trail for the four preceding commits, not part of the implementation.
* feat(dsh-web): follow-up issues #62-#67 (REVIEW-2026-08-15-2)
Review follow-ups for the dsh-web feature on feature/dsh-native-ui,
implementing the six follow-up issues filed at
docs/plans/dsh-native-ui/REVIEW-2026-08-15-2.md.
- #63 foreign-session activity advisory (record-only mitigation of
the dsh single-writer log-corruption boundary, see
CROSS-PROCESS-SESSION.md): a daemon-level SessionWatch
(sessionwatch.go) scans $DSH_HOME/sessions every 3s, treats
session.jsonl.zstd modified within 90s as active, and subtracts
sessions this daemon itself drives. Own-attribution sources:
* write-driving session.* RPC bodies observed through the
per-instance proxy (session.prompt / .cancel / .fork /
.rename / .selectModel / .attachment / .updateQueue;
subagent.prompt uses both parentSessionId and childSessionId);
read-only methods (session.history / .models / .list /
.search) are deliberately excluded — opening a session
does not write, and marking it would wrongly suppress the
warning for a session a terminal dsh is actively writing;
* session.create response (id lives in the value, not the
body) — teed via a sessionCreateBody wrapper that parses
the envelope once the stream ends. The real upstream value
shape is {sessionId: …}; the mock testdata was wrong and is
fixed in the same change;
* workspace-bootstrap preseed (createSession in bootstrap.go).
Remaining active sessions surface as foreign_active_sessions
on /api/instances/dsh/scope, and the frontend renders a
persistent warning bar telling the user to wait for the
session to idle. Record-only — no iframe DOM changes.
- #64 upstream mid-body failure abort: ErrorHandler cannot emit
a 502 once the response headers are on the wire, so the
previous code truncated the body silently. A new
headerTrackRW wrapper records whether headers were written
(WriteHeader, or the implicit 200 from the first Write) and
ErrorHandler panics http.ErrAbortHandler in that case so the
browser fails loud. Unwrap preserves http.ResponseController
Hijack/Flush paths (WebSocket upgrade still works).
- #65 validate r.Host-derived iframe URL: r.Host is client-
controlled and is the only source of a routable host when
the dsh proxy binds 0.0.0.0/::. A new validPublicHost only
allows plain DNS names, IPv4 literals, or bracketed IPv6; a
numericPort guard requires the host:port split to actually
have a numeric port before the host part is trusted. Any
failure path yields an empty iframe_src — the frontend
stays on the 'starting' placeholder instead of a dead URL.
- #66 remote iframe auth hint: when a remote page has neither
mw_token cookie nor ?token= in the address bar, the proxy's
mandatory token gate will 401 the first iframe navigation.
The renderer now detects this case and shows a 'remote
access needs a token' warning bar before the guaranteed 401,
in front of the existing restriction-effectiveness /
foreign-active / out-of-scope priority chain.
- #67 dedupe test helper: managedTestInstance(id, kind, …) in
internal/app/testhelper_test.go is the shared per-kind
ManagedInstance stub; the dsh tests (app_dsh_test.go) drop
the kind-specific dshTestInstance and call it instead. Future
web-ui kinds reuse the same helper.
- #62 pr5-remote-e2e.sh: half-automated LAN+TLS end-to-end
coverage (real myworktree daemon bound 0.0.0.0 with self-
signed TLS, real dsh 0.1.0-rc.6, isolated DSH_HOME /
worktree). Asserts: no token → 401 / first nav with
?token= → 302 Location=/ + Set-Cookie → 200 / cookie only →
200 / loopback passthrough → 200 / WS Upgrade → 101 Switching
Protocols / session-watch attribution (own preseed excluded,
forged foreign session raises the warning, deletion clears
it) / shutdown leaves no orphan processes. The full browser
click-through (SPA render, interactive WS frames) remains
manual per TASK PR5.
Docs: CHANGELOG §v0.4.3 (unreleased); API.md describes the new
foreign_active_sessions field; ARCHITECTURE.md §9 adds a
'Foreign-session activity advisory' paragraph and lists
sessionwatch.go in the file map; PLAN.md / TASK.md /
CROSS-PROCESS-SESSION.md get status markers and the 'mw
side' mitigation is now marked implemented (warn-only).
Tests: TestSessionWatchScanForeignActive /
TestSessionWatchOwnAttribution / TestSessionWatchMissingDir /
TestProxySessionOwnMarking /
TestProxySessionCreateResponseMarking /
TestProxyUpstreamMidBodyFailureAbortsConnection /
TestHandleInstanceDshInfoHostValidation /
TestHandleInstanceDshScopeForeignActiveSessions.
* docs(dsh-native-ui): record server-tokenized iframe_src fix (REVIEW-2026-08-15 #13 closure)
The original issue #13 (REVIEW-2026-08-15.md) documented that the
remote iframe needed ?token= on the first navigation. The previous fix
pushed the tokenization to the frontend (cookie → address-bar fallback),
which regressed in the portal/login flow: mw_token is HttpOnly (page JS
can never read it) and portal/login has no ?token= in the address bar,
so logged-in remote sessions got a false '远程访问需要认证' warning AND
a token-less cross-origin iframe URL that 401'd. The follow-up fix
moves tokenization back to the server (handleInstanceDshInfo), reading
Proxy.AuthToken (the field checkToken validates against).
CHANGELOG: new entry under ## Unreleased for the regression fix.
API.md / ARCHITECTURE.md §9: updated Remote access wording to reflect
that the server appends ?token= to iframe_src (page JS cannot read the
HttpOnly cookie).
REVIEW-2026-08-15.md: appended '附2: 服务端 token 化归档
(2026-08-16, issue #13 的回归修复)' — regression symptom, root cause,
fix contract, and verification (so the review file evolves from a
checklist into a checklist + fix archive).
* fix(dsh-web): server appends ?token= to iframe_src when proxy gate is on
Remote portal/login sessions were hitting a token-less cross-origin
iframe URL (the proxy's mandatory token gate 401'd) AND a false
'远程访问需要认证' warning. Root cause: the frontend tried to read
mw_token from document.cookie, but the cookie is HttpOnly — page JS
cannot read it — and portal/login flows carry no ?token= in the
address bar.
The server now owns the tokenization:
- handleInstanceDshInfo (internal/app/app.go) appends
?token=<Proxy.AuthToken> to iframe_src when the proxy's token
gate is on. Reads Proxy.AuthToken (not cfg.AuthToken) so the URL
stays correct even if the main token and the proxy token are
ever split. Sits behind withAuth (loopback bypass included), so
only authenticated clients receive it; the proxy then validates,
syncs its own HttpOnly cookie, and 302-redirects to the
token-free URL — the embedded document never retains the token
in its location.search (ARCHITECTURE §9).
- dsh_web.js _tokenizeIframeSrc: dropped the document.cookie read
(always failed) and rewrote as 'accept server-tokenized src /
fall back to address-bar token / warn only on genuinely
token-less remote srcs'. Local pages still never get a token.
- TestHandleInstanceDshInfoTokenizesRemoteSrc now sets cfg and
Proxy AuthToken to DIFFERENT values and asserts the iframe_src
carries the Proxy value (not the cfg value) — pins the token
source so a future regression to cfg.AuthToken is caught.
- New docs/plans/dsh-native-ui/dsh_web.test.mjs (node --test,
4 cases) covers the three branches: server-tokenized / address-
bar fallback / warn. Lives under docs/plans/ (not ui/static/)
to avoid being embedded as a static asset by //go:embed static/*.
- pr5-remote-e2e.sh: step 0 now runs the JS unit test; the new
server-tokenized-src check parses iframe_src with python3 +
urlparse + parse_qs and asserts the token query field equals
the expected value exactly (substring grep would have missed
?token=…&other=… regressions).
Verification: go build ./... / go vet / go test ./internal/app/
-clean; node --test dsh_web.test.mjs — 4/4 pass; pr5-remote-e2e.sh
full matrix green.
* fix(dsh-web): UX guard for remote access (issue #72)
dsh-web's session/workspace sync uses WebSocket, which the user's
remote network path silently drops (server-side, curl/node, and
raw-socket replays all return 101; only the browser's WS upgrade
dies on remote networks — see
docs/plans/dsh-native-ui/REMOTE-WS-BREAKAGE.md for the full
investigation). Result on remote pages: a user could Start a
dsh-web instance, the iframe would render, and the sidebar would
just say "暂无会话" with no indication that the network, not the
code, is the problem.
This is a UX mitigation, not a fix. Three layers:
1. Start Instance modal: disable the Start button on the
dsh-web tab when isRemoteAccess() and show a red
.form-warning hint. New CSS uses --danger-bg / --danger-
border variables (light + dark themes) shared with the
pre-existing #dsh-scope-warning.dsh-warning-danger.
2. doStartInstance defence in depth: alert + return if the
active tab is dsh-web and isRemoteAccess(), so re-enabling
the disabled button via DevTools does not bypass the gate.
3. Running-instance view: new _updateRemoteMask() in
kinds/dsh_web.js puts a full-screen gray mask with a
centred card over #dsh-frames, and _refreshWarning gets a
new highest-priority "Remote access" branch (case 0) that
surfaces the same message in the existing #dsh-scope-
warning bar. Closes the gap where a user with a locally-
started dsh-web instance opens the iframe remotely and
sees an empty sidebar with no explanation.
Centralization: window.isRemoteAccess() in framework.js is the
single source of truth (loopback = localhost / 127.0.0.1 / ::1 /
[::1]). Replaces four inline copies in index.html (Remote
badge, renderSidebar, renderTagsAndTemplateEditBtn,
updateLLMSettingsButtonVisibility) and the partial-IPv6 check
in dsh_web.js#_tokenizeIframeSrc — that one accepted "::1" but
not "[::1]", so an IPv6 loopback user would have been wrongly
treated as remote for token handling; now consistent everywhere.
Test stub (docs/plans/dsh-native-ui/dsh_web.test.mjs) mirrors
the new isRemoteAccess contract so existing _tokenizeIframeSrc
cases run against the same helper the production code uses.
The actual fix — a WS→SSE bridge in the proxy plus a shim
injection for the SPA's WebSocket constructor — is plan A in
REMOTE-WS-BREAKAGE.md §8 and is left for a follow-up. It
conflicts with the existing "dsh does not inject into the
iframe" decision and needs its own design review; the report's
§10 决策留痕 table tracks the open items.
* fix(ui): follow-up issues #68-#73 (REVIEW-2026-08-16)
Start Instance modal + instance-tab UI consistency batch,
filed as issues #68-#71/#73 at
docs/plans/dsh-native-ui/REVIEW-2026-08-16.md. #72 was
replied-to and closed (its UX guard already landed in
ec88389); no code change here.
- #68 tab-bar wrap: .modal-tabs gets flex-wrap: nowrap +
overflow-x: auto (thin scrollbar), .modal-tab gets
white-space: nowrap + flex: 0 0 auto — narrow windows
degrade to a horizontal scrollbar instead of wrapping the
tab text into tall blocks.
- #69 tab naming (plan A/C): the four tabs are now
Terminal / OpenCode-Web / Reasonix-Web / DSH-Web.
data-tab values and the backend kind mapping are
unchanged. The dormant Manifest().Label fields are synced
to the same scheme (OpenCode-Web / Reasonix-Web /
DSH-Web) so a future UI consumer never shows an old name.
- #70 unified kind badges: .badge-oc / .rx-badge /
.badge-dsh collapse into one .kind-badge base (10px,
line-height 1, weight 600, 2px 4px padding, 4px radius,
system font) with per-kind color-only classes; dsh colors
move to new --badge-dsh-bg / --badge-dsh-text CSS
variables (light #0969da / dark #1f6feb) so both themes
work. Legacy class names removed from the tab template.
- #71 Terminal tab form-info: explains it opens a PTY shell
in the worktree (no web service, no iframe) and that
Template / Custom Command still apply.
- #73 English-only UI, enforced across the whole served UI,
not just the modal: dsh missing-dependency dialog, and
every user-visible string in dsh_web.js / opencode_web.js
(warning bars, loading titles, timeout/error copy) were
translated; the last Chinese code comments were removed,
so served UI sources are zero-CJK. TestUICopyIsEnglishOnly
asserts zero CJK characters on /, all three web-kind
renderers and framework.js, plus positive English-copy
checks per file — a stray Chinese string fails loudly.
Docs: REVIEW-2026-08-16.md (batch record + second-round
review-response table, all four findings addressed);
TASK.md gains the PR6 entry and a Status line update;
CHANGELOG.md gains two Unreleased entries and drops the
stale "(uncommitted, pending review)" marker from the
already-committed #62-#67 entry.
Tests: TestStartInstanceModalTabLabelsAndWrap /
TestStartInstanceModalFormInfosAreEnglish /
TestKindBadgesUnified / TestUICopyIsEnglishOnly, plus the
fetchStaticPath helper and ReadAll error checks in
fetchIndexHTML / fetchStaticPath (review nit2).
* fix(ui): Start Instance modal width follows content + badge polish
- #68 rework: drop the modal-tabs horizontal-scrollbar band-aid
(overflow-x: auto / scrollbar-width: thin) in favour of sizing the
dialog to its content — #modal-start-instance is now
width: max-content with min-width: 400px (no narrower than the
other dialogs) and max-width: min(90vw, 640px) (long form-info
paragraphs cannot blow the dialog up). All four tabs fit on one
line at any normal window size; tabs keep flex: 0 0 auto +
white-space: nowrap. Test pins the scrollbar-free .modal-tabs
block and the three width caps.
- reasonix instance-tab badge text rx -> RX (consistent with the
OC / DS siblings) and .kind-badge font-size 10px -> 9px.
* fix(review): address PR #74 review findings (REVIEW-2026-08-16 #3 round)
- fix(security): strip mw_token cookie in dsh proxy Director before
forwarding upstream — checkToken syncs it onto the proxy origin, and
the unauthenticated dsh subprocess must never see the main token
(review #3 Major); pinned by TestProxyStripsMwTokenCookieUpstream
- fix(pty): close ptmx master fd when the log pump exits — every PTY
start/stop leaked a fd until exhaustion (review #2 HIGH-1)
- fix(framework): skip the 16-256 MB ring-buffer pre-allocation for
web-UI kinds — only PTY captures output (new BufferConsumer
interface; opencode-web / reasonix / dsh-web return false)
(review #2 HIGH-2)
- fix(ui): escapeHtml user-controlled values in innerHTML templates —
instance displayName/rename, diverge badges branch+error, Start-modal
branch names and tag command/cwd/preStart/env (review #2 HIGH-3)
- fix(dsh-web): persist https scheme in IframeURL when the main
listener is TLS (review #1-1)
Verified: go build, go vet, go test ./... all green (incl. -race on
dsh_web/pty).
* fix(review): CSRF guard for dsh install/launch + proxy hardening (MED-6/8)
- fix(security): dshWriteOriginGuard rejects cross-origin writes to
/api/instances/dsh/install and /dsh/launch even on loopback — withAuth
skips Origin checks for loopback clients, so a hostile webpage could
drive a no-cors fetch into `npm install -g` (arbitrary code exec).
Origin-less requests (curl/CLI) stay allowed. Pinned by
TestDshWriteOriginGuard (unknown-instance id: guard-passing requests
stop at 404, never reach npm).
- fix(dsh-web proxy): ReadHeaderTimeout/ReadTimeout/IdleTimeout on the
per-instance http.Server (slowloris guard) and constant-time token
comparison (crypto/subtle).
---------
Co-authored-by: linletian <linletian@users.noreply.github.com>
…(issue #75) (#76) opencode embeds the Bun runtime, whose fetch honors HTTP(S)_PROXY but does not match CIDR ranges in no_proxy (e.g. the common 127.0.0.0/8). With a LAN proxy configured, in-process plugin SDK calls to the loopback server were routed through the proxy and failed with 502, surfacing in oh-my-openagent as 'messages.map is not a function' and a 100% task-tool failure in web/serve mode (TUI is unaffected because plugins use in-process fetch there). buildEnv now merges NO_PROXY/no_proxy (both cases) and appends the literal hosts localhost, 127.0.0.1, ::1 so every no_proxy matcher bypasses loopback. No-op without a configured proxy or with a '*' wildcard; existing entries (CIDR included) are preserved; remote LLM provider traffic still goes through the proxy. Verified live: opencode serve + Bun probe returns 502 with CIDR-only no_proxy, and 401/200 (direct) after the fix. Unit tests cover append, merge/dedupe across cases, wildcard, and no-proxy no-op. Co-authored-by: linletian <linletian@users.noreply.github.com>
Codifies the v0.4.2/v0.4.3 release path: prep branch from develop, squash-merged release PR to main, annotated tag triggers the release workflow, verification (assets + /releases/latest redirect), and the back-merge into develop. Includes the flaky-test re-run note, tag immutability rules, and an unexercised hotfix flow.
…y (issue #72) (#78) * feat(dsh-web): add 0.1.5 remote-capability floor and bump npx pin to 0.1.5-rc.1 * feat(ws): add hand-rolled WebSocket client Dial with client-side frame masking * feat(dsh-web): capture upstream launch token, expose remote_capable, tolerate 401 health probes * feat(dsh-web): add upstream browser-session cookie relay (launch-token exchange) * feat(dsh-web): relay upstream browser-session auth through the instance proxy * fix(dsh-web): carry relay cookie in workspace bootstrap RPCs * feat(dsh-web): WS-to-SSE bridge with native-first shim for remote access * feat(dsh-web): pre-start remote capability probe endpoint * feat(dsh-web): gate remote Start on dsh version instead of blanket block * feat(dsh-web): make the remote mask version-conditional * test(dsh-web): cover auth relay and bridge through the full instance path * docs(dsh-web): record remote-support decisions and refresh limitations * fix(dsh-web): sanitize exchange errors so the launch token never reaches logs; F2 review findings * fix(dsh-web): PR #78 Go review — strip Referer upstream, parse the unredacted ready line, validate Sec-WebSocket-Accept in Dial, 413 on oversized uplink, SSE write deadline, client-side frame reassembly, fresh capability probes, invalidate throttle, gating/injection tests * fix(dsh-web): PR #78 shim review — gate bridge-open on the server frame, anti-wedge send queue, terminal SSE error teardown, close-code mapping, shim unit tests; honest Retry affordance; CI race step * test(dsh-web): PR #78 round-2 follow-ups — raise async-signal budgets for -race CI, drop the inert SSE retry preamble, cover wedged-queue close + probeVersion exec failure + npx capability * fix(dsh-web): PR #78 round-4 — distinguish probe failure from version-too-low in the Start defense; document loopback token-exchange premise, verMemo tradeoff, and shim binaryType fidelity --------- Co-authored-by: linletian <linletian@users.noreply.github.com>
# Conflicts: # CHANGELOG.md
…#85) * fix(terminal): serve log replay from the ring-buffer tail (issue #81) Re-selecting an idle terminal instance (or pressing Refresh) replayed the oldest 64 KB held in the ring buffer instead of the newest, so the UI showed stale session-start output; only a window resize (SIGWINCH -> TUI repaint) recovered the live screen. The kind refactor had rewired Manager.Tail from RingBuffer.Tail to ReadLogs(handle, 0, n), and ReadSince(0, n) clamps to the oldest live byte -- orphaning the RingBuffer.Tail implementation that already did the right thing. Kind.ReadLogs now has an explicit tail contract: a negative since means 'newest maxBytes, cursor set to the current end offset'. The pty driver serves it from RingBuffer.Tail; Manager.Tail requests it and clamps a non-conforming kind cursor to 0. Manager.ReadSince clamps a negative since to 0, so tail reads live behind the single Tail entry point, and the log stream handler routes an omitted/negative since to Tail instead of defaulting to offset 0. Bare GET /api/instances/log responses now carry X-Log-Offset (the end offset), so the frontend cursor is a real byte offset rather than a chunk length. Frontend: loadLog drops since=0 and validates the cursor with Number.isFinite, falling back to 0 (never replay) when the header is absent; the dead loadLogSince helper is removed. Docs synced to the behaviour (docs/API.md log + stream endpoints, incl. stopped/unknown instance response shapes, docs/ARCHITECTURE.md 4.1 cursor semantics, docs/PRD.md buffer path, CHANGELOG Unreleased). Pinned by TestDriver_ReadLogs_TailReturnsNewestBytes, TestManager_TailRequestsNewestBytes, TestManager_ReadSinceClampsNegativeSince, TestHandleInstanceLog_TailReturnsNewestBytes, TestHandleInstanceLogStream_StartsFromTail and TestHandleInstanceLogStream_NonTailKindDoesNotReplay. Closes #81 * fix(terminal): keep previous log cursor when X-Log-Offset is unavailable (issue #81) Resetting the cursor to 0 on a missing/unparseable X-Log-Offset header made startSSE connect with since=0, replaying the oldest 64KB on top of the tail just written. Only advance the cursor from a valid header, and drop the now-redundant pre-load reset (loadLog always overwrites the cursor from the header on success; the previous cursor is a strictly safer fallback on failure). * fix(terminal): close review findings on #81 tail-replay fix Follow-up to 23a6c58 / 844c261, addressing both PR #85 review reports. No behavior regression in the #81 fix itself; these close the residual paths and the contract/documentation gaps both reviews found. Frontend cursor sentinel (reviews #1-1 and #2-1, same defect): - A new session started at logCursor 0. If loadLog failed or was aborted, startSSE still sent since=0 and the server replayed the OLDEST 64KB — the #81 symptom on the very path this PR fixed. startSSE now omits `since` unless a real positive cursor exists, so an unknown cursor falls through to the server's tail default. The initial value is -1 ("cursor unknown"), which is the sentinel the server already defines. startSSE's cursor update switches from a truthy check to Number.isFinite(next) && next >= 0, matching loadLog. Kind contract (review #1-2, #1-3): - kind.go now states the rule for kinds without log capture (MUST return ("", 0, nil) for a negative since) instead of leaving it only in ARCHITECTURE.md, and its route reference is corrected to the real shape (/api/instances/log?id=<id>). - Manager.Tail additionally clamps a cursor that would end before the bytes it just returned, so a non-conforming kind cannot make the follow-up ReadSince re-deliver them. Docs (review #1-4, #2-2): - API.md no longer claims stopped/unknown streams always open with next=0 — an incremental read echoes the requested since. - API.md documents instance_log_tail's `n` and window direction; the MCP tool's public return window changed from oldest to newest. Tests: - TestHandleMCPCallInstanceLogTail_ReturnsNewestBytes pins the MCP path (mutation-checked: reverting Manager.Tail to ReadLogs(0, n) makes it fail). - TestRestartCreatesNewID's inst2 wait loop now reports the timeout instead of silently falling through, matching the loop above it. Refs: #81, #82, #83, #86, #87 * fix(framework): correct ReadLogs no-capture contract wording, test the cursor clamp Closes findings N1 and N2 from the re-review of 30e28da. Both concern the contract text and its test coverage, not the #81 behavior. N1 — the kind.go contract contradicted the code it describes. The new clause said kinds without log capture MUST return ("", 0, nil) for a negative since, but all three of them (reasonix, opencode-web, dsh-web) return ("", since, nil), i.e. ("", -1, nil) for a tail request. That also contradicted ARCHITECTURE.md §4.1 ("the manager clamps the cursor to 0") and manager.go's own comment. A kind implementer reading only the interface would have been told to change working code. Softened to describe the lenient strategy that is actually implemented, with the manager normalization stated explicitly. N2 — the cursor lower bound added in 30e28da had no test. No existing kind or test double can trigger it: pty's Tail returns off = head >= len(body), the no-capture kinds return an empty body, and the doubles report ("newest-bytes", 4242), ("tail-body", 42) and ("", since, nil). Deleting the clamp produced zero failures. TestManager_TailClampsShort- KindCursor adds a double reporting 10 bytes with cursor 4 and pins off >= len(body); verified non-vacuous by re-removing the clamp. Also qualified the clamp comment: the len(body) lower bound holds for zero-based offset spaces, which is all that exists today. Refs: #81 * test(ui): pin cursor bootstrap in both session factories; fix stale #86/PR text Third-round review round: one real miss on my side (a second cursor bootstrap path I had never looked at), plus documentation drift. The miss: kinds/pty.js builds its own session with its own logCursor and its own bootstrap in activate(). All four kinds register a renderer, so the index.html path I changed in 30e28da is the legacy no-renderer fallback and only runs if a renderer script fails to load — my earlier logCursor grep was scoped to index.html and never saw it. Reviewed whether the pty line should switch to the -1 sentinel and concluded no. activate() calls resetTerminalForSwitch first, which clears the screen; if loadLog then fails, retaining an old cursor makes startSSE resume with since=<oldOffset> and repaint only the post-offset delta onto an empty terminal. Zeroing the cursor makes startSSE omit `since`, so the server's tail default repaints the full screen — the current behavior is the correct one. Documented it in place so the next reader does not "fix" it into the regression, and scoped the index.html comments to say which factory they describe. Coverage, which is why this slipped through: internal/ui had zero logCursor assertions. - TestLogCursorBootstrapNeverRequestsOldestBytes pins the -1 sentinel, the `logCursor > 0` guard and both Number.isFinite acceptances, and fails if an unconditional since= URL or a since=0 fetch returns. - TestPtyRendererResetsCursorAfterTerminalReset pins that the pty renderer zeroes the cursor after clearing the terminal. Both verified non-vacuous by reverting the guarded code (MUT: drop the > 0 guard; drop `s.logCursor = 0`) — each makes its test fail. Housekeeping and doc drift: - startRecordingInstance's `k` parameter was dead (zero uses in the body); removed, so the new test no longer passes &k.recordingLogsKind to satisfy a parameter nobody reads. - Issue #86's Gap was written against an earlier state of this PR and all three of its steps were now false. Rewritten: the failure mode is no longer a replay of the oldest 64KB but a duplicate rendering of the tail when X-Log-Offset is stripped on a fresh session, on both the SSE and WS paths, with the two unaffected paths spelled out. - PR description: dropped the deleted `("", 0, nil)` MUST and added the two tests added since. Refs: #81, #86, #87 * fix(ui): reset the log cursor after every screen reset, in both factories Fourth-round review round. Two reviewers converged on one behavior bug and one documentation bug; the reviewers disagreed on the remedy, and the evidence favors changing the code rather than documenting the divergence. The bug: index.html's legacy no-renderer fallback cleared the terminal and then *retained* the previous cursor (that retention came from 30e28da). On a revisited running instance, if loadLog then failed, startSSE sent since=<oldOffset> and the server returned only the post-offset delta, which got painted onto the screen that had just been cleared. The user sees a blank or partial terminal. maybeDestroyInactiveStoppedSession returns early for running instances, so the session — and its positive cursor — survives a tab switch, which makes that state reachable. Both factories now reset the cursor to the -1 "unknown" sentinel right after resetTerminalForSwitch, so startSSE omits `since` and the server's tail default repaints the whole screen. pty.js already did this; it is now the same convention in the same words in both files, which also removes the two comments that told readers to do opposite things. This trades one failure mode for a less severe one. Previously the fallback retained the cursor, so a stripped X-Log-Offset header produced no second tail; now it does. A duplicated 64 KB is strictly better than a blank screen, and the duplicate case was already tracked by #86. Coverage, which is what let this survive four rounds: the pty renderer had an ordering assertion and index.html did not, so the asymmetry in coverage mirrored the asymmetry in behavior. - TestLogCursorBootstrapNeverRequestsOldestBytes now mirrors the ordering assertion: the cursor reset must come after resetTerminalForSwitch. Verified non-vacuous three ways: removing the reset, moving it before the clear, and reverting pty.js to 0 each make a test fail. Documentation: - pty.js: the old comment justified "Zero" as the load-bearing choice, but 0 and -1 are behaviorally identical under startSSE's `> 0` guard. Rewritten to say the value is not load-bearing and the shared convention is the point. Session init also moves 0 -> -1 so there is exactly one "unknown" sentinel. - ui_test.go: the bootstrap test's comment claimed it covered both factories; it only reads index.html. Reworded to point at its companion test. - Issue #86: two scope-note claims were path-specific and one is now false everywhere, since every screen reset clears the cursor. Rewrote the section with the three-way table and stated that the header-stripped duplicate now applies to any session. - PR description updated to match. Refs: #81, #86 --------- Co-authored-by: linletian <linletian@users.noreply.github.com>
…e PTY (issue #80) (#88) * fix(terminal): a new instance could show [Process Stopped] over a live PTY (issue #80) A just-created terminal instance sometimes rendered the red [Process Stopped] banner, greyed-out styling, and no visible input or output — while blind typing still reached the process and produced correct results. Switching to another tab and back, or pressing Refresh, "fixed" it. Root cause: the frontend collapsed the server's seven-state status enum into a `status === 'running'` boolean. Manager.Start persists a new record as `starting` and flips it to `running` on the kind's ready signal in a separate goroutine (a state.json version conflict costs 3 x 10ms of retries), so an instance created moments earlier is routinely observed as `starting` at selection time — and both activation paths treated that transient state as terminal: they disconnected the transport, replayed the log with the banner, and never reconnected. Nothing could recover from it either. selectInstance() returns early when the tab is already selected, and the 2s poll's renderTerminalSessions() only toggles container visibility, so activate() runs exactly once per selection change. The session stayed transport-less forever: input silently degraded to the buffered HTTP fallback (which is why typing "worked"), while output — published only to WS/SSE subscribers — never came back. Fix: - Classify the status into three buckets shared by both activation paths: live (running, unhealthy — the process is alive and takes I/O), pending (starting, stopping — transient) and terminal (stopped, failed, exited). Only the terminal bucket paints the banner, greys out the terminal, refuses input, or blocks the connect. `unhealthy` means a probe failed, not that the process died, so it now keeps its transport and its reconnect-on-close retry. - Move re-activation from selection-driven to poll-driven. reconcileTerminalSessions() runs on every refresh() with fresh state and promotes the active session as soon as its status turns live. The promotion calls connectTTY only — no loadLog, since the WS handshake replays the tail itself and that duplication stays tracked as #87 — and is guarded against a socket, stream, state transition, pending reconnect timer or in-flight activation so the 2s poll can never flap an existing connection. - Replay the log once on entry into the terminal bucket (tracked per session via lastKnownStatus), so a crashed or failed instance still gets its banner without every poll refetching and repainting it. Docs-first: the bucket contract, the poll-driven promotion and the revised recovery table are written up in docs/ARCHITECTURE.md §5.3 / §5.4 / §5.6 / §5.8. docs/API.md claimed the Start endpoint returns status "running"; it returns "starting", and that inaccuracy is what invited the bug. Tests: 22 node --test cases in testdata/terminal_status.test.mjs slice the shipped index.html / kinds/pty.js and run them against stubbed page state (wired into `go test` via TestTerminalStatusHandling), plus the source-contract TestTerminalStatusBucketsReplaceRunningEquality pinning that both activation paths classify the status. 15 mutations were checked: 14 are caught by the behavioural layer and all 15 by the source-contract layer; the one that survives behaviourally is index.html's legacy no-renderer fallback branch, which only runs when a renderer script fails to load. * fix(terminal): address review feedback on the issue #80 fix All findings were checked against the code first; this commit fixes the accurate ones and keeps the rest out (see the PR reply for the reasoning). Activation now shares the promotion's in-flight guard: - `hasTerminalTransportInFlight(session)` is extracted as one function and used by both `ensureTerminalLiveTransport()` and `kinds/pty.js` `activate()`. Previously activate() only checked `hasLiveTTYConnection()`, which is false while a socket is still CONNECTING — so switching to a tab that the poll-driven promotion had already started to bring up cleared the screen and opened a second socket, replaying the ring-buffer tail twice (#87). The guard is now single-sourced so the two paths cannot drift. - `ws.onclose` drops the session's socket reference. A CLOSED socket left behind read as "transport in flight" and blocked the promotion until something else cleared it. Fixed at the root rather than by loosening the guard. Terminal replay no longer strands the tab on a failed fetch: - `loadLog()` swallows its own errors, so the once-per-transition replay had no way to know it had failed: one transient 5xx left the tab with neither log nor `[Process Stopped]` banner until someone pressed Refresh by hand. `loadLog` now latches its own outcome in `session.lastLogLoadFailed` and `syncActiveTerminalStatus` retries while the last attempt failed. The legacy no-renderer fallback no longer diverges from `kinds/pty.js`: - An id that `state.instances` cannot resolve used to fall into the live branch and open a WebSocket against it, which the server accepts and then closes with 1013 — surfacing as a dropped connection rather than a bad request. The fallback now leaves the session alone, matching the renderer's `if (!inst) return`, and `connectTTY()` refuses an unresolvable id as a second layer for every other caller. `unhealthy` and `stopping` labels follow the buckets too: - The status-bar text for reasonix and dsh-web used `=== 'running'`, so a freshly created instance read "stopped" for seconds while `kinds/reasonix.js` kept its iframe navigating — the status bar contradicted the panel next to it. Both now go through `instanceStatusLabel()`. - The source contract banned `!== 'running'` forms but no `=== 'running'` form, which is exactly the gap that let those two survive; both shapes are now forbidden. Smaller items: - `kinds/pty.js`'s inline fallback chain guarded `isInstancePendingStatus` and `isInstanceTerminalStatus` but called `window.isInstanceLiveStatus` unguarded, so the terminal branch threw when the page had not published it yet. All three are guarded the same way now. - `docs/ARCHITECTURE.md` §5.3 described the pending bucket as "no transport"; the code deliberately keeps an existing one. §5.6's close row said "retry after 1s" against a 5s timer, and §5.3's promotion guard listed three conditions against the five now in `hasTerminalTransportInFlight()`. - CHANGELOG said "17 node --test cases" (there were 22, now 27) and implied `unhealthy` is observable. `runLifecycle` persists only `running` and the terminal statuses, and a kind's health-probe result is discarded, so the `unhealthy` and `stopping` branches cannot be exercised against a live backend — stated in the entry now. - `go-ci.yml` pins node 22 and runs the browser layer directly. The JS tests were reachable only through a Go wrapper that `t.Skip`s when node is absent, so a runner image without node would report green without ever running them. Tests: 27 `node --test` cases (was 22), including the shared guard from both call sites, a per-case table for its five conditions, the failed-replay retry, `instanceStatusLabel` across all seven statuses, and a loop that activates every status with the `window` helpers unpublished. The `connectTTY` stub now performs the synchronous writes the real function does, so the guard under test is actually exercised — which turned "connects exactly once per session" into a test that means what it says. 12 mutations were checked: 8 caught by the behavioural layer and all 12 by the source contract. The four that survive behaviourally are the legacy fallback branch (no kind registers a renderer), two call-site pins (`instanceStatusLabel`'s use in renderWorkspace, `connectTTY`'s null refusal) and `ws.onclose`, none of which the sliced harnesses can reach. * ci: pin setup-node to v7 to avoid the Node 20 deprecation warning * fix(terminal): third review round on the issue #80 fix - `hasTerminalTransportInFlight` was committed at column 0 — the only zero-indented declaration in the whole inline script — and the four-line comment above `ensureTerminalLiveTransport` still described five guards that had moved out of it, so an audit of the anti-flap logic read an explanation and found nothing to audit. Re-indented to 8 spaces, the surviving sentence folded into the helper's own doc, and `TestIndexHTMLInlineScriptIsIndented` now fails on any column-0 declaration so the slip cannot recur. - The in-flight early return in `activate()` was silent, unlike every other exit. `deactivate()` leaves the status bar on "idle", so re-selecting a tab whose reconnect was still queued looked inert for the whole 5s. It now reports "ws closed, retrying..." when a retry is queued and "connecting..." otherwise — and deliberately says nothing when an SSE stream is live, since that session is not connecting and its own message is the accurate one. The legacy no-renderer fallback mirrors the treatment. - The terminal-bucket replay retry is bounded now. `loadLog` reports its failures into the terminal, so an unbounded 2s retry appended one error line per tick forever. Retries wait 2^n seconds capped at 60s, driven by the consecutive-failure count and the attempt timestamp that `loadLog` records; a success resets both. The ordinary stopped case never enters this path: `Manager.Tail` answers `("", 0, nil)` for an instance that is no longer running, so the replay succeeds. - The replay latches are initialised in both session factories, next to the `lastKnownStatus` latch they now sit beside. - The CHANGELOG's case count had been wrong twice in two rounds (17, then 26, for a file holding 29), so it is no longer maintained by hand: `TestTerminalStatusChangelogCount` counts the cases in the test file and fails if the entry disagrees. It caught the stale number on this run. - The CHANGELOG now records the visible label change — the status bar for reasonix and dsh-web instances reports `starting...` where it used to say `stopped` — since that ships under a `fix(terminal):` heading. - The legacy branch edit in this round briefly inverted the in-flight test and dropped `resetTerminalForSwitch(session)`; the pre-existing ordering assertion in `TestIndexHTMLCoversReconcileLogic` caught it before push. Tests: 29 `node --test` cases (was 27) — the backoff schedule and the three in-flight status outcomes, plus `loadLog` asserting that it owns the backoff inputs rather than the reconciler. A 9-mutation matrix over this round's logic: 5 caught by the behavioural layer, 9 by the source contract. The four that survive behaviourally are the legacy no-renderer branch (three) and the replay latches in kinds/pty.js's session factory, none of which the sliced harnesses can reach. * fix(terminal): let an explicit tab re-selection take over a queued reconnect Fourth review round. Three of the four findings were accurate and are fixed here; the fourth is a product call and this takes option one. **A queued reconnect no longer freezes an explicit re-selection.** The in-flight guard was blocking activation on `ttyReconnectTimer`, so switching to a tab and back inside the 5s reconnect window did nothing: no reset, no replay, no connect. The tab sat on the pre-drop screen, and the handshake replay then painted the last screenful a second time. `hasTerminal TransportInFlight` now takes an options argument, and both activation paths pass `{ ignoreQueuedRetry: true }` while the poll-driven promotion passes nothing. The asymmetry is deliberate and now documented in ARCHITECTURE.md §5.3: the promotion is a bystander that would take over on every tick and flap the connection, while the user is not — re-selecting a tab is an explicit request, and waiting out a 5s timer to honour it is the freeze. The handover is safe because `connectTTY()` → `disconnectTTY()` cancels the pending timer before anything is opened, so it cannot leave two sockets. The other four conditions still block, so an activation already inside `await loadLog()` is never raced — which also removes the "brief flap" risk the review flagged. **The over-indented comment line is gone**, and the guard now covers the shape of the defect it missed: `TestIndexHTMLInlineScriptIsIndented` checks that every line of a `//` block sits at the same depth, not only that no declaration is at column 0. A moved block that strands its first line at a different indent is now caught. The declaration check stays one-sided on purpose — requiring exactly 8 spaces flags every nested `const`, and deciding top level properly needs a parser; that is noted in the test. The new test immediately found a drift of its own: the harness forwarded only the first argument to the shared guard, silently disabling the handover it was meant to exercise. It now forwards both. **`TestUICopyIsEnglishOnly` got its doc comment back.** Inserting the indentation test above its `func` had stranded its godoc on the new function, so `go doc` would have rendered the CJK-enforcement paragraph as the indentation test's. **ARCHITECTURE.md §5.3 caught up with the code**, which is the point of this PR: it existed because documentation drifted into a bug. §5.3's terminal bullet still said the replay happens "only" on the transition — pre-backoff semantics — so a reader would conclude a failed replay is never retried, which is the round-2 bug. It now documents the bounded retry, that an explicit user action bypasses the backoff, the `ignoreQueuedRetry` asymmetry and why it is safe, and the three-way status on the in-flight early return (`ws closed, retrying...` / `connecting...` / silence while SSE is live). Tests: 32 `node --test` cases (was 29) — the takeover and the three shapes it must still yield to, the promotion still standing aside, and the `connectTTY` stub now clearing the timer like the real one does. A 5-mutation matrix over this round: 3 caught by the behavioural layer, 5 by the source contract; the two survivors are the indentation defect (not behavioural) and the legacy no-renderer branch (unreachable). * fix(terminal): cancel the queued retry at the takeover, not at connect time Fifth review round. D1 was a real race in the handover this PR introduced, and the sentence added to §5.3 in the previous round was false without this. **The race.** `ttyReconnectTimer` is cleared in exactly one place, `disconnectTTY` (index.html:3893). `activate()` reaches `disconnectTTY` only through `connectTTY`, which it calls from `loadLog().finally(...)` — that is, *after* loadLog settles. So after an explicit re-selection took over from a queued retry, the timer was still armed: T0 ws.onclose arms the 5s retry, nulls ttySocket, ttyState = IDLE T0+Δ user re-selects; the guard passes; loadLog starts, timer still armed T0+5s if Δ + loadLog-duration > 5s, the retry fires → connectTTY → WS#1 opens and the handshake replays the tail later loadLog resolves → connectTTY → disconnectTTY kills WS#1 → WS#2 opens and the handshake replays the tail again Two connects, one wasted socket, and the tail painted twice — the outcome the whole guard family exists to prevent, reintroduced through the door `ignoreQueuedRetry` opened. `loadLogController` could not help: it guards activations, not timers. The window is narrow on localhost but Δ is the user's reaction time to a visibly dropped connection, which is most of the 5s window, and a 64 KB log fetch over Tailscale or LAN is not instant. Both activation paths now cancel the pending retry at the moment they take over, before `loadLog()` starts. The reviewer's one-liner, taken as given. **The vestigial status arm goes with it.** Once the takeover clears the timer, a queued retry can no longer be the reason an activation returns early — `ws.onclose` leaves the session IDLE with no socket, and a connect clears the timer again — so the `ws closed, retrying...` arm described a mapping the code no longer performs. The early return is now simply `connecting...`, or silence over a live SSE stream, and §5.3 says so. **The indentation guard no longer contradicts its own doc comment.** The doc promised an exact-8-spaces declaration rule that the code deliberately does not implement (it flags every nested `const`); the bullet is narrowed to column 0 and the reason stays in the function. The mixed-indent check now compares every line against the run's *minimum* indent instead of its first line's, so a block whose odd line is in the middle is caught too — the first-line comparison misses exactly that case, which a mutation confirms. Tests: 33 `node --test` cases (was 32). The new one holds `loadLog` open in the harness and asserts the retry is already cancelled while it is pending, which is the only way to observe the window the race lived in; moving the cancel after the load fails it. A 6-mutation matrix over this round: 3 caught by the behavioural layer, 6 by the source contract; the survivors are the legacy no-renderer branch and a restored ternary arm, neither reachable from the sliced harnesses. --------- Co-authored-by: linletian <linletian@users.noreply.github.com>
…sue #84) (#89) * feat(dsh-web): adapt the embedded DSH UI to dsh 0.2.x + --no-open (issue #84) * feat(dsh-web): adapt the embedded DSH UI to dsh 0.2.x + --no-open (issue #84) * feat(dsh-web): adapt the embedded DSH UI to dsh 0.2.x + --no-open (issue #84) * feat(dsh-web): adapt the embedded DSH UI to dsh 0.2.x + --no-open (issue #84) The dsh-web kind embeds the DeepSeek Harness browser UI (`dsh web`) as a managed instance, iframe'd behind a per-instance reverse proxy. Two problems: the integration spoke dsh 0.1.x, and every instance Start popped a stray tab in the host's default browser (issue #84). Everything below was verified by hand against the installed dsh 0.2.0-rc.2, not inferred from release notes. Issue #84 -- `dsh web` opens the default browser on every boot ("opening the default browser; pass --no-open to disable"). webArgs() now appends `--no-open`. It rides AFTER `--port 0`, grouped with the other web-app options: the launcher parses with allowUnknownOption + passThroughOptions, so the first option the web app does not know (`--patch`) flips it into pass-through, and a launcher flag after that reaches the web app and kills the boot. The ready line still prints (printUrl stays true), so pumpAndWatch is unaffected. launch_test.go pins the order independently of the expected slice, so reordering the expectation alongside the bug cannot pass. dsh 0.2.x adaptation, 0.1.x dropped rather than dual-branched. Two breaking upstream changes: - Endpoint segments became `<namespace>/<method>`: `POST /api/workspace/create`, `/api/session/create`, `session/prompt`, and `subagents/prompt` -- the subagent namespace is PLURAL. The envelope's `method` must still equal the URL-path endpoint, and bare `/api` still 404s. The 0.1.x dotted names 404 on 0.2. - Every verb's single object argument nests at `payload.args.request`, so the bootstrap sends `{args:{request:{path:<worktree>}}}` and the proxy reads ids and scope targets from `payload.args.request`. The envelope and the `{ok, value}` answer shape are unchanged, and so are the value shapes (`{workspace:{workspaceId,...}, created}` / `{sessionId, agentPreset?}`). There is no fallback shape: the gateway rejects a flat payload outright ("Remote payload must contain exactly one plain-object args field"). bootstrap.go gained a requestPayload wrapper; proxy.go matches `/api/session/create` for the session-id tee and routes ownSessionWriteMethods / ownSessionIDsInBody / classifyRPCBody through a shared decodeArgsRequest. Version gates move to 0.2.x only: minVersion 0.2.0, supportedMaxVersion 0.3.0 (exclusive), minRemoteVersion 0.2.0, NpxPin 0.2.0-rc.2 (npm publishes it as both `latest` and `next`, and it is the build every finding above was verified against). probeAndGate's error now names the slash endpoints and --no-open. Verified unchanged on 0.2.0 and deliberately untouched: --patch and --dump-config, the ready-line format, GET /?token= -> 303 + dsh-auth-* cookie (the auth relay is untouched and dsh's browser-session auth is still the proxy's sole credential to hold), the /api/remote.mux WS carrier, the injected static/dsh-bridge.js, and the restrict overlay rows -- all four still compose per --dump-config, and a live instance's pluginInventory/list shows directory-picker-auto and dsh-client-hmr disabled with dsh-host-directory-picker-browse active and NO dsh-client-ui-directory-picker-* client surface loaded, so the "Add workspace..." entry still cannot render (0.2's composer mounts backend and surface together, so disabling the composer and inserting the bare backend is now even more correct). Two review-driven fixes landed in the same round: - isSupportedVersion/hardVersionOK compared the RAW `dsh --version` string, so a `-rc.N` suffix broke splitVersion's Atoi and versionLess answered false for BOTH bounds. npx mode is the only caller passing the raw pin, so every npx-launched instance reported version_supported:false and the UI painted the permanent red "restriction may not be effective" bar even with overlay_verified true. Both gates now core-parse via parseVersion, like isRemoteCapable always did. Pinned by prerelease cases in version_test.go, TestVersionGatesAgreeOnPrereleaseCore and TestSpawnNpxModeReportsVersionSupported (a real npx Spawn). - Both testdata/dsh-mock*.go doubles ignored unknown argv, so a dsh build that dropped --no-open -- or one where --patch moved behind an app-level option -- would stay green in CI while every real Start died. They now validate the recognised web flag set and fail like commander (`unknown option '<flag>'`, exit 1); TestMockDshWebFlagContract pins both mocks, positive and negative. - TestProxySessionOwnMarking now tables all eight ownSessionWriteMethods entries (was three) with distinct ids, an exact-set comparison and a bidirectional coverage guard, so a typo in any verb can no longer silently disable own-session attribution. Docs: the stale /api/events.mux + /api/events.host carrier paths became /api/remote.mux (dsh 0.2 removed them), the false "dsh has no authentication surface" claim was corrected wherever it survived (it has had browser-session auth since 0.1.2), the now-dead "use it from the local 127.0.0.1" fallback advice was dropped from the capability warnings (a sub-0.2.0 dsh cannot start in ANY mode), API.md 5.14 shows the reachable version with missing_dsh split into its own failed-instance example, and PLAN.md gained a marked dsh-0.2.x amendment block declaring the 0.1.x text history. pr5-remote-e2e.sh now drives the real /api/remote.mux handshake and FAILS on a non-101. Verification: gofmt / vet / build / go test ./... all clean, plus `node --test docs/plans/dsh-native-ui/dsh_web.test.mjs`. pr5-remote-e2e.sh run end-to-end against a real dsh 0.2.0-rc.2 with an isolated DSH_HOME: the instance boots, the token gate matrix passes (401 / 302+cookie / 200 / loopback bypass), the server-tokenised iframe_src loads, the WS upgrade on /api/remote.mux returns 101 through the proxy, and the foreign-session watch flags and clears a synthetic active session. Review round 2 (two independent reports on the PR) — all findings adjudicated against the installed dsh 0.2.0-rc.2 before acting: Fixed: - version.go's hard-gate bullet claimed `--port 0`, `--patch` and the browser-auth ready line "did not exist yet" below 0.2.0. All three existed throughout 0.1.x (ready line since 0.1.2 — this same file's remote-floor bullet says so), so the justification contradicted itself. It now names only the genuinely 0.2-only surface: the slash endpoints, the payload.args.request nesting, `--no-open` and the plural subagents namespace; the other facts moved to the advisory bullet, marked as "verified, but NOT floor justifications". - decodeArgsRequest treated `request: null` as a successful decode into an all-zero struct ("null" is 4 bytes, so the emptiness check waved it through, and unmarshalling null into a struct is a documented silent no-op). Nothing mis-recorded today, but only via three INCIDENTAL guards downstream; a null request is now rejected structurally. Pinned by TestDecodeArgsRequestNullIsAbsent plus null rows in TestClassifyRPCBody and TestProxySessionOwnMarking. - The read-only exclusion comment named session/history and session/models, neither of which exists on 0.2 (the session namespace has no `history` — only dsh-schedule has that — and the model-list verb is session/modelCatalog). Replaced with verbs that do exist. - Both mocks now ROUTE the eight write verbs, so an upstream rename turns the end-to-end write-verb POSTs red instead of silently degrading own-session attribution; added negative cases for the dotted workspace.create and the singular subagent/prompt. - TestStartDshWebEndToEnd now asserts blob.WorkspaceID like its two siblings did (it is the whole point of the workspace/create bootstrap); TestDumpArgs pins dumpArgs' --patch-first order the way TestWebArgs pins webArgs'; the mock headers no longer imply the mock enforces flag ORDER (it enforces the flag SET); the mock models --trusted-host as the variadic flag it is; the remote-incapable mask branches are documented as defence-in-depth rather than deleted; ARCHITECTURE.md's §9 checklist item 6 regains the load-bearing half of the 0.2.0 re-verification (no dsh-client-ui-directory-picker-* client surface loaded at all, which is what actually proves the "Add workspace…" entry cannot render); the CHANGELOG names what a user still on dsh 0.1.x actually hits. Rejected, with reasons: - "index.html's cap._error branch still carries dead 'use the local 127.0.0.1' advice" — wrong: cap._error is set only when the capability FETCH failed, so the installed dsh version is unknown; it may be a fine 0.2.x and a local Start would work. The "terminology drift" between the cap.missing and !cap.remote_capable branches is also wrong — they state different facts and the differing wording is deliberate. - parseVersion's unanchored first match, probeAndGate's memo downgrade window, and sessionCreateBody's uncapped buffering are pre-existing and untouched by this change; they are follow-ups, not this PR. Review round 3 (two more reports on the PR). The round-2 fixes above never actually reached the PR — `git commit --amend -F <file>` replaces only the message and does NOT stage the working tree — so this amend carries both rounds. A reviewer caught that; the message below now matches what is committed. All findings were adjudicated against the installed dsh 0.2.0-rc.2 before acting. Fixed: - Three comments (both mocks, the end-to-end write-verb loop) claimed an automatic upstream-drift guard that does not exist: "if upstream renames one, it drops out of this table and the write-verb POSTs go red". The table is a hand-written Go switch, so an upstream rename changes nothing locally — the map, both mocks and the test loop go stale together and no test turns red. Reworded to say what is true: the table is a hand-maintained MIRROR of the upstream `@Remote('<verb>')` declarations that must move in the same change as ownSessionWriteMethods; what CI actually catches is internal drift between the copies; and the one automatic pin is TestProxySessionOwnMarking's bidirectional map<->table check. - The eight verb names were hardcoded in four places. The end-to-end loop now derives them from ownSessionWriteMethods (same package), so it cannot become a second hand-maintained copy; the explicit literal set stays in TestProxySessionOwnMarking's table, which is where that pin belongs. - The "trusted-host is variadic" subtest was renamed to what it actually pins (the accepted argv FORM). The accepted-argv set is provably identical under the old single-value map and the new variadic map — a bare token already fell through the generic positional branch — so no black-box test can discriminate them, and the comment now says so instead of implying coverage of the mechanism. - The remote-incapable mask note's two justifications were optimistic: a pre-upgrade blob's instance is marked stopped by ReconcileRunningOnStartup and activate() returns early without polling it, and the blob is written once at ready/proxy time and never re-derived, so a later downgrade cannot rewrite it. Both were traced and are unreachable. The code is kept — the conclusion is right — but the justification is now the honest forward- looking one: the two floors are deliberately separate constants, so a future spawnable-but-bridge-less dsh is exactly when the branch returns. - CHANGELOG said the loopback-fallback advice "was dropped from the dsh capability warnings". It was dropped from the two VERSION-GATED branches and deliberately kept in cap._error; narrowed to say exactly that. Rejected, with reasons: - "index.html's cap._error branches (:3081 / :3167) still carry dead loopback-fallback advice". cap._error is set only when the capability FETCH failed, so the installed dsh version is UNKNOWN — it may be a perfectly good 0.2.x, and a local Start would then succeed. That is exactly why the advice is actionable there rather than dead. (The reviewer now agrees the version is unknown; the disagreement is only whether a local Start helps, and it does when the version is unknown.) index.html unchanged. Review round 4 (two more reports; both reviewers approve this delta). All findings adjudicated before acting; one of them corrected an instruction I had given a sub-agent, and one corrected a claim I had made myself. Fixed: - kinds/dsh_web.js: the polling comment claimed "a dsh upgraded mid-session flips this on the next poll". It does not: the poll re-reads the SAME persisted blob every tick and handleInstanceDshInfo only unmarshals and echoes it, so an in-place upgrade cannot flip the value until the instance is re-spawned. That directly contradicted the note this PR had just added 100 lines away. Reworded, keeping the correct "zero value before readiness" sentence. - integration_test.go: the comment claimed the mock's accepted-argv set was "provably identical" before and after the variadic change. It is NOT, and I verified both directions by building the mock either side of the change: --trusted-host --bogus OLD: accepted, booted NEW: unknown option, exit 1 The old single-value i++ swallowed an option-like token as the flag's VALUE and never validated it, so a misspelled flag could hide behind --trusted-host. The new behaviour is commander-correct and closes a real (small) hole. The comment is corrected and the discriminating negative case is now pinned for both mocks. (My round-3 instruction to the sub-agent — "no black-box test can discriminate them" — was wrong; this counterexample is one.) - testdata/dsh-mock-modern.go: its comment claimed a CI guard that never ran against that file. buildMockDsh builds only dsh-mock.go, and nothing POSTed any of the eight write verbs to the modern mock, so its case list was entirely unexercised — a typo there kept CI green. Fixed both halves: the loop is now mirrored into TestDshWebRemoteEndToEnd (same relay-cookie path the existing session/create POST there uses, asserting result.ok rather than HTTP 200), and the comment names the guard that actually covers this copy. - version.go: "existed throughout 0.1.x (ready line since dsh 0.1.2)" was self-contradictory — 0.1.0/0.1.1 predate 0.1.2. Now "existed in 0.1.x". - Three places disagreed on the read-only exemplar: proxy.go was corrected this PR to verbs that exist on 0.2, but proxy_test.go and ARCHITECTURE.md still used session/history, which does NOT exist in dsh 2's session namespace (the only @Remote('history') anywhere is dsh-schedule's). Both siblings now use session/list. - CHANGELOG bullet 1 still called the loopback fallback "(now dead)", contradicting bullet 6 one line below. Aligned, and the PR description was corrected the same way. Not changed: nothing in index.html. The round-2 finding about the cap._error branches was retracted by the reviewer after they checked the shipped code rather than only reasoning about it — the capability probe is gated on onDsh && remote, the version is UNKNOWN there, and a local Start may well succeed, so that advice is the only one that can help. --------- Co-authored-by: linletian <linletian@users.noreply.github.com>
… (issue #87) (#90) * fix(terminal): WS handshake replayed the 64KB tail on every reconnect (issue #87) A network blip, a proxy idle timeout or a laptop sleep/wake duplicated or garbled the terminal. ws.onclose reconnects after 5s without clearing the screen, while completeHandshake unconditionally replayed Tail(id, 64*1024) to EVERY connection, so the newest 64 KB landed under whatever the terminal already showed. Root cause: the WS handshake had no offset contract. The connect URL carried only id/token, the handshake threw away Tail's end offset via `_`, and the client's logCursor served only the SSE path -- so the client could not even express a position, let alone have one honoured. Fix: the handshake gains a cursor round-trip. GET /api/instances/tty/ws?id=<id>[&since=<offset>] since absent/unparseable -> -1 sentinel -> Tail, unchanged (first connect). since >= 0 -> catch-up loop, see below. completeHandshake(since) loops ReadSince(id, cursor, 64*1024) until the body comes back empty, retaining only the NEWEST chunk, so peak memory stays ~64KB regardless of the ring cap and the replay is the newest <=64KB contiguous with head. A single capped ReadSince would instead publish a cursor 64KB BEHIND head while the live subscription starts AT head: those bytes land in neither path and, since a client cursor only moves forward, they are never re-requested. Retaining the newest chunk (rather than a rolling 64KB window) keeps the stronger property that every replayed byte is new to the client. After the replay the server emits a TEXT control frame: {"type":"sync","offset":N} even when the replay was empty (0 is a legitimate cursor on a fresh instance). The client stores it as `ttyOffset`, advances it by the wire byte count of every binary frame received after that sync, preserves it across ws.onclose, and sends it back as `since` on reconnect. RingBuffer.ReadSince supplies the silent stale-cursor clamp for free. Cursor invalidation is screen-content driven: callers that wipe the screen (activate(), refreshCurrentInstance, resetTerminalForSwitch) reset BOTH cursors to the -1 sentinel at the clear, and a successful loadLog re-pins ttyOffset to logCursor immediately afterwards (one ring end offset, two consumers). The SSE handler advances both cursors together so a later WS reconnect cannot replay bytes SSE already painted. Because the cursor counts wire bytes, the server must never emit a non-ring binary frame: both handshake error paths now CLOSE the connection with 1013 instead of writing the diagnostic as binary output, matching the existing newTTYClientHandle precedent. The replay write error also aborts the handshake rather than advertising a cursor over undelivered bytes. Defensive: a first read that makes no progress consults Tail for the real head, because ReadSince echoes the caller's own number once the cursor reaches head. A cursor AHEAD of head (not reachable today -- Manager.Restart mints a new instance id, so a given id's ring head is monotonic) degrades to the first-connect tail instead of publishing a cursor the ring never held; output written between that read and the consult is picked up by continuing the loop rather than skipped. Tests: internal/app/tty_ws_test.go gives the endpoint its first coverage -- 8 TestHandleInstanceTTYWS_* cases driving a real ws.Dial against the real handler (no-redelivery / tail / silent clamp / sync offset / over-64KB contiguity / read-failure close / cursor-ahead-of-head / consult-sees-new- bytes), plus the at-head no-op and the non-running instance. The pty driver gains a since>=0 contract test so the app-test mirror cannot drift. Frontend gains 6 cases in testdata/terminal_status.test.mjs (39 total). Docs: docs/API.md (TTY WS section) and docs/ARCHITECTURE.md. * fix(terminal): stream the WS replay and close the review-round holes (issue #87) Follow-up to the first #87 fix, driven by two independent reviews of the handshake cursor contract. Server (internal/app/app.go): - completeHandshake STREAMED the replay instead of buffering the newest chunk. `replay = body` advanced the published cursor across the whole delta while delivering only its newest <=64KB, so any delta larger than one read stranded the older part forever — measured on a 100100-byte delta: 34564 bytes delivered while publishing offset 100100. Each chunk is now written to the socket as it is read, at the same iteration count and the same ~64KB peak memory, and endOffset is taken from that write's own end offset, so the server never publishes ahead of the wire. - ttyHandshakeReplayBudget (8MB) bounds one handshake's replay; the loop previously ran up to 4096 iterations against the live ring mutex while the client sat at "websocket live" with its handshake timer already cleared. Truncation DEFERS: the remainder sits ahead of the published cursor and the next reconnect re-requests exactly those bytes. Its cost (up to 32 reconnects at a 256MB ring, xterm scrollback churn) is now documented rather than left implicit. - The !first empty-body exit no longer leaves endOffset at its zero value. On a closed ring ReadSince answers ("", head, nil) while since < head, so the handshake could publish offset 0 — a cursor the ring never held — and drop the client back into a full tail re-replay. Client (index.html, kinds/pty.js): - startSSE now takes the cursor that is actually ahead. logCursor alone is -1 on the ensureTerminalLiveTransport promote path, which connects the WS without loadLog, so the SSE fallback would have replayed the whole tail over a live screen. Each missing sibling degrades to the -1 "unknown" sentinel rather than poisoning the max with NaN. - resetTerminalForSwitch invalidates both cursors itself; two comments claimed it already did, and it is exported on window for other kinds. - loadLog pins both cursors only inside its session.term paint block. - ws.onmessage ignores frames from a socket that no longer owns the session, so a replaced socket can never move the live cursor. - The handshakeSynced comment now gives the real reason for the latch (a clamped stale cursor means the replay need not start where we asked) instead of an inverted one, and states the replay->sync death window. Docs/tests: docs/API.md, docs/ARCHITECTURE.md and CHANGELOG.md now describe the streamed replay, the budget and its real exceptions; stale exact-source pins in internal/ui/ui_test.go were re-pinned to the new text without weakening their intent. TestHandleInstanceTTYWS_LargeDeltaStreamsEveryByte SinceCursor replaces the test that pinned the bug, plus new cases for the budget boundary and the closed-ring cursor; four new node --test cases. gofmt clean; go vet clean; go test ./... 21 packages ok; node --test 43/43. --------- Co-authored-by: linletian <linletian@users.noreply.github.com>
…#82) (#91) * fix(pty): an overflowing output subscriber was silently skipped (issue #82) broadcast() did a non-blocking send per subscriber behind an EMPTY `default:` arm. When a subscriber's 64-slot channel filled, the chunk was discarded and the subscriber stayed registered -- unbounded, permanent loss, with no counter, no signal to the client and no disconnect. The chain that reached it: the WS writer in handleInstanceTTYWS drains one chunk per select iteration and writes it straight to the socket, and ws.Conn.writeFrame blocks inside rw.Flush() while Upgrade sets no write deadline. A throttled or backgrounded tab stalls that write, the loop stops draining, the queue fills, and from then on every chunk vanishes. A missed clear-screen or cursor-move corrupts everything the client renders afterwards, and neither side can notice it happened. An overflowing subscriber is now DISCONNECTED instead of skipped: it is removed from the registry and its channel is closed, so the handler sees `<-outputChan` closing, answers with a 1013 close frame and returns. The browser's ws.onclose logs the abnormal close and reconnects, and thanks to the #87 offset contract it resumes incrementally from its own cursor rather than re-appending the whole tail. The close needs an exactly-once guarantee, because the channel now has two closers: the caller's cancel and the overflow in broadcast. A bare channel cannot express "already closed", and moving cancel's close() inside the lock does NOT fix it -- cancel's delete would be a no-op against an already-removed channel and the second close would panic. The registry value is therefore a *subscriber holding the channel plus a closed flag, with a closeLocked() helper, and both closers delete from the map before closing, under the existing subsMu. Because every close happens under that lock and every close is preceded by a delete, no channel present in the map is ever closed, so broadcast's non-blocking send can never hit a closed channel. The 1013 frame is bounded and best-effort. This branch exists precisely because the socket stalled, so an unbounded close write would only move the stall from WriteBinary to WriteClose and leave the handler parked -- which would not have fixed anything. A 5 s write deadline is therefore set on this failure path only; nothing writes on the conn afterwards. Resync does not depend on the frame arriving: the browser reconnects on any socket close and resumes from its cursor. Capacity stays 64 on purpose. A bigger queue only postpones the disconnect of a consumer that cannot keep up, which is the right outcome anyway. The real threshold is ~64 KB of unread output (1024-byte pumpLogs chunks x 64 slots), and recovery costs the client one reconnect against the browser's hard-coded 5 s retry. A write deadline for HEALTHY connections -- the gentler knob -- is issue #83 and is deliberately not here. SubscribeOutput's public signature and its empty-id rejection are unchanged; the only production consumer is the tty WS handler. Tests: internal/instance/pty/driver_test.go gains overflow-disconnects, within-capacity-is-never-disconnected, cancel-after-overflow-does-not- panic and cancel-races-overflow. internal/app/tty_ws_test.go gains an end-to-end test driving the real handler over a hijacked WebSocket and asserting the 1013, the socket teardown and the empty registry. Note the end-to-end test exercises the producer-outruns-the-queue condition over a real socket; it does NOT reproduce the TCP stall itself -- what it pins is everything downstream of the close. * fix(pty): bound the overflow teardown and make it observable (issue #82) Follow-up to the PR #91 review round. Two independent reviews landed; every finding was reproduced against the code before being acted on, and all of them held. 1. The failure path was not actually bounded. Go delivers a closed channel's BUFFERED values with ok == true before it ever reports ok == false, so after broadcast closed the subscriber the WS handler had to WriteBinary every queued chunk -- up to 64 of them, ~64KB -- onto the very socket that stalled in the first place, and only then reach the `!ok` branch that sets the 5s write deadline. Those writes were unbounded, and in the canonical stall the handler is already parked inside a live WriteBinary and never reaches the deadline at all. broadcast now drains the closed subscriber's queue at the source (after the close, so the loop cannot block), so the consumer observes `!ok` on its very next receive. Discarding is safe: pumpLogs writes every chunk to the ring buffer BEFORE broadcasting it, so each drained chunk is replayed from the client's #87 cursor after it reconnects -- the whole premise of this fix. The 5s deadline is kept but its comment now says what it actually bounds: this close-frame write, on this teardown path. A handler already wedged inside a live write is reclaimed when that write returns; that is issue #83's write-deadline territory and stays out of scope. 2. The overflow had no server-side trace at all. An earlier round declined this on the premise that `internal/app` does no logging -- which was my own grep error: I searched for the stdlib `log.Printf` instead of `s.logger.Printf`. Server has a `logger *log.Logger` field (app.go:65) with 16 call sites in this file and a TTY-specific one at app.go:522. This PR introduces a new user-visible disconnect while the client retries on a hard-coded 5s timer with no backoff, so a consumer that overflows repeatedly was a silent infinite reconnect+replay loop whose only trace was a browser console warning. Added a nil-guarded log line matching that precedent. 3. TestSubscribeOutput_CancelRacesOverflow did not prove what its comment claimed. `broadcast` holds subsMu across BOTH its map delete and its closeLocked, so once broadcast has deleted the entry, cancel cannot reach its own delete until broadcast releases the lock: the two closeLocked calls can never run concurrently regardless of the flag. The serialisation is the map delete, not lock discipline around the flag. Moving cancel's closeLocked outside subsMu still passes under -race, which is exactly why the old wording was unfalsifiable. Both that comment and closeLocked's doc now state the real division of labour, and the test says plainly what it does NOT prove. The `closed` flag remains load-bearing for the sequential cases (cancel after an overflow, cancel twice), pinned by TestSubscribeOutput_CancelAfterOverflowDoesNotPanic. 4. The CHANGELOG asserted that the overflow arm preserves the "key exists iff it has a live subscriber" invariant, but removing `delete(subs, id)` left the package green. Now pinned with registeredInstanceCount() delta assertions at the len(subs) level, plus a complement asserting the key STAYS while a healthy subscriber remains. Removing the delete now fails the test; removing the drain fails three. 5. docs/API.md gained the new wire-visible outcome this PR introduces: close code 1013 with reason "subscriber overflow: slow consumer", the ~64KB trigger, that the discarded backlog is replayable, that the frame is best-effort, and that the 5s deadline is teardown-only. Behaviour change worth noting: draining flips three existing backlog-read assertions to pin the new `!ok`-first semantics. That is the mandated fix, not a weakened test -- no test was deleted and coverage strictly grew. * fix(pty): drain outside the process-global lock, and make the drain honest (issue #82) Second review round on PR #91. Four findings, all reproduced by the orchestrator against the code before being acted on; all four held. 1. BLOCKING (mcode): the drain-at-source ran INSIDE the subsMu critical section. subsMu is one process-global mutex shared by every instance, and `for range` over a channel terminates only because that channel is already closed -- which is discipline, not a compiler-enforced guarantee. One future edit that drops or reorders the closeLocked() call, while keeping the load-bearing-looking `delete`, would park broadcast holding the global lock forever: no panic, no log, no timeout, every instance's PTY output silently frozen. Before the drain existed the same slip produced a loud, local `panic: send on closed channel`; the drain had traded a local failure for a global silence. broadcast now collects closed subscribers under the lock and drains them in its deferred func after subsMu.Unlock() -- unlock is the first statement, so the drain provably runs lock-free and touches nothing but the collected slice. closeLocked() now returns whether THIS call performed the close, so a subscriber is collected only when we provably closed it in this same critical section. Worst case of any future breakage degrades from "global hang" to "no drain". 2. The overflow log was never executed by any test: ttyWSTestServer mounted `&Server{instanceMgr: m}` with a nil logger, so deleting the s.logger.Printf line left the whole internal/app package green. The test server now injects a real logger through a mutex-guarded buffer, and the overflow test asserts the handler logged the id. `app.New` rejects a nil logger, so the guard is defensive against a state production cannot reach -- now noted where the guard lives. 3. The e2e test double did not mirror the drain, so the test exercised the pre-drain shape and reverting production's drain left internal/app green. The double now drains after releasing its own lock, same as production, and its comment says plainly that production's drain is pinned at the pty layer, not here. 4. "Nothing is lost" needed a qualifier. `Manager.dropBuffer` calls RingBuffer.Close (closed=true, data=nil, used=0, head NOT reset) and deletes the buffer; pumpLogs holds its own pointer and still broadcasts chunks that went nowhere. On a closed ring Tail returns ("", head) and ReadSince clamps to head, so a drained chunk is not replayable in that narrow window. API.md and the CHANGELOG now say "nothing is lost THAT IS STILL IN THE RING BUFFER" and name the exception. No code change -- ARCHITECTURE 4.1 already declares post-swap chunk loss intentional. Separately, and found while fixing (3): the exact binary-frame-count assertion added last round is FLAKY -- measured at roughly 1 run in 10 under scheduling load, and missed entirely by clean-room runs. Its third premise is false: it assumed the handler takes nothing after the drain starts, but close() readies any receiver already parked on the channel, and the handler IS parked in its select, so it competes for the same buffered values and may win some. The count therefore varies with scheduling and cannot discriminate drained from undrained at that layer, so the assertion is removed rather than loosened -- a tolerance there would be a fake pin. The 1013 close code, the reason naming the overflow, and the no-second-sync check are unaffected and remain. Every production and test comment that promised the drain deterministically beats the consumer has been reworded to distinguish what IS guaranteed (the queue is empty when broadcast returns, so no full ~64 KB queue reaches a stalled socket) from what is merely typical (the next receive is the close). One test comment that stated the drain runs inside subsMu -- the exact opposite of the load-bearing post-unlock placement -- was corrected. Verified: gofmt clean, go build ./... and go vet ./... clean, go test -count=1 ./... all 22 packages green, go test -race -count=10 ./internal/instance/pty/ and -race -count=5 on the TTY WS suite clean, and the e2e suite run 150 times under concurrent load with 0 failures. * docs(test): retract an overreaching claim about the dropped frame-count pin (issue #82) Third review round. mcode approved with zero blocking items. The independent review raised one actionable item, and it was aimed at a sentence I wrote last round. When I removed the exact binary-frame-count assertion, the reason I recorded was too strong. I had measured only that the EXACT assertion `binaries == published-17` flakes (about 1 run in 10 under scheduling load, in two shapes), and I wrote from that: "No bound separates the drained case from the undrained one, so widening it to a tolerance would pin nothing" and that the frame count is "simply not an observable of the drain at this layer". Neither followed. A third-round review measured the discriminator directly and showed the count is in fact a clean separator here -- 280 drained runs all inside [0, 2] against 60 drain-less runs all at 17, so a loose `frames > 8` bound would very likely catch a revert. Both sentences are now retracted explicitly, quoting what they said and saying plainly that neither was measured. The justification that replaces them claims only what the measurements support, and is sharper than either argument alone: the reviewer's own re-measurement put drained at <= 2, but the 1-in-2000 flake that motivated removing the assertion was itself 17 frames WITH the drain in place -- inside the undrained region. So no threshold can both catch a revert at `> 8` and survive that run, and a bound here is load-sensitive for coverage that is already pinned deterministically one layer down (the pty package, where the test goroutine is the channel's only consumer and nobody races the drain). The decision to drop the assertion is unchanged; only the reason was wrong. No bound is reintroduced -- the reviewer explicitly leans that way and stability wins over coverage we already have. Three further overreaches of the same kind were found in the same file and corrected in the same pass: "nothing drains" (contradicted by the race documented further down), "socket buffers never become the binding constraint" (an unmeasured absolute), and "says nothing about whether the drain exists" (same overreach as the first two). Comment-only: the diff against the parent commit, filtered to non-comment lines, is empty. Verified green: gofmt clean, go build ./... and go vet ./... clean, go test -count=1 ./... all 22 packages, -race -count=10 on internal/instance/pty, and -count=30 on the overflow e2e test. --------- Co-authored-by: linletian <linletian@users.noreply.github.com>
* fix(terminal): a half-open TTY WebSocket read as live (issue #83) hasLiveTTYConnection checked four instantaneous conditions and nothing else: session exists, a socket reference, readyState === OPEN, and a client-side state string. A half-open TCP socket -- laptop sleep, a NAT/proxy idle timeout, a silently swapped network path -- keeps readyState OPEN and never fires onclose, so every one of those still held and the UI reported the connection as healthy. Consequences: selecting the instance showed the stale xterm buffer and never reloaded; the 2s poll RE-AFFIRMED the dead socket on every tick, because ensureTerminalLiveTransport gated on the same function; and keystrokes were silently swallowed, since term.onData gates on readyState === OPEN and ws.send() does not throw on a half-open socket, so the HTTP input fallback was never reached. Server-side, nothing on this path generated traffic or set a deadline, so ReadMessage blocked forever and the handler goroutine, reader goroutine, fd and ttyClients entry pinned the daemon for its entire lifetime. Liveness now rides on heartbeat TRAFFIC, never on the absence of program output -- a shell at a prompt, vim, less, top or an idle agent TUI emit nothing for hours and must never be disconnected for it. Server: internal/ws gains WritePing (the opcode and writeFrame primitive already existed; only WriteText/WriteBinary/WritePong/ WriteClose were exposed, so a server ping could not be emitted at all). handleInstanceTTYWS becomes a thin wrapper over handleInstanceTTYWSLiveness(w, r, pingInterval, readDeadline) -- the intervals are parameters, not mutable package state, so a test can compress the windows to milliseconds without making a parallel test's connection flap. The reader goroutine arms a read deadline before every ReadMessage; on expiry it returns, msgChan closes, and the EXISTING `!ok` branch unwinds the handler through its existing defers (no second exit path). A ticker emits an RFC 6455 ping -- the browser's network stack auto-Pongs, refreshing that deadline from outside the app layer -- plus a TEXT {"type":"ping"} frame, because onmessage never fires for control frames and that text frame is the only heartbeat page JavaScript can observe. A client-sent {"type":"ping"} is answered {"type":"pong"} in a branch placed BEFORE the SendInput fallthrough, since a probe is transport traffic and forwarding it would type JSON into the user's shell. Client: parseTTYControlMessage whitelists ping/pong (anything it returns null for falls through and is painted into the terminal as literal text). lastDataAt is stamped as the first statement of ws.onmessage for every frame. A 5s watchdog, armed on READY, sends the client probe -- the only way to detect a socket whose server->client direction is dead but whose client->server direction still looks writable -- and treats the socket as dead after 30s of silence, forcing a reconnect. isTTYHeartbeatStale is consulted by hasLiveTTYConnection, hasTerminalTransportInFlight and ensureTerminalLiveTransport, which is what breaks the poll's re-affirmation loop rather than just patching activate(). Staleness requires READY, so a CONNECTING socket is still in flight and no second socket can open. Two ordering bugs found in review, both real: - ensureTerminalLiveTransport's stale arm called connectTTY with the stale socket still attached. disconnectTTY only reaches IDLE when there is NO socket, so the session landed in DISCONNECTING and connectTTY burned ~1.05s in its 10x100ms wait loop. Since the 2s poll crosses the 30s threshold before the 5s watchdog, that was the path healing most half-open sockets. - Both activate() paths hit the same spin, because #83's !stale gates in hasTerminalTransportInFlight newly released the stale-socket case into loadLog().finally(connectTTY). Both are fixed by reconnectStaleTTY (disconnect -> force IDLE -> arm the retry AFTER disconnect, which clears any prior timer), used by the watchdog, the poll arm and the overlay recovery, plus a root fix in connectTTY: disconnectTTY unconditionally closes and nulls the socket, so a DISCONNECTING left behind describes a socket that no longer exists. The wait loop is now unreachable for every caller and is kept as labelled defence in depth. reconnectStaleTTY connects immediately when there is no socket -- that is the connection-overlay recovery path, where the user is waiting and there is nothing to back off from. Numbers, with the margin documented at the constants: server pings every 10s, read deadline 45s (4.5 ticks), client staleness 30s (3x the ping, below the server deadline so the browser reaps first and keeps its cursor). A background tab's setInterval is throttled to ~1/min while WebSocket delivery is not, so inbound heartbeats keep lastDataAt fresh there; a short server interval cannot cause background churn. Scope, stated honestly in the docs: the TTY write path still carries no write deadline, so a handler blocked inside WriteBinary is reclaimed when that write fails or returns, not by the read deadline. And non-browser clients get no automatic Pong, so any client of this endpoint must answer RFC 6455 pings or send {"type":"ping"}. Tests: internal/ws gains TestWritePingRoundtrip (0x9 on the wire, 0xA echo, empty payload included). internal/app gains heartbeat-frame, probe-not-typed-into-the-PTY, dead-peer-reaped, and responsive-peer-survives-3x-the-deadline cases -- the last proving the positive half of the invariant, that a peer emitting zero output with zero program bytes stays alive and usable. Frontend gains 7 cases in testdata/terminal_status.test.mjs (46 total), including the exact loadLog().finally(connectTTY) activation wiring that regressed. * fix(terminal): route input around a heartbeat-stale socket and correct the review's docs claims (issue #83) * docs(terminal): one read-side scope note, both healers named, the input gate documented (issue #83) Review round 2, prose and comments only -- zero executable lines changed, no test touched, so the node suite stays at 58 cases and CHANGELOG.md still says "Pinned by 58 `node --test` cases". F1: fe9b305 shipped docs/ARCHITECTURE.md line 297 with the new §5.6 read-side scope note pasted TWICE on one line, the two copies differing only in wording ("the deadline persisting" vs "`SetReadDeadline` persisting"), which is why grep -c reported 1. Kept the second copy: it names the API the reader goroutine actually calls (app.go's conn.SetReadDeadline), and its closing clause names the declined alternative precisely -- "a second deadline armed around the send ... with no measured failure to justify it" -- where the first copy said only "arming a deadline ... no measurement has ever shown open". No distinct content to merge; every claim survives. Line 297 goes 2683 -> 1613 bytes, and each of the five phrases now occurs exactly once. The write-deadline scope note in the same paragraph is byte-identical to fe9b305 (535 bytes). F2: the #83 CHANGELOG entry showed the input gate as the BUG (term.onData gating on readyState) and then tested the FIX (seven FIX-A cases), but never said the gate changed. Added the sentence that does: both factories' onData arms now gate on the heartbeat stamp, pty.js through the new window.isTTYHeartbeatStale export, and what that honestly buys -- not the 0-30s window, where the predicate cannot tell half-open from healthy and the keystroke is still send()'d into a socket nobody reads, but the sliver between the stamp crossing the threshold and the reaper, plus the two factories no longer answering "is this live?" two different ways. F3: fe9b305 bounded that sliver by the watchdog alone ("at most TTY_LIVENESS_CHECK_MS") in BOTH factories' comments, contradicting ARCHITECTURE.md and API.md two documents away, which both name the 2s poll as primary. Restated as the FASTER of the two: ~2s for the active session, because refresh() -> reconcileTerminalSessions() -> syncActiveTerminalStatus() -> ensureTerminalLiveTransport()'s stale arm -> reconnectStaleTTY() is the chain that nulls the socket there, and TTY_LIVENESS_CHECK_MS (5s) bounds only a session the poll does not promote. F4: §5.8 enumerated the stale rejection, the ping/pong whitelist, the helper routing and the interval clears, but not the input gate -- the one item whose removal silently reverts to readyState-only. Added to the existing bullet. F5: recorded, NOT fixed. A keystroke buffered in the sliver is wiped by the teardown that follows -- but only on the legacy path: disconnectTTY clears termSendTimer and resets termInputBuffer, while pty.js buffers under _inputBuffer/_inputFlushTimer, which it touches neither, so there the identical keystroke survives and its POST still fires. Another drift of the two session factories on parallel field names; noted in both factories and in the entry. --------- Co-authored-by: linletian <linletian@users.noreply.github.com>
…86) (#93) * fix(terminal): WS handshake replayed the 64KB tail on every reconnect (issue #87) A network blip, a proxy idle timeout or a laptop sleep/wake duplicated or garbled the terminal. ws.onclose reconnects after 5s without clearing the screen, while completeHandshake unconditionally replayed Tail(id, 64*1024) to EVERY connection, so the newest 64 KB landed under whatever the terminal already showed. Root cause: the WS handshake had no offset contract. The connect URL carried only id/token, the handshake threw away Tail's end offset via `_`, and the client's logCursor served only the SSE path -- so the client could not even express a position, let alone have one honoured. Fix: the handshake gains a cursor round-trip. GET /api/instances/tty/ws?id=<id>[&since=<offset>] since absent/unparseable -> -1 sentinel -> Tail, unchanged (first connect). since >= 0 -> catch-up loop, see below. completeHandshake(since) loops ReadSince(id, cursor, 64*1024) until the body comes back empty, retaining only the NEWEST chunk, so peak memory stays ~64KB regardless of the ring cap and the replay is the newest <=64KB contiguous with head. A single capped ReadSince would instead publish a cursor 64KB BEHIND head while the live subscription starts AT head: those bytes land in neither path and, since a client cursor only moves forward, they are never re-requested. Retaining the newest chunk (rather than a rolling 64KB window) keeps the stronger property that every replayed byte is new to the client. After the replay the server emits a TEXT control frame: {"type":"sync","offset":N} even when the replay was empty (0 is a legitimate cursor on a fresh instance). The client stores it as `ttyOffset`, advances it by the wire byte count of every binary frame received after that sync, preserves it across ws.onclose, and sends it back as `since` on reconnect. RingBuffer.ReadSince supplies the silent stale-cursor clamp for free. Cursor invalidation is screen-content driven: callers that wipe the screen (activate(), refreshCurrentInstance, resetTerminalForSwitch) reset BOTH cursors to the -1 sentinel at the clear, and a successful loadLog re-pins ttyOffset to logCursor immediately afterwards (one ring end offset, two consumers). The SSE handler advances both cursors together so a later WS reconnect cannot replay bytes SSE already painted. Because the cursor counts wire bytes, the server must never emit a non-ring binary frame: both handshake error paths now CLOSE the connection with 1013 instead of writing the diagnostic as binary output, matching the existing newTTYClientHandle precedent. The replay write error also aborts the handshake rather than advertising a cursor over undelivered bytes. Defensive: a first read that makes no progress consults Tail for the real head, because ReadSince echoes the caller's own number once the cursor reaches head. A cursor AHEAD of head (not reachable today -- Manager.Restart mints a new instance id, so a given id's ring head is monotonic) degrades to the first-connect tail instead of publishing a cursor the ring never held; output written between that read and the consult is picked up by continuing the loop rather than skipped. Tests: internal/app/tty_ws_test.go gives the endpoint its first coverage -- 8 TestHandleInstanceTTYWS_* cases driving a real ws.Dial against the real handler (no-redelivery / tail / silent clamp / sync offset / over-64KB contiguity / read-failure close / cursor-ahead-of-head / consult-sees-new- bytes), plus the at-head no-op and the non-running instance. The pty driver gains a since>=0 contract test so the app-test mirror cannot drift. Frontend gains 6 cases in testdata/terminal_status.test.mjs (39 total). Docs: docs/API.md (TTY WS section) and docs/ARCHITECTURE.md. * fix(terminal): a stripped X-Log-Offset replayed the tail twice (issue #86) GET /api/instances/log returns the tail and carries the real end offset in the X-Log-Offset header; the browser keeps that byte offset as session.logCursor. If a proxy in front of the daemon strips the header the browser has no cursor, so startSSE omits `since` entirely, the server falls into its tail default, and the tail is painted a second time on top of what loadLog just drew. The WS handshake had the same shape and duplicated the 64KB tail as binary frames. Root cause: TWO different "no valid offset" states were collapsed into one -1 sentinel. nothing painted -> the client genuinely wants the tail painted, offset unknown (header stripped) -> the client wants NO replay at all; it already has the tail Fix: a "follow from live end" sentinel, sinceFollowLiveEnd = -2, meaning send nothing, publish the current end offset, then stream only newly produced data. It is branched on EXPLICITLY in both handleInstanceLogStream and completeHandshake, ahead of their existing `since < 0` tail branch -- parseInt64Default passes any negative through untouched, so an unbranched -2 would behave exactly like an omitted `since` and reproduce the duplication this value exists to prevent. On SSE that is one empty frame `{"chunk":"","next":<head>}` (writeSSELogEvent already always writes both fields, so no new frame format is needed); on WS it is zero binary frames plus the existing `sync` control frame. New Manager.EndOffset(id) returns the head without copying the buffer: it is Tail(id, 0) in every respect that matters -- same lookup, same kind delegation with a negative since, same clamps (a kind without log capture echoes -1 and gets 0; a non-running instance reports 0, exactly as Tail returns ("", 0, nil)) -- except that RingBuffer.Tail short-circuits n <= 0 to ("", head). Tail(id, 0) itself cannot be reused because Manager.Tail clamps n <= 0 to 4096. internal/framework/ kind.go now states the contract this relies on: a tail read must carry its cursor even at maxBytes == 0. Client: CURSOR_UNKNOWN (-1) and CURSOR_FOLLOW_LIVE_END (-2) are named constants in index.html, mirrored (prefixed) in kinds/pty.js. loadLog sets the sentinel only after bytes have actually reached the screen -- the gate is the byte count writeSanitizedTerminalOutput returns, not the response length, because a tail of nothing but stripped escape sequences sanitizes to "" and paints nothing. A failed or aborted fetch leaves CURSOR_UNKNOWN, so an empty screen still asks for the tail. Both transports send the sentinel explicitly; neither ever sends since=0, which the server would read as the oldest live byte (issue #81). GET /api/instances/log now 400s the sentinel rather than silently serving the opposite of what it means. Trade-off, stated honestly in the code, docs and changelog rather than glossed: this trades duplication for a bounded loss. Bytes produced between the head loadLog's tail read (H1) and the head the sentinel publishes (H2) are in no painted body, no replay and no live frame, and the published cursor already sits past them, so no reconnect re-requests them. That is milliseconds normally; on the WS-handshake-timeout -> SSE fallback it is 5s + 500ms of real lost lines. It only applies behind a header-stripping proxy. Also fixed here: a guard test that fails when two classic scripts declare the same top-level name. kinds/pty.js and index.html's inline block both declared `const CURSOR_UNKNOWN`, and classic scripts share one global lexical environment, so index.html's entire application script died at parse time with "Identifier 'CURSOR_UNKNOWN' has already been declared" -- a total SPA outage. The existing node harness slices each function with new Function(...) and so is structurally blind to that constraint. * fix(terminal): check both sibling cursors for the live-end sentinel (PR #93 review) Two findings from the PR93 reviews, both verified accurate against the code before fixing. startSSE's sentinel fallback checked only session.logCursor while its own comment says a real cursor outranks -2 "on either side". The fallback now checks both siblings: else if (logCursor === CURSOR_FOLLOW_LIVE_END || ttyOffset === CURSOR_FOLLOW_LIVE_END) Unreachable today -- the only -2 assignment site (loadLog) sets both cursors together, confirmed by grep -- but a future single-sided ttyOffset assignment would otherwise silently omit `since` and replay the whole tail over the painted screen, the same shape as the #87 duplication. Behaviour is bit-identical in every state that exists today. connectTTY is deliberately unchanged: it reads ttyOffset in both branches, which is complete there because logCursor is only ever written at paired sites while ttyOffset additionally advances alone (WS sync frame, latched binary frames), making ttyOffset the superset cursor. That reasoning is written into both comment blocks, and the ui_test.go source anchor was updated to the new two-sided line. Also narrows writeSanitizedTerminalOutput's return-semantics comment: it returns the sanitized text's `.length` (UTF-16 code units, NOT bytes), used only as a `> 0` boolean gate, and must not be used for byte accounting. Aligned wording in loadLog's painted-gate comment, the harness stubs, docs/API.md and docs/ARCHITECTURE.md. Node suite gains a case driving the split state {logCursor:-1, ttyOffset:-2} asserting `since=-2` still goes on the wire, plus the mirror split, the both-unknown omit, and a real cursor outranking -2 (67 total, CHANGELOG count re-derived). --------- Co-authored-by: linletian <linletian@users.noreply.github.com>
- release-beta.yml: v*-beta.* tags build the platform matrix and publish a GitHub prerelease; release.yml jobs are guarded with !contains(github.ref_name, '-') so a beta tag never fires the stable pipeline (its v* pattern would otherwise match) - install.sh: --beta / MYWORKTREE_BETA=1 resolves the latest prerelease via the releases atom feed (API fallback); -v already accepts beta tags; the existing-binary version regex now tolerates prerelease suffixes - RELEASING.md: beta runbook (tag on develop tip, prerelease, no merge to main, no back-merge, /releases/latest stays on stable) - README/README.zh-CN: one-line beta install instructions First beta: v0.5.1-beta.1, cut from the tip of develop.
The documented 'NO_MODIFY_PATH=1' never took effect: the env value was compared literally against "true", so only --no-modify-path (or NO_MODIFY_PATH=true) actually skipped the rc-file edit. Normalize both no_modify_path and want_beta to accept 1/true/yes after arg parsing.
… instance (issue #96) selectInstance only calls prevRenderer.deactivate() when the previous renderer is a DIFFERENT object, so switching A(running) -> B(stopped) within one kind never hid A's keep-alive iframe; and each renderer's stopped branch shows the stopped overlay and returns early without hiding the other cached frames (only the running path's _showOnly does). The two stacked half/half — overlay on top, A's live iframe below. A cross-kind switch (terminal -> B) did not reproduce because it DID deactivate. All three web-ui renderers shared the defect in different shapes: kinds/dsh_web.js and kinds/opencode_web.js in their status === 'stopped' early-return branches, kinds/reasonix.js in _tick's not-running branch (which hid only the active id's own frame). Each stopped/not-running branch now hides EVERY cached frame via this._showOnly(null) — nothing but the stopped overlay should be on screen. Pinned by TestWebRenderersStoppedSwitchHidesAllFrames.
…ge root dsh web --dump-config emits long scalars as >- folded block scalars, wrapping the line AT the value's whitespace. Every macOS storages root lives under ~/Library/Application Support/..., so the dump split the root mid-path and verifyOverlayDump's literal substring match could never hit: overlay_verified=false on EVERY macOS instance and the frontend's 'version too new / overlay not effective' bar stayed up even though the overlay was fully composed (the other three rows matched on the same dump). Linux paths have no spaces, so the check only ever passed contiguously there — the failure was invisible in dev. Live-reported on v0.5.1-beta.1 with dsh 0.2.0-rc.2. The storage-json check now falls back to blockContainsFolded, a whitespace-stripped comparison over the row block: YAML folding only breaks at whitespace, so stripping both sides is the same check with the emitter's wrapping undone. Pinned by TestVerifyOverlayDumpFoldedRoot.
…s-up A process's comm is the basename of the path it was LAUNCHED through, so a daemon started via the mw symlink reports as 'mw' and pgrep -x myworktree alone never sees it — on a machine with six live daemons the check matched zero and the 'restart the daemon to pick up the new binary' warning never printed (found during the 0.5.0 -> 0.5.1 upgrade-path manual test). Detection now checks both the binary name and the resolved alias name, deduplicated; the force-fallback pkill hint lists the alias form too.
# Conflicts: # CHANGELOG.md # README.md # README.zh-CN.md # scripts/install.sh
…me visible (issue #101) (#103) * fix(ui): hide web panels when switching to an instance-less worktree (issue #101) * fix(ui): share worktree-switch leave cleanup, pin deactivate ordering in tests (issue #101 review) - Extract deactivateInstanceRenderer helper: kind-agnostic unconditional deactivate, skips unresolvable ids instead of falling back to 'pty' - selectWorktreeByID (create/import flows) now runs the same synchronous leave cleanup — deactivate previous renderer, clear activeInst, render — so the old web iframe no longer covers the new worktree during the refresh round-trip - TestEmptyWorktreeSwitchHidesAllWebPanels asserts deactivate BEFORE state.activeWT assignment (position, not presence) - sliceJSFunction derives the end-marker indent from the declaration line instead of hardcoding 8 spaces * test(ui): pin deactivate-before-render ordering in selectWorktreeByID; changelog covers review follow-ups (issue #101 review round 2) - swb assertions now pin deactivateInstanceRenderer BEFORE render(); presence alone let a deactivate-after-render variant pass (Qwen nit) - CHANGELOG Unreleased entry extended with the deactivateInstanceRenderer helper and selectWorktreeByID leave-cleanup behaviour change (mcode nit) --------- Co-authored-by: linletian <linletian@users.noreply.github.com>
…about gitignored files, and offers force (issue #102) (#105) * fix(worktree): detailed delete refusal, force exit, gitignored-file warning (issue #102) * chore: gofmt the issue #102 tests * fix(worktree): review-round fixes — trailing --force, force skips the status probe, MCP 409 parity, ignored-loss surfacing, -uall counts (issue #102) Two independent PR reviews (findings verified locally before fixing): - CLI: myworktree worktree delete <id> --force silently dropped the flag (Go flag parsing stops at the first positional); scan for --force manually and reject extra positionals with a usage error. - force no longer gates on git status: the probe only builds the refusal, and on a damaged worktree (corrupt index) it fails while git worktree remove --force still succeeds — force skips it entirely. - MCP worktree_delete dirty refusal now answers the same 409 + worktree_dirty + structured breakdown as the HTTP API (was a flat 400). - A clean-looking delete still destroys every gitignored file with the directory; the count is now returned and surfaced (API/MCP ignored_destroyed, dashboard post-delete notice, CLI stderr note). - Both status probes run with -uall (untracked/ignored dirs count file-by-file) and -c core.quotePath=false (literal non-ASCII paths); the carried porcelain payload is capped at 200 lines. - UI nits: failed force delete shows body.message instead of the raw error code; Esc closing the dialog resets its state via cancel. * fix(worktree): second review round — omit ignored_destroyed on force, --force=<bool> and -- parsing, cap-comment nits (issue #102) Both second-round reviews confirmed every first-round fix; remaining nits: - ignored_destroyed is now present only when counted: a forced success omits the field instead of answering a 0 that read as 'none destroyed' while meaning 'not counted' (HTTP + MCP). - The CLI's manual scan honors --force=<bool> and the -- terminator like the flag-based subcommands; a garbage --force value fails loudly. - Stale maxDirtyFirstPaths comment (porcelain is capped now), singular 'line' in the truncation marker, and a comment naming the ignored-count probe-timeout edge case. * test(cli): resolve symlinks on temp paths — macOS CI failure (issue #102) The CLI keys the project state dir off HashPath(git rev-parse --show-toplevel), which returns the REAL path; on macOS t.TempDir() lives under /var, a symlink to /private/var, so the test wrote state.json under the unresolved path and the CLI looked it up under the resolved one — 'unknown worktree id' on the three delete tests that reach Manager.Delete. Reproduced locally by pointing TMPDIR through a symlink (fails without this fix, passes with it). --------- Co-authored-by: linletian <linletian@users.noreply.github.com>
…t the first match (issue #95) (#106) * test(ui): the node-test count guard now checks every claim occurrence, not the first (issue #95) The FindSubmatch anchor was positional: entries are prepended in reverse chronological order, so any future entry using the claim phrase above the #80 entry would silently hijack the guard — a correct claim above plus a corrupted real claim would still pass. Take the issue's stricter proposal: EVERY 'Pinned by N node --test cases' occurrence must equal the derived total, making the phrase position-independent (subset counts keep their distinct phrasing). Verified by injection: a stale claim above the anchor fails, and a correct claim above plus a corrupted #80 claim fails — the exact silent-failure mode named in the issue. * test(ui): review fixes — entry under ## Unreleased, located violations, documented convention (issue #95) Both reviews flagged the new entry landing between the title and ## Unreleased (an orphan the release flow would strand above every version section): moved into the section and given the house-style 'Pinned by TestName' ending. The guard's failure now reports the offending line and lists every violation in one run (t.Errorf). And the strict-rule convention is written down where entry authors will see it: an HTML comment at the top of CHANGELOG.md plus a line in RELEASING.md's changelog maintenance step. --------- Co-authored-by: linletian <linletian@users.noreply.github.com>
…drops resume (issue #94) (#107) * feat(terminal): publish the replay's real start offset so mid-replay drops resume (issue #94) After #87 the streaming handshake replay widened the "painted but not yet synced" death window from one <=64KB frame to the full 8MB budget: the client's handshakeSynced latch blocks frame-by-frame cursor advancement until the closing sync, because RingBuffer.ReadSince silently clamps a stale since and accumulating from the request could publish a cursor over bytes never delivered. A drop mid-replay therefore re-pulled everything from the old cursor — which does not converge under sustained network degradation. The catch-up loop now brackets the replay with TWO sync frames: {"type":"sync","offset":S,"start":true} ahead of the FIRST binary chunk — S = first chunk's next - len(body), so S and the chunk come from the same ReadSince call (the clamped oldest-live value whenever clamping happened, never the raw request, no new Manager surface) — and the unchanged closing sync after the last chunk. The frame reuses the "sync" type instead of adding a frame type plus a capability gate: any recipient provably parses "sync" already (caps=sync or the inferred explicit-since opt-in, #98; every catch-up request presents since by definition), and the shipped client's existing handler — adopt the ABSOLUTE offset, lift the latch — is exactly the wanted semantics, so the client needed zero code changes and even a never-refreshed v0.5.1 page benefits from a new daemon. "start":true exists for consumers that must know when the replay ends; the UI ignores it. Tail / since=-2 / caught-up-at-head paths still send the closing sync alone; completeHandshake stays single-shot per connection. Server: internal/app/app.go (completeHandshake). Tests: TestHandleInstanceTTYWS_ReplayStartOffsetPublishesClampedStart, TestHandleInstanceTTYWS_MidReplayResumeContinuesFromRenderedBytes, TestHandleInstanceTTYWS_StartSyncOnlyOnTheStreamingCatchUpPath, the extended 8MB budget test, and a new node --test case driving start sync -> replay frames -> mid-replay drop -> reconnect since. Harness: dialHandshakeFrames stops only at the sync WITHOUT "start":true and fails on a start frame arriving after any chunk. Contract documented in docs/API.md (TTY WS) and docs/ARCHITECTURE.md. * fix(terminal): address PR #107 review — start frame on the beyond-head degrade, doc contract sweep (issue #94) Two independent reviews (Qwen3.8-Flash-Next, MiMo-V2.6-Flash); all findings verified accurate before fixing except one wrong file cite. R2#2 (main): the cursor>head degrade wrote its tail binary chunk from inside the catch-up branch WITHOUT a start frame, contradicting the documented "exceptions emit no streamed chunk" rule. Now emits {"type":"sync","offset":head-len(tail),"start":true} ahead of the tail, so EVERY replay addressed to a cursor-bearing client carries a start frame; only the cursor-less first-connect tail goes without. Pinned by the extended TestHandleInstanceTTYWS_CursorAheadOfHeadFallsBackToTail. R1M1/R2#5: the start-frame emit is now an emitStartSync closure whose comment states syncCap is PROVABLY true at both call sites (catch-up requires an explicit since, already the inferred opt-in half) and the guard stays as defensive depth against future inference changes. R1M2: documented the defensive next<=cursor corner where S dips below the client's own since (bounded, self-healing re-render of [S, since)). R2#3: softened "S + sum(frames) always equals the closing offset" in API.md and CHANGELOG — a full ring rollover mid-replay clamps a later chunk above the running cursor and the closing sync jumps the gap (truthfully). ARCHITECTURE.md carried no such claim (wrong cite). R2#1: API.md's numbered handshake protocol, Message Types -> Sync and the example flow diagram now describe the start/closing sync pair; two singular phrasings ("with sync at the end", "The sync offset is") now name the CLOSING sync. R2#4: tty_ws_test.go's overflow-teardown comment no longer claims sync is written exactly once (assertion — no sync on the LIVE stream — unchanged). R2#6: recorded the accepted UTF-8 split trade-off in the client's latch comment (pre-existing on the live path, cosmetic, unrecoverable either way). * docs(terminal): second review round — stale step cross-reference, dangling referent, boundary-test rename (issue #94) All three round-2 findings verified accurate: - API.md's step 4 referenced "the closing sync in step 5" after the renumbering moved the closing sync to step 6 (step 5 is the binary replay) — a stale cross-reference in exactly the numbered-protocol doc that can least afford one. - The client latch comment's "unrecoverable either way" had a dangling referent: read as "pre/post-#94" it contradicts the preceding clause ("the pre-#94 full re-pull redelivered it"). Now names the two paths that have already stepped the cursor past the lead bytes — the replay path post-#94 and the live path. - The boundary test's name/comment still said "only STREAMING replays announce a start", but the contract since the R2#2 fix is "every replay chunk addressed to a cursor-bearing client" (the single-frame degrade included). Renamed to TestHandleInstanceTTYWS_StartSyncOnlyForChunkedReplaysWithACursor with the comment spelling out both sides and the tail-path failure message now naming the cursor-less reason. CHANGELOG reference updated. --------- Co-authored-by: linletian <linletian@users.noreply.github.com>
…start reattach Records a discussion (no decision, no code change) on what happens to pty terminal instances and their inner processes (e.g. opencode cli) when the myworktree daemon exits, and what it would take to keep them alive, traceable, and re-attachable after a restart. Key facts captured with file:line references: - The terminal dies with the daemon via PTY hangup, not an explicit kill: myworktree holds the pty master fd in the per-instance Handle (internal/instance/pty/driver.go) and Shutdown deliberately does not stop pty kinds (internal/app/app.go) — process exit closes the master, the kernel sends SIGHUP to the foreground process group and session leader. nohup/setsid-escaped processes become untracked orphans (no PDEATHSIG anywhere). - The framework already has a RestartSurvivor/Reattach path, implemented by reasonix only (internal/framework/methods.go ReconcileRunningOnStartup); pty is explicitly excluded because its in-memory stdin/stdout bindings and ring buffer cannot resume. - Converged requirement: survive a daemon restart and re-attach the session as-is, uninterrupted. The single necessary and sufficient condition: move PTY master fd ownership out of myworktree (tmux with an isolated socket is the mature form; a self-built supervisor is the alternative). - Documents the honest boundaries of "uninterrupted", the long-term unreclaimed-risk list, and the open decision forks (scrollback depth, hard dependency vs fallback, Shutdown semantics, orphan reaping). Also flags an adjacent pre-existing hole: dsh-web never implements Reattach, so a kill -9 daemon leaves dsh processes orphaned on their ports. Co-authored-by: linletian <linletian@users.noreply.github.com>
CHANGELOG: - Rename `## Unreleased` to `## v0.5.2 (2026-10-09)` with a release summary line; keep an empty `## Unreleased` placeholder at the top. - Add the entry for the terminal-detach discussion draft (docs-only commit on develop, shipped without a CHANGELOG line). The five merged PRs since v0.5.1 (#103-#107) already carried entries. README: - Bump download URLs, example tarball names, pin-version example, and recommended-version wording from v0.5.1 to v0.5.2 in both README.md and README.zh-CN.md. install.sh: - Bump the `-v` pin examples from v0.5.1 to v0.5.2 (header comment, `--version` usage examples, and the Examples block). No code changes; release prep only.
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
1 participant
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.
Content delta (main..develop since v0.5.1)
Merged PRs
Docs
docs/TERMINAL_DETACH_REATTACH_DISCUSSION.md)Fixed issues
node --testcount guard anchored on the FIRST match, so an entry above it could silently hijack the coverage claimHighlights
caps=ping,syncopt-in (or an inferred explicit-sinceopt-in for the sync echo), closing the "any future server control frame leaks onto stale pages" classDirtyWorktreeErrorwith per-category counts, first paths and the full porcelain output (409worktree_dirty), a force exit on API/MCP/CLI, and a gitignored-files danger warningnode --testcases" phrase must now equal the derived total, making the claim position-independentPrep summary
Unreleased→v0.5.2 (2026-10-09)+ summary line; entry added for the discussion draft (the five PRs already carried entries)-vpin example, recommended-release wording)-vexample versions bumped to v0.5.2No code changes in the prep commit; release prep only.