Skip to content

Tags: inkeep/open-knowledge

Tags

v0.52.0-beta.6

Toggle v0.52.0-beta.6's commit message
Add appliesTo folder picker (PRD-7648) (#3401)

* feat(ok): pick appliesTo folders from a checkbox tree + live match count (PRD-7648)

Frontmatter schema rows gain a folder picker that authors folder/**
globs from the project's folder tree (per-folder doc counts, covered
descendants read indeterminate), keeping the raw pill input as the
untouched escape hatch. The summary line now reports 'Matches N of M
docs right now' live, so a bare folder name like 'blog' reads 0-of-N
the moment it's typed instead of only after the server's zero-match
warning.

* refactor(ok): folder picker as house-style cmdk combobox; okignore-style count

Replace the bespoke checkbox-tree popover with the NewItemDialog combobox
anatomy: full-width trigger summarizing the selection, trigger-width
popover, cmdk search over folder paths, Check-marked multi-select rows
that toggle without closing. Inherits the popover-inside-Dialog scroll
workaround (radix-ui/primitives#1159) and the cmdk-input focus routing
the bespoke popover was missing — wheel scroll inside the folder list
was dead inside Settings. The match-count line drops amber for the
.okignore preview conventions (polite status, de-emphasized zero) and
teaches the /** fix only when a bare folder name is actually authored.

* test(ok): e2e for the glob picker's composed write path + scroll pin (PRD-7648)

Substrate-adequacy pass: the DOM tier mocks both system boundaries
(config binding, page list), so it proves what the UI sends, not that
the system accepts it. New Playwright spec drives the real chain —
pick a folder -> CRDT patch -> config.yml bytes on disk -> server lint
compose -> validation badges scoped to the picked folder — plus the
live match count against the real page list, and a wheel-scroll pin
for the react-remove-scroll popover-in-dialog class jsdom is blind to
(the regression that shipped in the first picker iteration). Added to
the CI e2e subset.

* fix(ok): satisfy new lint rules post-merge (Trans-wrapped plural forms, ms-2)

Main grew two GritQL rules since the branch point: user-facing JSX text
must sit inside Trans (the Plural forms now wrap their sentences), and
physical direction utilities are banned (ml-2 -> ms-2 on the trigger
chevron). Rendering is unchanged — the count-line dom assertions and
the e2e pass as-is.

* i18n(ok): translate the folder picker's 11 strings across all ten locales

The picker-completeness gate requires every picker-promoted locale at
100%: the agent that writes a string writes its ten translations in the
same change (locales/REVIEW.md). Terminology follows GLOSSARY.md and the
existing catalog (esquema/schéma/架构/結構描述 for schema; patrón/motif/
模式 for pattern); plural forms carry each locale's own category set
(other-only for zh/id, one+many+other for es/fr/pt-BR, the full six for
ar). Machine-translated, plainest-literal renderings per
translate-ui-strings; no glossary additions needed.

* fixup! local-review: baseline (pre-review state)

* chore: drop stray baseline artifacts and unrelated MCP config churn

The local-review baseline snapshot swept five working-tree files into
the branch: a generated browser-skill scratch script, a pico editor
backup inside public/open-knowledge, and an ok-mcp-v1 to v2 rewrite of
the three harness MCP configs. None of them belong to PRD-7648. Remove
the scratch files, restore the configs to main, and ignore both scratch
patterns so they cannot be committed again.

* test(ok): picker remove-path e2e + coverage gaps (PRD-7648)

Review follow-ups for the folder picker:

- e2e: unchecking a picked folder now proves the full remove chain,
  CRDT patch to persistence to config.yml, and pins that emptying the
  list drops the appliesTo key instead of writing an empty array.
- unit: countMatchingDocs with two additive folder patterns pins union
  semantics. dom: a disabled trigger neither opens nor authors globs,
  and a no-hit search shows the empty message.
- docs: the settings-editor paragraph now names the Pick folders button
  and the live match count.
- selectedFolders returns ReadonlySet, matching coveredByAncestor.
- trim design-lineage narration from two comments per the comment
  discipline.
- worktree-service tests: accept both git wordings for an unresolvable
  base ref (Apple Git 2.39 says not a valid object name, newer gits say
  invalid reference), so the suite passes on stock macOS git.

* fix(ok): address folder picker review findings (PRD-7648)

* chore(ok): keep PRD-7648 changes scoped

* chore(ok): record folder picker bundle budget (PRD-7648)

* test(ok): harden folder glob boundaries (PRD-7648)

---------

GitOrigin-RevId: e191b4467d9f490d1d01da1d705e979897348657

v0.52.0-beta.5

Toggle v0.52.0-beta.5's commit message
feat(ok): remove ok ui command and sibling UI spawn machinery (#3421)

* feat(ok): remove ok ui command and sibling UI spawn machinery

Wave 5 removal 1. Plain ok start has served the editor, API, and MCP on
one port since the single-listener flip, and Desktop attaches via
server.lock v2, so the two-process sibling model retires:

- drop the ok ui Commander command; ui.ts survives only as the engine
  behind ok start --only ui --server-url, which now prints a deprecation
  notice (it was shipped without one, so it gets a notice release before
  its own removal alongside ui.lock)
- strip the start.ts sibling machinery: --ui-port, spawnOkUi,
  decideUiSpawn, awaitUiSiblingPort, connectUiSibling, the idle-shutdown
  sibling teardown, and the banner's resolvedUiPort plumbing
- bootServer loses attachUiSibling/spawnUiSiblingFn; ui.lock writing is
  untouched (retires in the follow-up wave)
- the clone/local-op flow resolves its redirect from server.lock
  (url + ui capability) instead of polling ui.lock and spawning ok ui;
  a live --only server holder is now an explicit no-UI error
- operator/agent hints that named ok ui (404 bodies, MCP preview
  messages, ok open, removed-keys redirect, app connect-banner strings
  in all locales, skill assets, docs) now point at ok start

* feat(ok): add ok ui removed-command tombstone

ok ui previously fell through to the default command's arity error
(too many arguments), which tells someone with the old command in
muscle memory or a stale script nothing actionable. Register a hidden
stub that prints the removal pointer (run ok start) and exits 1.
Accepts any flags/args so ok ui --port 39847 lands on the same message.
Drop at the removed-command horizon.

* fix(ok): address ok-ui removal review findings

- perf-prod.sh: single-listener model. One ok start serves the shell,
  API, and collab; Playwright targets the server port; the script also
  polled the pre-local/ lock path, so fix that to .ok/local/server.lock.
- clone redirect: restore the ui.lock fallback resolveUiInfo still
  honors. New resolveUiRedirectPort (ui-redirect-port.ts, unit-tested)
  runs the same two-source chain, so a live --only server + --only ui
  split pair resolves to the sibling port instead of a false no-ui
  error. Error copy now also names the missing-bundle cause, which
  writes the same no-ui capability set.
- --only ui deprecation notice moves above the refusals (mirrors the
  --remote ordering) and gains a test; the notice-free refusal path
  previously made it untestable.
- resolveStartShellDir: drop the dead remoteEnabled parameter left
  behind by the uiPortSet removal; delete the duplicate test whose
  title referenced the removed opt-out.
- stale comments: TOCTOU exited:true reference, desktop-attach-contract
  reader list, awaitUiBaseUrl sibling wording, mcp-mount shell note.
- docs: what-open-knowledge-writes lock rows no longer present the
  removed --ui-port as a live path; subtree README drops the ok ui
  command row and auto-spawn claims.

* chore(ok): review-pass housekeeping on the ok-ui removal

- README: drop the leftover cli.mjs ui example line next to the block
  the previous commit updated
- perf-prod.sh: remove the dead run_start_ts variable and fix the
  results-selection comment to describe the actual N-most-recent pick
- ui-redirect-port: rewrite the step-1 guard via De Morgan so the two
  admit cases (absent capabilities = optimistic, explicit ui entry)
  read directly, and pin the draining branches with two new tests
  (draining no-ui server.lock resolves null, not no-ui; a draining
  ui.lock is never a redirect target)

GitOrigin-RevId: 95bb30b4e8e2aeadaeaac6cdbcd9971860b472bf

v0.52.0-beta.4

Toggle v0.52.0-beta.4's commit message
chore(ok): skills cleanup deferred from the marketplace PR (PRD-7707)…

… (#3323)

* chore(ok): skills cleanup deferred from the marketplace PR (PRD-7707)

Six deferred items from PR #2923.

1. Fix a `use-skills` staleness bug. `fetchSkillsShared` coalesced on a bare
   promise, so a refetch triggered by a mutation joined the request that was
   already in flight before that write. The list settled on pre-write data and
   never self-healed. Requests now carry the generation they started at, every
   mutation signal bumps that generation synchronously, and a request from an
   older generation is never joined. The `.finally` reset is identity-guarded so
   a superseded request cannot clear the newer one that replaced it.

2. Hoist `.agents/skills` to `AGENTS_SKILLS_ROOT` in core. It was a bare literal
   at 17 code sites across core, server and app with no shared constant, so
   moving the hub meant finding every one of them by hand.

3. Trim both AGENTS.md files back under their caps. The Open Knowledge one was
   at 39,980 of 40,000 bytes, so the next ordinary edit would have been blocked
   by the pre-commit size guard; the root one was at 38,447 against its 38,500
   test limit. Now 37,584 and 36,715. Only prose that the linked canonical docs
   already carry was cut; every rule, pointer and STOP/WARN entry is intact.

4. Give the skills mirror real leak detection. `check-skills-mirror-faithful`
   already runs the real Copybara jar, but it only proved the emulator and the
   jar agree, which says nothing about whether what they both ship is safe to
   publish. It now runs the same apparatus and content-leak detectors the docs
   mirror uses against the materialized tree, and asserts the root furniture
   (README, LICENSE, marketplace manifest, authoring template) survives. That
   last arm is the one the emulator diff structurally cannot cover: an include
   narrowing drops a file from both sides at once and reads as clean.

   The ticket asked to parameterize `check-mirror-export-faithful` over both
   manifests instead. That was written before the skills gate existed, and doing
   it now would run the jar over the skills export twice, with a config path
   that skips the derived skill moves.

5. Extract the install-menu row derivation to `lib/skill-install-rows.ts`, with
   tests. The component keeps its JSX untouched.

6. Extract the sidebar path builder to `lib/skills-tree-paths.ts`, with tests.
   `EMPTY_SCOPE_SENTINEL` moves to the builder that emits it and is imported
   back by `SkillsTree` for its CSS.

Verification: OK lint, typecheck and knip clean; core 6501, server 7063 and app
integration 1529 tests pass; root `test:scripts` 582 pass, plus boundaries,
hook-sync, cross-harness-config, monorepo-traps and mirror-gate-reach. The
app `test:dom` suite fails 21 files / 341 tests, byte-identically on the clean
base: the known local-only Node 26 `localStorage` cluster, green on CI Node 24.

* chore(ok): add changeset for the skills-list staleness fix (PRD-7707)

* fix(ok): use a literal for the .agents adoption marker (PRD-7707)

The hub-adoption check asks a different question from the hub's install
path: "does this project use `.agents/` at all?" versus "where does the
hub live?". Deriving the first from the second by splitting
AGENTS_SKILLS_ROOT coupled them, so moving the hub to a nested path would
have silently broadened the adoption check to that path's first segment.

Back to the bare literal the pre-extraction code used, matching the other
reader of the same concept in AgentIconCluster.

* test(ok): cover the CC1 remote branch and the detected-active path (PRD-7707)

Two coverage gaps the review flagged, both in code this PR made testable.

The staleness fix invalidates from two subscriptions, the local skills bus
and the cross-client CC1 `files` signal, but only the local one was
exercised. Dropping the remote call or breaking its channel filter would
have regressed cross-window mutations with the suite still green. Both
branches now run the same case, plus one pinning that an unrelated channel
does NOT invalidate, so a backlink recompute cannot spin a full skills scan.

That second case forced the file to stop leaking module state between
tests. `useSkills` holds its list, in-flight request and generation at
module scope and vitest resets the registry per file, not per test, so a
second test would have mounted already-ready on the first one's data and
its waitFor would have passed before any fetch resolved. Each test now
re-imports a fresh module graph and asserts the loading state first, so
the reset itself is pinned.

Also covers the detected-skill active path in the tree builder, including
that an active managed skill still wins the highlight.

Verified non-vacuous: both staleness cases fail against the pre-fix hook,
and breaking the detected-active branch fails the new test.

* docs(ok): document error-path correctness on the in-flight guard (PRD-7707)

The guard is load-bearing on the rejection path too: .finally releases the
slot for a failed request so later callers do not join a promise that can
only re-reject, and the identity check is what stops a superseded request
settling late from nulling out the live one that replaced it. Names the
bare .catch reset a maintainer would reach for, since that runs for the
same superseded requests without the check.

* fix(ok): restore single-flight and complete the mirror survivor list (PRD-7707)

Self-review follow-up: two defects this PR introduced, one per file pair.

1. use-skills: the generation counter fixed the staleness this PR set out
to fix, but paid for it by dropping the guarantee the code it replaced
provided, and wired invalidation in the wrong place.

/api/skills is a synchronous walk of every editor skills root, and the old
`??=` meant N surfaces reacting to one mutation shared one scan. The
generation check made every mid-flight invalidation start ANOTHER
concurrent scan, so a multi-editor fan-out install could stack several full
walks on the server's event loop and stall CRDT sync. The shared request
now RE-RUNS instead of racing: one scan at a time, and if a write lands
while it is running its response is discarded and the fetch goes again.
Callers still share one promise, and that promise can now only resolve with
post-write data, which is why the shared lastKnownSkills seed no longer
needs a staleness check of its own; it was previously seeded by whatever
resolved, including a pre-write response landing inside the debounce window.

The retry is bounded, since it is driven by external writes and would
otherwise spin under a sustained mutation stream.

Invalidation moved to module scope. It was wired inside the per-instance
subscriber, so a write landing while nothing was mounted never invalidated,
and a slow request outliving the last unmount is exactly the case this
exists for. The next mount would join it and render a deleted skill.

Drops the counter pair: with the identity guard on the reset, the two
module-level mutables only fed one equality test a plain stale flag
expresses directly.

2. skills mirror gate: the survivor list was hand-copied from an export
listing and covered four of the seven furniture files the manifest names,
so the both-sides-drop hole it was added to close stayed open for the rest,
including the PR-bridge workflow whose silent loss strands external
contributor PRs rather than failing anything.

The missing direction was the test, not the list: it checked
entries -> assertions and never furniture-on-disk -> entries, which is why
an incomplete list passed. A test now walks the authored tree and fails on
anything unnamed.

Also scopes the content scan. findContentLeaks was written for a source
tree where a leading # or * means a comment; this export is 47 of 52
markdown, where they mean a heading and a bullet, so that arm would have
hard-failed a public contributor's PR for writing an internal tool name in
a heading with no way to suppress it. Dropped for markdown only; the arms
that apply to prose still run everywhere and non-markdown furniture keeps
the full check.

The DOM tests now assert the concurrency ceiling alongside freshness, since
the ceiling is what regressed. Both still fail against the pre-PR hook.

* fix(ok): do not require a DOM to import use-skills (PRD-7707)

Regression from the previous commit and my own fault for how I verified it.
Moving invalidation to module scope wired the listeners at module LOAD, and
subscribeToSkillsChanged calls window.addEventListener, so merely importing
this module needed a DOM. Every node-env test that transitively pulls it in
died at load with "window is not defined" - use-skill-origin,
use-skill-directory, use-reconcile-skill-tabs, FileSidebar and the command
palette suites all reported "0 test". SSR would have hit the same wall.

The listeners now attach on first use behind a typeof-window guard. That
keeps what the move was for: once attached they outlive every unmount, so a
write landing while nothing is mounted still invalidates, which is the
window the stale-join bug lives in. Nothing is cached before the first
fetch, so attaching there loses no coverage.

Adds a node-environment test that imports the module and asserts no DOM is
required, since that is the contract that broke and it is invisible from
the dom-config suite where window always exists.

I had only run the dom config and the lib tests before pushing; the whole
class was one `pnpm test` away. Full turbo test now green across all 13
tasks (core 279, server 448, app 492, desktop 194, cli 128, conformance
115 files).

* fix(ok): redirect a preview into the scope the skill landed at (PRD-7707)

Installing from the skills browser could leave two tabs open and strand the
new one on "Couldn't load document" until the sync timed out.

One cause behind both halves. After import the preview tab redirects into
the real skill via useOpenSkill, which resolves a skill's doc by (scope,
name). The redirect passed the LIVE `scope` selector rather than the scope
the bundle actually landed at, and the hook tracked the real one in
importedScopeRef but never exposed it. When the two disagree the entry
lookup misses, so the replaceActive swap never happens: the preview tab
survives and a second tab appears later, while the doc that did get opened
belongs to a scope the skill is not in and never resolves.

Observed with a skill that imported at global scope while the tab opened
".claude/skills/<name>/SKILL", a project-relative path that did not exist -
the bundle was at ~/.agents/skills/<name>/SKILL.md the whole time.

The hook now surfaces importedScope and the redirect prefers it, falling
back to the selector only for the bulk-install path, which imports at the
current scope and reports none of its own.

* fix(ok): wait for the skill to be resolvable before leaving the preview (PRD-7707)

Installing from the skills browser left you on the preview page even though
the skill had landed.

The redirect fired the moment the import returned, but useOpenSkill resolves
a skill's doc from the skills LIST, which lags a fresh import by the
skills-changed refetch. The fire-once ref is set BEFORE the open resolves,
so an early fire burns the single attempt on a lookup that cannot succeed
and the preview is stranded for good - no retry, no error, just a preview
tab for a skill you already own, with its slash-links dead and its INSTALL
menu still armed.

The redirect now waits until the skill is actually present in the list and
opens it at the scope the list reports. It stays gated on landedName, so
previewing a marketplace skill whose name already exists locally does not
bounce you out of the preview you asked for.

Tests: the per-agent install path had NO coverage - the existing suite only
exercised the bulk-plugin branch - which is why this shipped. Adds that
case plus a non-vacuity test asserting the redirect does NOT fire while the
list still lags, since firing early is the whole defect.

The bulk test needed its skills list mocked; when it failed on the first
attempt that was the test correctly reporting the new contract, not stale.

* fix(ok): close the stale preview tab after redirecting to the skill (PRD-7707)

Installing from the skills browser left two tabs: the real skill, and the
preview still advertising "Install it into your agents" for a skill you
already own.

replaceActive does not replace THIS tab. It opens with preview DISPOSITION,
so it swaps whatever currently holds the preview slot - which is not the
same thing. A skill-preview tab opened permanently therefore survives, and
the real skill lands beside it. When another surface had already opened and
focused the real skill, the swap consumed that tab instead and the preview
was orphaned outright.

The preview now captures its own tab id before opening and closes it after,
so the redirect ends with exactly one tab regardless of which tab held
focus. Closing AFTER the open means a failure to resolve can never leave
zero tabs.

* fix(ok): keep the Skills sidebar pinned across a scope move (PRD-7707)

Moving a skill between project and global dropped you into the Files tree
instead of following the skill.

Every other skill mutation pins the sidebar after it retargets - delete,
file delete, rename - but the scope move never did. A project skill's doc
is ordinary project CONTENT (.claude/skills/<name>/SKILL), so retargeting
onto it lets the surface autofollow to Files. The move now pins Skills the
same way its siblings do.

* fix(ok): repoint the real doc on a scope move, never a minted store path (PRD-7707)

A project-to-global move could leave the open tab pointing at a path the
move had just deleted, which then sat on "Couldn't load document" until the
sync timed out.

An in-place project skill is open at its real content path
(.claude/skills/<name>/SKILL). The retarget resolved its FROM doc from the
skills list and, when that lookup missed, fell back to skillLiveDocName -
the STORE shape, .ok/skills/<name>/SKILL. No tab carries that name, so the
retarget matched nothing and returned silently, stranding the tab.

The lookup misses routinely rather than rarely: beginOptimisticSkillMove
has already dropped the source row by the time this runs, and the list may
still be refetching. So the broken fallback was the common path.

It now resolves the FROM doc from the open tabs, and refuses to retarget at
all when no tab matches, rather than guessing. useOpenSkill already carries
the same never-mint-the-store-shape rule; this is the path that was missing
it. Global keeps the managed __skill__/global/<name> name, which is real
rather than minted.

Test drives the move with a deliberately not-ready list - the condition
that makes this fire - and pins that the retarget targets the in-place doc
and not .ok/skills. It fails against the previous fallback.

* fix(ok): restore the scope-move suite and cover the skip-retarget guard (PRD-7707)

CI caught a break I introduced: useMoveSkillScope now reads DocumentContext
(to pin the Skills sidebar and to find the open tab it must repoint), and
SkillScopeMoveDialog renders it inside a suite with no DocumentProvider, so
use-skill-scope-move.dom.test.tsx died with "must be used within
<DocumentProvider />". Stubbed the context there, matching how that file
already stubs its other dependencies.

Worth naming the cost: adding a context read to a shared hook makes every
consumer's test provide that context. One suite here, but that coupling is
real and this is where it showed up.

Also covers the guard the reviewer flagged. The suite exercised the case
where an open tab IS found, but not its complement - no tab matches, so
fromDoc is null and the retarget is skipped. That guard is the safety net
against the store-shaped phantom this change removed, so a regression that
dropped it or passed null through would have gone unnoticed. It now asserts
retarget is not called while the sidebar still pins to Skills, since the
move itself still succeeded.

* fix(ok): do not close a skill tab during its own scope move (PRD-7707)

Moving a skill from project to global closed its tab and dropped you on an
empty "New tab" with the sidebar falling back to Files.

The reconciler exists to close tabs for skills that no longer exist, and it
decides that by looking the skill up in the skills list. A move passes
through a window where the skill is in NEITHER scope: the source row is
hidden by beginOptimisticSkillMove and the destination row has not landed
yet. Absent at both scopes reads as deleted, so the tab is closed - the
user asked to move a skill, not to close it. The retarget that should have
repointed the tab then has nothing left to act on, which is why fixing the
retarget alone changed the symptom from a stale tab to no tab.

The optimistic-move bus already tracked the pending move but never exposed
it, so the reconciler could not tell "mid-move" from "deleted". It does
now, and holds the tab through the window; the move's own refetch retargets
it once the skill reappears at its new scope.

This is the structural fix rather than a timing one. The window's width
varies with how quickly the list settles - my earlier useSkills change
widened it, which is why this surfaced here - but any consumer that
destroys user state on a transient absence is wrong regardless of how
narrow the window gets.

Tests pin both directions from identical inputs: in-flight move holds the
tab, genuine deletion still closes it, and the retarget still fires once
the skill lands at its new scope.

* fix(ok): preserve a location's form when installing without a mode (PRD-7707)

Re-installing a skill whose locations were SYMLINKS silently rewrote them
as full copies, then reported the result as "changed outside".

The install fan-out passed input.installMode straight to
projectInPlaceSkill, which falls back to `copy` when that is undefined. So
an install that named no mode - the common case, since the menu only sends
one for an explicit choice - flattened every symlinked location into its
own copy. That duplicates bytes into git and detaches those locations from
the source, so editing the skill stops propagating to them. Nothing told
the user any of it happened.

The badge was then correct and blamed the wrong actor: the placement ledger
still recorded `link` for paths now holding real directories, so drift
detection fired on OK's own conversion.

effectiveInstallMode already exists for exactly this and documents the
intent - "a skill whose locations are all copies keeps getting copies, so a
new one matches its siblings rather than silently mixing forms".
skill-import already takes it as a dep; the install path did not. It does
now, and only the IMPLICIT default changes: convertLinks still honours an
explicit copy request, so deliberate link-to-copy conversion is untouched.

linkedHosts rides along on the canonical entry, which already comes
straight from the in-place scan that computes it.

Worth noting the earlier framing was wrong: the scope move never
re-projects at all, so it was not losing the form. The round-trip only made
the user re-install, and the install is where it was lost - which means any
re-install had this, not just one following a move.

* fix(ok): never close a skill tab while a write is in flight (PRD-7707)

Installing onto a second editor could close the skill's tab, dropping the
user on an empty "New tab" with the sidebar falling back to Files.

Same root cause as the scope-move case, wider trigger. A skill write is not
atomic on disk - projectInPlaceSkill rm's each destination before
materializing it - so a scan taken mid-write reports a skill that plainly
exists as absent. The reconciler reads absent-at-both-scopes as deleted and
closes the tab. The earlier guard only knew about optimistic MOVES, so an
ordinary install walked straight through it.

Rather than enumerate mutations, this adds the missing signal: a
write-in-flight registry in documents-events, marked by install and by
location writes (both of which already bracket their call with
setInstallingName, so the pairing is exact). Absence during a marked write
is no longer evidence of deletion.

Two design points worth keeping:

Only the DESTRUCTIVE branch is held. Retarget still follows the skill
immediately - repointing a tab is safe and reversible, and my first attempt
guarded both, which needlessly deferred the move retarget. The existing
test caught that.

pendingSkillWritesKey is folded into the dedupe signature. A guarded close
is deferred, not cancelled, so clearing the flag has to re-run the effect;
without it the list is identical either side of the write, the effect
short-circuits, and "close once it settles" becomes "never" - leaking dead
tabs. An earlier two-read confirmation approach died on exactly this and
was reverted.

Also collapses the now-subsumed move-only arm, which had been left as an
empty block with its comment lost to a formatter pass.

* fix(ok): an install must not restamp a location's existing form (PRD-7707)

Installing one more editor rewrote the form of locations the user had
already set up: adding .codex converted .agents from a copy into a symlink,
after which the placement ledger disagreed with disk and the UI reported
the user's own install as "changed outside".

The two directions were asymmetric in projectInPlaceSkill:

  link-to-canonical -> copy   required convertLinks (an EXPLICIT choice)
  same-copy         -> link   fired on any `mode === 'link'`

So an existing symlink survived an implicit install but an existing copy
did not. My earlier change made the implicit mode the skill's EFFECTIVE
form rather than a hard-coded copy, which is correct for a new location and
is what turned that latent asymmetry into a visible bug: for a skill whose
effective form was link, every existing copy got restamped.

Adds convertCopies as the mirror of convertLinks, set only from an explicit
link request. Now neither direction converts without the user asking, and
`mode` decides only what a BRAND-NEW location gets - which is what
effectiveInstallMode was always documented to be for.

Tests cover the repro directly: adding a second editor leaves an existing
copy a copy and an existing symlink a symlink, while a new location still
follows the preference, and an explicit choice still converts.

Fixture note: `same-copy` is decided by a content-hash compare, so the test
derives the real hash via parseSkillDir - an empty hash classifies an
identical copy as `different` and silently exercises the wrong branch.

* fix(ok): keep the surface on Skills when opening a skill (PRD-7707)

Opening a skill dropped the user into the Files tree. A PROJECT skill's doc
is ordinary project content (.claude/skills/<name>/SKILL), so the surface
autofollowed the target out of Skills.

Pinned in useOpenSkill, which its own docs establish as the single entry
point every open-a-skill surface must route through - sidebar click,
create, import, detected-adopt, and the post-install redirect out of a
preview tab. Pinning there covers all of them at once; the sidebar's own
handlers already did it locally, which is why a sidebar click behaved and
the install redirect did not.

* fix(ok): admit a freshly installed skill before responding (PRD-7707)

Opening a just-installed project skill hung for 30 seconds, flipped the UI
through fallback states, then worked. 30s is SYNC_TIMEOUT_MS exactly: the
document sync never resolved, timed out, and the retry succeeded.

A project skill's SKILL.md is only servable once its dir is in the
content-filter's in-place allow-list, and a dir only enters that list on
rebuildIgnorePatterns(). The skill-get handler states the trap in its own
comment - "which an API write does not otherwise trigger" - and both it and
the scope-move handler call it for exactly this reason. The install handler
never did, so the client could open a skill the server still judged
excluded and wait out the full timeout until some later rebuild admitted it.

The install path now refreshes the allow-list before responding, at each of
its four success exits, matching what its siblings already do.

Why it surfaced now rather than earlier in this branch: the redirect had to
work before it could hit this. While the post-install open was going to the
wrong scope, firing before the skill was resolvable, or being evicted by
the reconciler, it never reached the window where the doc is written but
not yet admitted. Fixing those exposed this one - each earlier fix removed a
visible consequence and left the hang underneath, which is why the symptom
kept changing shape instead of going away.

Full server suite green (457 files, 7333 tests).

* fix(ok): pin the Skills surface AFTER the open, not before (PRD-7707)

Installing a skill still dropped the user into the Files tree: my previous
commit pinned the surface BEFORE openTarget, and committing a tab re-arms
autofollow, so the pin was immediately overwritten.

SkillsSidebarSection already states the rule on its own call - 'Must run
AFTER the open - committing a new tab re-arms autofollow, and this is the
pin that overrides it.' That is exactly why a sidebar click behaved while
the shared opener did not: the sidebar pinned after, useOpenSkill pinned
before.

Ordering is now openTarget -> pin -> hash.

* fix(ok): record the old source's real form on a source move (PRD-7707)

Promoting a location to SOURCE left a "changed outside" badge on whichever
location used to be the source, and any later install cleared it.

promoteInPlaceSource recorded the old source path as mode 'link'
unconditionally, on the premise that relocating always leaves a symlink
behind. When it does not, the receipt claims a symlink while a real
directory sits there, so the next drift read flags OK's own promote as an
external edit. That matches the behaviour exactly: the badge always lands
on the PREVIOUS source, and it clears the moment an install rewrites and
re-records that location with its true form.

It now reads the form back with one lstat and records what is actually
there. No disk write changes - only what the ledger claims about one. A
receipt cannot assert a form the disk disagrees with, which is the property
that makes drift detection meaningful.

A missing path keeps the previous 'link' record rather than asserting
'copy' for something that no longer exists.

Note this does NOT explain why the old source is a real directory rather
than the symlink relocate is expected to leave - that question stands, and
if the symlink is genuinely not being written there is a second defect
underneath. This stops the ledger lying about it either way, which is what
produced the false badge.

* refactor(ok): route the sidebar skill click through the shared opener (PRD-7707)

useOpenSkill documents itself as the single entry point every open-a-skill
surface must use, naming "sidebar click" explicitly. The sidebar had
drifted: it minted the doc name itself and called openTarget directly.

So the same visible action behaved differently depending on where it was
invoked from, and every fix landed on one path only - scope resolution, the
fresh-entry fallback that refetches when the list lags an install, and
pinning the surface to Skills all applied to the redirect but not to a
click. That divergence is why these bugs kept reappearing in a slightly
different shape after each fix.

The click now delegates, preserving preview-tab behaviour by mapping
tabBehavior onto replaceActive and passing the entry's real path so the
opener can resolve a freshly installed skill without waiting on the list.

openSkillDoc stays for bundle FILES, which are ordinary content docs and
not skill identities, so they do not belong in the skill opener.

* fix(ok): hold the tab guard for the whole scope move (PRD-7707)

Moving a skill still closed its tab and dropped the user in Files, and the
relocated skill then would not open.

The guard was released too early. endOptimisticSkillMove fires as soon as
the request returns - BEFORE the retarget runs and before the list carries
the skill at its new scope. So the reconciler got a settled read with the
skill absent from both scopes and no guard set, and closed the tab. The
retarget then had nothing left to repoint, which is why the skill was
unopenable afterwards.

The move now brackets the whole operation with the write registry: marked
before the optimistic hide, released only after the retarget and the
surface pin, and on the failure path too. The optimistic row-hide keeps its
own lifetime - it is about hiding a row, not about protecting a tab, and
conflating the two is what left the gap.

Test mocks for the events bus needed the new exports; the failures were the
mocks, not the logic.

* fix(ok): a source promotion leaves the old location in its siblings' form (PRD-7707)

Changing the source left the old location as a symlink while the skill's
other locations were copies, so the menu immediately tagged it as divergent
and offered to convert it. The app warned about the state it had just
created - and guaranteed, not incidentally: a promote leaves exactly one
link, so with two or more copies it is always the minority.

relocateInPlaceCanonical hard-codes leaveLinkBehind. It never consults
effectiveInstallMode, whose contract is exactly this case: "a skill whose
locations are all copies keeps getting copies, so a new one matches its
siblings rather than silently mixing forms."

That helper is the established policy and the established pattern is that
the CALLER consults it and passes the result down - skill-import does,
and the install fan-out now does. The promote path was the remaining
caller that decided for itself. It now asks the same helper and reconciles
the left-behind location to match, using the same rm/copy/record shape the
install path already uses, so nothing new is introduced.

Deliberately not restructured: the primitive keeps its boolean and its
signature, and no new abstraction is added. The point was to stop diverging
from what is set up, not to invent a fourth way of deciding.

The fork `align` branch is the one remaining caller that passes no mode.
Left alone here - it resolves a conflict rather than relocating a source,
so it wants its own reasoning rather than being swept in.

Full server suite green (457 files, 7333 tests).

* fix(ok): a cross-scope move leaves the skill openable (PRD-7707)

After moving a skill between scopes the destination SKILL.md could not be
opened - clicking it fell back to a Files tab, and it only started working
roughly thirty seconds later once something else happened to rebuild.

handleSkillMoveScope never rebuilt the content filter. The destination
scope's skill dir is not servable until the filter's in-place allow-list
knows about it, so between the move and the next unrelated rebuild the file
the app had just been told to open did not exist as far as the server was
concerned.

This is the same defect, and the same fix, as the one already applied to
handleSkillInstall in this branch: rebuild before the success exit. The
install handler does it before all four of its exits; the move handler,
immediately above it, did it nowhere. Adding the call matches the
established placement rather than introducing anything new.

Worth naming for the next reader: the symptoms this produced were blamed on
several other things first - a duplicate skill entry, a stale tab, the
redirect logic. None of those were it. The file simply was not servable yet.

Full server suite green (457 files, 7333 tests).

* fix(ok): every skill-writing handler rebuilds the content filter (PRD-7707)

A skill dir is not servable until the content filter's in-place allow-list
knows about it, and an API write does not otherwise trigger a rebuild. Miss
the rebuild and the skill the user just created cannot be opened: the editor
falls back to a Files tab until something unrelated rebuilds, which is where
the roughly thirty second delay came from.

Nine handlers write skill dirs. Four rebuilt. Five did not: duplicate,
import, bulk import, restore, reimport. The import gap is the one users hit
most, since it backs install-from-preview.

This has now been fixed three times one caller at a time - install, then
move, and import was going to be the fourth. Patching the caller in front of
you is not a fix when the defect is that the obligation is per-caller and
nothing enforces it. So this sweeps all of them at once rather than waiting
for each to be reported.

The rebuild sits before the success exit, matching where handleSkillInstall
already puts it. For the import path it goes before the shared
respondSkillImport call, which both import entry points route through.

Still missing, and worth doing next: nothing stops a tenth handler from
shipping without the rebuild. A meta-test over the skill handlers - the same
shape as the existing route-registry coverage tests - would close that for
good. Named here so it is not rediscovered by another user report.

Full server suite green (457 files, 7333 tests); one ACP thread-socket
failure under parallel load passed in isolation and is unrelated.

* fix(ok): open a symlinked skill at the doc the index actually holds (PRD-7707)

A skill dir can be a symlink to a canonical dir elsewhere in the content tree,
which is how this repo keeps its bundles in plugins/ and links them into
.agents/skills. The document index holds one doc per inode, under the resolved
name, so the mounted name is not a page. The sidebar minted the mounted name,
the tab had nothing behind it, and the next page-list sync pruned it: the skill
flicked open and vanished, and the surface fell back to Files.

/api/skills now reports canonicalPath alongside path, and the doc-name builders
prefer it. path still answers where the bundle lives, which is what install,
reveal and host wiring reason about.

Resolved with realpath rather than the watcher's folder-alias index, which is
populated asynchronously after boot. Clicking a skill right after launch is
exactly when that index is cold, and a list that answers differently depending
on how warm the watcher is would have reintroduced the same bug on a timer.

On this KB, 17 of 18 symlinked skills now resolve to real pages.

* fix(ok): let the skills reconciler own the SKILL tab, not the page list (PRD-7707)

A scope move deletes the source bundle before its response lands, so the source
doc stops being a page mid-move. The page-list prune closed that tab, and the
retarget that runs moments later only acts on a tab that is still open: it
matched nothing, did nothing, and the moved skill could not be opened again
until the window was reloaded.

The prune now leaves the SKILL doc alone. Deletion is still handled, by
useReconcileSkillTabs, which decides against the skills list and holds off while
a write is in flight.

Scoped as narrowly as that reason allows. A references/* tab keeps riding the
page list, because an external delete (agent, MCP, another client) has no other
closer, and a live provider on a deleted file will rematerialise it on the next
keystroke. And only names the reconciler can parse are exempt: exempting a shape
it cannot read would leave a tab nothing could ever close.

* fix(ok): a scope move leaves nothing behind at the source (PRD-7707)

Two leftovers from a project to global move and back.

The move reused the uninstall path, which deliberately keeps one copy in the
.agents hub and removes only the editor projections. Correct for an uninstall,
wrong for a move: the bytes are already at the other scope, so the preserved
copy was re-detected as a second skill of the same name at the scope the user
had just moved away from.

The placement ledger is keyed by name and outlived the move, still describing
locations in their pre-move form. Coming back, the re-projection recreated them
as copies while the ledger claimed links, and the list rendered that gap as
CHANGED OUTSIDE: OK accusing another tool of a rewrite it had done itself.

The purge is content-guarded. Two same-named bundles with different bytes are
two distinct skills by design (conflictHosts), so symlinks go and a real
directory goes only when its hash matches the bundle that moved. A blanket
delete here would have destroyed a bundle its owner never touched. Roots come
from knownSkillRootsFor plus the legacy store, so a copy parked outside the
standard roots cannot survive to be re-detected.

* fix(ok): stop two module-level leaks that only a reload could clear (PRD-7707)

Both freeze the skills UI until the window is reloaded, which is why a reload
kept looking like the cure.

The scope move sets a write-pending flag that suppresses the tab reconciler for
that skill, and released it after a retarget that can throw. Leak it once and
the reconciler is disabled for the rest of the session: the tab is never
repointed and the toolbar keeps deriving its level from the stale doc name.
Releasing the flag matters more than the retarget succeeding, so the retarget is
guarded and the flag is released either way.

Every reader of the skills list joins one in-flight promise, released only in
its finally. A fetch that never settled froze the list for the session: stale
rows, a stale toolbar, and clicks minting doc names from entries that had since
moved. The request now carries an abort signal, so a stall becomes one failed
refresh that the next files signal retries. A failed refresh also keeps the last
good list rather than replacing it with an error screen.

* feat(ok): say when a skill is gitignored, and offer the one line that fixes it (PRD-7707)

OK will not index a doc the sync engine could never commit, so a gitignored
bundle is listed but never admitted. .claude/* is a common rule, which made this
read as a random bug: the row is right there, the click produces a tab with
nothing behind it, and nothing anywhere says why. The list and the document
index disagreed in silence.

Entries now carry ignored, and clicking such a skill explains the situation and
offers to add one line to .gitignore. The line is shown before anything is
written, because this edits the user's repo.

The rule names the skills directory, not the bundle: git cannot re-include a
file whose parent directory is excluded, so a per-skill negation looks right and
does nothing. After writing, the server asks the filter again and reverts if the
path is still ignored, so an ineffective rule fails loudly instead of sitting in
.gitignore looking like a fix.

Built-ins are exempt. OK ships them read-only and a repo that ignores them means
it, so offering to track one is an offer the user must not be given.

* fix(ok): a skill row click can always retry (PRD-7707)

Pierre selects a row on click, and a selected row emits no further selection
change. So if the first click is swallowed, no later click on that row can do
anything, and the only ways out are selecting a different skill or reloading -
which is exactly what people were doing.

Two ways it got swallowed. The handler lives behind a ref that starts as a
no-op and was assigned in a passive effect, while the tree remounts on every
paths change, so a click landing between paint and that effect hit the no-op;
it is a layout effect now. And the handler bailed when the clicked row was
already the active one, which made a failed open unretryable; re-dispatching is
idempotent, so that guard is gone.

A row also counts as active only when a tab is genuinely open for it. Matching
activeDocName alone was wrong twice over: it survives on the last doc tab when a
non-doc target takes over, and an open that never landed leaves it pointing at a
doc with no tab.

This is the one symptom in this batch I never captured in a log - the renderer
log showed the click dispatching and the doc resolving in 301ms - so treat it as
the best-supported explanation of the reports rather than a confirmed cure.

* test(ok): drop an `as never` that hid the shape it was asserting (PRD-7707)

Review finding. The cast disabled structural checking on the whole array, so a
change to the Pick this helper takes would compile silently here while every
sibling test in the file caught it through contextual typing.

Not academic on this branch: the entry shape gained canonicalPath, and a Pick
that omitted it was exactly the drift the reviewer is guarding against.

* test(ok): stop the admission suite's teardown racing the server's own writes (PRD-7707)

CI failure. Every assertion passed - 7204 tests green - and the suite still
failed, in afterAll:

  ENOTEMPTY: directory not empty, rmdir '.../proj-.../.ok/local/cache/main'

destroy() stops the server accepting work but does not join its background
flushes, so the recursive delete walks a directory that is repopulated under
it. Local runs pass; CI hit it on the original run and again on the retry,
which is what a slower, more contended filesystem does to this window.

Retry the unlink, and let cleanup failure not fail the suite: the assertions
have already run and the OS reaps /tmp either way.

* fix(ok): pin the surface to Skills even when the retarget throws (PRD-7707)

Review finding, and correct. The pin sat inside the guard added for the write
flag, so a throwing retarget skipped it — and a project skill's doc is ordinary
project content, which means the surface autofollows to Files. The move had
already succeeded at that point, so the user got a working move that dumped
them into the file tree.

Moved outside the guard. The test now asserts it, alongside the flag release.

* chore(ok): raise the combined-JS budget to fit the gitignored-skill dialog (PRD-7707)

  all JS chunks combined (gzipped)
  Size limit:   4700000 B
  Size:         4700364 B

Over by 364 B — 0.008% — from the new dialog, its store, and five strings
across thirteen locale catalogs. The budget had effectively no headroom, so
this branch is just the change that happened to cross it.

4.7 -> 4.75 MB buys back room for the next few features rather than landing
exactly on the line again. The main-bundle and CSS budgets are untouched and
still comfortably under.

* fix(ok): opening a skill must not eat the history entry it came from (PRD-7707)

navigation-history.e2e.ts, failing on this branch since the sidebar click was
routed through the shared opener:

  Expected: "#/__skills__"
  Received: "#/__asset__/nav-folder-.../nav-asset-....png"

One entry too FEW. The shared opener always replaced the hash, so opening a
skill from the Skills home overwrote #/__skills__ and Back skipped straight to
whatever preceded it. The last green run on this branch predates that routing
commit and the two runs in between were cancelled, so nothing reported it until
these commits landed.

History gets its own flag. `replaceActive` means "reuse the TAB" - it is the
preview-tabs preference, on by default - and says nothing about navigation, so
keying the hash off it (my first attempt) changed nothing. `replaceHistory` is
set only where an open genuinely supersedes what the user is standing on: the
preview tab becoming the real skill after an install.

Also reverts two changes from this session aimed at an unconfirmed "first click
does nothing" report - the removed same-row click guard, and installing the
click handler in a layout effect. Neither was shown to fix that symptom, both
touch this navigation path, and the guard is correct again anyway now that
activePath requires a genuinely open tab.

* style(ok): format the preview-redirect replaceHistory flag (PRD-7707)

* fix(ok): clear the five pending review recommendations (PRD-7707)

Carried since review rounds 17-19 and never picked up - they sit in review
BODIES, and the status script only counts unresolved inline threads, so they
never entered the work queue.

convertLocations was the actionable one: it rm's each destination before
materializing it, the same path install and runLocationWrite guard, but it
alone ran unguarded - so a scan taken mid-convert reads the skill as absent and
anything treating absence as deletion closes the user's tab. Now guarded.

skill-install-ops: dropped the `??` fallback to effectiveInstallMode. installMode
is required on that interface and the only call site resolves it beforehand, so
the branch could never fire while making the service look like it defaulted.
The caller owns resolution; the dep stays wired for the sibling service.

Three error paths were documented in comments but pinned by nothing - stale
beats blank on a failed refresh, the write flag released on a FAILED move, and
the dialog surfacing a failed preview instead of disabling its only button.
Tests now cover each.

skill-projection: covered the canonical-host arm of convertCopies. Every
existing test exercises the non-canonical arm; this one pins that the canonical
stays a real directory rather than becoming a link to itself.

* test(ok): assert the preview redirect's replaceHistory flag (PRD-7707)

Both SkillPreviewTab dom suites pin the exact openSkill(...) arguments, so
adding replaceHistory to that call site broke them. The flag is the point of
the change - the post-install redirect supersedes the preview rather than
stacking a second entry for the same skill - so the assertions gain it.

* fix(ok): reconnect a dead pooled provider on open (PRD-7707)

Opening a document in a window that had been sitting idle could hang on a
blank editor for the full 30 second sync timeout. The provider pool's
cache-hit path returned a pooled entry without checking whether its
connection was still alive, so a provider whose socket had dropped was
handed to syncPromise, which then waited for a synced event that could
never arrive. The error boundary recycled the entry once the timeout fired
and the same document loaded in about 300ms.

Recycle a known-disconnected entry on the hit path instead, reusing the
pool's existing recycle machinery. Gated on hasSynced and on the provider
holding no unsynced edits, matching the debounced disconnect recycle: an
entry still working through its first connect is waiting on a sync its own
backoff will deliver, and this path has no buffer-and-replay.

Verified against a packaged build by freezing the server long enough for
the client to drop the socket, then reopening a warm document: the pool
recycles and the document resolves in 315ms.

* fix(ok): keep the Skills surface on bundle files and scope moves (PRD-7707)

Two ways the Skills sidebar dropped the user into Files.

Clicking a markdown file a skill ships beside its SKILL.md switched the
sidebar to Files. The surface predicate admitted only SKILL and
references/**, so a companion doc read as ordinary content. Bundles are
free to ship companion markdown at their root, and skills on skills.sh do:
tdd carries tests.md and mocking.md, and its SKILL.md links to both. Right
after installing, with the bundle expanded, those rows are the obvious
thing to click. Widened to any file inside a bundle, which is what the
constant's own note reserves it for; the narrow parsers that feed the link
index are untouched.

Moving a skill between scopes landed on an unrelated document with the
sidebar on Files whenever the only tab open for that skill was one of its
bundle files, which is the normal state after clicking a skill and then one
of its files, since the file replaces the SKILL preview tab. Three causes,
all resting on the same assumption that a SKILL-level tab is always open:
the tab reconciler could not see companion docs at all, so a move or delete
stranded their tab on a removed doc; it closed a bundle-file tab on a scope
move on the grounds that the SKILL tab would carry the move; and the move
itself resolved its retarget source from the skills list before the open
tabs, aiming at a name no tab carried, so it repointed nothing.

The open tab now follows the skill to its new scope, a lone bundle-file tab
is promoted rather than closed (one per skill, so no duplicate tabs), and
companion docs are recognized by the reconciler.

One existing expectation changed: a test asserted that a lone reference tab
closes because the SKILL tab carries the move, while passing no SKILL tab.
It now covers both arms.

Reproduced and verified in a browser against the dev server, base versus
fixed, and again in a packaged build.

* test(ok): track the track-in-git route in both handler meta-tests (PRD-7707)

`handleSkillTrackInGit` landed without an entry in the two meta-tests that
enumerate every handler in the static route registry, so both reported it
as untracked and integration shard2 has been red since the commit that
added the route.

Exempt in both. The handler appends a negation to the project `.gitignore`
so a gitignored bundle becomes trackable: it rewrites a config file
fs-direct, never a Y.Doc body, so the per-doc conflict gate does not apply
and there is no authored content to attribute. Same posture as the sibling
project-config skill handlers.

GitOrigin-RevId: 3523232fc28dc6f8d94b35bb538ab57c53b6a68a

v0.52.0-beta.3

Toggle v0.52.0-beta.3's commit message
fix(ok): gate destructive MCP tools with ask, not deny (PRD-7891) (#3…

…419)

* fix(ok): gate destructive MCP tools with ask, not deny (PRD-7891)

The docked terminal's auto-approve settings put delete, move, share_link,
install, and import on Claude's permissions.deny list, on the assumption
that deny outranks allow and would therefore force a prompt. A bare
tool-name deny rule instead removes the tool from Claude's context
entirely, so agents launched from the built-in terminal saw 16 of the 21
registered OpenKnowledge tools and could not rename or delete a document
at all. Verified against a live MCP server: deny yields 16 tools, ask
yields 21 and still prompts before the call runs.

ask is the rule the original intent called for. It outranks allow the
same way and keeps the tool callable.

Reported at #1115.

* fix(ok): update the remaining pinned deny-list copies

The app-side docked-terminal launch test pins the full Claude --settings
string, so it still asserted the deny shape and failed test:dom. The
configuration reference carried the same "allow/deny list" sentence as
editor.mdx and also omitted import from its gated-tool list, which
predates this change but sits in the sentence being corrected.

GitOrigin-RevId: aa0b2d55d460c3fa4679c9261bc08f9e5efc6bf8

v0.52.0-beta.2

Toggle v0.52.0-beta.2's commit message
Stabilize desktop theme and Windows chrome smoke (#3424)

* fix desktop theme smoke synchronization

* address desktop smoke review findings

* fix desktop WCO tab reserve accounting

GitOrigin-RevId: fe3506dea04bb9fbbaa97e21225f332af188981d

v0.52.0-beta.1

Toggle v0.52.0-beta.1's commit message
feat(ok): report platform and version on download + update telemetry …

…(#3412)

* feat(ok): report os and arch on download + update telemetry

`app_update_downloaded` carried `artifact_type` but no platform dimension,
so the auto-update base could not be split by OS at all and arch was
invisible everywhere. Both are derivable from the artifact name the updater
requests, so the update proxy now emits `os` (zip→macos, exe→windows,
deb/rpm→linux) and `arch`, normalizing each packager's spelling — dpkg's
`amd64`, rpm's `x86_64`/`aarch64` — onto the same vocabulary
`download-targets.ts` already emits on `dmg_downloaded`. Unrecognized tokens
drop out rather than reaching PostHog as new free-form values.

`/download/beta` emitted no platform properties at all. It reports
macos/arm64/dmg, which is what it actually serves: the beta resolver matches
DMG_ASSET_NAME and nothing else, so there is no Windows or Linux beta to
attribute. Hardcoded rather than read from `?os=` so a link asking for a
platform the route cannot serve is not miscounted as one it did.

`/download/stable` and the `/d/` share splash already emit the triple.

* feat(ok): attribute Windows and Linux stable updates to a version

Only the macOS zip embeds its version in the filename. The Windows and
Linux installers are deliberately versionless permalinks that stable
resolves through GitHub's `latest` alias, so the proxy had nothing to
parse and every Windows/Linux stable update was recorded with a null
`to_version` — 15 of 15 exe and 1 of 1 deb over the last 14 days, against
0 of 9548 for the mac zip. Beta escapes this only because it resolves the
tag. Release-cohort and version-adoption views were macOS-only in effect.

The updater now names the version it is installing in `x-ok-to-version`,
and the proxy falls back to that header when it cannot derive the version
itself. Server-derived sources still win: a filename version and a
resolved beta tag both describe what the redirect actually serves, while
the header is only the client's claim, so it cannot overwrite either.
The header is validated against the same pattern as `x-ok-from-version`
before reaching analytics.

`requestHeaders` is read when `downloadUpdate()` runs rather than when the
feed is configured, so the value is set in the `update-available` handler
once the cross-channel veto has passed. That placement is safe only
because `autoDownload = false` puts the download call below the handler
instead of racing it, and it is guarded on the proxy feed so the GitHub
fallback never grows an OpenKnowledge header.

---------

GitOrigin-RevId: c08326f7cd9e24b3bec744cb366964dfb82582e7

v0.52.0-beta.0

Toggle v0.52.0-beta.0's commit message
feat(ok): re-point desktop attach to server.lock v2 (PRD-7903) (#3413)

* feat(ok): attach desktop through the server.lock v2 one-URL contract (PRD-7903)

Every desktop dial against a lock holder now derives from the single
address the server advertises in server.lock: the keepalive WS and the
branch-switch dialog proxies previously rebuilt ws://localhost:<port>
and http://localhost:<port> from the port alone, which misses a server
bound to a non-default loopback. Both now route through the shared
lockApiOrigin/lockWsOrigin helpers (v2 url preferred, port fallback for
pre-v2 locks), and the get-info and terminal-window collab URLs derive
from the context's apiOrigin instead of rebuilding from the port.

The packaged spawn drops the sibling-era --serve-content-assets flag;
the bundled CLI serves content assets on every boot path by default and
the spawned server is the canonical single-listener advertising url
plus capabilities ["ui"]. The explicit --react-shell-dist-dir stays,
since resolving the shell from inside app.asar is not guaranteed by the
packaging layout.

New end-to-end pins in the window-manager attach suite assert the v2
contract all the way through: the WS-upgrade probe and the renderer's
injected --ok-api-origin/--ok-collab-url args derive from a ui-capable
lock's url (including an ::1 bind), a pre-v2 lock keeps the localhost
port fallback, and a non-loopback url is refused by validation. These
pins are what make it provably safe for the later removal wave to
retire the ok ui sibling and ui.lock: the desktop attach surface
consumes only server.lock.

Also retires the stale README description of the launch.json
open-knowledge-ui rewrite, which no longer exists in the desktop.

* fix(ok): address PR #3413 review — one collab-URL helper, wss pin (PRD-7903)

Extracts httpOriginToWsOrigin as the single scheme-swap primitive and
collabUrlFromApiOrigin as the shared http-origin-to-/collab derivation.
lockWsOrigin and lockCollabUrl now delegate to them, and the two sites
that inlined the replace on an already-resolved apiOrigin (the
ok:project:get-info handler and resolveTerminalWindowProject) call the
helper instead, so the untested IPC site inherits the tested behavior.

Parity test gains the https-to-wss pin through lockCollabUrl (guards a
future narrowing of the regex to /^http:/) and an equivalence pin that
an apiOrigin-string consumer derives the same URL as the lock-shaped
helper.

Also refreshes resolveTerminalWindowProject's stale doc (the collab URL
derives from apiOrigin now, not the port) and replaces the CORS
injector's stale 'server binds 127.0.0.1 only' claim with the decision
record: the filter deliberately stays at localhost/127.0.0.1 because
pre-CORS CLI servers, the only servers the injector exists for, could
only ever bind those addresses; a server new enough for a non-default
loopback bind emits native CORS headers, so widening the filter would
guard an unreachable version-skew combination.

* fix(ok): drop the unused httpOriginToWsOrigin export (knip) (PRD-7903)

The scheme-swap primitive stays as the single internal derivation point;
external callers go through lockWsOrigin and collabUrlFromApiOrigin,
which are the exported, tested surfaces. Un-exporting satisfies knip's
unused-export gate without losing the one-primitive structure.

GitOrigin-RevId: db17f4d0c10d393b5007a84ccc3a4ee50abb53e8

v0.51.0-beta.7

Toggle v0.51.0-beta.7's commit message
feat(ok): make --remote a thin alias over server.* keys (PRD-7902) (#…

…3402)

* feat(ok): make --remote a thin alias over server.* keys (PRD-7902)

ok start --remote <url> now expands in the CLI to the ratified server.*
networking keys: server.publicUrl set to the tunnel URL, server.allowExternal
consent, and a loopback bind, flowing through the same resolution path as any
other exposure route. The dedicated remote topology (resolveRemoteAccess,
isRemoteAdmitted, RemoteConfigError, the legacyRemote arm of the ingress
policy, BootServerOptions.enableRemote) is deleted; the surviving host-header
and forwarding-header helpers move into ingress-policy.ts.

Behavior over the tunnel is preserved: same Host and Origin admission, the
stable remote port default, idle shutdown stays off, loopback bind coercion,
and the https-only rule for the tunnel URL. One deliberate improvement:
issued URLs (MCP serverUrl, preview_url) now name the tunnel URL instead of
an unreachable loopback address, since the alias sets server.publicUrl
through the successor key.

The flag is deprecated and prints a notice pointing at --public-url, --bind,
and OK_ALLOW_EXTERNAL; a new --public-url <url> flag sets server.publicUrl
for a single run. Combining --single-file with --remote is now refused
loudly (the legacy flow printed an enabled banner while never arming remote
there). The remote.* config keys and the flag keep working through the
deprecation window; removal ships separately.

Tests: alias-equivalence (the expansion resolves an identical
ServerRuntimeConfig to the explicit --public-url plus OK_ALLOW_EXTERNAL
spelling), deprecation-notice and exit-code assertions, and the remote-mcp
admission matrix now runs against the alias-shaped runtime as
behavior-preservation pins.

* test(ok): pin --remote alias self-consent and single deprecation notice (PRD-7902)

Two verification-review gaps: a boot-level characterization that the alias
shape (publicUrl + allowExternal on a loopback bind) boots outright with no
OK_ALLOW_EXTERNAL in the environment (the flag historically WAS the consent,
so an interlock refusal here would break every existing --remote user), and
a strengthened deprecation-notice assertion (fires exactly once and names
all three successors: --public-url, --bind, OK_ALLOW_EXTERNAL).

* fix(ok): scope --remote self-consent to loopback binds (PRD-7902)

Review finding (pullfrog): the alias granted server.allowExternal
unconditionally, so ok start --remote <url> on a project with a non-loopback
server.bind (a tailnet or LAN address coerceRemoteBindHost does not
recognize) booted and served that network with no explicit consent — before
the alias, the exposure interlock refused it with exit 78.

scopeRemoteAliasConsentToBind now drops allowExternal from the alias overlay
unless the effective bind is loopback-only, so the interlock refuses exactly
as it did pre-alias, with a warning that names the fix
(OK_ALLOW_EXTERNAL=1). Verified end-to-end against the built CLI: the
non-loopback form exits 78, the loopback tunnel form still self-consents and
serves the same admission matrix.

Also from review: deprecation notice now leads with the migration command,
the --single-file + --remote refusal explains itself, two stale
resolveRemoteAccess comment references in core are updated, and
remote-access.mdx gets a deprecation callout naming the successor
invocation.

* docs(ok): name OK_IDLE_SHUTDOWN=off in the --remote migration path (PRD-7902)

Review round 2: the written migration command named only two of the three
keys the alias pins. A migrator following it on a loopback bind got the
derived 30m idle shutdown, and the idle timer counts editor WS connections
only — the server would tear down under a live remote MCP client after 30
minutes. The deprecation notice, the remote-access.mdx callout, and the
cli.mdx --public-url row now all carry OK_IDLE_SHUTDOWN=off with the why,
and the notice test asserts the token.

Also from review: document the server.* networking keys (bind, port,
publicUrl, allowExternal, idleShutdown) in the configuration reference and
mark remote.* as superseded; de-duplicate the trailing-slash normalization
in expandRemoteAlias; move the expandRemoteAlias JSDoc back onto its
function; pin the mixed loopback+non-loopback bind case for
scopeRemoteAliasConsentToBind.

GitOrigin-RevId: 5fdc6e5e629b7d9b0f213df1eb6714af39837d91

v0.51.5

Toggle v0.51.5's commit message
fix(ok): give new projects an initial commit (PRD-7310) (#3335)

* fix(ok): give new projects an initial commit so worktrees work (PRD-7310)

Setting up a project ran `git init` but never committed, which left the
repository with an unborn HEAD: `main` pointed at nothing. Every
main-relative command then failed. The most visible casualty was New
worktree, which runs `git worktree add -b <name> <path> -- main` and died
with `fatal: invalid reference: main`.

`ensureProjectGit` now creates an empty root commit after it initializes a
repo, so the default branch resolves. That function is the shared
enforcement boundary for all four project-init paths (create-new-project,
the pick-existing consent flow, the utility-process boot, and `ok init`),
so no caller can forget it and no fifth caller will have to remember.

The commit is empty on purpose. `ensureProjectGit` runs before any
scaffolding, and three of its callers can target a folder that already
holds the user's un-versioned files, so staging content would put a whole
notes tree under version control in a commit nobody asked for. Only a repo
this call initialized is touched, so a user's own unborn-HEAD repo is left
alone.

Several subsystems already carried defensive skips for the unborn-HEAD
state (sync-engine push cycles, the server-factory drift check,
project-skill-git, project-log). Fresh projects now take the normal path
those skips were working around.

Worktree create failures are legible now too. A repo with no commits gets
its own `empty-repo` reason instead of the generic arm, and both worktree
surfaces render git's own stderr, so an unrelated failure like a lock file
stops reading as a bad branch name.

* fix(ok): repair stranded unborn-HEAD repos, drop locale-bound identity match (PRD-7310)

Review round on the initial-commit fix.

Gate the root commit on "this repo holds zero refs" rather than "this call ran
git init". The old gate was forward-only: every project shipped OK created
before the fix kept its unborn HEAD forever, and nothing else backfills it, so
the people who reported this bug would have stayed broken. A repo with no refs
has nothing to lose and nothing for a root commit to be disjoint from, so
adopting one is safe. Stranded projects now get their commit on next open,
since ensureProjectGit already runs on the utility-process boot path.

The same gate closes a second hole: the old `rev-parse --verify HEAD` probe
only answered for the current branch, so a salvaged `.git/` carrying history on
another branch with an unborn `main` would have been given a disjoint root.
`hasAnyRef` blocks that. A filesystem probe runs first so an already-populated
repo still invokes no git and never pays the preflight.

Identity detection no longer reads git's message. Git localizes it (a German
locale reports "Bitte geben Sie an, wer Sie sind"), so matching English phrases
silently skipped the fallback for exactly the non-English users it exists for.
`git var GIT_COMMITTER_IDENT` answers by exit code instead. The fallback stays
conditional: `-c` overrides configured values rather than being inert, so
supplying it unconditionally would misattribute commits for everyone who has an
identity set.

Tests: the no-identity case now sets `user.useConfigOnly` so git cannot
auto-derive `user@host` and the fallback is genuinely exercised, with a
companion test pinning that a configured identity is preferred. The old "does
not commit into a repo it did not initialize" encoded a promise this change
narrows on purpose; it is replaced by one test pinning the repair and one
pinning the real bound, that a repo holding any history is never touched.
`empty-repo` added to the exhaustive create-refusal enumeration.

Two integration helpers globbed all of `refs/wip/`, conflating agent writer
refs with the `git-upstream` import ref that exists now that fresh repos have
history. Both filter to session writers via `parseWriterId`, mirroring
`shadow-repo-stats.ts`. Their comments already said they meant agent writes.

Drop `initialCommitCreated`: no caller read it, so its documented `false` state
was unobservable.

* fix(ok): ask one question about repo emptiness, and answer it the same way twice (PRD-7310)

Second review round.

`repoHasCommits` in the worktree path still used `rev-parse --verify HEAD`, the
probe the server path had already moved off. That answers only for the CURRENT
branch, so a repo whose history sits on another branch while the requested base
is unborn was reported as `empty-repo` — telling the user "this project has no
commits yet" about a repo full of them. It now asks `for-each-ref` like
`hasAnyRef` does, and those failures fall to the generic arm, which carries
git's own `invalid reference` text. Test added for that shape.

`hasAnyRef`'s catch now logs. It is the guard the repair path turns on, so a
transient `for-each-ref` failure would silently skip the backfill and strand the
user in exactly the state this code exists to clear, with nothing to explain it.

`hasRefsOnDisk` documented honestly: it reads only `refs/heads/` and
`packed-refs`, so a repo carrying nothing but tags or remotes answers false.
That costs one wasted spawn and nothing more, because `hasAnyRef` re-asks over
the whole namespace before anything is written. The old comment implied
coverage it does not have.

Fallback identity is `[email protected]`, matching what the shadow
repo already commits under, rather than inventing a second address for OK.

GitOrigin-RevId: e572bff8750d1019f4de93444b3cd5ed575a1a08

v0.51.4

Toggle v0.51.4's commit message
feat(ok): make destructive confirmations undismissable (PRD-7907) (#3…

…389)

* feat(ok): make destructive confirmations undismissable (PRD-7907)

The delete confirmations were built on Dialog, which dismisses on outside
click, dismisses on Escape, and renders a close-X. That gave three ways to
walk away from a destructive decision without answering it, one of which is
a stray click. The most consequential case is the trash-failure fallback,
whose primary action bypasses the OS Trash and unlinks the file outright.

Adds components/ui/alert-dialog.tsx and moves DeleteConfirmationDialog and
TrashFailureModal onto it, along with the six Dialog roots that render them.
Radix AlertDialog gives role="alertdialog", blocks outside pointer dismissal,
and moves initial focus to the cancel choice rather than the first focusable
child. Escape is deliberately kept: it is the documented alertdialog keyboard
contract and reads as Cancel. The accidental-dismissal risk is the pointer,
not a keystroke someone chose to press.

Hand-authored rather than installed. The registry component resolves here,
unlike the spinner, but it omits the Electron titlebar drag opt-out that
makes a dialog near the top of the window clickable, omits the reduced-motion
opt-outs, has no body slot for the failure list, and styles its title with a
token this theme does not define. The class strings therefore mirror
dialog.tsx so migrated dialogs keep the proportions they have today.

Both confirming actions stay plain Buttons rather than AlertDialogAction.
Radix builds Action on Dialog.Close, so it would close the dialog the instant
it is pressed, tearing down the in-flight spinner state these two report in
the button's own label. The primitive's test pins that distinction so the
next migration does not have to rediscover it.

The audit scoped this as two call sites, meaning two components. Migrating
them actually touches six Dialog roots, because each owner supplies its own
root: FileTree owns three, and LintingSection, SkillDeleteDialog,
SkillFileDeleteDialog and TemplateDeleteDialog own one each. One of FileTree's
three is a convert-to-template confirmation reusing DeleteConfirmationDialog,
which is destructive in the same way and comes along with it.

Ten Playwright assertions queried these dialogs by role="dialog" and would
have gone red on merge; they now query alertdialog. The one in show-ok-folders
that presses Escape to dismiss is left as an end-to-end check that Escape
still works.

* fix(ok): suppress the alert-dialog overlay blur under reduced transparency (PRD-7907)

The prefers-reduced-transparency block listed the dialog and sheet
overlays but not the alert-dialog overlay, so destructive confirmations
kept their backdrop blur for users who had the setting on. The guard
test now names every overlay slot that opts into the blur.

* test(ok): allowlist the alert-dialog overlay for the safe-area invariant (PRD-7907)

The new AlertDialogOverlay is a fixed inset-0 backdrop, which the
macOS traffic-light safe-area invariant flags by class pattern. It is a
pure dimmer with no children, and the interactive footer lives in the
centered AlertDialogContent, so it qualifies for the same exemption the
Dialog and Sheet overlays already carry.

GitOrigin-RevId: 3dcdd5e8dc8c0a21cc1bcdff2011039785ca80e0