feat(router): semantic team routing and spec-aware Q&A — Phase 1#4
feat(router): semantic team routing and spec-aware Q&A — Phase 1#4JustAGhosT merged 1 commit intomainfrom
Conversation
|
Mention Blocks like a regular teammate with your question or request: @blocks review this pull request Run |
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 8 minutes and 44 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (17)
📝 WalkthroughWalkthroughThis PR adds a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as AskPanel
participant Server as HTTP Server
participant Router as Router Module
User->>UI: types query
UI->>UI: debounce 300ms
UI->>Server: GET /api/ask?q=query
Server->>Router: route(query, index)
Router->>Router: isOffTopic(query)?
alt Off-topic
Router-->>Server: AskResponse{scopeViolation: true}
else In-scope
Router->>Router: search(query, entries)
Router->>Router: tokenise & score by overlap
Router-->>Server: AskResponse{results[]}
end
Server-->>UI: 200 JSON
UI->>UI: render results or scope message
UI-->>User: display
sequenceDiagram
participant StateWatcher
participant CogmeshPoller
participant CogmeshAPI
participant WSServer
participant ClientUI
rect rgba(100,200,150,0.5)
StateWatcher->>CogmeshPoller: createCogmeshPoller(root,onHealth)
CogmeshPoller->>CogmeshPoller: parseCogmeshConfig(root)
CogmeshPoller->>CogmeshPoller: start 30s polling
end
loop every 30s
CogmeshPoller->>CogmeshAPI: fetch /health (5s timeout)
alt success
CogmeshAPI-->>CogmeshPoller: response
CogmeshPoller->>StateWatcher: onHealth({connected|degraded,latency})
else timeout/error
CogmeshPoller->>StateWatcher: onHealth({unreachable})
end
StateWatcher->>WSServer: broadcast cogmesh:health:updated
WSServer->>ClientUI: send HostMessage
ClientUI->>ClientUI: update store.cogmeshHealth
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related issues
Possibly related PRs
Poem
🚥 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4c0591e1e2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| case 'teams': { | ||
| const teams = parseTeams(workspaceRoot) | ||
| broadcast({ type: 'teams:updated', teams }) | ||
| setRouterIndex(buildIndex(teams)) |
There was a problem hiding this comment.
Refresh router index on snapshot-path team updates
setRouterIndex is only called in the teams-only branch, so when a teams change is coalesced with another kind in the 250ms debounce window (for example .retortconfig + teams.yaml written together during init), execution takes the full-snapshot early return and never rebuilds the router index. The UI receives updated teams, but /api/ask keeps routing against stale entries until a later isolated teams event.
Useful? React with 👍 / 👎.
| const candidates = [ | ||
| path.join(root, '.agentkit', 'spec', 'teams.yaml'), // canonical | ||
| path.join(root, '.agentkit', 'spec', 'AGENT_TEAMS.yaml'), // legacy | ||
| path.join(root, '.agentkit', 'overlays', detectRepoName(root), 'teams.yaml'), // overlay |
There was a problem hiding this comment.
Watch overlay teams.yaml for live team/index refreshes
This commit adds .agentkit/overlays/<repo>/teams.yaml as a teams source, but createWatcher still only tracks AGENT_TEAMS.md and .agentkit/spec/**. As a result, edits to the new overlay file are parsed on startup but won't emit teams:updated or trigger router index rebuilds while the daemon is running, leaving both dashboard data and Ask routing stale.
Useful? React with 👍 / 👎.
| }) | ||
|
|
||
| // port: 0 asks the OS to pick a free port | ||
| httpServer.listen(0, '127.0.0.1', () => { |
There was a problem hiding this comment.
Listen on localhost-compatible address family
The server now binds specifically to 127.0.0.1, while the VS Code webview iframe and WS URL use localhost. On environments where localhost resolves to ::1 first, the iframe/fetch/ws connections can fail because the daemon is not listening on IPv6. Binding to localhost (or omitting host to accept both families) avoids this platform-dependent connection break.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (6)
packages/ui/src/styles/index.css (1)
395-406: Add a visible keyboard focus style for.cm-result-link.This control has
:hoverstyling but no:focus-visible, which makes keyboard navigation feedback weaker.Suggested improvement
.cm-result-link { align-self: flex-start; background: none; border: none; color: var(--accent); cursor: pointer; font-size: 11px; padding: 0; text-decoration: underline; } .cm-result-link:hover { opacity: 0.8; } +.cm-result-link:focus-visible { + outline: 1px solid var(--accent); + outline-offset: 2px; + border-radius: 2px; +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/styles/index.css` around lines 395 - 406, Add a visible keyboard focus style for the .cm-result-link control by defining a :focus-visible rule that provides a clear outline or box-shadow (contrasting color and sufficient thickness) and preserves existing styles; update the .cm-result-link selector to include a matching :focus-visible rule (e.g., .cm-result-link:focus-visible) so keyboard users see a distinct focus indicator while keeping background, color, and underline behavior unchanged and ensuring the hover opacity stays intact.packages/state-watcher/src/index.ts (1)
47-58: Consider caching the initial teams parse to avoid redundant disk reads.
parseTeams(workspaceRoot)is called once here (line 52) to build the initial router index, and will be called again whenbuildSnapshot()is invoked. While not a bug, this results in redundant file I/O on startup.♻️ Optional: Cache initial teams parse
const { broadcast, close, setRouterIndex } = createServer( (port) => { // Signal VS Code extension that the server is ready process.stdout.write(`PORT:${port}\n`) // Build initial router index once teams are parsed - setRouterIndex(buildIndex(parseTeams(workspaceRoot))) + const initialTeams = parseTeams(workspaceRoot) + setRouterIndex(buildIndex(initialTeams)) },Alternatively, rely on the file watcher to trigger the initial index build after the server starts.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/state-watcher/src/index.ts` around lines 47 - 58, parseTeams(workspaceRoot) is invoked twice at startup causing redundant disk reads; compute and cache the initial parse once and reuse it when building the initial router index and when buildSnapshot runs. Specifically, call parseTeams(workspaceRoot) into a local variable (e.g. initialTeams) before createServer, call setRouterIndex(buildIndex(initialTeams)) in the server-ready callback, and modify buildSnapshot (or its caller) to accept an optional teams argument or read from the same cached initialTeams instead of re-reading from disk; reference functions/variables parseTeams, buildIndex, setRouterIndex, and buildSnapshot when making these changes.packages/state-watcher/src/server.ts (1)
49-68: Consider validating HTTP method for/api/ask.The endpoint currently handles all HTTP methods (GET, POST, DELETE, etc.) identically. While this works, it's cleaner to explicitly accept only GET requests and return 405 Method Not Allowed for others.
♻️ Optional: Add method check
if (url.pathname === '/api/ask') { + if (req.method !== 'GET') { + res.writeHead(405) + res.end(JSON.stringify({ error: 'method not allowed' })) + return + } const q = url.searchParams.get('q') ?? ''🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/state-watcher/src/server.ts` around lines 49 - 68, Add an explicit HTTP method check at the top of the /api/ask handler: verify req.method (or the equivalent request object) is 'GET' and if not, respond with 405 Method Not Allowed (include an Allow: GET header) instead of processing the request; keep existing logic that reads url.searchParams, validates q, checks routerIndex, and calls route(q, routerIndex) unchanged (referencing the /api/ask handler, routerIndex, route, and AskResponse to locate where to insert the check).packages/ui/src/App.tsx (1)
27-33:resolveBaseUrlis called on every render; consider memoizing.Since
window.locationdoesn't change during the component lifecycle, you could memoize this value withuseMemoto avoid recalculating on each render. This is a minor optimization.♻️ Optional memoization
+import { useEffect, useMemo } from 'react' -import { useEffect } from 'react' // Inside App component: - const baseUrl = resolveBaseUrl() + const baseUrl = useMemo(() => resolveBaseUrl(), [])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/App.tsx` around lines 27 - 33, resolveBaseUrl() is being invoked on every render even though window.location is static; memoize its result inside the React component (e.g., App) using useMemo so it's computed once: import useMemo from React, call useMemo(() => resolveBaseUrl(), []) (or inline the same logic in the memo) and replace direct resolveBaseUrl() calls with the memoized variable; reference the resolveBaseUrl function and the App component to locate where to replace the calls.packages/state-watcher/src/cogmesh-poller.ts (1)
20-44: Consider guarding against overlapping health checks.If
checkHealth()takes longer than 30 seconds (e.g., due to network issues before the 5s timeout kicks in), the interval will fire again while the previous check is still pending. This could lead to multiple concurrent health checks and out-of-orderonHealthcallbacks.♻️ Optional: Add a simple in-flight guard
+ let checking = false + async function checkHealth(): Promise<void> { + if (checking) return + checking = true + try { if (!config.endpoint) { onHealth({ status: 'unconfigured' }) return } // ... rest of function ... + } finally { + checking = false + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/state-watcher/src/cogmesh-poller.ts` around lines 20 - 44, checkHealth can overlap when previous call is still pending; add an in-flight guard (e.g. a module/closure-scoped boolean like healthInFlight) and early-return if it's true, then set it to true when starting the actual network call and reset it in a finally block so it always clears (even on errors). Update the checkHealth function so the guard is checked before making the fetch, set healthInFlight = true just before the fetch using HEALTH_TIMEOUT_MS, and reset healthInFlight = false in finally while still calling onHealth as currently done; ensure the early unconfigured return does not flip the guard.packages/router/src/index-builder.ts (1)
24-31: Minor: Handle emptyfocusto avoid awkward explanation.If
team.focusis an empty string, the explanation becomes"The X team handles ."which reads awkwardly. Consider a fallback.♻️ Proposed fix
keywords: tokenise(raw), - explanation: `The ${team.name} team handles ${team.focus.toLowerCase()}.`, + explanation: team.focus + ? `The ${team.name} team handles ${team.focus.toLowerCase()}.` + : `The ${team.name} team.`, suggestedCommand: `/team-${team.id}`,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/router/src/index-builder.ts` around lines 24 - 31, The explanation string built in index-builder.ts for a team may be awkward when team.focus is empty; update the logic that creates the returned object (the place building the explanation for team.name and team.focus, alongside tokenise() and suggestedCommand) to use a fallback when team.focus is falsy (e.g., "various responsibilities" or "multiple areas") or omit the "handles ..." clause entirely; ensure the code that sets explanation uses a conditional/formatted value derived from team.focus so the sentence reads correctly even when team.focus === "".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/router/package.json`:
- Around line 1-20: The package.json for the router plugin is missing the
"type": "module" field, causing Node to treat compiled ES module .js files as
CommonJS; update the package.json (the top-level JSON object in
packages/router/package.json) to include "type": "module" so the runtime
interprets dist/index.js and other compiled .js files as ESM and fixes
compatibility with consumers like `@retort-plugins/state-watcher`.
In `@packages/router/src/router.ts`:
- Around line 9-14: OFF_TOPIC_PATTERNS contains a broad regex
(/\bwhat\s+is\s+(the\s+)?[a-z]+\s+(of|in)\b/i) that causes false positives;
refine it by either narrowing the pattern to target known world-knowledge nouns
(e.g., country|city|capital|population|history|recipe|weather|sport|movie|music)
or add an allowlist check before applying OFF_TOPIC_PATTERNS to skip matches
containing workspace/team-related terms (e.g.,
scope|focus|team|frontend|backend|service|module); update the OFF_TOPIC_PATTERNS
entry or the router logic that uses it so team/config questions are excluded
from being flagged off-topic while still catching genuine general-knowledge
queries.
In `@packages/state-watcher/package.json`:
- Line 14: Update the internal dependency entry for the router in package.json
to use the workspace protocol instead of a wildcard; replace the existing
"@retort-plugins/router": "*" declaration with the workspace-scoped specifier
(e.g., "@retort-plugins/router": "workspace:*") so the package manager resolves
the local monorepo package consistently.
In `@packages/state-watcher/src/parsers/retortconfig.ts`:
- Around line 27-30: The returned endpoint/secret must be treated as
unconfigured when env placeholders remain unresolved; after calling
resolveEnv(extractScalar(block, 'endpoint')) and resolveEnv(extractScalar(block,
'secret')), detect unresolved placeholders (e.g. a regex like /\$\{[^}]+\}/) and
convert those values to null (or omit) so the poller sees them as unconfigured;
update the code that builds the return object in retortconfig.ts to check the
resolved strings for leftover ${...} and set endpoint/secret to null when found
(referencing resolveEnv and extractScalar to locate the calls to change).
In `@packages/state-watcher/src/parsers/teams.ts`:
- Around line 21-29: The detectRepoName function currently returns an empty
string if the .agentkit-repo marker exists but contains only whitespace, which
causes downstream paths to drop the repo segment; update detectRepoName (look
around markerPath and the fs.readFileSync call) to trim the file contents and
only return the value if it is non-empty (e.g., after trimming), otherwise fall
back to returning path.basename(root); keep the existing try/catch behavior so
errors still fall through to the basename fallback.
In `@packages/ui/src/panels/AskPanel.tsx`:
- Around line 49-64: The useEffect that debounces calls to fetchResults
references baseUrl via the fetchResults closure but doesn't include baseUrl in
its dependency array; update the effect's dependencies to include baseUrl (in
addition to query) so the effect re-runs when baseUrl changes, ensuring
fetchResults uses the latest baseUrl; locate the useEffect block that references
debounceRef, query, setResponse, setError and fetchResults and add baseUrl to
the dependency array.
- Line 2: Replace the direct internal import of the types AskResponse and
RouteResult with an import from the router package's public entrypoint (i.e.,
import these types from the package name/export that the router exposes) so the
code relies on the package API rather than ../../../router/src/types; update the
import statement in AskPanel.tsx to pull AskResponse and RouteResult from the
router package entrypoint and run type checks to ensure no other internal-only
types are used.
In `@packages/ui/src/panels/CognitiveMeshPanel.tsx`:
- Around line 21-25: Clamp the computed progress percentage to the 0–100 range
to avoid visual overflow: when computing pct (currently const pct = total > 0 ?
Math.round((stage / total) * 100) : 0), wrap the result with Math.max(0,
Math.min(100, ...)) or a small clamp helper so negative or >100 values are
constrained; update both occurrences of this computation (the pct used in the
cm-stage-fill width and the equivalent calculation around lines 42-47) so the
inline style width always receives a value between "0%" and "100%".
- Around line 143-149: The formatAge function can produce NaN-based text for
invalid ISO strings; update formatAge to validate the parsed timestamp (e.g.,
const ts = new Date(iso).getTime()) and if Number.isNaN(ts) return a safe
fallback like 'unknown' (or ''), and also handle negative diffs (future
timestamps) by clamping to 'just now'; keep the existing mins/hours logic using
the validated ts/diffMs.
In `@packages/ui/src/styles/index.css`:
- Around line 326-331: The CSS rule for the .cm-health class uses the keyword
"currentColor" which violates Stylelint's lowercase keyword rule; update the
border declaration in the .cm-health rule (the line containing border: 1px solid
currentColor) to use the lowercase keyword "currentcolor" so the property
becomes border: 1px solid currentcolor and satisfies the linting config.
---
Nitpick comments:
In `@packages/router/src/index-builder.ts`:
- Around line 24-31: The explanation string built in index-builder.ts for a team
may be awkward when team.focus is empty; update the logic that creates the
returned object (the place building the explanation for team.name and
team.focus, alongside tokenise() and suggestedCommand) to use a fallback when
team.focus is falsy (e.g., "various responsibilities" or "multiple areas") or
omit the "handles ..." clause entirely; ensure the code that sets explanation
uses a conditional/formatted value derived from team.focus so the sentence reads
correctly even when team.focus === "".
In `@packages/state-watcher/src/cogmesh-poller.ts`:
- Around line 20-44: checkHealth can overlap when previous call is still
pending; add an in-flight guard (e.g. a module/closure-scoped boolean like
healthInFlight) and early-return if it's true, then set it to true when starting
the actual network call and reset it in a finally block so it always clears
(even on errors). Update the checkHealth function so the guard is checked before
making the fetch, set healthInFlight = true just before the fetch using
HEALTH_TIMEOUT_MS, and reset healthInFlight = false in finally while still
calling onHealth as currently done; ensure the early unconfigured return does
not flip the guard.
In `@packages/state-watcher/src/index.ts`:
- Around line 47-58: parseTeams(workspaceRoot) is invoked twice at startup
causing redundant disk reads; compute and cache the initial parse once and reuse
it when building the initial router index and when buildSnapshot runs.
Specifically, call parseTeams(workspaceRoot) into a local variable (e.g.
initialTeams) before createServer, call setRouterIndex(buildIndex(initialTeams))
in the server-ready callback, and modify buildSnapshot (or its caller) to accept
an optional teams argument or read from the same cached initialTeams instead of
re-reading from disk; reference functions/variables parseTeams, buildIndex,
setRouterIndex, and buildSnapshot when making these changes.
In `@packages/state-watcher/src/server.ts`:
- Around line 49-68: Add an explicit HTTP method check at the top of the
/api/ask handler: verify req.method (or the equivalent request object) is 'GET'
and if not, respond with 405 Method Not Allowed (include an Allow: GET header)
instead of processing the request; keep existing logic that reads
url.searchParams, validates q, checks routerIndex, and calls route(q,
routerIndex) unchanged (referencing the /api/ask handler, routerIndex, route,
and AskResponse to locate where to insert the check).
In `@packages/ui/src/App.tsx`:
- Around line 27-33: resolveBaseUrl() is being invoked on every render even
though window.location is static; memoize its result inside the React component
(e.g., App) using useMemo so it's computed once: import useMemo from React, call
useMemo(() => resolveBaseUrl(), []) (or inline the same logic in the memo) and
replace direct resolveBaseUrl() calls with the memoized variable; reference the
resolveBaseUrl function and the App component to locate where to replace the
calls.
In `@packages/ui/src/styles/index.css`:
- Around line 395-406: Add a visible keyboard focus style for the
.cm-result-link control by defining a :focus-visible rule that provides a clear
outline or box-shadow (contrasting color and sufficient thickness) and preserves
existing styles; update the .cm-result-link selector to include a matching
:focus-visible rule (e.g., .cm-result-link:focus-visible) so keyboard users see
a distinct focus indicator while keeping background, color, and underline
behavior unchanged and ensuring the hover opacity stays intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 350816c6-28ba-4403-8c6a-e65a2b768689
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (26)
.github/workflows/vscode.yml.gitignorepackage.jsonpackages/router/package.jsonpackages/router/src/index-builder.tspackages/router/src/index.tspackages/router/src/router.test.tspackages/router/src/router.tspackages/router/src/search.tspackages/router/src/types.tspackages/router/tsconfig.jsonpackages/state-watcher/package.jsonpackages/state-watcher/src/cogmesh-poller.tspackages/state-watcher/src/index.tspackages/state-watcher/src/parsers/retortconfig.tspackages/state-watcher/src/parsers/teams.tspackages/state-watcher/src/protocol.tspackages/state-watcher/src/server.tspackages/ui/src/App.tsxpackages/ui/src/bridge/types.tspackages/ui/src/bridge/useStore.tspackages/ui/src/components/SystemHealthBar.tsxpackages/ui/src/panels/AgentFleetPanel.tsxpackages/ui/src/panels/AskPanel.tsxpackages/ui/src/panels/CognitiveMeshPanel.tsxpackages/ui/src/styles/index.css
| { | ||
| "name": "@retort-plugins/router", | ||
| "version": "0.1.0", | ||
| "private": true, | ||
| "description": "Keyword-based team/agent routing for Retort — answers natural-language questions about agent teams", | ||
| "main": "dist/index.js", | ||
| "types": "dist/index.d.ts", | ||
| "scripts": { | ||
| "build": "tsc -p ./", | ||
| "watch": "tsc -watch -p ./", | ||
| "typecheck": "tsc --noEmit", | ||
| "test": "vitest run", | ||
| "test:watch": "vitest" | ||
| }, | ||
| "devDependencies": { | ||
| "@types/node": "^20.0.0", | ||
| "typescript": "^5.3.0", | ||
| "vitest": "^1.0.0" | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if other workspace packages use "type": "module"
fd package.json packages --exec grep -l '"type".*module' {} \;
# Also check the root package.json
grep -n '"type"' package.json 2>/dev/null || echo "No type field in root package.json"Repository: phoenixvc/retort-plugins
Length of output: 88
🏁 Script executed:
# Check the tsconfig.json in packages/router to see the module setting
find packages/router -name "tsconfig.json" | head -5Repository: phoenixvc/retort-plugins
Length of output: 97
🏁 Script executed:
# If tsconfig exists, read it
cat packages/router/tsconfig.json 2>/dev/null || echo "No tsconfig.json found in packages/router"Repository: phoenixvc/retort-plugins
Length of output: 439
🏁 Script executed:
# Check the root tsconfig and see if there's a base config
cat tsconfig.json 2>/dev/null | head -30Repository: phoenixvc/retort-plugins
Length of output: 50
🏁 Script executed:
# Search for .js imports in the router package source
find packages/router -name "*.ts" -not -path "*/node_modules/*" | head -10Repository: phoenixvc/retort-plugins
Length of output: 257
🏁 Script executed:
# Check if there are actually any source files with imports
find packages/router/src -name "*.ts" 2>/dev/null | head -20Repository: phoenixvc/retort-plugins
Length of output: 257
🏁 Script executed:
# Check import patterns in the source files
rg "from ['\"]" packages/router/src --no-heading -A 0 | head -20Repository: phoenixvc/retort-plugins
Length of output: 1302
🏁 Script executed:
# More specifically, look for .js imports
rg "from ['\"].*\.js" packages/router/srcRepository: phoenixvc/retort-plugins
Length of output: 1221
🏁 Script executed:
# Check if dist/ exists and list its contents
ls -la packages/router/dist/ 2>/dev/null || echo "No dist/ directory"Repository: phoenixvc/retort-plugins
Length of output: 86
🏁 Script executed:
# Check the actual compiled output from one of the main files
cat packages/router/dist/index.js 2>/dev/null || echo "dist/index.js not found or empty"Repository: phoenixvc/retort-plugins
Length of output: 100
🏁 Script executed:
# Check if packages/router is imported anywhere in the codebase
rg "@retort-plugins/router" --no-heading | head -20Repository: phoenixvc/retort-plugins
Length of output: 938
🏁 Script executed:
# Check root package.json more thoroughly
cat package.jsonRepository: phoenixvc/retort-plugins
Length of output: 1014
🏁 Script executed:
# Check if there's any configuration or documentation about module type
rg "type.*module" --no-headingRepository: phoenixvc/retort-plugins
Length of output: 215
Add "type": "module" for ESM compatibility.
The tsconfig.json uses "module": "Node16" which compiles to ES modules, and source files use .js extensions in imports. When compiled, dist/index.js will contain ES module syntax (import/export statements). Without "type": "module" in package.json, Node.js will misinterpret these .js files as CommonJS, causing runtime errors when the package is consumed (e.g., by @retort-plugins/state-watcher).
🔧 Proposed fix
{
"name": "@retort-plugins/router",
"version": "0.1.0",
"private": true,
+ "type": "module",
"description": "Keyword-based team/agent routing for Retort — answers natural-language questions about agent teams",📝 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.
| { | |
| "name": "@retort-plugins/router", | |
| "version": "0.1.0", | |
| "private": true, | |
| "description": "Keyword-based team/agent routing for Retort — answers natural-language questions about agent teams", | |
| "main": "dist/index.js", | |
| "types": "dist/index.d.ts", | |
| "scripts": { | |
| "build": "tsc -p ./", | |
| "watch": "tsc -watch -p ./", | |
| "typecheck": "tsc --noEmit", | |
| "test": "vitest run", | |
| "test:watch": "vitest" | |
| }, | |
| "devDependencies": { | |
| "@types/node": "^20.0.0", | |
| "typescript": "^5.3.0", | |
| "vitest": "^1.0.0" | |
| } | |
| } | |
| { | |
| "name": "@retort-plugins/router", | |
| "version": "0.1.0", | |
| "private": true, | |
| "type": "module", | |
| "description": "Keyword-based team/agent routing for Retort — answers natural-language questions about agent teams", | |
| "main": "dist/index.js", | |
| "types": "dist/index.d.ts", | |
| "scripts": { | |
| "build": "tsc -p ./", | |
| "watch": "tsc -watch -p ./", | |
| "typecheck": "tsc --noEmit", | |
| "test": "vitest run", | |
| "test:watch": "vitest" | |
| }, | |
| "devDependencies": { | |
| "@types/node": "^20.0.0", | |
| "typescript": "^5.3.0", | |
| "vitest": "^1.0.0" | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/router/package.json` around lines 1 - 20, The package.json for the
router plugin is missing the "type": "module" field, causing Node to treat
compiled ES module .js files as CommonJS; update the package.json (the top-level
JSON object in packages/router/package.json) to include "type": "module" so the
runtime interprets dist/index.js and other compiled .js files as ESM and fixes
compatibility with consumers like `@retort-plugins/state-watcher`.
| const OFF_TOPIC_PATTERNS: RegExp[] = [ | ||
| /\b(write|create|generate|build)\s+(a\s+)?(react|vue|angular|component|function|class|script|code|test)\b/i, | ||
| /\b(capital|president|population|history|recipe|weather|sport|movie|music)\b/i, | ||
| /\bwho\s+is\b/i, | ||
| /\bwhat\s+is\s+(the\s+)?[a-z]+\s+(of|in)\b/i, | ||
| ] |
There was a problem hiding this comment.
Pattern 4 may cause false positives for legitimate in-scope queries.
The regex /\bwhat\s+is\s+(the\s+)?[a-z]+\s+(of|in)\b/i could incorrectly flag queries like "what is the scope of team-frontend" or "what is the focus of the backend team" as off-topic, even though these are legitimate questions about team configuration.
Consider narrowing the pattern or adding an allowlist check for scope-related keywords.
💡 Suggested refinement
const OFF_TOPIC_PATTERNS: RegExp[] = [
/\b(write|create|generate|build)\s+(a\s+)?(react|vue|angular|component|function|class|script|code|test)\b/i,
/\b(capital|president|population|history|recipe|weather|sport|movie|music)\b/i,
/\bwho\s+is\b/i,
- /\bwhat\s+is\s+(the\s+)?[a-z]+\s+(of|in)\b/i,
+ // Narrowed to exclude team/agent/scope/focus related questions
+ /\bwhat\s+is\s+(the\s+)?(capital|population|history|weather)\s+(of|in)\b/i,
]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/router/src/router.ts` around lines 9 - 14, OFF_TOPIC_PATTERNS
contains a broad regex (/\bwhat\s+is\s+(the\s+)?[a-z]+\s+(of|in)\b/i) that
causes false positives; refine it by either narrowing the pattern to target
known world-knowledge nouns (e.g.,
country|city|capital|population|history|recipe|weather|sport|movie|music) or add
an allowlist check before applying OFF_TOPIC_PATTERNS to skip matches containing
workspace/team-related terms (e.g.,
scope|focus|team|frontend|backend|service|module); update the OFF_TOPIC_PATTERNS
entry or the router logic that uses it so team/config questions are excluded
from being flagged off-topic while still catching genuine general-knowledge
queries.
| "start": "node dist/index.js" | ||
| }, | ||
| "dependencies": { | ||
| "@retort-plugins/router": "*", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Dependency spec in packages/state-watcher/package.json:"
node -e "const p=require('./packages/state-watcher/package.json'); console.log(p.dependencies['@retort-plugins/router'])"
if [ -f package-lock.json ]; then
echo
echo "Lockfile entries related to `@retort-plugins/router`:"
rg -n -C2 '"@retort-plugins/router"|packages/router|link:' package-lock.json
else
echo "No package-lock.json found at repo root."
fiRepository: phoenixvc/retort-plugins
Length of output: 796
Use workspace protocol for the internal router dependency.
For an internal monorepo package, prefer workspace:* over * to explicitly document the workspace-local resolution and ensure consistent behavior across different package manager configurations. While the lockfile currently shows correct resolution to the local workspace package (link: true), using the workspace protocol is a best practice that improves reproducibility and prevents accidental resolution to external npm registry entries.
Proposed fix
- "@retort-plugins/router": "*",
+ "@retort-plugins/router": "workspace:*",📝 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.
| "@retort-plugins/router": "*", | |
| "@retort-plugins/router": "workspace:*", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/state-watcher/package.json` at line 14, Update the internal
dependency entry for the router in package.json to use the workspace protocol
instead of a wildcard; replace the existing "@retort-plugins/router": "*"
declaration with the workspace-scoped specifier (e.g., "@retort-plugins/router":
"workspace:*") so the package manager resolves the local monorepo package
consistently.
| return { | ||
| endpoint: resolveEnv(extractScalar(block, 'endpoint')), | ||
| secret: resolveEnv(extractScalar(block, 'secret')), | ||
| } |
There was a problem hiding this comment.
Treat unresolved env placeholders in endpoint as unconfigured.
Currently, missing env vars leave ${VAR} intact (Line 41), so endpoint stays non-null and poller attempts ${VAR}/health requests repeatedly instead of reporting unconfigured.
Proposed fix
export function parseCogmeshConfig(root: string): CogmeshConfig {
@@
- return {
- endpoint: resolveEnv(extractScalar(block, 'endpoint')),
- secret: resolveEnv(extractScalar(block, 'secret')),
- }
+ const endpoint = resolveEnv(extractScalar(block, 'endpoint'))
+ const secret = resolveEnv(extractScalar(block, 'secret'))
+ return {
+ endpoint: hasUnresolvedEnvToken(endpoint) ? null : endpoint,
+ secret,
+ }
}
@@
function resolveEnv(value: string | null): string | null {
if (!value) return null
return value.replace(/\$\{([^}]+)\}/g, (_, name: string) => process.env[name] ?? `\${${name}}`)
}
+
+function hasUnresolvedEnvToken(value: string | null): boolean {
+ return value ? /\$\{[^}]+\}/.test(value) : false
+}Also applies to: 39-42
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/state-watcher/src/parsers/retortconfig.ts` around lines 27 - 30, The
returned endpoint/secret must be treated as unconfigured when env placeholders
remain unresolved; after calling resolveEnv(extractScalar(block, 'endpoint'))
and resolveEnv(extractScalar(block, 'secret')), detect unresolved placeholders
(e.g. a regex like /\$\{[^}]+\}/) and convert those values to null (or omit) so
the poller sees them as unconfigured; update the code that builds the return
object in retortconfig.ts to check the resolved strings for leftover ${...} and
set endpoint/secret to null when found (referencing resolveEnv and extractScalar
to locate the calls to change).
| function detectRepoName(root: string): string { | ||
| try { | ||
| const markerPath = path.join(root, '.agentkit-repo') | ||
| if (fs.existsSync(markerPath)) { | ||
| return fs.readFileSync(markerPath, 'utf-8').trim() | ||
| } | ||
| } catch { /* ignore */ } | ||
| return path.basename(root) | ||
| } |
There was a problem hiding this comment.
Guard against empty .agentkit-repo values before returning.
If Line 25 reads an empty marker, detectRepoName returns '', and Line 36 resolves to .agentkit/overlays/teams.yaml (missing repo segment), which can skip intended overlay resolution.
Proposed fix
function detectRepoName(root: string): string {
try {
const markerPath = path.join(root, '.agentkit-repo')
if (fs.existsSync(markerPath)) {
- return fs.readFileSync(markerPath, 'utf-8').trim()
+ const repoName = fs.readFileSync(markerPath, 'utf-8').trim()
+ if (repoName) return repoName
}
} catch { /* ignore */ }
return path.basename(root)
}📝 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.
| function detectRepoName(root: string): string { | |
| try { | |
| const markerPath = path.join(root, '.agentkit-repo') | |
| if (fs.existsSync(markerPath)) { | |
| return fs.readFileSync(markerPath, 'utf-8').trim() | |
| } | |
| } catch { /* ignore */ } | |
| return path.basename(root) | |
| } | |
| function detectRepoName(root: string): string { | |
| try { | |
| const markerPath = path.join(root, '.agentkit-repo') | |
| if (fs.existsSync(markerPath)) { | |
| const repoName = fs.readFileSync(markerPath, 'utf-8').trim() | |
| if (repoName) return repoName | |
| } | |
| } catch { /* ignore */ } | |
| return path.basename(root) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/state-watcher/src/parsers/teams.ts` around lines 21 - 29, The
detectRepoName function currently returns an empty string if the .agentkit-repo
marker exists but contains only whitespace, which causes downstream paths to
drop the repo segment; update detectRepoName (look around markerPath and the
fs.readFileSync call) to trim the file contents and only return the value if it
is non-empty (e.g., after trimming), otherwise fall back to returning
path.basename(root); keep the existing try/catch behavior so errors still fall
through to the basename fallback.
| @@ -0,0 +1,138 @@ | |||
| import { useState, useEffect, useRef } from 'react' | |||
| import type { AskResponse, RouteResult } from '../../../router/src/types' | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Import types from the package entrypoint, not internal paths.
Importing directly from ../../../router/src/types bypasses the package's public API and creates a fragile coupling to internal file structure. This will break if the router package reorganizes its internals.
🔧 Proposed fix
import { useState, useEffect, useRef } from 'react'
-import type { AskResponse, RouteResult } from '../../../router/src/types'
+import type { AskResponse, RouteResult } from '@retort-plugins/router'📝 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.
| import type { AskResponse, RouteResult } from '../../../router/src/types' | |
| import { useState, useEffect, useRef } from 'react' | |
| import type { AskResponse, RouteResult } from '@retort-plugins/router' |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ui/src/panels/AskPanel.tsx` at line 2, Replace the direct internal
import of the types AskResponse and RouteResult with an import from the router
package's public entrypoint (i.e., import these types from the package
name/export that the router exposes) so the code relies on the package API
rather than ../../../router/src/types; update the import statement in
AskPanel.tsx to pull AskResponse and RouteResult from the router package
entrypoint and run type checks to ensure no other internal-only types are used.
| useEffect(() => { | ||
| if (debounceRef.current) clearTimeout(debounceRef.current) | ||
| if (!query.trim()) { | ||
| setResponse(null) | ||
| setError(null) | ||
| return | ||
| } | ||
|
|
||
| debounceRef.current = setTimeout(() => { | ||
| void fetchResults(query) | ||
| }, 300) | ||
|
|
||
| return () => { | ||
| if (debounceRef.current) clearTimeout(debounceRef.current) | ||
| } | ||
| }, [query]) |
There was a problem hiding this comment.
Missing baseUrl in useEffect dependency array.
The fetchResults function captures baseUrl from the closure, but baseUrl is not listed in the dependency array. If the prop changes, the effect will continue using the stale value.
🔧 Proposed fix
useEffect(() => {
if (debounceRef.current) clearTimeout(debounceRef.current)
if (!query.trim()) {
setResponse(null)
setError(null)
return
}
debounceRef.current = setTimeout(() => {
void fetchResults(query)
}, 300)
return () => {
if (debounceRef.current) clearTimeout(debounceRef.current)
}
- }, [query])
+ }, [query, baseUrl])📝 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.
| useEffect(() => { | |
| if (debounceRef.current) clearTimeout(debounceRef.current) | |
| if (!query.trim()) { | |
| setResponse(null) | |
| setError(null) | |
| return | |
| } | |
| debounceRef.current = setTimeout(() => { | |
| void fetchResults(query) | |
| }, 300) | |
| return () => { | |
| if (debounceRef.current) clearTimeout(debounceRef.current) | |
| } | |
| }, [query]) | |
| useEffect(() => { | |
| if (debounceRef.current) clearTimeout(debounceRef.current) | |
| if (!query.trim()) { | |
| setResponse(null) | |
| setError(null) | |
| return | |
| } | |
| debounceRef.current = setTimeout(() => { | |
| void fetchResults(query) | |
| }, 300) | |
| return () => { | |
| if (debounceRef.current) clearTimeout(debounceRef.current) | |
| } | |
| }, [query, baseUrl]) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ui/src/panels/AskPanel.tsx` around lines 49 - 64, The useEffect that
debounces calls to fetchResults references baseUrl via the fetchResults closure
but doesn't include baseUrl in its dependency array; update the effect's
dependencies to include baseUrl (in addition to query) so the effect re-runs
when baseUrl changes, ensuring fetchResults uses the latest baseUrl; locate the
useEffect block that references debounceRef, query, setResponse, setError and
fetchResults and add baseUrl to the dependency array.
| const pct = total > 0 ? Math.round((stage / total) * 100) : 0 | ||
| return ( | ||
| <div className="cm-stage-bar" title={`Stage ${stage} of ${total}`}> | ||
| <div className="cm-stage-fill" style={{ width: `${pct}%` }} /> | ||
| </div> |
There was a problem hiding this comment.
Clamp stage progress to a safe range.
On Line 21, pct can exceed 100 (or go negative) when stage metadata is out of bounds, causing visual overflow on Line 24.
Suggested fix
function StageBar({ stage, total }: { stage: number; total: number }) {
- const pct = total > 0 ? Math.round((stage / total) * 100) : 0
+ const rawPct = total > 0 ? Math.round((stage / total) * 100) : 0
+ const pct = Math.max(0, Math.min(100, rawPct))
return (
<div className="cm-stage-bar" title={`Stage ${stage} of ${total}`}>
<div className="cm-stage-fill" style={{ width: `${pct}%` }} />
</div>
)
}Also applies to: 42-47
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ui/src/panels/CognitiveMeshPanel.tsx` around lines 21 - 25, Clamp
the computed progress percentage to the 0–100 range to avoid visual overflow:
when computing pct (currently const pct = total > 0 ? Math.round((stage / total)
* 100) : 0), wrap the result with Math.max(0, Math.min(100, ...)) or a small
clamp helper so negative or >100 values are constrained; update both occurrences
of this computation (the pct used in the cm-stage-fill width and the equivalent
calculation around lines 42-47) so the inline style width always receives a
value between "0%" and "100%".
| function formatAge(iso: string): string { | ||
| const diffMs = Date.now() - new Date(iso).getTime() | ||
| const mins = Math.floor(diffMs / 60_000) | ||
| if (mins < 1) return 'just now' | ||
| if (mins < 60) return `${mins}m ago` | ||
| return `${Math.floor(mins / 60)}h ago` | ||
| } |
There was a problem hiding this comment.
Handle invalid timestamps in formatAge.
On Line 144, invalid ISO input yields NaN, which can render broken text like NaNh ago.
Suggested fix
function formatAge(iso: string): string {
- const diffMs = Date.now() - new Date(iso).getTime()
+ const ts = Date.parse(iso)
+ if (Number.isNaN(ts)) return 'unknown'
+ const diffMs = Math.max(0, Date.now() - ts)
const mins = Math.floor(diffMs / 60_000)
if (mins < 1) return 'just now'
if (mins < 60) return `${mins}m ago`
return `${Math.floor(mins / 60)}h ago`
}📝 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.
| function formatAge(iso: string): string { | |
| const diffMs = Date.now() - new Date(iso).getTime() | |
| const mins = Math.floor(diffMs / 60_000) | |
| if (mins < 1) return 'just now' | |
| if (mins < 60) return `${mins}m ago` | |
| return `${Math.floor(mins / 60)}h ago` | |
| } | |
| function formatAge(iso: string): string { | |
| const ts = Date.parse(iso) | |
| if (Number.isNaN(ts)) return 'unknown' | |
| const diffMs = Math.max(0, Date.now() - ts) | |
| const mins = Math.floor(diffMs / 60_000) | |
| if (mins < 1) return 'just now' | |
| if (mins < 60) return `${mins}m ago` | |
| return `${Math.floor(mins / 60)}h ago` | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ui/src/panels/CognitiveMeshPanel.tsx` around lines 143 - 149, The
formatAge function can produce NaN-based text for invalid ISO strings; update
formatAge to validate the parsed timestamp (e.g., const ts = new
Date(iso).getTime()) and if Number.isNaN(ts) return a safe fallback like
'unknown' (or ''), and also handle negative diffs (future timestamps) by
clamping to 'just now'; keep the existing mins/hours logic using the validated
ts/diffMs.
| .cm-health { | ||
| font-size: 11px; | ||
| padding: 1px 7px; | ||
| border-radius: 10px; | ||
| border: 1px solid currentColor; | ||
| } |
There was a problem hiding this comment.
Fix the CSS keyword casing to satisfy configured lint rules.
Line 330 uses currentColor, but your Stylelint config expects lowercase keywords, so this will fail lint in CI.
Proposed fix
.cm-health {
font-size: 11px;
padding: 1px 7px;
border-radius: 10px;
- border: 1px solid currentColor;
+ border: 1px solid currentcolor;
}📝 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.
| .cm-health { | |
| font-size: 11px; | |
| padding: 1px 7px; | |
| border-radius: 10px; | |
| border: 1px solid currentColor; | |
| } | |
| .cm-health { | |
| font-size: 11px; | |
| padding: 1px 7px; | |
| border-radius: 10px; | |
| border: 1px solid currentcolor; | |
| } |
🧰 Tools
🪛 Stylelint (17.5.0)
[error] 330-330: Expected "currentColor" to be "currentcolor" (value-keyword-case)
(value-keyword-case)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ui/src/styles/index.css` around lines 326 - 331, The CSS rule for
the .cm-health class uses the keyword "currentColor" which violates Stylelint's
lowercase keyword rule; update the border declaration in the .cm-health rule
(the line containing border: 1px solid currentColor) to use the lowercase
keyword "currentcolor" so the property becomes border: 1px solid currentcolor
and satisfies the linting config.
…k endpoint + AskPanel - packages/router: new workspace package with keyword-based team routing - buildIndex() tokenises team name/focus/scope/accepts into a RouterIndex - search() scores query token overlap against index entries (0–1 confidence) - route() enforces scope (off-topic → scopeViolation: true), returns RouteResult[] - 14 unit tests, all passing - packages/state-watcher: HTTP server alongside WebSocket (same port) - Refactored createServer() to use http.createServer + ws attached to it - GET /api/ask?q=<query> → AskResponse (< 1ms, no ML model) - GET /health liveness probe - Router index built on startup and refreshed on teams file change - packages/ui: AskPanel + keyboard shortcut - AskPanel: debounced fetch to /api/ask, confidence bars, scope-violation message - Ctrl/Cmd+Shift+? keyboard shortcut to open Ask tab - Added 'ask' to ActivePanel union and TABS list - root: add packages/router to workspaces, .gitignore, CI step for router Closes #1 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
cabb418 to
9f90ad4
Compare
Summary
Implements Phase 1 of issue #1 — keyword-based team routing with no external ML dependencies.
packages/router— new workspace packagebuildIndex()tokenises team name/focus/scope/accepts into aRouterIndexsearch()computes keyword overlap confidence (0–1) against index entriesroute()enforces scope guard (off-topic queries →scopeViolation: true)packages/state-watcher— HTTP server added alongside WebSocket (same port)GET /api/ask?q=<query>→AskResponse(< 1ms, pure in-process)GET /healthliveness probepackages/ui—AskPanel+ keyboard shortcut/api/ask, confidence bars, scope-violation messageCtrl/Cmd+Shift+?opens the Ask tab'ask'added toActivePanelunion and tab barpackages/routeradded to workspaces,.gitignorecreated, CI step addedTest plan
npm run test -w @retort-plugins/router— 14 tests passnpm run typecheck --workspaces— zero errorsnpm run build(router → state-watcher → ui → vscode) — all succeedPhase 2 (follow-on)
Semantic embeddings via
@xenova/transformers(all-MiniLM-L6-v2) + CI workflow to regenerateembeddings.jsonweekly.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
Chores