Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 39 additions & 9 deletions src/renderer/src/components/ChatTile.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React, { useState, useEffect, useRef, useCallback, useMemo, Suspense } from 'react'
import type { AppSettings, Persona } from '../../../shared/types'
import { loadPersonas, getAgentIcon, DEFAULT_PERSONAS } from '../config/agentModes'
import { resolvePersonaModelSeed } from '../hooks/personaModelBinding'
import { resolvePersonaModelSeed, resolveSkillModelLock } from '../hooks/personaModelBinding'
import { MONO_DEFAULT } from '../FontContext'

const LazyTerminalTile = React.lazy(() => import('./TerminalTile').then(m => ({ default: m.TerminalTile })))
Expand Down Expand Up @@ -235,6 +235,24 @@ export function ChatTile({ tileId, workspaceId, workspaceDir: _workspaceDir, wid
() => agentModes.find(a => a.id === agentId) ?? null,
[agentModes, agentId],
)
// Precedence LAYER 1 (P1b-2): a linked skill's `requiredModel` HARD-locks the
// composer. Computed from the active persona + discovered workspace skills; when
// non-null the model/provider pills are disabled and the live state is pinned.
const modelLock = useMemo(
() => resolveSkillModelLock(resolvedAgentMode, workspaceSkills),
[resolvedAgentMode, workspaceSkills],
)
// Keep the live composer state pinned while a lock is active. This closes the
// gap onSelectAgent can't: an async skill load or a restored `agentId` (where
// the click handler never fires) would otherwise leave a stale model behind a
// disabled pill. Early-return when unlocked so layer 3 (user pick) is untouched.
// This effect only forces the HARD lock — the soft seed (layer 2) still happens
// exclusively in the onSelectAgent click handler, never in an effect.
useEffect(() => {
if (!modelLock) return
if (modelLock.provider) setProvider(modelLock.provider)
if (modelLock.model) setModel(modelLock.model)
}, [modelLock?.provider, modelLock?.model])

const {
chatSurfaceMenu,
Expand Down Expand Up @@ -943,6 +961,8 @@ export function ChatTile({ tileId, workspaceId, workspaceDir: _workspaceDir, wid
showModelMenu={showModelMenu}
currentProviderEntry={currentProviderEntry}
currentModelLabel={currentModel.label}
modelLocked={Boolean(modelLock)}
lockReason={modelLock?.reason}
model={model}
modelFilter={modelFilter}
onModelFilterChange={setModelFilter}
Expand Down Expand Up @@ -1014,14 +1034,24 @@ export function ChatTile({ tileId, workspaceId, workspaceDir: _workspaceDir, wid
agentModes={agentModes}
onSelectAgent={nextAgentId => {
setAgentId(nextAgentId)
// Precedence layer 2: a selected persona's SOFT defaultBinding seeds the
// composer's provider/model. Seed once here (NOT in an effect — an effect
// keyed on agentId would re-clobber the user's pick on restore/re-render).
// The user can freely change it afterward; the live composer state flows
// to req.model/provider, so the user pick (layer 3) always wins.
const modelSeed = resolvePersonaModelSeed(agentModes.find(a => a.id === nextAgentId) ?? null)
if (modelSeed?.provider) setProvider(modelSeed.provider)
if (modelSeed?.model) setModel(modelSeed.model)
const nextPersona = agentModes.find(a => a.id === nextAgentId) ?? null
// Precedence LAYER 1 (P1b-2): if a linked skill imposes a HARD model lock,
// it PINS provider/model and SHORT-CIRCUITS the soft seed below. The picker
// is disabled in the composer; the live-state effect keeps the pin honoured.
const skillLock = resolveSkillModelLock(nextPersona, workspaceSkills)
if (skillLock) {
if (skillLock.provider) setProvider(skillLock.provider)
if (skillLock.model) setModel(skillLock.model)
} else {
// Precedence layer 2: a selected persona's SOFT defaultBinding seeds the
// composer's provider/model. Seed once here (NOT in an effect — an effect
// keyed on agentId would re-clobber the user's pick on restore/re-render).
// The user can freely change it afterward; the live composer state flows
// to req.model/provider, so the user pick (layer 3) always wins.
const modelSeed = resolvePersonaModelSeed(nextPersona)
if (modelSeed?.provider) setProvider(modelSeed.provider)
if (modelSeed?.model) setModel(modelSeed.model)
}
setShowAgentMenu(false)
}}
planTodos={planTodos}
Expand Down
51 changes: 49 additions & 2 deletions src/renderer/src/components/CustomisationTile.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { useAppFonts } from '../FontContext'
import type { PromptTemplate, PromptField, SkillDefinition, Persona } from '../../../shared/types'
import { ChatMarkdown } from './shared/streamdown-utils'
import { DEFAULT_PERSONAS as DEFAULT_MODES, AGENT_COLORS, AGENT_ICONS } from '../config/agentModes'
import { useChatTileWorkspaceSkills } from '../hooks/useChatTileWorkspaceSkills'

type Tab = 'prompts' | 'skills' | 'tools' | 'agents'

Expand Down Expand Up @@ -794,6 +795,12 @@ function SkillEditor({ item, onSave, onCancel }: { item: SkillDefinition; onSave
<Field label="Name"><Input value={draft.name} onChange={v => up({ name: v })} placeholder="Skill name" /></Field>
<Field label="Description"><Input value={draft.description} onChange={v => up({ description: v })} placeholder="What does this skill do?" /></Field>
<Field label="Command"><Input value={draft.command ?? ''} onChange={v => up({ command: v })} placeholder="e.g. my-skill" /></Field>
<Field label="Required Model">
<Input value={draft.requiredModel ?? ''} onChange={v => up({ requiredModel: v.trim() || undefined })} placeholder="e.g. claude-opus-4-8 — pins the composer model when a persona links this skill" />
</Field>
<Field label="Required Provider">
<Input value={draft.requiredProvider ?? ''} onChange={v => up({ requiredProvider: v.trim() || undefined })} placeholder="optional — e.g. claude, codex (pins alongside the model)" />
</Field>
Comment on lines +801 to +803

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Prevent provider-only lock settings that silently do nothing.

Line [802] currently allows saving requiredProvider when requiredModel is empty. Since lock resolution is model-gated, this persists an inert config and can mislead users.

Suggested fix
       <Field label="Required Model">
-        <Input value={draft.requiredModel ?? ''} onChange={v => up({ requiredModel: v.trim() || undefined })} placeholder="e.g. claude-opus-4-8 — pins the composer model when a persona links this skill" />
+        <Input
+          value={draft.requiredModel ?? ''}
+          onChange={v => {
+            const nextModel = v.trim() || undefined
+            up({
+              requiredModel: nextModel,
+              requiredProvider: nextModel ? draft.requiredProvider : undefined,
+            })
+          }}
+          placeholder="e.g. claude-opus-4-8 — pins the composer model when a persona links this skill"
+        />
       </Field>
       <Field label="Required Provider">
-        <Input value={draft.requiredProvider ?? ''} onChange={v => up({ requiredProvider: v.trim() || undefined })} placeholder="optional — e.g. claude, codex (pins alongside the model)" />
+        <Input
+          value={draft.requiredProvider ?? ''}
+          onChange={v => up({ requiredProvider: draft.requiredModel ? (v.trim() || undefined) : undefined })}
+          placeholder="optional — e.g. claude, codex (pins alongside the model)"
+        />
       </Field>
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/renderer/src/components/CustomisationTile.tsx` around lines 801 - 803,
The Input field for requiredProvider (around line 802) allows saving a provider
value even when requiredModel is empty, which creates an inert configuration
since lock resolution is model-gated. Modify the onChange handler for the
requiredProvider Input field to only update the requiredProvider value if
requiredModel has a non-empty value, preventing users from persisting a
provider-only setting that won't actually function.

<Field label="Content (Markdown)"><Input value={draft.content} onChange={v => up({ content: v })} placeholder="# Skill content..." multiline mono rows={12} /></Field>
<div style={{ display: 'flex', gap: 8, justifyContent: 'flex-end', paddingTop: 8 }}>
<button onClick={onCancel} style={{ padding: '6px 16px', borderRadius: 6, border: `1px solid ${theme.border.default}`, background: theme.surface.panelElevated, color: theme.text.muted, fontSize: fonts.secondarySize, cursor: 'pointer' }}>Cancel</button>
Expand Down Expand Up @@ -1203,7 +1210,7 @@ export function AgentsSection({ workspacePath, hideHeaderText = false }: { works
scanDirs()
}, [workspacePath, locationText])

if (editing) return <AgentEditor item={editing} modes={items} onSave={handleSave} onCancel={() => setEditing(null)} />
if (editing) return <AgentEditor item={editing} modes={items} workspacePath={workspacePath} onSave={handleSave} onCancel={() => setEditing(null)} />
if (locationsOpen) return (
<LocationsPanel
title="Persona"
Expand Down Expand Up @@ -1242,11 +1249,21 @@ export function AgentsSection({ workspacePath, hideHeaderText = false }: { works
)
}

function AgentEditor({ item, modes, onSave, onCancel }: { item: Persona; modes: Persona[]; onSave: (m: Persona) => void; onCancel: () => void }): JSX.Element {
function AgentEditor({ item, modes, workspacePath, onSave, onCancel }: { item: Persona; modes: Persona[]; workspacePath: string; onSave: (m: Persona) => void; onCancel: () => void }): JSX.Element {
const theme = useTheme()
const fonts = useAppFonts()
const [draft, setDraft] = useState(item)
const up = (patch: Partial<Persona>) => setDraft(prev => ({ ...prev, ...patch }))
// Discovered + authored skills for the link picker. Linked skills are stored by
// NAME (stable across machines; discovered ids are path-based). A skill carrying a
// `requiredModel` HARD-locks the composer model for any persona that links it.
const { workspaceSkills } = useChatTileWorkspaceSkills(workspacePath)
const linkedSkills = draft.skills ?? []
const toggleSkill = (name: string) => up({
skills: linkedSkills.includes(name)
? linkedSkills.filter(s => s !== name)
: [...linkedSkills, name],
})
// tools semantics: null/undefined (unset) = unrestricted → checkbox off;
// [] = explicit deny-all and [names] = restricted → checkbox on. Loose `!=`
// so an absent (undefined) tools field reads as unrestricted, not restricted.
Expand Down Expand Up @@ -1302,6 +1319,36 @@ function AgentEditor({ item, modes, onSave, onCancel }: { item: Persona; modes:
)}
</Field>

<Field label="Linked Skills">
{workspaceSkills.length === 0 ? (
<div style={{ fontSize: fonts.secondarySize, color: theme.text.muted }}>
No workspace skills found. Skills that declare a Required Model will pin this persona&rsquo;s model when linked.
</div>
) : (
<div style={{ display: 'flex', flexDirection: 'column', gap: 4, maxHeight: 160, overflowY: 'auto' }}>
{workspaceSkills.map(skill => {
const checked = linkedSkills.includes(skill.name)
return (
<label key={skill.id} style={{ display: 'flex', alignItems: 'center', gap: 8, fontSize: fonts.secondarySize, color: theme.text.secondary, cursor: 'pointer' }}>
<input type="checkbox" checked={checked} onChange={() => toggleSkill(skill.name)} />
<span style={{ flex: 1, overflow: 'hidden', textOverflow: 'ellipsis', whiteSpace: 'nowrap' }}>{skill.name}</span>
{skill.requiredModel && (
<span style={{ fontSize: fonts.secondarySize, color: theme.accent.base, border: `1px solid ${theme.border.accent}`, borderRadius: 4, padding: '0 5px', flexShrink: 0 }}>
locks {skill.requiredModel}
</span>
)}
</label>
)
})}
</div>
)}
{linkedSkills.some(name => !workspaceSkills.find(s => s.name === name)) && (
<div style={{ marginTop: 6, fontSize: fonts.secondarySize, color: theme.text.muted }}>
Linked (not in current workspace): {linkedSkills.filter(name => !workspaceSkills.find(s => s.name === name)).join(', ')}
</div>
)}
</Field>

<Field label="Default Next Persona">
<select value={draft.defaultNextMode ?? ''} onChange={e => up({ defaultNextMode: e.target.value || undefined })}
style={{ padding: '6px 10px', fontSize: fonts.secondarySize, borderRadius: 6, background: theme.surface.input, color: theme.text.secondary, border: `1px solid ${theme.border.default}`, outline: 'none' }}>
Expand Down
11 changes: 10 additions & 1 deletion src/renderer/src/components/chat/ChatTileComposer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,10 @@ export interface ChatTileComposerProps {
showModelMenu: boolean
currentProviderEntry: ProviderEntry | undefined
currentModelLabel: string
/** P1b-2 (layer 1): a linked skill pins the model — disable the model/provider pickers. */
modelLocked?: boolean
/** Tooltip explaining the lock, shown on the disabled pills. */
lockReason?: string
model: string
modelFilter: string
onModelFilterChange: (value: string) => void
Expand Down Expand Up @@ -318,6 +322,8 @@ export function ChatTileComposer({
showModelMenu,
currentProviderEntry,
currentModelLabel,
modelLocked,
lockReason,
model,
modelFilter,
onModelFilterChange,
Expand Down Expand Up @@ -523,7 +529,8 @@ export function ChatTileComposer({
label={currentProviderEntry?.label ?? 'Provider'}
active={showProviderMenu}
onClick={() => onToggleMenu('provider')}
title="Choose the CLI agent (hidden once the conversation starts)"
disabled={modelLocked}
title={modelLocked ? (lockReason ?? 'Model locked') : 'Choose the CLI agent (hidden once the conversation starts)'}
/>
{showProviderMenu && (
<MenuPortal anchorRef={providerMenuRef}>
Expand All @@ -550,6 +557,8 @@ export function ChatTileComposer({
label={currentModelLabel}
active={showModelMenu}
onClick={() => onToggleMenu('model')}
disabled={modelLocked}
title={modelLocked ? (lockReason ?? 'Model locked') : undefined}
/>
{showModelMenu && (
<MenuPortal anchorRef={modelMenuRef}>
Expand Down
50 changes: 47 additions & 3 deletions src/renderer/src/hooks/personaModelBinding.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { Persona, PersonaBinding } from '../../../shared/types'
import type { Persona, PersonaBinding, SkillDefinition } from '../../../shared/types'

// ─── Persona soft model/provider seeding (P1b-1) ──────────────────────────────
// Dissociates model from persona IDENTITY: a Persona carries at most an OPTIONAL
Expand Down Expand Up @@ -28,8 +28,14 @@ export interface PersonaModelSeed {
provider?: string
/** Model id to seed into the composer, if the binding specifies one. */
model?: string
// SEAM (P1b-2): a hard skill-lock will add `locked?: boolean` (+ reason) here so
// callers can disable the composer control without reshaping this result.
/**
* P1b-2 (layer 1): true when this is a HARD skill-lock (from resolveSkillModelLock)
* rather than a soft seed. Callers disable the composer's model/provider picker and
* force the live state to provider/model. Unset/false for soft seeds (layer 2).
*/
locked?: boolean
/** Human-readable explanation of the lock, surfaced as the disabled pill's tooltip. */
reason?: string
}

function cleanField(value: string | undefined): string | undefined {
Expand All @@ -38,6 +44,44 @@ function cleanField(value: string | undefined): string | undefined {
return trimmed.length > 0 ? trimmed : undefined
}

/**
* Resolve the HARD skill-defined model lock for a persona (precedence LAYER 1 —
* runs ABOVE resolvePersonaModelSeed). A persona may LINK skills via `persona.skills`
* (matched against `workspaceSkills` by `id` OR `name`). The FIRST linked skill that
* declares a `requiredModel` wins: its model (and `requiredProvider`, if set) PIN the
* composer and the picker is disabled. Returns null when no linked skill imposes a
* lock, so the caller falls through to the soft default (layer 2) / user pick (layer 3).
*
* model is NOT a security boundary: this drives composer disablement only and never
* touches resolveAuthoritativeAgentMode (the trusted-disk tools/permission path).
*/
export function resolveSkillModelLock(
persona: Persona | null | undefined,
workspaceSkills: SkillDefinition[] | null | undefined,
): PersonaModelSeed | null {
const linked = persona?.skills
if (!Array.isArray(linked) || linked.length === 0) return null
if (!Array.isArray(workspaceSkills) || workspaceSkills.length === 0) return null
for (const ref of linked) {
const key = cleanField(typeof ref === 'string' ? ref : undefined)
if (!key) continue
const skill = workspaceSkills.find(s => s?.id === key || s?.name === key)
if (!skill) continue
// requiredModel is the lock TRIGGER ("first linked skill with requiredModel wins").
const model = cleanField(skill.requiredModel)
if (!model) continue
const provider = cleanField(skill.requiredProvider)
const lock: PersonaModelSeed = {
model,
locked: true,
reason: `Model locked by skill "${skill.name}"`,
}
if (provider) lock.provider = provider
return lock
}
return null
}

/**
* Resolve the SOFT model seed for a persona (precedence layer 2). Returns null
* when the persona carries no usable soft default, so the caller leaves the
Expand Down
16 changes: 12 additions & 4 deletions src/renderer/src/hooks/useChatTileWorkspaceSkills.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,15 +90,23 @@ export function useChatTileWorkspaceSkills(workspaceDir: string) {
})

const registerDiscoveredSkill = (filePath: string, fallbackName: string, content: string, dir: string): void => {
const nameMatch = content.match(/^---[\s\S]*?name:\s*(.+?)$/m)
const descriptionMatch = content.match(/^---[\s\S]*?description:\s*(.+?)$/m)
const name = nameMatch?.[1]?.trim() ?? fallbackName
const frontmatter = content.match(/^---\s*\r?\n([\s\S]*?)\r?\n---/)?.[1] ?? ''
const name = frontmatter.match(/^name:\s*(.+?)\s*$/m)?.[1]?.trim() ?? fallbackName
const description = frontmatter.match(/^description:\s*(.+?)\s*$/m)?.[1]?.trim() ?? `From ${dir}`
// P1b-2: an optional `model:`/`provider:` frontmatter key declares a HARD
// model lock for any persona that links this skill (see resolveSkillModelLock).
// Parse them ONLY from the leading `---`-fenced frontmatter block, line-anchored,
// so prose like "pick the best model: fast" in the body never trips a spurious lock.
const requiredModel = frontmatter.match(/^model:\s*(.+?)\s*$/m)?.[1]?.trim() || undefined
const requiredProvider = frontmatter.match(/^provider:\s*(.+?)\s*$/m)?.[1]?.trim() || undefined
registerSkill({
id: `discovered-${filePath}`,
name,
description: descriptionMatch?.[1]?.trim() ?? `From ${dir}`,
description,
content,
command: name,
...(requiredModel ? { requiredModel } : {}),
...(requiredProvider ? { requiredProvider } : {}),
})
}

Expand Down
24 changes: 24 additions & 0 deletions src/shared/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,19 @@ export interface SkillDefinition {
description: string
content: string
command?: string
/**
* OPTIONAL hard model/provider requirement (P1b-2, precedence layer 1). When a
* persona LINKS a skill that declares `requiredModel`, selecting that persona
* PINS the composer's model (+ provider, if given) and DISABLES the picker —
* an outer resolver (resolveSkillModelLock) runs ABOVE the soft default.
*
* Like a persona's soft binding, this is NOT a security boundary: it only
* drives renderer model-resolution + composer disablement; it never flows
* through resolveAuthoritativeAgentMode (the trusted-disk tools/permission path).
*/
requiredModel?: string
/** OPTIONAL provider to pin alongside `requiredModel`. */
requiredProvider?: string
}

/**
Expand Down Expand Up @@ -166,6 +179,17 @@ export interface Persona {
* See overlayPersonas() in src/shared/agentModes.ts.
*/
extends?: string
/**
* OPTIONAL linked skill references (P1b-2, precedence layer 1). Each entry
* matches a workspace skill by `id` OR `name`. If the FIRST linked skill that
* declares a `requiredModel` is found, selecting this persona HARD-locks the
* composer's model/provider and disables the picker (see resolveSkillModelLock).
*
* Built-ins carry NO skills (keeps DEFAULT_PERSONAS byte-identical across the
* shared<->daemon drift guard). Like `defaultBinding`, this is display/composer
* data only — never a permission boundary.
*/
skills?: string[]
/** Which tool this persona was discovered from: 'claude' | 'cursor' | 'opencode' | 'gemini' | etc. */
source?: string
}
Expand Down
Loading
Loading