* fix(skills): measure draft dirtiness against a seeded baseline (MUL-5645)
The detail page inferred "the user edited something" from "the draft differs
from the latest server skill". That difference has two independent causes —
the user typed, or the server moved — and the page could not tell them apart,
so it read both as a local edit. Two user-visible failures came out of it:
- A description ending in whitespace (what `description: |` frontmatter
yields, so every imported skill) compared unequal to itself, because the
dirty check trimmed the draft side and not the server side. The page opened
permanently dirty and Discard reseeded the same value, so the save bar could
not be dismissed at all — only Save cleared it, by rewriting the stored
description.
- Any remote update to an open skill was taken for a local edit, so the page
raised the conflict banner and refused to reseed. The editor stayed frozen
on pre-update text with no way to see what had changed, and saving from
there pushed the stale draft back over the newer version.
Record the seeded snapshot in `baselineRef` and compare against that instead.
With a baseline the two causes separate: `draft !== baseline` is a local edit,
and a new `updated_at` with no local edits is just a remote update, which now
reseeds silently. The conflict banner is left for the case it was written for
— a remote update landing on real unsaved work.
`toDraft` also trims name and description at the single seam where server data
becomes a draft, matching what Save persists, so later comparisons are plain
equality rather than a trim both sides have to remember. Content and file
bodies are not normalized: whitespace in a SKILL.md body is content. File sets
are compared through a path-sorted signature, since GET sorts files by path
while PUT echoes request order and that difference is not a content change.
Four of the five regression tests fail against the previous implementation;
the fifth covers the true-conflict path, which was already correct and must
stay that way.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: multica-agent <github@multica.ai>
* fix(skills): trim frontmatter name and description at the parse seam (MUL-5645)
Both fields are single-line labels everywhere they are consumed, but YAML clip
chomping gives `description: |` and `description: >` a trailing newline, so
imports stored a description that differed from its own trimmed form. The
detail page no longer depends on this — it normalizes when it seeds a draft —
but leaving it means every new import keeps writing the padded value, and any
future consumer that compares a stored description to a trimmed one inherits
the same trap.
Trimming here covers all four import paths (GitHub, skills.sh, archive,
runtime-local) in one place rather than asking each to remember. Callers that
need the raw SKILL.md still have it: `content` is stored untouched.
Defensive only. Reverting this commit alone does not reintroduce the bug.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: multica-agent <github@multica.ai>
* fix(skills): release the conflict when the user reverts their own edits (MUL-5645)
Review catch on the previous commit. Once a conflict was raised, the seed
effect only reconsidered when a new server version arrived, so a user who
resolved the conflict by hand — retyping the field back to what it was — got
stuck: the draft was clean again, and because the save bar renders only while
dirty, its Discard button unmounted. That left the banner sitting above stale
text with no control left to dismiss it, and no way forward short of a reload.
This state is a regression from measuring dirtiness locally. Previously the
draft was compared against the moved server value, so reverting still counted
as dirty and Discard stayed on screen.
Re-run the decision on draft changes too. When the local edits go away there
is nothing left to protect, so the page adopts the server version and clears
the banner — the same outcome as the never-edited case, reached a moment later.
The true-conflict path is unchanged, and its regression test still passes,
which is what keeps this from over-correcting into "always release".
Reseeding also grew a third caller, so the four pieces that have to move
together — draft, baseline, seeded key, conflict flag — are now assigned in
exactly one place, `adoptServerVersion`, used by first load, silent refresh,
Save and Discard alike.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: multica-agent <github@multica.ai>
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: multica-agent <github@multica.ai>
A skill_file row whose path is the skill's own SKILL.md (persisted by
older builds or direct create/update API calls) collides with the
primary content the daemon writes itself, failing task prep with
errPathPreExists on every non-codex local runtime (#3489).
#3526 guarded this with strings.EqualFold(path, "SKILL.md") at the
daemon write site and the three API ingress points, but the stored path
is not canonicalized: "./SKILL.md" or "sub/../SKILL.md" slip past the
exact-match guard while filepath.Join still resolves them onto the same
SKILL.md, so prep can still break.
Extract one canonical helper, skill.IsReservedContentPath, that cleans
the path before the case-insensitive compare, and use it at all four
sites (execenv writeSkillFiles, skill create, update, single-file
upsert). Add a daemon-side regression test for writeSkillFiles ignoring
a bundled SKILL.md (exact + "./" spellings) — the load-bearing fix
previously had only API-layer coverage — plus a unit test for the helper.
Existing poisoned rows are intentionally left in place (skipped at prep)
per the decision on MUL-2928.
MUL-2928
Follow-up to #3526; supersedes #3560.
Co-authored-by: J <j@multica.ai>
Co-authored-by: multica-agent <github@multica.ai>
The Go SKILL.md frontmatter parser unmarshalled into a {Name,Description}
string struct, so a non-scalar value (a list/map written where a scalar
belongs) made the whole decode fail and dropped even a valid sibling
`name`. The TS parser instead kept the name and JSON-encoded the value,
so the file-viewer (TS) and the import path (Go) could disagree about
the same SKILL.md.
Decode into a generic map and coerce per key on the Go side, mirroring
the TS coercion (scalars -> literal form, sequences/mappings -> JSON), so
both sides produce identical results and a structured value never
discards a sibling key. Rename ParseFrontmatter -> ParseSkillFrontmatter
to remove the cross-language name clash with the TS parseFrontmatter
(which returns {frontmatter, body}), and drop the unused TS
parseSkillFrontmatter export.
Add parity tests for sequence/mapping values plus name-only,
description-only, leading-blank-line and triple-dash-in-body edge cases
on both sides.
Follow-up to #3543 / MUL-2842.
Co-authored-by: J <j@multica.ai>
Co-authored-by: multica-agent <github@multica.ai>
Three independent line-based frontmatter parsers only handled
single-line `description: value`, so a YAML block scalar
(`description: |`) collapsed to the literal "|" and the rest of the
description was dropped before it ever reached the database.
Replace all three with real YAML decoders that understand block
scalars, folded scalars and quoted values:
- server/internal/skill: shared ParseFrontmatter via gopkg.in/yaml.v3,
used by both the handler import path and daemon local-skill discovery
- packages/core/skills: shared parseFrontmatter via the yaml package
- file-viewer renders multi-line frontmatter values (whitespace-pre-wrap)
Both parsers fall back to empty values on malformed YAML, preserving the
previous non-fatal behaviour.
