fix(cli): support custom ACP history synchronization - #335
Conversation
Model: gpt-5.6-luna
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f5d56fe91e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| candidate.cliType === this.provider.cliType && | ||
| candidate.agentType === this.provider.agentType | ||
| ); | ||
| const resolved = { ...this.provider, customAcp: config?.customAcp }; |
There was a problem hiding this comment.
Propagate the custom agent environment into history spawns
When a custom ACP config relies on config.env for API keys, base URLs, or PATH, this resolver retains only customAcp; the history client consequently builds the child environment from the daemon process and login shell rather than the selected machine config. Normal sessions launch with the config environment, so history listing, import, and conflict replay can now start the correct executable under the wrong credentials or endpoint and fail for otherwise working custom agents. Include the authoritative config environment in the resolved history launch and merge it before spawning.
Useful? React with 👍 / 👎.
Model: gpt-5.6-luna
|
Addressed the P1 review in commit History sync now resolves the selected machine's authoritative |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 87f89b469c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| return ( | ||
| isObjectRecord(value) && | ||
| (value.cliType === 'builtin' || value.cliType === 'registry') && | ||
| (value.cliType === 'builtin' || value.cliType === 'registry' || value.cliType === 'custom') && |
There was a problem hiding this comment.
Allow custom providers through the desktop IPC guard
In the OSS Electron flow, the renderer routes local-project history operations through LocalProjectsIpc.syncHistory/importHistory/resolveHistoryConflict, but apps/electron/src/main/ipc/services/local-projects-ipc.ts:59-67 still accepts only builtin and registry. Consequently, selecting a newly exposed custom provider returns Invalid history provider before the request reaches this updated validator or the CLI, so custom ACP history synchronization remains unusable from the desktop. Update the Electron guard alongside this accepted provider set.
Useful? React with 👍 / 👎.
Related issue
Closes #187
Problem / pressure
The local-project history UI lists every agent configuration, including custom ACP agents, but the history sync launch path only received
cliTypeandagentType. Builtin and registry agents can resolve their executables from static definitions; a custom agent's executable is stored in its machine-scopedcustomAcpconfiguration. As a result, every custom history sync failed before spawning the ACP process withCustom ACP <agentType> has no launch command configured.Summary
customAcplaunch spec through the shared history catalog client so session listing and replay use the same command and arguments as normal custom ACP sessions.Before / after
customAcpcommand before spawning history ACP processes.Test plan
corepack pnpm --filter acp-extension-core build— passed, required local ACP package entry was built.corepack pnpm --filter acp-extension-claude build— passed, required local workspace entry was built.corepack pnpm --filter lody typecheck— passed.corepack pnpm --filter @lody/shared typecheck— passed.corepack pnpm --filter lody exec vitest run tests/history-session-catalog-client.test.ts -t 'custom ACP'— passed, 1 test.corepack pnpm --filter @lody/shared exec vitest run tests/local-project-control.test.ts— passed, 29 tests.corepack pnpm --filter lody exec vitest run tests/local-project-history-sync-service.test.ts— passed, 30 tests.git diff --check— passed.@lody/sharedsuite — 1,012 passed, 3 unrelated baseline failures (one local CLI host-lease assertion and two source-guard tests timed out); no changed history-control test failed.Context handoff
Instructions for reviewing agents
LocalProjectHistorySyncService.resolveLaunchProvider, the three history operation paths, andhistory-session-catalog-client.tsto ensure only daemon-authoritative machine config supplies the custom command.cliType:agentTypeis the right identity boundary and that keepingcustomAcpout of the control-plane wire schema avoids trusting caller-supplied executable paths.Authoring context