feat(core): Skills system foundation — loader + 4 starter skills#33
feat(core): Skills system foundation — loader + 4 starter skills#33
Conversation
There was a problem hiding this comment.
Findings
-
[Major] Invalid/malformed skills are silently dropped, violating the no-silent-fallback constraint and making configuration errors hard to diagnose in UI flows. Evidence
packages/core/src/skills/loader.ts:226(same pattern at:190,:214).
Suggested fix:const errors: string[] = []; // ...on parse/validation/read failures errors.push(`[skills] Invalid frontmatter in ${filePath}: ${result.error.message}`); // after loop if (errors.length > 0) { throw new CodesignError( `Skill loading failed:\n${errors.join('\n')}`, 'SKILL_LOAD_FAILED', ); }
-
[Major] Injection scope is chosen from only the first active skill, so mixed-scope active skills can be injected into the wrong channel depending on array order. Evidence
packages/providers/src/skill-injector.ts:85.
Suggested fix:const scopes = new Set(active.map((s) => s.frontmatter.trigger?.scope ?? 'system')); if (scopes.size > 1) { throw new CodesignError( 'Mixed skill scopes are not supported in one request', 'SKILL_SCOPE_CONFLICT', ); } const scope = [...scopes][0] ?? 'system';
Summary
- Review mode: initial
- Not found in repo/docs:
docs/VISION.mdanddocs/PRINCIPLES.md(mentioned by process docs), so policy-conformance checks were limited toCLAUDE.md+ changed code.
Testing
- Not run (automation)
- Suggested additions: Vitest for mixed active scopes conflict behavior, and Vitest for YAML literal block scalar (
|) newline-preservation behavior.
open-codesign Bot
|
|
||
| const result = SkillFrontmatterV1.safeParse(raw_fm); | ||
| if (!result.success) { | ||
| console.warn(`[skills] Invalid frontmatter in ${filePath}:`, result.error.issues); |
There was a problem hiding this comment.
This currently drops invalid skills with only a console warning (continue). In this project, silent fallbacks are disallowed. Please return/throw structured load errors so the caller can surface them in UI.
const errors: string[] = [];
errors.push(`[skills] Invalid frontmatter in ${filePath}: ${result.error.message}`);
if (errors.length > 0) throw new CodesignError('Skill loading failed', 'SKILL_LOAD_FAILED');| // Use the scope of the first active skill as the injection strategy. | ||
| // All skills in a single generation share one scope; mixing scopes is not | ||
| // supported — the system setting wins. | ||
| const scope = active[0]?.frontmatter.trigger?.scope ?? 'system'; |
There was a problem hiding this comment.
active[0] makes injection behavior order-dependent when active skills mix system and prefix. Please make this deterministic by rejecting mixed scopes (or explicitly prioritizing one scope).
const scopes = new Set(active.map((s) => s.frontmatter.trigger?.scope ?? 'system'));
if (scopes.size > 1) throw new CodesignError('Mixed skill scopes are not supported', 'SKILL_SCOPE_CONFLICT');
const scope = [...scopes][0] ?? 'system';There was a problem hiding this comment.
Findings
-
[Major] Non-ENOENT directory read failures are silently ignored, which violates the no-silent-fallback constraint and can hide real filesystem/config errors. Evidence
packages/core/src/skills/loader.ts:188andpackages/core/src/skills/loader.ts:192.
Suggested fix:let entries: string[]; try { entries = await readdir(dir); } catch (err) { if ((err as NodeJS.ErrnoException)?.code === 'ENOENT') return []; throw new CodesignError( `Could not read skills directory ${dir}: ${err instanceof Error ? err.message : String(err)}`, 'SKILL_LOAD_FAILED', { cause: err }, ); }
-
[Major] Injection scope is still order-dependent (
active[0]), so mixed-scope active skills can be injected into the wrong channel depending on array order. Evidencepackages/providers/src/skill-injector.ts:85.
Suggested fix:const scopes = new Set(active.map((s) => s.frontmatter.trigger?.scope ?? 'system')); if (scopes.size > 1) { throw new CodesignError( 'Mixed skill scopes are not supported in one request', 'SKILL_SCOPE_CONFLICT', ); } const scope = [...scopes][0] ?? 'system';
-
[Minor] Literal block scalars (
|) are flattened into one line, so YAML semantics do not match the parser contract and multiline content is lost. Evidencepackages/core/src/skills/loader.ts:60.
Suggested fix:function parseBlockScalar( lines: string[], start: number, baseIndent: number, style: 'folded' | 'literal', ): [string, number] { const blockLines: string[] = []; let i = start; while (i < lines.length) { const next = lines[i] ?? ''; if (next.trim() === '') { blockLines.push(''); i++; continue; } if (indentOf(next) <= baseIndent) break; blockLines.push(next.trim()); i++; } return [style === 'literal' ? blockLines.join('\n') : blockLines.join(' '), i]; }
Summary
- Review mode: follow-up after new commits
- Prior no-silent-fallback finding in loader validation is fixed, but silent fallback still exists for non-ENOENT directory read errors.
- Not found in repo/docs:
docs/VISION.mdanddocs/PRINCIPLES.md.
Testing
- Not run (automation)
- Suggested tests: (1)
loadSkillsFromDirthrows onEACCES/non-ENOENT readdir errors, (2) mixedsystem+prefixactive skills throw deterministic conflict error, (3)|block scalar preserves newlines.
open-codesign Bot
| let entries: string[]; | ||
| try { | ||
| entries = await readdir(dir); | ||
| } catch { |
There was a problem hiding this comment.
This catch currently converts any readdir failure (including permission/IO errors) into an empty skill set. That is a silent fallback and can mask real configuration failures. Please treat only ENOENT as optional-missing and throw SKILL_LOAD_FAILED for all other error codes.
| // Use the scope of the first active skill as the injection strategy. | ||
| // All skills in a single generation share one scope; mixing scopes is not | ||
| // supported — the system setting wins. | ||
| const scope = active[0]?.frontmatter.trigger?.scope ?? 'system'; |
There was a problem hiding this comment.
active[0] still makes scope selection order-dependent. If one active skill is system and another is prefix, behavior changes based on array order. Please detect mixed scopes and throw a deterministic conflict error (or explicitly resolve with a fixed precedence).
| blockLines.push(next.trim()); | ||
| i++; | ||
| } | ||
| return [blockLines.join(' ').trim(), i]; |
There was a problem hiding this comment.
parseBlockScalar always uses join(' '), so literal style (|) loses newlines. Since the parser contract says literal/folded are supported, this should preserve line breaks for | and only fold for >.
There was a problem hiding this comment.
Findings
-
[Blocker] Missing DCO sign-off on latest commit — this violates the repo hard constraint and can block merge. Evidence: commit
1a21df570e740f4cd3d4a4cf76b27a5de4b5ba77message has noSigned-off-by:trailer.
Suggested fix:git commit --amend -s --no-edit git push --force-with-lease
-
[Major] Mixed-scope skill injection is order-dependent, so behavior can change based on array order when both
systemandprefixskills are active, causing nondeterministic prompt shaping. Evidencepackages/providers/src/skill-injector.ts:85.
Suggested fix:const scopes = new Set(active.map((s) => s.frontmatter.trigger?.scope ?? 'system')); if (scopes.size > 1) { throw new CodesignError( 'Mixed skill scopes are not supported in one request', 'SKILL_SCOPE_CONFLICT', ); } const scope = [...scopes][0] ?? 'system';
-
[Minor] Literal block scalar semantics are lost (
|is flattened), which contradicts the parser contract and can corrupt multiline skill frontmatter values. Evidencepackages/core/src/skills/loader.ts:60.
Suggested fix:const isLiteral = afterTrimmed.startsWith('|'); // for '|' keep newlines, for '>' fold to spaces return [isLiteral ? blockLines.join('\n').trim() : blockLines.join(' ').trim(), i];
Summary
- Review mode: follow-up after new commits
- Two prior issues remain unresolved in current head (scope conflict handling and
|scalar preservation). - Not found in repo/docs:
docs/VISION.md,docs/PRINCIPLES.md.
Testing
- Not run (automation)
- Suggested tests: mixed
system+prefixactive skills should throw deterministic conflict; literal|scalar should preserve line breaks.
open-codesign Bot
| // Use the scope of the first active skill as the injection strategy. | ||
| // All skills in a single generation share one scope; mixing scopes is not | ||
| // supported — the system setting wins. | ||
| const scope = active[0]?.frontmatter.trigger?.scope ?? 'system'; |
There was a problem hiding this comment.
Using active[0] makes scope resolution order-dependent when both system and prefix skills are active. Please detect mixed scopes and throw a deterministic conflict error, e.g.
const scopes = new Set(active.map((s) => s.frontmatter.trigger?.scope ?? 'system'));
if (scopes.size > 1) throw new CodesignError('Mixed skill scopes are not supported in one request', 'SKILL_SCOPE_CONFLICT');| blockLines.push(next.trim()); | ||
| i++; | ||
| } | ||
| return [blockLines.join(' ').trim(), i]; |
There was a problem hiding this comment.
parseBlockScalar always flattens with spaces, so literal style (|) loses newlines. Please preserve line breaks for | and only fold for >, for example:
const isLiteral = afterTrimmed.startsWith('|');
return [isLiteral ? blockLines.join('\n').trim() : blockLines.join(' ').trim(), i];1a21df5 to
1540ef6
Compare
There was a problem hiding this comment.
Findings
-
[Major] Mixed-scope skill injection is order-dependent, which can change prompt-shaping behavior based only on array order when both
systemandprefixskills are active. This makes behavior nondeterministic and can silently route instructions to the wrong channel. Evidencepackages/providers/src/skill-injector.ts:85.
Suggested fix:const scopes = new Set(active.map((s) => s.frontmatter.trigger?.scope ?? 'system')); if (scopes.size > 1) { throw new CodesignError( 'Mixed skill scopes are not supported in one request', 'SKILL_SCOPE_CONFLICT', ); } const scope = [...scopes][0] ?? 'system';
-
[Minor] Literal block scalar (
|) semantics are not preserved in frontmatter parsing; multiline values are always flattened with spaces. This can corrupt user-authored skill metadata that depends on newline preservation. Evidencepackages/core/src/skills/loader.ts:60.
Suggested fix:function parseBlockScalar( lines: string[], start: number, baseIndent: number, mode: 'folded' | 'literal', ): [string, number] { // ... collect blockLines return [mode === 'literal' ? blockLines.join('\n').trim() : blockLines.join(' ').trim(), i]; }
Summary
- Review mode: follow-up after new commits
- Two issues remain in current head: mixed-scope conflict handling and
|literal scalar preservation. - Not found in repo/docs:
docs/VISION.md,docs/PRINCIPLES.md.
Testing
- Not run (automation)
- Suggested tests: mixed
system+prefixactive skills should throwSKILL_SCOPE_CONFLICT; literal|block scalar should preserve line breaks.
open-codesign Bot
| // Use the scope of the first active skill as the injection strategy. | ||
| // All skills in a single generation share one scope; mixing scopes is not | ||
| // supported — the system setting wins. | ||
| const scope = active[0]?.frontmatter.trigger?.scope ?? 'system'; |
There was a problem hiding this comment.
active[0] makes scope resolution order-dependent when active skills contain both system and prefix scopes. Please detect mixed scopes and throw a deterministic SKILL_SCOPE_CONFLICT error before choosing the injection strategy.
| blockLines.push(next.trim()); | ||
| i++; | ||
| } | ||
| return [blockLines.join(' ').trim(), i]; |
There was a problem hiding this comment.
parseBlockScalar() always flattens with spaces, so YAML literal style (|) loses newlines. Please preserve line breaks for | and only fold whitespace for > style scalars.
1540ef6 to
cbe7e15
Compare
There was a problem hiding this comment.
Findings
-
[Major] Mixed-scope skills are merged into one block and injected using only the first skill's scope, so skills declaring the other scope are silently injected into the wrong channel. This can materially change model behavior and violates each skill's
trigger.scopecontract. Evidencepackages/providers/src/skill-injector.ts:105,packages/providers/src/skill-injector.ts:110.
Suggested fix:const systemSkills = active.filter((s) => (s.frontmatter.trigger?.scope ?? 'system') === 'system'); const prefixSkills = active.filter((s) => (s.frontmatter.trigger?.scope ?? 'system') === 'prefix'); let out = baseMessages; if (systemSkills.length > 0) out = prependSystemContent(out, buildSkillBlock(systemSkills)); if (prefixSkills.length > 0) out = prependUserContent(out, buildSkillBlock(prefixSkills)); return out;
-
[Minor] YAML literal block style (
|) is not preserved:parseBlockScalaralways joins with spaces, collapsing line breaks. This corrupts valid frontmatter values that intentionally use literal multiline text. Evidencepackages/core/src/skills/loader.ts:60.
Suggested fix:function parseBlockScalar( lines: string[], start: number, baseIndent: number, mode: 'folded' | 'literal', ): [string, number] { // ...collect blockLines return [mode === 'literal' ? blockLines.join('\n').trim() : blockLines.join(' ').trim(), i]; }
Summary
- Review mode: follow-up after new commits
- Two issues remain in current head: mixed-scope injection collapses per-skill scope semantics; YAML literal block style is not preserved.
Testing
- Not run (automation)
- Suggested tests: mixed
system+prefixskills should inject into both channels deterministically;|scalar parsing should preserve embedded newlines.
open-codesign Bot
| // Mixed `system`/`prefix` scopes resolve to the highest-precedence skill | ||
| // (project > user > builtin, then alphabetical), so the chosen channel is a | ||
| // function of the skill set rather than caller array order. | ||
| const scope = active[0]?.frontmatter.trigger?.scope ?? 'system'; |
There was a problem hiding this comment.
active[0] chooses a single scope for the whole merged block, so any skill with the other scope is injected into the wrong channel. Please split active by trigger.scope and inject each block to its own channel (system and/or prefix) instead of collapsing to one scope.
const systemSkills = active.filter((s) => (s.frontmatter.trigger?.scope ?? 'system') === 'system');
const prefixSkills = active.filter((s) => (s.frontmatter.trigger?.scope ?? 'system') === 'prefix');| blockLines.push(next.trim()); | ||
| i++; | ||
| } | ||
| return [blockLines.join(' ').trim(), i]; |
There was a problem hiding this comment.
parseBlockScalar() always flattens with spaces, so YAML literal style (|) loses line breaks. Please preserve newlines for | and fold only > style scalars.
return [mode === 'literal' ? blockLines.join('\n').trim() : blockLines.join(' ').trim(), i];cbe7e15 to
dce9ca9
Compare
There was a problem hiding this comment.
Findings
- [Minor] YAML literal block style (
|) is not preserved:parseBlockScalar()always joins lines with spaces, which collapses intended newlines and changes valid frontmatter semantics. Evidencepackages/core/src/skills/loader.ts:60,packages/core/src/skills/loader.ts:145.
Suggested fix:function parseBlockScalar( lines: string[], start: number, baseIndent: number, mode: 'folded' | 'literal', ): [string, number] { // ... collect blockLines return [mode === 'literal' ? blockLines.join('\n').trim() : blockLines.join(' ').trim(), i]; } // call site const mode = afterTrimmed.trim().startsWith('|') ? 'literal' : 'folded'; const [value, nextI] = parseBlockScalar(lines, i, baseIndent, mode);
Summary
- Review mode: follow-up after new commits
- 1 issue remains in the current head: literal YAML block (
|) handling is lossy. - Context files
docs/VISION.mdanddocs/PRINCIPLES.md: Not found in repo/docs.
Testing
- Not run (automation)
- Suggested tests: add a loader test asserting
description: |preserves embedded\nbetween lines.
open-codesign Bot
| blockLines.push(next.trim()); | ||
| i++; | ||
| } | ||
| return [blockLines.join(' ').trim(), i]; |
There was a problem hiding this comment.
parseBlockScalar() always returns blockLines.join(' '), which collapses YAML literal scalars (|) into a single line. This is lossy for valid frontmatter content.
Suggested fix:
function parseBlockScalar(lines: string[], start: number, baseIndent: number, mode: 'folded' | 'literal'): [string, number] {
// ...
return [mode === 'literal' ? blockLines.join('\n').trim() : blockLines.join(' ').trim(), i];
}
const mode = afterTrimmed.trim().startsWith('|') ? 'literal' : 'folded';
const [value, nextI] = parseBlockScalar(lines, i, baseIndent, mode);dce9ca9 to
63a3b1c
Compare
There was a problem hiding this comment.
Findings
- [Minor] YAML literal block (
|) semantics are still lost in frontmatter parsing —parseBlockScalar()always joins lines with spaces, so literal blocks no longer preserve line breaks, which can change skill descriptions/instructions unexpectedly. Evidencepackages/core/src/skills/loader.ts:60,packages/core/src/skills/loader.ts:145.
Suggested fix:function parseBlockScalar( lines: string[], start: number, baseIndent: number, mode: 'folded' | 'literal', ): [string, number] { const blockLines: string[] = []; let i = start; while (i < lines.length) { const next = lines[i] ?? ''; if (next.trim() === '') { blockLines.push(''); i++; continue; } if (indentOf(next) <= baseIndent) break; blockLines.push(next.trim()); i++; } return [mode === 'literal' ? blockLines.join('\n').trim() : blockLines.join(' ').trim(), i]; } // call site const mode = afterTrimmed.startsWith('|') ? 'literal' : 'folded'; const [value, nextI] = parseBlockScalar(lines, i, baseIndent, mode);
Summary
- Review mode: follow-up after new commits
- 1 issue found in added code: literal-block frontmatter parsing is lossy.
- Context files
docs/VISION.mdanddocs/PRINCIPLES.md: Not found in repo/docs.
Testing
- Not run (automation)
open-codesign Bot
| blockLines.push(next.trim()); | ||
| i++; | ||
| } | ||
| return [blockLines.join(' ').trim(), i]; |
There was a problem hiding this comment.
parseBlockScalar() currently collapses literal (|) block scalars by joining with spaces. Please preserve | blocks with \n joins while keeping folded (>) as space-joined.
- packages/shared/src/skills.ts: SkillFrontmatterV1 zod schema + LoadedSkill interface (canonical location; avoids core→providers circular dep) - packages/core/src/skills/: inline YAML frontmatter parser, loadSkillsFromDir, loadAllSkills with 3-tier priority (project > user > builtin), loader tests - packages/core/src/skills/builtin/: 4 starter skills (frontend-design-anti-slop, pitch-deck, data-viz-recharts, mobile-mock) — self-written, Apache-2.0, no Anthropic SKILL.md text copied - packages/providers/src/skill-injector.ts: injectSkillsIntoMessages, pure, supports system and prefix scope, provider-wildcard matching, injector tests No new runtime deps added. Inline YAML parser (~100 lines) handles folded scalars, nested mappings, inline + block sequences. Signed-off-by: hqhq1025 <[email protected]>
… drop Replace the three silent continue/console.warn paths in loadSkillsFromDir with error collection; after processing all files, throw CodesignError 'SKILL_LOAD_FAILED' if any errors were accumulated. Update tests to expect the throw, and add a new loadAllSkills test for a broken skill (missing description field). Signed-off-by: hqhq1025 <[email protected]>
Signed-off-by: hqhq1025 <[email protected]>
Mixed-scope skill injection was order-dependent: whichever skill appeared first in the caller's array decided whether the block went into the system prompt or the first user message. That made prompt-shaping behaviour a function of loader iteration order rather than the active skill set. Sort skills by source precedence (project > user > builtin) and then alphabetical name before building the block. The chosen scope, the concatenated body, and the resulting prompt blob are now byte-identical across runs regardless of input order, which also stabilises prompt caching and snapshot tests. Adds a vitest case that feeds three random permutations of a five-skill fixture and asserts the resulting system content is byte-identical to the canonical sort, plus a mixed system/prefix scope test that confirms the higher-precedence skill picks the channel. Signed-off-by: hqhq1025 <[email protected]>
Previously a mixed system/prefix skill set was concatenated into a single block and injected via the highest-precedence skill's channel, silently routing the rest into the wrong channel and violating each skill's trigger.scope contract. Now we partition active skills by scope and inject system-scope skills into the system message and prefix-scope skills into the first user message, preserving canonical order within each channel. Signed-off-by: hqhq1025 <[email protected]>
63a3b1c to
40645f9
Compare
There was a problem hiding this comment.
Findings
- [Minor] YAML literal block (
|) semantics are not preserved —parseBlockScalar()always joins block lines with spaces, so literal blocks lose intended newlines and can alter skill instructions before prompt injection. Evidencepackages/core/src/skills/loader.ts:46,packages/core/src/skills/loader.ts:60,packages/core/src/skills/loader.ts:145.
Suggested fix:function parseBlockScalar( lines: string[], start: number, baseIndent: number, mode: 'folded' | 'literal', ): [string, number] { const blockLines: string[] = []; let i = start; while (i < lines.length) { const next = lines[i] ?? ''; if (next.trim() === '') { blockLines.push(''); i++; continue; } if (indentOf(next) <= baseIndent) break; blockLines.push(next.trim()); i++; } return [mode === 'literal' ? blockLines.join('\n').trim() : blockLines.join(' ').trim(), i]; } const mode = afterTrimmed.startsWith('|') ? 'literal' : 'folded'; const [value, nextI] = parseBlockScalar(lines, i, baseIndent, mode);
Summary
Review mode: follow-up after new commits
- 1 issue found in changed lines.
docs/VISION.mdanddocs/PRINCIPLES.md: Not found in repo/docs.
Testing
- Not run (automation)
open-codesign Bot
| blockLines.push(next.trim()); | ||
| i++; | ||
| } | ||
| return [blockLines.join(' ').trim(), i]; |
There was a problem hiding this comment.
Literal (|) YAML blocks should preserve newlines, but this parser path currently collapses lines with spaces.
Suggested minimal change:
const mode = afterTrimmed.startsWith('|') ? 'literal' : 'folded';
const [value, nextI] = parseBlockScalar(lines, i, baseIndent, mode);
// ...inside parseBlockScalar return mode === 'literal' ? blockLines.join('\n').trim() : blockLines.join(' ').trim();There was a problem hiding this comment.
Findings
- No high-confidence issues found in added/modified lines of the current diff.
Summary
- Review mode: follow-up after new commits
- No new actionable issues detected in this revision.
- Residual risk/testing gap: unable to execute Vitest/Playwright in this runner (
pnpmunavailable), so runtime behavior was validated by static analysis only. docs/VISION.mdanddocs/PRINCIPLES.md: Not found in repo/docs.
Testing
- Not run (automation)
open-codesign Bot
* feat(core): skills loader + 4 starter skills + provider injector (PR-A) - packages/shared/src/skills.ts: SkillFrontmatterV1 zod schema + LoadedSkill interface (canonical location; avoids core→providers circular dep) - packages/core/src/skills/: inline YAML frontmatter parser, loadSkillsFromDir, loadAllSkills with 3-tier priority (project > user > builtin), loader tests - packages/core/src/skills/builtin/: 4 starter skills (frontend-design-anti-slop, pitch-deck, data-viz-recharts, mobile-mock) — self-written, Apache-2.0, no Anthropic SKILL.md text copied - packages/providers/src/skill-injector.ts: injectSkillsIntoMessages, pure, supports system and prefix scope, provider-wildcard matching, injector tests No new runtime deps added. Inline YAML parser (~100 lines) handles folded scalars, nested mappings, inline + block sequences. Signed-off-by: hqhq1025 <[email protected]> * fix(core): skills loader collects errors and throws instead of silent drop Replace the three silent continue/console.warn paths in loadSkillsFromDir with error collection; after processing all files, throw CodesignError 'SKILL_LOAD_FAILED' if any errors were accumulated. Update tests to expect the throw, and add a new loadAllSkills test for a broken skill (missing description field). Signed-off-by: hqhq1025 <[email protected]> * fix(core): propagate non-ENOENT errors in skills loader Signed-off-by: hqhq1025 <[email protected]> * fix(providers): sort skills into canonical order before injection Mixed-scope skill injection was order-dependent: whichever skill appeared first in the caller's array decided whether the block went into the system prompt or the first user message. That made prompt-shaping behaviour a function of loader iteration order rather than the active skill set. Sort skills by source precedence (project > user > builtin) and then alphabetical name before building the block. The chosen scope, the concatenated body, and the resulting prompt blob are now byte-identical across runs regardless of input order, which also stabilises prompt caching and snapshot tests. Adds a vitest case that feeds three random permutations of a five-skill fixture and asserts the resulting system content is byte-identical to the canonical sort, plus a mixed system/prefix scope test that confirms the higher-precedence skill picks the channel. Signed-off-by: hqhq1025 <[email protected]> * fix(providers): inject mixed-scope skills into separate channels Previously a mixed system/prefix skill set was concatenated into a single block and injected via the highest-precedence skill's channel, silently routing the rest into the wrong channel and violating each skill's trigger.scope contract. Now we partition active skills by scope and inject system-scope skills into the system message and prefix-scope skills into the first user message, preserving canonical order within each channel. Signed-off-by: hqhq1025 <[email protected]> * fix(core): preserve newlines in YAML literal block scalar (PR #33 codex Minor) --------- Signed-off-by: hqhq1025 <[email protected]>
* feat(core): system prompt — adopt Claude Design patterns Adds a new prompt section that embeds high-leverage craft directives paraphrased from patterns observed in the publicly leaked Claude Design system prompt. Direct response to feedback that generated designs are sparse, generic, and monotone. The new section sits between tweaks-protocol and anti-slop in the composer, applies to all create/revise/tweak modes, and codifies: - silent artifact-type classification (landing / dashboard / case study / pricing / deck / etc.) controlling section ladder and density target - density floor — default to "rich", drop only on explicit "minimal" - real, specific content — concrete bans on common placeholder strings - before/after side-by-side rendering when comparison is implied - big-number visual blocks (display weight + label + delta + sparkline) - three-family typography ladder (display + sans + mono) - dark-theme warmth requirements (accent + gradient + transparent borders) - SVG monogram/wordmark for logos (no emoji, no flat circles) - distinguished customer-quote treatment - single-page structure ladder (hero → trust → 3-5 sections → focal → closing CTA), with dashboard and slide deck substitutions Directives are independently authored — no original Claude Design text is reproduced verbatim. Attribution and the underlying structural analysis live in docs/research/15-claude-design-prompts.md (gitignored, internal). Vitest coverage: - new test asserts all ten directive headers appear in the create-mode composed prompt - existing drift test ensures the new .txt and the inlined TS constant stay byte-identical Signed-off-by: hqhq1025 <[email protected]> * fix(core): rename 'Claude-Design-style' → 'Craft directives' to remove provenance comment per codex review Signed-off-by: hqhq1025 <[email protected]> * test(core): cover revise mode for craft directives (PR #43 codex Minor) * fix(desktop): replace hardcoded text-[Npx] with --text-* tokens in preview/inline-comment (#57) Codex has flagged this exact pattern as a Blocker on PRs #50 and #51 ("hardcoded pixel font size violates token-only UI constraint"). Sweeps the chat-adjacent surface that is not covered by any in-flight worktree. Mappings: - text-[11px] -> text-[var(--text-xs)] (12px, closest token) - text-[12px] -> text-[var(--text-xs)] - text-[13px] -> text-[var(--text-sm)] Files touched: InlineCommentComposer.tsx, PreviewToolbar.tsx, PreviewPane.tsx. The bg-[rgba(255,255,255,0.88)] frosted pill in PreviewPane is left for a follow-up because it requires a new translucent surface token. * fix(desktop): localize Toast dismiss button aria-label (#56) Codex flagged a recurring i18n violation pattern across PRs #48 and #49: hardcoded English ARIA strings bypass the i18n layer and ship inconsistent screen-reader output to localized users. The toast close button was the last remaining hardcoded aria-label outside the onboarding flow. - Add common.dismissNotification key (en + zh-CN) - Wire useT() into ToastItem and consume the new key * fix(desktop): tokenize ConnectionStatusDot tooltip values (#58) * chore(desktop): biome auto-format InlineCommentComposer (unblock pre-push) * fix(desktop): tokenize ConnectionStatusDot tooltip hardcoded values * fix(desktop): tokenize LanguageToggle hardcoded sizes/spacing (#60) * fix(desktop): tokenize LanguageToggle hardcoded sizes/spacing * chore(desktop): biome format InlineCommentComposer (drive-by, unblocks pre-push) * [Claude Design adoption] PR-B: examples gallery (#50) * feat(hub): examples gallery as first-class section PR-B from doc 28 (Claude Design adoption). Ships eight curated examples (cosmic animation, organic loaders, landing page, case study, dashboard, pitch slide, welcome email, mobile habit tracker) with stylised inline SVG thumbnails, an `ExamplesTab` view component, and full en + zh-CN translations for every title, description, category label, and surrounding chrome. The tab is self-contained: a single `onUsePrompt` prop hands the chosen example back to the host so PR-A can wire it into the hub without further plumbing. No App.tsx changes here — independent of PR-A landing first. Compatibility ✅ Upgradeability ✅ No bloat ✅ Elegance ✅ Signed-off-by: hqhq1025 <[email protected]> * fix(hub): use --font-size-body-xs token for example card category badge Replaces hardcoded text-[10px] with the existing --font-size-body-xs typography token (11px). Resolves Codex token-only UI constraint blocker on PR #50. Signed-off-by: hqhq1025 <[email protected]> * fix(templates): throw on missing locale content for examples (no silent fallback) Codex blocker on PR #50: getExamples returned `{ title: id, description: '' }` when both the requested locale and the en fallback lacked an entry, shipping degraded UI without surfacing the bug. - Throw a descriptive Error when no registry has the example id - Add vitest case asserting the throw path - Drive-by: biome format apps/desktop/src/renderer/src/components/InlineCommentComposer.tsx (pre-existing format drift on main blocking pre-push) Signed-off-by: hqhq1025 <[email protected]> --------- Signed-off-by: hqhq1025 <[email protected]> * fix(desktop): tokenize PreviewToolbar values missed by #57 (#62) * fix(desktop): tokenize remaining PreviewToolbar hardcoded values missed by #57 Replaces three remaining hardcoded px values in PreviewToolbar with design tokens to keep the export menu aligned with the token system: - h-[30px] → h-[var(--size-control-sm)] - w-[14px] h-[14px] → w-[var(--size-icon-sm)] h-[var(--size-icon-sm)] - min-w-[200px] → min-w-[var(--size-stage-min)] Adds a new --size-stage-min (200px) token to packages/ui for menu/popover minimum widths. The other three font-size values noted in the original audit were already tokenized by #57. * chore: format Download icon and silence pre-existing core complexity lint - Wrap PreviewToolbar Download icon across multiple lines per Biome formatter - Add biome-ignore for pre-existing noExcessiveCognitiveComplexity in packages/core/src/index.ts runModel (blocks pre-push hook; refactor tracked separately, mirrors the precedent set in 9051fae) * fix(desktop): drop unused fileURLToPath import in main entry (#63) * fix(desktop): PreviewPane hint pill + iframe respect dark mode (#69) Co-authored-by: Claude <[email protected]> * fix(desktop): i18n ThemeToggle aria/tooltip strings (#70) Replace hardcoded English with t() calls; add theme.{toggleAria,switchToLight,switchToDark} to en.json and zh-CN.json. Co-authored-by: Claude <[email protected]> * fix(desktop): tokenize raw utilities in preview Loading/Error states (#64) * fix(desktop): tokenize raw utilities in preview Loading/Error states Signed-off-by: hqhq1025 <[email protected]> * fix(desktop): tokenize space-y-3 in LoadingState skeleton header Signed-off-by: hqhq1025 <[email protected]> --------- Signed-off-by: hqhq1025 <[email protected]> * feat(desktop): Snapshots SQLite schema + IPC handlers (PR-A) (#29) * feat(desktop): snapshots SQLite + IPC foundation (PR-A of version history) - packages/shared: DesignSnapshotV1 + DesignV1 Zod schemas, SnapshotCreateInput interface - apps/desktop: better-sqlite3 persistence (designs + design_snapshots tables, WAL, FK cascade) initSnapshotsDb(path) for production, initInMemoryDb() for tests - snapshots:v1:* IPC handlers: list-designs, create-design, list, get, create, delete All reject malformed payloads with CodesignError('IPC_BAD_INPUT') - preload: window.codesign.snapshots namespace bridged to renderer - Vitest: 26 new tests across snapshots-db + snapshots-ipc (111 pass total) New dep: better-sqlite3@^11 (MIT, native, already in CLAUDE.md stack) No ulid added — using crypto.randomUUID() instead. PR-B will wire auto-snapshot-on-sendPrompt + history sidebar UI. Signed-off-by: hqhq1025 <[email protected]> * style(desktop): fix biome formatting in snapshots-db.test.ts Signed-off-by: hqhq1025 <[email protected]> * fix(desktop): address codex snapshots cascade/parent/sort findings - Enable PRAGMA foreign_keys = ON in applySchema so ON DELETE CASCADE / SET NULL fire in production (previously only the test enabled it manually, hiding the regression). - Constrain design_snapshots.parent_id with a self-referential FK (ON DELETE SET NULL) and validate in the IPC layer that parentId resolves to a snapshot in the same design — prevents silent history corruption from stale or cross-design ids. - Sort listDesigns by updated_at DESC, created_at DESC so designs bubble after new snapshots are added (createSnapshot already bumps updated_at). - Drop the now-redundant manual pragma in the cascade test and add coverage for parent SET NULL, cross-design parent rejection, and the activity-based design sort. Signed-off-by: hqhq1025 <[email protected]> * fix(desktop): reject invalid create-design input instead of silent default The snapshots:v1:create-design IPC handler coerced empty/non-string payloads to 'Untitled design', hiding caller bugs and violating the no-silent-fallback rule. Reject with IPC_BAD_INPUT instead, drop the matching preload coercion, and update tests to cover the new contract. Signed-off-by: hqhq1025 <[email protected]> * fix(desktop): translate SQLite errors to typed IPC contract in snapshots-ipc Wraps every snapshot DB call in a translateSqliteError helper that maps better-sqlite3 SqliteError codes to typed CodesignError instances: - SQLITE_CONSTRAINT_FOREIGNKEY -> IPC_BAD_INPUT - SQLITE_BUSY / SQLITE_LOCKED -> IPC_DB_BUSY - SQLITE_FULL -> IPC_DB_FULL - other -> IPC_DB_ERROR (full details logged server-side) Renderer no longer sees raw provider error strings. Adds vitest cases that stub better-sqlite3 prepare() to throw each code and assert the right CodesignError is surfaced. Signed-off-by: hqhq1025 <[email protected]> * fix(snapshots-ipc): map SQLite constraint subcodes individually Bare SQLITE_CONSTRAINT also covers UNIQUE / NOT NULL / CHECK violations, so translating it as "Parent snapshot does not exist" misled the UI on unrelated failures. Match each subcode explicitly: - SQLITE_CONSTRAINT_FOREIGNKEY -> IPC_BAD_INPUT, parent-snapshot message - SQLITE_CONSTRAINT_UNIQUE/PK -> IPC_CONFLICT, "Snapshot already exists" - SQLITE_CONSTRAINT_NOTNULL/CHECK -> IPC_BAD_INPUT, neutral constraint message - bare SQLITE_CONSTRAINT (no suffix) -> generic IPC_DB_ERROR Adds vitest coverage for each new branch. Signed-off-by: hqhq1025 <[email protected]> * fix(snapshots-ipc): clarify FK error covers design and parent SQLITE_CONSTRAINT_FOREIGNKEY fires for both a missing design_id and a missing parent_id, but the previous translation always reported "Parent snapshot does not exist" — leading contributors to look for the wrong cause. Key the FK message by call-site context and use a message that names both columns ("Referenced design or parent snapshot does not exist"); fall back to a generic "Referenced item does not exist" for unmapped contexts. Also extracts the static SQLite-code lookup into a small helper to keep translateSqliteError below the cognitive-complexity threshold. Signed-off-by: hqhq1025 <[email protected]> * fix(desktop): schema-version snapshots:v1 IPC payloads Every snapshots:v1:* object payload now carries `schemaVersion: 1`, both on the wire (preload bridge) and in the main-process parser. Mismatched or missing versions throw IPC_BAD_INPUT so future handler revisions can break cleanly instead of silently mis-parsing legacy callers. - snapshots-ipc.ts: requireSchemaV1() helper applied to list-designs, list, get, create, delete, create-design (now object-shaped). - preload/index.ts: every snapshots invoke wraps the payload with { schemaVersion: 1, ... }. - snapshots-ipc.test.ts: updated existing fixtures via a v1() helper and added a parameterised gating suite that asserts every channel rejects missing and mismatched schemaVersion values. Addresses Codex Major review on PR #29. Signed-off-by: hqhq1025 <[email protected]> * fix(desktop): degrade gracefully when snapshots DB init fails Previously initSnapshotsDb() ran inside app.whenReady() without a guard, so any open/migration/native-binding failure rejected the boot promise and prevented createWindow() from ever firing — the user got a silent no-window app. Add safeInitSnapshotsDb() wrapper that captures the error, then in main boot: log it with full stack via electron-log, surface it through dialog.showErrorBox, skip registerSnapshotsIpc, and continue to open the window. Snapshot-dependent renderer features will fail loudly via their IPC channels, but the rest of the app stays usable. Also reset the singleton if applySchema throws so a retry can recover instead of returning a half-open DB handle. Signed-off-by: hqhq1025 <[email protected]> * fix(desktop): typed SNAPSHOTS_UNAVAILABLE stub when snapshots DB init fails When safeInitSnapshotsDb fails at boot, main/index.ts previously skipped registerSnapshotsIpc entirely. Renderer calls to window.codesign.snapshots.* then surfaced as Electron's opaque "No handler registered" rejection, violating the no-silent-fallback / error-context requirement (PR #29 codex Major). Install registerSnapshotsUnavailableIpc stubs for the same channel set so every renderer call rejects with a typed CodesignError carrying code SNAPSHOTS_UNAVAILABLE and a message pointing the user at Settings → Storage. The channel list is exported (SNAPSHOTS_CHANNELS_V1) so the test can pin it to the live registration set and prevent drift. Adds vitest coverage that asserts SNAPSHOTS_UNAVAILABLE is thrown for every channel and that the stub set matches registerSnapshotsIpc exactly. Signed-off-by: hqhq1025 <[email protected]> --------- Signed-off-by: hqhq1025 <[email protected]> * feat(exporters): Markdown export of generated artifact (#66) * feat(exporters): add Markdown export with simple HTML→MD conversion Adds a tier-1 Markdown exporter (.md with YAML frontmatter carrying schemaVersion: 1). Conversion is a small set of regex passes covering h1..h6, p, a, img, ul/ol, strong/em, code/pre — anything else is stripped. Zero new runtime deps. Wired through the existing exporter IPC + Preview toolbar Export menu, with en + zh-CN i18n strings. Compatibility ✅ Upgradeability ✅ No bloat ✅ Elegance ✅ Signed-off-by: hqhq1025 <[email protected]> * fix(exporters): allowlist URL schemes in markdown export Sanitize <a href> and <img src> during htmlToMarkdown so unsafe schemes (javascript:, vbscript:, file:, non-image data:, etc.) cannot ride into the exported .md and execute via downstream renderers. Allow http(s), mailto, relative URLs, fragments; permit data:image/* only on <img>. Unsafe links collapse to plain text, unsafe images are dropped. Also fix the IPC default extension for the markdown format (`design.md` instead of `design.markdown`) when no defaultFilename is provided. Addresses codex review on PR #66. Signed-off-by: hqhq1025 <[email protected]> * fix(exporters): close encoded-scheme bypass in markdown sanitizeUrl Decode HTML entities (named, hex, decimal), URL %escapes, and strip control characters (TAB/CR/LF/etc.) before scheme allowlisting so payloads like javascript:, %6Aavascript:, JavaScript:, and tab- prefixed schemes can no longer slip through the link/image sanitizer. Adds regression tests covering each bypass vector. Signed-off-by: hqhq1025 <[email protected]> * fix(exporters): only decode scheme prefix in sanitizeUrl The previous follow-up ran decodeURIComponent on the entire URL which both threw on legitimate inputs containing literal `%` (dropping the link entirely) and rewrote percent-escaped path/query characters such as %2F or %C3%A9. Restrict entity + percent decoding to the scheme portion (before the first colon) used solely for the safety check, and emit the original (control-stripped, trimmed) URL when the scheme is allowlisted. Adds regression tests for %2F in query, UTF-8 percent-escapes in path, literal trailing %, and confirms the existing entity / %-encoded javascript bypass guards still strip dangerous schemes. Signed-off-by: hqhq1025 <[email protected]> --------- Signed-off-by: hqhq1025 <[email protected]> * [Claude Design adoption] PR-A: designs hub + multi-type create wizard (#51) * feat(desktop): designs hub + multi-type create wizard (Claude Design adoption PR-A) Replaces the straight-to-composer launch flow with a designs hub modeled on Claude Design's Recent / Your designs / Examples / Design systems navigation plus a typed create wizard (Prototype / Slide deck / From template / Other). - Hub view with four sibling tabs and a primary "New design" CTA - Modal create flow; CTA stays disabled until a project name is provided - Per-type forms; PR-F will fill in the wireframe vs high-fidelity cards - Examples and Design systems tabs ship as placeholders for PR-B / PR-C - Project schema lives in shared with schemaVersion=1; persisted to localStorage for now (SQLite migration arrives with PR-C) - Full en + zh-CN coverage; tokens from packages/ui (no hardcoded values) - Vitest covering the create-draft logic; existing 144-test suite untouched Compatibility: green - no IPC/main changes, no schema breaks. Upgradeability: green - schemaVersion field on every Project payload. No bloat: green - no new dependencies; reuses lucide-react + zustand. Elegance: green - single store action per intent; per-type forms < 40 LOC. Signed-off-by: hqhq1025 <[email protected]> * fix(desktop): surface project storage errors via toast + warn (no silent fallback) - readStoredProjects/persistProjects now console.warn and return an error string instead of swallowing exceptions; createProject pushes an error toast on persist failure while keeping in-memory state consistent. - Validate stored projects with the Project zod schema (safeParse) and count rejected records so corrupted entries surface a toast instead of silently disappearing. - openProject resets project-scoped workspace state (messages, preview, inputs, generation flags) to prevent cross-project state leakage. - Add errors.projectStorageFailed i18n key (en + zh-CN). - Vitest: mock localStorage.setItem to throw, assert toast pushed and the new project is still added to in-memory state. Addresses Codex blocker on PR #51. Signed-off-by: hqhq1025 <[email protected]> * fix(desktop): replace hardcoded checkbox sizing with --size-icon-md token SlideDeckForm checkbox used `w-4 h-4` literals which violate the token-only UI constraint. Swap to `var(--size-icon-md)` (16px) so the control scales with centralized theming. Addresses Codex blocker on PR #51. Signed-off-by: hqhq1025 <[email protected]> * chore: clear pre-existing biome errors blocking pre-push hook - tailwindExtractor: replace non-null assertion with safe cast - InlineCommentComposer: apply formatter Signed-off-by: hqhq1025 <[email protected]> * fix(desktop): wire ExamplesTab onUsePrompt after rebase onto PR-B PR-B merged ExamplesTab as a real component requiring an onUsePrompt callback. After rebase, surface that prop through HubView and have App prefill the workspace prompt + switch view. Signed-off-by: hqhq1025 <[email protected]> * fix(desktop): reset project-scoped workspace state in createProject createProject was only clearing messages/previewHtml/generationStage, leaving inputFiles, referenceUrl, selectedElement, lastPromptInput, and generation/error flags inherited from the previously open project. Mirror openProject's full reset so a freshly created project starts with a clean workspace and prompt context. Adds vitest coverage for the reset. Refs codex review on PR #51. Signed-off-by: hqhq1025 <[email protected]> * fix(ui): replace hardcoded sizing in hub/create with new tokens PR-A introduced raw arbitrary-value Tailwind classes (360px sidebar, 560px modal, 60ch prose, 240px card minimum, 1px hover lift) that violate the token-only constraint in CLAUDE.md. Add five tokens to packages/ui/src/tokens.css and route every call site through them so themes and density tweaks stay centralized. --------- Signed-off-by: hqhq1025 <[email protected]> * feat(core): Skills system foundation — loader + 4 starter skills (#33) * feat(core): skills loader + 4 starter skills + provider injector (PR-A) - packages/shared/src/skills.ts: SkillFrontmatterV1 zod schema + LoadedSkill interface (canonical location; avoids core→providers circular dep) - packages/core/src/skills/: inline YAML frontmatter parser, loadSkillsFromDir, loadAllSkills with 3-tier priority (project > user > builtin), loader tests - packages/core/src/skills/builtin/: 4 starter skills (frontend-design-anti-slop, pitch-deck, data-viz-recharts, mobile-mock) — self-written, Apache-2.0, no Anthropic SKILL.md text copied - packages/providers/src/skill-injector.ts: injectSkillsIntoMessages, pure, supports system and prefix scope, provider-wildcard matching, injector tests No new runtime deps added. Inline YAML parser (~100 lines) handles folded scalars, nested mappings, inline + block sequences. Signed-off-by: hqhq1025 <[email protected]> * fix(core): skills loader collects errors and throws instead of silent drop Replace the three silent continue/console.warn paths in loadSkillsFromDir with error collection; after processing all files, throw CodesignError 'SKILL_LOAD_FAILED' if any errors were accumulated. Update tests to expect the throw, and add a new loadAllSkills test for a broken skill (missing description field). Signed-off-by: hqhq1025 <[email protected]> * fix(core): propagate non-ENOENT errors in skills loader Signed-off-by: hqhq1025 <[email protected]> * fix(providers): sort skills into canonical order before injection Mixed-scope skill injection was order-dependent: whichever skill appeared first in the caller's array decided whether the block went into the system prompt or the first user message. That made prompt-shaping behaviour a function of loader iteration order rather than the active skill set. Sort skills by source precedence (project > user > builtin) and then alphabetical name before building the block. The chosen scope, the concatenated body, and the resulting prompt blob are now byte-identical across runs regardless of input order, which also stabilises prompt caching and snapshot tests. Adds a vitest case that feeds three random permutations of a five-skill fixture and asserts the resulting system content is byte-identical to the canonical sort, plus a mixed system/prefix scope test that confirms the higher-precedence skill picks the channel. Signed-off-by: hqhq1025 <[email protected]> * fix(providers): inject mixed-scope skills into separate channels Previously a mixed system/prefix skill set was concatenated into a single block and injected via the highest-precedence skill's channel, silently routing the rest into the wrong channel and violating each skill's trigger.scope contract. Now we partition active skills by scope and inject system-scope skills into the system message and prefix-scope skills into the first user message, preserving canonical order within each channel. Signed-off-by: hqhq1025 <[email protected]> * fix(core): preserve newlines in YAML literal block scalar (PR #33 codex Minor) --------- Signed-off-by: hqhq1025 <[email protected]> * feat(desktop): viewport switcher (desktop / tablet / mobile with phone bezel) (#32) * feat(desktop): mobile/tablet/desktop viewport preview with phone bezel - Add PreviewViewport type and previewViewport/setPreviewViewport to store - Add PhoneFrame component (CSS-only iPhone bezel, fully tokenised) - Add viewport switcher (Desktop/Tablet/Mobile) to PreviewToolbar - PreviewPane renders iframe inside PhoneFrame at 375x812 for mobile, centred 768px container for tablet, and full-width for desktop - Add preview.viewport i18n keys (en + zh-CN) - Add 4 unit tests for setPreviewViewport in store.test.ts Signed-off-by: hqhq1025 <[email protected]> * fix(ui): tokenize phone frame dimensions + give tablet wrapper a height Replace hard-coded px values in PhoneFrame with CSS custom properties from packages/ui tokens.css. Add --size-preview-mobile-*, --size-preview-tablet-width, --radius-phone, and --border-width-strong tokens. Fix tablet branch in PreviewPane to propagate h-full so the iframe is no longer zero-height. Signed-off-by: hqhq1025 <[email protected]> * fix(ui): tokenize preview viewport sizes (mobile/tablet/desktop) Signed-off-by: hqhq1025 <[email protected]> * fix(ui): tokenize preview hint badge color/size Address Codex blocker re-flagged on PR #32: - Replace hardcoded values in PreviewPane hint badge (left-5/top-5, bg-[rgba(255,255,255,0.88)], px-3/py-1, text-[11px]) with --space-5/--space-3/--space-1, --color-surface-elevated, --text-xs. - Replace hardcoded sizes/motion in PreviewToolbar viewport controls and Download button (w-[14px], w-[30px], h-[30px], duration-150, literal cubic-bezier) with --size-icon-sm, --size-control-xs, --duration-fast, --ease-out. - Add tokens: --color-surface-elevated (light + dark), --size-control-xs (30px), --size-icon-sm (14px). Signed-off-by: hqhq1025 <[email protected]> * fix(ui): tokenize PhoneFrame border + remaining hardcoded values Replace hard-coded `outline: '1px solid ...'` with the existing `--shadow-inset-soft` token composed into boxShadow. Removes the last non-tokenized value in PhoneFrame so the component is fully driven by packages/ui tokens. Signed-off-by: hqhq1025 <[email protected]> * fix(ui): make PhoneFrame notch overlay click-through (pointer-events: none) Signed-off-by: hqhq1025 <[email protected]> * fix(desktop): add aria-label to viewport switcher buttons title alone is not a reliable accessible name for screen readers; add aria-label using the same i18n string so assistive tech announces Mobile/Tablet/Desktop instead of a generic button. Signed-off-by: hqhq1025 <[email protected]> * fix(desktop): tokenize iframe background in mobile preview (PR #32 codex Major) * fix(desktop): tokenize desktop preview iframe bg + dedup icon size tokens (PR #32 codex follow-up) --------- Signed-off-by: hqhq1025 <[email protected]> * fix(core): exclude craft-directives from tweak mode (PR #43 codex Major) --------- Signed-off-by: hqhq1025 <[email protected]> Co-authored-by: Claude <[email protected]>
#33 added the skills loader and four builtin skills, but generate() never called the loader or injected skill bodies into the system prompt, so data-viz-recharts (and the other three) were dead code at runtime. Changes: - packages/core: generate() now calls loadBuiltinSkills() + matchSkillsToPrompt() per request and passes matched skill bodies through composeSystemPrompt({ skills }) (the parameter already existed). Loader failure is best-effort: log and continue. - packages/core/src/skills/loader.ts: export loadBuiltinSkills() that resolves the bundled builtin/ dir via import.meta.url. - packages/providers/src/skill-injector.ts: add matchSkillsToPrompt(skills, prompt) using a curated keyword set extracted from the four builtin descriptions. - vitest: added three skills-injection tests (match → injected, no match → not injected, loader failure → graceful fallback).
* feat(core): wire skills loader into generation pipeline (Skills PR-B) #33 added the skills loader and four builtin skills, but generate() never called the loader or injected skill bodies into the system prompt, so data-viz-recharts (and the other three) were dead code at runtime. Changes: - packages/core: generate() now calls loadBuiltinSkills() + matchSkillsToPrompt() per request and passes matched skill bodies through composeSystemPrompt({ skills }) (the parameter already existed). Loader failure is best-effort: log and continue. - packages/core/src/skills/loader.ts: export loadBuiltinSkills() that resolves the bundled builtin/ dir via import.meta.url. - packages/providers/src/skill-injector.ts: add matchSkillsToPrompt(skills, prompt) using a curated keyword set extracted from the four builtin descriptions. - vitest: added three skills-injection tests (match → injected, no match → not injected, loader failure → graceful fallback). * fix(core): surface skill-load failures via warnings + console.warn Codex flagged the silent fallback in collectMatchedSkillBlobs as a violation of "no silent fallbacks". Generation must still succeed when the builtin skills directory is unreadable (UX), but the failure now surfaces three ways: - console.warn with errorClass + message (visible in dev/main process) - logger.error step=load_skills.fail (structured event for desktop logs) - GenerateOutput.warnings[] so the renderer/UI can display it Test updated to assert the warning is returned and the error log fires.
Summary
First PR of 3 implementing the Skills system (research 12).
This PR delivers the backend foundation only:
Not in scope (follow-up PRs):
Architecture note
SkillFrontmatterV1andLoadedSkillare defined inpackages/shared(notpackages/core) to avoid a circular dependency (coredepends onproviders;providersnow depends onsharedfor the skill types).packages/core/src/skills/types.tsre-exports from shared for convenience.Built-in starters
frontend-design-anti-slop— design quality guard (oklch / no Inter / asymmetry)pitch-deck— slide ratio, whitespace, one-claim-per-slidedata-viz-recharts— chart palette + recharts patternsmobile-mock— viewport ratio + 44pt touch targetsNo new dependencies
Inline YAML subset parser (~100 lines) handles: folded/literal block scalars, nested block mappings, inline + block sequences. No
gray-matteradded.Test plan
Compatibility