feat: skill expansion, detection, and SKILL.md sidebar preview

[Zhang-Henry]

Summary

• Skill expansion for non-Claude providers: Codex, Gemini, and OpenRouter now receive full SKILL.md instructions when users invoke /skill-name commands, instead of opaque slash commands
• Auto-bypass permissions: Autoresearch workflows automatically lock to bypass mode across all providers
• Improved skill detection in sidebar: Skills are now detected from .agents/, .gemini/, .claude/ directories, from skillExpander heading format, and from Bash/FileChanges events (critical for Codex which doesn't emit Read tool events)
• SKILL.md preview: Clicking a skill in the "Referenced Context" sidebar opens a markdown preview of its SKILL.md content, with API-based resolution for skills outside the project root

Test plan

• Use /skill-name in Codex session → verify skill is expanded and detected in sidebar
• Use /skill-name in Gemini session → verify skill is detected from read_file tool
• Click a skill in the sidebar → verify SKILL.md preview modal renders markdown
• Verify skills under .gemini/skills/ and .agents/skills/ are detected
• Verify GET /api/skills/resolve?name=xxx returns 404 for nonexistent and 400 for traversal attempts
• Run npx tsc --noEmit — passes clean

[bbsngg]

@Zhang-Henry Nice work on the skill expansion feature — the core idea of expanding /skill-name into full SKILL.md content for non-Claude providers is solid. A few issues to address before merging:

🔴 Security

1. Auto-bypass permissions needs user consent

useChatComposerState.ts:844, ChatComposer.tsx:636

Autoresearch workflows silently force bypassPermissions mode with no user confirmation and the mode switch button is disabled. This is a significant security concern — granting full tool execution without explicit user opt-in.

Suggestion: Require a one-time confirmation dialog (or make it a user preference) before engaging bypass mode. At minimum, show a visible indicator that permissions are elevated.

2. /api/skills/resolve — workingDir parameter not validated

server/routes/skills.js:233

The name parameter is checked for path traversal, but workingDir is not validated against known project roots. An attacker could pass an arbitrary workingDir to probe the filesystem for SKILL.md files in unexpected locations.

const effectiveWorkingDir = (typeof workingDir === 'string' && workingDir.trim()) || process.cwd();

Suggestion: Validate workingDir against known/registered project paths, or require it to be a project the current user owns.

🟡 Code Quality

3. Duplicated autoresearch check — extract to shared utility

The condition attachedPrompt?.scenarioId?.startsWith('autoresearch-') is duplicated in:

• useChatComposerState.ts:844
• ChatComposer.tsx:636

This will diverge over time. Extract to a small helper like isAutoResearchScenario(scenarioId).

4. Misplaced export statement

server/utils/skillExpander.js:118

export { findSkillMdPath } is wedged between the JSDoc and expandSkillCommand's function definition. Move it to the top or bottom of the file for clarity.

5. Over-cautious null-safe chaining

server/openai-codex.js, server/openrouter.js

command?.trim?.() || command

?.trim?.() is redundant — .trim is always a function on strings. Simplify to command?.trim() || command, or just pass command since expandSkillCommand already handles null.

6. Sequential sub-skill resolution — use Promise.all

server/utils/skillExpander.js:89-104

buildSubSkillIndex sequentially stats each sub-skill. Use Promise.all to parallelize for better latency when there are many references.

🔵 Minor

• package-lock.json: Large number of "peer": true additions/removals look unrelated to this feature. Was this from an npm install on a different Node version? Consider reverting these noise changes.
• splitContextPrefix while-loop could be simplified to a single greedy regex match.
• ChatContextSidebar.tsx:598 — when resolveSkill fails, falling back to setPreviewTask silently hides the error. A toast notification would improve UX.

Overall the feature implementation is clean and well-structured. Requesting changes primarily for the security items (1 & 2).

[Zhang-Henry]

Thanks for the thorough review @bbsngg! All items addressed in 8e6c10d:

🔴 Security

1. Auto-bypass permissions — Mode switch button is no longer disabled when autoresearch is attached, so the user can override the bypass. Added a visible amber "Auto" badge next to the controls to indicate elevated permissions. The auto-bypass behavior itself is kept (autoresearch workflows need it to function), but it's now transparent and overridable.

2. workingDir validation — Added validation against FORBIDDEN_PATHS (same list used by project path validation) and rejects non-absolute paths. When workingDir matches process.cwd() the check is skipped since that's the default fallback.

🟡 Code Quality

3. Duplicated autoresearch check — Extracted isAutoResearchScenario(scenarioId) into src/components/chat/utils/autoResearch.ts, used in both useChatComposerState.ts and ChatComposer.tsx.

4. Misplaced export — Moved export { findSkillMdPath } to the end of skillExpander.js, after expandSkillCommand.

5. Over-cautious null-safe chaining — Simplified to command?.trim() in both openai-codex.js and openrouter.js.

6. Sequential sub-skill resolution — Converted buildSubSkillIndex to use Promise.all for parallel stat calls.

🔵 Minor

• package-lock.json — Reverted the peer: true noise (was from a different npm version).
• splitContextPrefix — Replaced the while-loop with a single greedy regex: /^(\s*\[Context:[^\]]*\]\s*)+/i.
• ChatContextSidebar.tsx — Added console.warn for both non-ok response and catch block. No toast infrastructure exists in the project currently — can add that as a follow-up if desired.