Add MCP workflows and growth automation docs#48
Merged
Conversation
Contributor
|
Caution Review failedThe pull request is closed. 📝 WalkthroughSummary by CodeRabbit
WalkthroughTransitions IPC to a socket-based daemon protocol, implements an HTTP MCP bridge with retries/timeouts, enriches service profiles and health probes, rewrites the TUI to a snapshot/provider model, expands CLI doctor/hive-mind/daemon commands and tests, adds desktop-commander compose profile, and replaces MIT with AGPLv3. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI / App
participant Socket as IPC Socket
participant Daemon as Background Daemon
participant ServiceMgr as Service Manager
participant Bridge as MCP Bridge
participant MCP as MCP Service
CLI->>Socket: open connection / send JSON-line request
Socket->>Daemon: deliver request
Daemon->>ServiceMgr: probe/ensureService/status
ServiceMgr->>MCP: probe (HTTP/TCP) or notify via Bridge
MCP-->>ServiceMgr: health/response
ServiceMgr->>Daemon: return health/status
Daemon->>Daemon: update & persist runtime snapshot
Daemon->>Socket: respond (DaemonSocketResponse)
Socket-->>CLI: return snapshot + telemetry
alt MCPBridge retry flow
Daemon->>Bridge: sendMessage
Bridge->>MCP: POST (attempt 1 fails)
Bridge->>Bridge: backoff / retry
Bridge->>MCP: POST (attempt 2 succeeds)
MCP-->>Bridge: response
Bridge-->>Daemon: MCPBridgeResponse (ok=true)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ❌ 4❌ Failed checks (4 warnings)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Contributor
Summary of ChangesHello @clduab11, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly upgrades the project's operational capabilities by integrating advanced MCP-aware workflows and automation. It introduces a new licensing model, overhauls daemon management for a more robust and unified system state, and provides a new interactive terminal dashboard for live monitoring. Additionally, a comprehensive diagnostic tool has been added to ensure system readiness and proper configuration of MCP services, while the MCP bridge has been hardened for improved reliability in external communications. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
coderabbitai
Bot
added
ci/cd
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This is a substantial and high-quality pull request that significantly improves the project's architecture, operator experience, and documentation. The introduction of a daemon with socket-based communication, a comprehensive doctor command, and a TUI are excellent additions. The refactoring of the MCP bridge to be more robust with retries and timeouts is also a major improvement. The documentation updates are clear and make the new features easy to understand. I have one medium-severity suggestion regarding Docker image tagging for better reproducibility. Overall, this is an impressive set of changes.
![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: 9b9832d35e
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Contributor
Co-authored-by: clduab11 <185000089+clduab11@users.noreply.github.com>
Co-authored-by: clduab11 <185000089+clduab11@users.noreply.github.com>
Co-authored-by: clduab11 <185000089+clduab11@users.noreply.github.com>
Co-authored-by: clduab11 <185000089+clduab11@users.noreply.github.com>
Co-authored-by: clduab11 <185000089+clduab11@users.noreply.github.com>
Co-authored-by: clduab11 <185000089+clduab11@users.noreply.github.com>
Co-authored-by: clduab11 <185000089+clduab11@users.noreply.github.com>
Fix type safety, async I/O blocking, and error handling across core modules
Collaborator
Author
|
@copilot , reporting in that CodeFactor found multiple issues: Complex Method
Use of vulnerable @modelcontextprotocol/sdk (CVE-2025-66414) [update to 1.24.0]
|
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
- Update @modelcontextprotocol/sdk from 1.20.1 to 1.26.0 (fixes CVE-2025-66414) - Extract helper functions to reduce complexity in hive-mind spawn command - Create hive-mind-helpers.ts with GOAP, task execution, and result rendering functions - Create doctor-helpers.ts with health check functions - Refactor doctor command to use extracted helper functions - Reduce method complexity from 364+ lines to more maintainable chunks Co-authored-by: clduab11 <185000089+clduab11@users.noreply.github.com>
Co-authored-by: clduab11 <185000089+clduab11@users.noreply.github.com>
- Restore original auth check logic using negative test for "not logged in" - Change require() to dynamic import() for ES module consistency - Ensure completion message is always shown before YAML output - Make renderExecutionSummary async to support dynamic import Co-authored-by: clduab11 <185000089+clduab11@users.noreply.github.com>
Collaborator
Author
|
@copilot , In
In
Ensure no further conflicts remain, using the latest commit information to resolve any and all conflicts prior to push. |
Contributor
- Add ExecutionResultData interface for type safety - Add comprehensive JSDoc comments with @param and @returns tags - Remove unused checkMcpProfile function - Update function signatures to use proper types instead of any Co-authored-by: clduab11 <185000089+clduab11@users.noreply.github.com>
Co-authored-by: clduab11 <185000089+clduab11@users.noreply.github.com>
Replace generic Error with DaemonConflictError in split-brain guard
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Fix CVE-2025-66414 and reduce CLI method complexity
Collaborator
Author
|
@coderabbitai , resolve all conflicts in .codex-synaptic/memory.db, README.md, docs/runbooks/autoscaler-daemon-coordination.md, package-lock.json, src/cli/index.ts |
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 555 🏁 Script executed: Length of output: 2361 🏁 Script executed: Length of output: 10479 🏁 Script executed: Length of output: 1500
Conflict Resolution Plan1.
|
Collaborator
Author
@copilot , execute final fixes for the above. |
Contributor
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@src/cli/doctor-helpers.ts`:
- Around line 112-114: Add a specific error class and throw it instead of a
generic Error: define and export a DoctorCheckFailedError class (name should be
'DoctorCheckFailedError' and its constructor accepts failedCount and calls super
with `Doctor found ${failedCount} failing check(s).`) near the top of the file,
then replace the throw in the strict branch (the code that checks options.strict
&& failed > 0) to throw new DoctorCheckFailedError(failed) so callers can
distinguish doctor failures programmatically.
In `@src/cli/hive-mind-helpers.ts`:
- Around line 15-47: The ExecutionResultData interface is too permissive—replace
the broad any types (artifacts: Record<string, any>, stages: any[],
consensusStatus.votes?: any, totPlan?: any) with stricter types: define small
lightweight interfaces (e.g., ArtifactRecord, Stage, Vote, TotPlan) or use
unknown and require callers to perform type-narrowing; update
ExecutionResultData to reference these new types and add optional fields where
necessary (e.g., consensusStatus.votes?: Vote[]), ensuring codexContext and
mesh/consensus sub-objects remain typed.
- Around line 214-219: The consensus votes shape mismatch: orchestrateConsensus
supplies a numeric total in consensusStatus.votes but renderExecutionSummary
(and the logging here referencing resultData.consensusStatus.votes.for/against)
expects an object with .for/.against; update renderExecutionSummary and this
logging branch to accept either a numeric total or an object by detecting typeof
resultData.consensusStatus.votes === 'number' and treating that as total
votes-for (or as configured), otherwise use votes.for and votes.against; adjust
any code that constructs or reads consensusStatus.votes (e.g.,
orchestrateConsensus and renderExecutionSummary) to handle both shapes
consistently and include a clear comment about which shape is preferred.
- Around line 112-117: The Promise.race use can leak the timeout and cause
unhandled rejections; capture the timer id when creating the timeout promise
(store in a variable like timeoutId from setTimeout) and await the race inside a
try block, then in a finally block call clearTimeout(timeoutId) to cancel the
pending timer regardless of whether system.executeTask(prompt) or the timeout
settled; update the block referencing outcome, system.executeTask(prompt), and
config.timeout accordingly so the timeout is always cleared.
In `@src/cli/index.ts`:
- Around line 4241-4258: The async action for the CLI "status" command
(.command('status') ... .action(async (names?: string[]) => { ... })) can throw
unhandled rejections; wrap the async handler with the centralized error wrapper
handleCommand so all errors are caught; locate the .action call for the 'status'
command and replace the inline async handler with handleCommand(async (names?:
string[]) => { ... }), ensuring handleCommand is imported/available in this
module and that serviceManager.status and any await calls remain inside the
wrapped async function.
- Around line 4445-4475: Replace the blocking spawnSync usage that populates
codexMcpNames with the async execCodexCommand helper: call await
execCodexCommand('codex', ['mcp','list','--json']) (or wrap in a small async
helper function), then use the returned { stdout, stderr, code, timedOut } to
mirror the existing success/failure branches—on success JSON.parse stdout into
codexMcpNames (Set of entry.name), push the same checks entry, and on failure
use stderr/timedOut/code to populate the failing checks entry and remediation;
ensure you await execCodexCommand, catch JSON parse errors and thrown errors,
and preserve the same check ids ('codex.mcp_list') and messages.
🧹 Nitpick comments (5)
src/cli/doctor-helpers.ts (3)
42-56: Consider async execution and add timeout to prevent indefinite hangs.Two concerns here:
spawnSyncblocks the event loop. While this may be acceptable for a diagnostic CLI, the coding guidelines discourage blocking operations, and the PR objectives call for async patterns with timeouts.No timeout is specified—if
nodehangs, the doctor command hangs indefinitely.Additionally,
cliHelp.statuscan benullif the process fails to spawn (e.g.,nodenot found), which would makecliHelp.status === 0false but could benefit from explicit handling.♻️ Suggested async refactor with timeout
-import { spawnSync } from 'node:child_process'; +import { spawn } from 'node:child_process'; + +function execWithTimeout( + cmd: string, + args: string[], + timeoutMs = 10000 +): Promise<{ status: number | null; stdout: string; stderr: string }> { + return new Promise((resolve) => { + const proc = spawn(cmd, args, { cwd: process.cwd() }); + let stdout = ''; + let stderr = ''; + const timer = setTimeout(() => { + proc.kill(); + resolve({ status: null, stdout, stderr: 'Timed out' }); + }, timeoutMs); + proc.stdout?.on('data', (d) => (stdout += d)); + proc.stderr?.on('data', (d) => (stderr += d)); + proc.on('close', (code) => { + clearTimeout(timer); + resolve({ status: code, stdout, stderr }); + }); + proc.on('error', (err) => { + clearTimeout(timer); + resolve({ status: null, stdout, stderr: err.message }); + }); + }); +}Then convert
checkCliExecutionto async:-export function checkCliExecution(distCliPath: string): HealthCheck | null { +export async function checkCliExecution(distCliPath: string): Promise<HealthCheck | null> { if (!existsSync(distCliPath)) { return null; } - const cliHelp = spawnSync('node', [distCliPath, '--help'], { - cwd: process.cwd(), - encoding: 'utf8' - }); + const cliHelp = await execWithTimeout('node', [distCliPath, '--help']);As per coding guidelines: "Don't block the event loop with synchronous operations."
63-66: Same async/timeout concern applies tocheckCodexAuth.The
codex login statuscall usesspawnSyncwithout a timeout. If thecodexCLI is unresponsive or prompts for input, this will block indefinitely.Also consider handling the case where
codexis not installed—currentlyloginStatus.statuswould benulland the error message would come from stderr, but an explicit check could improve the user experience.♻️ Suggested improvement
-export function checkCodexAuth(): HealthCheck { - const loginStatus = spawnSync('codex', ['login', 'status'], { - cwd: process.cwd(), - encoding: 'utf8' - }); +export async function checkCodexAuth(): Promise<HealthCheck> { + const loginStatus = await execWithTimeout('codex', ['login', 'status']); + + if (loginStatus.status === null && loginStatus.stderr === 'Timed out') { + return { + id: 'codex.auth', + ok: false, + details: 'Codex login status check timed out.', + remediation: 'Ensure `codex` CLI is installed and responsive.' + }; + }
93-109: Consider structured Logger for consistency, or document the console choice.The coding guidelines specify using structured logging, but for CLI user-facing output (especially formatted/colored output like this),
console.logis often the pragmatic choice since Logger typically adds metadata prefixes.If this is intentional, a brief comment explaining why
consoleis used here (user-facing formatted output) would help future maintainers understand the decision.src/cli/index.ts (2)
4324-4345: Use a specific error type for Codex CLI failures.
execCodexCommandthrows genericErrorfor missing CLI and timeouts; a typed error helps callers/tests distinguish failure modes and aligns with the typed-error convention.
As per coding guidelines: "Throw specific error types, not generic Error objects".♻️ Suggested refactor
+class CodexCliError extends Error { + constructor( + message: string, + public readonly kind: 'not_found' | 'timeout' | 'non_zero', + public readonly exitCode?: number, + cause?: Error + ) { + super(message); + this.name = 'CodexCliError'; + if (cause) { + this.cause = cause; + } + } +} @@ - throw new Error('codex command not found. Ensure Codex CLI is installed and in PATH.'); + throw new CodexCliError( + 'codex command not found. Ensure Codex CLI is installed and in PATH.', + 'not_found', + undefined, + err as Error + ); @@ - throw new Error(`codex command timed out after ${timeoutMs}ms`); + throw new CodexCliError( + `codex command timed out after ${timeoutMs}ms`, + 'timeout' + );
4820-4824: UseDaemonConflictErrorfor the TUI split‑brain guard.This aligns with the
useSystemguard and lets callers/tests handle the conflict explicitly.
As per coding guidelines: "Throw specific error types, not generic Error objects".🛠️ Suggested fix
- throw new Error( - 'Background daemon is already running. Use `codex-synaptic tui --attach-daemon` ' + - 'or stop the daemon first to avoid split-brain orchestration.' - ); + throw new DaemonConflictError( + 'Background daemon is already running. Use `codex-synaptic tui --attach-daemon` ' + + 'or stop the daemon first to avoid split-brain orchestration.', + { + daemonPid: background.pid, + allowLocalWithDaemon: process.env.CODEX_ALLOW_LOCAL_WITH_DAEMON + } + );
Comment on lines
+112
to
+114
| if (options.strict && failed > 0) { | ||
| throw new Error(`Doctor found ${failed} failing check(s).`); | ||
| } |
Contributor
There was a problem hiding this comment.
Replace generic Error with a specific error class.
Per coding guidelines, specific error types should be thrown instead of generic Error objects. This allows callers to distinguish doctor failures from other errors programmatically.
🛠️ Proposed fix
Add near the top of the file:
export class DoctorCheckFailedError extends Error {
constructor(failedCount: number) {
super(`Doctor found ${failedCount} failing check(s).`);
this.name = 'DoctorCheckFailedError';
}
}Then update the throw:
if (options.strict && failed > 0) {
- throw new Error(`Doctor found ${failed} failing check(s).`);
+ throw new DoctorCheckFailedError(failed);
}As per coding guidelines: "Throw specific error types, not generic Error objects."
🤖 Prompt for AI Agents
In `@src/cli/doctor-helpers.ts` around lines 112 - 114, Add a specific error class
and throw it instead of a generic Error: define and export a
DoctorCheckFailedError class (name should be 'DoctorCheckFailedError' and its
constructor accepts failedCount and calls super with `Doctor found
${failedCount} failing check(s).`) near the top of the file, then replace the
throw in the strict branch (the code that checks options.strict && failed > 0)
to throw new DoctorCheckFailedError(failed) so callers can distinguish doctor
failures programmatically.
Comment on lines
+15
to
+47
| export interface ExecutionResultData { | ||
| executionId: string; | ||
| status: string; | ||
| duration: number; | ||
| originalPrompt: string; | ||
| summary?: string; | ||
| artifacts: Record<string, any>; | ||
| stages: any[]; | ||
| agentCount: number; | ||
| taskCount: number; | ||
| meshStatus: { | ||
| nodeCount: number; | ||
| connectionCount: number; | ||
| }; | ||
| consensusStatus: { | ||
| performed: boolean; | ||
| proposalId?: string; | ||
| accepted?: boolean; | ||
| votes?: any; | ||
| timedOut?: boolean; | ||
| error?: string; | ||
| }; | ||
| swarmStatus: { | ||
| algorithm: string; | ||
| isOptimizing: boolean; | ||
| }; | ||
| totPlan?: any; | ||
| codexContext: { | ||
| enabled: boolean; | ||
| contextHash?: string; | ||
| sizeBytes?: number; | ||
| }; | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Tighten ExecutionResultData typing to keep strict TS guarantees.
any in artifacts/stages/votes/totPlan weakens type safety and makes consumer logic fragile. Consider lightweight interfaces or unknown with narrowing.
As per coding guidelines: "Use strict TypeScript with proper type annotations".
♻️ Suggested type tightening
-export interface ExecutionResultData {
+export interface ExecutionStageResult {
+ stage: string;
+ taskId?: string;
+ result?: { summary?: string };
+}
+
+export interface ConsensusVoteTally {
+ for: number;
+ against: number;
+}
+
+export interface ExecutionResultData {
executionId: string;
status: string;
duration: number;
originalPrompt: string;
summary?: string;
- artifacts: Record<string, any>;
- stages: any[];
+ artifacts: Record<string, unknown>;
+ stages: ExecutionStageResult[];
agentCount: number;
taskCount: number;
meshStatus: {
nodeCount: number;
connectionCount: number;
};
consensusStatus: {
performed: boolean;
proposalId?: string;
accepted?: boolean;
- votes?: any;
+ votes?: ConsensusVoteTally | number;
timedOut?: boolean;
error?: string;
};
swarmStatus: {
algorithm: string;
isOptimizing: boolean;
};
- totPlan?: any;
+ totPlan?: unknown;
codexContext: {
enabled: boolean;
contextHash?: string;
sizeBytes?: number;
};
}🤖 Prompt for AI Agents
In `@src/cli/hive-mind-helpers.ts` around lines 15 - 47, The ExecutionResultData
interface is too permissive—replace the broad any types (artifacts:
Record<string, any>, stages: any[], consensusStatus.votes?: any, totPlan?: any)
with stricter types: define small lightweight interfaces (e.g., ArtifactRecord,
Stage, Vote, TotPlan) or use unknown and require callers to perform
type-narrowing; update ExecutionResultData to reference these new types and add
optional fields where necessary (e.g., consensusStatus.votes?: Vote[]), ensuring
codexContext and mesh/consensus sub-objects remain typed.
Comment on lines
+112
to
+117
| const outcome: any = await Promise.race([ | ||
| system.executeTask(prompt), | ||
| new Promise((_, reject) => | ||
| setTimeout(() => reject(new Error('Hive-mind execution timeout')), config.timeout) | ||
| ) | ||
| ]); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/cli/hive-mind-helpers.ts | sed -n '100,130p'Repository: clduab11/codex-synaptic
Length of output: 1165
🏁 Script executed:
cat -n src/cli/hive-mind-helpers.ts | sed -n '95,145p'Repository: clduab11/codex-synaptic
Length of output: 2050
Clear the timeout timer in a finally block to prevent unhandled rejections and resource leaks.
When Promise.race settles with system.executeTask completing first, the timeout's setTimeout continues running in the background. Once it fires, it attempts to reject an orphaned promise, creating an unhandled rejection warning. This also leaves the timer consuming resources in long-running CLI processes.
The fix captures the timeoutId and clears it in a finally block, ensuring proper resource cleanup regardless of which promise in the race settles first. This aligns with the coding guideline requiring cleanup of resources in finally blocks.
Suggested fix
- const outcome: any = await Promise.race([
- system.executeTask(prompt),
- new Promise((_, reject) =>
- setTimeout(() => reject(new Error('Hive-mind execution timeout')), config.timeout)
- )
- ]);
+ let timeoutId: NodeJS.Timeout | undefined;
+ const timeoutPromise = new Promise<never>((_, reject) => {
+ timeoutId = setTimeout(
+ () => reject(new Error('Hive-mind execution timeout')),
+ config.timeout
+ );
+ });
+ let outcome: any;
+ try {
+ outcome = await Promise.race([
+ system.executeTask(prompt),
+ timeoutPromise
+ ]);
+ } finally {
+ if (timeoutId) {
+ clearTimeout(timeoutId);
+ }
+ }🤖 Prompt for AI Agents
In `@src/cli/hive-mind-helpers.ts` around lines 112 - 117, The Promise.race use
can leak the timeout and cause unhandled rejections; capture the timer id when
creating the timeout promise (store in a variable like timeoutId from
setTimeout) and await the race inside a try block, then in a finally block call
clearTimeout(timeoutId) to cancel the pending timer regardless of whether
system.executeTask(prompt) or the timeout settled; update the block referencing
outcome, system.executeTask(prompt), and config.timeout accordingly so the
timeout is always cleared.
Comment on lines
+214
to
+219
| if (resultData.consensusStatus.performed) { | ||
| const statusIcon = resultData.consensusStatus.accepted ? '✅' : '❌'; | ||
| console.log( | ||
| chalk.cyan( | ||
| `\n🗳️ Consensus ${statusIcon} (${resultData.consensusStatus.votes?.for ?? 0} for, ${resultData.consensusStatus.votes?.against ?? 0} against)` | ||
| ) |
Contributor
There was a problem hiding this comment.
Consensus vote rendering assumes {for, against} but upstream provides a number.
renderExecutionSummary expects votes.for/against, while orchestrateConsensus supplies a numeric count. This will render 0 for, 0 against. Either change the payload shape or accept numeric totals.
🛠️ Suggested fix
- const statusIcon = resultData.consensusStatus.accepted ? '✅' : '❌';
- console.log(
- chalk.cyan(
- `\n🗳️ Consensus ${statusIcon} (${resultData.consensusStatus.votes?.for ?? 0} for, ${resultData.consensusStatus.votes?.against ?? 0} against)`
- )
- );
+ const statusIcon = resultData.consensusStatus.accepted ? '✅' : '❌';
+ const votes = resultData.consensusStatus.votes;
+ const votesFor = typeof votes === 'number' ? votes : votes?.for ?? 0;
+ const votesAgainst = typeof votes === 'number' ? 0 : votes?.against ?? 0;
+ const votesLabel = typeof votes === 'number'
+ ? `${votesFor} total`
+ : `${votesFor} for, ${votesAgainst} against`;
+ console.log(
+ chalk.cyan(`\n🗳️ Consensus ${statusIcon} (${votesLabel})`)
+ );🤖 Prompt for AI Agents
In `@src/cli/hive-mind-helpers.ts` around lines 214 - 219, The consensus votes
shape mismatch: orchestrateConsensus supplies a numeric total in
consensusStatus.votes but renderExecutionSummary (and the logging here
referencing resultData.consensusStatus.votes.for/against) expects an object with
.for/.against; update renderExecutionSummary and this logging branch to accept
either a numeric total or an object by detecting typeof
resultData.consensusStatus.votes === 'number' and treating that as total
votes-for (or as configured), otherwise use votes.for and votes.against; adjust
any code that constructs or reads consensusStatus.votes (e.g.,
orchestrateConsensus and renderExecutionSummary) to handle both shapes
consistently and include a clear comment about which shape is preferred.
Comment on lines
+4445
to
+4475
| // Check Codex MCP list | ||
| let codexMcpNames = new Set<string>(); | ||
| const codexMcpList = spawnSync('codex', ['mcp', 'list', '--json'], { | ||
| cwd: process.cwd(), | ||
| encoding: 'utf8' | ||
| }); | ||
| if (codexMcpList.status === 0) { | ||
| try { | ||
| const parsed = JSON.parse(codexMcpList.stdout || '[]') as Array<{ name?: string }>; | ||
| codexMcpNames = new Set(parsed.map((entry) => String(entry.name)).filter(Boolean)); | ||
| checks.push({ | ||
| id: 'codex.mcp_list', | ||
| ok: true, | ||
| details: `Loaded ${codexMcpNames.size} Codex MCP registration(s).` | ||
| }); | ||
| } catch (error) { | ||
| checks.push({ | ||
| id: 'codex.mcp_list', | ||
| ok: false, | ||
| details: `Failed to parse codex mcp list output: ${(error as Error).message}`, | ||
| remediation: 'Run `codex mcp list --json` and inspect output.' | ||
| }); | ||
| } | ||
| } else { | ||
| checks.push({ | ||
| id: 'codex.mcp_list', | ||
| ok: false, | ||
| details: codexMcpList.stderr?.trim() || 'codex mcp list failed', | ||
| remediation: 'Verify Codex CLI install and MCP support (`codex mcp --help`).' | ||
| }); | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check the file structure and line count
wc -l src/cli/index.tsRepository: clduab11/codex-synaptic
Length of output: 88
🏁 Script executed:
# Read the specific section flagged in the review
sed -n '4440,4480p' src/cli/index.tsRepository: clduab11/codex-synaptic
Length of output: 1545
🏁 Script executed:
# Search for execCodexCommand to verify it exists
rg -n "execCodexCommand" src/cli/index.ts | head -20Repository: clduab11/codex-synaptic
Length of output: 412
🏁 Script executed:
# Check the broader context - find the function containing this code
sed -n '4400,4445p' src/cli/index.tsRepository: clduab11/codex-synaptic
Length of output: 1531
🏁 Script executed:
# Check function signature of execCodexCommand
sed -n '4324,4345p' src/cli/index.tsRepository: clduab11/codex-synaptic
Length of output: 960
🏁 Script executed:
# Verify if the doctor command handler is async
sed -n '4410,4420p' src/cli/index.tsRepository: clduab11/codex-synaptic
Length of output: 525
🏁 Script executed:
# Search for all spawnSync calls in src/cli/index.ts
rg -n "spawnSync" src/cli/index.tsRepository: clduab11/codex-synaptic
Length of output: 200
🏁 Script executed:
# Check if checkCodexAuth() or similar helper functions exist
rg -n "function check" src/cli/index.tsRepository: clduab11/codex-synaptic
Length of output: 49
🏁 Script executed:
# Get more context on the doctor command handler - check how it ends and what it awaits
sed -n '4410,4550p' src/cli/index.tsRepository: clduab11/codex-synaptic
Length of output: 5284
🏁 Script executed:
# Check if execFileAsync is imported properly
rg -n "execFileAsync\|promisify" src/cli/index.ts | head -10Repository: clduab11/codex-synaptic
Length of output: 49
🏁 Script executed:
# Check the import statements at the top of src/cli/index.ts
sed -n '1,50p' src/cli/index.ts | rg -n "import|require"Repository: clduab11/codex-synaptic
Length of output: 1561
🏁 Script executed:
# Check how execFileAsync is created - search for its definition or if it's an alias
sed -n '1,4350p' src/cli/index.ts | grep -A 5 "execFileAsync"Repository: clduab11/codex-synaptic
Length of output: 509
🏁 Script executed:
# Verify the exact setup near execCodexCommand definition
sed -n '4315,4365p' src/cli/index.tsRepository: clduab11/codex-synaptic
Length of output: 1928
🏁 Script executed:
# Verify the rest of the doctor handler uses async patterns consistently
sed -n '4475,4515p' src/cli/index.tsRepository: clduab11/codex-synaptic
Length of output: 1515
Replace spawnSync with await execCodexCommand to avoid blocking the event loop.
This synchronous call in an async handler violates the coding guideline "Don't block the event loop with synchronous operations." Use the existing execCodexCommand wrapper, which properly handles timeouts and error states. Optionally extract the Codex MCP check into a helper function to reduce complexity.
Suggested fix
- const codexMcpList = spawnSync('codex', ['mcp', 'list', '--json'], {
- cwd: process.cwd(),
- encoding: 'utf8'
- });
- if (codexMcpList.status === 0) {
- try {
- const parsed = JSON.parse(codexMcpList.stdout || '[]') as Array<{ name?: string }>;
- codexMcpNames = new Set(parsed.map((entry) => String(entry.name)).filter(Boolean));
- checks.push({
- id: 'codex.mcp_list',
- ok: true,
- details: `Loaded ${codexMcpNames.size} Codex MCP registration(s).`
- });
- } catch (error) {
- checks.push({
- id: 'codex.mcp_list',
- ok: false,
- details: `Failed to parse codex mcp list output: ${(error as Error).message}`,
- remediation: 'Run `codex mcp list --json` and inspect output.'
- });
- }
- } else {
- checks.push({
- id: 'codex.mcp_list',
- ok: false,
- details: codexMcpList.stderr?.trim() || 'codex mcp list failed',
- remediation: 'Verify Codex CLI install and MCP support (`codex mcp --help`).'
- });
- }
+ try {
+ const codexMcpList = await execCodexCommand(['mcp', 'list', '--json']);
+ if (codexMcpList.exitCode === 0) {
+ const parsed = JSON.parse(codexMcpList.stdout || '[]') as Array<{ name?: string }>;
+ codexMcpNames = new Set(parsed.map((entry) => String(entry.name)).filter(Boolean));
+ checks.push({
+ id: 'codex.mcp_list',
+ ok: true,
+ details: `Loaded ${codexMcpNames.size} Codex MCP registration(s).`
+ });
+ } else {
+ checks.push({
+ id: 'codex.mcp_list',
+ ok: false,
+ details: codexMcpList.stderr?.trim() || 'codex mcp list failed',
+ remediation: 'Verify Codex CLI install and MCP support (`codex mcp --help`).'
+ });
+ }
+ } catch (error) {
+ checks.push({
+ id: 'codex.mcp_list',
+ ok: false,
+ details: (error as Error).message,
+ remediation: 'Verify Codex CLI install and MCP support (`codex mcp --help`).'
+ });
+ }🤖 Prompt for AI Agents
In `@src/cli/index.ts` around lines 4445 - 4475, Replace the blocking spawnSync
usage that populates codexMcpNames with the async execCodexCommand helper: call
await execCodexCommand('codex', ['mcp','list','--json']) (or wrap in a small
async helper function), then use the returned { stdout, stderr, code, timedOut }
to mirror the existing success/failure branches—on success JSON.parse stdout
into codexMcpNames (Set of entry.name), push the same checks entry, and on
failure use stderr/timedOut/code to populate the failing checks entry and
remediation; ensure you await execCodexCommand, catch JSON parse errors and
thrown errors, and preserve the same check ids ('codex.mcp_list') and messages.
- Keep PR's restructured README and add CLI auto-shutdown note from main - Use renderBackgroundDaemonStatus helper consistently - Refactor GOAP and orchestration logic into helper functions - Regenerate package-lock.json to resolve dependency conflicts - Fix GoapExecutor import and remove duplicate setupWorkflowEventHandlers
Merge main and resolve conflicts - integrate CLI auto-shutdown and refactor helpers
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Codex PR ReviewFindings
Suggested Fixes
MCP Usage
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Testing