Make the local telemetry file sink flush spans on span end…#2
Merged
Conversation
* fix(open-knowledge): deflake 4 merge-queue-blocking CI tests Root-cause fixes for the Bun integration/unit tests that intermittently evicted PRs from the merge queue under CPU contention. Each removes the flake mechanism rather than bumping a timeout: - rename-history: the 50ms awaitWipCommit poll re-ran getHistory (several git subprocesses per call) ~20x/sec, starving the commitWip it awaited. Replace with growing intervals [100,250,500,1000] (the Playwright expect.poll pattern); budget unchanged so the file's 60s/3-cycle tests still fit. - c11 CC1 signal: the fixed 5s poll inherited Bun's 5s default test timeout. Widen to a 20s bounded poll keyed to the commit-signal chain plus an explicit 30s test timeout. - telemetry Gate-4: use SimpleSpanProcessor for the local file sink so spans flush on span end instead of racing the 5s batch timer against the 5s shutdown timeout. Batch stays for the OTLP push exporter. - templates SCAN_CAP: sort readdirSync so the BFS walk and cap truncation are deterministic across filesystems. lint and typecheck clean; repeated local runs green (rename 12/12, telemetry 10/10, templates 20/20, c11 6x). * fix(open-knowledge): telemetry sink uses raw fs to avoid SimpleSpanProcessor recursion Addresses the cloud review's Critical finding on #1588: switching the local telemetry file sink to SimpleSpanProcessor exposed a latent recursion. The sink's RotatingAppender wrote via the fs-traced wrappers, whose spans SimpleSpanProcessor re-exported back into the sink — an unbounded chain at disk-I/O speed (BatchSpanProcessor's 5s timer had masked it to ~1 step/tick). Tests passed because shutdown breaks the chain; it only manifests during sustained operation. - telemetry-file-sink.ts: RotatingAppender writes with raw node:fs/promises. It backs only observability sinks (spans + logs), so no application write loses fs tracing. Updated the now-stale BatchSpanProcessor comments. - c11 CC1 test: replace the opaque poll-exhaustion assertion with a diagnostic (signal-call count + channels seen) for CI triage (review Consider item). - Regenerated md-audit registry for the shifted test line numbers. * fix(open-knowledge): raise test:integration default timeout to 30s The integration tier inherited Bun's 5s default test timeout, so any test that boots a server (createTestServer/bootServer) in its body could exceed it under merge-queue CPU contention — surfacing as "timed out after 5000ms" then a server.port-undefined TypeError. A ts-morph scan found 28 such at-risk tests (e.g. persistence-failure-surfacing, which flaked on this PR's own CI run). Systemic fix for the whole class: one --timeout on the tier, mirroring test:fidelity (--timeout 120000) and test:perf:sessions (60000) — not 28 per-test bandaids, and it covers future tests too. Per-test explicit timeouts still override it, so the heavier tests (rename-history's 60-180s) are unaffected. GitOrigin-RevId: c0d8fca9ee3b7bd396fd5d5fa8ec95415ce40ebe
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/26903818384). Source of truth is the monorepo; direct edits on inkeep/open-knowledge are overwritten on next sync.
inkeep-oss-sync Bot
pushed a commit
that referenced
this pull request
Jun 3, 2026
* test(open-knowledge): pin replace atomic-overwrite (PRD-6667) At zero concurrency, applyAgentMarkdownWrite(..., 'replace') must emit a full Y.Text delete plus full insert. Today the case 'replace' branch routes through composeAndWriteRawBody (DMP-incremental, item-preserving) instead of the atomic sibling replaceRawBody. Test fails RED on origin/main; the fix lands in a follow-up commit. Linear: https://linear.app/inkeep/issue/PRD-6667 * fix(open-knowledge): replace honors atomic-overwrite (PRD-6667) The case 'replace' branch of applyAgentMarkdownWriteInner now routes through replaceRawBody (atomic delete + insert) instead of composeAndWriteRawBody (DMP-incremental, item-preserving). The latter broke replace's atomic-overwrite contract even at zero concurrency: a payload that shared substrings with the prior content only inserted the differing characters. append and prepend keep composeAndWriteRawBody since their merge-style semantics are correct. The atomic primitive already existed locally (rollback's call site) with a contract test pinning the FULL OVERWRITE shape in bridge-intake.test.ts. Layer 2 residue from concurrent peer CRDT ops is a separate structural-failure class deferred for a product decision. Linear: https://linear.app/inkeep/issue/PRD-6667 * test(open-knowledge): pin replace contract beyond the primitive shape (PRD-6667) Additional contract tests covering the post-fix behavior: - single-writer replace lands payload bytes verbatim - replace with FM in payload supersedes existing FM - replace with body-only payload preserves existing FM - replace with identical content is a no-op (idempotent at Y.Text) - session.um.undo() after a replace restores prior bytes (one atomic frame) These describe what is true now and what future changes must keep true. * test(open-knowledge): pin replace convergence at CRDT layer (PRD-6667) Deterministic two-Y.Doc + manual Y.applyUpdate harness. The real-WS integration harness consistently sequenced human edits through Observer A before the agent's HTTP fire, masking the race; the two-Y.Doc pattern gives us deterministic control over in-flight peer timing. Two contracts: - single-writer (peer fully synced): converged Y.Text equals the agent payload byte-for-byte. - concurrent peer typing: the agent payload survives as a CONTIGUOUS substring in the converged doc. This is the distinguishing observable property of the atomic primitive (replaceRawBody): all prior items are tombstoned by the full delete, and the agent's content lands as a contiguous block of fresh items; the peer's in-flight op anchors adjacent to the tombstoned region, not interior. Under DMP-incremental (the pre-fix behavior) shared bytes are preserved and the peer's op fragments the payload. Layer 2 residue (peer's text surviving in the converged doc) is not asserted on shape since Yjs item-resolution internals can evolve. That's a separate structural-failure class (#6 cross-time mutation) deferred for a Layer 2 product decision. * chore(open-knowledge): satisfy Biome on PRD-6667 tests - agent-sessions.test.ts: collapse multi-line applyAgentMarkdownWrite call onto one line (biome format). - agent-write-replace-crdt-convergence.test.ts: peerText.charAt(i) in place of peerText[i]! (biome lint forbids non-null assertion). No behavior change. * chore(open-knowledge): changeset for replace atomic-overwrite fix * docs(open-knowledge): note two-primitive dispatch in applyAgentMarkdownWrite JSDoc The function-level docstring still claimed agent writes routed through the shared composeAndWriteRawBody primitive. After the atomic-overwrite fix (replace -> replaceRawBody), the dispatch is two-primitive. Updated the docstring to reflect the split so it matches the block comment at the call site. Reviewer feedback on PR 1270. * docs(open-knowledge): note replace -> replaceRawBody in W3 write-surfaces table AGENTS.md (symlinked from CLAUDE.md) line 141 still showed composeAndWriteRawBody for both columns of the W3 Agent API row. After the atomic-overwrite fix, replace routes through replaceRawBody; append and prepend still use composeAndWriteRawBody. Updated the row in place; kept the change minimal given the 40k-byte hard cap (file now at 39966 bytes, 34 bytes under). Reviewer feedback on PR 1270 from claude[bot]. * chore(open-knowledge): retrigger CI for flaky vercel preview deploys 3 Vercel preview deploys failed terminally on c2ea67984 with no source changes that should affect them (1-line AGENTS.md doc edit). The same projects deployed SUCCESS on the prior HEAD 56e364c5c. agents-docs is a known pre-existing failure on main; the other two (agents-manage-ui, inkeep-cloud-mcp) build from paths this PR does not touch and were green 4 hours ago. Empty commit forces Vercel preview retry without source churn. Linear: PRD-6667. * feat(open-knowledge): Track 1 content gate + observability symmetry (PRD-6667) Closes the meta-problem of silent agent-write success at the producer boundary. applyAgentMarkdownWrite now performs a post-primitive content-divergence check inside the transact and returns AgentWriteContentDivergence | undefined; handleAgentWriteMd and handleAgentPatch surface it as warning: { kind: "content-divergence", intendedBytes, actualBytes, byteDelta, hint } on their success envelopes. MCP shims (write-document, edit-document) relay it under structuredContent.contentDivergence and append a one-line text hint. The gate covers both surfaces because both handlers internally call applyAgentMarkdownWrite. Inside the transact (after the primitive write, before transact close) it sees the post-primitive Y.Text exactly; divergence here signals a primitive regression or — forward-compat — observer-side canonicalization that leaked through the paired-write fast-path. Cross-time-mutation residue from peer ops arrives post-transact and is NOT caught here; that's Site B's territory. Observability symmetry: - replaceRawBody is now wrapped in withSpanSync(bridge.replaceRawBody); matches the existing composeAndWriteRawBody instrumentation and gives handleRollback the same observability (net positive). - replaceRawBody JSDoc now reflects both call sites (rollback + agent- write replace) rather than overspecifying ROLLBACK_ORIGIN. - agent.applyAgentMarkdownWrite span gets new attributes (agent.content_divergent / agent.intended_bytes / agent.actual_bytes / agent.byte_delta) when the gate fires. - structured JSON warn log on agent-write-content-divergence for aggregate counting (CLAUDE.md logging convention #2). Schema: - ContentDivergenceWarningSchema added to @inkeep/open-knowledge-core (additive, .loose()). - AgentWriteMdSuccessSchema and AgentPatchSuccessSchema gain optional warning field. Docs: - SKILL.md gets a content-divergence note under Writing (deliberately outside the 4 sections that feed mcp/instructions.ts so we stay under the byte-budget ceiling). - write-document and edit-document tool descriptions reference the warning field. Tests: - T1.1 (negative path): applyAgentMarkdownWrite returns undefined for valid single-writer input. Skips Pattern A tautology per /error-boundary-discrimination Q5 — no real input can make the primitive diverge from its byte-faithful contract. Replaces the "Track 1 deferred to follow-up" disposition the prior review-bot pass surfaced; brings the OTel span symmetry the agents-sessions.ts:221 thread also flagged into the same PR. No deferred-tech-debt remaining for this PR's scope. * docs(open-knowledge): retrospective SPEC + corrigenda for PRD-6667 work Adds specs/2026-05-22-agent-write-replace-atomicity/ as the authoritative architectural reference for the work that shipped in commits f3695fd62 through d65c5f0d0: - SPEC.md covers Layer 1 / Layer 2 framing, decisions D1-D14 (all LOCKED at shipped state), goals G1-G11, non-goals NG1-NG10, persona discipline, primary user journey, implementation pointers, future work, agent constraints. - evidence/diagnostic-pin.md walks the L1.2 primitive-shape test and why it (not L1.1/L1.4) is the load-bearing regression guard. - evidence/crdt-convergence.md documents the deterministic two-Y.Doc pattern and what it pins (contiguous-substring contract, not residue shape). - evidence/boundary-classification.md applies /navigate-boundaries to both layers and traces ownership. - meta/_changelog.md captures the retrospective-authorship process notes. Corrigenda per OK CLAUDE.md post-ship convention (never rewrite shipped prose; append same-line breadcrumb): - specs/2026-04-10-mcp-write-tools/SPEC.md — write_document tool row notes the new two-primitive dispatch + contentDivergence warning. - specs/2026-04-14-bridge-convergence-under-concurrent-writes/SPEC.md FR-1 notes applyAgentMarkdownWrite's new return type + the per-position primitive dispatch. - specs/2026-04-16-bridge-correctness/SPEC.md — atomic-writers list notes agent-write replace joined the set via session.origin. All three breadcrumbs point at the new spec as the authoritative reference. No prose rewrites. PRD-6667 architecture work is now durably documented. * feat(open-knowledge): batch divergence relay + agent-identity in divergence log (PRD-6667) Addresses claude[bot] review-3 (review 4348446689) findings: 1. agent-write-content-divergence structured log now includes agent.id + agent.client_name in both handleAgentWriteMd and handleAgentPatch warn payloads (both bounded-cardinality per CLAUDE.md OTel STOP rule; both were already in scope at the call site). Makes the event self-contained for aggregate queries — an engineer triaging the log no longer needs to correlate by doc.name + timestamp. 2. write_document batch path now surfaces per-doc contentDivergence on each documents[] entry (previously only the single-doc path relayed the server's warning field; the batch path silently dropped it). 4/5 sub-reviewers independently flagged the asymmetry. Text output includes a per-doc divergence line on fire. Tool description updated to specify both shapes (single: structuredContent.contentDivergence; batch: structuredContent.documents[].contentDivergence). 3. SKILL.md content-divergence note clarifies both shapes for agent readers. No production code path behavior change beyond observability + relay — the gate's fire condition is unchanged. bun run check GREEN at the server-package level (85/85 agent-sessions + bridge-intake + api-agent-patch); pre-existing desktop test failures (install-skill-ipc.test.ts version drift between server/package.json 0.6.0 and runtime build's 0.6.1-beta.2) are environmental and unrelated to this diff — confirmed by stash-test on prior HEAD. * fix(open-knowledge): edit_document stays incremental (de-overload position replace) PRD-6667's replace->replaceRawBody swap silently regressed edit_document: handleAgentPatch computes a full recomposed body and routed it through the shared replace case, so a surgical find/replace became a whole-document atomic overwrite (lost item-preservation, churned undo attribution, whole-doc CRDT delta per patch, widened the concurrent-edit residue surface from the matched span to the whole document). Add a server-internal position 'patch' that composes the body like replace but dispatches to composeAndWriteRawBody (incremental). write_document replace stays atomic; edit_document, append, and prepend stay incremental. 'patch' is never exposed on the write_document MCP enum. Tests (all RED if patch routes to the atomic primitive): - api-agent-patch.test.ts: HTTP-level minimal-delta pin - agent-sessions.test.ts: dispatch-level delta + empty-body-clears - agent-patch-crdt-convergence.test.ts: concurrent peer edit outside the patched span stays item-preserved SPEC D15/D16 document the de-overloading and right-size the residual narrative. * docs(open-knowledge): correct SPEC edit_document refs to 'patch' + add G12 The problem statement and NG7 still described handleAgentPatch as calling applyAgentMarkdownWrite('replace', ...); it now calls 'patch' (D15). Add G12 headlining the edit_document-stays-incremental goal, and note the patch path on the Layer 1 fix bullet. * feat(open-knowledge): inline divergence recovery + rollback gate (#1582) * feat(open-knowledge): inline recovery state, divergence counters, rollback gate, probe Follow-up to PRD-6667's content-divergence warning. So an agent recovers without a second read, the warning now carries the converged document inline via currentState ({kind:inline,content} or {kind:truncated,byteLength,hint} over a 50 KB cap) plus a coarse divergenceType. The Site A gate is extracted to a shared content-divergence-gate.ts and extended to handleRollback. Divergence is rate-measurable via two OTel counters (gate_fired plus content_divergence, bounded {handler}). Ports the production-Electron agent-patch divergence probe as permanent regression coverage (CI trial count trimmed to 25). Concurrent-human post-transact residue (Site B) and the 422 hard-error model are deliberately out of scope. * docs(open-knowledge): document the #1582 divergence-recovery increment in the atomicity spec Folds the follow-up into specs/2026-05-22-agent-write-replace-atomicity/SPEC.md (the canonical home for the Site A content-divergence gate): - Goals G13-G16: inline recovery via currentState, coarse divergenceType, rollback coverage, rate-measurable counters + the production probe. - Decisions D17-D21: the currentState/divergenceType schema additions, the shared content-divergence-gate.ts module, the handleRollback gate, the two bounded OTel counters, and the production-Electron probe. - Corrects the contradictions the follow-up introduced against the #1270 baseline: D7's "no dedicated counter" and NG10 now point to D20 (counter landed); the section 11 EXCLUDE of handleRollback now points to D19 (gate added); section 8 future-work and Q1 tracking updated. No behavior change; documentation only. Scope excludes Site B and the 422 hard-error model, consistent with the implementation. * fix(open-knowledge): actor identity on rollback divergence log + fix orphaned schema JSDoc Addresses two claude[bot] review findings on #1582: - handleRollback's content-divergence log omitted actor identity, while the sibling agent-write-md / agent-patch divergence logs include it. Adds actor.kind plus a conditional actor.writer_id (agent/principal only), mirroring the existing actor narrowing in api-extension.ts. A "should never fire" regression signal is far more useful with the triggering actor on it. - The ContentDivergenceWarningSchema JSDoc was orphaned above the new ContentDivergenceCurrentStateSchema and still read "re-read the doc and decide whether to retry", contradicting this PR's currentState feature. Moved it back above the schema it documents, updated the recovery guidance to reference currentState (inline vs truncated), and added handleRollback to the emitter list. * fix(open-knowledge): address claude[bot] review-body recommendations (7) The follow-up review surfaced 7 recommendations from the first automated review. All addressed: - Rollback divergence gate now runs INSIDE the transact (matching write/patch), reading the primitive's atomic post-state before observer settlement. This avoids false-positive divergence from post-transact canonicalization. - MCP contentDivergence relay now has test coverage: fake servers return a warning, and the shims' relay plus safeParse drop-path are asserted across the write/edit/version tools (single-doc and batch). - The version tool gains the same content-divergence text line plus DESCRIPTION documentation the write/edit tools already have. - The three shims validate the server warning with ContentDivergenceWarningSchema.safeParse instead of an unchecked cast across the HTTP boundary. - The byte fields and the 50 KB cap now count UTF-8 bytes, not UTF-16 code units; locked by a multibyte regression test. - evaluateContentDivergence's label is a bounded ContentDivergenceLabel union so the derived span attribute stays low-cardinality. - The stale "Re-read the doc" fallback hint now references currentState. GitOrigin-RevId: c4fd47bf9123dbe8284bb383681e5e8034baa2eb
inkeep-oss-sync Bot
pushed a commit
that referenced
this pull request
Jun 10, 2026
* Clarify OpenAI embeddings key in settings and CLI
- Settings 'Embeddings provider key' text now states it is your OpenAI
API key, used only to create embeddings, stored in ~/.ok/secrets.yml
(readable only by your user account).
- secrets.yml field renamed from 'embeddings' to OPENAI_API_KEY so the
file is self-evident. No back-compat (feature is unreleased).
- CLI 'ok embeddings' set-key/clear-key messages now say 'OpenAI
embeddings API key' so the key purpose is unambiguous.
- Env override stays OK_EMBEDDINGS_API_KEY (deliberate egress safeguard
against an ambient OPENAI_API_KEY).
- Regenerated lingui catalogs for the changed Settings string.
* Semantic search in the Cmd+K omnibar
Add a 'By meaning' mode to the omnibar that reuses the existing /api/search
semantic fusion (the same engine the MCP search tool uses). Client-side trigger
and presentation only; no new ranking engine.
- Capability-gated: the 'By meaning' pill shows only when semantic search is
enabled for the project and an embeddings API key is present
(/api/semantic-status). When unavailable the omnibar is unchanged.
- Exclusive deliberate-submit mode: entering it (a component state flag, not a
query prefix, so the raw query is what gets embedded) suppresses the lexical
palette. Typing stays instant and local; Enter fires one vector search.
- Sticky results: after a fire for query Q, editing the query keeps the prior
results (dimmed and inert, labeled Q) and offers a one-key re-fire, so a stray
keystroke never costs the expensive set, and a click can't open a stale row.
Cleared on mode exit / palette close.
- Latency + degradation: a spinner while embedding; on timeout a retry row with
the prior results retained. Escape exits the mode first, then closes.
- Telemetry: search requests carry a bounded source enum (omnibar | mcp | http);
the semantic-query counter splits by source so interactive cost reads apart
from agent cost.
Per-keystroke search stays lexical and byte-identical; the MCP tool path is
unchanged (it now also tags source: mcp). Pure decision logic is unit-pinned in
command-palette-semantic.ts; an e2e covers the gate, submit, sticky, and escape.
* Make semantic search retrieval rank-based instead of cosine-thresholded
Semantic ("by meaning") search returned nothing for synonym queries:
"cybersecurity" missed a page about hacking, "kid" missed a page about a child.
The cause was an absolute cosine floor of 0.35 applied before ranking, not the
model. text-embedding-3-small produces a compressed similarity band for short
keyword queries against whole-document embeddings (correct hits commonly score
0.13 to 0.29), so the floor discarded every vector candidate and search fell
back to pure lexical, which a synonym cannot satisfy. Measured on the reported
vault, the model ranks the right page #1 for "cybersecurity" (0.147) and #2 for
"kid" (0.152); only the floor suppressed them.
An absolute threshold is the wrong mechanism: the right value is model-specific,
and on a compressed scale a weak-but-real hit and weak noise overlap (a correct
hit scored 0.147 while an unrelated query scored 0.151), so no threshold
separates them. Retrieval is now rank-based:
- Core: the default floor is 0 (drops only negative cosines, never matches on any
model). Admission is top-K by cosine via the existing candidate limit, and RRF
orders them, so the absolute score never gates a real hit. An optional
per-project hard cutoff stays via search.semantic.similarityFloor, threaded
fresh-per-search so a model with a known cosine scale can be tuned without code.
- Omnibar: the "By meaning" list is bounded by a small result count, not a score.
Nearest-neighbor search always has a closest match, so a count cap (not a
threshold) keeps a vague query from filling the list with weak hits while rank
carries the relevance signal.
There is deliberately no "strong vs related" tier: no unsupervised rule separates
a nonsense flat-pack of near-equal scores from a broad query where many pages are
genuinely relevant, so an asserted boundary would mislabel both.
Fixes both the MCP search tool and the omnibar "By meaning" mode. A core test
pins the rank-based default (low cosines admitted, ordered by similarity); a
factory test pins that a configured similarityFloor reaches core ranking.
* Semantic search follow-ups: lexical-matched cap, masked key tail, spacing
Three review tweaks on the semantic search feature:
- Match the omnibar "By meaning" result cap to the lexical search cap (30) — a
count bound (nearest-neighbor search always has a closest match), not a score.
- Show a redacted last-4 tail of the stored embeddings key in Settings → Account
so a user can tell which key is set. The status endpoint exposes only the tail
(never the full key, and only when the key is long enough to redact safely).
- Add a little vertical spacing under the "Replace key" label.
* Self-heal loose embeddings secrets-file permissions on read
The embeddings API key file (~/.ok/secrets.yml) was re-asserted to 0600 only on
write. Since the key is read on every semantic search but rewritten rarely (often
never after the initial set), a file left group/other-readable by an older build,
an external tool, or a hand-edit could stay world-readable indefinitely.
The shared read() path (hit by get(), the lazy reader on every search, the
desktop reader, and `ok embeddings status`) now stat-checks the mode and chmods
to 0600 when it finds anything group/other-accessible, emitting a one-time
warning. Best-effort and never throws — reads are on the search path and must not
fail because a chmod did; chmod is a metadata op, so (like the write-path call)
it stays outside the fs-traced requirement.
* Address review: release in-flight semantic refs on settle; cover cleared-query states
- fireSemanticSearch now nulls semanticTimerRef/semanticAbortRef once a fire
settles (guarded so a superseding fire isn't clobbered), keeping the refs'
"non-null = a search is in flight" invariant true for resetSemanticState and
the open-toggle effect.
- Add computeSemanticModeView unit cases for clearing the query mid-re-fire
(spinner stays) and after an error (no retry row for an empty query).
- Match the e2e capability fixture to the real /api/semantic-status shape (keyHint).
* Address re-review: warn on failed secrets self-heal; log failed semantic fire
- tightenPermsIfLoose now separates a benign stat miss (file vanished / no POSIX
modes — stay silent) from a real chmod failure (the file is known-loose but
couldn't be tightened): the latter warns that the API key remains exposed and
how to fix it, instead of swallowing it. Still never throws on the read path.
- The semantic-fire catch logs at debug level (matching the no-console carve-out
for diagnostic catch sites, e.g. use-semantic-search-status) so a timeout or
network failure is diagnosable; the UI still surfaces the retry row.
* Omnibar by-meaning: carry typed text into the mode, show indexing progress
- enterSemanticMode keeps the current query (stripping only a 'tag:' prefix)
instead of blanking it, so a query typed before clicking the pill carries in
and becomes the pending semantic search.
- Add an indexing banner with live coverage in the by-meaning view, shown while
embedded < total. The first by-meaning search lazily kicks off the background
embed; a bounded 2.5s poll (only while in semantic mode + indexing) ticks the
count up, so the user knows results may be incomplete until indexing finishes.
- Settings: make the spacing under "Replace key" reliable (explicit block label
margin rather than space-y, which wasn't rendering) and give "API key set" the
same breathing room.
- e2e: text carries into by-meaning mode; indexing banner shows coverage.
* Read a legacy-named embeddings key as a back-compat fallback; add negative banner test
- secrets-store get() falls back to a key stored under the prior 'embeddings'
field so a key written by an earlier build keeps resolving instead of silently
vanishing after the rename to OPENAI_API_KEY. set() drops the legacy field on
the next write (one-shot, self-clearing) and clear() removes both so a cleared
key can't resurrect via the fallback. No-op when no legacy key exists.
- e2e: assert the indexing banner is absent when the corpus is fully embedded,
pinning the `embedded < total` condition against inversion.
* Adopt web-first count asserts; bump combined JS budget to 2.75 MB
Main's new playwright-prefer-to-have-count biome rule flags the two
one-shot count() reads the semantic palette e2e shipped with; convert
them to await expect(...).toHaveCount(0). The omnibar mode UI also
tips the all-chunks gzipped budget by 1.5 kB, so raise it from 2.7 MB
following the existing budget-bump precedent.
GitOrigin-RevId: aa4d074ef41b0858e08d8fe84c7b22014d18806f
inkeep-oss-sync Bot
pushed a commit
that referenced
this pull request
Jun 17, 2026
…#1937) Two of the four strands in the PRD-7058 cluster — the two with clear scope. #3 (Align broken) did not reproduce once #2 was fixed; the reporter had no path to reach the Align field through the un- scrollable popover. #4 (drag-resize for image/PDF) is a separate feature and lands in its own PR. 1. **#2 popup scroll** — `max-h-[calc(100vh-2rem)]` ignored the popover's top position, so a popover positioned 200px down the viewport was capped at the FULL viewport height — overflowed off- screen and never engaged its internal scroller. Switch to `max-h-[var(--radix-popper-available-height)]`, which Radix's Popper plugin pre-computes as "height available between the anchor and the chosen-side viewport edge." The popover now caps at the actual available space, so its `overflow-y-auto` engages and every advanced prop becomes reachable. 2. **#1 silent dismiss** — add an explicit "Done" button at the bottom of the popover, separated from the form by a top border. PropPanel still auto-saves on every keystroke / select change (the button doesn't gate the write); the affordance gives users the psychological closure UX research flagged was missing ("I just write, and it just, like, disappears"). Click closes the popover; the existing `onCloseAutoFocus` editor-refocus path handles focus restore. GitOrigin-RevId: 1e721d365b69fee7904d4d88c8c8e9303add8a99
inkeep-oss-sync Bot
pushed a commit
that referenced
this pull request
Jun 26, 2026
* OK skill editor: author, install, version skills (PRD-6935)
Adds skills as fs-direct .ok/skills artifacts, modeled on the template
spine: not CRDT, attributed, shadow-repo versioned. Resolves PRD-6934 by
folding pack skills into the content model.
- CRUD API + MCP verbs for skills (project and personal scope). Personal
skills live at <home>/.ok/skills, install into the user-global editor
folders, and are unversioned.
- Install projects a skill into editor host dirs + records an install
marker; uninstall reverse-projects and demotes the skill to a draft.
Reclaim re-materializes installed skills on launch.
- Editable .ok/skill-targets.json set re-projects every managed skill
(authored + bundled) when changed.
- Per-skill history + restore via the shadow repo (project scope).
- Skills UI in Settings: scope-grouped list with state/host badges,
create (scope-first), edit, delete, install/uninstall, history/restore,
a targets picker, and Open-with-AI that hands a draft to an installed
agent via the bundled write-skill meta-skill.
- Pack install now authors the pack skill into .ok/skills so it appears
in the list and is editable; the project skill carries a setup pointer.
* Address pullfrog review on #1662: skill move/restore/reproject correctness
- move: carry the install marker + host projection from fromName to toName
on rename (was orphaning the projection + leaving a stale marker key).
- restore: read all file contents from the shadow repo before clearing the
skill dir, so a mid-loop git failure can't leave a torn source.
- reproject: when a managed skill's source is gone, reverse-project from
every recorded host, not just the no-longer-targeted ones.
- restore schema: constrain version to a 40-char SHA (matches rollback).
* Fix React Compiler build error in use-skill-targets (CI #1662)
The save() helper used try/finally with no catch, which the React Compiler
can't lower (and it also rejects throw inside try/catch). Rewrite without a
try block: tolerate fetch rejection with .catch, reset saving on each exit,
and throw at function scope. tsc missed this; the failure only surfaced in
the production build (React Compiler runs at build time).
* Use optional chain in use-skill-targets (lint)
* Organize core index exports (lint/format gate)
* Fix CI gates for #1662: conflict-gate exempt, size budget, md-audit drift
- conflict-gate-coverage: register the 7 skill handlers as EXEMPT (skills are
fs-direct .ok/skills artifacts, not CRDT docs, so doc-in-conflict gating
doesn't apply). Was failing test:integration.
- size: bump the all-JS budget 2.7->2.8 MB + main 400->410 kB; the skill UI
legitimately grew the bundle and CI's build tipped the razor-thin limit.
- md-audit: regenerate byte-preservation-audit.json (registry-drift from an
upstream change to the audit source that never regenerated the committed
JSON; surfaced in the lint gate).
* Address claude review on #1662: git-exclude TOCTOU, reproject non-dup, install desc
- git-exclude: guard the install-marker readFileSync with try-catch so a
TOCTOU (marker rewritten atomically mid-read) can't throw through
readSharingMode's documented never-throws contract.
- skill-reproject: reuse core's derived PROJECT_SKILL_EDITOR_IDS instead of a
hand-maintained editor list, so a new skill-surface editor is picked up
automatically.
- install MCP tool: personal scope is implemented; drop the stale 'coming
soon' and describe what it does.
* Address claude review on #1662: shared scope desc + reproject contentHash
- SKILL_SCOPE_DESCRIBE (shared by every MCP verb): personal scope is
implemented; drop 'not available yet' and the inaccurate 'synced across
devices' (it's local per-machine, OF1a sync deferred). Describe it
accurately as a user-level store available in every project on this machine.
- skill-reproject: recompute contentHash on re-projection instead of carrying
the stale install-time hash, so drift detection isn't inverted when the
source was edited between install and a targets change (matches install/move).
* feat(open-knowledge): full-screen skill editor with CodeMirror + live preview
Slice 2 of PRD-6935: editing a skill now opens a full-screen window with a
CodeMirror markdown source pane beside a live preview, replacing the cramped
modal textarea.
Reuse-first, no new infra:
- MarkdownSplitEditor (editor subtree) mounts vanilla CM6 the same way
CodeMirrorPropInput / RawMdxFallbackCMView do, reusing the shared
computeChange minimal-diff helper; the preview renders through core's
markdownToHtml into the editor's .tiptap.ProseMirror prose classes, so it
matches how OK renders the doc on disk.
- SkillIdentityFields extracted from SkillForm so the window header and the
modal create form share the same name/description controls + validation.
- SkillEditorWindow reuses useSkill + useSkillForm (rename via git mv still
carries the edited body in one save).
Wires the manager's edit action to the window and retires SkillEditDialog.
* fix(open-knowledge): reserve macOS traffic-light safe area on the skill editor window
The full-screen skill editor's w-[96vw] DialogContent sits its top-left corner
under the macOS traffic-light buttons (Electron titleBarStyle: hiddenInset),
which would overlap the header's name/description controls. Adopt the shared
safe-area marker (pl-[var(--ok-titlebar-reserve-left,1rem)]) so content clears
the close/minimize/zoom footprint in Electron, with a 1rem web fallback.
Satisfies the fullscreen-overlay-safe-area-coverage invariant.
* fix(open-knowledge): label the skill editor preview region + sync CM aria-label
Address review on MarkdownSplitEditor: give the preview pane a labeled
role="region" (aria-label "Preview") so screen-reader users can tell it apart
from the source editor, and move the CM contenteditable aria-label into a
dedicated sync effect keyed on the prop, matching the sibling
CodeMirrorPropInput idiom instead of setting it once at mount.
* fix(open-knowledge): use semantic <section> for the skill editor preview region
Biome lint/a11y/useSemanticElements rejects role="region" on a div; a
<section> with an accessible name is the implicit region landmark.
* refactor(open-knowledge): address review backlog wave 1 (skill editor PR)
Reuse / type-safety / correctness fixes from the PR review backlog:
- Restore error classification: restoreSkillVersion returns a discriminated
result with a `code`; the route maps git/disk I/O + path-escape to 5xx
instead of collapsing every failure to 404. (#1, #4)
- Path containment: a shadow tree entry resolving outside the skill dir is
refused before the destructive clear. (#13)
- NO_SHADOW_REPO restore error now uses urn:ok:error:shadow-not-configured
instead of the generic invalid-request. (#15)
- SkillInstallRequestSchema.targets typed z.array(SkillTargetEditorSchema)
not z.array(z.string()); install handler matches by value to bridge the
EditorId/SkillTargetEditor widths. (#6)
- Import SkillScope from core (re-exported from skill-target.ts) and
SkillFrontmatter from core instead of redeclaring. (#7, #11)
- Diagnostic warn when the installed-skills marker / skill-targets store is
unreadable, instead of silently returning empty. (#5)
- aria-describedby on the skill name/description inputs. (#16)
- Preview pane uses useDeferredValue so the full markdown pipeline doesn't
run on every keystroke. (#17)
* refactor(open-knowledge): address review backlog wave 2 (reproject + marker)
- resolvedHosts() helper narrows persisted string[] host lists to valid
EditorId[] at the read boundary; replaces the unchecked `as EditorId[]`
casts in skill-reproject + the api-extension install/move/uninstall paths. (#2)
- reprojectAllManagedSkills isolates each skill in its own try/catch so one
skill's projection failure no longer aborts re-projection of the rest after
writeSkillTargets has already committed. (#3)
- recordSkillInstall / removeSkillInstall serialize their read-modify-write
through a per-marker in-process lock, closing the race where two concurrent
installs of different skills could clobber each other's marker entry. (#9)
* test(open-knowledge): skill-editor review backlog wave 3 (tests + style nits)
Tests (#8, #14, #18):
- skill-install.test.ts: rename of an installed skill carries its install-state
(hosts + marker entry) to the new name and drops the old. (#8)
- skill-target.test.ts: MCP skill verb tools (write/delete/move) return the
not-running error with no server URL, and reject an invalid name with the
teaching error before any network call. (#14)
- MarkdownSplitEditor.dom.test.tsx: real CM mount + preview render through
markdownToHtml + external-value reconciliation + labeled preview region. (#18)
Style nits (#10, #12):
- SKILL_NAME_REGEX is now a single core export imported by the server write
validator and the MCP verb schema instead of two redeclared literals. (#10)
- Documented why the install-state schemas use .loose() (the producer writes a
`surface` field beyond the client contract; strict would reject the response)
rather than flipping to strict and breaking it. (#12)
This clears the full 18-item review backlog (13 fixes in waves 1-2 + these 5).
* feat(open-knowledge): WYSIWYG skill editor reusing the real editor surface
Replace the modal plus CodeMirror split editor for skills with a full-screen
surface that mounts OK's real WYSIWYG editor (the same sharedExtensions and
.tiptap theming as the document editor) on a standalone non-CRDT doc seeded
from and saved to the skill REST API. Skills stay non-CRDT (D1/D6): no
Hocuspocus pipeline, no shared editor-cache/mount-promise singletons, so the
document editor is untouched.
- skill-local-doc: seed a local Y.Doc from markdown and serialize back via
MarkdownManager plus @tiptap/y-tiptap; round-trip is lossless versus the
markdown pipeline's own canonical output (10 tests)
- SkillBodyEditor: lightweight TipTap mount on the seeded doc, EditorContent
portaled per the H6 no-unportaled-editor-content contract
- SkillEditorWindow: full-screen (not modal), reuses SkillIdentityFields,
useSkillForm, and SkillBodyEditor
- retire MarkdownSplitEditor (the bespoke source plus preview split)
Honors the locked spec D13b (full-screen, not modal) and R11 (real editor
plus frontmatter fields) without crossing the .ok/ CRDT boundary.
* feat(open-knowledge): render skill editor in the main pane like a document
Editing a skill now opens in the main editor pane with the file sidebar and
app chrome still visible, instead of a window-covering overlay. The body uses
the real editor's centered content column with no bordered box, so name,
description, and prose share one document column matching the document editor.
- skill-editor-store: a module store (useSyncExternalStore) hands the selected
skill from Settings to EditorPane, which renders the editor absolute inset-0
over the main pane; Settings closes on open so the sidebar stays visible
- SkillBodyEditor: portal EditorContent with tiptap-editor-portal-content so it
lands in the .tiptap-editor content-column grid, matching the doc editor;
vertical padding only (the grid owns the horizontal column)
- SkillEditorWindow: in-pane absolute overlay instead of a body portal; align
the identity fields to the editor content column via editor-content-aligned;
clear stranded body pointer-events left by the Radix Settings dialog close,
which was making the editor unclickable and hiding the caret
Verified in the running app: the sidebar stays, the real WYSIWYG renders with
theming and block handles, and the cursor and editing work.
* feat(open-knowledge): match the document Properties panel for skill name and description
The skill editor's name and description now render as the document editor's
"Properties" section (collapsible header, type-icon + label column + inline
value rows), so editing a skill matches editing a document.
Presentational only and additive: a new SkillProperties component reuses the
visual chrome (property-panel, Collapsible, the FrontmatterRow row layout) but
keeps the controlled Input/Textarea that commit on every keystroke, plus the
existing validation and rename-on-save behavior from useSkillForm. It does not
touch the doc editor's PropertyPanel, DocumentContext, PropertyContext, or any
CRDT binding, and NewSkillDialog keeps using SkillIdentityFields unchanged.
Keeping the controlled inputs (not the doc panel's commit-on-blur widget) means
the explicit Save button can never capture a stale value.
* refactor(open-knowledge): share one Properties disclosure between the doc and skill panels
Extract the "Properties" collapsible section header (container + chevron +
title + content) into a PropertyDisclosure component used by both the document
PropertyPanel and the skill editor's SkillProperties, so the skill panel no
longer copies the doc panel's header markup and class strings.
PropertyPanel keeps owning its collapsed state (the toolbar "Add property"
signal still force-expands it) via the component's optional controlled open
prop; SkillProperties uses it uncontrolled. No behavior change: PropertyPanel's
26 tests pass and it renders identically, and the skill editor is visually
unchanged.
The full FrontmatterRow is intentionally NOT shared with the skill rows: it is
coupled to the CRDT value widget and commit-on-blur, so the skill rows keep
controlled inputs (no stale-value-on-Save race) with a small local row helper.
* feat(open-knowledge): keep skill editor open on save + add Install to its header
Saving a skill no longer closes the editor; it stays open with the saved
content so you can keep editing, and on rename it re-points to the moved skill
instead of trying to reopen the gone one. The editor header now carries the
install controls that previously lived only in the Settings list: a
Draft/Installed badge plus Install / Reinstall / Uninstall, reusing
installSkill/uninstallSkill (no new install logic).
- useSkillForm.onCommitted reports the saved name so the parent re-points a
renamed skill instead of closing the editor
- EditorPane owns install/uninstall (reuses the skills-api client and updates
the skill-editor store so the open editor reflects the new install state)
- SkillEditorWindow header: state badge + Install/Reinstall + Uninstall + Save
Verified in the running app: Save keeps the editor open with edits intact;
Install flips Draft to Installed and surfaces Uninstall/Reinstall.
* feat(open-knowledge): browse skills from a sidebar section
Add a "Skills" section to the file sidebar so skills are browsable and
selectable like documents, grouped by scope (Project/Personal) with an
install-state badge per row and a "+" to create. Clicking a skill opens it in
the main editor pane via the existing skill-editor store, so you can switch
between skills without the Settings detour. Settings -> Skills stays for config
(which editors to install into).
Shared, non-duplicated building blocks (used by the sidebar section, the
Settings list, the list row, and the editor header):
- lib/skill-scope.ts: SKILL_SCOPE_ORDER + skillNameSetsByScope (create-dialog
collision sets) + useSkillScopeLabels (scope titles)
- SkillStateBadge: the Installed/Draft badge
Reuses NewSkillDialog for create and the sidebar primitives for the rows; no
new install or form logic. SkillsManagerSection and SkillRow now consume the
shared helpers/badge instead of their own copies.
* refactor(open-knowledge): cohesive skill sidebar rows + Save/Install header
Sidebar Skills rows now match the file tree's visual language: a leading icon
plus the same sans font and natural-case name (was monospace), normal-case
scope labels (was uppercase), and a quiet muted Draft/Installed label instead
of a loud filled pill (new `subtle` variant on the shared SkillStateBadge; the
Settings list and editor header keep the filled badge).
Editor header is now Save then Install (Save first), Cancel removed — closing is
Escape or selecting another item. Uninstall stays as a quiet ghost only when the
skill is installed.
* fix(open-knowledge): close skill editor on doc open + consistent header button case
Opening a document now dismisses the skill editor overlay so the doc shows
(EditorPane closes the skill-editor store when activeDocName changes — which
only happens on a real doc navigation, not when opening a skill or closing
Settings). Previously the overlay stayed and the clicked file never appeared.
Make the editor header buttons consistent: Install/Reinstall/Uninstall now use
the same font-mono uppercase as the primary Save button (the default Button
variant is uppercase; the outline/ghost variants were not), so it reads SAVE /
INSTALL instead of SAVE / Install.
* fix(open-knowledge): close skill editor when re-selecting the active file
Re-clicking the already-active document row left the skill editor overlay up:
the tree's already-selected-row click handler early-returns when the URL hash
already matches the doc (skills aren't hash-routed), so it never fired. Now both
the tree activation path and that already-selected handler dismiss the skill
editor, so clicking the file you were on always returns you to it.
* feat(open-knowledge): skill editor Visual/Markdown toggle, Open with AI, clickable header
Add a Visual <-> Markdown mode toggle to the skill editor (centered, like the
document editor), an Open with AI menu in the header (same spot as docs), and
fix the header buttons not being clickable in the desktop app.
- EditorModeToggle: extracted the Visual/Markdown segmented control from
EditorToolbar so the doc toolbar and the skill editor share one toggle
- SkillSourceEditor: a themed CodeMirror markdown view reusing the document
source editor's createNestedCMExtensions + computeChange; non-CRDT, so the
body lives in the form and the WYSIWYG re-seeds from it on switch back
- Open with AI: reuses OpenInAgentMenu + buildSkillHandoffInput (the same wiring
the Settings skill row uses)
- header clickability: -webkit-app-region is inherited and the header sits in
the Electron title-bar drag zone, so its buttons weren't clickable; mark the
header draggable and opt the buttons out, mirroring EditorHeader
Verified in the running app: the toggle round-trips WYSIWYG to markdown source
and back; Open with AI is present; install button matches Save's casing.
* feat(open-knowledge): make skills URL-addressable + skill previewUrl
Add a skill hash route (#/__skill__/<scope>/<name>) so the skill editor is reload-stable, shareable, and deep-linkable like a document. Opening a skill updates the URL; loading a skill URL opens straight into the editor with the sidebar visible; closing routes back to the document underneath. The write and edit MCP tools now return a route-only previewUrl for skills (the same envelope documents carry), and preview_url accepts a { skill } target, so an agent that authors a skill can hand back a URL that opens it in the running editor.
* fix(open-knowledge): guard skill-editor Escape and add a visible close button
Address PR review. The document-level Escape listener now skips events already handled inside the editor: TipTap's slash menu and CodeMirror's search panel call preventDefault without stopPropagation, so dismissing one of those popups was also tearing down the whole skill editor and discarding edits. The handler now ignores a defaultPrevented Escape. Also add a visible close (X) button to the header so Escape is no longer the only keyboard dismiss path.
* fix(open-knowledge): green CI + address skill-editor review recommendations
Fix the two failing checks: extract the new lingui strings (Close skill editor, skill description helper) so the i18n catalog is in sync (lint), and stub SkillsSidebarSection in FileSidebar.menu-action.dom.test so its sidebar imports don't need mocking (test:dom). Address review recommendations: focus the editor surface on open for keyboard users; gate SkillBodyEditor serialization on transaction.docChanged so cursor-only updates don't serialize; wire aria-describedby on the create-dialog description input; add dom tests for the Escape defaultPrevented guard, the close button, and the CodeMirror source view.
* feat(open-knowledge): skill 3-dot actions in the sidebar + consistent editor header
Add a 3-dot action menu (Edit / Install / Uninstall / History / Delete) to the file-sidebar Skills rows, matching the file-row pattern, so a skill can be deleted and managed without opening Settings. Extract a shared useSkillActions hook (install/uninstall side effects plus the delete/history dialogs) and a SkillActionMenuItems component, both reused by the Settings list, the Settings row 3-dot menu, and the new sidebar menu, so the action surface is identical everywhere and the Settings code drops its duplicated handlers. Re-theme the skill editor header to match the document editor's EditorHeader (same height, muted background, inset bottom border) so a skill and a document read as the same surface.
* feat(open-knowledge): full file-style context menu for sidebar skills
Give the file-sidebar skill rows the same context menu a file row has: Reveal in Finder, Open with AI, Open in Terminal, Copy Path, Duplicate, Rename, and Delete, with Install/Uninstall in place of Hide. Reuses the file menu's own primitives (the desktop bridge showItemInFolder, dispatchOpenInTerminal, OpenInAgentContextSubmenu, the clipboard adapter) plus moveSkill and useSkillActions; Duplicate is a client-side compose (GET source + PUT a <name>-copy) and Rename is a thin moveSkill dialog. The /api/skills entries now carry the skill's absolute SKILL.md path so the desktop-native rows can resolve a target; the path-based rows hide on the web host and on partial entries, matching the file menu.
* feat(open-knowledge): float skill editor mode toggle + always-on Copy Path
Move the skill editor's Visual/Markdown toggle out of the header into a floating row centered over the content, matching the document editor's toolbar placement so the two read as the same surface. Make the sidebar skill menu's Copy Path always render (Relative Path needs no host; Full Path appears when the server-supplied absolute path is present), mirroring the file menu which shows Copy Path on every host.
* fix(open-knowledge): refresh the skills list after a starter pack installs its skill
A starter pack authors its pack skill into .ok/skills/ server-side (installPackSkill), but the seed-apply path never emitted skills-changed, so an already-mounted Skills list (the file-sidebar section) kept showing its startup snapshot — 'No skills yet' — until a reload or until Settings mounted a fresh useSkills. Emit skills-changed on a successful apply, centralized in seedClient so the IPC and HTTP transports and every caller get it.
* feat(open-knowledge): skill editor header matches the document editor (tab + Share + app controls)
Rebuild the skill editor header to mirror EditorHeader: the sidebar (hide-dock) toggle and a real document-style tab for the skill (name + state badge + close) on the left; Open with AI, project Share (content-root share + publish-to-GitHub fallback), Save, Install/Uninstall, then the shared Settings and Help controls on the right. Reuses SidebarTrigger, ShareButton, PublishToGitHubDialog, SettingsButton, HelpPopover, and buildFolderShareInput. No presence/sync row — a skill is single-author, not a collaborative document.
* fix(open-knowledge): skill editor toolbar dock + tab merge + source-mode scroll
Move Save/Install out of the header into the floating toolbar as subtle ghost buttons, and add a History (timeline) icon to that right-aligned dock; History is removed from the sidebar 3-dot menu accordingly (Settings keeps its own). The skill tab now carries z-10 so it merges into the body like a document tab. Fix the Markdown (source) editor never scrolling: an absolute wrapper's computed height is auto, so CodeMirror's height:100% grew to the full document; make .cm-editor a flex child of a flex-column wrapper so the scroller is bounded.
* feat(open-knowledge): autosave the skill editor, drop the Save button
Skills now persist like documents: editing the body or description debounce-autosaves (silent), with an inline Saving/Saved status where the Save button was. A name change is a rename (git mv), so it commits on the name field's blur/Enter rather than per-keystroke; autosave pauses while a rename is pending. Invalid or in-flight states are skipped so a half-typed field never persists. The create dialog keeps its explicit submit (non-silent, toasts). useSkillForm gains a silent submit option, a saveStatus, and commitName.
* Open the skill editor directly for new skills instead of a modal
The New skill action now drops a draft into the main editor pane with an
inline Project/Personal scope toggle in its Properties panel, rather than
opening the NewSkillDialog modal. Filling in a name and description enables
Create; on commit the editor re-points to the saved skill and switches into
the existing autosave edit experience, so creating and editing a skill are
one continuous surface.
Parameterizes EditForm for create vs edit, adds openDraft/isDraftSkill to the
skill-editor store, and routes the sidebar and Settings New skill buttons to
openDraft. Removes the now-orphaned NewSkillDialog, SkillFormFields, and
SkillIdentityFields.
* Hoist the shared scopeControl out of both skill body layouts
* Allow reinstalling open-knowledge-pack-* starter skills
Pack skills are named open-knowledge-pack-<packId>, under the open-knowledge*
prefix reserved for OK's shipped skills. The seed installs them by copying
directly, but a user-triggered reinstall re-validates and was rejected by the
reserved-prefix guard. Exempt the open-knowledge-pack-* namespace from that
guard since pack skills are OK's own shipped content.
* Tighten the skill editor top and give its fields visible borders
The editor led with a thin Properties disclosure sitting below a tall empty
band, which read as barren next to the document editor's title-anchored top.
Pull the Properties panel up under the tab (it fills the floating toolbar's
empty left cell) and give the scope/name/description fields a visible border
so the form reads as substantial rather than empty.
* Route the skill editor install/uninstall through the shared hook
EditorPane re-implemented install/uninstall instead of using useSkillActions,
and the copy had drifted: it dropped the server warnings (e.g. the OF5 scripts
trust warning) that the sidebar/Settings path surfaces. Make the shared hook's
install/uninstall return their result so the editor reuses the one canonical
toast/warning flow and only adds the open-skill store update on top. Also thread
the in-flight indicator through uninstall so the list surfaces show it too.
* Make skill history/restore project-scope explicit + tighten preview_url name
* Reuse core editor-root map in the seed + centralize the projection symlink guard
* Address pending PR review recommendations for the skill editor
- Add create-mode (draft skill) behavioral test coverage to
SkillEditorWindow.dom.test (the deleted NewSkillDialog test was never
migrated). Wires DocumentFragment into the jsdom preload so the scope Radix
Select renders in the dom tier.
- Fix the editor section aria-label: New skill for a draft instead of the
empty Edit skill.
- Associate the rename dialog error with its input via aria-describedby.
- Park a rename requested during an in-flight autosave and flush it when the
save settles, so commitName no longer silently drops it.
* Seamlessly merge the skill editor tab into the body
The tab was vertically centered in the h-12 header, so its bottom floated
above the header's bottom border and the border showed as a line under the
tab. Wrap it in a full-height items-end strip (like EditorTabs) so the h-10
tab bottom-aligns to the header edge and its -mb-px overlaps the tab bg into
the body, hiding the border — matching the document editor tab.
* Fix Skill not found after creating a personal-scope skill
The create flow's onSaved only carried the skill name, so EditorPane reopened
it using the draft's scope (always project). A skill created with Personal
scope was written to the personal store but reopened reading project scope,
so the follow-up load 404'd with Skill not found. Thread the scope the form
actually saved to through onSaved so the reopen reads the right store.
* Keep the skill editor open on cold URL load when a doc resolves underneath
In the preview browser, opening a skill deep-link briefly showed the skill
then auto-closed to the empty/home view: a default doc (base canvas) resolves
after mount, changing activeDocName, and the close-on-nav effect stomped the
skill. Guard that effect on the hash still being a skill route — the hash is
the skill editor's source of truth, so an underlying-doc resolution is not a
real nav away. A genuine doc nav updates the hash first and SkillHashSync
handles the close.
* Skills sidebar: hexagon icon, collapsible section, project<->personal move
- Give the Skills sidebar header a hexagon icon and make the whole section
collapsible like the Files tree (click Skills to toggle); Personal/Project
stay as sections inside. The + (New skill) stays an always-visible action.
- Rename the project scope label This project -> Project.
- Let the edit-mode Properties scope dropdown move a skill across stores
(project<->personal). Composed from the existing read+write+delete endpoints
(delete also uninstalls the old scope's projections); refuses to overwrite a
same-named destination skill, and re-points the editor to the moved skill.
* Compact the Skills sidebar rows + default the section collapsed
Drop the per-row sparkle icon, shrink the rows below the Skills group title
(h-6, text-[11px]) and tighten the scope subheaders, and default the section
to collapsed so it doesn't crowd the file tree until opened.
* Reset install state when moving a skill across scopes
After a scope move the editor re-pointed by spreading the old (installed)
skill entry, so the toolbar kept showing Reinstall/Uninstall even though the
moved skill is a Draft in its new scope. Thread installed/hosts through onSaved
so the move re-points to a Draft (Install button); rename still keeps the
current install state by omitting them.
* Keep the skills list live across clients via the CC1 files signal
useSkills/useSkillHistory only listened to the same-window skills-changed
event, so a skill created/deleted/renamed in one client (e.g. the preview
browser) didn't update another client (the desktop app) until reload. Every
skill handler already broadcasts the CC1 'files' signal, which reaches other
clients as emitDocumentsChanged(['files']) — subscribe the skill hooks to it
too so the list stays current everywhere. useSkill (open editor detail) stays
local-only to avoid clobbering in-progress edits on a remote change.
* Localize the skill move-scope toast label
The move-scope success toast interpolated the raw SkillScope value
(project/personal) instead of the localized label from useSkillScopeLabels —
the one pending PR review recommendation. Use the label.
* docs(open-knowledge): OK skill import spec + amend PRD-6935 to CRDT skills/templates
Add specs/2026-06-08-ok-skill-import: round-trip adopt of existing
Cursor/Codex/Claude skills into .ok/skills (one-shot adopt, personal +
project, UI+API), built on the shared Agent Skills SKILL.md standard.
Amend specs/2026-06-04-ok-skill-editor: skills and templates become
CRDT-backed documents (D21-D28), reversing the non-CRDT pivot
(supersedes D1/D6/D18/NG1). Resolves OF8-OF11; surfaces OF12 (the
per-site isManagedArtifactDoc gate triage). Blocks PR #1662 for the
rework. Both specs cold-reader audited (factual + soundness).
* feat(open-knowledge): Slice A — CRDT managed-artifact doc class + persistence
Skills/templates become CRDT-backed docs persisted to .ok/ (PRD-6935 CRDT
amendment, D21/D27/D28). Adds the managed-artifact doc class
(__skill__/__template__ synthetic names), a third persistence branch that
serializes Y.Text('source') verbatim (precedent #38), the path resolver with
slug+containment guards, tree-exclusion, and the loopback admission guard.
Observer bridge intentionally left enabled for these docs (WYSIWYG). 11 tests.
* feat(open-knowledge): Slice B — exclude managed-artifact docs from content indexers (OF12)
Flip the tree/index SKIP gates (backlink-index, tag-index, live-derived-index,
removal-redirect-guard, file-watcher, server-factory enumeration loops) from
isSystemDoc||isConfigDoc to isReservedForUserTree so live skill/template docs
aren't indexed as content. RUN sites (agent-sessions throw, client provider
pool) already let managed-artifact through (neither system nor config), so the
write path + editor binding work with no change.
* test(open-knowledge): end-to-end proof of CRDT skill doc (Slice A+B)
Through the real server: a __skill__/<scope>/<name> doc is admitted, the
observer bridge runs (full doc, not Y.Text-only), edits persist verbatim to
.ok/skills/<name>/SKILL.md, and a fresh client loads the persisted source.
Validates admission + third persistence branch + gate exclusion over the wire.
* feat(open-knowledge): Slice A watcher — reconcile external .ok/skills edits
Add an explicit chokidar watch over the project + personal skills roots
(<projectDir>/.ok/skills, ~/.ok/skills) routing SKILL.md disk changes to
applyExternalManagedArtifactChange against the live __skill__ doc. .ok/ is
excluded from the content file-watcher by default, so this is the only
disk to doc reconcile path for skills (plan Risk #4).
- managed-artifact-watcher.ts: depth-1 SKILL.md watcher (new), mirrors the
config-file-watcher polling/awaitWriteFinish contract.
- managed-artifact-persistence.ts: managedArtifactDocNameForPath reverse
resolver + managedArtifactSkillsRoots; applyExternalManagedArtifactChange
now takes a nullable doc and short-circuits self-writes via LKG equality
(mirrors applyExternalConfigChange) returning an outcome.
- persistence.ts: expose managedArtifactCtx on PersistenceHandle.
- server-factory.ts: start the skills watcher after the config watchers
(skipped for ephemeral servers); register cleanup.
- tests: reverse-resolver + roots + null/self-write no-op units, a watcher
unit, and an end-to-end disk-edit-reconciles-into-open-doc integration test.
* chore(open-knowledge): Slice A/B cleanup — strip spec-decision markers, regen md-audit registry, test tags
Removes D-number spec-decision markers from source comments (comment-discipline),
regenerates the byte-preservation-audit registry (pre-existing drift on base),
and adds @covers tags to the managed-artifact integration test.
* feat(open-knowledge): Slice C — route skill-put body through CRDT doc
The PUT /api/skill body write (and MCP write/edit({skill}), which delegate
to it) now mutates the __skill__/<scope>/<name> CRDT doc instead of writing
SKILL.md straight to disk. The managed-artifact persistence branch (Slice A)
serializes Y.Text verbatim to .ok/skills/<name>/SKILL.md.
- skills-write.ts: extract composeSkillContent (pure validate + serialize,
no fs); applySkillWrite reuses it. One frontmatter-composition core shared
by the fs writer and the CRDT path (R3 purity preserved).
- api-extension.ts handleSkillPut: open an agent session, write the full
SKILL.md via composeAndWriteRawBody under session.dc.document.transact(fn,
session.origin) (precedent #24/#38), force the store, then attribute +
shadow-commit (project scope). Response path/created/warnings unchanged.
- handleSkillDelete / handleSkillMove: tear down the live __skill__ doc via
the shared captureAndCloseDocuments spine BEFORE removing/relocating the
dir, so persistence can't resurrect a deleted/moved skill on a later store.
(skill-restore is covered by the Slice A watcher reconciling the open doc.)
- tests: Slice C proof that an HTTP PUT mutates the SAME Y.Doc an editor
client is bound to; existing skill CRUD/install/attribution suites green.
* feat(open-knowledge): Slice D — skill editor binds the real CRDT doc
The skill editor now edits a live CRDT doc (__skill__/<scope>/<name>) through
OK's real document editor instead of a local non-collaborative Y.Doc behind an
autosave to PUT loop. Body + description are CRDT mutations the server persists
verbatim to .ok/skills/<name>/SKILL.md — no debounced save, real-time collab.
The payoff is deletion: the bespoke skill-editor stack exists only because
skills were not real docs. Removed skill-local-doc.ts, SkillBodyEditor.tsx,
SkillSourceEditor.tsx, the useSkillForm autosave/rename machinery (SkillForm.tsx),
the source/WYSIWYG reparse dance, and the now-dead useSkill GET-load hook.
- use-skill-doc-provider.ts (new): standalone HocuspocusProvider for the skill
doc, modeled on config-provider.tsx (synthetic docs own their provider, not
the file-tree ProviderPool). Re-keyed on server epoch.
- SkillDocEditor.tsx (new): reuses TiptapEditor / SourceEditor (lazy, matching
the pool's bundle-split boundary) / DocumentBoundary unchanged; source-default
for verbatim projection fidelity.
- SkillProperties.tsx: description binds live via bindFrontmatterDoc (same as
PropertyPanel); name commits a git-mv rename, never a frontmatter patch.
- SkillEditorWindow.tsx: edit mode mounts the provider behind error-boundary +
Suspense; create mode scaffolds the doc on name commit (no orphan folders),
then re-points into live editing. Header/install/history/scope chrome kept.
- Tests: SkillProperties.dom (CRDT description round-trip + rename) and a
rewritten SkillEditorWindow.dom (window flows); old body-editor tests deleted.
Full DOM suite + app unit suite green; knip + i18n catalogs clean.
* feat(open-knowledge): Slice E (server) — templates on the CRDT spine
Templates become folder-addressed CRDT docs. Unlike skills (project/personal
scope), a template lives at <folder>/.ok/templates/<name>.md, so the managed-
artifact grammar now branches by kind.
- core/cc1.ts: ParsedManagedArtifactName is a kind-discriminated union —
skill keeps { scope }, template gets { folder }. parseManagedArtifactName
splits __template__/<folderRel>/<name> on the last slash (folderRel may be
empty/nested). TEMPLATE_NAME_REGEX added to core (shared, dedupes the
templates-write literal).
- managed-artifact-persistence.ts: managedArtifactAbsPath resolves the
template branch to <projectDir>/<folder>/.ok/templates/<name>.md with name +
folder-escape + containment guards. Reverse resolver stays skills-only (a
per-folder template disk-watch is deferred — recursive project-wide watch).
- templates-write.ts: extract composeTemplateContent (pure validate + serialize,
no fs), shared by applyTemplateWrite and the handler — mirrors composeSkillContent.
- api-extension.ts: PUT /api/template body routes through the template CRDT doc
via session.dc.document.transact(..., session.origin) + composeAndWriteRawBody;
shared templateDocNameFor helper; delete + move tear the doc down via
captureAndCloseDocuments so persistence can't resurrect a removed/moved template.
- Templates are already tree-excluded via isManagedArtifactDoc (both prefixes).
- tests: template path-resolution + escape units; end-to-end folder-addressed
persistence + PUT-through-open-doc integration. Core (3397) + server writes green.
Remaining: the app half (TemplateEditorWindow + #/__template__/ hash routing +
delete the modal), mirroring Slice D.
* feat(open-knowledge): Slice E (app) — template editor on the real CRDT doc
Editing a template now uses OK's real document editor bound to its CRDT doc
(__template__/<folder>/<name>) instead of the modal textarea. Title +
description + body are live CRDT mutations the server persists to
<folder>/.ok/templates/<name>.md. Deletes the TemplateEditDialog modal.
Reuse over duplication (per the same spine Slice D built):
- use-managed-artifact-provider.ts (renamed from use-skill-doc-provider):
one kind-agnostic provider hook; skill/template are thin wrappers +
skillDocName/templateDocName builders.
- ManagedArtifactDocEditor.tsx: the editor shell (DocumentBoundary + lazy
SourceEditor + TiptapEditor + a caller-supplied properties panel), extracted
from the old SkillDocEditor so skill + template share it.
- ManagedArtifactEditorHeader.tsx: shared header chrome (was inline in
SkillEditorWindow).
- use-frontmatter-field.ts: shared bindFrontmatterDoc field hook; SkillProperties
(description) + TemplateProperties (title + description) both use it.
- TemplateProperties.tsx: title + description bound live; name commits a
moveTemplate rename; folder read-only.
- TemplateEditorWindow.tsx: edit-only full-screen overlay (portaled to <body>),
mode toggle + the shared editor; rename re-points in place. Create stays in
NewTemplateDialog.
- TemplatesCard + settings/TemplatesManagerSection swap the modal for the window;
dead useTemplate/TemplateDetail removed.
- tests: TemplateProperties.dom (title/description CRDT round-trip + rename),
TemplateEditorWindow.dom (render + escape/close); Skill suites updated for the
generalization. Full DOM (553) + app unit (5052) + knip + i18n green.
KNOWN FOLLOW-UP: the Settings-launched overlay's Radix focus interaction needs a
manual/e2e check (skills solve this via an app-level editor store; templates use
a body portal here).
* chore(open-knowledge): regen md-audit registry for CRDT artifact test coverage
The skill + template CRDT integration tests added in Slices A/C/E carry
@covers-surface / @covers-construct tags; regenerate byte-preservation-audit
so md-audit:check matches the codebase.
* feat(open-knowledge): address Slice E follow-ups — template store, hash routing, disk watch
Resolves the three follow-ups from the template CRDT work.
(1)+(3) Template editor store + hash routing. The template editor now renders
in the MAIN editor pane via a module store (mirrors skill-editor-store), driven
by `#/__template__/<folder>/<name>` URL hashes — exactly like skills. This
retires the body-portal overlay (and its Radix focus-trap/dismiss risk when
launched from Settings): opening a template writes the template hash, which
closes the Settings dialog naturally, and the editor lives above it in
EditorPane. Templates are now URL-addressable + reload-stable.
- template-editor-store.ts + TemplateHashSync (App) + EditorPane render.
- doc-hash.ts: templateFromHash / hashFromTemplate (folder-addressed) + the
docName/nav guards skip template hashes.
- TemplatesCard + settings/TemplatesManagerSection call templateEditorStore.open;
TemplateEditorWindow renders absolute-inset-0 (no portal); rename re-points
through the store. Round-trip hash tests added.
(2) Template disk→doc reconcile watcher. The managed-artifact watcher is
generalized (depth + leaf-matcher options); a template watcher reconciles
external/CLI/cross-instance edits to `<folder>/.ok/templates/*.md` into the open
doc, sharing the skill reconcile closure. Roots enumerated at boot via
resolveProjectTemplates (+ always the project root); a template in a brand-new
folder mid-session reconciles after restart (bounded, documented).
- managedArtifactDocNameForPath gains a template branch; reverse-resolver +
watcher units + an end-to-end root-template reconcile integration test.
Validated: server units + managed-artifact integration (7), app unit (5056) +
full DOM (553), doc-hash (53), knip, lint, i18n, md-audit all green.
* feat(open-knowledge): version editor edits of skills/templates like any doc
Editor-typed CRDT edits to skills + templates now create attributed shadow-repo
versions exactly like a regular document edit — the missing piece for seamless
parity. Before, only the MCP write/edit path (which calls commitOkArtifactWrite)
versioned; the managed-artifact persistence branch wrote the file and returned
before scheduleGitCommit, so typing in the editor persisted but left no
attributed version in skill history / the folder timeline.
The fix mirrors storeDocumentNow's browser-write safety-net: on a real
managed-artifact store ('persisted'), resolve the writer from the transaction
origin and record + scheduleGitCommit. New managedArtifactContributorAttribution
maps the synthetic doc name to the timeline's contract — the artifact key
(.ok/skills/<name>, <folder>/.ok/templates/<name>) + a skill-/template- subject —
so the version surfaces in getSkillHistory / getFolderTimeline.
Single, non-double versioning path, consistent with docs:
- Editor/principal edits (connection origin) version at the store (debounced
scheduleGitCommit, exactly like a doc editor edit).
- MCP write/edit (agent sessions) stay attributed by the api-extension handler
(rich actor + summary + immediate commit, the .ok/ analogue of flushDocToGit);
the store skips agent origins to avoid double-attribution.
- Personal skills are unversioned (no project shadow) → attribution returns null.
Tests: managedArtifactContributorAttribution units (skill project/personal,
template root/nested); skill-restore + persistence-fan-out + skill/template
envelopes stay green (the commit machinery is unchanged).
* fix(open-knowledge): wrap skill/template editor overlays in OpenInAgentMenuRequestProvider
The skill + template editor overlays rendered as siblings AFTER
OpenInAgentMenuRequestProvider closed, so their header's OpenInAgentMenu — and
SourceEditor, which skills default to — crashed with 'useOpenInAgentMenuRequest
must be used within <OpenInAgentMenuRequestProvider />'. Move the provider's
closing tag past both overlays so they share the same context the document
editor uses.
Audited the rest of the shared editor stack: the only editor-area-scoped context
the overlays consume is useOpenInAgentMenuRequest (header menu + SourceEditor);
PropertyProvider/useProperties is doc-PropertyPanel-only (overlays use their own
frontmatter panels), and DocumentContext/config/theme/identity are app-level.
So this is the one boundary that needed fixing for skills/templates to behave
like docs. The DOM test mocked OpenInAgentMenu, which is why it slipped.
* Edit skills and templates as first-class editor tabs
Skills and templates open as normal editor tabs through the same pipeline
documents use, replacing the bespoke full-screen overlays. They get the
file sidebar, tab bar, property panel, and version history; skills also
carry install/reinstall/uninstall plus history in the per-document toolbar.
Skill and template tabs resolve as real documents everywhere (navigation,
hash round-trip, graph/links), and a doc that links to a skill or template
by its .ok/skills or .ok/templates file path now opens the artifact editor
instead of offering to create a missing page.
Deletes the overlay, module-store, and standalone-provider stack.
* Index skills and templates in the link graph like documents
Managed-artifact docs (skills/templates) now participate in the link,
backlink, and graph index keyed by their __skill__/__template__ identity,
while staying excluded from the file tree. A new isLinkIndexExcludedDoc
predicate (system + config only) replaces isReservedForUserTree at the
index gates in backlink-index and live-derived-index.
A doc that links to a skill or template by its .ok/skills or .ok/templates
file path normalizes to the artifact identity, so the edge connects to the
entity (backlinks both directions) instead of a dangling file path. The
normalizer lives in core and is shared by the client link resolver and the
server link index.
Editing a skill or template re-derives its own outgoing links via the live
path. Tree-display exclusion (content-filter) is unchanged.
* Lazy-load skill/template editor chrome; raise app size limit
The skill/template identity panel (ManagedArtifactProperties) and skill
install/history chrome (SkillEditorActions) now lazy-load — they only mount
for managed-artifact docs, so they stay out of the eager editor/toolbar
bundle every document loads.
Raise the main-app-bundle size-limit 410->450 kB: merging main plus the
skill/template editor feature pushed it over, and the residual is the
pre-existing eager PropertyPanel tech-debt (date-fns + dnd-kit) already
tracked for lazy-loading.
* test(open-knowledge): stub SkillsSidebarSection in FileSidebar dom test
The sidebar's skills section pulls in skill-actions (dropdown submenu +
handoff builders), whose deep graph the partial dropdown-menu/useHandoffDispatch
mocks didn't cover. Stub it like the other heavy sidebar children so the
sidebar's own behavior tests don't depend on the skills subtree.
* chore(open-knowledge): regenerate byte-preservation audit after main merge
Generated doc; merged registry data + main's pipeline produce content that
differs from either pre-merge side. Regenerated via md-audit:generate.
* chore(open-knowledge): regenerate ng-anchors catalog after main merge
Main's markdown-pipeline changes shifted the NG anchor enumeration; rebuild the
committed catalog so the freshness gate matches a fresh build.
* fix(open-knowledge): index managed-artifact tags on the link axis
tag-index gated on isReservedForUserTree (tree axis), excluding skills/templates
from tag indexing — while the backlink and live-derived indexes already moved to
isLinkIndexExcludedDoc (link axis = system + config only). That split was applied
inconsistently: live-derived-index still called tagIndex.updateDocumentFromMarkdown
and signalled the tags channel on managed-artifact edits, so every skill/template
edit did wasted work the tag index then dropped.
Align tag-index to the link axis so managed-artifact docs participate in tags like
they do in backlinks, and re-export isLinkIndexExcludedDoc alongside its peer
predicates. Adds a regression test pinning the two-axis split.
* test(open-knowledge): widen managed-artifact watcher deadlines for CI
The two startManagedArtifactWatcher tests timed out at exactly 4020ms in CI
(Linux) while passing on macOS. The watcher uses chokidar usePolling:true, so
detection latency tracks the poll interval + awaitWriteFinish window; under a
saturated runner (whole server suite on one event loop) those polls stretch
past a 4s deadline. Widen the eventually() deadline to 20s and set explicit 25s
per-test Bun timeouts so this stays a correctness check, not a load race.
* test(open-knowledge): cover link-index predicate + managed-artifact normalizer
Adds the two unit tests review flagged as missing:
- isLinkIndexExcludedDoc (cc1-broadcast.test.ts): system+config excluded,
managed-artifact + ordinary docs admitted — pins the two-axis split.
- managedArtifactDocNameFromContentTarget (new core cc1.test.ts): skill/template
file-path → managed-artifact doc name, with null for non-artifact paths.
Also clarifies the documentName-gate STOP rule in AGENTS.md: link/graph/tag
indexes use isLinkIndexExcludedDoc (admits managed-artifact docs), distinct from
the tree-axis isReservedForUserTree.
* fix(open-knowledge): admit managed-artifact docs in link-graph endpoints
collectAdmittedDocNames() only walked getFileIndex() (tree docs), so links to
skills/templates fell outside the admitted set — the link/graph/backlink panels
treated them as out-of-scope and dropped their frontmatter metadata. This is the
read-side counterpart to indexing managed-artifact docs on the link axis.
Enumerate skills (resolveSkillsList over both scope roots) and templates
(resolveProjectTemplates) and add their managed-artifact doc names, matching the
form the backlink index normalizes link targets to. Best-effort + try/catch so a
scan failure narrows the set rather than failing the endpoint. projectDir
defaults to contentDir, so the scan roots match where the artifacts live.
* fix(open-knowledge): resolve titles + metadata for managed-artifact link targets
Completes the link-graph fix: admitting managed-artifact docs alone left their
titles rendering as the raw `__skill__/...` doc name, because readPageTitle/
metadata resolve disk paths via resolveDocPath, which only maps content-tree
docs. Route managed-artifact doc names through managedArtifactAbsPath instead so
their SKILL.md / template file is read for title + frontmatter.
Narrows managedArtifactAbsPath's param to the two location fields it actually
consults (projectDir + homedirOverride) so a path-only caller needn't fabricate
the store-cache/reconcile hooks. Adds an integration test: a doc linking to a
skill gets the skill admitted and its title resolved, not the raw doc name.
* test(open-knowledge): skip managed-artifact watcher event tests on CI
The two tests assert real chokidar event delivery. The watcher uses
usePolling:true, so detection needs a setInterval tick on the event loop — on the
Linux CI runner the whole server suite saturates one loop and the poll never
fires within even a 20s deadline (consistent timeouts at exactly the deadline).
macOS, the only supported platform, runs them fast. Gate the block off CI via
describe.skipIf so the watcher stays verified locally without flaking CI.
* fix(open-knowledge): show the Links panel for skills/templates
The backlinks + forward-links sections gated their queries on pages.has(docName),
the file-tree page set — which excludes managed-artifact docs (skills/templates
are tree-hidden). So a skill's Links panel stayed empty even though the link
graph (no per-doc gate) showed it connected. Admit managed-artifact doc names to
the enabled gate so the panel fetches the index data the graph already exposes.
* fix(open-knowledge): resolve skill/template timelines by artifact key
getDocumentHistory built its git-log pathspec + OkActor post-filter from the
raw docName. For a skill the editor passes the synthetic __skill__/project/<name>,
but its versions are committed under the .ok/skills/<name> artifact key (file
.ok/skills/<name>/SKILL.md) — so the pathspec matched nothing and a saved
version never showed in the timeline. Translate managed-artifact doc names to
their recorded key (managedArtifactContributorAttribution): file path for the
pathspec, docKey for the predecessor chain + actor filter. Personal skills are
unversioned, so their timeline is empty by design. Test commits a skill version
under its key and asserts the synthetic-name query surfaces it.
* fix(open-knowledge): address skill/template timeline, diff + rollback by artifact key
Skills/templates are versioned in the shadow repo under their .ok/<...> artifact
key, but the editor addresses them by the synthetic __skill__/__template__ doc
name. getDocumentHistory's git pathspec + OkActor filter and safeDocPath (used by
version-read, diff and rollback) all built paths from the raw doc name, so they
targeted files with no commits — timelines were empty and rollback/diff failed.
Add one shared resolver, managedArtifactTimelinePaths, that maps the synthetic
name to { docKey, filePath } (or flags personal skills unversioned), and route
both getDocumentHistory and safeDocPath through it. Tests cover the resolver and
an end-to-end skill timeline query by synthetic name.
* fix(open-knowledge): outline panel + new-doc flag parity for skills
OutlinePanel gated its headings query on pages.has(docName) — the file-tree set
that excludes managed-artifact docs — so a skill's Outline tab was empty (same
class as the Links panel gate). And EditorActivityPool derived isNewDoc from
!pages.has(docName), so every skill was treated as an unsaved new doc. Admit
managed-artifact doc names in both.
* fix(open-knowledge): resolve skill/template content path in one place (outline fix)
handlePageHeadings (Outline panel) read content via resolveDocPath, which mapped
a skill's synthetic __skill__/<name> name to <contentDir>/__skill__/<name>.md —
a path that doesn't exist — so the outline 404'd and rendered blank. Fold the
managed-artifact translation (managedArtifactAbsPath) into resolveDocPath itself
and drop the separate resolveLinkedDocPath wrapper, so every docName→disk reader
(titles, metadata, page-headings) resolves skills through one path.
* spec(open-knowledge): symlink skill install, detection, self-heal
Audited + locked spec (D1-D21) for replacing copy-projection with symlink-based
install: .ok/skills source of truth, per-editor partial install, auto detect/
adopt/heal on open, scope/targets in .ok/ not SKILL.md, scripts-as-text viewer,
properties unified with docs FM editor, + the review's UI bug fixes. Supersedes
the copy-import idea. Cold-read findings folded into the seam map.
* fix(open-knowledge): allow hyphens in the new-skill name input
The name field slugified on every keystroke and trimmed trailing hyphens, so
the '-' in 'image-location' was deleted the instant it was typed — hyphenated
names were impossible. Sanitize leniently while typing (keep trailing hyphens),
trim a trailing hyphen only on blur. Grammar unchanged.
* fix(open-knowledge): clarify the scripts install warning
The old copy ('OK never runs them — your editor gates execution') left users
unsure whether scripts run at all. Reword to say plainly that the editor's AI
agent runs them and Open Knowledge never does.
* feat(open-knowledge): codex skills install to .codex/skills, not shared .agents
D16/D22: codex's per-editor skill dir was the shared .agents/skills, which
conflated Cursor and Codex and broke 'install on Codex only'. Move codex
fully to .codex/skills — authored skills AND the shipped open-knowledge
discovery bundle's codex copy. .agents/skills stops being any editor's
per-editor root; the all-agents central discovery store at
~/.agents/skills/open-knowledge-discovery stays (it is a broadcast path, not
codex's). Updated both sources of truth (core EDITOR_PROJECT_SKILL_ROOT and
the CLI EDITOR_TARGETS.projectSkillPath), the desktop + CLI reclaim host
lists, the legacy-removal sweep (still cleans the .agents pre-split central),
git-exclude + MCP install copy, and every codex-path test.
* feat(open-knowledge): install skills by symlink, not copy
D1/D11: install now symlinks an editor host dir (.claude/skills/<name>) to the
single source of truth at .ok/skills/<name> instead of copying bytes. Editing
the source is instantly visible to every installed editor, so there is nothing
to re-project on edit. Link target is relative when the source is inside the
project (portable with a committed .ok/skills) and absolute for personal-scope
sources (D14b). Authoritative replace still drops any prior entry first
(stale/broken link or a legacy real-dir copy).
Deletes the now-dead drift machinery: hashSkillSource + listFilesSorted, and
the marker's contentHash field (a copied-snapshot hash has no meaning when the
host entry IS the source). The marker stays a cache; truth is the on-disk
symlink. looseObject keeps old markers (with contentHash) parsing. Adds a
traced symlink fs wrapper. The shipped open-knowledge bundle stays a copy
(D19) — it ships in the app asar with no .ok/skills source to link to.
* feat(open-knowledge): reconcile skill installs on project open
D2/D3/D4: new skill-reconcile pass runs in bootServer and brings editor skill
dirs into line with the symlink model — heal a drifted/broken managed link,
remove an orphan link whose source is gone, adopt a foreign or legacy-copy
skill by moving it into .ok/skills and symlinking it, replace a redundant copy
(same content) with a symlink, and suffix-adopt a colliding copy (different
content) under <name>-<editor> without ever deleting it (D13). Detection scans
every editor skills root plus the generic .agents broadcast dir; the shipped
open-knowledge bundle (copy, D19) is left untouched. Install state is the
on-disk symlink reality — the marker is a refreshed cache.
Retires the dead rematerializeInstalledSkills (its re-copy-from-marker job is
obsolete: a symlink persists across launches, and a deleted link now means
uninstalled per truth-is-detection). The change-targets reprojectAllManagedSkills
stays — it is the live per-editor install-set sync. Shadow attribution of a
boot-time adopt is deferred to the source's next edit through the skills-write
spine (the moved bytes are safe under .ok/skills, collisions recoverable).
* feat(open-knowledge): per-editor skill install menu + status pill
D5/D8/D10/D17/FR10/FR11: replace the skill tab toolbar's wide Install /
Reinstall / Uninstall button row with one consolidated control — a
non-interactive status pill (Installed/Draft) plus a single install menu
carrying a per-editor checkbox each (Claude / Cursor / Codex, reflecting the
on-disk hosts), Install-on-all, Uninstall-everywhere, and project-scope
version history. Collapsing the row keeps the right-aligned cluster narrow so
it no longer overlaps the markdown toggle. 'Reinstall' is gone everywhere
(symlinks are always live) — also dropped from SkillActionMenuItems.
The install handler is now set-exact: install({targets}) reverse-projects any
editor the skill was previously installed into but is not in the new set, so
toggling one editor off in the menu removes just that symlink. useSkillActions
.install takes an optional per-editor targets list.
* docs(open-knowledge): record symlink-skill-install implementation status
* feat(open-knowledge): skill properties fully reuse the document property panel
FR13: the skill Properties panel no longer reimplements frontmatter rows. It
renders the EXACT document PropertyPanel (same component, same CRDT binding,
same recursive object/nested-frontmatter editor + add/rename/reorder/tags), so
a skill gets nested-object editing for free and can never drift from how docs
edit frontmatter. The only skill-specific rows are the identity affordances
above it: name (a rename → git-mv, never a plain patch) and the create-mode
scope picker. PropertyPanel gains an additive reservedKeys prop (default none,
docs unchanged) so name is rendered once, as identity, not duplicated as a
frontmatter row — exactly as a document's filename is not one of its
properties.
* feat(open-knowledge): browse a skill's bundled files as read-only text
FR17: a skill is a folder, so its bundled files (scripts/, reference/, assets)
are now browsable beside the editable SKILL.md body. GET /api/skill returns the
files inlined as read-only text (readSkillBundledFiles: recursive, sorted,
excludes SKILL.md, caps at 256KB, null for binary). Scripts come back as TEXT,
never an executable byte stream — the agent in the editor runs them, OK only
displays them. A new SkillBundledFiles panel (reusing PropertyDisclosure +
Collapsible + Button) lists each file with click-to-reveal text, mounted under
the skill identity panel. Editing bundled files in-app is future work.
* docs(open-knowledge): mark FR13/FR14/FR17 done in implementation status
* feat(open-knowledge): index project skills in workspace search
FR18: project skills (.ok/skills/<name>/SKILL.md) are tree-excluded from the
file index, so they were unsearchable. The corpus builder now enumerates them
from disk and adds each as a search document under its managed-artifact doc
path (__skill__/project/<name>), titled by frontmatter name with description +
body as content — so a skill is findable by what it does, and a hit opens the
skill tab via the shared nav resolution. Semantic search covers them for free
(embedCorpus embeds every corpus doc). The corpus fingerprint includes a
stat-only skill pass so it rebuilds on skill add/edit/remove without paying a
content read on the per-search path.
* fix(open-knowledge): no duplicate tab when opening an already-open doc
FR15: opening a doc into a stale blank 'new tab' (replace-active) minted a
duplicate '<doc> doc-tab:1' instance when that doc was ALREADY open, instead of
activating the existing tab. Skills hit this often — the Skills panel opens
them via the hash while a blank tab may be active — which read as the reported
duplicate/disappearing skill tabs. New pure helper blankReplaceDocTabId returns
the existing canonical tab id (activate, discard the blank) when the doc is
open, else the next available instance id; nextAvailableDocTabId's duplicate
instances stay reserved for explicitly opening a second copy. Unit-tested in
editor-tabs.test.ts; DocumentContext consumes the helper.
* docs(open-knowledge): mark FR18 + FR15 done in implementation status
* feat(open-knowledge): skills are browsable folders in the sidebar tree
Per feedback: a skill is a folder, so the Skills section now renders each skill
as an expandable folder you browse like the file tree above it — expand to see
everything it ships nested inside (SKILL.md + scripts/ + reference/ + assets),
with real nested directories. SKILL.md opens the skill editor; any other file
reveals its text read-only in place (scripts shown as text, never executed),
reusing the FR17 GET /api/skill files payload. Builds a nested tree from the
flat file paths; folders-first sort. Removes the now-redundant properties-panel
SkillBundledFiles viewer (the sidebar is the single file-browsing surface).
* fix(open-knowledge): skill files open read-only in the editor, not a sidebar box
Replaces the cramped inline preview ('mini mode') in the Skills tree: clicking a
bundled file n…
2 tasks
inkeep-oss-sync Bot
pushed a commit
that referenced
this pull request
Jul 8, 2026
…linkify, Cmd+K dual-role (#2478)
* [US-001] Add pure GFM-shape link token detector
New packages/app/src/editor/gfm-link-detector.ts exposes
detectGfmLinkToken(token), a string-in / {href,text}|null-out recognizer
for the three GFM autolink-literal shapes (https?://, www., email). It is
editor-independent (no ProseMirror imports) so the typed-autolink plugin
and the clipboard dispatcher can both reuse it.
The recognizer is ported from mdast-util-gfm-autolink-literal (the markdown
pipeline's own transform) so it agrees with MarkdownManager.parse by
construction: a token converts only when GFM itself would, so bare domains,
filenames, and localhost:port stay plain text with no TLD table. www.
tokens resolve to an http:// href and emails to mailto:, matching the
pipeline exactly.
The co-located test pins the acceptance matrix, a parity check against the
real pipeline, structural rejection of bare domains, and the scheme allowlist.
* [US-002] Add origin-guarded typed-URL autolink plugin
Typed URL + boundary key (space or Enter) converts to a link mark with
linkStyle gfm-autolink so it serializes as a bare literal. Detection reuses
stock TipTap's changed-range last-word scan but swaps linkify's tokenizer for
the GFM-parity detector, so only what the markdown pipeline itself would
linkify converts.
Safe in the multi-writer CRDT by construction: a fail-closed origin guard
skips any transaction batch carrying ySyncPluginKey meta (remote peers,
agents, disk loads, observer echoes), and an active/focused-editor predicate
keeps a stray local write into a pooled hidden editor from converting. The
mark is added by its own deferred dispatch (never returned from
appendTransaction), which also lets the flush honor IME composition and
re-validate the target range before mutating.
Registered only in the app editor extension list, so linkification stays a
client-side behavior; the core and persistence extension sets are untouched.
* [US-003] Give typed autolink conversions their own single-undo step
The deferred mark-add dispatch in the autolink flush is now bracketed by
yUndoPluginKey undoManager stopCapturing() calls. The call before the
dispatch keeps the mark out of the typing's capture window so one undo
removes just the link and leaves the typed text and trailing space
intact; the call after keeps subsequent keystrokes from merging into the
mark's stack item. Covered by three real-Collaboration-binding tests
(undo keeps text, redo reapplies the mark, follow-up typing undoes
independently), each proven red without its half of the mechanism.
* [US-004] Add lone-URL paste/drop linkification in the dispatcher
A single-URL payload now linkifies at a dedicated dispatcher step after the plain-paste and codeBlock gates. At a cursor, GFM autolink shapes route through MarkdownManager.parse for bare-literal bytes; over a one-block selection the selected text is kept and link-marked (allowlisted schemes verbatim, emails as mailto, dotted hosts https-prepended). Drop shares the branch tree. Every dispatcher-minted transaction now carries preventAutolink meta so the typed-autolink plugin never re-scans paste output as typing. Exports isAllowedLinkUri from core link-fidelity as the shared scheme gate.
Also re-keys shift-tracker listener attachment from process lifetime to window identity: the new real-editor tests were the first callers with a live DOM window, and the old attach-once latch orphaned the tracker's own fake-window tests.
* [US-005] Add byte-pinning e2e for linkification paths and per-path undo
Pins exact Y.Text bytes for typed autolink, paste at cursor, and paste
over selection against the real app and CRDT server, plus one-undo
semantics per path and the clean copy round-trip. Adds a reusable
selectText helper to the e2e helper barrel. The typed-path oracles pin
the observed byte lifecycle: the mark-only conversion is within the
bridge's escape-collapse tolerance, so bytes settle to the bare literal
on the next content-bearing edit. Also appends the spec to the test:e2e
list and removes the stale file-tree-compact-folders entry.
* [US-006] Add Cmd+K dual-role: add-link claim + palette narrowing
* [US-007] Add clipboard URL pre-fill to the link popover
* [US-008] Add typed [text](url) input rule
Typing the closing paren of a well-formed inline-link literal collapses
[text](url) to its display text carrying an inline link mark (serializes
back to [text](url)). The input rule is only the local-focused trigger,
so it never fires on y-sync/remote/agent/programmatic transactions and the
FR1 no-cross-writer invariant holds without an explicit origin guard. The
collapse lands as a deferred separate dispatch with stopCapturing before
and after, so one undo restores the full literal and the trigger char is
kept — matching the gfm-autolink typed path.
Href policy via isAllowedLinkUri; code/inline-code contexts refused by
TipTap's own input-rule runner; existing-link overlap and wikilink
shorthand excluded. Shared headless-editor rigs extracted to
editor-rig.test-helper.ts (gfm-autolink test rewired to them).
* [US-009] Add apex e2e: FR1 cross-writer + hidden editor, ⌘K matrix, clipboard denial
Real-app release gate for the P0 hazard and the shortcut contract:
- FR1 two live clients: a boundary-less URL typed by a peer syncs to the
receiver and is never linkified there; only a client's own boundary-typed
URL converts (typed at doc start so its boundary never lands adjacent to
the peer's URL). Converges across both clients.
- FR1 backgrounded editor: a peer's boundary-less URL reaches a hidden
Activity's pooled editor and stays plain.
- FR7 ⌘K routing matrix: selection -> link popover; collapsed caret ->
palette; caret in a link -> chip edit dialog; source pane -> palette;
Cmd+Shift+K -> not the palette (exact-⌘K narrowing regression guard).
- FR8 clipboard pre-fill under a real withheld permission: popover opens
empty and stays functional.
Injection is via live typing, not agent/markdown writes: the pipeline
autolinks bare URLs on parse, so only live-typed URLs stay plain — which is
also what exercises the client plugin's origin discipline. selectText helper
already existed in _helpers/editor-state.ts. Spec appended to the test:e2e list.
* [US-010] Pin gfm-autolink serializer edges; reconcile stale link docs
- FR-17 serializer block gains two programmatic gfm-autolink pins: a mark
whose text != href emits the text (href dropped — the documented
single-text-child contract), and a mark spanning a non-text child falls
through to the explicit [text](url) form. Parse never produces these; the
WYSIWYG linkify paths could, so their emit contract is fixed.
- InternalLink.addProseMirrorPlugins gains a comment: it intentionally does
not spread this.parent?.(), dropping the stock TipTap autolink/linkOnPaste/
clickHandler plugins in favour of OK's own linkify/paste/click surfaces.
LinkFidelity's header is reconciled to say the same (it inherits the stock
plugins; the app subclass replaces them).
- Docs: CI test:e2e subset count 30 -> 43 in AGENTS.md (CLAUDE.md is a
symlink to it); playwright.config.ts clipboard comment updated to note the
FR8 denial case that exercises the real permission gate.
Fidelity link-edge + I1-I7 unchanged (41/0); FR-17 235/0; typecheck clean.
* docs(link-authoring-ux): add feature spec + research evidence
Internal-only (copybara-excluded): SPEC.md with D1-D19 decision log,
FR1-FR9, §16 agent constraints, and the evidence/ + meta/ grounding used
to author and audit the feature.
* fix(link-authoring): satisfy knip — declare starter-kit dep, drop dead exports
The full `bun run check` gate (knip) surfaced two issues the per-story
`bun run test` gate does not run:
- @tiptap/starter-kit is imported by three test files (the shared editor
rig + two bubble-menu/clipboard tests) but was never declared; add it as a
devDependency (^3.22.3, matching the other @tiptap packages) and refresh
bun.lock.
- gfmAutolinkPlugin / gfmAutolinkPluginKey / GfmAutolinkPluginOptions were
exported only as a test seam; the gfm test now drives the plugin through
the GfmAutolink extension, so those exports are dead — make them
module-private (internal references unchanged).
* fix(link-authoring): strip FR/D process markers from comments (md-audit)
md-audit's comment-discipline rule (run only by `bun run check`, not the
per-story `bun run test`) flagged FR1/FR7/FR8/D18 tokens in the input-rule
docstring and the apex e2e comments + describe titles. Per OK convention,
functional-requirement and spec-decision IDs live in the PR/commit, not
source. Reworded to describe the behaviors directly; no test or code change.
* fix(link-authoring): land popover focus reliably; stop Escape-swallow after panel dismissal
Two real bugs found by browser QA of the ⌘K keyboard flow, both pre-existing
and newly exposed now that ⌘K makes the popover keyboard-first:
- LinkEditPopover auto-focus was a single rAF, but the input lives in the
floating bubble menu, which is unfocusable (visibility:hidden) until
floating-ui finishes positioning — so focus() was a silent no-op and the
URL input never took focus on open (instrumented: 13 focus() calls, zero
focusin events). Now retries each frame until focus lands once, then stops
permanently so it can never fight the user; cleanup cancels on close.
- link-path-suggestions gated its keydown handling on suggestions EXISTING
(showSuggestionOptions) rather than the panel being VISIBLE. Suggestions
still exist for the value after an Escape dismissal, so every subsequent
Escape/Enter was swallowed against an invisible panel and the popover
could not be closed from the keyboard — contradicting the handler's own
'the parent gets the next Escape' contract. Gate on visible options.
Verified end-to-end: ⌘K focuses the input; Escape #1 dismisses the
suggestion panel; Escape #2 closes the popover and returns focus to the
editor. Pinned in the apex e2e (focus + two-stage Escape) and a dom test
(after dismissal, keys reach the parent onKeyDown).
* refactor(link-authoring): apply local-review findings
Nine accepted findings from the local review gate (0 critical / 0 major):
- ⌘K claim reads the non-throwing editorView field structurally instead of
the editor.view throwing proxy — a ⌘K during an Activity recycle now falls
through to the palette instead of crashing the nearest ErrorBoundary.
- linkifySelection is try/caught like every sibling dispatch branch, with
logConversionFail telemetry (new linkifySelection stage label), so a throw
falls through to the normal paste tree instead of silently dropping.
- Extract dispatchAsOwnUndoStep (undo-isolation.ts): the correctness-critical
stopCapturing-dispatch-stopCapturing sequence now lives in one place, used
by both mark-producing surfaces; the input rule's microtask-deferred second
stopCapturing is aligned to the proven synchronous variant.
- AddLinkShortcutAction consumer is an exhaustive switch with an
assertNeverAddLinkAction backstop (assertNeverLinkTarget convention).
- Drop a no-op 'as any' + misleading biome-ignore on schema.nodeFromJSON
(ProseMirror types the parameter as any).
- IME guard test shadows the PUBLIC view.composing getter instead of mutating
the private input slot, so a PM restructure can't false-green it.
- Strengthened assertions: cross-block URL paste also pins the delivered
plain-text content; the Branch-A malformed-JSON test pins the dispatcher's
handled=true fall-through; new empty-URL [text]() stays-literal case.
- Apex hidden-editor test replaces its fixed 500ms sleep with a deterministic
poll of the HIDDEN pooled doc via ProviderPool.peek — now proving delivery
during the backgrounded window instead of hoping for it.
- Clipboard pre-fill catch logs a console.debug breadcrumb for unexpected
non-DOMException failures (routine denials stay silent).
Declined with evidence: exact-⌘K palette narrowing (deliberate, spec decision
log D12), the ⌘K popover-open pub/sub module (established *-events.ts idiom;
per-instance gating), and the pre-fill vs paste-over-selection trust asymmetry
(deliberate, D14).
* fix(link-authoring): exception-safe undo capture; document link authoring
Second local-review round (1 Major, 2 Minor accepted; plus three small
Consider items):
- dispatchAsOwnUndoStep closes the capture in a finally: view.dispatch runs
third-party plugin hooks and can throw; without the closing stopCapturing
the capture is left split-open and the user's next keystrokes merge into
the failed item (one undo would then strip them together). Both microtask
call sites (gfm flush, input-rule collapse) also catch+warn instead of
letting the throw escape as an uncaught async error, matching the paste
dispatcher's degradation contract.
- Clipboard pre-fill's unexpected-error breadcrumb moves from console.debug
(suppressed by default filters) to console.warn.
- User docs: editor.mdx gains a Links section covering the four authoring
paths and a Cmd+K row in the shortcut table describing the dual-role.
- Palette registry description now mentions the selection carve-out so the
two Cmd+K rows in settings read as scoped, not conflicting.
- attrsEqual only recurses into matching plain-object/array shapes (an array
vs an index-keyed object, or two Dates, no longer compare equal).
- Comment typo .len -> .length in the input rule's position math.
Declined with evidence: stopImmediatePropagation claim (spec decision D12,
byte-for-byte Edit-with-AI precedent); permissions.query gating for pre-fill
(D14 accepted the one-time web prompt; granted-only would make first-use
pre-fill silently never work); trailing placeholder ellipsis (the repo's own
microcopy lint rule forbids it — tooling beats generic style guidance).
* polish(link-authoring): third review round — logs, tests, comments, docs
Accepted from the third local-review pass:
- Deferred-linkify failure logs carry operational context (candidate ranges
+ hrefs); the input rule guards only the dispatch (trust boundary), so its
internal guards fail loud instead of logging as benign.
- Two test gaps closed: typing after an input-rule conversion stays its own
undo step (the closing-stopCapturing invariant, mirroring the gfm suite),
and a throwing mdManager.parse falls through for Branch B and the lone-URL
cursor path (Branch A was the only pinned peer).
- Shared LinkStyle union exported from link-fidelity (type-only) and the
producer constant typed against it, so a drifted linkStyle literal is a
compile error instead of a mis-serialized link.
- Comment-accuracy corrections, each verified against the actual source:
detector consumer is lone-url.ts; the whitespace class is TipTap's
UNICODE_WHITESPACE_PATTERN; PREVENT_AUTOLINK_META is defined here (TipTap
only shares the string value); captureTimeout 500ms is Y.js's default; the
AGENTS.md prepend example is flagged as a deliberate edge. Detector header
now names its upstream coupling and the parity tests that guard it.
- Docs: paste bullet covers drop, typed-trigger wording tightened,
frontmatter description mentions link authoring.
- ⌘K exact-match narrowing rationale + the capture/bubble phase-ordering
contract documented on the palette registry entry (the narrowing itself is
the spec's deliberate decision and stays).
- Popover focus retry capped at ~1s with a warn on exhaustion.
* test(link-authoring): pin the wikilink atom exclusion on the typed path
The typed-autolink plugin's wikilink safety rested on a structural argument
(the wikiLink node is an atom, so textBetween's word scan can never see
inside it) with no test on this path. Pin it: a typed URL next to a wikilink
converts alone and the atom stays intact — if WikiLink ever stops being an
atom, this fails instead of the plugin silently linkifying wikilink internals.
* changeset(link-authoring): minor release notes for link authoring
* fix(link-authoring): regen ok-marketing vendored core dist; close LinkStyle loop
The LinkStyle type export changed core's built index.mjs, and ok-marketing
vendors core's dist — regenerate it via the prescribed
'pnpm --filter @inkeep/ok-marketing regen-core-dist' (fixes the
verify-core-dist CI check).
Also adopts the local-review repair pass's completion of the LinkStyle
coupling work, verified green before adoption: the PM->mdast serializer's
linkStyle variable is typed against the shared union (a drifted literal is
now a compile error on BOTH producer and serializer sides; no behavior
change), the detector parity matrix gains eight edge rows (trailing
punctuation, unbalanced paren, underscore-domain and email bad-tail rejects,
and their keep-side complements) checked against the real parse, the
remark-gfm registration carries the reciprocal hand-port coupling note, and
the md-audit anchors catalog is regenerated accordingly.
* fix(link-authoring): notify on linkify-selection degradation; use meta constant in test
Cloud review round 1: linkifySelection's catch now calls notifyPasteDegraded
(Branch D's established degradation contract) so a dispatch failure is no
longer silent to the user — while still returning false so the branch tree
delivers the clipboard content as a standard paste (claiming the failed
paste would drop the content entirely, the outcome the dispatcher contract
forbids). The origin-guard suppression test uses the exported
PREVENT_AUTOLINK_META constant instead of a raw string literal, so a key
rename can't silently defang it.
* test(link-authoring): condition-based negative wait in the exact-cmdk apex test
Main's new e2e STOP rule forbids page.waitForTimeout in stress/visual/a11y
specs; the Cmd+Shift+K test used a 300ms sleep before asserting the palette
stayed shut. Replace with a condition-based sandwich: assert palette absent,
then press exact Cmd+K and wait for the palette to APPEAR — the positive
signal proves the keystroke pipeline processed events after the
Cmd+Shift+K press, and would fail if that press had opened or toggled the
palette. Verified in a real browser.
* fix(link-authoring): eager view capture; throw-path + events tests; notify siblings
Cloud review round 2 (APPROVE WITH SUGGESTIONS):
- The input rule captures editor.view eagerly at handler time instead of
re-reading it inside the microtask, where it is a throwing proxy if an
Activity recycle starts in the gap (the sibling gfm plugin's boundView
discipline; input rules always fire from a mounted view).
- New undo-isolation.test.ts pins the exception-safety property that IS the
helper's reason to exist: a throwing dispatch still closes the capture
(stopCapturing runs exactly twice) — a try/finally-to-try/catch refactor
now fails a test instead of silently corrupting undo.
- link-edit-popover-events gains its co-located test per the *-events.ts
peer convention (emit/unsubscribe/target-scoping).
- tryBranchA / tryBranchMarkdown / applyJsonSlice catch sites now call
notifyPasteDegraded like Branch D and linkifySelection — same-shape
sibling completion of the degradation contract this PR standardized.
* chore(link-authoring): regen catalogs + vendored core dist after rebase onto main
* fix(link-authoring): accept explicit-scheme dotless hosts in typed/cursor autolink
The dotted-domain check is a schemeless-fuzz defense: micromark's
http_autolink production skips it when an explicit scheme is present, so
http://localhost:5174 linkifies and round-trips as a bare literal. The
detector was stricter than the parser it mirrors, leaving typed/pasted
localhost URLs plain while http://127.0.0.1:8080 converted. Split the
domain check: label-tail rules apply to both arms, the dotted-host
requirement to the www arm only. Schemeless tokens (localhost:5173) and
underscore labels stay rejected; parity rows pin every new shape against
MarkdownManager.parse.
* chore(link-authoring): regen catalogs + vendored core dist after rebase onto main
* test(link-authoring): reset shift-tracker singleton around its test file
The attach guard is a module singleton; when a sibling test file in the
same bun process wires the tracker to its own window first (handle-paste
does, transitively), installShiftTracker() early-returns and the fake
window's dispatches reach nobody. Reproduced deterministically by running
handle-paste.test.ts before shift-tracker.test.ts — the exact merge_group
failure that dequeued this PR. Detach in beforeAll/afterAll via the
__resetShiftTrackerForTests hook the implementation ships for this.
GitOrigin-RevId: 25b1b0d1bc6a68dc65e9723434a4fa219bd0a49a
inkeep-oss-sync Bot
pushed a commit
that referenced
this pull request
Jul 8, 2026
…linkify, Cmd+K dual-role (#2478)
* [US-001] Add pure GFM-shape link token detector
New packages/app/src/editor/gfm-link-detector.ts exposes
detectGfmLinkToken(token), a string-in / {href,text}|null-out recognizer
for the three GFM autolink-literal shapes (https?://, www., email). It is
editor-independent (no ProseMirror imports) so the typed-autolink plugin
and the clipboard dispatcher can both reuse it.
The recognizer is ported from mdast-util-gfm-autolink-literal (the markdown
pipeline's own transform) so it agrees with MarkdownManager.parse by
construction: a token converts only when GFM itself would, so bare domains,
filenames, and localhost:port stay plain text with no TLD table. www.
tokens resolve to an http:// href and emails to mailto:, matching the
pipeline exactly.
The co-located test pins the acceptance matrix, a parity check against the
real pipeline, structural rejection of bare domains, and the scheme allowlist.
* [US-002] Add origin-guarded typed-URL autolink plugin
Typed URL + boundary key (space or Enter) converts to a link mark with
linkStyle gfm-autolink so it serializes as a bare literal. Detection reuses
stock TipTap's changed-range last-word scan but swaps linkify's tokenizer for
the GFM-parity detector, so only what the markdown pipeline itself would
linkify converts.
Safe in the multi-writer CRDT by construction: a fail-closed origin guard
skips any transaction batch carrying ySyncPluginKey meta (remote peers,
agents, disk loads, observer echoes), and an active/focused-editor predicate
keeps a stray local write into a pooled hidden editor from converting. The
mark is added by its own deferred dispatch (never returned from
appendTransaction), which also lets the flush honor IME composition and
re-validate the target range before mutating.
Registered only in the app editor extension list, so linkification stays a
client-side behavior; the core and persistence extension sets are untouched.
* [US-003] Give typed autolink conversions their own single-undo step
The deferred mark-add dispatch in the autolink flush is now bracketed by
yUndoPluginKey undoManager stopCapturing() calls. The call before the
dispatch keeps the mark out of the typing's capture window so one undo
removes just the link and leaves the typed text and trailing space
intact; the call after keeps subsequent keystrokes from merging into the
mark's stack item. Covered by three real-Collaboration-binding tests
(undo keeps text, redo reapplies the mark, follow-up typing undoes
independently), each proven red without its half of the mechanism.
* [US-004] Add lone-URL paste/drop linkification in the dispatcher
A single-URL payload now linkifies at a dedicated dispatcher step after the plain-paste and codeBlock gates. At a cursor, GFM autolink shapes route through MarkdownManager.parse for bare-literal bytes; over a one-block selection the selected text is kept and link-marked (allowlisted schemes verbatim, emails as mailto, dotted hosts https-prepended). Drop shares the branch tree. Every dispatcher-minted transaction now carries preventAutolink meta so the typed-autolink plugin never re-scans paste output as typing. Exports isAllowedLinkUri from core link-fidelity as the shared scheme gate.
Also re-keys shift-tracker listener attachment from process lifetime to window identity: the new real-editor tests were the first callers with a live DOM window, and the old attach-once latch orphaned the tracker's own fake-window tests.
* [US-005] Add byte-pinning e2e for linkification paths and per-path undo
Pins exact Y.Text bytes for typed autolink, paste at cursor, and paste
over selection against the real app and CRDT server, plus one-undo
semantics per path and the clean copy round-trip. Adds a reusable
selectText helper to the e2e helper barrel. The typed-path oracles pin
the observed byte lifecycle: the mark-only conversion is within the
bridge's escape-collapse tolerance, so bytes settle to the bare literal
on the next content-bearing edit. Also appends the spec to the test:e2e
list and removes the stale file-tree-compact-folders entry.
* [US-006] Add Cmd+K dual-role: add-link claim + palette narrowing
* [US-007] Add clipboard URL pre-fill to the link popover
* [US-008] Add typed [text](url) input rule
Typing the closing paren of a well-formed inline-link literal collapses
[text](url) to its display text carrying an inline link mark (serializes
back to [text](url)). The input rule is only the local-focused trigger,
so it never fires on y-sync/remote/agent/programmatic transactions and the
FR1 no-cross-writer invariant holds without an explicit origin guard. The
collapse lands as a deferred separate dispatch with stopCapturing before
and after, so one undo restores the full literal and the trigger char is
kept — matching the gfm-autolink typed path.
Href policy via isAllowedLinkUri; code/inline-code contexts refused by
TipTap's own input-rule runner; existing-link overlap and wikilink
shorthand excluded. Shared headless-editor rigs extracted to
editor-rig.test-helper.ts (gfm-autolink test rewired to them).
* [US-009] Add apex e2e: FR1 cross-writer + hidden editor, ⌘K matrix, clipboard denial
Real-app release gate for the P0 hazard and the shortcut contract:
- FR1 two live clients: a boundary-less URL typed by a peer syncs to the
receiver and is never linkified there; only a client's own boundary-typed
URL converts (typed at doc start so its boundary never lands adjacent to
the peer's URL). Converges across both clients.
- FR1 backgrounded editor: a peer's boundary-less URL reaches a hidden
Activity's pooled editor and stays plain.
- FR7 ⌘K routing matrix: selection -> link popover; collapsed caret ->
palette; caret in a link -> chip edit dialog; source pane -> palette;
Cmd+Shift+K -> not the palette (exact-⌘K narrowing regression guard).
- FR8 clipboard pre-fill under a real withheld permission: popover opens
empty and stays functional.
Injection is via live typing, not agent/markdown writes: the pipeline
autolinks bare URLs on parse, so only live-typed URLs stay plain — which is
also what exercises the client plugin's origin discipline. selectText helper
already existed in _helpers/editor-state.ts. Spec appended to the test:e2e list.
* [US-010] Pin gfm-autolink serializer edges; reconcile stale link docs
- FR-17 serializer block gains two programmatic gfm-autolink pins: a mark
whose text != href emits the text (href dropped — the documented
single-text-child contract), and a mark spanning a non-text child falls
through to the explicit [text](url) form. Parse never produces these; the
WYSIWYG linkify paths could, so their emit contract is fixed.
- InternalLink.addProseMirrorPlugins gains a comment: it intentionally does
not spread this.parent?.(), dropping the stock TipTap autolink/linkOnPaste/
clickHandler plugins in favour of OK's own linkify/paste/click surfaces.
LinkFidelity's header is reconciled to say the same (it inherits the stock
plugins; the app subclass replaces them).
- Docs: CI test:e2e subset count 30 -> 43 in AGENTS.md (CLAUDE.md is a
symlink to it); playwright.config.ts clipboard comment updated to note the
FR8 denial case that exercises the real permission gate.
Fidelity link-edge + I1-I7 unchanged (41/0); FR-17 235/0; typecheck clean.
* docs(link-authoring-ux): add feature spec + research evidence
Internal-only (copybara-excluded): SPEC.md with D1-D19 decision log,
FR1-FR9, §16 agent constraints, and the evidence/ + meta/ grounding used
to author and audit the feature.
* fix(link-authoring): satisfy knip — declare starter-kit dep, drop dead exports
The full `bun run check` gate (knip) surfaced two issues the per-story
`bun run test` gate does not run:
- @tiptap/starter-kit is imported by three test files (the shared editor
rig + two bubble-menu/clipboard tests) but was never declared; add it as a
devDependency (^3.22.3, matching the other @tiptap packages) and refresh
bun.lock.
- gfmAutolinkPlugin / gfmAutolinkPluginKey / GfmAutolinkPluginOptions were
exported only as a test seam; the gfm test now drives the plugin through
the GfmAutolink extension, so those exports are dead — make them
module-private (internal references unchanged).
* fix(link-authoring): strip FR/D process markers from comments (md-audit)
md-audit's comment-discipline rule (run only by `bun run check`, not the
per-story `bun run test`) flagged FR1/FR7/FR8/D18 tokens in the input-rule
docstring and the apex e2e comments + describe titles. Per OK convention,
functional-requirement and spec-decision IDs live in the PR/commit, not
source. Reworded to describe the behaviors directly; no test or code change.
* fix(link-authoring): land popover focus reliably; stop Escape-swallow after panel dismissal
Two real bugs found by browser QA of the ⌘K keyboard flow, both pre-existing
and newly exposed now that ⌘K makes the popover keyboard-first:
- LinkEditPopover auto-focus was a single rAF, but the input lives in the
floating bubble menu, which is unfocusable (visibility:hidden) until
floating-ui finishes positioning — so focus() was a silent no-op and the
URL input never took focus on open (instrumented: 13 focus() calls, zero
focusin events). Now retries each frame until focus lands once, then stops
permanently so it can never fight the user; cleanup cancels on close.
- link-path-suggestions gated its keydown handling on suggestions EXISTING
(showSuggestionOptions) rather than the panel being VISIBLE. Suggestions
still exist for the value after an Escape dismissal, so every subsequent
Escape/Enter was swallowed against an invisible panel and the popover
could not be closed from the keyboard — contradicting the handler's own
'the parent gets the next Escape' contract. Gate on visible options.
Verified end-to-end: ⌘K focuses the input; Escape #1 dismisses the
suggestion panel; Escape #2 closes the popover and returns focus to the
editor. Pinned in the apex e2e (focus + two-stage Escape) and a dom test
(after dismissal, keys reach the parent onKeyDown).
* refactor(link-authoring): apply local-review findings
Nine accepted findings from the local review gate (0 critical / 0 major):
- ⌘K claim reads the non-throwing editorView field structurally instead of
the editor.view throwing proxy — a ⌘K during an Activity recycle now falls
through to the palette instead of crashing the nearest ErrorBoundary.
- linkifySelection is try/caught like every sibling dispatch branch, with
logConversionFail telemetry (new linkifySelection stage label), so a throw
falls through to the normal paste tree instead of silently dropping.
- Extract dispatchAsOwnUndoStep (undo-isolation.ts): the correctness-critical
stopCapturing-dispatch-stopCapturing sequence now lives in one place, used
by both mark-producing surfaces; the input rule's microtask-deferred second
stopCapturing is aligned to the proven synchronous variant.
- AddLinkShortcutAction consumer is an exhaustive switch with an
assertNeverAddLinkAction backstop (assertNeverLinkTarget convention).
- Drop a no-op 'as any' + misleading biome-ignore on schema.nodeFromJSON
(ProseMirror types the parameter as any).
- IME guard test shadows the PUBLIC view.composing getter instead of mutating
the private input slot, so a PM restructure can't false-green it.
- Strengthened assertions: cross-block URL paste also pins the delivered
plain-text content; the Branch-A malformed-JSON test pins the dispatcher's
handled=true fall-through; new empty-URL [text]() stays-literal case.
- Apex hidden-editor test replaces its fixed 500ms sleep with a deterministic
poll of the HIDDEN pooled doc via ProviderPool.peek — now proving delivery
during the backgrounded window instead of hoping for it.
- Clipboard pre-fill catch logs a console.debug breadcrumb for unexpected
non-DOMException failures (routine denials stay silent).
Declined with evidence: exact-⌘K palette narrowing (deliberate, spec decision
log D12), the ⌘K popover-open pub/sub module (established *-events.ts idiom;
per-instance gating), and the pre-fill vs paste-over-selection trust asymmetry
(deliberate, D14).
* fix(link-authoring): exception-safe undo capture; document link authoring
Second local-review round (1 Major, 2 Minor accepted; plus three small
Consider items):
- dispatchAsOwnUndoStep closes the capture in a finally: view.dispatch runs
third-party plugin hooks and can throw; without the closing stopCapturing
the capture is left split-open and the user's next keystrokes merge into
the failed item (one undo would then strip them together). Both microtask
call sites (gfm flush, input-rule collapse) also catch+warn instead of
letting the throw escape as an uncaught async error, matching the paste
dispatcher's degradation contract.
- Clipboard pre-fill's unexpected-error breadcrumb moves from console.debug
(suppressed by default filters) to console.warn.
- User docs: editor.mdx gains a Links section covering the four authoring
paths and a Cmd+K row in the shortcut table describing the dual-role.
- Palette registry description now mentions the selection carve-out so the
two Cmd+K rows in settings read as scoped, not conflicting.
- attrsEqual only recurses into matching plain-object/array shapes (an array
vs an index-keyed object, or two Dates, no longer compare equal).
- Comment typo .len -> .length in the input rule's position math.
Declined with evidence: stopImmediatePropagation claim (spec decision D12,
byte-for-byte Edit-with-AI precedent); permissions.query gating for pre-fill
(D14 accepted the one-time web prompt; granted-only would make first-use
pre-fill silently never work); trailing placeholder ellipsis (the repo's own
microcopy lint rule forbids it — tooling beats generic style guidance).
* polish(link-authoring): third review round — logs, tests, comments, docs
Accepted from the third local-review pass:
- Deferred-linkify failure logs carry operational context (candidate ranges
+ hrefs); the input rule guards only the dispatch (trust boundary), so its
internal guards fail loud instead of logging as benign.
- Two test gaps closed: typing after an input-rule conversion stays its own
undo step (the closing-stopCapturing invariant, mirroring the gfm suite),
and a throwing mdManager.parse falls through for Branch B and the lone-URL
cursor path (Branch A was the only pinned peer).
- Shared LinkStyle union exported from link-fidelity (type-only) and the
producer constant typed against it, so a drifted linkStyle literal is a
compile error instead of a mis-serialized link.
- Comment-accuracy corrections, each verified against the actual source:
detector consumer is lone-url.ts; the whitespace class is TipTap's
UNICODE_WHITESPACE_PATTERN; PREVENT_AUTOLINK_META is defined here (TipTap
only shares the string value); captureTimeout 500ms is Y.js's default; the
AGENTS.md prepend example is flagged as a deliberate edge. Detector header
now names its upstream coupling and the parity tests that guard it.
- Docs: paste bullet covers drop, typed-trigger wording tightened,
frontmatter description mentions link authoring.
- ⌘K exact-match narrowing rationale + the capture/bubble phase-ordering
contract documented on the palette registry entry (the narrowing itself is
the spec's deliberate decision and stays).
- Popover focus retry capped at ~1s with a warn on exhaustion.
* test(link-authoring): pin the wikilink atom exclusion on the typed path
The typed-autolink plugin's wikilink safety rested on a structural argument
(the wikiLink node is an atom, so textBetween's word scan can never see
inside it) with no test on this path. Pin it: a typed URL next to a wikilink
converts alone and the atom stays intact — if WikiLink ever stops being an
atom, this fails instead of the plugin silently linkifying wikilink internals.
* changeset(link-authoring): minor release notes for link authoring
* fix(link-authoring): regen ok-marketing vendored core dist; close LinkStyle loop
The LinkStyle type export changed core's built index.mjs, and ok-marketing
vendors core's dist — regenerate it via the prescribed
'pnpm --filter @inkeep/ok-marketing regen-core-dist' (fixes the
verify-core-dist CI check).
Also adopts the local-review repair pass's completion of the LinkStyle
coupling work, verified green before adoption: the PM->mdast serializer's
linkStyle variable is typed against the shared union (a drifted literal is
now a compile error on BOTH producer and serializer sides; no behavior
change), the detector parity matrix gains eight edge rows (trailing
punctuation, unbalanced paren, underscore-domain and email bad-tail rejects,
and their keep-side complements) checked against the real parse, the
remark-gfm registration carries the reciprocal hand-port coupling note, and
the md-audit anchors catalog is regenerated accordingly.
* fix(link-authoring): notify on linkify-selection degradation; use meta constant in test
Cloud review round 1: linkifySelection's catch now calls notifyPasteDegraded
(Branch D's established degradation contract) so a dispatch failure is no
longer silent to the user — while still returning false so the branch tree
delivers the clipboard content as a standard paste (claiming the failed
paste would drop the content entirely, the outcome the dispatcher contract
forbids). The origin-guard suppression test uses the exported
PREVENT_AUTOLINK_META constant instead of a raw string literal, so a key
rename can't silently defang it.
* test(link-authoring): condition-based negative wait in the exact-cmdk apex test
Main's new e2e STOP rule forbids page.waitForTimeout in stress/visual/a11y
specs; the Cmd+Shift+K test used a 300ms sleep before asserting the palette
stayed shut. Replace with a condition-based sandwich: assert palette absent,
then press exact Cmd+K and wait for the palette to APPEAR — the positive
signal proves the keystroke pipeline processed events after the
Cmd+Shift+K press, and would fail if that press had opened or toggled the
palette. Verified in a real browser.
* fix(link-authoring): eager view capture; throw-path + events tests; notify siblings
Cloud review round 2 (APPROVE WITH SUGGESTIONS):
- The input rule captures editor.view eagerly at handler time instead of
re-reading it inside the microtask, where it is a throwing proxy if an
Activity recycle starts in the gap (the sibling gfm plugin's boundView
discipline; input rules always fire from a mounted view).
- New undo-isolation.test.ts pins the exception-safety property that IS the
helper's reason to exist: a throwing dispatch still closes the capture
(stopCapturing runs exactly twice) — a try/finally-to-try/catch refactor
now fails a test instead of silently corrupting undo.
- link-edit-popover-events gains its co-located test per the *-events.ts
peer convention (emit/unsubscribe/target-scoping).
- tryBranchA / tryBranchMarkdown / applyJsonSlice catch sites now call
notifyPasteDegraded like Branch D and linkifySelection — same-shape
sibling completion of the degradation contract this PR standardized.
* chore(link-authoring): regen catalogs + vendored core dist after rebase onto main
* fix(link-authoring): accept explicit-scheme dotless hosts in typed/cursor autolink
The dotted-domain check is a schemeless-fuzz defense: micromark's
http_autolink production skips it when an explicit scheme is present, so
http://localhost:5174 linkifies and round-trips as a bare literal. The
detector was stricter than the parser it mirrors, leaving typed/pasted
localhost URLs plain while http://127.0.0.1:8080 converted. Split the
domain check: label-tail rules apply to both arms, the dotted-host
requirement to the www arm only. Schemeless tokens (localhost:5173) and
underscore labels stay rejected; parity rows pin every new shape against
MarkdownManager.parse.
* chore(link-authoring): regen catalogs + vendored core dist after rebase onto main
* test(link-authoring): reset shift-tracker singleton around its test file
The attach guard is a module singleton; when a sibling test file in the
same bun process wires the tracker to its own window first (handle-paste
does, transitively), installShiftTracker() early-returns and the fake
window's dispatches reach nobody. Reproduced deterministically by running
handle-paste.test.ts before shift-tracker.test.ts — the exact merge_group
failure that dequeued this PR. Detach in beforeAll/afterAll via the
__resetShiftTrackerForTests hook the implementation ships for this.
GitOrigin-RevId: 25b1b0d1bc6a68dc65e9723434a4fa219bd0a49a
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Make the local telemetry file sink flush spans on span end (SimpleSpanProcessor) so
ok diagnose bundlereliably captures spans — previously a batch-timer vs shutdown-timeout race could drop them under load. Also make project template enumeration order deterministic across filesystems by sorting the directory walk, so which templates fall inside the scan cap is stable run to run.