Desktop: enable the openknowledge.ai update proxy for the stable…#378
Closed
inkeep-oss-sync[bot] wants to merge 9 commits into
Closed
Desktop: enable the openknowledge.ai update proxy for the stable…#378inkeep-oss-sync[bot] wants to merge 9 commits into
inkeep-oss-sync[bot] wants to merge 9 commits into
Conversation
…ift toast (#2242) * fix(desktop): shut down stale server on app update to stop version-drift toast The desktop spawns its CRDT server detached so it survives app-quit (precedent #1261). But app updates install via app-quit too — either the silent `autoInstallOnAppQuit` path or a drag-replace — and only the "Relaunch now" button ran `prepareForRelaunch` → `stopAllOwnedServers`. So on the common update paths the old-version server kept running, the relaunched app attached to it, read the older version off `server.lock`, and showed the "this project is running an older version… restart it" toast on nearly every update. Hook the teardown to `before-quit-for-update` — the native autoUpdater signal that fires on BOTH install paths and ONLY on an update install, never a plain quit. New `WindowManager.signalStopAllOwnedServers()` sends a synchronous best-effort SIGTERM to every detached server this desktop spawned (the "Relaunch now" path already drained them, so it no-ops there). The event can't hold the quit open, but the server's own SIGTERM handler flushes pending writes and releases its lock in ~25ms (measured) — far inside the multi-second reinstall+relaunch window — so the new app spawns fresh instead of re-attaching. A normal quit never fires the event, so it still leaves the server running, as designed. The signalling loop is extracted as the pure `signalDetachedServerStop` helper with unit coverage (normal kill, ESRCH already-gone, non-ESRCH failure keeps going, empty no-op), since `WindowManager`'s teardown methods have no unit harness. * fix(desktop): also reap ephemeral servers on update + dedup utility-fork loop Address both reviewer "Consider" findings on #2242: 1. Ephemeral single-file (`ok <file>`) session servers are tracked on `ctx.ephemeral` (keyed by file path, not project root) and so weren't reaped by the new before-quit-for-update teardown — they'd orphan on the silent `autoInstallOnAppQuit` path (process + temp dir until reboot), the way the async sibling `stopAllOwnedServers` already prevents. Signal their pids too. Temp-dir removal is the async half this best-effort path can't do; the orphaned process (which holds the bundle binary) is what matters and dies on the SIGTERM. 2. The utility-fork SIGKILL loop was duplicated near-verbatim across both teardown methods. Extract the shared pure `signalStopOwnedUtilityForks` helper and call it from both, so the predicate + error handling can't drift — and the branch gains unit coverage (3 new tests). * test(desktop): cover signalStopAllOwnedServers orchestrator end-to-end Re-review follow-up: the extracted helpers were unit-tested but the orchestrator entry point `signalStopAllOwnedServers()` (wired to before-quit-for-update) had no direct coverage of its composition — collecting detached pids from `spawnedDetachedPids` + ephemeral pids from `windowsByPath`, draining the map, merging. Add a test through the existing ephemeral harness: seed a detached pid + an open ephemeral session, assert both receive SIGTERM, and assert a second call doesn't re-signal the detached pid (map drained). Guards against a future field-rename in the collection chain that would pass the helper tests but break orchestration. --------- GitOrigin-RevId: ab2b0fe9c2a6d159d9b3f3ccb3183fc170ae952d
…251) pickLatestBetaDmgUrl returned the first -beta.N release in GitHub's "List releases" array order, trusting it to be newest-first. It isn't: the API has been observed returning an older beta ahead of newer ones, and the bug surfaced the moment betas crossed from single- to double-digit (beta.9 was served while beta.10-13 existed), stalling /download/beta and the /updates/beta auto-update feed on a stale build. Pick the newest beta by numeric (major, minor, patch, beta) rank instead of position. Fixes both surfaces, since the updates route derives its tag from the same resolver. Adds regression tests for the older-first array order and the beta.9-vs-beta.10 lexical trap. GitOrigin-RevId: 353c018fe8860fd1ad02051f36989f6d2c97715c
GitOrigin-RevId: aa53d68b0739a9b4e289a10c46ab732544579bfa
* feat: align macOS File menu with the in-app project switcher Group the File menu's project actions (Recent project, New project, Switch project, Open folder) directly under "New from template..." in the same order as the bottom-left ProjectSwitcher, and update the switcher's action order to match. Rename "Create new project..." to "New project..." and "Open recent" to "Recent project" so both surfaces read identically. Wiring, accelerators, and behavior unchanged. * chore: centralize project menu labels, refresh stale references Address PR review feedback: - Fold the dual-surface 'New project' / 'Open folder' labels into MENU_LABELS so menu-label-parity.test.ts guards menu<->switcher wording drift (matching the existing shared-label pattern); normalize their ellipsis to the same escape the sibling labels use. - Add contiguity assertions to the menu order test so a separator inserted mid-section would be caught, not just relative ordering. - Refresh stale 'Create New Project...' references to the renamed label at the onNewProject call sites (index.ts, App.tsx, CreateProjectMenuTrigger, NavigatorApp) and in the onNewProject JSDoc. GitOrigin-RevId: ac7db1562d92e3ba8230ff7b17daa106d33d9e2a
…cs (PRD-7194) (#2248)
* fix(open-knowledge): stop dead-link check flagging freshly-written docs
`links({ kind: "dead" })` reported recently-created docs as dead even
though their target files existed and the backlink edge was already
registered. Dead-link resolution decided existence solely from the
caller-supplied admitted set (the file-watcher's file index), which lags
behind in-session writes. The same backlink index, meanwhile, registers
a new doc as a live forward node the instant `onStoreDocument` runs — so
the graph simultaneously held a backlink FOR the doc and reported it as a
dead link, an internal inconsistency that only cleared on a server
re-index/restart.
`getDeadLinks` now treats a target as existing when it is admitted OR is
a live forward node in the graph (`state.forward.has(target)`). A
genuinely-missing target is never a forward node (only `state.backward`
carries it), so this never hides a real dead link; a freshly-written doc
becomes a valid link target immediately within the same session.
Fixes PRD-7194.
* test(open-knowledge): pin dead-link deletion inverse + document existence contract
Address claude[bot] review on #2248 (both low-risk suggestions):
- Add a companion test asserting that after `deleteDocument` drops a doc
from `state.forward`, `getDeadLinks` reports it dead again when it's also
absent from the admitted set — guards against a future refactor that
retains stale forward entries and silently swallows real dead links.
- Add JSDoc to `getDeadLinks` making the existence contract explicit:
existence is `admittedDocs ∪ keys(state.forward)`, an additive second
oracle, not a narrowing of the admitted set.
---------
GitOrigin-RevId: 6834ae8bfb71348f858c05c55abc489b883733fa
…chain (#2254)
* test(open-knowledge): folder-rename regression coverage for PRD-7045 chain
PRD-7045 (folder rename corruption chain: duplicate folder, stale relative
links, blank WYSIWYG, perceived data loss) is already fixed in code — each
facet by a separate PR that merged before the 6/12 UX session that reported
it:
- duplicate folder + spontaneous reappearance → #740 (removalRedirectGuard
+ recentlyRemovedDocs, phantom-resurrection defense)
- blank WYSIWYG while source shows content → #1011 / PRD-6673 (spine moves
the file on disk before captureAndCloseDocuments, so the redirected client
loads real content instead of an empty Y.Doc)
- stale relative ../ links / orphaned inbound links → #1473 / PRD-6839
(folder rename enumerates descendant docs from disk, not the lagging index)
But the regression suites only covered FILE rename, while PRD-7045's reported
symptom is specifically a FOLDER rename. This adds folder-level coverage so the
chain can't silently regress:
removal-phantom-resurrection.test.ts (QA-FOLDER): a folder rename arms the
removal cache for every descendant doc — a reconnect to ANY old descendant
docName is redirected (resurrection vector severed for each), and every doc's
body survives the move (the blank-WYSIWYG facet at the disk layer).
api-folder-rename-disk-enum.test.ts: inbound links in every shape — wiki,
root-relative, dot-relative, and ../ / ../../ from nested sources — are
rewritten to the new folder with zero stale references; and relative links
authored INSIDE the folder (sibling ../, descendant ./sub/, outbound ../)
still resolve to a real file after the move (no orphans).
Test-only; no behavior change.
* test(open-knowledge): tighten folder-rename link assertions (review)
Address claude[bot] review on #2254:
- Pin full link shapes (`[banana](sub/banana.md)` etc.) instead of loose
`toContain('sub/banana.md')` substrings, so an incorrect re-relativization
(e.g. `../recipes/sub/banana.md`) would now fail. Note the rewrite
normalizes a leading `./` away while resolution stays correct.
- Assert the old `foods/` directory is gone after the rename, matching the
pre-existing describe block's self-containedness.
* test(open-knowledge): harden QA-FOLDER seed against CI watcher lag
QA-FOLDER failed twice on the merge tier with `pollUntilAsync timed out
after 8000ms` during seeding — the chokidar watcher lagged past 8s before
`/api/documents` reflected the fresh nested file, under a shard also running
the OK_TEST_STORE_FAULT disk-failure-simulation suites. The folder rename
enumerates descendants from disk and the guard keys off the spine-populated
cache + existsSync, so the index-appearance wait is the only fragile part.
Make `seedDoc`'s index-wait timeout overridable (default unchanged at 8000 so
sibling tests keep their timing), pass 20000 for QA-FOLDER's two seeds, and
raise that test's deadline to 60s to fit two generous seeds + the rename.
* test(open-knowledge): seed QA-FOLDER via agent-write-md (deterministic)
The prior fix (bigger seedDoc timeout) still flaked: QA-FOLDER hit the 20s
index-wait in the full test:integration job (990 tests / 220 files), where
the chokidar watcher lags past any fixed timeout under event-loop contention.
Replace the watcher-poll seed with `POST /api/agent-write-md`
(`position: 'replace'`), the same deterministic seed the Playwright tests use:
it lands the doc on disk AND in the index synchronously within the awaited
request, and routes through `writeTracker` so the watcher fires no later `add`
event for the path. The latter also closes a reverse-race — a post-rename
`add` for an old descendant path would otherwise invalidate the spine's
removal-cache entry (`onUpstreamAdd` → `recentlyRemovedDocs.delete`) and flake
the redirect assertion. Bonus: the seed loads a live Y.Doc, so the
body-preservation check now exercises the same "editor connected at rename"
path PRD-6673 fixed. seedDoc reverts to its original signature.
---------
GitOrigin-RevId: 5366010ca559145626a177bc7b2f9acca5e6593d
…259) Add 'latest' to the proxy-feed channel set so stable builds fetch updates through openknowledge.ai/updates/stable and stable updates are counted per version. Follows the verified end-to-end beta update (beta.13 -> beta.14) through the proxy. Stable resolves via GitHub's authoritative releases/latest alias (no version-sort dependency), and a feed failure still reverts to the GitHub provider for the session. GitOrigin-RevId: 56ff072a229b345f1d961f30b8285d7b567f3b85
…061)
* refactor: relocate process-scan discovery to the server package
Off-cwd server discovery (discoverLockDirs + process-scan helpers) moves from
packages/cli to packages/server so the MCP preview resolver, which lives in
server, can consume it directly. cli depends on server (not the reverse), so
the discovery primitive belongs in the lower layer.
cli keeps its existing import path via a re-export shim; ok ps / stop /
diagnose are unchanged. Resolves the resolver-home question for the off-cwd
addressing work.
* feat(server): off-cwd target resolver for the addressing guardrail
Add resolveOffCwdTarget: given an absolute target path, find the running OK
server whose content directory contains it (longest-prefix among ALIVE
servers), returning that server's base URL + the ext-less route. This is the
target-aware half of the cwd-priority guardrail - a bare doc name stays
cwd-scoped, but an absolute path (a loose file or a doc in another git
worktree) resolves to the right server instead of silently serving cwd.
Implements the stress-tested precedence rule: realpath first, content-dir
(config-derived, not project-dir) longest-prefix match, liveness gating.
Discovery + per-candidate inspection are injected; createOffCwdResolverDeps
wires the production surface. 9 unit tests cover nested worktrees, content.dir
subdirs, dead-server skipping, symlinks, and no-match.
* style: biome import order on the process-scan shim
* fix: drop unused re-exports flagged by knip
The cli shim only needs the process-scan symbols cli actually consumes
(discoverLockDirs/extractOkBinaryPath/processCommand/processUsage/ProcessUsage);
findOkProcessPids and pidCwd are server-internal. Also drop a gratuitous
fsRealpath re-export from off-cwd-resolver.
* feat(server): preview_url file branch for out-of-project opening
Add a `file?` input to preview_url that opens a single markdown file which may
live outside any Open Knowledge project. It is a pre-project-config branch: it
bypasses the project-config gate (which requires a project at cwd) and resolves
the absolute path to the running single-file / worktree session whose content
directory contains it, via the off-cwd resolver. Returns that session's URL, or
running:false with an `ok open <file>` hint when nothing serves it yet.
document / folder / file are now three-way mutually exclusive; cwd is ignored on
the file branch; autoOpen defaults true (no project config to read for a loose
file - user-scoped source is a later refinement). 3 new tests cover the hit,
the no-session hint, and the exclusion.
* docs(server): document the preview_url file param for out-of-project opening
The tool description is the cross-host channel that reaches Codex/Cursor (which
don't load the global skill), so it must name the file param + the out-of-project
capability.
* feat(skill): teach the global discovery skill to open files outside a project
Add an 'Opening a file outside a project' section to the discovery (global)
skill with the per-host capability ladder (in-app browser -> preview_url file;
Claude CLI -> ok open <file>; fallback -> system browser), and fix the intro's
'discovery-only / no runtime rules' framing which the new section contradicted.
The global skill reaches Claude Desktop + Claude CLI; Codex/Cursor get the same
guidance via the preview_url tool description (separate commit), since ok init
does not project this skill to them.
* feat(cli): ephemeral session reaper - delete the temp dir on idle
An ephemeral single-file session now self-reaps sooner than a project server
(10 min vs 30) AND its idle-shutdown removes the throwaway temp projectDir.
Previously boot's idle-shutdown only destroyed the server, leaking the temp dir
whenever a browser tab closed with no Ctrl-C - the leak the off-cwd-addressing
audit flagged for agent-opened sessions that have no foreground process.
The temp-dir reap is wired into bootStartServer's idle handler (which knows the
ephemeral flag + projectDir) via withEphemeralTempDirReap, a small pure wrapper
with an injected rm for testing. The 10 min threshold is floored above a
sleep/reconnect window so a momentary disconnect mid-view does not reap a
session the user is still in. 2 unit tests cover the reap + best-effort failure.
* feat: boot-on-demand for out-of-project file opening
When preview_url({ file }) finds no running session for a loose file, it now
boots one and returns its URL - so an agent on a no-CLI host (Codex/Cursor) can
cold-open a markdown file outside any project, not just an already-open one.
The session runs in a DETACHED, headless `ok <file>` subprocess (new
OK_SINGLE_FILE_NO_OPEN flag suppresses the browser open) so it survives the MCP
process and is cleaned up by the ephemeral reaper. ensureSingleFileSession
spawns + polls discovery until the session registers, single-flighted on
realpath so concurrent opens of one file coalesce to a single spawn. Wired only
into registrations with spawn authority (the serverUrl gate); a non-existent or
non-markdown path is never spawned. 7 new tests (4 ensure + 3 file-branch).
* docs(skill): Claude Desktop pane is a project surface — loose files use ok open
Make the discovery skill's out-of-project routing definitive for Claude Code
Desktop: the preview pane only opens on the workspace's own project (it spawns
from a project launch.json), so it cannot host a file from outside that project.
A loose file routes to ok open / the OK Desktop deep-link, not the pane — the
pane stays for in-project docs. Removes the earlier 'until pane-attach ships'
hedge; off-cwd pane-attach is a non-goal (a non-project file does not belong in
a project-bound surface).
* fix: address PR review + CI loopback-discipline failure
- Test fix (CI red): off-cwd-resolver.test.ts used interpolated
`http://localhost:${port}`, which the loopback-bind-discipline meta-test
forbids in server test sources (localhost resolves ::1-first); switch the
mock URLs to 127.0.0.1. Only the full `bun run test` runs that meta-test, not
the local check:fast, so it slipped the pre-push.
- argv guard (MAJOR): ensureSingleFileSession's production spawn no longer
spawns with an empty process.argv[1] (embedded runtime) — it logs + skips,
so a no-session result is attributable instead of an invisible 8s dead end.
- reaper try/finally (MAJOR): withEphemeralTempDirReap now reaps the temp dir
even if the inner idle handler throws, and logs unexpected (non-ENOENT) rm
failures instead of swallowing them.
- ensureSingleFileSession failure (MAJOR): the file-branch .catch now logs the
unexpected rejection instead of silently treating it as "no session".
- Rename __resetEnsureSingleFileInflight → ...ForTests (convention); fix a
stale docstring; add a test that different files spawn independently.
* test: cover the reaper finally-guarantee and ensure-rejection fallback
Address two follow-up review 'Consider' comments by testing the exact behavior
the prior MAJOR fixes added: (1) withEphemeralTempDirReap reaps the temp dir
even when the inner handler throws (the try/finally); (2) a rejected
ensureSingleFileSession falls back to running:false + the ok-open hint rather
than surfacing a tool error.
* fix: reject a relative file path in preview_url (must be absolute)
A relative `file` passed Zod and silently resolved against the MCP process cwd
inside resolveOffCwdTarget, opening the wrong file. Reject non-absolute paths
with a clear error (a loose file has no cwd to anchor a relative path).
Addresses a review Consider; adds a test.
* chore: address carry-over PR review pending recommendations
Work through the reviewer's 7 follow-up recommendations:
- Perf: boot-on-demand poll interval 150ms -> 500ms, cutting the per-tick
discoverLockDirs process-scan churn ~3x over the boot window (cold-start only;
detection still well within a multi-second boot).
- Observability: the off-cwd resolver now logs a candidate dropped for an
unreadable config instead of silently returning null.
- Layering: drop findOkProcessPids/pidCwd from the server index export — they
are server-internal (only the co-located test uses them), matching the CLI
shim's stated intent.
- UX: the no-session hint is now capability-neutral (ok open on a terminal host,
else OK Desktop) rather than assuming ok open everywhere.
- Test: cover the boot-true-but-not-discoverable path (falls back to the hint).
Deliberately deferred (documented, non-blocking): autoOpen for loose files is
spec Q18 (user-scoped config) follow-up; the file/document/folder param names
are a settled 1-way-door contract that reads clearly and mirrors `ok open`.
* feat: respect user-scoped autoOpen for out-of-project file preview
Pending recommendation #6 (Q18): the `file` branch hardcoded `autoOpen: true`,
overriding the user's `appearance.preview.autoOpen` preference for loose files.
A loose file has no project config, so read the USER-scoped config
(~/.ok/config.yml) via resolveConfigPath('user'); readConfigSafely applies
schema defaults (autoOpen defaults true) and never throws, so a missing file
yields true. Injectable via resolveUserAutoOpen for tests; added a test that a
user autoOpen=false is honored.
The remaining pending rec (file/document/folder parameter naming) is a
deliberate keep: `file` mirrors `ok open <file>`, forms a clean target triad
with document/folder, and the absolute-outside-project constraint is already
explicit in the schema description + enforced by the isAbsolute guard.
* test: declare resolveUserAutoOpen on the test EnsureDeps interface
Consistency with the sibling injected deps (offCwdResolverDeps,
ensureSingleFileSession) — the autoOpen=false test relied on the runtime spread
passing it through rather than a typed contract.
* docs(skill): never ok init to view a file; prefer in-app browser
Rewrite the discovery skill's out-of-project open section to fix the two
behaviors that misfired in real agent runs:
- Broaden the framing beyond 'loose file' to explicitly include a file inside a
regular repo/folder that was never ok-init'd, and state outright that opening
a file never requires ok init (it opens in a throwaway temp session; the repo
is never touched). This kills the 'this repo isn't OK, so I must init' instinct.
- Lead with: prefer your in-app/built-in browser whenever you have one. Collapse
the routing to the real fork — have the preview_url MCP tool (find/boot, then
open the url in the in-app browser) vs no OK MCP tools (ok open <file>, or the
npx fallback) — and note the Claude pane is project-only for these files.
* docs(skill): make ok open (Desktop) vs preview_url (browser) explicit
A real Codex run flailed on 'open it in the browser here': ok open had put the
file in the OK Desktop app, and to move it into the embedded browser the agent
hunted ok ps / status / ui / start and guessed ports, then hand-spawned ok mcp
over raw JSON-RPC — the user had to say 'use the mcp to get the url' repeatedly.
Make the surface choice unmistakable in both the discovery skill and the
preview_url tool description:
- ok open opens the Desktop app and PREFERS Desktop when installed; preview_url
is the only way to force a browser, and the source of the URL.
- Route by surface: embedded browser -> preview_url({file}) -> navigate in-app
browser; desktop -> ok open; Claude pane is project-only.
- Explicit anti-pattern: do not hunt the URL via ps/status/ui/start/ports or
reconstruct preview_url by hand.
* fix: boot-on-demand must keep ELECTRON_RUN_AS_NODE in the packaged app
preview_url({file}) is supposed to spin up the single-file session when none is
running, so the first 'open this file' returns a URL the agent opens in its
in-app browser. In the packaged app it never did: the detached boot spawned
process.execPath (the Electron binary) with the CLI entry but DELETED
ELECTRON_RUN_AS_NODE — so the binary launched the Electron GUI with [cli.mjs,
file] as argv instead of running the CLI headless. No session ever registered,
preview_url returned the no-session hint, and the agent fell back to ok open
(Desktop). The scrub is correct for the ok open LaunchServices path, not for a
re-exec-as-Node spawn; ok start / ok ui / the mcp shim all keep the var.
Inherit ELECTRON_RUN_AS_NODE (the bundle launcher sets it to 1) instead of
stripping it, and raise the cold-boot poll timeout 8s -> 15s so the first
preview_url call returns the URL rather than timing out into the Desktop hint.
* refactor: drop dead off-cwd-resolver re-exports from server index
Nothing imports the off-cwd resolver via the package root — every consumer
(get-preview-url, ensure-single-file-session, tests) uses the relative path.
knip can't flag it because re-exports from a package's main entry count as
public API. Speculative surface; re-add if an external consumer appears. The
process-scan re-export block stays — the cli shim imports those from the root.
* docs(skill): open-in-browser only when the host has one; pane is project-only
Refine the out-of-project open routing into a capability decision:
- in-app/built-in browser (Cursor, Codex) is the default — call preview_url then
IMMEDIATELY navigate that browser ('open it' means navigate, not print the URL
and stop), so a plain 'open X in openknowledge' lands in-app in one step.
- only open a browser when you actually have one; never pop a tab on a host
without one.
- Claude Code Desktop preview pane is project-only — it can't host an
out-of-project file, so route those to ok open (Desktop), not the pane.
- no browser + no pane (pure stdio) -> ok open.
* refactor: set ELECTRON_RUN_AS_NODE explicitly in boot-on-demand spawn
Match the sibling re-exec sites (ok start / ok ui / mcp shim) which set it
explicitly rather than relying on implicit inheritance from process.env.
Behavior-equivalent in the packaged app (the launcher already sets it) but more
robust and consistent. Addresses PR review consider.
* polish: address PR pending recommendations (contract, logging, test)
Work the carry-over pending recs from PR #2061 review:
- ensure-single-file-session: guard isServing() with .catch(()=>false) so the
'never throws for the wait path' JSDoc is actually true (off-cwd discovery can
reject); callers no longer rely on masking it.
- get-preview-url readUserAutoOpen: log the unexpected-exception catch (like the
sibling isServerLive) instead of silently defaulting.
- off-cwd-resolver test: add the missing edge — a live shorter-prefix match wins
over a dead longer-prefix match (liveness gates before longest-prefix).
Not changed (deliberate): readUiLock/isProcessAlive are both defensive (return
null / bool, never throw) so the off-cwd observability rec is moot; the
file/document/folder/skill param naming stays per the earlier decision.
GitOrigin-RevId: b833c641d7a347528f33c620de3ffccd6094192b
…d (#2231) * [wip] claim work on fix-ok-351-terminal-reload-state-loss Claiming issue #351 (in-app terminal dock loses its tabs/sessions from the UI after an Electron renderer reload). Diagnosis complete; RED test + fix to follow. Scoped to the deterministic terminal-dock reload-survival invariant; the occasional editor-tab race is tracked as a separate follow-up. * test(open-knowledge): RED tests for terminal-dock reload survival (#351) Failing tests pinning the intended fixed behavior for issue #351 invariant A (terminal-dock reload survival). No production code changed; the fix lands separately. - terminal-reload-survival.test.ts (main): the manager exposes a per-window live-session enumerator so a reloaded renderer can rediscover surviving shells - TerminalDock.reload-survival.dom.test.tsx (renderer): the dock recovers a tab per surviving session instead of seeding a single fresh one - terminal-dock.e2e.ts (desktop-only smoke): a real renderer reload preserves the open terminal and its live session * test(open-knowledge): RED tests for adoptSession reload edge-correctness (#351) * fix(open-knowledge): restore terminal dock sessions across renderer reload (#351) A renderer reload (View > Reload, or the window reload macOS sleep/wake forces) tore down the dock's session list while the per-window PTY host and its live shells kept running orphaned in the main process. The dock had no way to reach them, so it remounted empty and collapsed and the running agents became unreachable from the UI. Add a per-window live-session inventory the reloaded renderer rehydrates from: - Producer (main): TerminalManager.listSessions enumerates the live ptyIds for a window and adoptSession rebinds a surviving session to the reloaded renderer (refresh its webContents, clear the backpressure the dead page stranded, resume the host), or refuses an unknown-session so the panel spawns fresh. Exposed over ok:pty:list / ok:pty:adopt. Dock visibility is retained per window and read back over ok:terminal:dock-state, written from the existing view-menu push, and cleared on window close and app quit. - Consumer (renderer): on mount the dock queries the inventory and rebuilds one tab per surviving session, each adopting its ptyId rather than spawning a fresh shell, so the running program and its live IO survive. A capability branch and a settle flag keep a transient visibility flip from seeding a shell the adopted set would replace. Cold start is preserved: zero survivors seeds exactly one fresh session. EditorPane restores the dock's expanded state from main so a reloaded window reopens the dock it had open. Adoption reconnects the surviving shell instead of respawning, so a fresh create never orphans the old PTY and the env of the running process is preserved. Scrollback replay is out of scope: the session and its live IO are restored, not historical output. Bumps the IPC channel-count ratchet from 71 to 74 for the three new channels (each with a documented could-not-fold rationale) and updates the header commitment in lock-step. Mirrors the new bridge methods and types across the three drift-tested bridge copies and the two terminal bridge mocks. * fix(open-knowledge): re-expand the terminal dock after reload The dock-visibility restore was inert: on a renderer reload, EditorPane's mount-time view-menu push fired the initial terminalVisible:false over the same FIFO IPC channel before the restore effect read main's retained per-window visibility, so getDockState always returned false and the dock came back collapsed (the session survived but the panel did not re-expand). Gate the view-menu push on a dockRestoreSettled latch (mirroring TerminalDock's rehydrationSettled) so the restore reads the retained value before the mount-initial false can overwrite it; the first push after settling carries the restored visibility. Mark the restore-driven reveal so it is not counted as a user-initiated open in the adoption telemetry. Add a headless test modeling main's per-window visibility map (push writes, getDockState reads) that pins the re-expand-after-reload behavior the prior hardcoded stub could not cover. Log a breadcrumb on a list() failure so a dropped session inventory is distinguishable from a real cold start. * chore(open-knowledge): add changeset for terminal dock reload survival * fix(open-knowledge): harden terminal reload-adopt path per review Address cloud review robustness findings on the reload-survival fix: - adoptSession guards its resume postMessage against utility-process exit (the same list-to-adopt TOCTOU window the session-presence check covers): a dead host now refuses the adopt so the panel spawns fresh, mirroring safeKillUtility, instead of throwing into the IPC handler. - EditorPane's dock-state restore capability-guards getDockState (like TerminalDock's list()) and logs a breadcrumb on failure instead of swallowing it. - TerminalDock resets its rehydrate run-once guard in the effect cleanup so rehydration survives React StrictMode's dev double-mount. Add headless coverage for the degrade-gracefully branches: zero-survivors and a rejecting list() both still settle so a later dock-open cold-starts. * fix(open-knowledge): null-safe terminal bridge access + log adopt refusal Two genuine CI failures from the reload-survival change: - A session-only desktop bridge (no terminal surface) crashed the editor pane on mount: EditorPane's getDockState restore and TerminalDock's list() capability check both did bridge.terminal.X, which throws when terminal is absent (the editor-tab-restore E2E installs exactly such a bridge). Optional- chain terminal at all three mount-time sites so a terminal-less bridge degrades to no-restore / no-rehydrate instead of crashing the column. - The ok:pty:adopt handler's no-window { ok: false } return was not paired with a logIpcError call, failing the IPC-log-coverage meta-test. Add the paired logIpcError like the sibling ok:pty:create handler. Verified: the editor-tab-restore E2E (file-tree-create.e2e.ts) and the ipc-log-coverage meta-test both pass; the reload-survival + cold-start DOM suites stay green. * chore(open-knowledge): include windowId+ptyId in adopt-resume warn log * test: pin adopt edge paths and dock-restore rejection for terminal reload survival Close fast-tier coverage gaps for paths the reload-survival fix introduced that were previously pinned only by the CI-quarantined desktop E2E: - terminal-manager adoptSession: the resume-postMessage TOCTOU catch (refuse and warn on a non-ESRCH code, refuse silently on the expected ESRCH), plus cross-window ptyId isolation (a sibling window's ptyId is refused). - TerminalPanel: the renderer adopt-vs-create fallback across all three branches (adopt ok attaches and resizes, refused falls through to create, throws is caught and falls through to create). - EditorPane: a rejecting getDockState still settles the dock-restore gate so the deferred view-menu push converges. Also document the shared-per-window webContents invariant on adoptSession (the first surviving tab's adopt rebinds delivery for all its siblings). The only non-test change is that comment; all other additions are tests. * test: assert resize gate and cancelled-adopt no-kill invariant Strengthen the TerminalPanel reload-rehydration tests per re-review: - The refused and thrown adopt cases now assert resize() is never called. resize is gated behind 'if (adopted.ok)', so a dead ptyId must never be resized; a refactor moving it before the check would regress silently otherwise. - New test: an unmount mid-adopt must not kill the surviving session. This pins the deliberate asymmetry with cancelled create (which reaps the orphan it just made) and guards against a React StrictMode double-mount killing live shells. Test-only. * fix(open-knowledge): replay buffered terminal output on reload adopt The docked terminal survived a renderer reload by re-adopting the live PTY, but the reattached tab came back blank: the main process kept no screen history, only the in-flight flush buffer that is cleared on every flush. So a reconnected shell showed nothing until its next output or keystroke. Add a capped per-session replay ring in the terminal manager, fed from the same host-output path, and return it from adoptSession. The renderer writes it into the fresh xterm before wiring live delivery, so the prior screen and scrollback repaint instead of showing blank, then live output resumes. The cap is injectable (replayCapBytes, default 256 KiB) to match the existing water-mark tunables. Completes the reload-survival work for issue #351. * fix(open-knowledge): clear stale outbound on terminal adopt to avoid duplicate output Output produced while the renderer was dead piled up in the session's coalescing outbound buffer (flush bails on the destroyed webContents without clearing it) and also in the replay ring. On adopt we rebound webContents and returned replay, but left outbound and its pending flush timer intact, so a post-adopt flush re-delivered those bytes live after the renderer had already painted them via replay: duplicate scrollback, or a corrupted screen for full-screen TUIs. Clear the coalescing buffer and cancel its pending flush on adopt, mirroring the exit / onUtilityExit cleanup. No data is lost: the cleared bytes are already in replay; bytes arriving after adopt enter outbound and replay fresh. * test(open-knowledge): cover the renderer replay-write path on terminal adopt Adds a component-tier test asserting TerminalPanel writes the adopted session's replay into xterm, closing the one tier gap between the main-process test (replay produced) and the renderer (replay consumed). The makeBridge adopt mock now carries replay, matching the real OkPtyAdoptResult shape. --------- GitOrigin-RevId: 80b3d72575033b5e06faba3f43cee0ee865ef5f1
![inkeep-internal-ci[bot]](https://avatars.githubusercontent.com/u/123133689?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Automated approval from agents-private public-mirror-sync (run: https://github.com/inkeep/agents-private/actions/runs/28402313696). Source of truth is the monorepo; direct edits on inkeep/open-knowledge are overwritten on next sync.
|
|
Contributor
Author
|
Auto-closed by public-mirror-sync: a newer sync run is rebuilding copybara/sync with accumulated changes from agents-private. A fresh PR will be opened momentarily. |
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
5 participants
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.
Desktop: enable the openknowledge.ai update proxy for the stable (
latest) channel too. Stable builds now fetch updates throughopenknowledge.ai/updates/stable, which 302s to the byte-identical GitHub asset (preserving the manifest sha512 and the macOS signature) so stable updates are counted per version. This follows the verified end-to-end beta update (beta.13 to beta.14) through the proxy. The stable path resolves via GitHub's authoritativereleases/latestalias, and a feed failure still reverts to the GitHub provider for the session, so auto-update reliability never drops below GitHub-direct.Align the macOS File menu with the in-app project switcher. The File menu's project actions now sit together directly under "New from template…" and read in the same order as the bottom-left switcher: Recent project, New project, Switch project, Open folder. "Create new project…" is renamed "New project…" and the recents submenu is renamed "Recent project" to match the switcher's labels. The switcher's own action order is updated to the same sequence (New project, Switch project, Open folder) so both surfaces are consistent. Wiring, accelerators, and behavior are unchanged.
Fix
links({ kind: "dead" })falsely reporting freshly-written docs as dead. The dead-link check decided a target existed only from the file-watcher's file index, which lags behind in-session writes, so a doc the link graph had just registered a backlink for could still be flagged dead until a server restart. Dead-link resolution now also treats any doc the graph already holds as a live node (its body has been indexed) as a valid target, so a newly-written doc is a valid link target immediately — without changing how genuinely-missing targets are reported.Docked terminals now survive a renderer reload. Reloading the editor window previously collapsed the terminal dock and discarded the running shell; the dock now comes back expanded with the same live session reconnected, its running program intact, and its prior on-screen output and scrollback repainted, without re-opening it. A fresh app launch still starts with the dock hidden, and quitting or restarting the app still spawns a fresh shell.