Repository navigation
feat: scaffold Nexus Agent core loop - #3
Himanshunashtech wants to merge 4 commits into
Conversation
📝 WalkthroughWalkthroughChangesNexus adds typed task classification, model routing, cost governance, connector authorization, local persistence and memory, training planning, CLI normalization, and a budget-gated dry-run orchestrator with build scripts and tests. Nexus orchestration
Connector platform
Local learning and training
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant normalizeCliInput
participant NexusOrchestrator
participant classifyTask
participant routeModel
participant CostGovernor
participant InMemoryCostLedger
CLI->>normalizeCliInput: raw arguments
normalizeCliInput-->>CLI: InternalMessage
CLI->>NexusOrchestrator: run(message)
NexusOrchestrator->>classifyTask: message text
classifyTask-->>NexusOrchestrator: TaskClassification
NexusOrchestrator->>routeModel: classification
routeModel-->>NexusOrchestrator: ModelChoice and estimated cost
NexusOrchestrator->>CostGovernor: estimated cost
CostGovernor-->>NexusOrchestrator: allow or deny
NexusOrchestrator->>InMemoryCostLedger: approved cost entry
InMemoryCostLedger-->>NexusOrchestrator: ledger entries
NexusOrchestrator-->>CLI: AgentResult
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e30a33f823
ℹ️ 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".
| const governor = new CostGovernor({ | ||
| dailyCapUsd: options.dailyCapUsd ?? 5, | ||
| taskBudgetUsd: options.taskBudgetUsd ?? Math.max(0.01, classification.tokenBudget * 0.00001), | ||
| }); |
There was a problem hiding this comment.
Because the CostGovernor is allocated inside every run(), spentToday starts at zero for each task while the orchestrator's ledger persists across runs. When a caller reuses one NexusOrchestrator with dailyCapUsd set, multiple tasks that are each under the cap can all proceed even after their cumulative cost exceeds the daily cap; keep daily spend in shared orchestrator state or seed the governor from the ledger before checkBeforeCall.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
tsconfig.nexus.json (1)
9-11: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueKeep
DOM; only dropdeclarationif the.d.tsoutput is unused.src/nexus/gateway/normalize.tsrelies onglobalThis.crypto.randomUUID(), soDOMis still needed here. The Nexus build/test flow only consumes the emitted.jsfiles, sodeclarationlooks like unnecessary overhead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tsconfig.nexus.json` around lines 9 - 11, Update the tsconfig.nexus.json compiler options to retain the DOM library for globalThis.crypto.randomUUID() compatibility, and remove declaration generation because the Nexus build/test flow consumes only emitted JavaScript files.src/nexus/classifier/task-classifier.ts (1)
23-23: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
gemini_flashtier is defined but never wired through the pipeline.
gemini_flashexists inModelTierandMODEL_BY_TIER, but the classifier'ssuggestedModellogic never produces it, making the router'sgemini_flashbranch dead code and the cost ledger's lack of Gemini-specific rates a latent defect. All three sites share this single root cause.
src/nexus/classifier/task-classifier.ts#L23: Add classification logic to produce'gemini_flash'for appropriate tasks (e.g., high-volume trivial/simple tasks), or removegemini_flashfromModelTieruntil it's needed (YAGNI).src/nexus/router/model-router.ts#L15-L16: Theclassification.suggestedModel === 'gemini_flash'branch is unreachable. Ifgemini_flashis retained, this branch will activate once the classifier produces it; if removed, delete this branch.src/nexus/ledger/cost-ledger.ts#L19-L27:estimateCostUsdfalls through to haiku rates ($0.8/$4) forgemini-2.0-flash. Gemini 2.0 Flash has significantly different pricing. Add a Gemini-specific rate tier ifgemini_flashis retained.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/nexus/classifier/task-classifier.ts` at line 23, Wire the existing gemini_flash tier through all three sites: update suggestedModel in task classification to assign gemini_flash for appropriate high-volume trivial/simple tasks, retain the corresponding branch in model-router.ts, and add Gemini-specific pricing in estimateCostUsd for gemini-2.0-flash instead of falling through to haiku rates. Apply these changes in src/nexus/classifier/task-classifier.ts:23, src/nexus/router/model-router.ts:15-16, and src/nexus/ledger/cost-ledger.ts:19-27.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@package.json`:
- Around line 10-11: Update the test:nexus script in package.json to remove the
Unix-only rm -rf and shell-dependent unquoted glob. Use the repository’s
cross-platform cleanup utility, such as rimraf, and quote the
tests/nexus/*.test.mjs pattern so Node handles test discovery consistently
across platforms.
In `@src/nexus/gateway/cli.ts`:
- Around line 5-13: Update the CLI entry point around main so errors from
normalizeCliInput and NexusOrchestrator.run are caught instead of discarded by
void main(). Write a concise error message to stderr and terminate with a
nonzero exit code, while preserving the existing successful JSON output
behavior.
In `@src/nexus/orchestrator/state-machine.ts`:
- Around line 18-21: Make CostGovernor state persist across run() invocations so
daily spending accumulates consistently with the class-level ledger. Hoist the
governor to a class-level field or persist its spentToday value, then reset
spentThisTask and update taskBudgetUsd at the start of each run() without
resetting spentToday.
In `@src/nexus/router/model-router.ts`:
- Around line 3-8: Update the model and tier entries in MODEL_BY_TIER to use
currently supported provider model IDs, replacing the retired Claude Sonnet,
Claude Opus, and Gemini Flash identifiers while preserving their providers,
tiers, and maxOutputTokens settings.
---
Nitpick comments:
In `@src/nexus/classifier/task-classifier.ts`:
- Line 23: Wire the existing gemini_flash tier through all three sites: update
suggestedModel in task classification to assign gemini_flash for appropriate
high-volume trivial/simple tasks, retain the corresponding branch in
model-router.ts, and add Gemini-specific pricing in estimateCostUsd for
gemini-2.0-flash instead of falling through to haiku rates. Apply these changes
in src/nexus/classifier/task-classifier.ts:23,
src/nexus/router/model-router.ts:15-16, and
src/nexus/ledger/cost-ledger.ts:19-27.
In `@tsconfig.nexus.json`:
- Around line 9-11: Update the tsconfig.nexus.json compiler options to retain
the DOM library for globalThis.crypto.randomUUID() compatibility, and remove
declaration generation because the Nexus build/test flow consumes only emitted
JavaScript files.
🪄 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: 7d04b9c9-b968-43fc-851a-e385107b8ca8
📒 Files selected for processing (12)
package.jsonsrc/nexus/classifier/task-classifier.tssrc/nexus/gateway/cli.tssrc/nexus/gateway/normalize.tssrc/nexus/governor/cost-governor.tssrc/nexus/index.tssrc/nexus/ledger/cost-ledger.tssrc/nexus/orchestrator/state-machine.tssrc/nexus/router/model-router.tssrc/nexus/types/index.tstests/nexus/nexus.test.mjstsconfig.nexus.json
| "typecheck:nexus": "tsc -p tsconfig.nexus.json --noEmit", | ||
| "test:nexus": "rm -rf .tmp/nexus-build && tsc -p tsconfig.nexus.json && node --test tests/nexus/*.test.mjs" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if rimraf or shx is already a dependency
rg -n '"rimraf"|"shx"' package.json
# Check if CI runs on Windows
fd -i 'ci' .github/workflows/ --type f -x cat {} \; 2>/dev/null | grep -i 'windows\|os:' || trueRepository: Himanshunashtech/app
Length of output: 158
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## package.json\n'
cat -n package.json | sed -n '1,80p'
printf '\n## workflow files\n'
fd -a . .github/workflows -t f 2>/dev/null || true
printf '\n## workflow snippets mentioning OS\n'
for f in $(fd -a . .github/workflows -t f 2>/dev/null || true); do
echo "### $f"
rg -n -i 'windows|ubuntu|macos|os:' "$f" || true
done
printf '\n## nexus tests\n'
fd -a . tests/nexus -t f 2>/dev/null || trueRepository: Himanshunashtech/app
Length of output: 2809
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## README and docs mentions of Windows / cross-platform\n'
for f in $(git ls-files | rg '(^|/)(README|readme|docs?/.*|.*\.md)$'); do
rg -n -i 'windows|cross-platform|cross platform|cmd\.exe|powershell|posix' "$f" || true
done
printf '\n## package scripts and toolchain references\n'
rg -n '"scripts"|cross-env|rimraf|shx|node --test|test:nexus|rm -rf|\.test\.mjs' package.json tests -g '!**/node_modules/**' || trueRepository: Himanshunashtech/app
Length of output: 531
🌐 Web query:
Node.js node --test glob patterns shell expansion Windows cmd.exe npm scripts
💡 Result:
When using the Node.js test runner (node --test) with glob patterns, it is critical to enclose the patterns in double quotes. This prevents the host shell (such as cmd.exe on Windows, or shells like bash or zsh on Unix-like systems) from performing glob expansion before the arguments reach Node.js [1][2][3]. If a shell expands the glob (e.g., tests/*.js becomes tests/a.js tests/b.js before Node.js receives the command), behavior becomes inconsistent and non-portable across different environments, especially between Windows cmd.exe and POSIX shells [2][4]. By using double quotes, you pass the raw pattern string to Node.js, allowing the built-in test runner to handle the glob matching natively and consistently [1][5]. Key points for npm scripts: 1. Use Double Quotes: Always wrap your glob patterns in double quotes (e.g., "node --test \"tests//*.test.js\"") within your package.json scripts [2]. Note that inside a package.json file, you must escape the inner double quotes with a backslash. 2. Version Requirements: Native glob support for node --test was introduced in Node.js v21 [4][5]. If you are using an older version of Node.js (e.g., v18 or v20), native glob patterns will not work, and the runner may fail to find files or behave unexpectedly [4]. 3. Cross-Platform Consistency: Relying on the shell to perform expansion is considered an anti-pattern for portability [2]. Relying on Node.js to handle the glob ensures that your test command behaves the same way regardless of the operating system or the shell being used [1][3]. Example of a robust npm script: "scripts": { "test": "node --test "tests//*.test.js"" }
Citations:
- 1: https://nodejs.org/dist/latest/docs/api/test.html
- 2: test runner: not execute the glob pattern of files correctly nodejs/node#52191
- 3: https://github.com/nodejs/node/blob/main/doc/api/test.md
- 4: fix: make npm test run on Windows (node <21) DietrichGebert/ponytail#118
- 5: https://nodejs.org/learn/test-runner/using-test-runner
🌐 Web query:
Node.js test runner glob patterns are shell expanded npm scripts Windows cmd.exe literal wildcard
💡 Result:
When using the Node.js test runner with glob patterns, you should always enclose the patterns in double quotes (e.g., "/*.test.js") [1][2]. This practice is critical for cross-platform portability because it prevents the underlying shell from attempting to expand the glob pattern before it reaches Node.js [1][2]. On POSIX-like systems, shells (like /bin/sh or bash) may perform glob expansion, which can lead to inconsistent behavior depending on the shell's specific configuration and support for features like globstars [3]. On Windows, while cmd.exe does not natively perform glob expansion [4], relying on this behavior is fragile; if the shell does not expand the pattern, the literal string is passed to Node.js, which handles the glob expansion internally [2]. By quoting the pattern, you ensure that the raw glob string is consistently passed to the Node.js test runner, allowing Node.js to handle the expansion reliably across all operating systems [1][2]. If you find that your glob patterns are not working as expected in npm scripts, it is often because the shell has either incorrectly expanded them or passed them in a way that Node.js cannot interpret correctly [5][4]. Always quoting your arguments in your package.json scripts is the standard recommended solution to maintain cross-platform compatibility [1][4].
Citations:
- 1: https://nodejs.org/api/test.html
- 2: https://nodejs.org/dist/latest/docs/api/test.html
- 3: How to run the Node built-in testrunner for TypeScript files inside a specific directory? nodejs/help#3902
- 4: https://medium.com/@jakubsynowiec/you-should-always-quote-your-globs-in-npm-scripts-621887a2a784
- 5: node:test --test with glob patterns and npm can't find files nodejs/node#50658
Make test:nexus cross-platform (package.json:11) rm -rf is Unix-only, and the unquoted tests/nexus/*.test.mjs glob depends on shell expansion. Use rimraf/fs.rm and a quoted node --test pattern or explicit file list.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@package.json` around lines 10 - 11, Update the test:nexus script in
package.json to remove the Unix-only rm -rf and shell-dependent unquoted glob.
Use the repository’s cross-platform cleanup utility, such as rimraf, and quote
the tests/nexus/*.test.mjs pattern so Node handles test discovery consistently
across platforms.
| async function main(): Promise<void> { | ||
| const task = process.argv.slice(2).join(' '); | ||
| const message = normalizeCliInput(task); | ||
| const orchestrator = new NexusOrchestrator(); | ||
| const result = await orchestrator.run(message); | ||
| process.stdout.write(`${JSON.stringify(result, null, 2)}\n`); | ||
| } | ||
|
|
||
| void main(); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
CLI lacks error handling — unhandled promise rejections on the most common error path.
void main() discards the promise. When normalizeCliInput throws on empty input (the default path for users running the CLI with no arguments) or orchestrator.run rejects, the error surfaces as an unhandled promise rejection rather than a clean stderr message with a proper exit code.
🔒 Proposed fix: wrap main in try/catch with stderr and exit code
async function main(): Promise<void> {
- const task = process.argv.slice(2).join(' ');
- const message = normalizeCliInput(task);
- const orchestrator = new NexusOrchestrator();
- const result = await orchestrator.run(message);
- process.stdout.write(`${JSON.stringify(result, null, 2)}\n`);
+ try {
+ const task = process.argv.slice(2).join(' ');
+ const message = normalizeCliInput(task);
+ const orchestrator = new NexusOrchestrator();
+ const result = await orchestrator.run(message);
+ process.stdout.write(`${JSON.stringify(result, null, 2)}\n`);
+ } catch (error) {
+ process.stderr.write(`${error instanceof Error ? error.message : String(error)}\n`);
+ process.exit(1);
+ }
}📝 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.
| async function main(): Promise<void> { | |
| const task = process.argv.slice(2).join(' '); | |
| const message = normalizeCliInput(task); | |
| const orchestrator = new NexusOrchestrator(); | |
| const result = await orchestrator.run(message); | |
| process.stdout.write(`${JSON.stringify(result, null, 2)}\n`); | |
| } | |
| void main(); | |
| async function main(): Promise<void> { | |
| try { | |
| const task = process.argv.slice(2).join(' '); | |
| const message = normalizeCliInput(task); | |
| const orchestrator = new NexusOrchestrator(); | |
| const result = await orchestrator.run(message); | |
| process.stdout.write(`${JSON.stringify(result, null, 2)}\n`); | |
| } catch (error) { | |
| process.stderr.write(`${error instanceof Error ? error.message : String(error)}\n`); | |
| process.exit(1); | |
| } | |
| } | |
| void main(); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/nexus/gateway/cli.ts` around lines 5 - 13, Update the CLI entry point
around main so errors from normalizeCliInput and NexusOrchestrator.run are
caught instead of discarded by void main(). Write a concise error message to
stderr and terminate with a nonzero exit code, while preserving the existing
successful JSON output behavior.
| const governor = new CostGovernor({ | ||
| dailyCapUsd: options.dailyCapUsd ?? 5, | ||
| taskBudgetUsd: options.taskBudgetUsd ?? Math.max(0.01, classification.tokenBudget * 0.00001), | ||
| }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Governor is instantiated per-call, defeating the daily cost cap.
A new CostGovernor is created inside run(), so spentToday resets to 0 on every invocation. The dailyCapUsd option implies cross-task accumulation, but it effectively becomes a per-task cap. Meanwhile, this.ledger is class-level and persists across calls — the two are inconsistent. The daily budget enforcement is non-functional for multi-task scenarios.
The fix requires making the governor (or at least the daily spending tracker) a class-level field, while resetting spentThisTask and updating taskBudgetUsd per task.
🔧 Proposed direction
export class NexusOrchestrator {
private readonly ledger = new InMemoryCostLedger();
+ private readonly dailySpent = { total: 0 };
async run(message: InternalMessage, options: OrchestratorOptions = {}): Promise<AgentResult> {
const classification = classifyTask(message.text);
const model = routeModel(classification);
- const governor = new CostGovernor({
+ const governor = new CostGovernor({
dailyCapUsd: options.dailyCapUsd ?? 5,
taskBudgetUsd: options.taskBudgetUsd ?? Math.max(0.01, classification.tokenBudget * 0.00001),
+ spentToday: this.dailySpent.total,
+ });This requires adding a spentToday initializer to CostGovernorOptions so the governor picks up accumulated daily spend. Alternatively, hoist the entire governor to a class field and add a resetTaskBudget(budget: number) method called at the start of each run().
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/nexus/orchestrator/state-machine.ts` around lines 18 - 21, Make
CostGovernor state persist across run() invocations so daily spending
accumulates consistently with the class-level ledger. Hoist the governor to a
class-level field or persist its spentToday value, then reset spentThisTask and
update taskBudgetUsd at the start of each run() without resetting spentToday.
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (6)
.env.example (1)
1-19: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider alphabetizing keys within each section.
dotenv-linter flags 7
UnorderedKeywarnings. Keys within each grouped section should be sorted alphabetically (e.g.,DEEPSEEK_API_KEYbeforeGOOGLE_GENERATIVE_AI_API_KEY,LM_STUDIO_BASE_URLbeforeOLLAMA_BASE_URL,EXPO_PUBLIC_SUPABASE_ANON_KEYbeforeEXPO_PUBLIC_SUPABASE_URL).♻️ Suggested alphabetical ordering
# Nexus Agent local/self-hosted model endpoints. -LM_STUDIO_BASE_URL= OLLAMA_BASE_URL=http://localhost:11434 +LM_STUDIO_BASE_URL= # Nexus Agent provider keys. Configure any subset; the router picks the cheapest # available model for the requested task tier and falls back to metadata-only # dry runs when no key is present. +DEEPSEEK_API_KEY= +GOOGLE_GENERATIVE_AI_API_KEY= +GROQ_API_KEY= +MISTRAL_API_KEY= ANTHROPIC_API_KEY= -OPENAI_API_KEY= -GOOGLE_GENERATIVE_AI_API_KEY= -MISTRAL_API_KEY= -XAI_API_KEY= -DEEPSEEK_API_KEY= -GROQ_API_KEY= OPENROUTER_API_KEY= +XAI_API_KEY= # Existing Expo/Supabase app settings. -EXPO_PUBLIC_SUPABASE_URL= EXPO_PUBLIC_SUPABASE_ANON_KEY= +EXPO_PUBLIC_SUPABASE_URL=🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.env.example around lines 1 - 19, Alphabetize the environment variable keys within each section of .env.example: place LM_STUDIO_BASE_URL before OLLAMA_BASE_URL, sort the provider keys alphabetically, and place EXPO_PUBLIC_SUPABASE_ANON_KEY before EXPO_PUBLIC_SUPABASE_URL. Preserve the existing section grouping and comments.Source: Linters/SAST tools
src/nexus/connectors/types.ts (1)
13-15: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSeveral union members are never produced by the catalog.
ConnectorAuthModeincludes'no_auth_public_read', andConnectorRiskincludes'draft_only','financial', and'destructive', butauthModesFor()andbuildDefinition()inapp-catalog.tsnever emit any of these. If these are intentional placeholders for future phases, consider documenting that; otherwise they create a dead type surface that misleads consumers about what the system actually supports.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/nexus/connectors/types.ts` around lines 13 - 15, Align ConnectorAuthMode and ConnectorRisk with the values actually emitted by authModesFor() and buildDefinition() in app-catalog.ts by removing unsupported union members. If those members are intentional future placeholders, document that explicitly instead of changing the type surface.src/nexus/connectors/consent-policy.ts (1)
4-4: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
EXTERNAL_WRITE_ACTIONSduplicatesWRITE_ACTIONSfromapp-catalog.ts.Both sets contain the same four actions:
['send_message', 'publish_post', 'create_task', 'update_record']. If one is updated and the other isn't, the consent policy and the catalog's risk assignment will drift. ExportWRITE_ACTIONSfromapp-catalog.tsand import it here, or extract both to a shared constant.♻️ Proposed refactor: share the write-actions constant
In
app-catalog.ts:-const WRITE_ACTIONS = new Set(['send_message', 'publish_post', 'create_task', 'update_record']); +export const WRITE_ACTIONS = new Set(['send_message', 'publish_post', 'create_task', 'update_record']);In
consent-policy.ts:-import { findConnector } from './app-catalog'; +import { findConnector, WRITE_ACTIONS } from './app-catalog'; import type { DelegatedActionDecision, DelegatedActionRequest } from './types'; -const EXTERNAL_WRITE_ACTIONS = new Set(['send_message', 'publish_post', 'create_task', 'update_record']); - export function evaluateDelegatedAction(request: DelegatedActionRequest): DelegatedActionDecision { const connector = findConnector(request.connectorId); if (!connector) { return deny('Unknown connector.', false); } if (!connector.actions.includes(request.action)) { return deny(`Connector ${connector.id} does not support ${request.action}.`, false); } if (!request.credentialRef) { return deny('A credential reference is required. Nexus does not bypass third-party authentication.', true); } - if (EXTERNAL_WRITE_ACTIONS.has(request.action) && !request.consentToken) { + if (WRITE_ACTIONS.has(request.action) && !request.consentToken) { return deny('Human consent token is required before Nexus can send or publish on behalf of a user.', true); } - if (connector.authModes.includes('no_auth_public_read') && EXTERNAL_WRITE_ACTIONS.has(request.action)) { + if (connector.authModes.includes('no_auth_public_read') && WRITE_ACTIONS.has(request.action)) { return deny('Unauthenticated connectors are limited to public read-only operations.', true); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/nexus/connectors/consent-policy.ts` at line 4, Remove the duplicate EXTERNAL_WRITE_ACTIONS definition and reuse the shared WRITE_ACTIONS constant from app-catalog.ts. Export WRITE_ACTIONS from app-catalog.ts, import it in consent-policy.ts, and update the consent policy references to use that imported constant so both policies remain synchronized.src/nexus/connectors/mcp-bridge.ts (1)
45-62: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe
actionenum is hardcoded and duplicates theConnectorActiontype.The enum at Line 55 manually lists all
ConnectorActionvalues. If the type changes, this schema won't automatically reflect it, risking drift. Additionally, this enum advertisescreate_taskandupdate_recordto MCP clients even though no connector in the catalog supports them (seeapp-catalog.tsreview). Consider deriving the enum from the type or a shared constant.♻️ Proposed refactor: derive action enum from a shared constant
In
types.tsor a shared constants file:+export const CONNECTOR_ACTIONS: readonly string[] = [ + 'read', 'search', 'draft', 'send_message', 'publish_post', 'create_task', 'update_record', +] as const;In
mcp-bridge.ts:- action: { type: 'string', enum: ['read', 'search', 'draft', 'send_message', 'publish_post', 'create_task', 'update_record'] }, + action: { type: 'string', enum: [...CONNECTOR_ACTIONS] },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/nexus/connectors/mcp-bridge.ts` around lines 45 - 62, Update buildRequestDelegatedActionTool to use the shared ConnectorAction values or canonical action constant for inputSchema.properties.action.enum instead of a hardcoded list. Ensure the shared source reflects only actions supported by the connector catalog, excluding unsupported create_task and update_record values, so the MCP schema stays synchronized with available connectors.tests/nexus/connectors.test.mjs (1)
1-47: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing test coverage for critical consent-policy denial paths.
The tests cover the happy path (allowed send) and one denial (missing credential), but several important paths are untested:
- Unknown connector ID → should deny with
requiresHumanConfirmation: false.- Unsupported action for a valid connector (e.g.,
create_taskoncredentialRefpresent butconsentTokenmissing for an external write → should deny with consent message.- Read action without
credentialRef→ should deny (credential required for all actions).no_auth_public_readconnector with external write → should deny (once such a connector exists).These are the authorization boundaries that protect against unauthorized delegated actions; they should have explicit test coverage.
♻️ Proposed additional tests
+test('delegated action denied for unknown connector', () => { + const decision = evaluateDelegatedAction({ + userId: 'user_123', + connectorId: 'nonexistent_app', + action: 'read', + payload: {}, + credentialRef: 'vault://users/user_123/nonexistent_app', + }); + assert.equal(decision.allowed, false); + assert.equal(decision.requiresHumanConfirmation, false); +}); + +test('delegated action denied when connector does not support the action', () => { + const decision = evaluateDelegatedAction({ + userId: 'user_123', + connectorId: 'whatsapp', + action: 'publish_post', + payload: {}, + credentialRef: 'vault://users/user_123/whatsapp', + consentToken: 'consent_abc', + }); + assert.equal(decision.allowed, false); + assert.match(decision.reason, /does not support/); +}); + +test('external write denied with credential but without consent token', () => { + const decision = evaluateDelegatedAction({ + userId: 'user_123', + connectorId: 'whatsapp', + action: 'send_message', + payload: { text: 'hello' }, + credentialRef: 'vault://users/user_123/whatsapp', + }); + assert.equal(decision.allowed, false); + assert.match(decision.reason, /consent/i); +}); + +test('read action denied without credential reference', () => { + const decision = evaluateDelegatedAction({ + userId: 'user_123', + connectorId: 'whatsapp', + action: 'read', + payload: {}, + }); + assert.equal(decision.allowed, false); + assert.match(decision.reason, /credential/i); +});🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/nexus/connectors.test.mjs` around lines 1 - 47, Expand the consent-policy tests around evaluateDelegatedAction to cover unknown connectors, unsupported actions, missing consentToken despite a credentialRef, and read actions without a credentialRef, asserting denial and the specified requiresHumanConfirmation or consent-message behavior. Add the no_auth_public_read external-write case only when the catalog exposes such a connector, using its actual connector definition and preserving existing happy-path tests.src/nexus/connectors/app-catalog.ts (1)
67-71: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
paymentsconnectors are read-only withread_onlyrisk, butConnectorRiskincludes'financial'.The
paymentscategory is excluded from the writable list (Line 50), so all payment connectors (Stripe, PayPal, Adyen, etc.) only get['read', 'search']andrisk: 'read_only'. Given thatConnectorRiskdefines a'financial'level, payment connectors should arguably carryrisk: 'financial'even in read-only mode, to signal the sensitivity of the data involved. Consider adding a risk-override for thepaymentscategory.♻️ Proposed refactor: assign financial risk to payments
return { id, name, category, authModes: authModesFor(category), actions: actions as AppConnectorDefinition['actions'], - risk: actions.some((action) => WRITE_ACTIONS.has(action)) ? 'external_write' : 'read_only', + risk: category === 'payments' ? 'financial' + : actions.some((action) => WRITE_ACTIONS.has(action)) ? 'external_write' + : 'read_only',🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/nexus/connectors/app-catalog.ts` around lines 67 - 71, Update the payment connector risk assignment so the payments category uses the existing ConnectorRisk value 'financial', including when its capabilities remain read-only. Locate the category-based risk construction near authModesFor and add a payments-specific override while preserving current risk behavior for all other categories.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/nexus/connectors/app-catalog.ts`:
- Around line 48-65: Update buildDefinition so productivity connectors receive
create_task and CRM connectors receive update_record, while preserving
publish_post for social and send_message for other writable categories. Keep
these actions included in the writable action set so their existing risk, scope,
and consent handling remains consistent.
In `@src/nexus/connectors/consent-policy.ts`:
- Around line 14-22: Update the credentialRef validation in the consent-policy
flow to allow read-only actions (read and search) when connector.authModes
includes no_auth_public_read. Preserve the credentialRef requirement for all
other connector modes and actions, and keep the existing write-action and
consent-token checks unchanged.
In `@src/nexus/connectors/types.ts`:
- Around line 30-37: Update the DelegatedActionRequest interface to make
credentialRef required, aligning its type with the consent policy and MCP tool
schema. Keep the field as a string and remove its optionality; do not add a
conditional exemption path.
In `@src/nexus/router/model-router.ts`:
- Around line 14-15: Update the ModelChoice construction in the router flow to
determine the capped maxOutputTokens value before calculating estimatedCostUsd.
Pass that capped value, rather than estimatedOutputTokens, to
estimateRegistryEntryCost so the cost matches the returned
ModelChoice.maxOutputTokens and ledger accounting.
In `@src/nexus/training/self-improvement.ts`:
- Line 26: Update the record ID construction in the self-improvement training
flow to include a unique timestamp alongside signal.taskId and the
accepted/rejected status. Ensure each feedback signal receives a distinct ID so
repeated signals for the same task are preserved and counted independently.
---
Nitpick comments:
In @.env.example:
- Around line 1-19: Alphabetize the environment variable keys within each
section of .env.example: place LM_STUDIO_BASE_URL before OLLAMA_BASE_URL, sort
the provider keys alphabetically, and place EXPO_PUBLIC_SUPABASE_ANON_KEY before
EXPO_PUBLIC_SUPABASE_URL. Preserve the existing section grouping and comments.
In `@src/nexus/connectors/app-catalog.ts`:
- Around line 67-71: Update the payment connector risk assignment so the
payments category uses the existing ConnectorRisk value 'financial', including
when its capabilities remain read-only. Locate the category-based risk
construction near authModesFor and add a payments-specific override while
preserving current risk behavior for all other categories.
In `@src/nexus/connectors/consent-policy.ts`:
- Line 4: Remove the duplicate EXTERNAL_WRITE_ACTIONS definition and reuse the
shared WRITE_ACTIONS constant from app-catalog.ts. Export WRITE_ACTIONS from
app-catalog.ts, import it in consent-policy.ts, and update the consent policy
references to use that imported constant so both policies remain synchronized.
In `@src/nexus/connectors/mcp-bridge.ts`:
- Around line 45-62: Update buildRequestDelegatedActionTool to use the shared
ConnectorAction values or canonical action constant for
inputSchema.properties.action.enum instead of a hardcoded list. Ensure the
shared source reflects only actions supported by the connector catalog,
excluding unsupported create_task and update_record values, so the MCP schema
stays synchronized with available connectors.
In `@src/nexus/connectors/types.ts`:
- Around line 13-15: Align ConnectorAuthMode and ConnectorRisk with the values
actually emitted by authModesFor() and buildDefinition() in app-catalog.ts by
removing unsupported union members. If those members are intentional future
placeholders, document that explicitly instead of changing the type surface.
In `@tests/nexus/connectors.test.mjs`:
- Around line 1-47: Expand the consent-policy tests around
evaluateDelegatedAction to cover unknown connectors, unsupported actions,
missing consentToken despite a credentialRef, and read actions without a
credentialRef, asserting denial and the specified requiresHumanConfirmation or
consent-message behavior. Add the no_auth_public_read external-write case only
when the catalog exposes such a connector, using its actual connector definition
and preserving existing happy-path tests.
🪄 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: a1049e5a-5ba9-4e4d-a7f8-fe006b8da86f
📒 Files selected for processing (19)
.env.examplesrc/nexus/config/model-registry.tssrc/nexus/connectors/app-catalog.tssrc/nexus/connectors/consent-policy.tssrc/nexus/connectors/mcp-bridge.tssrc/nexus/connectors/types.tssrc/nexus/index.tssrc/nexus/ledger/cost-ledger.tssrc/nexus/local-store/machine-store.tssrc/nexus/local-store/memory-manager.tssrc/nexus/local-store/types.tssrc/nexus/orchestrator/state-machine.tssrc/nexus/router/model-router.tssrc/nexus/training/methods.tssrc/nexus/training/self-improvement.tssrc/nexus/types/index.tstests/nexus/connectors.test.mjstests/nexus/local-learning.test.mjstests/nexus/model-registry.test.mjs
✅ Files skipped from review due to trivial changes (4)
- tests/nexus/model-registry.test.mjs
- src/nexus/local-store/types.ts
- src/nexus/training/methods.ts
- src/nexus/local-store/memory-manager.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/nexus/orchestrator/state-machine.ts
| function buildDefinition(category: ConnectorCategory, name: string): AppConnectorDefinition { | ||
| const id = toConnectorId(name); | ||
| const writable = category === 'messaging' || category === 'social' || category === 'productivity' || category === 'crm' || category === 'marketing'; | ||
| const actions = writable ? ['read', 'search', 'draft', category === 'social' ? 'publish_post' : 'send_message'] : ['read', 'search']; | ||
|
|
||
| return { | ||
| id, | ||
| name, | ||
| category, | ||
| authModes: authModesFor(category), | ||
| actions: actions as AppConnectorDefinition['actions'], | ||
| risk: actions.some((action) => WRITE_ACTIONS.has(action)) ? 'external_write' : 'read_only', | ||
| mcpServerName: `nexus-${id}`, | ||
| officialApiBaseUrl: undefined, | ||
| requiredScopes: scopesFor(category, writable), | ||
| enabledByDefault: false, | ||
| }; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
create_task and update_record actions are never assigned to any connector.
buildDefinition maps writable categories to either publish_post (social) or send_message (everything else). No connector ever receives create_task or update_record. Yet these actions are declared in ConnectorAction, included in WRITE_ACTIONS (Line 36), enforced in EXTERNAL_WRITE_ACTIONS in consent-policy.ts (Line 4), and advertised in the MCP tool schema enum (mcp-bridge.ts Line 55). Clients consuming the MCP manifest will see create_task and update_record as valid actions, attempt them, and always receive a denial ("Connector X does not support create_task"). Productivity connectors (Asana, Jira, Todoist, etc.) should support create_task, and CRM connectors (Salesforce, HubSpot, etc.) should support update_record.
🐛 Proposed fix: map category-specific write actions
function buildDefinition(category: ConnectorCategory, name: string): AppConnectorDefinition {
const id = toConnectorId(name);
const writable = category === 'messaging' || category === 'social' || category === 'productivity' || category === 'crm' || category === 'marketing';
- const actions = writable ? ['read', 'search', 'draft', category === 'social' ? 'publish_post' : 'send_message'] : ['read', 'search'];
+ const writeAction =
+ category === 'social' ? 'publish_post'
+ : category === 'productivity' ? 'create_task'
+ : category === 'crm' ? 'update_record'
+ : 'send_message';
+ const actions = writable ? ['read', 'search', 'draft', writeAction] : ['read', 'search'];📝 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 buildDefinition(category: ConnectorCategory, name: string): AppConnectorDefinition { | |
| const id = toConnectorId(name); | |
| const writable = category === 'messaging' || category === 'social' || category === 'productivity' || category === 'crm' || category === 'marketing'; | |
| const actions = writable ? ['read', 'search', 'draft', category === 'social' ? 'publish_post' : 'send_message'] : ['read', 'search']; | |
| return { | |
| id, | |
| name, | |
| category, | |
| authModes: authModesFor(category), | |
| actions: actions as AppConnectorDefinition['actions'], | |
| risk: actions.some((action) => WRITE_ACTIONS.has(action)) ? 'external_write' : 'read_only', | |
| mcpServerName: `nexus-${id}`, | |
| officialApiBaseUrl: undefined, | |
| requiredScopes: scopesFor(category, writable), | |
| enabledByDefault: false, | |
| }; | |
| } | |
| function buildDefinition(category: ConnectorCategory, name: string): AppConnectorDefinition { | |
| const id = toConnectorId(name); | |
| const writable = category === 'messaging' || category === 'social' || category === 'productivity' || category === 'crm' || category === 'marketing'; | |
| const writeAction = | |
| category === 'social' ? 'publish_post' | |
| : category === 'productivity' ? 'create_task' | |
| : category === 'crm' ? 'update_record' | |
| : 'send_message'; | |
| const actions = writable ? ['read', 'search', 'draft', writeAction] : ['read', 'search']; | |
| return { | |
| id, | |
| name, | |
| category, | |
| authModes: authModesFor(category), | |
| actions: actions as AppConnectorDefinition['actions'], | |
| risk: actions.some((action) => WRITE_ACTIONS.has(action)) ? 'external_write' : 'read_only', | |
| mcpServerName: `nexus-${id}`, | |
| officialApiBaseUrl: undefined, | |
| requiredScopes: scopesFor(category, writable), | |
| enabledByDefault: false, | |
| }; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/nexus/connectors/app-catalog.ts` around lines 48 - 65, Update
buildDefinition so productivity connectors receive create_task and CRM
connectors receive update_record, while preserving publish_post for social and
send_message for other writable categories. Keep these actions included in the
writable action set so their existing risk, scope, and consent handling remains
consistent.
| if (!request.credentialRef) { | ||
| return deny('A credential reference is required. Nexus does not bypass third-party authentication.', true); | ||
| } | ||
| if (EXTERNAL_WRITE_ACTIONS.has(request.action) && !request.consentToken) { | ||
| return deny('Human consent token is required before Nexus can send or publish on behalf of a user.', true); | ||
| } | ||
| if (connector.authModes.includes('no_auth_public_read') && EXTERNAL_WRITE_ACTIONS.has(request.action)) { | ||
| return deny('Unauthenticated connectors are limited to public read-only operations.', true); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
The credentialRef requirement for all actions makes no_auth_public_read connectors unusable for reads.
Line 14 denies any request without credentialRef, regardless of connector auth mode or action type. If a future connector uses no_auth_public_read for public read-only operations (e.g., a public API), it would still be denied at Line 14 before the no_auth_public_read check at Line 20 is reached. Consider skipping the credentialRef requirement for no_auth_public_read connectors performing read-only actions (read, search).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/nexus/connectors/consent-policy.ts` around lines 14 - 22, Update the
credentialRef validation in the consent-policy flow to allow read-only actions
(read and search) when connector.authModes includes no_auth_public_read.
Preserve the credentialRef requirement for all other connector modes and
actions, and keep the existing write-action and consent-token checks unchanged.
| export interface DelegatedActionRequest { | ||
| userId: string; | ||
| connectorId: string; | ||
| action: ConnectorAction; | ||
| payload: Record<string, unknown>; | ||
| consentToken?: string; | ||
| credentialRef?: string; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
credentialRef is optional in the type but required by the consent policy for all actions.
DelegatedActionRequest.credentialRef is declared optional (credentialRef?: string), yet consent-policy.ts denies every request without a credentialRef (Line 14–16), and the MCP tool schema marks it required (mcp-bridge.ts Line 51). This type-level optionality misleads callers into thinking credential-less requests are valid for some connectors. Either make credentialRef required in the type, or add a conditional exemption path in the consent policy for no_auth_public_read connectors performing read-only actions.
🔧 Proposed fix: make credentialRef required
export interface DelegatedActionRequest {
userId: string;
connectorId: string;
action: ConnectorAction;
payload: Record<string, unknown>;
consentToken?: string;
- credentialRef?: string;
+ credentialRef: string;
}📝 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.
| export interface DelegatedActionRequest { | |
| userId: string; | |
| connectorId: string; | |
| action: ConnectorAction; | |
| payload: Record<string, unknown>; | |
| consentToken?: string; | |
| credentialRef?: string; | |
| } | |
| export interface DelegatedActionRequest { | |
| userId: string; | |
| connectorId: string; | |
| action: ConnectorAction; | |
| payload: Record<string, unknown>; | |
| consentToken?: string; | |
| credentialRef: string; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/nexus/connectors/types.ts` around lines 30 - 37, Update the
DelegatedActionRequest interface to make credentialRef required, aligning its
type with the consent policy and MCP tool schema. Keep the field as a string and
remove its optionality; do not add a conditional exemption path.
| const selected = selectCheapestAvailableModel(targetTier, estimatedInputTokens, estimatedOutputTokens, options.env); | ||
| const estimatedCostUsd = estimateRegistryEntryCost(selected, estimatedInputTokens, estimatedOutputTokens, true); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
estimatedCostUsd uses uncapped output tokens, inconsistent with maxOutputTokens.
estimatedCostUsd is computed from estimatedOutputTokens (line 14-15), but maxOutputTokens on the returned ModelChoice is capped at Math.min(selected.maxOutputTokens, …) (line 21). When the caller passes estimatedOutputTokens exceeding the computed maxOutputTokens (e.g., the orchestrator passes estimatedSteps * 160 which can exceed the cap), estimatedCostUsd will overstate the cost relative to what the orchestrator actually records in the ledger. Compute the cost after determining maxOutputTokens and use the capped value.
🔧 Proposed fix
export function routeModel(classification: TaskClassification, options: RouteModelOptions = {}): ModelChoice {
const targetTier = targetTierFor(classification);
const estimatedInputTokens = options.estimatedInputTokens ?? Math.ceil(classification.tokenBudget * 0.35);
const estimatedOutputTokens = options.estimatedOutputTokens ?? Math.ceil(classification.tokenBudget * 0.2);
const selected = selectCheapestAvailableModel(targetTier, estimatedInputTokens, estimatedOutputTokens, options.env);
- const estimatedCostUsd = estimateRegistryEntryCost(selected, estimatedInputTokens, estimatedOutputTokens, true);
+ const maxOutputTokens = Math.min(selected.maxOutputTokens, Math.max(512, Math.floor(classification.tokenBudget / 2)));
+ const effectiveOutputTokens = Math.min(maxOutputTokens, estimatedOutputTokens);
+ const estimatedCostUsd = estimateRegistryEntryCost(selected, estimatedInputTokens, effectiveOutputTokens, true);
return {
tier: selected.tier,
provider: selected.provider,
model: selected.model,
- maxOutputTokens: Math.min(selected.maxOutputTokens, Math.max(512, Math.floor(classification.tokenBudget / 2))),
+ maxOutputTokens,
rationale: [
`Targeted ${targetTier} for ${classification.complexity} ${classification.domain} task with ${classification.estimatedSteps} estimated step(s).`,
selected.available ? `Selected cheapest configured ${selected.provider} model.` : `No API key was configured; selected cheapest ${selected.provider} fallback metadata.`,
].join(' '),
apiKeyEnvVar: selected.envKey,
apiKeyConfigured: selected.available,
estimatedCostUsd,
};
}Also applies to: 21-21
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/nexus/router/model-router.ts` around lines 14 - 15, Update the
ModelChoice construction in the router flow to determine the capped
maxOutputTokens value before calculating estimatedCostUsd. Pass that capped
value, rather than estimatedOutputTokens, to estimateRegistryEntryCost so the
cost matches the returned ModelChoice.maxOutputTokens and ledger accounting.
| if (!feedback) throw new Error('Training feedback is required.'); | ||
|
|
||
| await this.store.put({ | ||
| id: `train_${signal.taskId}_${signal.accepted ? 'accepted' : 'rejected'}`, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Record ID collision overwrites previous feedback for the same task.
The ID train_${taskId}_${accepted|rejected} means a second feedback signal for the same task with the same accepted status silently overwrites the first. If multiple feedback rounds per task are expected, earlier feedback data is lost and the plan() event count is reduced, which could prevent a method from reaching its minimumExamples threshold.
🔧 Proposed fix: append timestamp to preserve all signals
async recordSignal(signal: TrainingSignal): Promise<void> {
const feedback = signal.feedback.trim();
if (!feedback) throw new Error('Training feedback is required.');
await this.store.put({
- id: `train_${signal.taskId}_${signal.accepted ? 'accepted' : 'rejected'}`,
+ id: `train_${signal.taskId}_${signal.accepted ? 'accepted' : 'rejected'}_${Date.now()}`,
kind: 'training_event',📝 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.
| id: `train_${signal.taskId}_${signal.accepted ? 'accepted' : 'rejected'}`, | |
| id: `train_${signal.taskId}_${signal.accepted ? 'accepted' : 'rejected'}_${Date.now()}`, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/nexus/training/self-improvement.ts` at line 26, Update the record ID
construction in the self-improvement training flow to include a unique timestamp
alongside signal.taskId and the accepted/rejected status. Ensure each feedback
signal receives a distinct ID so repeated signals for the same task are
preserved and counted independently.
1 participant
Motivation
Description
src/nexus/types, a lightweight task classifier insrc/nexus/classifier/task-classifier.ts, and a model router insrc/nexus/router/model-router.tsfor tiered routing (haiku/sonnet/opus/gemini_flash).CostGovernorinsrc/nexus/governor/cost-governor.ts, an in-memory cost ledger and estimator insrc/nexus/ledger/cost-ledger.ts, and a bounded dry-run orchestrator insrc/nexus/orchestrator/state-machine.tsthat wires classification, routing, pre-call budget checks, and ledger recording.src/nexus/gateway/normalize.tsandsrc/nexus/gateway/cli.tsfor local dry runs, and exported the Nexus surface viasrc/nexus/index.ts.tsconfig.nexus.json, Node tests undertests/nexus, andpackage.jsonscriptstypecheck:nexusandtest:nexusto typecheck and run the Nexus unit tests.Testing
npm run typecheck:nexuswhich executedtsc -p tsconfig.nexus.json --noEmitand completed successfully.npm run test:nexuswhich builds the Nexus artifacts and executednode --test tests/nexus/*.test.mjs, with all tests passing; tests cover the classifier, router, cost governor, normalizer, and orchestrator ledger behavior.npm run lintin this environment and it failed due to Expo attempting automatic ESLint setup and a remotefetcherror (environmental/network failure), which blocked lint from completing.Codex Task
Summary by CodeRabbit