(MOT-4472) feat(console): unify shared UI patterns across workers - #837
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
skill-check — worker0 verified, 61 skipped (no docs/).
Four for four. Nicely done. |
|
Important Review skippedToo many files! This PR contains 290 files, which is 190 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (290)
You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
andersonleal
added a commit
that referenced
this pull request
Aug 20, 2026
…ills root
Skills gain full CRUD parity with prompts: directory::skills::create
{id, content} writes <skills_folder>/<id>.md with a two-layer conflict
check (visible-set resolution incl. the <id>/index alias, then raw
dest.exists(); D114), and directory::skills::delete removes a resolved
skill plus any parent directories the removal left empty, so a deleted
namespace can't keep shadowing a lower-precedence root. Both fan out
{op, namespace, id} on directory::skills::on-change, and deletes (skill
AND prompt) now self-write-mark so the watcher stops firing a spurious
op:"external" on top of the precise op.
A third scan root, agents_skills_folder (default ~/.agents/skills),
serves system-installed agent skills read-only: scanned shallowly (only
<skill>/SKILL.md, id <skill>/index) so support payload never floods
list; namespaces shadowed by the same namespace under the global/local
roots; exempt from filter_unregistered by namespace NAME (dirs actually
carrying a SKILL.md) so manual global-root forks stay visible; excluded
from the per-worker skills::index by file provenance; refused by
update/delete (D116) and reserved against create (D115). The watcher
picks the root up only when it already exists — the worker never
creates ~/.agents/skills. SkillFrontmatter learns name: as a title
fallback (title -> name -> H1) for the agents/SKILL.md convention.
Create also rejects ids the reader could never serve: filter-hidden
namespaces while filter_unregistered is on, and prompts/system-prompts
path segments the classifier would route away from skills (D115).
The directory UI's skills tab gains the new/delete buttons (id-shaped
namePattern, applied to creates only — frontmatter titles are display
fields on update), skills::create renders as a trigger card, and the
config form gains the new knob. The form is also brought in line with
the #837 shared-UI conventions: sentence-case copy end to end, no CSS
case transforms, sans panel with mono confined to path/URL inputs, and
the shared Input + Chip(tone=warning) primitives replacing hand-rolled
controls.
Tests pin agents_skills_folder into tempdirs everywhere (the default
resolves to a REAL directory on dev machines): unit cfg helpers, the
BDD harness, and the e2e config. cargo test --lib 354 passing; BDD
unchanged at its pre-existing baseline.
andersonleal
added a commit
that referenced
this pull request
Aug 20, 2026
…stem-installed ~/.agents/skills (#851) * docs(agents): wire issue-tracker, triage-label, and domain-doc guides into AGENTS.md Adds docs/agents/{issue-tracker,triage-labels,domain}.md — how agent skills reach Linear (iii team, MOT-###), the five canonical triage labels, and the CONTEXT.md/ADR reading order — and points AGENTS.md at them under a new 'Agent skills' section. * feat(iii-directory): skill create/delete and a read-only ~/.agents/skills root Skills gain full CRUD parity with prompts: directory::skills::create {id, content} writes <skills_folder>/<id>.md with a two-layer conflict check (visible-set resolution incl. the <id>/index alias, then raw dest.exists(); D114), and directory::skills::delete removes a resolved skill plus any parent directories the removal left empty, so a deleted namespace can't keep shadowing a lower-precedence root. Both fan out {op, namespace, id} on directory::skills::on-change, and deletes (skill AND prompt) now self-write-mark so the watcher stops firing a spurious op:"external" on top of the precise op. A third scan root, agents_skills_folder (default ~/.agents/skills), serves system-installed agent skills read-only: scanned shallowly (only <skill>/SKILL.md, id <skill>/index) so support payload never floods list; namespaces shadowed by the same namespace under the global/local roots; exempt from filter_unregistered by namespace NAME (dirs actually carrying a SKILL.md) so manual global-root forks stay visible; excluded from the per-worker skills::index by file provenance; refused by update/delete (D116) and reserved against create (D115). The watcher picks the root up only when it already exists — the worker never creates ~/.agents/skills. SkillFrontmatter learns name: as a title fallback (title -> name -> H1) for the agents/SKILL.md convention. Create also rejects ids the reader could never serve: filter-hidden namespaces while filter_unregistered is on, and prompts/system-prompts path segments the classifier would route away from skills (D115). The directory UI's skills tab gains the new/delete buttons (id-shaped namePattern, applied to creates only — frontmatter titles are display fields on update), skills::create renders as a trigger card, and the config form gains the new knob. The form is also brought in line with the #837 shared-UI conventions: sentence-case copy end to end, no CSS case transforms, sans panel with mono confined to path/URL inputs, and the shared Input + Chip(tone=warning) primitives replacing hand-rolled controls. Tests pin agents_skills_folder into tempdirs everywhere (the default resolves to a REAL directory on dev machines): unit cfg helpers, the BDD harness, and the e2e config. cargo test --lib 354 passing; BDD unchanged at its pre-existing baseline. * (MOT-4506) fix(iii-directory): show the function-search card inline in chat The search renderer declared no presentation metadata, so the console kept a successful DiscoverCard behind "show raw request and response" and chat only showed the generic compact card for directory::search_functions. Declare metadata.display, matching the focused result-bearing renderers in browser (screenshot), shell (agent-run, file-changes), and web. The general catch-all renderers (browser/page.js#calls, shell/page.js#shell, and iii-directory's own #directory) deliberately stay compact, so this is scoped to the search card only. Safe by construction: every non-render path in tryRender already returns null, so a pending-approval, errored, or unparseable call still falls through to the compact card. Arrived with the discovery-worker absorption (#839); shipping here since it rides the same PR. * (MOT-4506) feat(iii-directory): announce a skill's base directory so payload skills can run Live session console-1f62d4cf showed why a payload skill (impeccable: SKILL.md + scripts/ + reference/) degrades to prose-only: the console freezes only the SKILL.md body into the session addon, so the model has no way to locate scripts/context.mjs and probes .agents/skills/ relative to the project cwd, which doesn't exist. - directory::skills::get now returns `path`, the absolute on-disk file; its parent dir is the skill's base directory per the agent-skills convention. - The console appends a 'Skill base directory: <dir>' line at both freeze sites (session addon picker and /skill:<id> slash expansion) via one shared helper; body-only when the worker predates the field. * feat(harness): copy the session id from the context chip popover * (MOT-4511) feat(state)!: default the kv store to file_based persistence and flush on shutdown The in_memory default silently lost every scope (harness turn records, context snapshots, barriers, namespace claims) on each worker restart. Defaults now: store_method file_based under ./data/state (worker-cwd relative). Shutdown flushes pending dirty scopes instead of relying on the next save-loop tick, and main handles SIGTERM (workers-dev's stop signal) so that flush actually runs. Explicit in_memory pins (CI e2e) keep their behavior; tests that leaned on the volatile default now pin it explicitly. BREAKING CHANGE: a bare state worker now persists to ./data/state; pin store_method: in_memory to keep the old volatile behavior. * (MOT-4515) fix(iii-directory): keep unsaved skill/prompt drafts across tab switches The console unmounts the directory page on every tab switch, so a half-typed new skill (or an unsaved edit to an existing one) died with the component — the editor came back empty with no warning. Unsaved work now mirrors to localStorage under `${storageKey}:draft` (already per-tab and per-collection) and is restored on mount. A creating draft is self-contained; a draft over an existing entry re-fetches its on-disk baseline so dirty tracking and save() still diff against the real file. The write/clear/keep decision is a pure function in draft-storage.ts so it is testable without a DOM: notably it KEEPS storage while a baseline load is in flight, since clearing there would destroy the work just restored. Every deliberate discard (open another entry, start a create, drill out) clears immediately rather than waiting for the load to land. Storage only ever holds work that differs from disk, so a save, delete, or discard leaves nothing behind. * (MOT-4511) fix(state): make the shutdown flush actually durable Three defects in the file_based persistence path, from CodeRabbit review on #851. Each one loses exactly the write the file_based default exists to keep. - Periodic and explicit flushes both drained the whole dirty map with no mutual exclusion, so the shutdown flush could find it already empty while the save loop's write was still in flight — and process exit then cancelled that write. A shared async mutex now spans drain, persist, and requeue for both paths. - A failed write requeued its scope with a plain insert, clobbering any newer op a concurrent set/delete had queued meanwhile. A failed Delete landing on a fresh Upsert deletes live data on the next flush. Requeue is now entry().or_insert(), so newer intent always wins. - flush_dirty only logged failures, so KvStoreAdapter::flush always returned Ok and BootHandle::shutdown's warn branch was dead code. It now attempts every scope, then reports how many failed; the adapter propagates instead of discarding. Shutdown also disconnects before flushing. shutdown_async clears the SDK's running flag, which stops its receive loop dispatching further invocations, so no request accepted after the signal can mutate the store behind the flush. Requests already executing remain a race the SDK gives no way to await — closing that needs in-flight tracking, which is out of scope here. Two tests: the requeue ordering rule, and that a failed persist surfaces as an error and leaves the scope dirty (a directory planted where the index file belongs makes the write fail without touching permissions). * (MOT-4506) fix(iii-directory): correct the base-dir helper, trigger contract, and README anchor CodeRabbit review on #851. - skillBodyWithBaseDir cut the directory at the last '/' only. The worker ships Windows binaries, so path can be C:\...\SKILL.md; worse, with no separator at all lastIndexOf returned -1 and slice(0, -1) produced the whole path minus its last character, presented to the model as a directory. Now cuts on either separator and falls back to body-only when there is none. - All three on-change trigger descriptions listed only write ops, but the fs watcher also fires them with { op: "external" }; subscribers read these descriptions as the event contract. The prompts one also omitted delete, which it has fired since the prompt CRUD landed. - README linked #layout; the heading anchor is #on-disk-layout. - Long session ids could not ellipse in the harness context chip: a flex item will not shrink past its content without min-width: 0, so the id pushed the copy button out of the popover. Same pairing the sibling .harness-ui-pop-model already uses. Also documents what happens when the agents skills root is created after boot — it stays unwatched until a restart, and reads still serve it because every read re-scans disk, so only the live doorbell is missing. Re-arming the watcher for a root that may never appear is not worth the machinery.
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.
Summary
Validation
Fixes MOT-4472
Refs MOT-4477
Refs MOT-4481