feat(skills): disable skills from PromptComposer and terminal AI Input - #167
Merged
Merged
Conversation
Add a shared composer skills control that toggles manageable skills using the same .atmos/skills/.disabled move as the Skills page for projects, and workplace-rooted disable for workspaces (with symlink ancestor materialization so sync-dir copies/symlinks stay local). Tip users that changes apply on new Agent sessions only. Co-authored-by: AruNi_Lu <hello@0x3f4.run>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughAdds project/workspace-aware skill scanning and enable/disable operations, safe symlink handling, WebSocket contracts, and Dynamic Skills controls for Welcome and terminal composer interfaces. ChangesComposer Skills Disable
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Composer
participant skillsApi
participant WsMessageService
participant SkillManager
Composer->>skillsApi: scanRoot or setEnabled
skillsApi->>WsMessageService: send WebSocket request
WsMessageService->>SkillManager: scan or toggle skill
SkillManager-->>WsMessageService: return skills or success
WsMessageService-->>skillsApi: return JSON response
skillsApi-->>Composer: update skill state and session chip
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
|
| Filename | Overview |
|---|---|
| apps/web/src/features/terminal/components/TerminalSideChatModal.tsx | Builds project or workspace Dynamic Skills context for terminal side chats. |
| apps/web/src/features/terminal/hooks/use-terminal-side-chats.tsx | Threads the project id into the terminal side-chat layer. |
| apps/web/src/features/canvas/components/CanvasTerminalCard.tsx | Passes the canvas terminal scope id into side-chat skill controls and keeps workplace terminals in workspace mode. |
| apps/web/src/features/skills/components/ComposerSkillsControl.tsx | Adds the shared Dynamic Skills popover and project/workspace skill loading behavior. |
| apps/api/src/api/ws/router/skills.rs | Adds WebSocket handlers for scoped root scans and scoped skill toggles. |
Reviews (5): Last reviewed commit: "fix(skills): address Dynamic Skills revi..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
E2E report (expired)This report has been superseded by a newer CI - E2E run.
|
AruNi-01
marked this pull request as ready for review
July 22, 2026 02:16
…ain, and use that for project-mode skills context instead of workspaceId.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (3)
apps/web/src/features/terminal/components/terminal-mosaic-scoped-pane-window.tsx (1)
156-173: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate skill-context derivation logic across terminal pane components.
This block is duplicated verbatim in
terminal-mosaic-workspace-pane-window.tsx(L185-202), and a divergent (buggy) variant exists inTerminalSideChatModal.tsx(L81-100). See consolidated comment for the cross-file recommendation.🤖 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 `@apps/web/src/features/terminal/components/terminal-mosaic-scoped-pane-window.tsx` around lines 156 - 173, Extract the shared skills-context derivation into a reusable helper or hook, then replace the duplicated logic in terminal-mosaic-scoped-pane-window.tsx and terminal-mosaic-workspace-pane-window.tsx with it. Update TerminalSideChatModal.tsx to use the same implementation so project and workspace contexts consistently handle local paths, active projects, IDs, names, and paths.apps/web/src/features/skills/components/ComposerSkillsControl.tsx (1)
58-261: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftConsider adding basic tests for this new interactive component.
ComposerSkillsControlhas non-trivial async state (dual-fetch merge for workspace mode, optimistic toggle with rollback-on-error) but ships with no accompanying test file in this batch. A few unit tests (loadSkills merge logic, handleToggle success/error paths) would guard this critical toggle path against regressions.🤖 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 `@apps/web/src/features/skills/components/ComposerSkillsControl.tsx` around lines 58 - 261, The new ComposerSkillsControl interactive flow lacks regression coverage. Add focused unit tests for loadSkills, including workspace dual-fetch merging and filtering, and for handleToggle success and error paths, verifying optimistic state updates, refresh/invalidation calls, rollback reload behavior, and user-facing errors without changing production behavior.apps/web/src/features/terminal/components/TerminalSideChatModal.tsx (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThree components hand-roll the same
ComposerSkillsContextderivation; extract a shared hook. The duplicated logic already diverged once (see theTerminalSideChatModal.tsxmajor-issue comment), which is exactly the kind of drift a shared helper would prevent.
apps/web/src/features/terminal/components/TerminalSideChatModal.tsx#L81-100: replace this local derivation with the shared hook/helper, sourcing a real project id instead ofworkspaceIdfor the project-mode branch.apps/web/src/features/terminal/components/terminal-mosaic-scoped-pane-window.tsx#L156-173: replace this block with a call to the shared hook (using its already-resolvedactiveProject).apps/web/src/features/terminal/components/terminal-mosaic-workspace-pane-window.tsx#L185-202: replace this identical block with the same shared hook call.🤖 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 `@apps/web/src/features/terminal/components/TerminalSideChatModal.tsx` at line 1, Extract the duplicated ComposerSkillsContext derivation into a shared hook/helper and use it from TerminalSideChatModal, terminal-mosaic-scoped-pane-window, and terminal-mosaic-workspace-pane-window. In TerminalSideChatModal, pass the actual project id for project mode instead of workspaceId; in the two mosaic components, pass their existing resolved activeProject. Remove each local derivation while preserving the current context behavior.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@apps/api/src/api/ws/router/skills.rs`:
- Around line 144-146: Wrap the synchronous
SkillManager::set_enabled_with_extra_roots call in spawn_blocking within the
async handler, moving or cloning the required project_paths and extra_roots into
the blocking closure. Await the JoinHandle and propagate both join failures and
the service error before continuing the handler flow.
- Around line 133-142: Both handlers must stop trusting client-supplied
skill-root path, id, and name values: in apps/api/src/api/ws/router/skills.rs
lines 133-142, replace the direct SkillScopeRoot mapping before
set_enabled_with_extra_roots with an authenticated project/workspace lookup,
canonicalize its path, and reject mismatches; apply the same server-owned root
resolution and validation at lines 162-167 before scan_root.
In `@apps/web/src/features/skills/components/ComposerSkillsControl.tsx`:
- Around line 42-56: Update scopeLabel to handle the inside_project and system
SkillInfo.scope values with their corresponding translation keys, following the
existing t("scope.*") pattern; remove the raw skill.scope fallback so every
supported scope returns a translated label.
In `@apps/web/src/features/skills/components/InstalledSkillListCard.tsx`:
- Around line 43-48: Update getScopeMeta to accept the translator and replace
the hardcoded workspace label with the localized scope.workspace key, matching
SkillDetail.tsx. Ensure the InstalledSkillListCard path obtains the translator
via useTranslations and passes it into getScopeMeta while preserving the
existing metadata.
In `@apps/web/src/features/terminal/components/TerminalSideChatModal.tsx`:
- Around line 81-100: Update the project-root branch of the skillsContext
useMemo in TerminalSideChatModal to use the resolved active project id, matching
the sibling terminal pane behavior, instead of workspaceId. Preserve workspaceId
for non-project workspace contexts and ensure the real project id reaches
ComposerSkillsControl for project skill filtering.
In `@crates/core-service/src/service/skill/support.rs`:
- Around line 147-210: Normalize the path used by ensure_entry_local_to_root
before calling find_symlink_ancestor so it is in the same canonical form as
root, while preserving the existing validation behavior for missing or invalid
paths. Update find_symlink_ancestor’s candidate-prefix comparisons to use this
normalized path, ensuring ancestor traversal still reaches parent symlinks when
root resolves differently from the raw path.
In `@specs/APP/APP-040_composer-skills-disable/PRD.md`:
- Line 26: Update the M5 requirement to replace “Parent compensated symlinks”
with “parent symlinks in compensated trees,” or define “compensated symlink”
before using it, while preserving the existing behavior describing
materialization without mutating the project tree.
In `@specs/APP/APP-040_composer-skills-disable/TECH.md`:
- Around line 51-53: Update the scope-selection logic in the
TerminalAgentInputShell footer to resolve the nearest canonical workspace
worktree ancestor of localPath, rather than checking only exact root equality.
Use workspace mode when an ancestor is found, and retain
activeProjectId/project-root project mode only when no workspace ancestor
exists.
- Around line 43-44: Update the server-side handling of skills_set_enabled and
skills_scan_root so scope_root.path is resolved from authenticated server state
or rejected unless it exactly matches a known workspace root; do not trust
arbitrary client-provided absolute paths when performing scans or moves.
In `@specs/APP/APP-040_composer-skills-disable/TEST.md`:
- Around line 3-8: Expand the TEST.md test plan beyond the current “manual /
agent-browser” note to include test strategy, coverage and execution maps,
concrete scenarios with setup/actions/assertions, exploratory checks, regression
checklist, acceptance criteria, manual verification steps, and explicit
non-coverage. Document the required Agent Browser skill/setup procedure and
record a not_run outcome when setup is unavailable, then add a post-run Coverage
Status section summarizing execution results.
- Around line 20-28: Reconcile the S1–S3 entries in the execution map with the
command evidence and “Coverage Status,” using a single consistent status source.
Update the acceptance checklist for the tests that actually passed, while
keeping the manual S4–S5 items pending and unchecked until executed.
---
Nitpick comments:
In `@apps/web/src/features/skills/components/ComposerSkillsControl.tsx`:
- Around line 58-261: The new ComposerSkillsControl interactive flow lacks
regression coverage. Add focused unit tests for loadSkills, including workspace
dual-fetch merging and filtering, and for handleToggle success and error paths,
verifying optimistic state updates, refresh/invalidation calls, rollback reload
behavior, and user-facing errors without changing production behavior.
In
`@apps/web/src/features/terminal/components/terminal-mosaic-scoped-pane-window.tsx`:
- Around line 156-173: Extract the shared skills-context derivation into a
reusable helper or hook, then replace the duplicated logic in
terminal-mosaic-scoped-pane-window.tsx and
terminal-mosaic-workspace-pane-window.tsx with it. Update
TerminalSideChatModal.tsx to use the same implementation so project and
workspace contexts consistently handle local paths, active projects, IDs, names,
and paths.
In `@apps/web/src/features/terminal/components/TerminalSideChatModal.tsx`:
- Line 1: Extract the duplicated ComposerSkillsContext derivation into a shared
hook/helper and use it from TerminalSideChatModal,
terminal-mosaic-scoped-pane-window, and terminal-mosaic-workspace-pane-window.
In TerminalSideChatModal, pass the actual project id for project mode instead of
workspaceId; in the two mosaic components, pass their existing resolved
activeProject. Remove each local derivation while preserving the current context
behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6128c241-08b7-482b-86b7-2fc036145dda
📒 Files selected for processing (28)
apps/api/src/api/ws/message.rsapps/api/src/api/ws/message/skills.rsapps/api/src/api/ws/router/mod.rsapps/api/src/api/ws/router/skills.rsapps/web/messages/en.jsonapps/web/messages/zh.jsonapps/web/src/api/ws/skills-api.tsapps/web/src/features/connection/hooks/use-websocket.tsapps/web/src/features/skills/components/ComposerSkillsControl.tsxapps/web/src/features/skills/components/InstalledSkillListCard.tsxapps/web/src/features/skills/components/SkillDetail.tsxapps/web/src/features/terminal/components/TerminalAgentInputOverlay.tsxapps/web/src/features/terminal/components/TerminalSideChatModal.tsxapps/web/src/features/terminal/components/terminal-mosaic-scoped-pane-window.tsxapps/web/src/features/terminal/components/terminal-mosaic-workspace-pane-window.tsxapps/web/src/features/welcome/components/SlashCommandPopover.tsxapps/web/src/features/welcome/components/WelcomeComposerCard.tsxapps/web/src/features/welcome/components/WelcomeComposerControls.tsxapps/web/src/features/welcome/components/WelcomePage.tsxcrates/core-service/src/service/skill.rscrates/core-service/src/service/skill/scanner.rscrates/core-service/src/service/skill/support.rscrates/core-service/src/service/skill/tests.rsspecs/APP/APP-040_composer-skills-disable/BRAINSTORM.mdspecs/APP/APP-040_composer-skills-disable/PRD.mdspecs/APP/APP-040_composer-skills-disable/TECH.mdspecs/APP/APP-040_composer-skills-disable/TEST.mdspecs/README.md
Comment on lines
+133
to
+142
| let extra_roots: Vec<SkillScopeRoot> = req | ||
| .scope_root | ||
| .into_iter() | ||
| .map(|root| SkillScopeRoot { | ||
| scope: root.scope, | ||
| id: root.id, | ||
| name: root.name, | ||
| path: root.path, | ||
| }) | ||
| .collect(); |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the target file and nearby symbols
ast-grep outline apps/api/src/api/ws/router/skills.rs --view expanded || true
# Show the relevant section with line numbers
sed -n '1,260p' apps/api/src/api/ws/router/skills.rs | cat -n
# Find the request/validation types and any helpers used by these routes
rg -n "SkillScopeRoot|scope_root|SkillsScanRootRequest|set_enabled_with_extra_roots|scan" apps/api/src/api/ws/router/skills.rs apps/api/src/api/ws -SRepository: AruNi-01/atmos
Length of output: 18560
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect related service and repository types referenced by the router
rg -n "set_enabled_with_extra_roots|SkillScopeRoot|SkillsScanRootRequest|canonical|path" crates apps/api -S
# Locate definitions for server-owned project/workspace records and any root resolution helpers
rg -n "project.*record|workspace.*record|resolve.*root|root.*resolver|authorized.*root|registered.*root" crates apps/api -SRepository: AruNi-01/atmos
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the service signatures and implementations for the skills operations
rg -n "set_enabled_with_extra_roots|scan_root|SkillScopeRoot|SkillsScanRootRequest" crates -S
# If found, print the surrounding implementation
files=$(rg -l "set_enabled_with_extra_roots|SkillsScanRootRequest" crates -S || true)
for f in $files; do
echo "===== $f ====="
sed -n '1,260p' "$f" | cat -n
doneRepository: AruNi-01/atmos
Length of output: 21325
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the request DTOs and any root-related schema/types
rg -n "struct SkillsScanRootRequest|struct SkillScopeRoot|enum .*Skill.*Request|scope_root" apps crates -SRepository: AruNi-01/atmos
Length of output: 2199
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,140p' apps/api/src/api/ws/message/skills.rs | cat -n
echo '===== web api ====='
sed -n '1,180p' apps/web/src/api/ws/skills-api.ts | cat -nRepository: AruNi-01/atmos
Length of output: 8085
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' crates/core-service/src/service/skill/scanner.rs | cat -n
echo '===== support ====='
sed -n '1,220p' crates/core-service/src/service/skill/support.rs | cat -nRepository: AruNi-01/atmos
Length of output: 17656
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Focus on the root-based scanning logic and any path validation/canonicalization.
rg -n "fn scan_root|derive_original_path|canonical|strip_prefix|root_path|project_id|scope_root" crates/core-service/src/service/skill/scanner.rs crates/core-service/src/service/skill/support.rs -n -S
sed -n '220,420p' crates/core-service/src/service/skill/scanner.rs | cat -n
sed -n '420,620p' crates/core-service/src/service/skill/scanner.rs | cat -nRepository: AruNi-01/atmos
Length of output: 21112
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find direct tests or call sites for scan_root and extra_roots behavior
rg -n "scan_root\(|scan_all_with_extra_roots|set_enabled_with_extra_roots|SkillsScanRootRequest|scope_root" crates/core-service/src/service/skill tests apps/api/src/api/ws -SRepository: AruNi-01/atmos
Length of output: 3023
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "scanRoot\\(" apps/web apps/api -S
echo '===== call sites ====='
rg -n "skills_scan_root|skills_set_enabled|scopeRoot|scope_root" apps/web/src -SRepository: AruNi-01/atmos
Length of output: 1164
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check whether the UI sources roots from server-owned project/workspace data.
sed -n '1,220p' apps/web/src/api/ws/skills-api.ts | cat -n
echo '===== likely consumers ====='
rg -n "skillsApi\\.scanRoot|skillsApi\\.setEnabled|scopeRoot" apps/web/src -SRepository: AruNi-01/atmos
Length of output: 7154
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant tests to see how root handling is expected to behave.
sed -n '260,360p' crates/core-service/src/service/skill/tests.rs | cat -nRepository: AruNi-01/atmos
Length of output: 4242
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the path validation helper that controls move safety, if any.
sed -n '220,340p' crates/core-service/src/service/skill/support.rs | cat -nRepository: AruNi-01/atmos
Length of output: 4707
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '60,210p' apps/web/src/features/skills/components/ComposerSkillsControl.tsx | cat -nRepository: AruNi-01/atmos
Length of output: 6133
Bind skill roots to server-owned project/workspace records. Both handlers accept client-supplied path/id/name, and core_service only checks scope plus existence, so a forged project/workspace root can leak skill metadata and steer enable/disable moves onto an arbitrary directory.
apps/api/src/api/ws/router/skills.rs#L133-L142: resolvescope_rootfrom authenticated project/workspace data, canonicalize the path, and reject mismatches before callingset_enabled_with_extra_roots.apps/api/src/api/ws/router/skills.rs#L162-L167: apply the same server-side root lookup beforescan_root.
📍 Affects 1 file
apps/api/src/api/ws/router/skills.rs#L133-L142(this comment)apps/api/src/api/ws/router/skills.rs#L162-L167
🤖 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 `@apps/api/src/api/ws/router/skills.rs` around lines 133 - 142, Both handlers
must stop trusting client-supplied skill-root path, id, and name values: in
apps/api/src/api/ws/router/skills.rs lines 133-142, replace the direct
SkillScopeRoot mapping before set_enabled_with_extra_roots with an authenticated
project/workspace lookup, canonicalize its path, and reject mismatches; apply
the same server-owned root resolution and validation at lines 162-167 before
scan_root.
Comment on lines
+144
to
+146
| SkillManager::set_enabled_with_extra_roots( | ||
| &project_paths, | ||
| &extra_roots, |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== skills router excerpt ==\n'
sed -n '120,190p' apps/api/src/api/ws/router/skills.rs
printf '\n== search set_enabled_with_extra_roots ==\n'
rg -n "set_enabled_with_extra_roots|spawn_blocking|scan handler|set_enabled" apps/api/src -S
printf '\n== locate SkillManager definition ==\n'
rg -n "struct SkillManager|impl SkillManager|fn set_enabled_with_extra_roots|fn scan" crates apps -SRepository: AruNi-01/atmos
Length of output: 6676
🏁 Script executed:
trueRepository: AruNi-01/atmos
Length of output: 152
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- apps/api/src/api/ws/router/skills.rs ---'
cat -n apps/api/src/api/ws/router/skills.rs | sed -n '130,190p'
echo '--- search for spawn_blocking in apps/api/src/api/ws/router/skills.rs and related files ---'
rg -n "spawn_blocking|set_enabled_with_extra_roots|scan handler" apps/api/src/api/ws/router/skills.rs apps/api/src -S
echo '--- locate SkillManager method definition ---'
rg -n "fn set_enabled_with_extra_roots|impl SkillManager|struct SkillManager" crates apps -SRepository: AruNi-01/atmos
Length of output: 4864
🏁 Script executed:
#!/bin/bash
set -euo pipefail
git ls-files 'apps/api/src/api/ws/router/skills.rs' 'crates/**' | sed -n '1,40p'Repository: AruNi-01/atmos
Length of output: 1556
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- crates/core-service/src/service/skill.rs ---'
cat -n crates/core-service/src/service/skill.rs | sed -n '1,220p'
echo '--- crates/core-service/src/service/skill/scanner.rs ---'
cat -n crates/core-service/src/service/skill/scanner.rs | sed -n '1,380p'Repository: AruNi-01/atmos
Length of output: 24883
Run the skill toggle in spawn_blocking. SkillManager::set_enabled_with_extra_roots is synchronous filesystem work, so calling it directly from this async handler can block Tokio workers on large or slow workspaces. Offload it and propagate the join + service errors.
🤖 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 `@apps/api/src/api/ws/router/skills.rs` around lines 144 - 146, Wrap the
synchronous SkillManager::set_enabled_with_extra_roots call in spawn_blocking
within the async handler, moving or cloning the required project_paths and
extra_roots into the blocking closure. Await the JoinHandle and propagate both
join failures and the service error before continuing the handler flow.
Comment on lines
+42
to
+56
| function scopeLabel( | ||
| skill: SkillInfo, | ||
| t: ReturnType<typeof useTranslations>, | ||
| ) { | ||
| switch (skill.scope) { | ||
| case "global": | ||
| return t("scope.global"); | ||
| case "workspace": | ||
| return t("scope.workspace"); | ||
| case "project": | ||
| return t("scope.project"); | ||
| default: | ||
| return skill.scope; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
scopeLabel falls back to an untranslated raw scope string.
SkillInfo.scope includes "inside_project" and "system", but the switch only handles global/workspace/project; the default branch returns skill.scope verbatim instead of a translated label.
🌐 Proposed fix
case "project":
return t("scope.project");
+ case "inside_project":
+ return t("scope.insideProject");
+ case "system":
+ return t("scope.system");
default:
return skill.scope;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function scopeLabel( | |
| skill: SkillInfo, | |
| t: ReturnType<typeof useTranslations>, | |
| ) { | |
| switch (skill.scope) { | |
| case "global": | |
| return t("scope.global"); | |
| case "workspace": | |
| return t("scope.workspace"); | |
| case "project": | |
| return t("scope.project"); | |
| default: | |
| return skill.scope; | |
| } | |
| } | |
| function scopeLabel( | |
| skill: SkillInfo, | |
| t: ReturnType<typeof useTranslations>, | |
| ) { | |
| switch (skill.scope) { | |
| case "global": | |
| return t("scope.global"); | |
| case "workspace": | |
| return t("scope.workspace"); | |
| case "project": | |
| return t("scope.project"); | |
| case "inside_project": | |
| return t("scope.insideProject"); | |
| case "system": | |
| return t("scope.system"); | |
| default: | |
| return skill.scope; | |
| } | |
| } |
🤖 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 `@apps/web/src/features/skills/components/ComposerSkillsControl.tsx` around
lines 42 - 56, Update scopeLabel to handle the inside_project and system
SkillInfo.scope values with their corresponding translation keys, following the
existing t("scope.*") pattern; remove the raw skill.scope fallback so every
supported scope returns a translated label.
Source: Coding guidelines
Comment on lines
+43
to
+48
| case "workspace": | ||
| return { | ||
| label: "Workspace", | ||
| icon: Folder, | ||
| className: "bg-muted text-foreground", | ||
| }; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Localize the workspace scope label.
"Workspace" is hardcoded here, so this card renders English in the Chinese locale even though SkillDetail.tsx uses t("scope.workspace") for the same scope. Pass the translator into getScopeMeta and use the localized key.
As per coding guidelines, apps/web/**/*.{ts,tsx} must read user-facing copy with useTranslations(...) and avoid hardcoded user-facing labels.
🤖 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 `@apps/web/src/features/skills/components/InstalledSkillListCard.tsx` around
lines 43 - 48, Update getScopeMeta to accept the translator and replace the
hardcoded workspace label with the localized scope.workspace key, matching
SkillDetail.tsx. Ensure the InstalledSkillListCard path obtains the translator
via useTranslations and passes it into getScopeMeta while preserving the
existing metadata.
Source: Coding guidelines
| | M2 | Terminal AI Input exposes the same control. In a project terminal it uses project disable; in a workspace terminal it uses workplace-rooted disable. | | ||
| | M3 | Project disable/enable uses the existing Skills Manager move into `project/.atmos/skills/.disabled/...` and stays consistent with the Skills management page. | | ||
| | M4 | Workspace disable/enable reuses the same move logic with storage under `workplace/.atmos/skills/.disabled/...`. | | ||
| | M5 | Sync-dir symlink and copy placements can both be disabled by moving the workplace-visible skill entry into `.disabled` so Agents no longer discover it. Parent compensated symlinks are materialized as needed so the move does not mutate the project tree. | |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Clarify the symlink terminology.
Line [26] should say “parent symlinks in compensated trees” (or define “compensated symlink”) so the requirement is unambiguous.
🧰 Tools
🪛 LanguageTool
[grammar] ~26-~26: Use a hyphen to join words.
Context: ... so Agents no longer discover it. Parent compensated symlinks are materialized as...
(QB_NEW_EN_HYPHEN)
🤖 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 `@specs/APP/APP-040_composer-skills-disable/PRD.md` at line 26, Update the M5
requirement to replace “Parent compensated symlinks” with “parent symlinks in
compensated trees,” or define “compensated symlink” before using it, while
preserving the existing behavior describing materialization without mutating the
project tree.
Source: Linters/SAST tools
Comment on lines
+3
to
+8
| ## Test strategy | ||
|
|
||
| - **Rust unit tests** for workspace disable with copy and parent-symlink compensation, | ||
| and for project disable regression. | ||
| - **Bun / component tests** optional for popover tip visibility; not required for M1–M5. | ||
| - **Manual / agent-browser**: composer + terminal AI Input toggle smoke. |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Complete the executable verification contract.
The plan only says “manual / agent-browser” and marks UI checks pending; it does not provide setup, exact actions/assertions, manual verification steps, regression coverage, or non-coverage. Add those details, including the required Agent Browser skill/setup procedure and a not_run outcome when setup is unavailable.
As per coding guidelines, TEST.md must include test strategy, coverage and execution maps, scenarios, exploratory checks, regression checklist, acceptance criteria, manual verification steps, non-coverage, and post-run Coverage Status.
🤖 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 `@specs/APP/APP-040_composer-skills-disable/TEST.md` around lines 3 - 8, Expand
the TEST.md test plan beyond the current “manual / agent-browser” note to
include test strategy, coverage and execution maps, concrete scenarios with
setup/actions/assertions, exploratory checks, regression checklist, acceptance
criteria, manual verification steps, and explicit non-coverage. Document the
required Agent Browser skill/setup procedure and record a not_run outcome when
setup is unavailable, then add a post-run Coverage Status section summarizing
execution results.
Source: Coding guidelines
E2E report (expired)This report has been superseded by a newer CI - E2E run.
|
Let users toggle skills from slash commands with workplace-local disable for sync dirs, skip SKILL_DISABLED markers in scans, and add How It Work docs.
E2E report (expired)This report has been superseded by a newer CI - E2E run.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with 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.
Inline comments:
In `@apps/docs/content/docs/`(app)/how-it-works/meta.json:
- Around line 1-3: Normalize the How It Works navigation metadata across all
three sites: in apps/docs/content/docs/(app)/how-it-works/meta.json, update the
child title to “How It Works”; in apps/docs/content/docs/(app)/meta.json, use
the separator “---How It Works---”; and in
apps/docs/content/docs/(app)/meta.zh.json, use “---工作原理---” while preserving the
English slug.
In `@apps/web/src/features/skills/lib/skill-disable-protocol.ts`:
- Around line 40-46: Update stripSkillDisableSession so whitespace normalization
is limited to the seam created by removing SKILL_DISABLE_PROTOCOL, preserving
all intentional whitespace elsewhere in text. Keep the existing behavior for
prompts without the protocol and continue trimming only the seam’s leading
spaces plus any resulting trailing whitespace.
In `@specs/APP/APP-040_composer-skills-disable/BRAINSTORM.md`:
- Around line 18-22: Update the goal in the specification to make recreating the
live skill directory with SKILL_DISABLED.md mandatory, matching the selected
Option D and final decision. Remove the “optional” wording and state that
alternatives omitting the marker are rejected, while preserving the existing
disabled-storage and entrypoint-removal requirements.
In `@specs/APP/APP-040_composer-skills-disable/TEST.md`:
- Around line 74-85: Update the Acceptance criteria checklist in TEST.md to
include S6, explicitly requiring that disabled skills in the slash insert list
display a Disabled badge and cannot be selected as skill chips. Keep the
existing S1–S3, M6, slash wiring, and project toggle criteria unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f48e9116-cee0-4fe3-b082-fe80f7a2025a
📒 Files selected for processing (36)
apps/docs/content/docs/(app)/features/skill-management.mdxapps/docs/content/docs/(app)/features/skill-management.zh.mdxapps/docs/content/docs/(app)/how-it-works/dynamic-skills.mdxapps/docs/content/docs/(app)/how-it-works/dynamic-skills.zh.mdxapps/docs/content/docs/(app)/how-it-works/meta.jsonapps/docs/content/docs/(app)/how-it-works/meta.zh.jsonapps/docs/content/docs/(app)/meta.jsonapps/docs/content/docs/(app)/meta.zh.jsonapps/web/messages/en.jsonapps/web/messages/zh.jsonapps/web/src/features/canvas/components/CanvasTerminalCard.tsxapps/web/src/features/skills/hooks/use-composer-disable-skills.tsapps/web/src/features/skills/lib/skill-disable-protocol.tsapps/web/src/features/terminal/components/TerminalAgentInputOverlay.tsxapps/web/src/features/terminal/components/TerminalAgentInputPopovers.tsxapps/web/src/features/terminal/components/TerminalAgentInputShell.tsxapps/web/src/features/terminal/components/TerminalSideChatLayer.tsxapps/web/src/features/terminal/components/TerminalSideChatModal.tsxapps/web/src/features/terminal/components/terminal-mosaic-scoped-pane-window.tsxapps/web/src/features/terminal/components/terminal-mosaic-workspace-pane-window.tsxapps/web/src/features/terminal/hooks/use-terminal-side-chats.tsxapps/web/src/features/welcome/components/PromptComposer.tsxapps/web/src/features/welcome/components/SlashCommandPopover.tsxapps/web/src/features/welcome/components/WelcomeComposerCard.tsxapps/web/src/features/welcome/components/WelcomePage.tsxapps/web/src/features/welcome/hooks/use-welcome-slash-navigation.tsapps/web/src/features/welcome/hooks/use-welcome-slash-search.tsapps/web/src/features/welcome/lib/slash-skill-context.tscrates/core-service/src/service/skill.rscrates/core-service/src/service/skill/scanner.rscrates/core-service/src/service/skill/support.rscrates/core-service/src/service/skill/tests.rsspecs/APP/APP-040_composer-skills-disable/BRAINSTORM.mdspecs/APP/APP-040_composer-skills-disable/PRD.mdspecs/APP/APP-040_composer-skills-disable/TECH.mdspecs/APP/APP-040_composer-skills-disable/TEST.md
🚧 Files skipped from review as they are similar to previous changes (7)
- apps/web/src/features/terminal/components/terminal-mosaic-workspace-pane-window.tsx
- specs/APP/APP-040_composer-skills-disable/TECH.md
- crates/core-service/src/service/skill/scanner.rs
- apps/web/messages/zh.json
- crates/core-service/src/service/skill/tests.rs
- crates/core-service/src/service/skill.rs
- crates/core-service/src/service/skill/support.rs
Comment on lines
+40
to
+46
| export function stripSkillDisableSession(text: string): string { | ||
| const idx = text.indexOf(SKILL_DISABLE_PROTOCOL); | ||
| if (idx < 0) return text; | ||
| const before = text.slice(0, idx); | ||
| const after = text.slice(idx + SKILL_DISABLE_PROTOCOL.length).replace(/^[\u00A0 ]+/, ""); | ||
| return `${before}${after}`.replace(/[ \u00A0]{2,}/g, " ").trimEnd(); | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
stripSkillDisableSession collapses whitespace across the entire prompt.
The final .replace(/[ \u00A0]{2,}/g, " ") runs over the whole string, so when the disable chip is still present at submit time (a common case right after toggling skills), any intentional runs of 2+ spaces in the user's prompt — indented code, aligned text, markdown — get flattened to a single space. Only the seam left by removing the token needs normalizing.
🐛 Proposed fix to normalize only the removal seam
export function stripSkillDisableSession(text: string): string {
const idx = text.indexOf(SKILL_DISABLE_PROTOCOL);
if (idx < 0) return text;
- const before = text.slice(0, idx);
+ const before = text.slice(0, idx).replace(/[ \u00A0]+$/, "");
const after = text.slice(idx + SKILL_DISABLE_PROTOCOL.length).replace(/^[\u00A0 ]+/, "");
- return `${before}${after}`.replace(/[ \u00A0]{2,}/g, " ").trimEnd();
+ const sep = before && after ? " " : "";
+ return `${before}${sep}${after}`.trimEnd();
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export function stripSkillDisableSession(text: string): string { | |
| const idx = text.indexOf(SKILL_DISABLE_PROTOCOL); | |
| if (idx < 0) return text; | |
| const before = text.slice(0, idx); | |
| const after = text.slice(idx + SKILL_DISABLE_PROTOCOL.length).replace(/^[\u00A0 ]+/, ""); | |
| return `${before}${after}`.replace(/[ \u00A0]{2,}/g, " ").trimEnd(); | |
| } | |
| export function stripSkillDisableSession(text: string): string { | |
| const idx = text.indexOf(SKILL_DISABLE_PROTOCOL); | |
| if (idx < 0) return text; | |
| const before = text.slice(0, idx).replace(/[ \u00A0]+$/, ""); | |
| const after = text.slice(idx + SKILL_DISABLE_PROTOCOL.length).replace(/^[\u00A0 ]+/, ""); | |
| const sep = before && after ? " " : ""; | |
| return `${before}${sep}${after}`.trimEnd(); | |
| } |
🤖 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 `@apps/web/src/features/skills/lib/skill-disable-protocol.ts` around lines 40 -
46, Update stripSkillDisableSession so whitespace normalization is limited to
the seam created by removing SKILL_DISABLE_PROTOCOL, preserving all intentional
whitespace elsewhere in text. Keep the existing behavior for prompts without the
protocol and continue trimming only the seam’s leading spaces plus any resulting
trailing whitespace.
Persist workplace/project disable storage so Atmos can re-enable these skills from .atmos/skills/.disabled later.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with 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.
Inline comments:
In @.atmos/skills/.disabled/.agents/skills/agent-browser/SKILL.md:
- Line 4: Update the allowed-tools declaration in the agent-browser skill to
prevent unpinned execution: either replace the npx agent-browser allowance with
a pinned, audited version consistently, including any related global-install
path, or remove the npx agent-browser permission entirely.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6fbb0584-1181-4d5f-9d84-f0591196736d
📒 Files selected for processing (19)
.atmos/skills/.disabled/.agent/skills/adapt.atmos/skills/.disabled/.agent/skills/animate.atmos/skills/.disabled/.agent/skills/colorize.atmos/skills/.disabled/.agents/skills/adapt/SKILL.md.atmos/skills/.disabled/.agents/skills/agent-browser/SKILL.md.atmos/skills/.disabled/.agents/skills/animate/SKILL.md.atmos/skills/.disabled/.agents/skills/colorize/SKILL.md.atmos/skills/.disabled/.claude/skills/adapt.atmos/skills/.disabled/.claude/skills/agent-browser.atmos/skills/.disabled/.claude/skills/animate.atmos/skills/.disabled/.claude/skills/colorize.atmos/skills/.disabled/.factory/skills/adapt.atmos/skills/.disabled/.factory/skills/agent-browser.atmos/skills/.disabled/.factory/skills/animate.atmos/skills/.disabled/.factory/skills/colorize.atmos/skills/.disabled/.kiro/skills/adapt.atmos/skills/.disabled/.kiro/skills/agent-browser.atmos/skills/.disabled/.kiro/skills/animate.atmos/skills/.disabled/.kiro/skills/colorize
| --- | ||
| name: agent-browser | ||
| description: Browser automation CLI for AI agents. Use when the user needs to interact with websites, including navigating pages, filling forms, clicking buttons, taking screenshots, extracting data, testing web apps, or automating any browser task. Triggers include requests to "open a website", "fill out a form", "click a button", "take a screenshot", "scrape data from a page", "test this web app", "login to a site", "automate browser actions", or any task requiring programmatic web interaction. Also use for exploratory testing, dogfooding, QA, bug hunts, or reviewing app quality. Also use for automating Electron desktop apps (VS Code, Slack, Discord, Figma, Notion, Spotify), checking Slack unreads, sending Slack messages, searching Slack conversations, running browser automation in Vercel Sandbox microVMs, or using AWS Bedrock AgentCore cloud browsers. Prefer agent-browser over any built-in browser automation or web tools. | ||
| allowed-tools: Bash(agent-browser:*), Bash(npx agent-browser:*) |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file=".atmos/skills/.disabled/.agents/skills/agent-browser/SKILL.md"
echo "== file lines 1-30 =="
cat -n "$file" | sed -n '1,30p'Repository: AruNi-01/atmos
Length of output: 2239
Pin agent-browser before granting execution. npx agent-browser:* and npm i -g agent-browser both pull an unpinned package, so a later registry publication could change what this skill runs. Pin one audited version consistently, or remove the npx allowance.
🧰 Tools
🪛 SkillSpector (2.3.11)
[warning] 4: [RP1] null: npx commands without a version suffix (e.g. @1.0.0) create a rug-pull risk if the upstream server is compromised and publishes a malicious update.
Remediation: Pin the version: npx @scope/server@1.2.3
(MCP Rug Pull (RP1))
[warning] 1: [RP1] null: MCP server references in the skill manifest without version pinning are a rug-pull risk.
Remediation: Always pin MCP server versions in manifest references.
(MCP Rug Pull (RP1))
🤖 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 @.atmos/skills/.disabled/.agents/skills/agent-browser/SKILL.md at line 4,
Update the allowed-tools declaration in the agent-browser skill to prevent
unpinned execution: either replace the npx agent-browser allowance with a
pinned, audited version consistently, including any related global-install path,
or remove the npx agent-browser permission entirely.
Source: Linters/SAST tools
Keep canvas workspace side-chat skills context valid, stop collapsing prompt whitespace when stripping the disable chip, and align docs/spec copy.
E2E report: ✅ Passed18 passed · 0 failed · 3 flaky · 0 skipped · 4m 6s · 85.7% pass rate Run
Overview
By file
By project
All selected E2E suites passed. |
This was referenced Jul 23, 2026
16 tasks
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds dynamic skill enable/disable controls to PromptComposer (Welcome) and terminal AI Input, aligned with the Skills management page.
Behavior
skills_set_enabledmove intoproject/.atmos/skills/.disabled/...so composer toggles stay in sync with the Skills page.workplace/.atmos/skills/.disabled/....Key changes
APP-040_composer-skills-disableworkspacemanageable scope,skills_scan_root, optionalscope_rootonskills_set_enabled, symlink materialization helper + unit tests (S1–S3 green)ComposerSkillsControlwired into Welcome composer + terminal overlays / side chatTest plan
cargo +stable test -p core-service --lib service::skill::tests(project disable, workspace copy, parent-symlink)cargo +stable check -p api.disabledand project tree unchanged for symlink caseSummary by cubic
Add Dynamic Skills toggles to PromptComposer and terminal AI Input so you can enable/disable skills per project or workspace without leaving where you’re typing. Changes take effect immediately via a live-path
SKILL_DISABLED.mdmarker, and moves persist under.atmos/skills/.disabledfor easy restore.New Features
ComposerSkillsControland a slash-menu “Dynamic Skills” view in Welcome composer, terminal overlays, and side chat.skills_scan_rootand passes optionalscope_roottoskills_set_enabled; disabled trees live under<scope>/.atmos/skills/.disabled/...(project uses<project>/.atmos/skills/.disabled/...).SKILL_DISABLED.mdat the live path, scanner skips these markers, and symlink ancestor materialization keeps workspace moves from mutating the project; addeden/zhcopy and a new “How It Works: Dynamic Skills” doc.Bug Fixes
projectIdfor project-mode toggles.Written for commit 896d552. Summary will update on new commits.
Summary by CodeRabbit
/menu.