Huge Cleanup#355
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
Capy auto-review is paused for this organization because the monthly auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews. |
📝 WalkthroughWalkthroughAdds normalized runtime role/build-hash checks across CLI/desktop/dev, enforces CTO role for desktop local runtimes, introduces constrained chat model selection in UI, improves orchestrator sync with failed-run interventions, normalizes/persists computer-use backend style, consolidates config/DB migration tests, adds test sharding, and updates docs validator. ChangesRuntime roles and runtime freshness
Constrained chat model selection and UI wiring
Orchestrator sync/status safeguards
Computer-use artifacts backend style
Config tests, DB migrations, and git/lane tests
UI/services miscellany
Test scripts, docs, ignore
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
Reviewed commit |
|
To use Codex here, create a Codex account and connect to github. |
arul28
force-pushed
the
ade/huge-cleanup-8469674a
branch
from
5f60d58 to
d15aeaa
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Review
Scope: 105 file(s), +5380 / −5673
Verdict: Minor issues
Launch-prep cleanup: consolidates desktop/CLI tests and docs, tightens dev/runtime handshake (build hash + default role), hardens constrained chat model selection, and improves mission/orchestrator failure sync. No blocking security defects found; a few behavior changes are worth confirming before merge.
🔒 Security
[Medium] CLI default role elevated to cto when unset
File: apps/ade-cli/src/cli.ts:2022
Issue: parseCliArgs now uses resolveAdeDefaultRole(process.env.ADE_DEFAULT_ROLE, "cto") instead of defaulting to agent. Any local ade / headless runtime started without --role or ADE_DEFAULT_ROLE now runs with CTO-scoped RPC tools rather than agent-scoped tools.
Attack path: On a machine where a shared/local RPC socket is reachable, a process that can spawn or attach to the runtime without setting role inherits the broader CTO tool surface (operator/Linear tools) that previously required an explicit role or env var.
Fix: Keep cto only for desktop-owned spawn paths (already set in localRuntimeConnectionPool / TUI), and restore agent (or external) as the CLI fallback for generic invocations:
role: resolveAdeDefaultRole(process.env.ADE_DEFAULT_ROLE, "agent"),🐛 Functionality
[Medium] Mission step sync can target the wrong step by title
File: apps/desktop/src/main/services/orchestrator/aiOrchestratorService.ts:6982-6985
Issue: When runStep.missionStepId is missing, resolveMissionStepForRunStep falls back to mission.steps.find((step) => step.title === runStep.title). Duplicate step titles map to the first match, so orchestrator status updates can be applied to the wrong mission step.
Repro: Create a mission with two steps titled "Implement API"; run an orchestrator graph whose run step lacks missionStepId but shares that title; mark one run step succeeded — the first mission step with that title flips status while the other stays stale.
Fix: Drop the title-only fallback (or gate it behind a unique-title check). Prefer metadata.orchestratorStepId / metadata.stepKey only, and log/skip when no stable link exists.
[Medium] TCP dev runtime no longer auto-recovers when stale
File: scripts/dev-shared.mjs:432-433
Issue: ensureRuntime now throws if the socket is tcp://… and build hash/role/project root mismatches, instead of restarting like the Unix socket path.
Repro: Start ade serve over TCP with an old build hash, then run npm run dev:desktop in attach mode against that socket — startup fails with a stale-runtime error rather than recycling the daemon.
Fix: Document the limitation in dev UX, or implement a TCP-safe shutdown/restart path parallel to the Unix-socket flow.
🎨 UI/UX
[Low] Send stays enabled with a disallowed model on constrained surfaces
File: apps/desktop/src/renderer/components/chat/AgentChatComposer.tsx:2716-2726
Issue: singleReady only checks Boolean(modelId). On constrained surfaces (availableModelIdsOverride), the composer can enable Send while modelId is outside effectiveAvailableModelIds; the user only sees constrainedModelSelectionError after submit (AgentChatPane.tsx:5537-5539).
Fix: Treat constrained surfaces like missing-model when modelId is not in effectiveAvailableModelIds (disable send + surface modelUnavailableMessage in the tooltip).
Notes
- Runtime freshness checks (build hash,
defaultRole, project root) and consolidated migration/config tests look like solid launch hygiene. capabilities.actions.listChanged: falseis documented; external clients must poll tool lists instead of relying on push notifications.- Large
goal.mdchurn is planning text only — not reviewed for product behavior.
Sent by Cursor Automation: BUGBOT in Versic
|
@codex review |
|
@copilot review but do not make fixes |
![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: d15aeaa00d
ℹ️ 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".
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2dabe69cb8
ℹ️ 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".
|
@codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 30f99ab735
ℹ️ 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}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/desktop/src/renderer/components/chat/AgentChatComposer.tsx (1)
2623-2631:⚠️ Potential issue | 🟠 Major | ⚡ Quick winParallel launch bypasses constrained-model blocking.
When
modelUnavailableMessageis set, single send is blocked, but the parallel path still allows submit (submitComposerDraft) and enables send (parallelReady) without checking that blocked state. That contradicts the constrained selection behavior and can launch with an invalid/disallowed model setup.Suggested fix
@@ const singleModelBlockedMessage = (modelUnavailableMessage?.trim() ?? "").length > 0 ? modelUnavailableMessage : null; const singleModelReady = Boolean(modelId) && !singleModelBlockedMessage; @@ const submitComposerDraft = useCallback(() => { @@ if (parallelChatMode) { + if (singleModelBlockedMessage) { + onSubmitBlocked?.(singleModelBlockedMessage); + return; + } if (busy || parallelLaunchBusy) return; if (parallelModelSlots.length < 2) return; @@ - const parallelReady = + const parallelReady = parallelChatMode + && !singleModelBlockedMessage && parallelModelSlots.length >= 2 && (draft.trim().length > 0 || attachments.length > 0 || contextAttachmentCount > 0); @@ function sendButtonTitle(): string { @@ if (parallelChatMode) { + if (singleModelBlockedMessage) return singleModelBlockedMessage; if (parallelModelSlots.length < 2) return "Add at least two models"; if (draft.trim().length === 0 && attachments.length === 0 && contextAttachmentCount === 0) return "Add a message or at least one attachment"; return "Send to all lanes"; }Also applies to: 2713-2732, 2743-2750
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/components/chat/AgentChatComposer.tsx` around lines 2623 - 2631, The parallel-send path currently bypasses the constrained-model block by not checking modelUnavailableMessage (so parallelChatMode can call onSubmit/submitComposerDraft or set parallelReady even when the selected model is disallowed); update the parallel branch in AgentChatComposer (the block guarded by parallelChatMode that references parallelLaunchBusy, parallelModelSlots, parallelReady, onSubmit/submitComposerDraft) to include the same modelUnavailableMessage check used in the single-send path and return early if set, and mirror this check in the other parallel-send blocks around the same file (the other occurrences you noted) so parallel submissions are blocked when modelUnavailableMessage indicates an invalid model.apps/ade-cli/src/cli.ts (1)
13442-13446:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReport the invocation role instead of rereading the environment.
This now prints
resolveAdeDefaultRole(process.env.ADE_DEFAULT_ROLE, "agent"), which can disagree with the actual--roleused for this command on socket-backed paths.ade --role external auth statuswill still report the ambient env/default role unless the headless path happened to mutate the env first.Suggested fix
function summarizeExecution(args: { plan: CliPlan & { kind: "execute" }; connection: CliConnection; values: JsonObject; + role: GlobalOptions["role"]; }): unknown { - const { plan, connection, values } = args; + const { plan, connection, values, role } = args; ... authMode: connection.mode === "desktop-socket" ? "local-desktop-socket" : "local-headless-project", - role: resolveAdeDefaultRole(process.env.ADE_DEFAULT_ROLE, "agent"), + role, projectRoot: connection.projectRoot,- return summarizeExecution({ plan, connection, values }); + return summarizeExecution({ plan, connection, values, role: options.role });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/ade-cli/src/cli.ts` around lines 13442 - 13446, The role field is re-evaluating the environment via resolveAdeDefaultRole(process.env.ADE_DEFAULT_ROLE, "agent") which can disagree with the actual CLI invocation; instead assign the already-parsed invocation role (the CLI/argv role variable used when parsing flags — e.g., the parsed "role" or "argv.role") to the role property alongside authMode (keep the existing authMode logic that checks connection.mode === "desktop-socket"), removing the extra resolveAdeDefaultRole call so the reported role matches the command's --role.
🧹 Nitpick comments (5)
apps/desktop/src/renderer/components/settings/AiFeaturesSection.test.tsx (2)
34-80: ⚡ Quick winConsider typing the window.ade mock structure.
The
installAdeMocksfunction creates a detailedwindow.adestructure with inline object literals. If the actual IPC types change, this mock could silently become stale.♻️ Suggested improvement
Import and cast to the actual preload types:
import type { AdePreloadApi } from '../../preload/types'; // adjust path as needed function installAdeMocks() { (window as any).ade = { ai: { getStatus: vi.fn().mockResolvedValue({ // ... existing mock data } as Awaited<ReturnType<AdePreloadApi['ai']['getStatus']>>), updateConfig: vi.fn().mockResolvedValue(undefined), }, // ... rest of mock } satisfies Partial<AdePreloadApi>; }This ensures the mock structure stays aligned with actual IPC contracts.
As per coding guidelines, keep IPC contracts, preload types, shared types, and renderer usage in sync whenever an interface changes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/components/settings/AiFeaturesSection.test.tsx` around lines 34 - 80, The window.ade mock in installAdeMocks should be typed against the real preload IPC types so it doesn't drift; import the AdePreloadApi type (or the correct preload types), cast the ai.getStatus mock resolved value as Awaited<ReturnType<AdePreloadApi['ai']['getStatus']>> and cast/declare the whole (window as any).ade assignment as satisfies Partial<AdePreloadApi> (or equivalent), keeping the existing vi.fn mocks (ai.getStatus, ai.updateConfig, projectConfig.get) but typed to the real API so failures surface at compile time.
12-23: ⚡ Quick winConsider importing actual ModelPicker prop types.
The mock uses an inline type definition that could drift from the actual
ModelPickercomponent signature. Importing the actual props type would improve type safety and catch interface changes at compile time.♻️ Suggested refactor
+import type { ModelPickerProps } from "../shared/ModelPicker/ModelPicker"; + vi.mock("../shared/ModelPicker/ModelPicker", () => { return { - ModelPicker: (props: { surfaceKey: string; onOpenSignIn?: () => void }) => { + ModelPicker: (props: Pick<ModelPickerProps, 'surfaceKey' | 'onOpenSignIn'>) => { modelPickerProps.push(props);As per coding guidelines, IPC contracts, preload types, shared types, and renderer usage should be kept in sync whenever an interface changes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/components/settings/AiFeaturesSection.test.tsx` around lines 12 - 23, The inline prop type in the ModelPicker mock can drift from the real component; import the actual props type from "../shared/ModelPicker/ModelPicker" (e.g., ModelPickerProps) and use that type for the mock signature and for modelPickerProps so the mock's signature stays in sync with the real ModelPicker component; update the mock factory to type its props parameter with the imported ModelPickerProps and adjust any related variable typings (modelPickerProps) accordingly.apps/desktop/src/main/services/lanes/laneService.test.ts (1)
3566-3591: ⚡ Quick winFail fast on unhandled git calls in the shared responder.
The default fallback currently returns
{ exitCode: 1 }for unknown commands, which can mask lane-service git contract drift and let tests pass on unintended paths. Prefer throwing here so new git probes are explicitly handled in each test.Suggested change
function makeRunGitResponder(custom?: (args: string[], opts: any) => { exitCode: number; stdout: string; stderr: string } | null) { return async (args: string[], opts: any = {}) => { if (custom) { const v = custom(args, opts); if (v) return v; } @@ if (args[0] === "status" && args[1] === "--porcelain=v1") { return { exitCode: 0, stdout: "", stderr: "" }; } - return { exitCode: 1, stdout: "", stderr: `unhandled: ${args.join(" ")}` }; + throw new Error(`Unexpected git call: ${args.join(" ")}`); }; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/main/services/lanes/laneService.test.ts` around lines 3566 - 3591, The shared test helper makeRunGitResponder currently returns a silent failure object for unhandled git args; change its default fallback (the final return in makeRunGitResponder) to throw an Error (including the unhandled command string) instead of returning { exitCode: 1, ... } so tests fail-fast when a new git probe is introduced; update any tests that relied on the old fallback to provide a custom handler via the custom parameter or explicitly mock the expected git call.apps/desktop/src/main/services/orchestrator/aiOrchestratorService.test.ts (1)
6951-6956: ⚡ Quick winAssert exactly one open failed-step intervention in both new regression tests.
some(...)will still pass if duplicate open interventions are created. Since this PR is guarding sync duplication paths, tightening to a count check makes the regression intent explicit.Suggested assertion tightening
- expect(refreshed?.interventions.some((entry) => - entry.status === "open" && entry.interventionType === "failed_step" - )).toBe(true); + const openFailedStepInterventions = refreshed?.interventions.filter( + (entry) => entry.status === "open" && entry.interventionType === "failed_step" + ) ?? []; + expect(openFailedStepInterventions).toHaveLength(1);Also applies to: 7000-7004
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/main/services/orchestrator/aiOrchestratorService.test.ts` around lines 6951 - 6956, The test currently uses refreshed?.interventions.some(...) which only checks existence and can miss duplicates; update the assertions for the mission fetched via fixture.missionService.get(mission.id) to assert exactly one open failed-step intervention by counting matching entries (e.g., filter refreshed?.interventions for entry.status === "open" && entry.interventionType === "failed_step" and expect the resulting length toBe(1)); apply the same tightening to the second occurrence around lines referencing the same refreshed?.interventions check.apps/desktop/src/renderer/components/chat/AgentChatPane.tsx (1)
3046-3055: ⚡ Quick winMake the constrained empty-state message actionable.
No models are available for this chat surface.explains the block, but not how the user can recover. This path now hard-blocks create/send, so the message should include the next action as well.As per coding guidelines: "Keep user-facing copy concrete and stateful: say what changed, what is blocked, and what the next action is."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/components/chat/AgentChatPane.tsx` around lines 3046 - 3055, constrainedModelSelectionError currently returns an un-actionable string when there are no available models; update the message returned by the useMemo (in the constrainedModelSelectionError computation that depends on modelSelectionConstrained, effectiveAvailableModelIds, and modelId) to state what changed, what is blocked, and the next action the user should take (for example: explain that sending is blocked because no models are available for this chat surface and instruct the user to switch the chat surface or enable/select models in Settings or Model Selection). Ensure the string replaces the current "No models are available for this chat surface." text and remains localized/consistent with existing UX copy conventions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/ade-cli/src/adeRpcServer.ts`:
- Around line 7340-7345: The handler removed dynamic change signaling causing
clients to cache stale tool/action lists; restore either (A) signaling by
re-adding the change hook so actions.listChanged is set true when
listToolSpecsForSession()’s underlying sources
(computerUseArtifactBrokerService.getBackendStatus() and runtime.macosVmService)
change, or (B) make the advertised action set static for the session by
computing and storing the tool list at createAdeRpcRequestHandler() time (use
the runtime and serverVersion closure) and always returning that static list
with actions.listChanged = false; locate logic around
createAdeRpcRequestHandler, actions.listChanged, and listToolSpecsForSession to
implement one of these two fixes so clients won’t see stale or disappearing
computer-use actions.
In `@apps/ade-cli/src/cli.ts`:
- Around line 10802-10810: The early return when runtimeVersion exists and !=
PLACEHOLDER_VERSION skips build-hash/role checks in source builds; change the
condition so it only returns null if both the daemon and the CLI are
non-placeholder versions. Concretely, update the `if (runtimeVersion &&
runtimeVersion !== PLACEHOLDER_VERSION) return null;` to also check the CLI
VERSION (e.g., `&& VERSION !== PLACEHOLDER_VERSION`) so that when `VERSION ===
PLACEHOLDER_VERSION` the subsequent checks against `expectedBuildHash`,
`runtimeInfo.buildHash`, `runtimeInfo.defaultRole`, and `expectedDefaultRole`
still run.
In `@apps/desktop/package.json`:
- Around line 34-36: The unit workspace projects in
apps/desktop/vitest.workspace.ts are including src/main/**/*.test.{ts,tsx} and
src/renderer/**/*.test.{ts,tsx} without excluding integration tests, causing
files like remoteRuntime.offlineRpc.offlineRpc.integration.test.ts to run under
the "test:unit" script; update the unit project configs in vitest.workspace.ts
(the project entries that define include for unit tests) to add exclude:
["**/*.integration.test.{ts,tsx}"] or tighten the include globs to omit
*.integration.test.{ts,tsx} so unit runs no longer pick up integration test
files.
In `@apps/desktop/src/main/services/config/projectConfigService.test.ts`:
- Around line 597-661: The tests that call createLinearFixture() ("merges
shared/local linear sync config with local precedence" and "clamps linear sync
confidence threshold to valid range") only call fixture.db.close() on success;
wrap the test body in try/finally (or add a finally block) so fixture.db.close()
always runs, ensuring the DB handle is closed on assertion failures — locate the
fixture variable created via createLinearFixture() and move the existing
fixture.db.close() call into the finally block (or call fixture.db.close()
inside finally) for both test cases.
In `@apps/desktop/src/main/services/orchestrator/aiOrchestratorService.test.ts`:
- Around line 6981-6996: The test currently assumes startMissionRun created a
step to mark failed which is brittle; explicitly seed a step for the run before
forcing terminal statuses. After obtaining runId from
aiOrchestratorService.startMissionRun, create an orchestrator_steps row tied to
that run (or use an existing fixture helper) with a known id and initial status
(e.g., "pending" or "running") and timestamps, then run the existing
fixture.db.run updates that set status = 'failed' and completed_at/updated_at
using runId and the known step id; reference startMissionRun, runId,
fixture.db.run, and orchestrator_steps when making the change.
In `@apps/desktop/src/main/services/orchestrator/aiOrchestratorService.ts`:
- Around line 7047-7079: The current ensureTerminalFailedRunIntervention (in
aiOrchestratorService.ts) bail-out checks any open "failed_step" globally for
the mission, which can suppress or mis-attach interventions when a mission has
multiple failed steps; update this function to reuse the same run-step →
mission-step matching logic used in syncMissionStepsFromRun: for the
failedRunStep find the corresponding missionStep (using the same matcher) and
then check mission.interventions only for an open "failed_step" that targets
that matched missionStep.id (or, if no missionStep matched, target the same
orchestrator run step id via metadata.orchestratorStepId); only create a new
intervention when no such matching open intervention exists and ensure
missionService.addIntervention metadata.stepId/stepKey/orchestratorStepId
reflect the matched identifiers.
In `@apps/desktop/src/main/services/state/kvDb.migrations.test.ts`:
- Around line 56-410: The tests that open a DB using openKvDb (e.g., the it
blocks with descriptions "creates Phase 1.5 context hardening tables and
indexes", "creates mission tables and key indexes", and "creates W2 worker
tables and indexes") currently call db.close() only on the success path; wrap
each test body so that after creating const db = await openKvDb(...) you use a
try { ... } finally { db.close(); } block to ensure the DB handle is always
closed on teardown even when assertions fail; ensure any returns/awaits remain
inside the try block and that db.close() is called in the finally for each
affected it block.
In `@apps/desktop/src/renderer/components/chat/AgentChatPane.tsx`:
- Around line 4838-4841: When constrainedModelSelectionError is encountered in
createSession(), don't resolve to null (which the eager-create path treats as a
successful attempt); instead surface the failure so the caller doesn't flip the
one-shot guard — either throw the constrainedModelSelectionError after calling
setError(constrainedModelSelectionError) or return a rejected Promise so callers
(the eager-create flow) can detect the failure and retry later; update
references in createSession(), constrainedModelSelectionError, and any
eager-create one-shot guard handling to expect a thrown/rejected error rather
than a null sentinel.
In `@apps/desktop/src/renderer/components/shared/ModelPicker/ModelPicker.tsx`:
- Around line 165-185: The early return of models bypasses the constraint logic;
change the start of the memo to not return raw models immediately but instead,
when models && models.length: if constrainToAvailableModelIds is true, build the
same trimmed Set from availableModelIds (as used for selectedValue) and filter
models by id (trim + Boolean) keeping only those present in the Set; if the
filtered list has items return it, otherwise fall through to the existing
fallback/merge logic (so mergeSelectorModels, catalogModels.models, and the rest
handle empties); if constrainToAvailableModelIds is false just return models as
before. Use the existing symbols models, constrainToAvailableModelIds,
availableModelIds, mergeSelectorModels, and catalogModels.models to locate and
implement this change.
---
Outside diff comments:
In `@apps/ade-cli/src/cli.ts`:
- Around line 13442-13446: The role field is re-evaluating the environment via
resolveAdeDefaultRole(process.env.ADE_DEFAULT_ROLE, "agent") which can disagree
with the actual CLI invocation; instead assign the already-parsed invocation
role (the CLI/argv role variable used when parsing flags — e.g., the parsed
"role" or "argv.role") to the role property alongside authMode (keep the
existing authMode logic that checks connection.mode === "desktop-socket"),
removing the extra resolveAdeDefaultRole call so the reported role matches the
command's --role.
In `@apps/desktop/src/renderer/components/chat/AgentChatComposer.tsx`:
- Around line 2623-2631: The parallel-send path currently bypasses the
constrained-model block by not checking modelUnavailableMessage (so
parallelChatMode can call onSubmit/submitComposerDraft or set parallelReady even
when the selected model is disallowed); update the parallel branch in
AgentChatComposer (the block guarded by parallelChatMode that references
parallelLaunchBusy, parallelModelSlots, parallelReady,
onSubmit/submitComposerDraft) to include the same modelUnavailableMessage check
used in the single-send path and return early if set, and mirror this check in
the other parallel-send blocks around the same file (the other occurrences you
noted) so parallel submissions are blocked when modelUnavailableMessage
indicates an invalid model.
---
Nitpick comments:
In `@apps/desktop/src/main/services/lanes/laneService.test.ts`:
- Around line 3566-3591: The shared test helper makeRunGitResponder currently
returns a silent failure object for unhandled git args; change its default
fallback (the final return in makeRunGitResponder) to throw an Error (including
the unhandled command string) instead of returning { exitCode: 1, ... } so tests
fail-fast when a new git probe is introduced; update any tests that relied on
the old fallback to provide a custom handler via the custom parameter or
explicitly mock the expected git call.
In `@apps/desktop/src/main/services/orchestrator/aiOrchestratorService.test.ts`:
- Around line 6951-6956: The test currently uses
refreshed?.interventions.some(...) which only checks existence and can miss
duplicates; update the assertions for the mission fetched via
fixture.missionService.get(mission.id) to assert exactly one open failed-step
intervention by counting matching entries (e.g., filter refreshed?.interventions
for entry.status === "open" && entry.interventionType === "failed_step" and
expect the resulting length toBe(1)); apply the same tightening to the second
occurrence around lines referencing the same refreshed?.interventions check.
In `@apps/desktop/src/renderer/components/chat/AgentChatPane.tsx`:
- Around line 3046-3055: constrainedModelSelectionError currently returns an
un-actionable string when there are no available models; update the message
returned by the useMemo (in the constrainedModelSelectionError computation that
depends on modelSelectionConstrained, effectiveAvailableModelIds, and modelId)
to state what changed, what is blocked, and the next action the user should take
(for example: explain that sending is blocked because no models are available
for this chat surface and instruct the user to switch the chat surface or
enable/select models in Settings or Model Selection). Ensure the string replaces
the current "No models are available for this chat surface." text and remains
localized/consistent with existing UX copy conventions.
In `@apps/desktop/src/renderer/components/settings/AiFeaturesSection.test.tsx`:
- Around line 34-80: The window.ade mock in installAdeMocks should be typed
against the real preload IPC types so it doesn't drift; import the AdePreloadApi
type (or the correct preload types), cast the ai.getStatus mock resolved value
as Awaited<ReturnType<AdePreloadApi['ai']['getStatus']>> and cast/declare the
whole (window as any).ade assignment as satisfies Partial<AdePreloadApi> (or
equivalent), keeping the existing vi.fn mocks (ai.getStatus, ai.updateConfig,
projectConfig.get) but typed to the real API so failures surface at compile
time.
- Around line 12-23: The inline prop type in the ModelPicker mock can drift from
the real component; import the actual props type from
"../shared/ModelPicker/ModelPicker" (e.g., ModelPickerProps) and use that type
for the mock signature and for modelPickerProps so the mock's signature stays in
sync with the real ModelPicker component; update the mock factory to type its
props parameter with the imported ModelPickerProps and adjust any related
variable typings (modelPickerProps) accordingly.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e8e1b9ce-3fa6-4aca-a91d-3e37ce422139
⛔ Files ignored due to path filters (12)
AGENTS.mdis excluded by!*.mdREADME.mdis excluded by!*.mddocs/ARCHITECTURE.mdis excluded by!docs/**docs/features/ade-code/README.mdis excluded by!docs/**docs/features/agents/tool-registration.mdis excluded by!docs/**docs/features/chat/composer-and-ui.mdis excluded by!docs/**docs/features/cto/README.mdis excluded by!docs/**docs/features/missions/README.mdis excluded by!docs/**docs/features/missions/orchestration.mdis excluded by!docs/**docs/features/onboarding-and-settings/README.mdis excluded by!docs/**docs/features/remote-runtime/internal-architecture.mdis excluded by!docs/**goal.mdis excluded by!*.md
📒 Files selected for processing (98)
.gitignoreapps/ade-cli/src/adeRpcServer.test.tsapps/ade-cli/src/adeRpcServer.tsapps/ade-cli/src/cli.test.tsapps/ade-cli/src/cli.tsapps/ade-cli/src/multiProjectRpcServer.tsapps/ade-cli/src/runtimeRoles.tsapps/ade-cli/src/services/sync/syncRemoteCommandService.tsapps/ade-cli/src/stdioRpcDaemon.test.tsapps/ade-cli/src/tuiClient/__tests__/connection.test.tsapps/ade-cli/src/tuiClient/connection.tsapps/desktop/package.jsonapps/desktop/src/main/main.tsapps/desktop/src/main/services/ai/tools/universalTools.tsapps/desktop/src/main/services/ai/tools/workflowTools.tsapps/desktop/src/main/services/appControl/appControlService.test.tsapps/desktop/src/main/services/appControl/appControlService.tsapps/desktop/src/main/services/chat/agentChatService.suggestLaneName.test.tsapps/desktop/src/main/services/chat/agentChatService.test.tsapps/desktop/src/main/services/computerUse/computerUseArtifactBrokerService.test.tsapps/desktop/src/main/services/computerUse/computerUseArtifactBrokerService.tsapps/desktop/src/main/services/config/projectConfigService.aiModeMigration.test.tsapps/desktop/src/main/services/config/projectConfigService.automationExecution.test.tsapps/desktop/src/main/services/config/projectConfigService.laneEnvInit.test.tsapps/desktop/src/main/services/config/projectConfigService.linearSync.test.tsapps/desktop/src/main/services/config/projectConfigService.notifications.test.tsapps/desktop/src/main/services/config/projectConfigService.processGroups.test.tsapps/desktop/src/main/services/config/projectConfigService.providers.test.tsapps/desktop/src/main/services/config/projectConfigService.test.tsapps/desktop/src/main/services/cto/workerAgentService.tsapps/desktop/src/main/services/git/gitOperationsService.branchSwitch.test.tsapps/desktop/src/main/services/git/gitOperationsService.test.tsapps/desktop/src/main/services/ipc/registerIpc.tsapps/desktop/src/main/services/lanes/laneService.branchSwitch.test.tsapps/desktop/src/main/services/lanes/laneService.test.tsapps/desktop/src/main/services/localRuntime/localRuntimeConnectionPool.test.tsapps/desktop/src/main/services/localRuntime/localRuntimeConnectionPool.tsapps/desktop/src/main/services/macosVm/macosVmService.test.tsapps/desktop/src/main/services/macosVm/macosVmService.tsapps/desktop/src/main/services/missions/missionPreflightService.tsapps/desktop/src/main/services/missions/missionService.tsapps/desktop/src/main/services/onboarding/onboardingService.tsapps/desktop/src/main/services/orchestrator/aiOrchestratorService.test.tsapps/desktop/src/main/services/orchestrator/aiOrchestratorService.tsapps/desktop/src/main/services/orchestrator/coordinatorAgent.tsapps/desktop/src/main/services/orchestrator/coordinatorTools.tsapps/desktop/src/main/services/orchestrator/delegationContracts.tsapps/desktop/src/main/services/orchestrator/executionPolicy.tsapps/desktop/src/main/services/orchestrator/orchestratorContext.tsapps/desktop/src/main/services/orchestrator/orchestratorQueries.tsapps/desktop/src/main/services/orchestrator/orchestratorService.tsapps/desktop/src/main/services/state/kvDb.laneWorktreeLockMigration.test.tsapps/desktop/src/main/services/state/kvDb.migrations.test.tsapps/desktop/src/main/services/state/kvDb.missionsMigration.test.tsapps/desktop/src/main/services/state/kvDb.orchestratorMigration.test.tsapps/desktop/src/main/services/state/kvDb.pipelineSettingsMigration.test.tsapps/desktop/src/main/services/state/kvDb.tsapps/desktop/src/main/services/state/kvDb.workerAgentsMigration.test.tsapps/desktop/src/main/services/usage/usageTrackingService.tsapps/desktop/src/renderer/components/app/AppShell.tsxapps/desktop/src/renderer/components/chat/AgentChatComposer.test.tsxapps/desktop/src/renderer/components/chat/AgentChatComposer.tsxapps/desktop/src/renderer/components/chat/AgentChatPane.submit.test.tsxapps/desktop/src/renderer/components/chat/AgentChatPane.tsxapps/desktop/src/renderer/components/cto/CtoPage.tsxapps/desktop/src/renderer/components/cto/LinearSyncPanel.tsxapps/desktop/src/renderer/components/cto/TeamPanel.tsxapps/desktop/src/renderer/components/cto/pipeline/config/CloseoutConfig.tsxapps/desktop/src/renderer/components/history/eventTaxonomy.tsapps/desktop/src/renderer/components/history/useTimelineLayout.tsapps/desktop/src/renderer/components/lanes/LaneGitActionsPane.tsxapps/desktop/src/renderer/components/lanes/LanesPage.tsxapps/desktop/src/renderer/components/missions/missionThreadEventAdapter.tsapps/desktop/src/renderer/components/settings/AiFeaturesSection.test.tsxapps/desktop/src/renderer/components/settings/AiFeaturesSection.tsxapps/desktop/src/renderer/components/settings/ChatAppearancePreview.tsxapps/desktop/src/renderer/components/settings/LaneTemplatesSection.tsxapps/desktop/src/renderer/components/settings/ProvidersSection.tsxapps/desktop/src/renderer/components/settings/ProxyAndPreviewSection.tsxapps/desktop/src/renderer/components/shared/ModelPicker/ModelPicker.tsxapps/desktop/src/renderer/components/shared/ModelPicker/ModelPickerContent.tsxapps/desktop/src/renderer/lib/computerUse.test.tsapps/desktop/src/renderer/onboarding/tours/ctoTour.tsapps/desktop/src/renderer/onboarding/tours/firstJourneyTour.test.tsapps/desktop/src/renderer/onboarding/tours/firstJourneyTour.tsapps/desktop/src/shared/chatModelSwitching.test.tsapps/desktop/src/shared/chatModelSwitching.tsapps/desktop/src/shared/types/computerUseArtifacts.tsapps/desktop/src/shared/types/orchestrator.tspackage.jsonscripts/dev-all.mjsscripts/dev-code.mjsscripts/dev-desktop.mjsscripts/dev-runtime.mjsscripts/dev-shared.mjsscripts/run-desktop-test-shards.mjsscripts/tui-web.mjsscripts/validate-docs.mjs
💤 Files with no reviewable changes (44)
- apps/desktop/src/renderer/components/cto/CtoPage.tsx
- apps/desktop/src/renderer/components/cto/pipeline/config/CloseoutConfig.tsx
- apps/desktop/src/renderer/components/history/eventTaxonomy.ts
- apps/desktop/src/main/services/state/kvDb.orchestratorMigration.test.ts
- apps/desktop/src/main/services/config/projectConfigService.automationExecution.test.ts
- apps/desktop/src/main/services/config/projectConfigService.processGroups.test.ts
- apps/desktop/src/main/services/config/projectConfigService.providers.test.ts
- apps/desktop/src/main/services/config/projectConfigService.notifications.test.ts
- apps/desktop/src/main/services/state/kvDb.laneWorktreeLockMigration.test.ts
- apps/desktop/src/main/services/config/projectConfigService.linearSync.test.ts
- apps/desktop/src/main/services/lanes/laneService.branchSwitch.test.ts
- apps/desktop/src/renderer/components/history/useTimelineLayout.ts
- apps/desktop/src/main/services/chat/agentChatService.suggestLaneName.test.ts
- apps/desktop/src/main/services/state/kvDb.missionsMigration.test.ts
- apps/desktop/src/main/services/config/projectConfigService.aiModeMigration.test.ts
- apps/desktop/src/renderer/components/settings/ChatAppearancePreview.tsx
- apps/desktop/src/main/services/orchestrator/orchestratorService.ts
- apps/desktop/src/main/services/state/kvDb.workerAgentsMigration.test.ts
- apps/desktop/src/main/services/state/kvDb.pipelineSettingsMigration.test.ts
- apps/desktop/src/renderer/components/settings/LaneTemplatesSection.tsx
- apps/desktop/src/main/services/orchestrator/orchestratorQueries.ts
- apps/desktop/src/shared/types/orchestrator.ts
- apps/desktop/src/main/services/git/gitOperationsService.branchSwitch.test.ts
- apps/desktop/src/renderer/components/settings/ProvidersSection.tsx
- apps/desktop/src/main/services/usage/usageTrackingService.ts
- apps/desktop/src/main/services/config/projectConfigService.laneEnvInit.test.ts
- apps/desktop/src/renderer/components/settings/ProxyAndPreviewSection.tsx
- apps/desktop/src/main/services/orchestrator/coordinatorAgent.ts
- apps/desktop/src/renderer/components/app/AppShell.tsx
- apps/desktop/src/renderer/components/cto/TeamPanel.tsx
- apps/desktop/src/main/services/missions/missionPreflightService.ts
- apps/desktop/src/main/services/onboarding/onboardingService.ts
- apps/desktop/src/main/services/ai/tools/universalTools.ts
- apps/desktop/src/main/services/missions/missionService.ts
- apps/desktop/src/renderer/components/lanes/LanesPage.tsx
- apps/desktop/src/main/services/cto/workerAgentService.ts
- apps/ade-cli/src/services/sync/syncRemoteCommandService.ts
- apps/desktop/src/renderer/onboarding/tours/firstJourneyTour.ts
- apps/desktop/src/main/main.ts
- apps/desktop/src/main/services/ipc/registerIpc.ts
- apps/desktop/src/main/services/state/kvDb.ts
- apps/desktop/src/main/services/orchestrator/coordinatorTools.ts
- apps/desktop/src/renderer/components/missions/missionThreadEventAdapter.ts
- apps/desktop/src/renderer/components/lanes/LaneGitActionsPane.tsx
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d380ab632b
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7a92f249d3
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 493ba96dd6
ℹ️ 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".
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/ade-cli/src/cli.ts (1)
10955-10961:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't reject
tcp://runtimes in the allow-spawn path.Line 10956 still turns every TCP mismatch into a hard failure because
isTcpSocketshort-circuits the branch. That contradicts the new TCP compatibility behavior: whenallowSpawnis true, the existing TCP connection should be kept instead of being rejected here.Suggested fix
if (mismatch) { - if (!allowSpawn || isTcpSocket) { + if (isTcpSocket) { + if (!allowSpawn) { + client.close(); + throw new Error( + `ADE runtime daemon ${mismatch}.`, + ); + } + return client; + } + if (!allowSpawn) { client.close(); throw new Error( `ADE runtime daemon ${mismatch}.`, ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/ade-cli/src/cli.ts` around lines 10955 - 10961, The current mismatch handling rejects on either !allowSpawn or isTcpSocket, which causes TCP runtimes to be hard-failed even when allowSpawn is true; change the condition so we only close the client and throw when spawning is not allowed (i.e., remove isTcpSocket from the OR). Update the if that checks mismatch to use if (!allowSpawn) { client.close(); throw new Error(...) } so that when allowSpawn is true the existing TCP connection (isTcpSocket) is retained.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/desktop/src/main/services/orchestrator/aiOrchestratorService.ts`:
- Around line 7058-7071: The current code picks the first failed run step
(failedRunStep) without checking per-step whether an open "failed_step"
intervention already exists, causing later failures to be ignored; change the
selection to iterate failed run steps (graph.steps.filter(s => s.status ===
"failed")) and choose the first step for which there is no matching open
intervention using the same matching logic (check mission.interventions entries
with status === "open" and interventionType === "failed_step", compare
metadata.stepId to resolved mission step id or metadata.orchestratorStepId to
run step id); set failedRunStep to that chosen step (and compute
failedMissionStep via resolveMissionStepForRunStep for that step) and only
return if no such step exists (also apply analogous logic when starting from
mission.steps if there are no failed run steps).
---
Outside diff comments:
In `@apps/ade-cli/src/cli.ts`:
- Around line 10955-10961: The current mismatch handling rejects on either
!allowSpawn or isTcpSocket, which causes TCP runtimes to be hard-failed even
when allowSpawn is true; change the condition so we only close the client and
throw when spawning is not allowed (i.e., remove isTcpSocket from the OR).
Update the if that checks mismatch to use if (!allowSpawn) { client.close();
throw new Error(...) } so that when allowSpawn is true the existing TCP
connection (isTcpSocket) is retained.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: be587e70-53b0-4494-971a-fd474cc3e3e9
📒 Files selected for processing (15)
apps/ade-cli/src/adeRpcServer.test.tsapps/ade-cli/src/adeRpcServer.tsapps/ade-cli/src/cli.tsapps/ade-cli/src/multiProjectRpcServer.test.tsapps/ade-cli/src/multiProjectRpcServer.tsapps/ade-cli/src/stdioRpcDaemon.test.tsapps/desktop/src/main/services/config/projectConfigService.test.tsapps/desktop/src/main/services/missions/missionService.tsapps/desktop/src/main/services/orchestrator/aiOrchestratorService.test.tsapps/desktop/src/main/services/orchestrator/aiOrchestratorService.tsapps/desktop/src/main/services/state/kvDb.migrations.test.tsapps/desktop/src/renderer/components/chat/AgentChatPane.tsxapps/desktop/src/renderer/components/shared/ModelPicker/ModelPicker.test.tsxapps/desktop/src/renderer/components/shared/ModelPicker/ModelPicker.tsxapps/desktop/vitest.workspace.ts
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Summary
Describe the change.
What Changed
Key files and behaviors.
Validation
How you tested.
Risks
Anything to watch.
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Greptile Summary
This is a large cleanup PR that consolidates scattered per-feature test files into single suite files, and ships three self-contained features alongside the reorganisation.
connection.ts(TUI client) andlocalRuntimeConnectionPool.ts(desktop) now validate that an attached runtime reportsdefaultRole === \"cto\"and a matching build hash; mismatches trigger automatic replacement of the old daemon.backendStyleon computer-use artifacts: a newComputerUseBackendStyleunion (external_cli | manual | local_fallback) replaces the previously hardcoded\"external_cli\"string;normalizeBackendStyledefaults to\"external_cli\"for null rows, keeping existing data valid.ModelPickergains aconstrainToAvailableModelIdsprop that limits selectable models to a caller-supplied allowlist;AgentChatPaneuses this whenavailableModelIdsOverrideis set and surfaces aconstrainedModelSelectionErrorthat blocks session creation when the active model is outside the allowed set.Confidence Score: 5/5
Safe to merge; all three feature additions are well-guarded and the daemon startup already handles the stale socket file via
fs.unlinkSyncbefore rebinding.The stale-daemon replacement path in
connection.tsis sound: the ADE daemon callsfs.unlinkSync(socketPath)before binding, so the new daemon can always claim the socket. The constrained model-picker feature properly guards the submit path through both the composer'ssingleModelReadycheck and theconstrainedModelSelectionErrorcallback insidecreateSession. ThebackendStyleaddition is backward-compatible. The only finding is a per-retry file-hash re-computation in the 25-attempt retry loop, which is inefficiency rather than incorrectness.No files require special attention; the stale-daemon retry logic in
apps/ade-cli/src/tuiClient/connection.tsis the most complex new path but works correctly.Important Files Changed
normalizeAdeRuntimeRoleandresolveAdeDefaultRolereplace inline role checks acrosscli.tsandconnection.ts.StaleAdeSocketError, and auto-respawn;computeCliEntrypointBuildHashis re-invoked on every retry in the 25-attempt loop instead of being memoized.defaultRolefield toreadRuntimeInfoand enforces that attached runtimes must reportdefaultRole === "cto"; mismatch throwsLocalRuntimeCompatibilityError, triggering the existingreplaceStaleRuntimepath that terminates the process and unlinks the socket.ComputerUseBackendStyleunion type and makesbackendStylea required field onComputerUseArtifactRecord, replacing the previously implicit hardcoded"external_cli"value.backendStylefrom the DB column;normalizeBackendStyledefaults to"external_cli"for null/unknown values, preserving backward compatibility with existing rows.constrainToAvailableModelIdsprop that restricts the selectable model list and disables registry expansion; fallback to unconstrained mode when the prop is false is correct.modelSelectionConstrained, wiresconstrainedModelSelectionErrorinto both session-creation paths, and propagates the constrained flag to the composer.constrainModelSelectionandmodelUnavailableMessageprops;singleModelReadycorrectly blocks submission when the model is absent or the unavailability message is set.switchCtoTabhelper and hooksbeforeEnterinto the team and workflows steps so the correct tab is pre-selected.Sequence Diagram
sequenceDiagram participant Client as TUI / Desktop Client participant Conn as connection.ts / localRuntimeConnectionPool participant Socket as Unix Socket participant OldDaemon as Old Daemon (wrong role) participant NewDaemon as New Daemon (role=cto) Client->>Conn: connectToAde / ensureProject Conn->>Socket: connect Socket-->>OldDaemon: ade/initialize OldDaemon-->>Conn: "runtimeInfo { defaultRole: "agent" }" Conn->>Conn: attachedRuntimeMismatchReason → role mismatch Conn->>OldDaemon: shutdown Conn-->>Client: StaleAdeSocketError Note over Conn: Wait 200ms, spawn new daemon Conn->>NewDaemon: "spawnDaemon (ADE_DEFAULT_ROLE=cto)" NewDaemon->>Socket: unlink + bind new socket loop up to 25 retries Conn->>Socket: connect NewDaemon-->>Conn: "runtimeInfo { defaultRole: "cto" }" Conn-->>Client: AdeCodeConnection established endComments Outside Diff (1)
apps/desktop/src/main/services/orchestrator/aiOrchestratorService.ts, line 7182-7200 (link)elsebranch callstransitionMissionStatuswithoutallowTerminalRestartafterensureTerminalFailedRunInterventionalready set the statusWhen
graph.run.status === "failed"and failed steps are present,ensureTerminalFailedRunInterventionadds afailed_stepintervention and callstransitionMissionStatus(mission.id, "intervention_required", { allowTerminalRestart: true }). BecausederiveMissionStatusFromRunnow evaluateshasBlockingInterventionbefore checkingrun.status === "failed", the newly-added intervention makes it return"intervention_required"— sonextMissionStatusends up in theelsebranch rather thanelse if (nextMissionStatus === "failed"). Theelsebranch then re-derives the status (same result) and callstransitionMissionStatus(mission.id, "intervention_required")a second time, this time without{ allowTerminalRestart: true }. IftransitionMissionStatusis not idempotent, this causes duplicate side-effects; the missingallowTerminalRestartflag on the second call could also silently change restart eligibility.Prompt To Fix With AI
Prompt To Fix All With AI
Reviews (8): Last reviewed commit: "Select uncovered failed mission steps" | Re-trigger Greptile