feat(skills): add a create-skill button to capture a session

[RolianTan]

Problem / Motivation

When a chat works out something reusable — a deploy runbook, a debugging path, a fixed command sequence — there is no low-friction way to keep it at that moment. The on-demand crystallize skill authors a skill from the session but runs inline, which derails the conversation you are in; automatic post-session generation only fires on the idle path. So a user who knows the current chat is worth keeping must either interrupt their session or wait and hope the idle pass catches it.

Why it matters

Hard-won context is captured while it is fresh instead of being lost or left to a passive pass. The capture is non-blocking, so the user hands off and keeps working — removing the one real drawback of inline crystallize. It reuses proven machinery (crystallize authoring + the pending-review/approve pipeline), so the added surface is small and low-risk.

What changed (motivation → approach → change)

• Goal: a one-click "save this session as a skill" that never interrupts the chat.
• Approach: rather than a new authoring engine, hand the session to a fire-and-forget background subagent that runs the existing crystallize skill, and reuse the existing staging → pending-review → approve pipeline. Keep user-initiated captures separate from auto-generated ones via a new manual/ namespace — auto skills have an archive-if-unused lifecycle, and a deliberate capture should not be aged out or confused with a suggestion.
• Built:
  • Owner-only POST /api/skills/-/from-session: validates the caller (rejects app / internal-secret callers and incognito sessions), shapes the session transcript, injects the user's required purpose into the authoring instructions, and spawns the subagent (silent=True, approval_mode="spawn" — the spawn is authorized but the subagent's tool calls stay approval-gated); returns 202 immediately.
  • New MANUAL_SKILL_NAMESPACE; approve_pending_skill reads a candidate's .meta.json namespace and promotes to auto/ or manual/ (default stays auto/) — the rest of the pipeline is unchanged.
  • Frontend: a Create skill item in the chat header's session actions menu (directly under the Switch to Autopilot toggle) opens a modal with a required "Skill purpose" input (submit disabled until non-empty; Enter submits; opts out of password-manager autofill decorations), wired through ChatPage to api.createSkillFromSession; strings added to all catalogs.
  • Specs docs/system-specs/modules/memory-skills-hooks.md and subagent.md updated in the same change.

Tests

• Backend (test/test_dashboard_create_skill.py, test/test_skill_pending.py, test/test_subagent.py): the endpoint's auth/validation guards (app / internal / owner-only, missing / non-string / over-length / empty purpose, unknown session, incognito refusal, empty session), the successful background spawn (202, approval_mode="spawn") and at-capacity (429); that a candidate marked namespace: "manual" is promoted to manual/<slug> on approval while auto/ stays the default; and that a "spawn" subagent does not inherit a trusted parent's auto policy.
• Frontend (CreateSkillDialog.test.tsx, AssistantMessage.test.tsx): the dialog submits the trimmed purpose, disables/no-ops on empty or whitespace, submits on Enter once non-empty, and carries the autofill opt-out attributes; catalog parity holds. 147 tests (incl. catalogParity).
• Gates green against current main: flake8 / isort / mypy, tsc -b, npm run i18n:check, check_brand_name, docs-lint.

Manual verification

Ran the dashboard against an isolated dev home: opened the chat header's session menu → Create skill, entered a purpose, confirmed the request returns immediately (chat stays interactive), the candidate appears in Skills → Pending review, and approving it promotes the skill under manual/<slug>. Confirmed the Create button stays disabled until a non-empty purpose is entered.

Screenshots / video

User-visible surface is the Create skill item in the chat header's session menu and its purpose modal.

1. Session menu → Create skill

2. Purpose description (required input)

3. Background authoring — the chat stays fully interactive

4. Candidate in Skills → Pending review

Related Issues

Closes #3181

Checklist

• Single commit with a Conventional Commits title
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (memory-skills-hooks + subagent specs, same commit)
• No secrets, credentials, or internal references in the diff

PR revision fixes

Successive fork review rounds (Opus 4.8 / GPT 5.6 / SAST / First Principles) and a rebase onto the latest main were all addressed. Complete list of fixes applied, in their final state:

• Audit owner-only denials (backend-security-controls) — every denial branch of POST /api/skills/-/from-session emits a SEL outcome="denied" event.
• Untrusted-transcript approval — the authoring subagent is spawned with approval_mode="spawn": the spawn is authorized, but its tool calls stay approval-gated and it does not inherit any auto policy (trusted parent, YOLO, global config, or the auto_approve_subagent_tools hook). Suppressing that auto grant emits a single subagent.deny_auto_inherit SEL denial event naming the overridden source (backend-security-controls).
• Linked private-session leak — the incognito guard checks the target session's canonical restriction via _session_key_is_restricted(effective_session_key(slot)), so a channel-born session whose linked Slack thread is set to !incognito (restriction on the SessionMap, not the slot's memory_mode) is refused.
• Required purpose — the purpose is validated server-side (400 purpose_required); the unreachable "infer from transcript" branch was removed.
• SAST raw-html-format — false positive on the subagent prompt string (never rendered as HTML); suppressed with a justified # nosemgrep.
• max-two-buttons-per-row — "Create skill" left the assistant-message footer entirely and is now an item in the chat header's session actions menu, positioned directly under Switch to Autopilot (via a generic modeSlots slot on the shared SessionActionsMenu; CreateSkillPopover deleted).
• Hook auto-approval bypass (subagent.py) — spawn mode also suppresses a hook TOOL_AUTO_APPROVE, not just the inherited policy, so an untrusted-transcript authoring subagent cannot auto-execute a tool matching auto_approve_tools; its calls route to interactive approval or deny-by-default.
• Dialog data loss (CreateSkillDialog.tsx) — the purpose modal awaits submission and clears/closes only on success; a capacity/network failure keeps the dialog open with the typed purpose intact.
• Governance identity on the authoring subagent (create_skill.py) — the subagent now runs under the target session's agent/app identities (not the default agent), so an app- or named-agent session's scoped governance profile still bounds it.
• TOCTOU on the privacy check (create_skill.py) — the incognito/restriction check is re-run immediately before spawn (the transcript read is awaited), so a session flipped to !incognito mid-read is refused rather than authored.
• TOCTOU on the privacy re-check (create_skill.py) — the restriction re-check now runs as the last step before the synchronous spawn, after the agent-cache warm await, so no awaited step can invalidate it.
• Suppressed hook auto-approve is audited (subagent.py) — when a spawn-mode subagent suppresses a hook TOOL_AUTO_APPROVE, it now emits a subagent.hook_auto_approve_suppressed SEL denial event (backend-security-controls).
• No event-loop FS walk for named-agent capture (create_skill.py) — the target agent is validated off the event loop (asyncio.to_thread(_validate_agent, …)), then spawn() skips its own on-loop list_agents() scan via _agent_prevalidated; an unresolvable agent is refused (400 agent_unavailable) rather than downgraded to the default.
• Dialog close during submit (CreateSkillDialog.tsx) — Escape / outside-click is ignored while a submission is in flight, so the typed purpose can't be discarded before the request resolves.
• Single commit / rebased — all changes squashed into one Conventional-Commits commit and rebased onto the latest main (i18n catalogs union-merged, en-XA regenerated).

[github-actions]

UX Review (Fable 5, fork) — 🟡 CONCERNS

UX-level review of 49d0e5b9cbfad139ad1e429337c9d729c7908c9d via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Evidence gathered. Composing the review.

UX-Verdict: CONCERNS

Kicking off a background skill capture gives zero success feedback or destination — the dialog just closes and the result surfaces nowhere the user was told about.

Watch

• Silent success, no closure. handleCreateSkill only alerts on failure; on 202 the dialog closes with no confirmation, the spawn is silent=True (completion notification suppressed), and skills.pending_changed only refreshes an already-open Skills tab. A first-time user clicks "Create", sees nothing, and must already know the result lands in Skills → Pending review — every use, task-completion in doubt. Fix: success toast/inline notice naming the destination ("Skill creation started — review it under Skills → Pending review"), or say it in create_skill_hint.
• Half-translated dialog in 5 of 12 locales. de/es/fr/it/pt ship English "create_skill": "Create skill", "Skill purpose", "Create", and "Could not start skill creation: {{error}}" next to a translated hint (bn/hi/ja/ko/ru/zh are fully translated). Every non-English user of those locales sees a mixed-language dialog, persistently. Fix: translate the four keys.
• Enter-submit breaks IME composition. CreateSkillDialog's onKeyDown fires submit() on any Enter with no ime.claimEnter(e) guard (the sibling convention, e.g. FolderConfigModal.tsx:265), so a ja/ko/zh user committing an IME candidate submits a half-composed purpose — high frequency for CJK locales this PR explicitly translates for. Fix: if (e.key === 'Enter' && ime.claimEnter(e)) submit().
• Private-session failure after the user types. The "Create skill" menu item renders regardless of memory_mode, though ChatPage already holds it (currentSlot?.memory_mode === 'incognito', line 6424); an incognito user types a purpose, submits, and only then gets a 400 alert. Fix: hide or disable the item (with the refusal as tooltip) when the slot/linked session is restricted.

[UX-REVIEWED] 49d0e5b

[github-actions]

GPT 5.6 Review (fork) — 🔴 changes requested (blocking)

Reviewed 49d0e5b9cbfad139ad1e429337c9d729c7908c9d via the fork AI-review pipeline; updated in place on each push.

BLOCKING -- src/kiro_crew/skills.py:3301 -- LLM metadata controls lifecycle namespace
namespace = self._read_pending_meta(slug).get("namespace")
Injected transcript -> hidden namespace: manual with visible name: auto/... -> approval bypasses auto archival and capacity controls.
Anchor: backend-security-controls
Fix: Keep promotions in auto/; do not select the namespace from candidate metadata.

FINDING -- src/kiro_crew/dashboard/handlers/create_skill.py:183 -- "_validate_agent, target_agent" omits the slot project, so capturing a project-agent session returns agent_unavailable instead of spawning -> Fix: warm and validate with slot.project, pass it as cwd, and avoid _agent_prevalidated for the normal queued path.

FINDING -- website/src/pages/ChatPage.tsx:4947 -- "await api.createSkillFromSession" performs a server write outside the required React Query mutation path -> Fix: define a useMutation and submit through mutateAsync.

[BLOCK-MERGE] 49d0e5b
[GPT-REVIEWED] 49d0e5b

[github-actions]

Design Review (Fable 5, fork) — 🟡 CONCERNS

Design-level review of 49d0e5b9cbfad139ad1e429337c9d729c7908c9d via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: CONCERNS

Sound reuse of crystallize + pending-review, with a careful trust story — but the fire-and-forget path has no failure feedback, and the manual namespace rides on prompt compliance.

Watch

• Failure is invisible by design. The spawn is silent=True and the handler returns 202 immediately; success surfaces via skills.pending_changed, but there is no counterpart for failure. Because approval_mode="spawn" suppresses every auto-approve source, the authoring subagent's staging writes depend on interactive approval — a missed prompt (2h timeout) or a model error means the capture the user explicitly requested never appears, and nothing tells them why. Consider un-silencing the error outcome only.
• The manual/ classification is enforced only by the prompt. The endpoint knows the capture is manual, but that fact travels as an instruction ("in .meta.json set "namespace": "manual"") the LLM may drop; approve_pending_skill then defaults to auto/, filing the deliberate capture into the archive-if-unused lifecycle — the exact confusion the namespace exists to prevent — with no detection at review time. Surfacing the namespace in the pending-review UI, or flagging a source: "make-it-skill" candidate lacking it, would make the miss visible before approval.

[DESIGN-REVIEWED] 49d0e5b

[github-actions]

First Principles Review (Fable 5, fork) — 🟡 CONCERNS

Premise-level review of 49d0e5b9cbfad139ad1e429337c9d729c7908c9d via the fork AI-review pipeline — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Reviewed the contract, the patch, and the base tree (crystallize skill, the skills lifecycle sweep, SessionActionsMenu, subagent.py's approval chain). Emitting the review as the contract's final output.

First-Principles-Verdict: CONCERNS

The feature and its guards are derived and earn their place; a one-consumer modeSlots array and a re-stated crystallize contract ride along.

What this change ships

Intent: let a user capture the current chat as a reusable skill without interrupting it — an ADDITION (closes #3181).

1. "Create skill" item in the chat header session menu — justified
2. Modal with a required purpose input — justified
3. POST /api/skills/-/from-session, owner-only — justified
4. Authoring runs in a background subagent; chat stays interactive — justified (this is the harm being removed)
5. New manual/ namespace; approved captures skip the auto age-out — declared; overlaps an existing mechanism, see Watch
6. New subagent approval_mode="spawn": spawn pre-authorized, tools stay gated — justified (untrusted-transcript boundary, named in the contract)
7. SEL denial events on every refusal and suppressed auto-grant — justified (backend-security-controls)
8. Incognito/linked-session refusal, re-checked before spawn — justified (privacy boundary)
9. modeSlots?: ReactNode[] slot on the shared menu — one consumer, generalized
10. New strings in all locale catalogs + failure alert — rides with the feature, declared

Watch

• manual/ vs the existing pinned exemption. The age-out half of the namespace's justification is already covered: run_skill_lifecycle exempts pinned: true skills from both the inactivity pass and the max-N backstop (src/kiro_crew/skills.py:2302-2308). What pinned does not cover is "filed as an auto suggestion" — that residual UX harm is the namespace's whole support. Grepped MANUAL_SKILL_NAMESPACE: 1 real consumer (approve_pending_skill).
• _SKILL_AUTHOR_INSTRUCTIONS is a second spelling of the crystallize contract. It says "follow the crystallize skill" and then restates its rules (pending dir, .meta.json keys, 4 KB script bound, dedup) — 2 places that must now co-evolve; a .meta.json schema change in builtin_skills/crystallize/SKILL.md silently strands the prompt copy. Shrink the prompt to only the manual-capture deltas (namespace, name, source) and defer the rest to the skill.

Subtractions

• Replace modeSlots?: React.ReactNode[] (SessionActionsMenu.tsx:44) with a single optional node — 1 consumer ever passes it, always a one-element array (ChatPage.tsx:313); grep modeSlots: 2 non-test sites, both this PR.
• Drop the purpose? optionality in createSkillFromSession (website/src/api/client.ts:1858) — the only caller (ChatPage.tsx handleCreateSkill) always passes a non-empty value and the backend 400s on empty; take the required parameter.

[FIRST-PRINCIPLES-REVIEWED] 49d0e5b

[github-actions]

Opus 4.8 Review (fork) — ✅ no blocking findings

Reviewed 49d0e5b9cbfad139ad1e429337c9d729c7908c9d via the fork AI-review pipeline; updated in place on each push.

Review details

The single candidate rests on whether an agent-resolution refusal (agent_unavailable) is a "permission decision" that backend-security-controls requires auditing. Falsifying it: within this very handler, only the authorization gates (app_forbidden, human_only, forbidden) and the privacy gates (incognito_session) call _audit_denied. Every validation/availability failure — unknown_session, subagents_unavailable, history_unavailable, invalid_json, session_key_required, purpose_required, purpose_too_long, empty_session — returns without a SEL event. The agent_unavailable refusal is a validation error of that second class (the named target doesn't exist), not an authority-based denial of the caller. The guard itself is not removed — the spawn is still refused, so no privilege escalation, crash, or data loss follows; only an audit line is absent for a branch whose classification as a "permission decision" is genuinely ambiguous. The candidate itself scores this "low." Confidence that this is a real rule violation is well below 80. Dropped.

Step 2: I traced the deny_auto_inherit suppression across all four auto-grant sources (parent/yolo/global/hook), the spawn-gate ("auto","spawn") widening, the mutually-exclusive elif audit block, the TOCTOU re-check (no await between check and synchronous spawn), the canonical _session_key_is_restricted on the effective session key, and the MANUAL_SKILL_NAMESPACE promotion. Each is sound; the Kiro path gains no new conditional. Nothing new grounds to 80+.

No findings.

[OPUS-REVIEWED] 49d0e5b

[NicholasRBowers]

🤖 Kiro Crew [operator: NicholasRBowers#a942f9ca]: Security scan flagged this PR for manual review.

Findings:

• Rule: destructive-operation pattern (recursive-force-delete of filesystem root) — test/test_subagent.py:237
  • Snippet: yield LLMEvent(kind=EVENT_PERMISSION_REQUEST, title="rm -rf ∕", request_id=7) (destructive path defanged here; see the diff for the literal line)
  • Context for the reviewer: the flagged line is inside a test fixture — a mocked EVENT_PERMISSION_REQUEST event whose title carries the dangerous string, apparently to test that such a permission request is handled/rejected. The string is data in a mock, not executed code. This looks like a scanner false positive, but per pipeline policy any scan hit requires human adjudication before automation touches the PR.

A maintainer will review before any automated action is taken. If cleared, remove needs-human-security-review and re-add needs-pr-triage to re-enter the triage cycle.

[RolianTan]

Superseded by #5973 — same feature, rebased cleanly onto current main as a single commit with all gates green (black / flake8 / 146 pytest / i18n:check / tsc -b). Continuing there.