Sync Open Knowledge from agents-private#539
Merged
Merged
Conversation
…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-internal-ci[bot]](https://avatars.githubusercontent.com/u/123133689?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Automated approval from agents-private public-mirror-sync (run: https://github.com/inkeep/agents-private/actions/runs/28933094177). Source of truth is the monorepo; direct edits on inkeep/open-knowledge are overwritten on next sync.
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.
Copybara-translated commits from agents-private. Rebase-merge this PR so the prepared commits land directly on public main.