Repository navigation
feat: skills management page, chat layout refresh and UI fixes - #158
Conversation
A /skills page lists the agent skills on disk, opens their files in the editor, and creates, edits, copies and deletes local skills. Part B of task 20261022-0307-skills-management; installing and updating through the skills CLI follows separately. Skills are read in the layout of the skills CLI (vercel-labs/skills): the shared store .agents/skills (read by Codex; entries in .skill-lock.json are managed) and .claude/skills (read by Claude Code: links into the store plus real skill dirs), under HOME and under a project's directory. No engine CLI is called. Managed skills are read-only because skills update overwrites local edits; copying makes a local skill under a new name. New skills go to .agents/skills with a relative link in .claude/skills so both engines see them; deleting moves a local skill to data/trash/skills and drops its link. Paths are realpath-checked inside the skill; these roots do not relax the file browser's workspace check.
Install from a GitHub repository with a preview of its skills, update and uninstall through the skills CLI (one operation at a time, live output), and upload a SKILL.md or a .zip / .skill archive. Installed skills show whether their repository has a newer version, comparing the lock's skillFolderHash with GitHub's tree API. The page adapts to small screens: the list or the open skill at full width, with a collapsible file tree as in the workspace. Markdown files preview with their frontmatter as YAML, and the delete dialog no longer promises a restore the UI does not offer.
Each assistant message flips between rendered Markdown and its source in place instead of opening a dialog. The turn navigator follows the latest turn on first load and while scrolling.
Share the header height, the collapsible group bars and the issue rows, so the review list gets the same right-click and overflow menus.
The desktop rail and the mobile menu highlight the global page being viewed, including its nested routes. Header selects match the small button height and group their items, page headers drop the duplicate logo, and the mobile menu's close button sits in its header row.
The 1 s findByTestId window covered the cold lazy import of the library on top of React's 300 ms Suspense reveal throttle, so the test failed under load.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2d22199741
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| previewHome = await mkdtemp(join(tmpdir(), 'bkd-skills-preview-')) | ||
| await mkdir(join(previewHome, '.codex'), { recursive: true }) | ||
| args = ['add', input.source!, '-s', '*', '-a', 'codex', '-g', '-y', '--json'] | ||
| env = cliEnv({ HOME: previewHome }) |
There was a problem hiding this comment.
When the server has XDG_STATE_HOME set, overriding only HOME does not isolate the preview: the pinned CLI resolves its global lock from $XDG_STATE_HOME/skills/.skill-lock.json, so merely previewing a repository can add or overwrite entries in the real lock while the installed files are later deleted with the temporary home. Override XDG_STATE_HOME to a temporary directory as well. See the CLI's getSkillLockPath() implementation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4193673: the preview now sets XDG_STATE_HOME inside the throwaway HOME, so the CLI's lock stays there. Covered by a test in api-skills-operations.test.ts.
| agents: join(home(), '.agents/skills'), | ||
| claude: join(home(), '.claude/skills'), | ||
| lockFile: join(home(), '.agents/.skill-lock.json'), | ||
| } |
There was a problem hiding this comment.
On hosts that set XDG_STATE_HOME, [email protected] reads and writes $XDG_STATE_HOME/skills/.skill-lock.json, but this code always reads ~/.agents/.skill-lock.json. Consequently CLI-installed directories are presented as unmanaged local skills, allowing the UI to edit or delete them and omitting their source/update metadata. Match the pinned CLI's getSkillLockPath() precedence.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4193673: globalRoots() follows the CLI's getSkillLockPath(), using $XDG_STATE_HOME/skills/.skill-lock.json when set. It also follows CLAUDE_CONFIG_DIR for the Claude skills dir, which the CLI uses for claude-code's globalSkillsDir.
| const headers: Record<string, string> = { 'Accept': 'application/vnd.github+json', 'User-Agent': 'bkd' } | ||
| const token = process.env.GITHUB_TOKEN || process.env.GH_TOKEN | ||
| if (token) headers.Authorization = `Bearer ${token}` | ||
| const res = await fetch(`https://api.github.com/repos/${source}/git/trees/HEAD?recursive=1`, { |
There was a problem hiding this comment.
For a skill installed from a tag or non-default branch, the lock entry records a ref, but every check here fetches HEAD. This can report an unchanged pinned skill as updated or removed, and sources with multiple refs also collide in the source-only cache. Preserve the lock's ref and fetch/cache the corresponding tree; the pinned CLI's lock schema includes the installation ref.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 03ad599: the lock's ref is passed to the tree fetch (HEAD when absent), and the cache is keyed by source and ref.
| <SkillDetail | ||
| key={skillKey(selected)} | ||
| skill={selected} | ||
| projectId={projectId || undefined} |
There was a problem hiding this comment.
If two projects contain a project skill with the same location/name and the destination project's query is already cached, switching the project picker reuses this SkillDetail instance because its key is unchanged. Its drafts state from the first project is then displayed for the second project, and Save writes that draft into the second project's skill. Include projectId in the key for project-scoped skills or clear selection/editor state on project changes.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in a2ceb17: project skills are now keyed by project as well, so switching projects remounts the editor. Covered by a test in skills-page.test.tsx.
| import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query' | ||
| import { useEffect } from 'react' | ||
| import { kanbanApi } from '@/lib/kanban-api' | ||
| import type { SkillLocation } from '@/types/kanban' |
There was a problem hiding this comment.
This introduces the skills endpoint wrappers in a separate hook module, while the repository convention requires new kanban-api.ts endpoints to be wrapped in use-kanban.ts. Keeping another independent query-key and invalidation layer makes the established server-state entry point incomplete; move or re-export these hooks through the mandated module.
AGENTS.md reference: AGENTS.md:L230-L231
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Not changing this one. The repo already keeps feature hooks in their own modules (use-messages.ts, use-notes.ts), and use-skills.ts follows that pattern with its own key factory. The AGENTS.md line predates those modules.
[email protected] keeps its global lock under $XDG_STATE_HOME when set and installs Claude Code skills under $CLAUDE_CONFIG_DIR. Reading only the HOME paths showed CLI-installed skills as editable local ones on such hosts, and a repository preview wrote into the real lock. The preview now gets its own XDG_STATE_HOME inside the throwaway HOME.
A skill installed from a branch or tag records it as the lock's ref; comparing it with the default branch reported pinned skills as updated or removed. Fetch and cache the tree per source and ref.
Two projects can hold a project skill with the same name. Switching the project kept the editor mounted, so its unsaved draft showed for the other project's skill and Save wrote it there.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a2ceb17915
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| } catch { | ||
| return {} |
There was a problem hiding this comment.
When an existing CLI lock is malformed or temporarily unreadable, this returns an empty object, so scan() marks every CLI-installed directory as unmanaged. The save and delete endpoints then pass assertEditable() and can overwrite or move skills that are still owned by the CLI; distinguish a genuinely missing lock from parse or permission failures, or otherwise keep those skills read-only.
Useful? React with 👍 / 👎.
| function pickName(skillMd: string, fallback: string): string | null { | ||
| const fromMeta = parseFrontmatter(skillMd).name?.trim() | ||
| if (fromMeta && NAME_RE.test(fromMeta)) return fromMeta | ||
| const base = fallback.replace(/\.(zip|skill|md)$/i, '') | ||
| return NAME_RE.test(base) ? base : null |
There was a problem hiding this comment.
When an uploaded .md has no valid name, this falls back to the filename without modifying or rejecting the file, and it never requires a description; the API therefore reports success for a directory whose SKILL.md the pinned CLI will skip because both fields are required by its parseSkillMd() validation. Validate both fields before installation, or rewrite the accepted fallback name into valid frontmatter.
Useful? React with 👍 / 👎.
| } | ||
| if (skill.location.endsWith('agents')) { | ||
| // <base>/.agents/skills/<name> -> <base>/.claude/skills | ||
| const claudeDir = join(dirname(dirname(dirname(skill.path))), '.claude/skills') |
There was a problem hiding this comment.
When CLAUDE_CONFIG_DIR is configured, creation places the Claude symlink under that configured directory, but deletion reconstructs ~/.claude/skills from the canonical agent path instead. Deleting a global local skill consequently leaves a dangling link in the real Claude directory, and recreating the same name later fails with a conflict because placeNewSkill() still sees that link; derive this directory from globalRoots().claude for global skills.
Useful? React with 👍 / 👎.
…NFIG_DIR A lock file that exists but cannot be read or parsed was treated as empty, so CLI-installed skills became editable and deletable. Keep every store skill read-only until the lock can be read again. Deleting a global skill now removes its link from the Claude dir it was created in, which is CLAUDE_CONFIG_DIR when that is set; before, the link was left dangling and blocked recreating the name.
The skills CLI ignores a SKILL.md without both fields, so an upload that fell back to the file name produced a skill no engine would load. Skip such files and report why.
Success cleared the selection but left the dialog state on 'uninstall', so selecting any skill afterwards reopened the dialog for it. Close the dialog on success, and reset skill dialogs whenever a skill is selected.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d1927d276d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| for (const entry of await readEntries(roots.agents)) { | ||
| if (!entry.isDirectory() && !entry.isSymbolicLink()) continue | ||
| const dir = join(roots.agents, entry.name) | ||
| const meta = await readDescription(dir) |
There was a problem hiding this comment.
When a project checkout contains .agents/skills/name as a symlink to another directory containing SKILL.md, this accepts it as a project skill. resolveInside() subsequently realpaths that external directory and treats it as the allowed root, so the project-scoped read and save endpoints can expose or overwrite files outside the project, including another project's skill directory. Reject project-store symlinks or verify their resolved target remains under the project's skill root.
AGENTS.md reference: AGENTS.md:L234-L234
Useful? React with 👍 / 👎.
| async function validate(input: StartOperationInput): Promise<void> { | ||
| if (input.scope === 'project' && !input.projectDir) throw new SkillError('Project has no directory', 400) |
There was a problem hiding this comment.
When a project's directory resolves to HOME, project-scoped add/update operations are still accepted even though listSkills() deliberately suppresses that project's store because it aliases the global store. An add therefore writes into the global skill directory with a project lock, after which the UI presents the installed skill as unmanaged and permits editing or deletion; remove validation also cannot find it as a managed project skill. Apply the existing assertProjectScope() guard to CLI operations before starting them.
Useful? React with 👍 / 👎.
| <Sheet | ||
| open={open} | ||
| onOpenChange={(next) => { | ||
| if (!next) { | ||
| reset() | ||
| setSource('') | ||
| } |
There was a problem hiding this comment.
If the user dismisses the sheet with its default close button or Escape while a preview or install is running, this clears both operation IDs without cancelling the server operation. The operation continues to hold the global one-at-a-time slot for up to five minutes, but reopening the sheet cannot poll or cancel it and subsequent starts only receive a 409. Either prevent dismissal while running, cancel on close, or retain the ID so the operation remains reachable.
Useful? React with 👍 / 👎.
| .then((t) => { | ||
| const map = new Map<string, string>([['', t.sha]]) | ||
| for (const e of t.tree) { | ||
| if (e.type === 'tree') map.set(e.path, e.sha) | ||
| } | ||
| return map |
There was a problem hiding this comment.
For sufficiently large repositories, GitHub can return a successful recursive tree response marked truncated; this mapping ignores that marker and treats every omitted folder as absent. A managed skill whose folder falls outside the partial response is consequently reported as removed even though it still exists. Carry the truncated field through the response and return unknown, or fetch the relevant subtree before declaring removal.
Useful? React with 👍 / 👎.
A checkout can link .agents/skills/<name> to any directory holding a SKILL.md. Listing it let the project-scoped read and save routes reach files outside the project, such as another project's or a global skill. Project skills must now resolve inside their project.
Such a project's store is the global one, so a project-scoped add wrote into the global store under a project lock and the skill then showed as editable. Apply the same guard as create and upload.
…known GitHub can return a partial recursive tree for a large repository; a folder absent from it may still exist, so it no longer counts as removed.
Closing the install sheet, or the update or uninstall dialog, dropped the operation id while the server kept running it in its only slot, so it could be neither followed nor cancelled and new operations got 409. The close button is hidden and dismissal ignored until the operation ends or is cancelled.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2c34f2b21d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| for (const entry of await readEntries(roots.claude)) { | ||
| if (!entry.isDirectory()) continue |
There was a problem hiding this comment.
When .claude/skills itself is a symlink, readdir() reports the target's children as ordinary directories, so this loop classifies them as unmanaged Claude-only skills. If it points at .agents/skills, managed skills gain an editable/deletable alias; for a project, a link outside the checkout exposes external skill directories through project-scoped read, write, and delete routes. Resolve and validate the Claude store root, deduplicate aliases of the agents store, and apply the project confinement check here as well.
AGENTS.md reference: AGENTS.md:L234-L234
Useful? React with 👍 / 👎.
| const location: SkillLocation = skill.scope === 'global' ? 'global-agents' : 'project-agents' | ||
| const { roots } = rootsFor(location, projectDir) | ||
| await placeNewSkill(roots, newName, async (dir) => { | ||
| await cp(await realpath(skill.path), dir, { recursive: true, dereference: true }) |
There was a problem hiding this comment.
When a skill contains an inner symlink to a file or directory outside its root, dereference: true follows that link and materializes the external content in the new editable skill, even though the normal file APIs reject the same traversal. A project skill can therefore copy another project's files or host files into an API-readable directory; preserve nested links or reject any link whose resolved target leaves the source skill before copying.
AGENTS.md reference: AGENTS.md:L234-L234
Useful? React with 👍 / 👎.
| await fill(dir) | ||
| await mkdir(roots.claude, { recursive: true }) | ||
| await symlink(relative(roots.claude, dir), link) |
There was a problem hiding this comment.
If fill() fails partway through, or if creating the Claude symlink fails because of permissions or a race, the agents directory remains on disk even though the API reports an error. The partial skill may then appear in the list, and every retry fails with a name conflict; wrap placement in cleanup that removes the newly created directory and link on failure.
Useful? React with 👍 / 👎.
…ed creates - A .claude/skills linked wholesale to .agents/skills is one store: its entries are no longer listed again as editable Claude-only skills, and new skills skip the extra link that would collide. - Project .claude/skills entries must resolve inside the project, like .agents/skills entries. - Copy keeps inner symlinks as links instead of copying what they point to from outside the skill. - A create, copy or upload that fails midway removes its half-made directory, so the name is not blocked on retry.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 90e3f85cb5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| export async function assertProjectScope(scope: 'global' | 'project', projectDir?: string | null): Promise<void> { | ||
| if (scope === 'project' && projectDir && await isHomeDir(projectDir)) { | ||
| throw new SkillError('This project\'s directory is your home directory, so its skills are the global ones: use the global scope', 400) |
There was a problem hiding this comment.
When a checkout makes .agents/skills or .claude/skills a symlink outside the project, this guard still accepts the scope; create, copy, upload, and CLI-add operations then follow that store and write files or links into another project or host directory. The scan-time confinement only hides those entries afterward, so resolve both project store roots and reject roots outside projectDir before allowing any project-scoped mutation.
AGENTS.md reference: AGENTS.md:L234-L234
Useful? React with 👍 / 👎.
| export const CreateSkillSchema = z.object({ | ||
| scope: z.enum(['global', 'project']), | ||
| name: skillName, | ||
| description: z.string().min(1).max(1024), |
There was a problem hiding this comment.
When an API client submits a description containing only whitespace, this schema accepts it and createSkill() trims it to an empty string before writing description: "". The upload path rejects the same empty metadata, so creation can report success for a skill that does not satisfy the required description contract; trim before applying the minimum-length check.
Useful? React with 👍 / 👎.
| const repo = sourceHref(sourceUrl) | ||
| if (!repo || new URL(repo).host !== 'github.com') return null | ||
| if (!sourcePath) return repo | ||
| return `${repo.replace(/\/+$/, '')}/tree/HEAD/${sourcePath.split('/').map(encodeURIComponent).join('/')}` |
There was a problem hiding this comment.
For a skill installed from a tag or non-default branch, this always constructs the folder URL using HEAD, even though the lock records the installation ref. Clicking “Open on GitHub” can therefore open unrelated content or a 404; carry the ref through SkillInfo and use it in this URL.
Useful? React with 👍 / 👎.
…alled ref - Project-scoped writes are refused when the project's .agents or .claude/skills store links outside the project; before, create, copy, upload and CLI add followed the link. - A whitespace-only description no longer passes create validation. - Skills carry the ref they were installed from (sourceRef), and "Open on GitHub" uses it instead of HEAD.
The status flipped to succeeded before the repository's SKILL.md files were read, so a poll in between saw a finished preview with no skills. The preview test failed intermittently under load for this reason.
The thinking summary was cut at 80 characters, a command outside a tool group at 90 and a note preview at 80, so on a wide screen the text stopped short of the row. They now render one line and let CSS truncate it, like the grouped command summaries.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 40e4965e50
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (!entry?.source || !SOURCE_RE.test(entry.source) || (entry.sourceType && entry.sourceType !== 'github') || !entry.skillPath || !entry.skillFolderHash) { | ||
| return { ...base, status: 'unknown' } |
There was a problem hiding this comment.
For every project-scoped CLI install, the pinned CLI records computedHash rather than skillFolderHash in skills-lock.json (see the official LocalSkillLockEntry). This condition therefore returns unknown for all such skills, so the UI never exposes their Update button even when an update exists. Project locks need their own comparison strategy instead of applying the global-lock tree-hash schema.
Useful? React with 👍 / 👎.
| for (const store of [roots.agents, roots.claude]) { | ||
| let probe = store |
There was a problem hiding this comment.
When a checkout contains skills-lock.json as a symlink to a writable file outside the project, this guard accepts it because it checks only the two skill stores. Project add/update/remove then run the CLI with the checkout as cwd, and the pinned CLI's writeLocalLock() writes that path directly, following the symlink and overwriting the external target. Reject a project lock path that resolves outside the checkout before starting project-scoped CLI operations.
AGENTS.md reference: AGENTS.md:L234-L234
Useful? React with 👍 / 👎.
- Every chat block shares one shape (chat-block.ts): one radius, one padding, one vertical rhythm. Messages lose the thick leading bar and are told apart by background; tool groups and command output keep a 1px outline. - User messages sit on the right as a bubble sized to their text, tinted with the primary colour. - An assistant message has two actions: switch between rendered and source (the code icon is highlighted while the source shows) and copy, which copies what is on screen. Footer actions stay visible and share the timestamp colour. - Every truncated line in the chat carries its full text as a title.
…s outside The skills CLI writes skills-lock.json in place, following a symlink (even a dangling one), so a checkout could make a project operation overwrite a file outside the project.
- Nothing is checked when the page opens. An installed global skill with a GitHub source has a "Check for updates" button; the API checks one skill via ?location=&name= and no longer caches between checks. - Project locks record a content hash GitHub cannot be compared with, so project skills are not checked and offer Update directly. - Every update, single or "Update all", first opens a dialog listing the scope's installed skills to tick; skills known to have an update come pre-selected, and the CLI runs only for the ticked ones.
cd996
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dce64bab06
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (confine && !(await resolvesInside(dir, confine))) continue | ||
| const meta = await readDescription(dir) | ||
| if (!meta) continue | ||
| const locked = lock?.[entry.name] |
There was a problem hiding this comment.
When a CLI-installed skill has an uppercase or otherwise sanitized name, the pinned CLI stores it in a lowercased/sanitized directory while preserving the original name as the lock key; this exact lookup therefore misses the entry. For example, installing MySkill creates myskill/ but leaves the lock keyed by MySkill, so BKD presents the installation as unmanaged and permits editing or deleting it while omitting its source/update metadata. Match entries using the CLI's sanitization rules; its resolveSkillsToRemove() documentation explicitly describes this folder-name/lock-key mismatch.
Useful? React with 👍 / 👎.
| // Remount per opening, so the ticks start from this opening's pre-selection | ||
| key={`${updatePick.scope}:${updatePick.preselected.join(',')}`} |
There was a problem hiding this comment.
When the user changes selections, cancels, and later reopens the same scope with the same preselection, this key is unchanged and updatePick was never cleared, so the still-mounted dialog retains its previous picked state. The user can consequently confirm skills they thought were deselected by cancelling; use an opening nonce, clear updatePick on close, or synchronize the picker state when it opens.
Useful? React with 👍 / 👎.
[email protected] keys the lock by a skill's own name but names its folder after sanitizeName(name), so `My Skill` lives in `my-skill/`. The exact lookup missed such entries and showed CLI-installed skills as editable local ones without source or update status. Lookups now match the sanitized name, and update sends the lock key, which is what the CLI matches update names against. Also skip starting the CLI when the operation was cancelled while it was being prepared: the abort had already fired, so the runner would never see it and the operation hung.
Reopening the dialog for the same scope and pre-selection kept the ticks of a cancelled pick, so the user could confirm skills they had deselected. Each opening now remounts it.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d93c823915
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| await cp(await realpath(skill.path), dir, { recursive: true, verbatimSymlinks: true }) | ||
| const skillMd = join(dir, 'SKILL.md') | ||
| const text = await readFile(skillMd, 'utf8') | ||
| await writeFile(skillMd, text.replace(/^(---\r?\n[\s\S]*?)^name:.*$/m, `$1name: ${newName}`), 'utf8') |
There was a problem hiding this comment.
When a project skill's SKILL.md is a symlink to a writable file outside the checkout, verbatimSymlinks: true preserves that link in the destination and this writeFile then follows it, overwriting the external file's name: field; the copied skill also remains linked outside its root. Reject such a link or materialize it only after verifying its resolved target is inside the source skill.
AGENTS.md reference: AGENTS.md:L234-L234
Useful? React with 👍 / 👎.
| <div> | ||
| {issues.map(issue => ( | ||
| <ReviewIssueRow | ||
| <IssueRow |
There was a problem hiding this comment.
When an issue is deleted through the menu newly inherited from IssueRow, useDeleteIssue invalidates only the project issue list, while the delete route emits no issue-updated SSE event. Consequently useReviewIssues retains the deleted row—even after the active issue navigates back to /review—until an unrelated refetch; invalidate queryKeys.reviewIssues() after deletion.
Useful? React with 👍 / 👎.
Copy keeps inner symlinks and then rewrites SKILL.md's name, so a SKILL.md linked to a file elsewhere was rewritten in place. Such a link is refused, and a SKILL.md linked inside the skill becomes a real file in the copy before the rewrite.
The review list gained the issue row menu, but deleting refreshed only the project's issues, and the delete route emits no issue-updated event, so the deleted row stayed in the review queue.
With no outline or tint, the padding only pushed the reply in from the footer and tool rows; without it they line up.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 65904f0cae
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| function pickName(skillMd: string): string | null { | ||
| const meta = parseFrontmatter(skillMd) | ||
| const name = meta.name?.trim() | ||
| if (!name || !NAME_RE.test(name) || !meta.description?.trim()) return null |
There was a problem hiding this comment.
When uploaded frontmatter uses a name such as foo..bar, this local regex accepts it, but the shared assertSkillName() later rejects names containing ... By then placeNewSkill() has already created both the agents directory and Claude link, so getSkill() returns a 400 after mutating disk; the upload appears to fail and leaves a skill the API cannot open or delete. Apply the same name validator before placement or roll back when the post-placement lookup fails.
Useful? React with 👍 / 👎.
| */ | ||
| export function findLockEntry(lock: Record<string, LockEntry> | null | undefined, folder: string): [string, LockEntry] | undefined { | ||
| if (!lock) return undefined | ||
| if (lock[folder]) return [folder, lock[folder]] |
There was a problem hiding this comment.
When no lock entry exists and a local skill is named constructor, toString, or another inherited property, lock[folder] resolves a truthy Object.prototype member. Scanning consequently marks the unmanaged skill as managed/read-only, preventing API edits and deletion while exposing bogus lock metadata. Check Object.hasOwn(lock, folder) before returning the exact match.
Useful? React with 👍 / 👎.
| onChange={value => setDrafts(prev => ({ ...prev, [path]: value }))} | ||
| onSave={handleSave} | ||
| wrap={wrap} | ||
| preview={preview && isMarkdown(path)} |
There was a problem hiding this comment.
When a skill Markdown file references a bundled image with a relative URL, enabling this preview leaves the <img> relative to the /skills page because this FileEditor has no asset root and there is no raw skill-file route. Although uploaded and copied skills preserve their asset folders, those images cannot load in rendered preview; provide a confined raw-file URL for the selected skill and pass its base into the renderer.
Useful? React with 👍 / 👎.
# Conflicts: # docs/task/index.md
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d2ac394a99
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (await exists(dir) || await exists(link)) throw new SkillError('A skill with this name already exists', 409) | ||
| await mkdir(roots.agents, { recursive: true }) | ||
| try { | ||
| await fill(dir) |
There was a problem hiding this comment.
When two create, copy, or upload requests target the same name concurrently, both can pass this existence check and write the same directory. The request that loses the Claude-link race then enters the cleanup path and recursively removes the directory written by the successful request, potentially leaving a dangling link and causing both responses to fail. Reserve the name atomically or serialize placement per store/name before filling and cleaning it up.
Useful? React with 👍 / 👎.
| <div className="space-y-1.5"> | ||
| <Label htmlFor="skill-source">{t('skills.repository')}</Label> | ||
| <div className="flex gap-2"> | ||
| <Input id="skill-source" value={source} onChange={e => setSource(e.target.value)} placeholder="owner/repo" disabled={running} /> |
There was a problem hiding this comment.
After a preview succeeds, this input becomes editable again while the old skill selection remains visible. If the user changes the repository and clicks Install without previewing again, runInstall() sends the new source with skill names and descriptions selected from the old source, so it can install an unintended same-named skill or fail unexpectedly. Clear the preview on source changes or keep the previewed source immutable until installation finishes.
Useful? React with 👍 / 👎.
| const dir = join(previewHome, '.agents/skills', r.name) | ||
| const text = await readFile(join(dir, 'SKILL.md'), 'utf8').catch(() => '') |
There was a problem hiding this comment.
For a skill whose name the pinned CLI sanitizes for its directory—for example, MySkill being installed under myskill—this constructs a nonexistent path from the unsanitized result name and silently returns an empty description. The preview therefore loses metadata for otherwise installable skills; use the result's path or apply the same folder-name sanitization used elsewhere.
Useful? React with 👍 / 👎.
- Update check: a lock entry with a content sha256 (recorded by the CLI when GitHub's tree API was unavailable at install) cannot be compared with a tree hash; report it as unknown instead of always "update". - Preview reads each skill from the CLI's sanitized folder name, so a skill named `My Skill` gets its description; install accepts the name the repository declares and only refuses flag-like or control names. - The install sheet, upload, create and copy dialogs reset on every way of closing, not only on dismissal, so reopening starts afresh. - The delete dialog shows a failure and disables its action while the request runs. - Drop stale comments and an invalidation of a query that no longer exists.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 721e5071d1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (skill.managed || !dirty) return | ||
| save.mutate( | ||
| { location: skill.location, name: skill.name, path, content, projectId }, | ||
| { onSuccess: () => setDrafts(({ [path]: _, ...rest }) => rest) }, |
There was a problem hiding this comment.
If the user continues typing after clicking Save but before the request completes, the success callback unconditionally removes the file's draft. The editor remains editable during the request, so those newer changes disappear and the editor reverts to the older content returned by the save. Only clear the draft if its current content still matches the submitted content; otherwise retain it as unsaved, as the workspace file editor already does.
Useful? React with 👍 / 👎.
Summary
Adds a Skills page for managing agent skills from BKD. Also reworks the chat message layout and fixes several UI issues found along the way.
Skills (
/skills)Browse and edit
skillsCLI uses:~/.agents/skillstogether with its lock file;~/.claude/skills;.agents/skillsand.claude/skills.[email protected]does:$XDG_STATE_HOMEfor the lock,$CLAUDE_CONFIG_DIRfor the Claude dir.data/trash/skills/.skills updatewould overwrite local edits. "Copy to local skill" makes an editable copy.Install, update, uninstall
npx [email protected]as an argv list, one operation at a time. Each operation has live output, a 5-minute timeout and can be cancelled. A dialog cannot be dismissed while its operation runs.skillFolderHashwith GitHub's tree API, at the ref the skill was installed from. Nothing is cached between checks.SKILL.mdor a.zip/.skillarchive...or absolute paths, at most 1000 files, at most 50 MB unpacked.nameanddescriptionare both required, and an existing skill is never overwritten.Safety
skills-lock.jsonmust resolve inside the project. Symlinks that point outside are ignored or refused.Chat
chat-block.ts): one radius, one padding, one vertical rhythm.Other UI fixes
SelectGroup.Tests
patch-diff-options: the library was cold-imported inside the timed window.Test plan
bun run check(lint, typecheck, 977 API and 375 frontend tests, build)