feat: Teams asset sharing dashboard, remove Claude Code installation, docs and pricing updates#90
Conversation
…t cleanup - Remove 4-tab layout (Assets/Push/Setup/Roles), flatten to single page - Add per-asset action buttons: Install, Update (via sx remove+add), Remove - Add new POST /api/teams/update-asset endpoint for proper version updates - Inline push panel with "Push to:" scope selector - Add Reconfigure button to repository setup section - Remove Roles feature entirely (TeamsRolesTab, roles endpoints, hook methods) - Rename all Vault* types to Teams* (VaultStatus→TeamsStatusResponse, etc.) - Rename vaultUrl→repoUrl across full stack (JSON wire format + consumers) - Clean up vault references in settings.json, installer, rules, README - Update navigation: /vault→/teams route, sidebar, command palette, shortcuts
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
Claude finished @maxritter's task in 2m 9s —— View job Code Review
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (6)
WalkthroughMigrates "Vault" to "Teams" across backend, frontend, docs, tests, and installer. Adds a new TeamsRoutes API and Teams UI (views, setup, push, gating), removes Vault routes and UI, updates hooks and types to Teams, adjusts installer/dependency handling (removes automated Claude Code install), and updates CI, docs, and tests accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Browser
participant TeamsView as TeamsView (UI)
participant UseTeams as useTeams Hook
participant Worker as Worker (HTTP /api/teams)
participant SX as sx CLI
participant GitFS as Git / Filesystem
Browser->>TeamsView: user action (open Teams / push / install)
TeamsView->>UseTeams: call discover / push / init / update / remove
UseTeams->>Worker: HTTP POST /api/teams/... (discover|push|init|install|remove)
Worker->>SX: spawn `sx ...` command (with timeout)
SX->>GitFS: read/write repo or .claude dir (push/discover)
SX-->>Worker: command stdout/exit (JSON or text)
Worker-->>UseTeams: JSON response (status, detail, errors)
UseTeams-->>TeamsView: update UI state (toasts, table, summaries)
TeamsView-->>Browser: render updated asset states
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (7)
README.md-315-315 (1)
315-315:⚠️ Potential issue | 🟡 MinorComplete the README terminology migration to avoid user confusion.
Good update here, but the README still references legacy Vault terms elsewhere (for example: Line 158
/vault, Line 186Vaultview, Line 517/vault). If/teamsis now the canonical route/feature name, these should be updated in the same pass so docs match product behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` at line 315, The README still uses legacy "Vault" terminology and routes (e.g., occurrences of "/vault", "Vault" view, and the legacy file reference `team-vault.md`) which should be migrated to the new canonical "teams" naming; search for and replace all references of "/vault" with "/teams", rename mentions of "Vault" view to "Teams" view (preserving context/capitalization), and update or remove `team-vault.md` references to the new `teams` documentation or filename so the docs consistently reflect the product naming.console/src/ui/viewer/components/CommandPalette.tsx-124-132 (1)
124-132:⚠️ Potential issue | 🟡 MinorArrow-key navigation breaks when no commands are available.
When
filteredCommands.length === 0, modulo operations setselectedIndextoNaN.🔧 Suggested guard
const handleKeyDown = (e: React.KeyboardEvent) => { + if ( + filteredCommands.length === 0 && + (e.key === "ArrowDown" || e.key === "ArrowUp" || e.key === "Enter") + ) { + e.preventDefault(); + return; + } + switch (e.key) { case "ArrowDown": e.preventDefault(); setSelectedIndex((i) => (i + 1) % filteredCommands.length); break;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@console/src/ui/viewer/components/CommandPalette.tsx` around lines 124 - 132, Arrow-key handlers in CommandPalette.tsx compute new selected index using modulo which yields NaN when filteredCommands.length === 0; add a guard in the ArrowDown and ArrowUp cases (or before invoking setSelectedIndex) to no-op when filteredCommands.length === 0 so setSelectedIndex is not called with invalid math, keeping the current selection unchanged (or explicitly set to a safe default like -1/0 if that state is handled elsewhere).console/src/ui/viewer/views/Teams/TeamsSummaryCards.tsx-49-54 (1)
49-54:⚠️ Potential issue | 🟡 MinorKeep the stat description aligned with the displayed metric.
When title/value include “Other”, the description “Slash commands” is misleading.
💡 Suggested tweak
- <div className="stat-desc">Slash commands</div> + <div className="stat-desc"> + {other > 0 ? "Slash commands + other asset types" : "Slash commands"} + </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@console/src/ui/viewer/views/Teams/TeamsSummaryCards.tsx` around lines 49 - 54, The stat description in TeamsSummaryCards.tsx (the elements with classNames stat-title, stat-value, stat-desc that display commands and other via variables commands and other) can become misleading when other > 0; update the rendering logic for the stat-desc so it conditionally matches the displayed metric (e.g., when other > 0 show a description like "Slash commands and other" or similar, otherwise keep "Slash commands") so the description aligns with the title/value.console/tests/ui/teams-view.test.ts-116-116 (1)
116-116:⚠️ Potential issue | 🟡 MinorUpdate stale test naming to Teams terminology.
The test title still says “vault assets”; rename it to “team assets” for accurate behavior description.
As per coding guidelines, "Review test code briefly. Focus on: Clear test names that describe behavior".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@console/tests/ui/teams-view.test.ts` at line 116, Rename the test title string in teams-view.test.ts from "renders install button for uninstalled vault assets" to "renders install button for uninstalled team assets" so the test name matches the Teams terminology; locate the it(...) call in the file (the test case starting with it("renders install button for uninstalled vault assets", ...) and update only the descriptive string, leaving the test body and assertions unchanged.console/src/ui/viewer/views/Teams/index.tsx-41-42 (1)
41-42:⚠️ Potential issue | 🟡 MinorAvoid gating with
nulltier before license load completes.Because
useLicense().isLoadingisn’t used,TeamGatereceivesnullduring fetch and can briefly render the locked/upgrade state for eligible users. Gate these sections only after license loading settles.Also applies to: 182-190, 210-220, 254-256
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@console/src/ui/viewer/views/Teams/index.tsx` around lines 41 - 42, The Team gating logic is using license which can be null while the license is still loading; update the code that calls useLicense() to also read isLoading and defer rendering any TeamGate checks until loading settles (for example, return a loader or skip rendering TeamGate when isLoading is true), and then pass the resolved license/tier (or undefined) into TeamGate instead of null so eligible users don't briefly see locked UI; apply the same change to all TeamGate usages referenced (the blocks around where useLicense() is read and TeamGate is rendered).console/src/ui/viewer/views/Dashboard/TeamsStatus.tsx-68-71 (1)
68-71:⚠️ Potential issue | 🟡 MinorUse a composite key for availability counts (name-only matching is lossy).
Current logic matches installed/catalog entries by
nameonly. If two assets share a name across types,availableCountis incorrect (Line [68]-Line [71]). Match ontype + nameto avoid false exclusions.Proposed fix
- const installedNames = new Set(assets.map((a) => a.name)); + const installedKeys = new Set(assets.map((a) => `${a.type}:${a.name}`)); const availableCount = catalog.filter( - (c) => !installedNames.has(c.name), + (c) => !installedKeys.has(`${c.type}:${c.name}`), ).length;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@console/src/ui/viewer/views/Dashboard/TeamsStatus.tsx` around lines 68 - 71, The availability check in TeamsStatus.tsx builds installedNames from assets using name-only and computes availableCount by filtering catalog by name, which breaks when assets share names across types; update the logic to use a composite key (e.g., `${type}::${name}`) for both installedNames and the catalog filter so matching is done on type+name (ensure you reference the installedNames set and availableCount calculation), and handle possible missing/undefined type values consistently when building the composite keys.console/src/ui/viewer/views/Teams/TeamsSetupTab.tsx-66-71 (1)
66-71:⚠️ Potential issue | 🟡 MinorReplace stale “vault” placeholders in Teams UI copy.
Lines [68]-[70] still show legacy vault wording (
team-vault.git,/path/to/vault). This is user-facing and inconsistent with the Teams rename.Proposed fix
placeholder={ repoType === "git" - ? "git@github.com:org/team-vault.git" + ? "git@github.com:org/team-repo.git" : repoType === "path" - ? "/path/to/vault" + ? "/path/to/team-repo" : "https://skills.new/..." }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@console/src/ui/viewer/views/Teams/TeamsSetupTab.tsx` around lines 66 - 71, In TeamsSetupTab (the placeholder ternary using repoType), replace legacy "vault" wording in the user-facing placeholders: change "git@github.com:org/team-vault.git" to a non-vault name (e.g., "git@github.com:org/team-repo.git") and change "/path/to/vault" to a matching local path (e.g., "/path/to/team-repo" or "/path/to/teams") so the placeholders reflect the Teams rename and remain consistent with the UI copy.
🧹 Nitpick comments (4)
console/tests/hooks/useSettings.test.ts (1)
47-53: Strengthen this test to assert the exact command key set.Line 47 says “all six commands,” but the current assertions allow additional unexpected keys (except
vault). Assert exact keys to catch accidental defaults drift.Proposed test tightening
it('DEFAULT_SETTINGS has all six commands', async () => { const { DEFAULT_SETTINGS } = await import('../../src/ui/viewer/hooks/useSettings.js'); const expected = ['spec', 'spec-plan', 'spec-implement', 'spec-verify', 'sync', 'learn']; - for (const cmd of expected) { - expect(DEFAULT_SETTINGS.commands[cmd]).toBeDefined(); - } - expect(DEFAULT_SETTINGS.commands['vault']).toBeUndefined(); + expect(Object.keys(DEFAULT_SETTINGS.commands).sort()).toEqual(expected.sort()); });As per coding guidelines, "
**/tests/**: Review test code briefly. Focus on: Test coverage for the feature being tested."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@console/tests/hooks/useSettings.test.ts` around lines 47 - 53, The test currently checks only presence of six expected commands and absence of 'vault', so update the assertion to verify the exact set of keys in DEFAULT_SETTINGS.commands to prevent accidental additions; in the test for DEFAULT_SETTINGS (in useSettings.test.ts) import DEFAULT_SETTINGS and replace the per-key checks with a single assertion that Object.keys(DEFAULT_SETTINGS.commands) (sorted) strictly equals the expected array (sorted) — referencing DEFAULT_SETTINGS and its commands property to locate the code to change.console/tests/worker/teams-routes.test.ts (1)
168-190: Discover test should avoid real fs/git dependencies and sleep-based waiting.Mock filesystem/git interactions and
awaitthe handler directly to keep tests deterministic and non-flaky.As per coding guidelines, "`**/tests/**`: Review test code briefly. Focus on: Proper mocking of external dependencies".✅ Minimal deterministic pattern
- discoverHandler({}, fakeRes); - await new Promise((r) => setTimeout(r, 50)); + await discoverHandler({}, fakeRes);Also applies to: 184-186
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@console/tests/worker/teams-routes.test.ts` around lines 168 - 190, The test calls TeamsRoutes.setupRoutes then invokes the discover handler and uses a sleep to wait; instead, mock external FS/Git interactions used by TeamsRoutes (e.g., any fs, git, or repo helpers it depends on) using jest mocks/spies and call the route handler as an async function so you can await it directly; locate the route via TeamsRoutes.setupRoutes and the discoverHandler reference, provide a fakeReq/fakeRes that returns a Promise from json/status (or make the handler return a Promise) and then await discoverHandler(fakeReq, fakeRes) instead of using setTimeout to make the test deterministic and avoid touching the real filesystem or git.console/tests/ui/teams-navigation.test.ts (1)
11-49: Prefer module/behavior assertions over file-content matching for routing wiring.These checks are mostly string scans, which are brittle and can miss real wiring regressions. Prefer asserting exported route/nav/command data structures directly where possible.
As per coding guidelines, "Review test code briefly. Focus on: Test coverage for the feature being tested".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@console/tests/ui/teams-navigation.test.ts` around lines 11 - 49, Replace brittle fs string-search assertions with direct module imports and assertions against the exported route/nav/command structures: import the routes export from the module that defines App.tsx routes (e.g., the exported routes array or a getRoutes function) and assert there is an entry with component/name "TeamsView" and path "/teams"; import the nav items export from SidebarNav (e.g., navItems or sidebarConfig) and assert an item with label "Teams" has icon "users" and href or to "/teams"; import the commands export from CommandPalette (e.g., commands or paletteItems) and assert a command labeled "Go to Teams" exists with keybinding "G V" and a navigate handler or route pointing to "/teams". Ensure tests use the exported symbols (routes, navItems, commands) rather than reading file text so wiring is verified at runtime.console/src/ui/viewer/views/Teams/index.tsx (1)
12-19: DeduplicateformatRepoUrlinto a shared utility.This formatter is now repeated across Teams views, which increases drift risk for URL display behavior. Extract to a shared helper and import it in both places.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@console/src/ui/viewer/views/Teams/index.tsx` around lines 12 - 19, The formatRepoUrl function is duplicated; extract it into a shared helper (e.g., export function formatRepoUrl(url: string): string in a new/util file) and replace the local definition in Teams view(s) with an import of that helper; update the callers in Teams/index.tsx (and any other Teams view files using formatRepoUrl) to import the new shared function and remove the local implementation so URL formatting is centralized.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@console/src/services/worker/http/routes/TeamsRoutes.ts`:
- Around line 457-468: The update-asset handler (handleUpdateAsset) mutates the
lockfile and runs installs without synchronizing with concurrent installs; guard
the critical section by checking and setting the shared flag _isInstalling: at
the start of the handler, if (this._isInstalling) return a 409/conflict
response, otherwise set this._isInstalling = true before performing lockfile
mutation and the install, and ensure you clear it in a finally block so it’s
always reset; apply the same pattern to the other install/update handler
referenced around lines 495-499 (the other route handler that mutates the
lockfile) so both use the same check/set/try...finally/unset sequence and avoid
race conditions.
- Around line 320-324: The catch block in the Teams push handler currently
returns a 200 OK with an error payload; update the error handling to send an
appropriate non-2xx HTTP status (e.g., res.status(500)) before returning the
JSON error, and include the same message and logger.error usage (see the catch
that defines const message = (error as Error).message and calls
logger.error("HTTP", "Teams push failed", { name }, error as Error)); apply the
identical change to the catch blocks in handleRemove, handleInit, and
handleUpdateAsset so failures return a non-2xx status with the same error JSON
and logging behavior.
- Around line 470-494: The current remove-then-add sequence can leave the
lockfile degraded if add/install fails; change the flow in the method that calls
runSxCommand so you add the new asset before removing the old (build addArgs and
call runSxCommand(addArgs, PUSH_TIMEOUT_MS) first), and only on successful add
call runSxCommand([...sxPath, "remove", name, "--version",
String(currentVersion), "--yes"], REMOVE_TIMEOUT_MS) to drop the old entry;
additionally wrap both operations in try/catch and on any failure attempt a
compensating rollback by calling runSxCommand([...sxPath, "add", name,
"--version", String(currentVersion), "--yes"], PUSH_TIMEOUT_MS) to restore the
previous version (use currentVersion, addArgs, runSxCommand, REMOVE_TIMEOUT_MS,
PUSH_TIMEOUT_MS to locate code).
- Around line 282-290: The current prefix check on resolvedSource can be
bypassed via symlinks; change validation to compare real filesystem paths: call
fs.promises.realpath (or fs.realpathSync) for projectRoot and resolvedSource
(referencing projectRoot and resolvedSource variables and path.resolve usage)
and verify the resolvedSourceRealpath equals projectRootRealpath or
startsWith(projectRootRealpath + path.sep) before accepting; return the same 400
error on failure. Ensure to handle realpath errors (treat as invalid) and
preserve use of process.env.CLAUDE_PROJECT_ROOT when computing
projectRootRealpath.
In `@console/src/ui/viewer/components/TeamGate.tsx`:
- Around line 16-18: The overlay wrapper currently uses pointer-events-none
which doesn't prevent keyboard focus; update the wrapper div in TeamGate (the
element rendering {children}) to remove keyboard access by adding accessibility
attributes: set aria-hidden="true" and add the inert attribute (and fallback to
tabIndex={-1} if inert not supported) on that div so underlying interactive
controls are excluded from the accessibility tree and cannot receive keyboard
focus; ensure the same element keeps the visual classes (opacity-30 blur-[2px]
select-none) while adding these attributes.
In `@console/src/ui/viewer/hooks/useStats.ts`:
- Around line 178-183: The loadTeamsStatusData callback currently assigns any
JSON response to teamsStatusData; update loadTeamsStatusData to first verify the
fetch response with if (!res.ok) return or set a safe fallback, then parse JSON
and validate its shape (e.g., confirm expected keys/array via Array.isArray or
typeof checks) before calling setTeamsStatusData; on invalid shape or parse
errors, set a safe default (empty array/object) and handle/log the error in the
catch block so setTeamsStatusData is never called with unexpected payloads.
In `@console/src/ui/viewer/hooks/useTeams.ts`:
- Around line 241-317: The issue: successful mutations do not consistently
refresh Teams state—modify initTeams, pushAsset, and removeAsset to call and
await fetchStatus() when the API returns success (like updateAsset does) so the
UI isn't stale; also update the useCallback dependency arrays for pushAsset and
removeAsset to include fetchStatus so closures reference the latest function.
Ensure each of initTeams, pushAsset, and removeAsset awaits fetchStatus() after
detecting data.success (or after res.ok and parsed success) before returning.
In `@console/src/ui/viewer/views/Teams/TeamsAssetTable.tsx`:
- Around line 163-174: The remove flow in handleRemoveClick can leave isRemoving
true if onRemove rejects; wrap the await onRemove(asset.name) in a try/finally
(or try/catch/finally) so setIsRemoving(false) always runs, and keep the
existing setConfirmRemove(false) behavior; update handleRemoveClick to
setIsRemoving(true) before the try, await onRemove inside the try, and move
setIsRemoving(false) into the finally block so failures won't leave the UI
stuck.
In `@console/src/ui/viewer/views/Teams/TeamsPushPanel.tsx`:
- Around line 70-95: The handlePush function can leave the UI stuck if pushAsset
throws; wrap the per-asset loop in a try/catch and ensure setIsPushing(false)
runs in a finally block. Specifically, in handlePush use try { ... for each
asset await pushAsset(...) ... } catch (err) { record the error into
localResults for the failing asset (set success: false and error: String(err))
and update setPushResults } finally { setIsPushing(false) } and only call
onPushComplete when allSucceeded remains true after the try/catch; reference
handlePush, pushAsset, setIsPushing, setPushResults, localResults, and
onPushComplete when making the changes.
- Around line 40-48: The effect in TeamsPushPanel that calls discover() doesn't
handle promise rejection, leaving isLoading true on errors; update the useEffect
so the async call to discover() is awaited inside a try/catch/finally (or add a
.catch/.finally chain) to ensure setIsLoading(false) runs on both success and
failure, still guarding with the mounted flag before calling
setAssets/setRepoUrl; ensure any error is swallowed or logged (e.g., via
console.error or an existing logger) and that the cleanup function sets mounted
= false as before.
In `@console/src/ui/viewer/views/Teams/TeamsSetupTab.tsx`:
- Around line 24-35: handleInit currently calls initTeams directly so if
initTeams throws the async error the UI stays stuck with
setIsInitializing(true); wrap the initTeams call in a try/catch/finally inside
handleInit: in try await initTeams(...); in catch setInitError to the caught
error message (or a friendly fallback) and ensure setIsReconfiguring/setRepoUrl
behavior only on success; in finally always call setIsInitializing(false) so the
Initialize button is never left disabled. Reference symbols: handleInit,
initTeams, setIsInitializing, setInitError, setIsReconfiguring, setRepoUrl.
In `@console/tests/hooks/use-teams.test.ts`:
- Around line 166-179: Replace the brittle source-string assertions in the test
with behavioral tests that mock global.fetch and exercise the hook's exported
actions (discover, pushAsset, removeAsset, initTeams, updateAsset) from
src/ui/viewer/hooks/useTeams.ts: for each action, mock fetch responses and
assert the correct endpoint, HTTP method, and request body were used, then
verify the hook's state/mutation calls (e.g., that isInstalling toggles,
mergeAssets is called with returned assets, removeAsset/updateAsset cause
expected state changes, and initTeams triggers the init endpoint and subsequent
refresh); use jest.fn() or a fetch-mock and clear/reset mocks between tests to
ensure isolation and meaningful failure signals.
In `@console/tests/ui/teams-install.test.ts`:
- Around line 4-71: The current tests only assert source text; add a runtime
behavior test in console/tests/ui/teams-install.test.ts that renders the Teams
view (importing the component from src/ui/viewer/views/Teams/index.tsx), mocks
the sync API and/or the internal sync handler (e.g., handleInstall or
TeamsSyncButton click), simulates a user click on the "Sync All" button, and
asserts that the useToast methods (.success, .error, .warning) are invoked for
success, error, and timeout scenarios; use jest.fn() or your existing test
helpers to spy on the toast returned by useToast() and stub network calls to
drive each outcome.
In `@console/tests/ui/teams-view.test.ts`:
- Around line 87-143: Add interaction tests for TeamsAssetTable that mount the
component (instead of only SSR render) and assert the onUpdate and onRemove
callbacks are invoked through user interactions: locate and click the "Update"
button and verify onUpdate was called; locate and click the "Remove" button,
drive the confirmation dialog flow (click confirm) and verify onRemove was
called; and for uninstalled vault assets locate and click "Install" and verify
appropriate install handler or onUpdate is invoked. Use the same props pattern
from the existing tests, pass jest.fn() spies for onUpdate/onRemove, render
TeamsAssetTable (imported from TeamsAssetTable.js) with tier "team" and "solo"
scenarios, simulate clicks with a DOM testing utility (e.g.,
`@testing-library/react` + userEvent), and assert the spies are called and the
remove-confirmation path triggers onRemove only after confirm.
In `@console/tests/worker/teams-routes.test.ts`:
- Around line 19-37: Add behavior tests for the POST /api/teams/update-asset
route (in addition to the existing registration test) that exercise validation,
command sequencing, and failure paths: instantiate TeamsRoutes, capture the
registered POST handler for "/api/teams/update-asset" (same pattern used to
capture registrations), then call that handler directly with mocked req/res
objects; add separate tests to assert (1) invalid input returns the expected 4xx
validation response, (2) valid input triggers the expected sequence of
operations by mocking and spying the lockfile/installer services (assert call
order and payloads), and (3) when a mocked dependency throws, the handler
returns the correct error status and logs/response; use the same registered
capture mechanism and mock services used elsewhere in this test file to keep
tests focused and isolated.
In `@docs/site/src/pages/docs/TeamsSection.tsx`:
- Around line 10-16: The docs still reference Vault/Roles terminology and the
removed commands; update all occurrences of "Vault", "vault settings", "Vault
Setup" and wording like "manage roles" to the new Teams terminology (e.g.,
"Team" or "Teams") and replace deprecated CLI commands such as `sx vault show`
with the current Teams equivalents (e.g., `sx teams show`), and remove any
instructions about role management flow; update the strings in TeamsSection.tsx
(e.g., the "Vault Setup" and "Push & Pull" entries) and the other affected
ranges (around lines 85-89 and 176-177) to use the new Team/Teams naming and
capabilities.
---
Minor comments:
In `@console/src/ui/viewer/components/CommandPalette.tsx`:
- Around line 124-132: Arrow-key handlers in CommandPalette.tsx compute new
selected index using modulo which yields NaN when filteredCommands.length === 0;
add a guard in the ArrowDown and ArrowUp cases (or before invoking
setSelectedIndex) to no-op when filteredCommands.length === 0 so
setSelectedIndex is not called with invalid math, keeping the current selection
unchanged (or explicitly set to a safe default like -1/0 if that state is
handled elsewhere).
In `@console/src/ui/viewer/views/Dashboard/TeamsStatus.tsx`:
- Around line 68-71: The availability check in TeamsStatus.tsx builds
installedNames from assets using name-only and computes availableCount by
filtering catalog by name, which breaks when assets share names across types;
update the logic to use a composite key (e.g., `${type}::${name}`) for both
installedNames and the catalog filter so matching is done on type+name (ensure
you reference the installedNames set and availableCount calculation), and handle
possible missing/undefined type values consistently when building the composite
keys.
In `@console/src/ui/viewer/views/Teams/index.tsx`:
- Around line 41-42: The Team gating logic is using license which can be null
while the license is still loading; update the code that calls useLicense() to
also read isLoading and defer rendering any TeamGate checks until loading
settles (for example, return a loader or skip rendering TeamGate when isLoading
is true), and then pass the resolved license/tier (or undefined) into TeamGate
instead of null so eligible users don't briefly see locked UI; apply the same
change to all TeamGate usages referenced (the blocks around where useLicense()
is read and TeamGate is rendered).
In `@console/src/ui/viewer/views/Teams/TeamsSetupTab.tsx`:
- Around line 66-71: In TeamsSetupTab (the placeholder ternary using repoType),
replace legacy "vault" wording in the user-facing placeholders: change
"git@github.com:org/team-vault.git" to a non-vault name (e.g.,
"git@github.com:org/team-repo.git") and change "/path/to/vault" to a matching
local path (e.g., "/path/to/team-repo" or "/path/to/teams") so the placeholders
reflect the Teams rename and remain consistent with the UI copy.
In `@console/src/ui/viewer/views/Teams/TeamsSummaryCards.tsx`:
- Around line 49-54: The stat description in TeamsSummaryCards.tsx (the elements
with classNames stat-title, stat-value, stat-desc that display commands and
other via variables commands and other) can become misleading when other > 0;
update the rendering logic for the stat-desc so it conditionally matches the
displayed metric (e.g., when other > 0 show a description like "Slash commands
and other" or similar, otherwise keep "Slash commands") so the description
aligns with the title/value.
In `@console/tests/ui/teams-view.test.ts`:
- Line 116: Rename the test title string in teams-view.test.ts from "renders
install button for uninstalled vault assets" to "renders install button for
uninstalled team assets" so the test name matches the Teams terminology; locate
the it(...) call in the file (the test case starting with it("renders install
button for uninstalled vault assets", ...) and update only the descriptive
string, leaving the test body and assertions unchanged.
In `@README.md`:
- Line 315: The README still uses legacy "Vault" terminology and routes (e.g.,
occurrences of "/vault", "Vault" view, and the legacy file reference
`team-vault.md`) which should be migrated to the new canonical "teams" naming;
search for and replace all references of "/vault" with "/teams", rename mentions
of "Vault" view to "Teams" view (preserving context/capitalization), and update
or remove `team-vault.md` references to the new `teams` documentation or
filename so the docs consistently reflect the product naming.
---
Nitpick comments:
In `@console/src/ui/viewer/views/Teams/index.tsx`:
- Around line 12-19: The formatRepoUrl function is duplicated; extract it into a
shared helper (e.g., export function formatRepoUrl(url: string): string in a
new/util file) and replace the local definition in Teams view(s) with an import
of that helper; update the callers in Teams/index.tsx (and any other Teams view
files using formatRepoUrl) to import the new shared function and remove the
local implementation so URL formatting is centralized.
In `@console/tests/hooks/useSettings.test.ts`:
- Around line 47-53: The test currently checks only presence of six expected
commands and absence of 'vault', so update the assertion to verify the exact set
of keys in DEFAULT_SETTINGS.commands to prevent accidental additions; in the
test for DEFAULT_SETTINGS (in useSettings.test.ts) import DEFAULT_SETTINGS and
replace the per-key checks with a single assertion that
Object.keys(DEFAULT_SETTINGS.commands) (sorted) strictly equals the expected
array (sorted) — referencing DEFAULT_SETTINGS and its commands property to
locate the code to change.
In `@console/tests/ui/teams-navigation.test.ts`:
- Around line 11-49: Replace brittle fs string-search assertions with direct
module imports and assertions against the exported route/nav/command structures:
import the routes export from the module that defines App.tsx routes (e.g., the
exported routes array or a getRoutes function) and assert there is an entry with
component/name "TeamsView" and path "/teams"; import the nav items export from
SidebarNav (e.g., navItems or sidebarConfig) and assert an item with label
"Teams" has icon "users" and href or to "/teams"; import the commands export
from CommandPalette (e.g., commands or paletteItems) and assert a command
labeled "Go to Teams" exists with keybinding "G V" and a navigate handler or
route pointing to "/teams". Ensure tests use the exported symbols (routes,
navItems, commands) rather than reading file text so wiring is verified at
runtime.
In `@console/tests/worker/teams-routes.test.ts`:
- Around line 168-190: The test calls TeamsRoutes.setupRoutes then invokes the
discover handler and uses a sleep to wait; instead, mock external FS/Git
interactions used by TeamsRoutes (e.g., any fs, git, or repo helpers it depends
on) using jest mocks/spies and call the route handler as an async function so
you can await it directly; locate the route via TeamsRoutes.setupRoutes and the
discoverHandler reference, provide a fakeReq/fakeRes that returns a Promise from
json/status (or make the handler return a Promise) and then await
discoverHandler(fakeReq, fakeRes) instead of using setTimeout to make the test
deterministic and avoid touching the real filesystem or git.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (9)
docs/site/api/events/ingest.tsis excluded by!docs/site/api/**docs/site/api/portal/session.tsis excluded by!docs/site/api/**docs/site/api/trial/heartbeat.tsis excluded by!docs/site/api/**docs/site/api/trial/start.tsis excluded by!docs/site/api/**pilot/rules/team-vault.mdis excluded by!pilot/**pilot/scripts/worker-service.cjsis excluded by!pilot/**pilot/settings.jsonis excluded by!pilot/**pilot/ui/viewer-bundle.jsis excluded by!pilot/**pilot/ui/viewer.cssis excluded by!pilot/**
📒 Files selected for processing (38)
README.mdconsole/src/services/worker-service.tsconsole/src/services/worker/http/routes/SettingsRoutes.tsconsole/src/services/worker/http/routes/TeamsRoutes.tsconsole/src/services/worker/http/routes/VaultRoutes.tsconsole/src/ui/viewer/App.tsxconsole/src/ui/viewer/components/CommandPalette.tsxconsole/src/ui/viewer/components/TeamGate.tsxconsole/src/ui/viewer/constants/shortcuts.tsconsole/src/ui/viewer/hooks/useSettings.tsconsole/src/ui/viewer/hooks/useStats.tsconsole/src/ui/viewer/hooks/useTeams.tsconsole/src/ui/viewer/layouts/Sidebar/SidebarNav.tsxconsole/src/ui/viewer/views/Dashboard/TeamsStatus.tsxconsole/src/ui/viewer/views/Dashboard/index.tsxconsole/src/ui/viewer/views/Teams/TeamsAssetDetail.tsxconsole/src/ui/viewer/views/Teams/TeamsAssetTable.tsxconsole/src/ui/viewer/views/Teams/TeamsPushPanel.tsxconsole/src/ui/viewer/views/Teams/TeamsSetupTab.tsxconsole/src/ui/viewer/views/Teams/TeamsSummaryCards.tsxconsole/src/ui/viewer/views/Teams/index.tsxconsole/src/ui/viewer/views/Vault/index.tsxconsole/src/ui/viewer/views/index.tsconsole/tests/hooks/use-teams.test.tsconsole/tests/hooks/useSettings.test.tsconsole/tests/settings-routes.test.tsconsole/tests/ui/SidebarNav.test.tsconsole/tests/ui/search-removal.test.tsconsole/tests/ui/teams-install.test.tsconsole/tests/ui/teams-navigation.test.tsconsole/tests/ui/teams-view.test.tsconsole/tests/ui/vault-install.test.tsconsole/tests/ui/vault-navigation.test.tsconsole/tests/ui/vault-view.test.tsconsole/tests/ui/views-index.test.tsconsole/tests/worker/teams-routes.test.tsdocs/site/src/pages/docs/TeamsSection.tsxinstaller/steps/finalize.py
💤 Files with no reviewable changes (9)
- console/src/ui/viewer/hooks/useSettings.ts
- console/tests/ui/vault-install.test.ts
- console/tests/settings-routes.test.ts
- console/src/services/worker/http/routes/SettingsRoutes.ts
- console/tests/ui/vault-view.test.ts
- console/tests/ui/vault-navigation.test.ts
- console/src/services/worker/http/routes/VaultRoutes.ts
- console/src/ui/viewer/views/Vault/index.tsx
- installer/steps/finalize.py
Comment on lines
+282
to
+290
| const projectRoot = process.env.CLAUDE_PROJECT_ROOT || process.cwd(); | ||
| const resolvedSource = path.resolve(projectRoot, source); | ||
| if ( | ||
| resolvedSource !== projectRoot && | ||
| !resolvedSource.startsWith(projectRoot + path.sep) | ||
| ) { | ||
| res.status(400).json({ error: "Source path must be within project" }); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Source-path validation is vulnerable to symlink escape.
Prefix checks on path.resolve(...) do not prevent paths that traverse outside the project via symlinks.
🔒 Suggested hardening
+import { readdirSync, existsSync, realpathSync } from "fs";
...
const projectRoot = process.env.CLAUDE_PROJECT_ROOT || process.cwd();
const resolvedSource = path.resolve(projectRoot, source);
+ let realProjectRoot: string;
+ let realSource: string;
+ try {
+ realProjectRoot = realpathSync(projectRoot);
+ realSource = realpathSync(resolvedSource);
+ } catch {
+ res.status(400).json({ error: "Source path is invalid or does not exist" });
+ return;
+ }
if (
- resolvedSource !== projectRoot &&
- !resolvedSource.startsWith(projectRoot + path.sep)
+ realSource !== realProjectRoot &&
+ !realSource.startsWith(realProjectRoot + path.sep)
) {
res.status(400).json({ error: "Source path must be within project" });
return;
}📝 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
| const projectRoot = process.env.CLAUDE_PROJECT_ROOT || process.cwd(); | |
| const resolvedSource = path.resolve(projectRoot, source); | |
| if ( | |
| resolvedSource !== projectRoot && | |
| !resolvedSource.startsWith(projectRoot + path.sep) | |
| ) { | |
| res.status(400).json({ error: "Source path must be within project" }); | |
| return; | |
| } | |
| const projectRoot = process.env.CLAUDE_PROJECT_ROOT || process.cwd(); | |
| const resolvedSource = path.resolve(projectRoot, source); | |
| let realProjectRoot: string; | |
| let realSource: string; | |
| try { | |
| realProjectRoot = realpathSync(projectRoot); | |
| realSource = realpathSync(resolvedSource); | |
| } catch { | |
| res.status(400).json({ error: "Source path is invalid or does not exist" }); | |
| return; | |
| } | |
| if ( | |
| realSource !== realProjectRoot && | |
| !realSource.startsWith(realProjectRoot + path.sep) | |
| ) { | |
| res.status(400).json({ error: "Source path must be within project" }); | |
| return; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@console/src/services/worker/http/routes/TeamsRoutes.ts` around lines 282 -
290, The current prefix check on resolvedSource can be bypassed via symlinks;
change validation to compare real filesystem paths: call fs.promises.realpath
(or fs.realpathSync) for projectRoot and resolvedSource (referencing projectRoot
and resolvedSource variables and path.resolve usage) and verify the
resolvedSourceRealpath equals projectRootRealpath or
startsWith(projectRootRealpath + path.sep) before accepting; return the same 400
error on failure. Ensure to handle realpath errors (treat as invalid) and
preserve use of process.env.CLAUDE_PROJECT_ROOT when computing
projectRootRealpath.
Comment on lines
+320
to
+324
| } catch (error) { | ||
| const message = (error as Error).message || "Push failed"; | ||
| logger.error("HTTP", "Teams push failed", { name }, error as Error); | ||
| res.json({ success: false, error: message }); | ||
| } |
There was a problem hiding this comment.
Failures are returned as HTTP 200 on mutating endpoints.
Returning 200 for operational failures makes monitoring/retries and client handling ambiguous. Use non-2xx status codes for failed mutations.
🔧 Suggested response semantics
- res.json({ success: false, error: message });
+ res.status(502).json({ success: false, error: message });Apply the same pattern in handleRemove, handleInit, and handleUpdateAsset.
Also applies to: 355-358, 394-397, 503-512
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@console/src/services/worker/http/routes/TeamsRoutes.ts` around lines 320 -
324, The catch block in the Teams push handler currently returns a 200 OK with
an error payload; update the error handling to send an appropriate non-2xx HTTP
status (e.g., res.status(500)) before returning the JSON error, and include the
same message and logger.error usage (see the catch that defines const message =
(error as Error).message and calls logger.error("HTTP", "Teams push failed", {
name }, error as Error)); apply the identical change to the catch blocks in
handleRemove, handleInit, and handleUpdateAsset so failures return a non-2xx
status with the same error JSON and logging behavior.
Comment on lines
+457
to
+468
| private handleUpdateAsset = this.wrapHandler( | ||
| async (req: Request, res: Response): Promise<void> => { | ||
| const { name, currentVersion, scope, scopeUrl } = req.body; | ||
| if (!name || !NAME_REGEX.test(name)) { | ||
| res.status(400).json({ error: "Invalid asset name" }); | ||
| return; | ||
| } | ||
| const sxPath = this.resolveSxBinary(); | ||
| if (!sxPath) { | ||
| res.status(500).json({ error: "sx CLI not found" }); | ||
| return; | ||
| } |
There was a problem hiding this comment.
update-asset is not synchronized with ongoing installs.
This endpoint performs lockfile mutation plus install but does not check/set _isInstalling, so concurrent install/update requests can race and corrupt state.
🧵 Suggested synchronization fix
private handleUpdateAsset = this.wrapHandler(
async (req: Request, res: Response): Promise<void> => {
+ if (this._isInstalling) {
+ res.status(409).json({ error: "Installation already in progress" });
+ return;
+ }
+ this._isInstalling = true;
+ this.statusCache = null;
+
const { name, currentVersion, scope, scopeUrl } = req.body;
if (!name || !NAME_REGEX.test(name)) {
res.status(400).json({ error: "Invalid asset name" });
+ this._isInstalling = false;
return;
}
...
try {
// Remove old version from lock file
...
await this.runSxCommand(
[sxPath, "install", "--repair", "--target", projectRoot],
INSTALL_TIMEOUT_MS,
);
this.statusCache = null;
this.detailCache.clear();
res.json({ success: true, error: null });
} catch (error) {
...
res.json({ success: false, error: message });
+ } finally {
+ this._isInstalling = false;
}
},
);Also applies to: 495-499
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@console/src/services/worker/http/routes/TeamsRoutes.ts` around lines 457 -
468, The update-asset handler (handleUpdateAsset) mutates the lockfile and runs
installs without synchronizing with concurrent installs; guard the critical
section by checking and setting the shared flag _isInstalling: at the start of
the handler, if (this._isInstalling) return a 409/conflict response, otherwise
set this._isInstalling = true before performing lockfile mutation and the
install, and ensure you clear it in a finally block so it’s always reset; apply
the same pattern to the other install/update handler referenced around lines
495-499 (the other route handler that mutates the lockfile) so both use the same
check/set/try...finally/unset sequence and avoid race conditions.
Comment on lines
+470
to
+494
| // Remove old version from lock file | ||
| if (currentVersion) { | ||
| await this.runSxCommand( | ||
| [ | ||
| sxPath, | ||
| "remove", | ||
| name, | ||
| "--version", | ||
| String(currentVersion), | ||
| "--yes", | ||
| ], | ||
| REMOVE_TIMEOUT_MS, | ||
| ); | ||
| } | ||
| // Re-add with scope (picks up latest version) | ||
| const addArgs = [sxPath, "add", name, "--yes"]; | ||
| if (scope === "global") { | ||
| addArgs.push("--scope-global"); | ||
| } else if (scopeUrl) { | ||
| addArgs.push("--scope-repo", scopeUrl); | ||
| } else { | ||
| addArgs.push("--scope-global"); | ||
| } | ||
| await this.runSxCommand(addArgs, PUSH_TIMEOUT_MS); | ||
| // Install the updated version |
There was a problem hiding this comment.
Remove-first update flow can leave the asset missing on partial failure.
If remove succeeds and add or install fails, the lockfile is left in a degraded state. Add a compensating rollback or use an update sequence that commits only after success.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@console/src/services/worker/http/routes/TeamsRoutes.ts` around lines 470 -
494, The current remove-then-add sequence can leave the lockfile degraded if
add/install fails; change the flow in the method that calls runSxCommand so you
add the new asset before removing the old (build addArgs and call
runSxCommand(addArgs, PUSH_TIMEOUT_MS) first), and only on successful add call
runSxCommand([...sxPath, "remove", name, "--version", String(currentVersion),
"--yes"], REMOVE_TIMEOUT_MS) to drop the old entry; additionally wrap both
operations in try/catch and on any failure attempt a compensating rollback by
calling runSxCommand([...sxPath, "add", name, "--version",
String(currentVersion), "--yes"], PUSH_TIMEOUT_MS) to restore the previous
version (use currentVersion, addArgs, runSxCommand, REMOVE_TIMEOUT_MS,
PUSH_TIMEOUT_MS to locate code).
Comment on lines
+16
to
+18
| <div className="opacity-30 pointer-events-none blur-[2px] select-none"> | ||
| {children} | ||
| </div> |
There was a problem hiding this comment.
Locked content remains keyboard-focusable behind the overlay.
pointer-events-none only blocks pointer interaction. Underlying focusable controls can still be reached via keyboard/assistive tech.
🔧 Suggested hardening
- <div className="opacity-30 pointer-events-none blur-[2px] select-none">
+ <div
+ inert
+ aria-hidden="true"
+ className="opacity-30 pointer-events-none blur-[2px] select-none"
+ >
{children}
</div>📝 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
| <div className="opacity-30 pointer-events-none blur-[2px] select-none"> | |
| {children} | |
| </div> | |
| <div | |
| inert | |
| aria-hidden="true" | |
| className="opacity-30 pointer-events-none blur-[2px] select-none" | |
| > | |
| {children} | |
| </div> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@console/src/ui/viewer/components/TeamGate.tsx` around lines 16 - 18, The
overlay wrapper currently uses pointer-events-none which doesn't prevent
keyboard focus; update the wrapper div in TeamGate (the element rendering
{children}) to remove keyboard access by adding accessibility attributes: set
aria-hidden="true" and add the inert attribute (and fallback to tabIndex={-1} if
inert not supported) on that div so underlying interactive controls are excluded
from the accessibility tree and cannot receive keyboard focus; ensure the same
element keeps the visual classes (opacity-30 blur-[2px] select-none) while
adding these attributes.
Comment on lines
166
to
179
| it("source contains API endpoint fetch logic", async () => { | ||
| const fs = await import("fs"); | ||
| const source = fs.readFileSync("src/ui/viewer/hooks/useVault.ts", "utf-8"); | ||
| const source = fs.readFileSync("src/ui/viewer/hooks/useTeams.ts", "utf-8"); | ||
|
|
||
| expect(source).toContain("/api/vault/status"); | ||
| expect(source).toContain("/api/vault/detail/"); | ||
| expect(source).toContain("/api/vault/install"); | ||
| expect(source).toContain("/api/teams/status"); | ||
| expect(source).toContain("/api/teams/detail/"); | ||
| expect(source).toContain("/api/teams/install"); | ||
| expect(source).toContain("isInstalling"); | ||
| expect(source).toContain("mergeAssets"); | ||
| expect(source).toContain("/api/teams/remove"); | ||
| expect(source).toContain("removeAsset"); | ||
| expect(source).toContain("/api/teams/init"); | ||
| expect(source).toContain("initTeams"); | ||
| }); |
There was a problem hiding this comment.
Replace source-string assertions with behavioral fetch-mocked tests for Teams mutations.
This block can pass even when runtime behavior regresses (wrong method/body, missing refresh, wrong endpoint usage). For this PR, add behavior tests around discover, pushAsset, removeAsset, initTeams, and updateAsset using mocked fetch.
As per coding guidelines, "/tests/: Review test code briefly. Focus on: Test coverage for the feature being tested; Proper mocking of external dependencies; Clear test names that describe behavior**."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@console/tests/hooks/use-teams.test.ts` around lines 166 - 179, Replace the
brittle source-string assertions in the test with behavioral tests that mock
global.fetch and exercise the hook's exported actions (discover, pushAsset,
removeAsset, initTeams, updateAsset) from src/ui/viewer/hooks/useTeams.ts: for
each action, mock fetch responses and assert the correct endpoint, HTTP method,
and request body were used, then verify the hook's state/mutation calls (e.g.,
that isInstalling toggles, mergeAssets is called with returned assets,
removeAsset/updateAsset cause expected state changes, and initTeams triggers the
init endpoint and subsequent refresh); use jest.fn() or a fetch-mock and
clear/reset mocks between tests to ensure isolation and meaningful failure
signals.
Comment on lines
+4
to
+71
| it('TeamsSyncButton component exists and renders correctly', async () => { | ||
| const fs = await import('fs/promises'); | ||
| const teamsViewContent = await fs.readFile( | ||
| 'src/ui/viewer/views/Teams/index.tsx', | ||
| 'utf-8' | ||
| ); | ||
|
|
||
| expect(teamsViewContent).toContain('function TeamsSyncButton'); | ||
|
|
||
| expect(teamsViewContent).toMatch(/className="btn btn-primary btn-sm"/); | ||
|
|
||
| expect(teamsViewContent).toContain('loading loading-spinner'); | ||
| expect(teamsViewContent).toContain('Syncing...'); | ||
|
|
||
| expect(teamsViewContent).toContain('Sync All'); | ||
| }); | ||
|
|
||
| it('TeamsView uses useToast for notifications', async () => { | ||
| const fs = await import('fs/promises'); | ||
| const teamsViewContent = await fs.readFile( | ||
| 'src/ui/viewer/views/Teams/index.tsx', | ||
| 'utf-8' | ||
| ); | ||
|
|
||
| expect(teamsViewContent).toMatch(/import.*useToast.*from.*ToastContext/); | ||
|
|
||
| expect(teamsViewContent).toMatch(/useToast\(\)/); | ||
| }); | ||
|
|
||
| it('handles successful install with toast notification', async () => { | ||
| const fs = await import('fs/promises'); | ||
| const teamsViewContent = await fs.readFile( | ||
| 'src/ui/viewer/views/Teams/index.tsx', | ||
| 'utf-8' | ||
| ); | ||
|
|
||
| expect(teamsViewContent).toMatch(/\.success\(['"].*[Tt]eams.*sync/i); | ||
| }); | ||
|
|
||
| it('handles install error with toast notification', async () => { | ||
| const fs = await import('fs/promises'); | ||
| const teamsViewContent = await fs.readFile( | ||
| 'src/ui/viewer/views/Teams/index.tsx', | ||
| 'utf-8' | ||
| ); | ||
|
|
||
| expect(teamsViewContent).toMatch(/\.error\(/); | ||
| }); | ||
|
|
||
| it('handles install timeout with warning toast', async () => { | ||
| const fs = await import('fs/promises'); | ||
| const teamsViewContent = await fs.readFile( | ||
| 'src/ui/viewer/views/Teams/index.tsx', | ||
| 'utf-8' | ||
| ); | ||
|
|
||
| expect(teamsViewContent).toMatch(/\.warning\(/); | ||
| }); | ||
|
|
||
| it('handleInstall wrapper function exists', async () => { | ||
| const fs = await import('fs/promises'); | ||
| const teamsViewContent = await fs.readFile( | ||
| 'src/ui/viewer/views/Teams/index.tsx', | ||
| 'utf-8' | ||
| ); | ||
|
|
||
| expect(teamsViewContent).toMatch(/handle(Install|Sync)/); | ||
| }); |
There was a problem hiding this comment.
These tests validate source text, not install behavior.
All assertions are string/regex checks on file content, so they can still pass if sync handlers or toast wiring break at runtime. Add at least one behavior-level test that renders Teams view, triggers sync, and asserts success/error/timeout toast calls.
As per coding guidelines, "Review test code briefly. Focus on: Test coverage for the feature being tested".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@console/tests/ui/teams-install.test.ts` around lines 4 - 71, The current
tests only assert source text; add a runtime behavior test in
console/tests/ui/teams-install.test.ts that renders the Teams view (importing
the component from src/ui/viewer/views/Teams/index.tsx), mocks the sync API
and/or the internal sync handler (e.g., handleInstall or TeamsSyncButton click),
simulates a user click on the "Sync All" button, and asserts that the useToast
methods (.success, .error, .warning) are invoked for success, error, and timeout
scenarios; use jest.fn() or your existing test helpers to spy on the toast
returned by useToast() and stub network calls to drive each outcome.
console/tests/ui/teams-view.test.ts
Outdated
Comment on lines
+87
to
+143
| it("renders action buttons for installed assets with update", async () => { | ||
| const { TeamsAssetTable } = await import("../../src/ui/viewer/views/Teams/TeamsAssetTable.js"); | ||
|
|
||
| const assets = [ | ||
| { name: "my-skill", type: "skill", latestVersion: "3", versionsCount: 3, installedVersion: "v2", installed: true, hasUpdate: true, scope: "Global", clients: [] }, | ||
| ]; | ||
|
|
||
| const html = renderToString( | ||
| React.createElement(TeamsAssetTable, { | ||
| assets, | ||
| searchQuery: "", | ||
| activeTab: "all", | ||
| onTabChange: () => {}, | ||
| onSearchChange: () => {}, | ||
| expandedAsset: null, | ||
| onAssetClick: () => {}, | ||
| fetchDetail: () => Promise.resolve(), | ||
| detailCache: new Map(), | ||
| loadingDetails: new Set<string>(), | ||
| tier: "team", | ||
| onUpdate: () => {}, | ||
| onRemove: () => Promise.resolve(), | ||
| }), | ||
| ); | ||
|
|
||
| expect(html).toContain("Update"); | ||
| expect(html).toContain("Remove"); | ||
| }); | ||
|
|
||
| it("renders install button for uninstalled vault assets", async () => { | ||
| const { TeamsAssetTable } = await import("../../src/ui/viewer/views/Teams/TeamsAssetTable.js"); | ||
|
|
||
| const assets = [ | ||
| { name: "my-rule", type: "rule", latestVersion: "1", versionsCount: 1, installedVersion: null, installed: false, hasUpdate: false, scope: null, clients: [] }, | ||
| ]; | ||
|
|
||
| const html = renderToString( | ||
| React.createElement(TeamsAssetTable, { | ||
| assets, | ||
| searchQuery: "", | ||
| activeTab: "all", | ||
| onTabChange: () => {}, | ||
| onSearchChange: () => {}, | ||
| expandedAsset: null, | ||
| onAssetClick: () => {}, | ||
| fetchDetail: () => Promise.resolve(), | ||
| detailCache: new Map(), | ||
| loadingDetails: new Set<string>(), | ||
| tier: "solo", | ||
| onUpdate: () => {}, | ||
| onRemove: () => Promise.resolve(), | ||
| }), | ||
| ); | ||
|
|
||
| expect(html).toContain("Install"); | ||
| expect(html).not.toContain("Remove"); | ||
| }); |
There was a problem hiding this comment.
CRUD action coverage is incomplete for TeamsAssetTable.
These tests only check button labels in SSR output. Please add interaction-level tests that verify onUpdate/onRemove are invoked correctly, including the remove confirmation path.
As per coding guidelines, "Review test code briefly. Focus on: Test coverage for the feature being tested".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@console/tests/ui/teams-view.test.ts` around lines 87 - 143, Add interaction
tests for TeamsAssetTable that mount the component (instead of only SSR render)
and assert the onUpdate and onRemove callbacks are invoked through user
interactions: locate and click the "Update" button and verify onUpdate was
called; locate and click the "Remove" button, drive the confirmation dialog flow
(click confirm) and verify onRemove was called; and for uninstalled vault assets
locate and click "Install" and verify appropriate install handler or onUpdate is
invoked. Use the same props pattern from the existing tests, pass jest.fn()
spies for onUpdate/onRemove, render TeamsAssetTable (imported from
TeamsAssetTable.js) with tier "team" and "solo" scenarios, simulate clicks with
a DOM testing utility (e.g., `@testing-library/react` + userEvent), and assert the
spies are called and the remove-confirmation path triggers onRemove only after
confirm.
Comment on lines
+19
to
+37
| it("registers all 8 routes", () => { | ||
| const routes = new TeamsRoutes(); | ||
| const registered: { method: string; path: string }[] = []; | ||
| const fakeApp = { | ||
| get: (path: string, _handler: any) => registered.push({ method: "GET", path }), | ||
| post: (path: string, _handler: any) => registered.push({ method: "POST", path }), | ||
| }; | ||
| routes.setupRoutes(fakeApp as any); | ||
|
|
||
| expect(registered).toContainEqual({ method: "GET", path: "/api/vault/status" }); | ||
| expect(registered).toContainEqual({ method: "POST", path: "/api/vault/install" }); | ||
| expect(registered).toContainEqual({ method: "GET", path: "/api/vault/detail/:name" }); | ||
| expect(registered).toContainEqual({ method: "GET", path: "/api/teams/status" }); | ||
| expect(registered).toContainEqual({ method: "POST", path: "/api/teams/install" }); | ||
| expect(registered).toContainEqual({ method: "GET", path: "/api/teams/detail/:name" }); | ||
| expect(registered).toContainEqual({ method: "POST", path: "/api/teams/push" }); | ||
| expect(registered).toContainEqual({ method: "POST", path: "/api/teams/remove" }); | ||
| expect(registered).toContainEqual({ method: "POST", path: "/api/teams/init" }); | ||
| expect(registered).toContainEqual({ method: "GET", path: "/api/teams/discover" }); | ||
| expect(registered).toContainEqual({ method: "POST", path: "/api/teams/update-asset" }); | ||
| expect(registered.length).toBe(8); | ||
| }); |
There was a problem hiding this comment.
/api/teams/update-asset is only registration-tested, not behavior-tested.
Given it mutates lockfile/install state, add endpoint behavior tests (validation, command sequencing, and failure-path expectations).
As per coding guidelines, "**/tests/**: Review test code briefly. Focus on: Test coverage for the feature being tested".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@console/tests/worker/teams-routes.test.ts` around lines 19 - 37, Add behavior
tests for the POST /api/teams/update-asset route (in addition to the existing
registration test) that exercise validation, command sequencing, and failure
paths: instantiate TeamsRoutes, capture the registered POST handler for
"/api/teams/update-asset" (same pattern used to capture registrations), then
call that handler directly with mocked req/res objects; add separate tests to
assert (1) invalid input returns the expected 4xx validation response, (2) valid
input triggers the expected sequence of operations by mocking and spying the
lockfile/installer services (assert call order and payloads), and (3) when a
mocked dependency throws, the handler returns the correct error status and
logs/response; use the same registered capture mechanism and mock services used
elsewhere in this test file to keep tests focused and isolated.
# Conflicts: # pilot/scripts/worker-service.cjs
…ved asset management - Replace Sync All button with background auto-sync on page load - Add Teams onboarding screen for first-time sx setup - Add Asset Types help modal with format reference - Add project slug prefix convention for /sync and /learn commands - Add modified indicator (git diff) and Push New Version action - Improve sx error messages with parseSxError - Add force refresh param to status endpoint - Gate all actions behind team/trial tier - Replace green checkmark with styled check badge for on-disk assets - Remove rename feature (sx lacks native support) - Remove /vault command (replaced by Teams dashboard) - Remove hooks and MCP from Teams (focus on rules, skills, commands, agents) - Filter out sx metadata .toml files from discover endpoint - Disable non-claude-code sx clients in installer - Add GIT_OPTIONAL_LOCKS=0 to all console git operations - Add distinct badge colors per asset type - Update all /vault references across docs, README, and blog posts
…top guard and plan questions
From stash@{0} (WIP on dev: 6a18f80):
- Remove legacy 'standard'/'enterprise' tier names from installer, formatter, tests
- Simplify tier checks in statusline formatter (== instead of 'in' tuple)
- Remove 'vault' model config row from Settings page
- Add git-crypt for console directory in .gitattributes
- Update PricingSection with Teams Dashboard features
- Auto-merged launcher auth, banner, cli, trial, widgets changes
- Add Teams row to README commands table
- Fix duplicate .gitattributes entry
- Clean up stray conflict marker in sync.md
…ms and pricing consistency
maxritter
changed the title
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (6)
.coderabbit.yaml (1)
23-26: Comment no longer accurately describes all exclusions.The comment on line 24 states these filters are for "git-crypt encrypted directories," but
console/**is the active Teams UI codebase (with 1033+ passing tests per PR description), not an encrypted directory. Consider updating the comment to reflect the actual reason for excluding console, or splitting the filters with separate comments.📝 Suggested comment update
path_filters: - # Exclude git-crypt encrypted directories from reviews (see .gitattributes) + # Exclude git-crypt encrypted directories (see .gitattributes) and console UI - "!launcher/**" - "!console/**"Alternatively, group with separate comments:
path_filters: # Exclude git-crypt encrypted directories from reviews (see .gitattributes) - "!launcher/**" + # Exclude console UI (reviewed separately or via other tooling) + - "!console/**" - "!pilot/**"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.coderabbit.yaml around lines 23 - 26, Update the inaccurate comment above the path_filters entry: change or split the comment that currently says "Exclude git-crypt encrypted directories from reviews (see .gitattributes)" so it no longer implies console/** is encrypted; either revise the single comment to reflect the actual intent (e.g., specify launcher/** is git-crypt encrypted and console/** is the active Teams UI excluded for another reason) or add two separate comments immediately above the respective filters referencing "!launcher/**" and "!console/**" to clarify each exclusion's purpose.docs/site/src/components/WorkflowSteps.tsx (1)
195-204: Consider whether Teams belongs in "All Commands" section.Teams is no longer a slash command (as
/vaultwas removed), yet it's listed under "All Commands". Additionally, the label uses<span>while other items use<code>, creating visual inconsistency.Options to consider:
- Rename the section to something like "Features" or "Tools" to encompass both commands and navigation destinations.
- Keep it as-is if the intent is to show all major entry points regardless of invocation method.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/site/src/components/WorkflowSteps.tsx` around lines 195 - 204, Teams is rendered as a command in WorkflowSteps.tsx but is not a slash command and also uses a <span> for the label while other items use <code>, causing semantic and visual inconsistency; update the component by either moving this block out of the "All Commands" group into a more generic "Features" or "Tools" section (or rename the section) or keep it but make its presentation consistent by replacing the <span className="text-sm font-medium text-primary">Teams</span> with the same <code> element used by other entries so visuals match; ensure you update any section title text (e.g., "All Commands") and surrounding grouping logic in WorkflowSteps to reflect the chosen approach.installer/ui.py (1)
163-165: Minor: Redundant fallback intier_map.get().Since line 163 guarantees
tier in ("solo", "team"), the fallbacktier.title()in line 165 will never execute—the key is always present intier_map. You can simplify to a direct lookup.♻️ Suggested simplification
if tier in ("solo", "team"): tier_map = {"solo": "Solo", "team": "Team"} - tier_display = tier_map.get(tier, tier.title()) + tier_display = tier_map[tier]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@installer/ui.py` around lines 163 - 165, The code redundantly uses tier_map.get(tier, tier.title()) even though the preceding conditional guarantees tier in ("solo", "team"); change the lookup to a direct dictionary index (e.g., tier_map[tier]) when assigning tier_display and remove the unreachable fallback expression so tier_map and tier_display use a simple deterministic lookup for the tier variable.README.md (1)
49-51: Consider removing hard-coded third-party plan multipliers from prerequisites text.The exact usage multipliers and plan wording here are likely to drift quickly and become stale. Prefer linking to the pricing page without embedding detailed numeric claims.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 49 - 51, Remove the hard-coded numeric plan multipliers and detailed pricing claims under the "Claude Subscription" section and replace that text with a brief, generic recommendation directing users to the official pricing page; update the "Claude Subscription" paragraph (and mention of "Claude Code" install note if needed) to link to https://claude.com/pricing and advise choosing a plan based on expected usage (solo, team, enterprise) without asserting specific multipliers or exact plan names..github/workflows/release.yml (1)
159-166: Consider centralizing git-crypt bootstrap across jobs.The same install+unlock sequence is repeated in multiple jobs; extracting this into a reusable composite action (or YAML anchor) will reduce drift and make security updates easier.
Also applies to: 191-198
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yml around lines 159 - 166, The install+unlock steps for git-crypt are duplicated (steps named "Install git-crypt" and "Unlock repository" that run apt-get and bash .github/workflows/scripts/setup-git-crypt.sh); extract these into a single reusable unit (either a composite action under .github/actions or a YAML anchor/extension) and replace the repeated blocks in each job (including the blocks at the current location and lines ~191-198) with a call to that composite action or anchor to centralize installation and unlocking logic, pass GIT_CRYPT_KEY as an input/env to the composite action, and update callers to use the new action name (e.g., "git-crypt-bootstrap") so future updates occur in one place.docs/site/src/pages/docs/TeamsSection.tsx (1)
6-7: Include “Update” in the feature summary to reflect full CRUD actions.Line 7 currently skips update, while this PR introduces Install/Update/Remove actions.
Suggested copy tweak
- desc: "Browse, push, install, and remove assets from a visual Teams page", + desc: "Browse, push, install, update, and remove assets from a visual Teams page",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/site/src/pages/docs/TeamsSection.tsx` around lines 6 - 7, The feature summary string (desc) in TeamsSection.tsx omits "Update" even though the PR adds Install/Update/Remove actions; update the desc value (the "desc" property) to include "Update" so it reads something like "Browse, push, install, update, and remove assets from a visual Teams page" to accurately reflect CRUD actions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/release-dev.yml:
- Line 41: The workflow currently excludes the console directory from Trivy
scans by listing "console" in the skip-dirs setting (skip-dirs:
".venv,node_modules,console/node_modules,launcher,docs/site/api,console"), which
contradicts the git-crypt decryption and the CI security-scan intent; either
remove "console" from the skip-dirs value in the release-dev.yml so Trivy scans
the decrypted console content, or keep the exclusion but add a clear
justification in .trivyignore (referencing the security-scan job and git-crypt
decryption) explaining why console is an accepted exclusion (e.g., documented
false positives or accepted risk).
In @.github/workflows/release.yml:
- Line 91: The workflow currently excludes the console directory via the
skip-dirs setting (skip-dirs:
'.venv,node_modules,console/node_modules,launcher,docs/site/api,console'),
causing Trivy to skip scanning decrypted console sources; update the skip-dirs
value to remove the standalone "console" entry (keep console/node_modules if
desired) so Trivy will scan the console code after it is decrypted (ensure the
existing console decryption step runs before Trivy), and verify .trivyignore
still documents the intended behavior.
In @.gitignore:
- Around line 231-232: The .gitignore entry '.env**' is too broad and hides
committed env templates; replace it with explicit ignore patterns (e.g., '.env',
'.env.local', '.env.*.local' or other runtime/private files you intend to
exclude) and add explicit allowlist entries for committed templates such as
'.env.example' and '.env.template' so those files remain tracked; update the
'.env**' entry in the .gitignore to the narrower patterns and add allowlist
lines for the example/template filenames.
In `@docs/site/src/components/InstallSection.tsx`:
- Around line 101-103: Fix the comma splice and missing object in the JSX
sentence inside InstallSection (the fragment containing "extracts knowledge, the
<strong>Teams dashboard</strong> shares with your team."): replace it with a
clear phrasing such as "extracts knowledge that the Teams dashboard shares with
your team." so the sentence reads smoothly and specifies what is being shared;
update the string in the InstallSection.tsx JSX where that fragment appears.
In `@docs/site/src/content/blog/team-vault-sharing-ai-assets.md`:
- Line 4: Frontmatter "description" contains an invalid YAML escape sequence
`developer\'s`; update the description value (the "description" frontmatter
field in team-vault-sharing-ai-assets.md) to remove the backslash so it reads
developer's (or alternatively switch to a single-quoted YAML string and escape
as needed), ensuring the double-quoted string contains a plain apostrophe
without `\`.
In `@docs/site/src/pages/docs/PrerequisitesSection.tsx`:
- Around line 60-83: The inline <code> element in PrerequisitesSection.tsx can
overflow on narrow screens; update the <code className="..."> for the npm
command token (the code element inside Claude Code paragraph) to enable wrapping
and prevent clipping by adding Tailwind utilities such as break-words (or
break-all) and whitespace-normal/whitespace-pre-wrap and/or display inline-block
(e.g., add "break-words whitespace-normal inline-block" or "break-all
whitespace-normal") so the long token wraps instead of causing horizontal
overflow.
In `@docs/site/src/pages/docs/TeamsSection.tsx`:
- Around line 64-65: Header copy in TeamsSection.tsx is inconsistent with the
listed shareable asset types; update the header string that currently reads
"Share rules, commands, and skills across your team via the Console dashboard"
to include "agents" (e.g., "Share agents, rules, commands, and skills across
your team via the Console dashboard") so the header matches the documented items
and locate the exact text in the TeamsSection component to replace it.
In `@installer/steps/dependencies.py`:
- Around line 413-415: The loop that disables clients inside install_sx
currently ignores _run_bash_with_retry results; modify install_sx so each call
to _run_bash_with_retry(f"sx clients disable {client}") checks the return value
and on failure logs a clear error referencing the client (e.g., "failed to
disable {client}") and returns False (or raises) so install_sx does not return
True on partial failure; use the existing logger (or raise RuntimeError) and
stop further processing when a disable fails to ensure proper user feedback and
error handling.
In `@README.md`:
- Around line 195-208: Update the "Guided onboarding" bullet under the "What
Teams includes" details: change the noun form "setup" to the verb phrase "set
up" so the sentence reads "New team members get a step-by-step screen to set
up"; locate the bullet that starts with "Guided onboarding — New team members
get a step-by-step screen to setup" and replace "setup" with "set up".
---
Nitpick comments:
In @.coderabbit.yaml:
- Around line 23-26: Update the inaccurate comment above the path_filters entry:
change or split the comment that currently says "Exclude git-crypt encrypted
directories from reviews (see .gitattributes)" so it no longer implies
console/** is encrypted; either revise the single comment to reflect the actual
intent (e.g., specify launcher/** is git-crypt encrypted and console/** is the
active Teams UI excluded for another reason) or add two separate comments
immediately above the respective filters referencing "!launcher/**" and
"!console/**" to clarify each exclusion's purpose.
In @.github/workflows/release.yml:
- Around line 159-166: The install+unlock steps for git-crypt are duplicated
(steps named "Install git-crypt" and "Unlock repository" that run apt-get and
bash .github/workflows/scripts/setup-git-crypt.sh); extract these into a single
reusable unit (either a composite action under .github/actions or a YAML
anchor/extension) and replace the repeated blocks in each job (including the
blocks at the current location and lines ~191-198) with a call to that composite
action or anchor to centralize installation and unlocking logic, pass
GIT_CRYPT_KEY as an input/env to the composite action, and update callers to use
the new action name (e.g., "git-crypt-bootstrap") so future updates occur in one
place.
In `@docs/site/src/components/WorkflowSteps.tsx`:
- Around line 195-204: Teams is rendered as a command in WorkflowSteps.tsx but
is not a slash command and also uses a <span> for the label while other items
use <code>, causing semantic and visual inconsistency; update the component by
either moving this block out of the "All Commands" group into a more generic
"Features" or "Tools" section (or rename the section) or keep it but make its
presentation consistent by replacing the <span className="text-sm font-medium
text-primary">Teams</span> with the same <code> element used by other entries so
visuals match; ensure you update any section title text (e.g., "All Commands")
and surrounding grouping logic in WorkflowSteps to reflect the chosen approach.
In `@docs/site/src/pages/docs/TeamsSection.tsx`:
- Around line 6-7: The feature summary string (desc) in TeamsSection.tsx omits
"Update" even though the PR adds Install/Update/Remove actions; update the desc
value (the "desc" property) to include "Update" so it reads something like
"Browse, push, install, update, and remove assets from a visual Teams page" to
accurately reflect CRUD actions.
In `@installer/ui.py`:
- Around line 163-165: The code redundantly uses tier_map.get(tier,
tier.title()) even though the preceding conditional guarantees tier in ("solo",
"team"); change the lookup to a direct dictionary index (e.g., tier_map[tier])
when assigning tier_display and remove the unreachable fallback expression so
tier_map and tier_display use a simple deterministic lookup for the tier
variable.
In `@README.md`:
- Around line 49-51: Remove the hard-coded numeric plan multipliers and detailed
pricing claims under the "Claude Subscription" section and replace that text
with a brief, generic recommendation directing users to the official pricing
page; update the "Claude Subscription" paragraph (and mention of "Claude Code"
install note if needed) to link to https://claude.com/pricing and advise
choosing a plan based on expected usage (solo, team, enterprise) without
asserting specific multipliers or exact plan names.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 976dc08b-abfa-4305-85db-19ed3f2b61fe
⛔ Files ignored due to path filters (42)
docs/img/dashboard.pngis excluded by!**/*.pngdocs/img/teams.pngis excluded by!**/*.pngdocs/site/public/console/dashboard.pngis excluded by!**/*.pngdocs/site/public/console/teams.pngis excluded by!**/*.pngdocs/site/public/console/vault.pngis excluded by!**/*.pnglauncher/auth.pyis excluded by!launcher/**launcher/banner.pyis excluded by!launcher/**launcher/build.pyis excluded by!launcher/**launcher/claude_installer.pyis excluded by!launcher/**launcher/cli.pyis excluded by!launcher/**launcher/model_config.pyis excluded by!launcher/**launcher/statusline/formatter.pyis excluded by!launcher/**launcher/statusline/widgets.pyis excluded by!launcher/**launcher/tests/unit/statusline/test_formatter.pyis excluded by!launcher/**launcher/tests/unit/statusline/test_widgets.pyis excluded by!launcher/**launcher/tests/unit/test_auth.pyis excluded by!launcher/**launcher/tests/unit/test_claude_installer.pyis excluded by!launcher/**launcher/tests/unit/test_model_config.pyis excluded by!launcher/**launcher/tests/unit/test_wrapper.pyis excluded by!launcher/**launcher/trial.pyis excluded by!launcher/**launcher/utils.pyis excluded by!launcher/**launcher/wrapper.pyis excluded by!launcher/**pilot/agents/spec-reviewer.mdis excluded by!pilot/**pilot/commands/learn.mdis excluded by!pilot/**pilot/commands/spec-bugfix-verify.mdis excluded by!pilot/**pilot/commands/spec-plan.mdis excluded by!pilot/**pilot/commands/spec-verify.mdis excluded by!pilot/**pilot/commands/sync.mdis excluded by!pilot/**pilot/commands/vault.mdis excluded by!pilot/**pilot/hooks/_util.pyis excluded by!pilot/**pilot/hooks/tests/test__util.pyis excluded by!pilot/**pilot/rules/development-practices.mdis excluded by!pilot/**pilot/rules/standards-frontend.mdis excluded by!pilot/**pilot/rules/standards-golang.mdis excluded by!pilot/**pilot/rules/standards-python.mdis excluded by!pilot/**pilot/rules/standards-typescript.mdis excluded by!pilot/**pilot/rules/team-vault.mdis excluded by!pilot/**pilot/scripts/mcp-server.cjsis excluded by!pilot/**pilot/scripts/worker-service.cjsis excluded by!pilot/**pilot/settings.jsonis excluded by!pilot/**pilot/ui/viewer-bundle.jsis excluded by!pilot/**pilot/ui/viewer.cssis excluded by!pilot/**
📒 Files selected for processing (75)
.coderabbit.yaml.gitattributes.github/ISSUE_TEMPLATE/config.yml.github/ISSUE_TEMPLATE/feature_request.md.github/workflows/claude.yml.github/workflows/release-dev.yml.github/workflows/release.yml.gitignore.trivyignoreLICENSEREADME.mdconsole/src/services/worker-service.tsconsole/src/services/worker/BranchManager.tsconsole/src/services/worker/http/routes/SettingsRoutes.tsconsole/src/services/worker/http/routes/TeamsRoutes.tsconsole/src/services/worker/http/routes/TeamsTypes.tsconsole/src/services/worker/http/routes/WorktreeRoutes.tsconsole/src/ui/viewer/App.tsxconsole/src/ui/viewer/components/CommandPalette.tsxconsole/src/ui/viewer/components/LicenseBadge.tsxconsole/src/ui/viewer/components/TeamGate.tsxconsole/src/ui/viewer/constants/shortcuts.tsconsole/src/ui/viewer/hooks/useSettings.tsconsole/src/ui/viewer/hooks/useStats.tsconsole/src/ui/viewer/hooks/useTeams.tsconsole/src/ui/viewer/layouts/Sidebar/SidebarNav.tsxconsole/src/ui/viewer/views/Dashboard/TeamsStatus.tsxconsole/src/ui/viewer/views/Dashboard/VexorStatus.tsxconsole/src/ui/viewer/views/Dashboard/index.tsxconsole/src/ui/viewer/views/Settings/index.tsxconsole/src/ui/viewer/views/Teams/TeamsAssetDetail.tsxconsole/src/ui/viewer/views/Teams/TeamsAssetTable.tsxconsole/src/ui/viewer/views/Teams/TeamsContentModal.tsxconsole/src/ui/viewer/views/Teams/TeamsHelpModal.tsxconsole/src/ui/viewer/views/Teams/TeamsOnboarding.tsxconsole/src/ui/viewer/views/Teams/TeamsPushPanel.tsxconsole/src/ui/viewer/views/Teams/TeamsSetupTab.tsxconsole/src/ui/viewer/views/Teams/TeamsSummaryCards.tsxconsole/src/ui/viewer/views/Teams/index.tsxconsole/src/ui/viewer/views/index.tsconsole/tests/hooks/use-teams.test.tsconsole/tests/hooks/useSettings.test.tsconsole/tests/settings-routes.test.tsconsole/tests/ui/SidebarNav.test.tsconsole/tests/ui/search-removal.test.tsconsole/tests/ui/teams-install.test.tsconsole/tests/ui/teams-view.test.tsconsole/tests/ui/vexor-status-widget.test.tsconsole/tests/ui/views-index.test.tsconsole/tests/worker/teams-routes.test.tsdocs/site/src/components/ConsoleSection.tsxdocs/site/src/components/DeepDiveSection.tsxdocs/site/src/components/FAQSection.tsxdocs/site/src/components/HeroSection.tsxdocs/site/src/components/InstallSection.tsxdocs/site/src/components/PricingSection.tsxdocs/site/src/components/TeamsDashboardSection.tsxdocs/site/src/components/WhatsInside.tsxdocs/site/src/components/WorkflowSteps.tsxdocs/site/src/content/blog/claude-code-settings-reference.mddocs/site/src/content/blog/slash-commands-and-init.mddocs/site/src/content/blog/team-vault-sharing-ai-assets.mddocs/site/src/pages/DocsPage.tsxdocs/site/src/pages/Index.tsxdocs/site/src/pages/docs/ConsoleSection.tsxdocs/site/src/pages/docs/InstallationSection.tsxdocs/site/src/pages/docs/LearnSection.tsxdocs/site/src/pages/docs/PrerequisitesSection.tsxdocs/site/src/pages/docs/RulesSection.tsxdocs/site/src/pages/docs/SyncSection.tsxdocs/site/src/pages/docs/TeamsSection.tsxinstaller/cli.pyinstaller/steps/dependencies.pyinstaller/tests/unit/steps/test_dependencies.pyinstaller/ui.py
💤 Files with no reviewable changes (4)
- .github/ISSUE_TEMPLATE/config.yml
- installer/cli.py
- docs/site/src/content/blog/slash-commands-and-init.md
- installer/tests/unit/steps/test_dependencies.py
✅ Files skipped from review due to trivial changes (8)
- docs/site/src/components/DeepDiveSection.tsx
- docs/site/src/pages/docs/RulesSection.tsx
- docs/site/src/pages/docs/SyncSection.tsx
- .gitattributes
- docs/site/src/components/HeroSection.tsx
- docs/site/src/pages/docs/InstallationSection.tsx
- .trivyignore
- docs/site/src/components/PricingSection.tsx
| exit-code: "1" | ||
| ignore-unfixed: true | ||
| skip-dirs: ".venv,node_modules,console/node_modules,launcher,docs/site/api" | ||
| skip-dirs: ".venv,node_modules,console/node_modules,launcher,docs/site/api,console" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, verify the workflow file exists and check lines around 24-30 and 41
cat -n .github/workflows/release-dev.yml | head -50Repository: maxritter/pilot-shell
Length of output: 1735
🏁 Script executed:
# Check if .trivyignore exists and what it contains
ls -la | grep trivyignore
cat .trivyignore 2>/dev/null || echo "No .trivyignore found"Repository: maxritter/pilot-shell
Length of output: 862
🏁 Script executed:
# Check the structure of the console directory
fd -t d console . --max-depth 1 2>/dev/nullRepository: maxritter/pilot-shell
Length of output: 75
🏁 Script executed:
# Check for pre-commit configuration
cat .pre-commit-config.yaml 2>/dev/null || echo "No .pre-commit-config.yaml found"Repository: maxritter/pilot-shell
Length of output: 97
🏁 Script executed:
# Check git history for when console was added to skip-dirs (recent commits)
git log --oneline -20 -- .github/workflows/release-dev.yml 2>/dev/null | head -10Repository: maxritter/pilot-shell
Length of output: 168
Confirm concern: console in skip-dirs contradicts the intent to scan decrypted content.
The workflow decrypts the repository via git-crypt (lines 24-30), but then Trivy skips scanning console entirely on line 41. The .trivyignore file documents that "In CI, the security-scan job decrypts them before scanning," indicating the intent is to scan decrypted directories. However, adding console to skip-dirs defeats this purpose.
Either remove console from skip-dirs to scan the decrypted content, or document in .trivyignore the specific reason for the exclusion (e.g., known false positives, acceptable risk, or design decision).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/release-dev.yml at line 41, The workflow currently
excludes the console directory from Trivy scans by listing "console" in the
skip-dirs setting (skip-dirs:
".venv,node_modules,console/node_modules,launcher,docs/site/api,console"), which
contradicts the git-crypt decryption and the CI security-scan intent; either
remove "console" from the skip-dirs value in the release-dev.yml so Trivy scans
the decrypted console content, or keep the exclusion but add a clear
justification in .trivyignore (referencing the security-scan job and git-crypt
decryption) explaining why console is an accepted exclusion (e.g., documented
false positives or accepted risk).
| exit-code: '1' | ||
| ignore-unfixed: true | ||
| skip-dirs: '.venv,node_modules,console/node_modules,launcher,docs/site/api' | ||
| skip-dirs: '.venv,node_modules,console/node_modules,launcher,docs/site/api,console' |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Checking Trivy skip-dirs in workflow ==="
rg -n "skip-dirs:" .github/workflows/release.yml -C2
echo
echo "=== Checking .trivyignore for encrypted-dir expectations ==="
if [ -f ".trivyignore" ]; then
rg -n "git-crypt|encrypted|decrypts" .trivyignore -C1 || echo "No matching patterns found"
else
echo ".trivyignore file not found"
fi
echo
echo "=== Full Trivy configuration section in workflow ==="
rg -n "trivy" .github/workflows/release.yml -A5 -B5Repository: maxritter/pilot-shell
Length of output: 1993
Remove console from Trivy skip-dirs to prevent security scan blind spot.
The console directory is git-crypt encrypted and explicitly decrypted in this workflow before scanning (as documented in .trivyignore), yet it's included in skip-dirs on line 91, preventing any vulnerability or secret scanning of decrypted console sources. This contradicts the documented intent and creates a security coverage gap in a high-change area.
🔧 Proposed fix
- skip-dirs: '.venv,node_modules,console/node_modules,launcher,docs/site/api,console'
+ skip-dirs: '.venv,node_modules,console/node_modules,launcher,docs/site/api'📝 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
| skip-dirs: '.venv,node_modules,console/node_modules,launcher,docs/site/api,console' | |
| skip-dirs: '.venv,node_modules,console/node_modules,launcher,docs/site/api' |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/release.yml at line 91, The workflow currently excludes
the console directory via the skip-dirs setting (skip-dirs:
'.venv,node_modules,console/node_modules,launcher,docs/site/api,console'),
causing Trivy to skip scanning decrypted console sources; update the skip-dirs
value to remove the standalone "console" entry (keep console/node_modules if
desired) so Trivy will scan the console code after it is decrypted (ensure the
existing console decryption step runs before Trivy), and verify .trivyignore
still documents the intended behavior.
Comment on lines
+231
to
232
| sx.json | ||
| .env** |
There was a problem hiding this comment.
.env** is overly broad and may hide tracked env templates.
Ignoring .env** will also ignore files like .env.example / .env.template, which are often intentionally committed. Prefer a narrower pattern and explicit allowlist entries.
Suggested `.gitignore` tweak
-sx.json
-.env**
+sx.json
+.env
+.env.*
+!.env.example
+!.env.template🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.gitignore around lines 231 - 232, The .gitignore entry '.env**' is too
broad and hides committed env templates; replace it with explicit ignore
patterns (e.g., '.env', '.env.local', '.env.*.local' or other runtime/private
files you intend to exclude) and add explicit allowlist entries for committed
templates such as '.env.example' and '.env.template' so those files remain
tracked; update the '.env**' entry in the .gitignore to the narrower patterns
and add allowlist lines for the example/template filenames.
Comment on lines
+101
to
103
| extracts knowledge, the{" "} | ||
| <strong>Teams dashboard</strong>{" "} | ||
| shares with your team. |
There was a problem hiding this comment.
Fix grammar/clarity in the post-install guidance sentence.
At Line 101, the sentence uses a comma splice and leaves “shares” without a clear object, which makes the instruction read awkwardly.
✏️ Proposed copy fix
- extracts knowledge, the{" "}
- <strong>Teams dashboard</strong>{" "}
- shares with your team.
+ extracts knowledge, and the{" "}
+ <strong>Teams dashboard</strong>{" "}
+ helps you share with your team.📝 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
| extracts knowledge, the{" "} | |
| <strong>Teams dashboard</strong>{" "} | |
| shares with your team. | |
| extracts knowledge, and the{" "} | |
| <strong>Teams dashboard</strong>{" "} | |
| helps you share with your team. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/site/src/components/InstallSection.tsx` around lines 101 - 103, Fix the
comma splice and missing object in the JSX sentence inside InstallSection (the
fragment containing "extracts knowledge, the <strong>Teams dashboard</strong>
shares with your team."): replace it with a clear phrasing such as "extracts
knowledge that the Teams dashboard shares with your team." so the sentence reads
smoothly and specifies what is being shared; update the string in the
InstallSection.tsx JSX where that fragment appears.
| slug: "team-vault-sharing-ai-assets" | ||
| title: "Sharing AI Assets Across Your Team with Vault" | ||
| description: "Sync rules, commands, skills, and hooks across your team. Keep every developer\'s Claude consistent with a shared vault." | ||
| description: "Sync rules, skills, commands, and agents across your team. Keep every developer\'s Claude consistent with a shared repository." |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if the file exists and examine the frontmatter
head -20 docs/site/src/content/blog/team-vault-sharing-ai-assets.mdRepository: maxritter/pilot-shell
Length of output: 908
🏁 Script executed:
# Run the verification script from the review comment
rg -n "\\\\'" docs/site/src/content/blog/team-vault-sharing-ai-assets.mdRepository: maxritter/pilot-shell
Length of output: 208
🏁 Script executed:
# Also check the exact state of line 4
sed -n '4p' docs/site/src/content/blog/team-vault-sharing-ai-assets.md | cat -ARepository: maxritter/pilot-shell
Length of output: 207
Fix invalid YAML escape sequence in frontmatter description.
Line 4 contains developer\'s in a double-quoted YAML string. Single quotes do not require escaping in YAML double-quoted strings; \' is not a valid escape sequence and may cause parsing issues or unexpected behavior depending on YAML parser strictness.
Proposed fix
-description: "Sync rules, skills, commands, and agents across your team. Keep every developer\'s Claude consistent with a shared repository."
+description: "Sync rules, skills, commands, and agents across your team. Keep every developer's Claude consistent with a shared repository."📝 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
| description: "Sync rules, skills, commands, and agents across your team. Keep every developer\'s Claude consistent with a shared repository." | |
| description: "Sync rules, skills, commands, and agents across your team. Keep every developer's Claude consistent with a shared repository." |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/site/src/content/blog/team-vault-sharing-ai-assets.md` at line 4,
Frontmatter "description" contains an invalid YAML escape sequence
`developer\'s`; update the description value (the "description" frontmatter
field in team-vault-sharing-ai-assets.md) to remove the backslash so it reads
developer's (or alternatively switch to a single-quoted YAML string and escape
as needed), ensuring the double-quoted string contains a plain apostrophe
without `\`.
Comment on lines
+60
to
+83
| <div className="rounded-xl p-4 border border-primary/20 bg-primary/5 mb-6"> | ||
| <div className="flex items-center gap-2 mb-2"> | ||
| <Terminal className="h-4 w-4 text-primary" /> | ||
| <h3 className="font-semibold text-foreground text-sm"> | ||
| Claude Code (Required) | ||
| </h3> | ||
| </div> | ||
| <p className="text-xs text-muted-foreground leading-relaxed"> | ||
| Pilot Shell installs <span className="text-foreground font-medium">on top of Claude Code</span> — you must install it first. | ||
| Install via npm:{" "} | ||
| <code className="text-primary bg-primary/10 px-1 py-0.5 rounded">npm install -g @anthropic-ai/claude-code</code> | ||
| {" "}or use the{" "} | ||
| <a | ||
| href="https://docs.anthropic.com/en/docs/claude-code/setup" | ||
| className="text-primary hover:underline" | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| > | ||
| native installer | ||
| </a> | ||
| . Pilot Shell does not install or update Claude Code for you. | ||
| </p> | ||
| </div> | ||
|
|
There was a problem hiding this comment.
Prevent inline command overflow on small screens.
The npm command token can overflow in narrow viewports. Add wrapping behavior on the <code> element to avoid clipping/horizontal scroll.
💡 Suggested tweak
- <code className="text-primary bg-primary/10 px-1 py-0.5 rounded">npm install -g `@anthropic-ai/claude-code`</code>
+ <code className="text-primary bg-primary/10 px-1 py-0.5 rounded break-all">npm install -g `@anthropic-ai/claude-code`</code>📝 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
| <div className="rounded-xl p-4 border border-primary/20 bg-primary/5 mb-6"> | |
| <div className="flex items-center gap-2 mb-2"> | |
| <Terminal className="h-4 w-4 text-primary" /> | |
| <h3 className="font-semibold text-foreground text-sm"> | |
| Claude Code (Required) | |
| </h3> | |
| </div> | |
| <p className="text-xs text-muted-foreground leading-relaxed"> | |
| Pilot Shell installs <span className="text-foreground font-medium">on top of Claude Code</span> — you must install it first. | |
| Install via npm:{" "} | |
| <code className="text-primary bg-primary/10 px-1 py-0.5 rounded">npm install -g @anthropic-ai/claude-code</code> | |
| {" "}or use the{" "} | |
| <a | |
| href="https://docs.anthropic.com/en/docs/claude-code/setup" | |
| className="text-primary hover:underline" | |
| target="_blank" | |
| rel="noopener noreferrer" | |
| > | |
| native installer | |
| </a> | |
| . Pilot Shell does not install or update Claude Code for you. | |
| </p> | |
| </div> | |
| <div className="rounded-xl p-4 border border-primary/20 bg-primary/5 mb-6"> | |
| <div className="flex items-center gap-2 mb-2"> | |
| <Terminal className="h-4 w-4 text-primary" /> | |
| <h3 className="font-semibold text-foreground text-sm"> | |
| Claude Code (Required) | |
| </h3> | |
| </div> | |
| <p className="text-xs text-muted-foreground leading-relaxed"> | |
| Pilot Shell installs <span className="text-foreground font-medium">on top of Claude Code</span> — you must install it first. | |
| Install via npm:{" "} | |
| <code className="text-primary bg-primary/10 px-1 py-0.5 rounded break-all">npm install -g `@anthropic-ai/claude-code`</code> | |
| {" "}or use the{" "} | |
| <a | |
| href="https://docs.anthropic.com/en/docs/claude-code/setup" | |
| className="text-primary hover:underline" | |
| target="_blank" | |
| rel="noopener noreferrer" | |
| > | |
| native installer | |
| </a> | |
| . Pilot Shell does not install or update Claude Code for you. | |
| </p> | |
| </div> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/site/src/pages/docs/PrerequisitesSection.tsx` around lines 60 - 83, The
inline <code> element in PrerequisitesSection.tsx can overflow on narrow
screens; update the <code className="..."> for the npm command token (the code
element inside Claude Code paragraph) to enable wrapping and prevent clipping by
adding Tailwind utilities such as break-words (or break-all) and
whitespace-normal/whitespace-pre-wrap and/or display inline-block (e.g., add
"break-words whitespace-normal inline-block" or "break-all whitespace-normal")
so the long token wraps instead of causing horizontal overflow.
Comment on lines
+64
to
+65
| Share rules, commands, and skills across your team via the Console | ||
| dashboard |
There was a problem hiding this comment.
Header copy is inconsistent with listed shareable asset types.
Line 64 mentions rules/commands/skills, but this section also documents agents as shareable.
Suggested copy tweak
- Share rules, commands, and skills across your team via the Console
+ Share rules, commands, skills, and agents across your team via the Console
dashboard📝 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
| Share rules, commands, and skills across your team via the Console | |
| dashboard | |
| Share rules, commands, skills, and agents across your team via the Console | |
| dashboard |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/site/src/pages/docs/TeamsSection.tsx` around lines 64 - 65, Header copy
in TeamsSection.tsx is inconsistent with the listed shareable asset types;
update the header string that currently reads "Share rules, commands, and skills
across your team via the Console dashboard" to include "agents" (e.g., "Share
agents, rules, commands, and skills across your team via the Console dashboard")
so the header matches the documented items and locate the exact text in the
TeamsSection component to replace it.
Comment on lines
+413
to
+415
| # Disable all clients except claude-code to prevent .cursor/.gemini folders | ||
| for client in ("gemini", "cursor", "github-copilot", "codex"): | ||
| _run_bash_with_retry(f"sx clients disable {client}") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n installer/steps/dependencies.py | sed -n '400,430p'Repository: maxritter/pilot-shell
Length of output: 1252
🏁 Script executed:
# Find the install_sx function definition to understand full context
rg -n "def install_sx" installer/steps/dependencies.py -A 30Repository: maxritter/pilot-shell
Length of output: 1230
🏁 Script executed:
# Search for _run_bash_with_retry definition to understand its behavior
rg -n "_run_bash_with_retry" installer/steps/dependencies.py -B 2 -A 8 | head -50Repository: maxritter/pilot-shell
Length of output: 1865
🌐 Web query:
sleuth-io sx CLI clients disable command documentation
💡 Result:
SX CLI has a clients command group for managing which AI coding assistants receive installed assets:
- Disable a client
- Command:
sx clients disable <client-id> - What it does: disables that client “so that assets will not be installed to it.” [1]
- Command:
- Valid
<client-id>values:claude-code,cursor,gemini,github-copilot,codex[2]
Related commands:
sx clients— list detected clients and their installation status (supports--json). [1]sx clients enable <client-id>— re-enable a client. [1]sx clients reset— revert to default behavior (assets install to all detected clients). [1]
Sources: [1] [2]
Handle sx clients disable failures explicitly.
At lines 414-415, the disable calls ignore their return values; if any call fails, install_sx() still returns True, so users get no feedback and the post-install state may be incomplete.
Suggested fix
# Disable all clients except claude-code to prevent .cursor/.gemini folders
- for client in ("gemini", "cursor", "github-copilot", "codex"):
- _run_bash_with_retry(f"sx clients disable {client}")
+ failed_clients: list[str] = []
+ for client in ("gemini", "cursor", "github-copilot", "codex"):
+ if not _run_bash_with_retry(f"sx clients disable {client}"):
+ failed_clients.append(client)
+
+ if failed_clients:
+ return FalsePer coding guidelines, installer/**/*.py must include proper error handling and user feedback.
📝 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
| # Disable all clients except claude-code to prevent .cursor/.gemini folders | |
| for client in ("gemini", "cursor", "github-copilot", "codex"): | |
| _run_bash_with_retry(f"sx clients disable {client}") | |
| # Disable all clients except claude-code to prevent .cursor/.gemini folders | |
| failed_clients: list[str] = [] | |
| for client in ("gemini", "cursor", "github-copilot", "codex"): | |
| if not _run_bash_with_retry(f"sx clients disable {client}"): | |
| failed_clients.append(client) | |
| if failed_clients: | |
| return False |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@installer/steps/dependencies.py` around lines 413 - 415, The loop that
disables clients inside install_sx currently ignores _run_bash_with_retry
results; modify install_sx so each call to _run_bash_with_retry(f"sx clients
disable {client}") checks the return value and on failure logs a clear error
referencing the client (e.g., "failed to disable {client}") and returns False
(or raises) so install_sx does not return True on partial failure; use the
existing logger (or raise RuntimeError) and stop further processing when a
disable fails to ensure proper user feedback and error handling.
Comment on lines
+195
to
+208
| ### Team Asset Sharing | ||
|
|
||
| Share rules, skills, commands, and agents across your team from a central, private Git repository: | ||
|
|
||
| <img src="docs/img/teams.png" alt="Teams Dashboard — shared asset management" width="700"> | ||
|
|
||
| <details> | ||
| <summary><b>What Teams includes</b></summary> | ||
|
|
||
| - **Shared asset management** — Push and install rules, skills, commands, and agents from the Console UI | ||
| - **Project-scoped** — Assets are tied to repositories, so each project gets exactly what it needs | ||
| - **Automatic versioning** — See installed vs. latest versions at a glance | ||
| - **Guided onboarding** — New team members get a step-by-step screen to setup | ||
|
|
There was a problem hiding this comment.
Fix grammar in onboarding bullet (setup → set up).
Line 207 should use the verb form: “step-by-step screen to set up”.
✏️ Proposed doc fix
-- **Guided onboarding** — New team members get a step-by-step screen to setup
+- **Guided onboarding** — New team members get a step-by-step screen to set up🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` around lines 195 - 208, Update the "Guided onboarding" bullet
under the "What Teams includes" details: change the noun form "setup" to the
verb phrase "set up" so the sentence reads "New team members get a step-by-step
screen to set up"; locate the bullet that starts with "Guided onboarding — New
team members get a step-by-step screen to setup" and replace "setup" with "set
up".
github-actions bot
pushed a commit
that referenced
this pull request
Mar 5, 2026
# [7.2.0](v7.1.5...v7.2.0) (2026-03-05) ### Features * New Teams asset sharing dashboard for skills, rules, commands and agents ([#90](#90)) ([e8f59fc](e8f59fc))
Contributor
|
🎉 This PR is included in version 7.2.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
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
1 participant
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
Redesigns the Teams feature in the Console dashboard for team asset sharing (rules, skills, commands, agents) and removes Claude Code installation from Pilot Shell — it's now a prerequisite users install themselves.
Changes
Teams Asset Sharing Dashboard
Claude Code Installation Removed
launcher/claude_installer.py— Pilot Shell no longer installs or updates Claude Code_update_claude_code()from the launcher startup flowinstall_claude_code()from the installer dependencies stepDocumentation & Pricing
Other
Commits
Testing
Summary by CodeRabbit
New Features
Refactor
Documentation
Chores