feat(desktop): complete Antigravity ACP plugin execution - #5224
Conversation
6a6b3be to
84bbed6
Compare
9767546 to
b278ff0
Compare
Astro-Han
left a comment
There was a problem hiding this comment.
Review — comment only
Targeted review of the ACP rebuild. I did not read all ~2.5k lines; I focused on packages/acp-executor-plugin/src/index.ts, packages/antigravity-acp-plugin/src/index.ts, packages/runtime-host/src/server/builtin-external-agent-plugins.ts, the generic boundary changes in packages/runtime/src/plugin-executor-{service,backend}.ts, the host wiring, and the tests.
The boundary design reads well and I think it's the right shape: ACP mechanics stay in one bundle, adapters own only launch policy, and routing/lifecycle stay with #5283's generic executor path. The items below are about robustness and one class of silent failure, not about the architecture.
Issues
1. AcpExecutor marks a conversation permanently "history-only" for every execution failure, not just process loss — packages/acp-executor-plugin/src/index.ts:212 and :215
execute()'s catch calls await this.#lose(session) unconditionally, and #lose (:508) sets session.lost = true, which makes #session() throw acp_history_only for that conversationKey for the rest of the Entry generation. That's correct for genuine process loss (owner.failed, cancel timeout at :499), but it also fires for:
- the 30s
INITIALIZE_TIMEOUT_MS(:59,:283) — a slow first spawn bricks the conversation; acp_config_unavailable/acp_config_invalidfrom#applyInitialConfig— a wrongmodelvalue in the Entry config permanently disables the conversation even after the config is corrected;- a transient
checkedExecutableENOENT while the agent app is being reinstalled; - a caller abort during
#ensureInitialized—#initializeusesAbortSignal.any([signal, timeout])(:283), sostartupSignal.throwIfAborted()on a user stop during the first prompt also lands here.
The follow-up error is also misleading: it reports "history-only because its external process was lost" when no process was ever started. Suggest only calling #lose() when the connection actually failed, and for other errors clearing session.initialization/session.owner so the next prompt can retry.
2. Agent-authored tool metadata and >64 diffs throw inside context.emit, and the ACP SDK swallows the error — packages/acp-executor-plugin/src/index.ts:421-470, packages/runtime/src/plugin-executor-service.ts:507
PluginExecutorService's emit wrapper calls normalizeOutputEvent outside its try/catch (plugin-executor-service.ts:296-306), so a validation failure throws back into #acceptTool, which runs inside the ACP session/update notification handler. The SDK's dispatch catches notification-handler errors and only console.error("Error handling notification", …) — the connection stays up, so the failure is silent in the product. Two realistic triggers:
#acceptToolpassesdisplayName: snapshot.titleandname: snapshot.name ?? …straight from the agent.normalizeOutputEventboundsdisplayNameat 8192 chars andnameat 256 chars with no\r\n(plugin-executor-service.ts:423-426). A long agent-authored title makestool_startthrow, and becausetoolUseIdsis only populated on a successfultool_start(plugin-executor-backend.ts#publishOutputEvent), the latertool_resultis dropped too and#closeOptionalOutputnever backstops it — the tool vanishes from the transcript entirely.projectToolResult(:781) bounds the combined diff atMAX_TOOL_RESULT_DIFFbut not the path count, whileisPluginToolResultContentrejectspaths.length > 64(plugin-executor-service.ts:507). One multi-file tool call (a rename across >64 files) makes thetool_resultemit throw;snapshot.terminal = trueis set before the emit (:457), so no latertool_call_updateretries it. The backend then backstops with the synthetic "External executor ended before reporting a tool result" for a tool that actually succeeded.
Suggest clamping displayName/name/paths before crossing the boundary (or degrading >64 paths to the text summary), setting terminal only after a successful emit, and wrapping #acceptUpdate in a try/catch that logs.
3. Retained ACP processes are unbounded — packages/acp-executor-plugin/src/index.ts:149, :255, :226
#sessions only ever grows (set in #session, cleared only in dispose), and every entry keeps a live child process plus its tree. One retained process per Maka conversation key, for the lifetime of the Entry generation, with no idle eviction, cap, or LRU. A long-running Host will accumulate one Antigravity process per conversation ever started. Either bound it (evict idle sessions — the continuity marker already encodes the "history-only" consequence) or state the limit explicitly in the README so operators know.
4. No authentication handling on the execution path — packages/acp-executor-plugin/src/index.ts:579
child.stderr is drained to nowhere and initializeResponse.authMethods is ignored (the SDK exposes it at dist/schema/zod.gen.js:1134, along with an authenticate method). The setup path merged in #5164 does parse the auth line out of stderr (packages/runtime-host/src/server/acp/antigravity.ts:144-160). If the saved Antigravity login expires, the first prompt fails as acp_execution_failed / "ACP execution failed" with no re-auth affordance and nothing logged. At minimum, surface authMethods as a distinct code so the Desktop can route back into the existing setup flow.
5. A plugin-projection failure requests a Host drain — packages/runtime-host/src/server/execution-composition.ts:1962-1967
applyRuntimePolicyMutationEffects now runs builtinExternalAgentPlugins.reconcile() inside the existing context.requestDrain(); throw error; path. The setting is already committed by then, and HostPluginPlatform already records the failure and schedules its own reconcile (#recordPackageFailure / #scheduleReconcile), so draining the whole Host because a derived, replaceable package layer failed to install seems heavier than needed. Worth confirming this is intended.
Nits
- Dropped
SessionUpdatekinds —#acceptUpdate(:405) handles 4 of the 14 kinds in the SDK union.config_option_updateis dropped, so the in-memorysession.configOptionsgoes stale once the agent changes a value mid-session (relevant to Set B);plan/plan_updateare dropped as well. A debug log for dropped/unknown kinds would make agent behavior diagnosable. - Diagnostics are discarded —
errorCode(:879) collapses every non-AcpRuntimeErrortoacp_execution_failedandsafeErrorMessage(:886) to'ACP execution failed', with nocauseand no log line. This makes issues 1 and 4 very hard to diagnose in the field. - String-sniffed error channel —
active.text.trimStart().startsWith('Agent execution error:')(:200) treats model-authored transcript text as a failure signal. A response that legitimately begins with that phrase fails the turn. Prefer an explicit stop reason /_metasignal if Antigravity exposes one. supportsAttachmentsis static — the initialize response already reportsagentCapabilities.promptCapabilities(image/audio/embeddedContext). Deriving support from the negotiated capabilities would avoid failing a whole turn withacp_attachments_unsupportedfor agents that do accept images.- Prompt flattening —
promptTextfolds instructions, quotes, and directory references into prose prefixes inside onetextblock. ACPContentBlocksupportsresource_link/resource/image, which would carry that structure instead of text the agent may act on. Relatedly,session/newsendsmcpServers: [], so ACP agents get none of Maka's MCP servers — worth stating that explicitly. dispose()throwing breaks teardown —terminate()throws'ACP process cleanup failed'if the child is still alive at the deadline, anddispose()aggregates that, so a stuck child makes the fiber's effect cleanup fail during plugin uninstall/reload. Consider logging and continuing. (terminatealso has no finalclose/exitawait, so a child exiting just past the last poll is a false positive.)createWholeFileDiff(:812) emits a single whole-file hunk with no\ No newline at end of filemarker, so a diff whoseoldText/newTextlacks a trailing newline is technically malformed for strict patch consumers.- Bundled host runtime code —
package.jsonlists@maka/runtimeas a devDependency, butindex.tsvalue-importsterminateChildProcessTreefrom@maka/runtime/process-tree-terminator, so that implementation is esbuild-bundled intodist/plugin.mjs. That's consistent with the deliberate "no cross-bundleinstanceof" isolation, but it means the shipped bundle carries its own copy that won't track@maka/runtime. Worth a line in the README. - Docs self-reference —
docs/antigravity-acp-plugin-rebuild.mdopens with "Why PR #5224 cannot be carried forward unchanged" / "PR #5224 predates #5283", but this is PR #5224, so the doc reads as arguing against itself. Naming it "the pre-#5283 revision of this PR" would fix it. - Non-hosted permission denial —
PluginExecutorBackend.#requestPermission(plugin-executor-backend.ts:186) returns{ outcome: 'cancelled' }wheneverinput.hostedInteractionis absent, so every ACP permission request is denied for CLI/API/scheduled execution. Safe default, but worth documenting since it silently narrows what external agents can do headlessly.
Verified / no action needed
- Recovery ordering is correct.
builtin-external-agent-pluginsis registered afterplugin-platformin the module array (execution-composition.ts:2504, platform module ~:2494), andrecoverRuntimeHostDomainModulesiterates in array order, sorecover()'spackageProjections()/installPackage()hit a platform where#assertReadable()passes. Good — I checked this specifically because the coordinator depends on platform recovery. - Digest idempotency holds.
extensionPackageDirectoryContentDigestuses the same sorted-path + content hash asPluginPackageStore.decodePackage, andprepareInstallcopies into its own transaction directory before the staging dir is disposed, so the "no generation churn on restart" claim is sound (andbuiltin-external-agent-plugins.test.tsasserts a stableauthorityEpoch). - Packaging.
dependenciesis inWORKSPACE_RELEASE_MANIFEST_FIELDS, so@maka/acp-executor-plugin/@maka/antigravity-acp-pluginreach the packaged app through runtime-host's production closure (files: ["dist"]includesplugin.mjs). - Adapter isolation.
isolate: { acp: true }gives theacp-runtimeEntry its ownacplabel that children inherit;ctx.provide('acp', …)is on the runtime Entry's Context and the adapter passes its own Context explicitly, soexecutors.registerscopes to the consumer Entry (PluginExecutorService.registerreadsthis.ctx, whichService._bindrebinds per consumer). - Cancellation and teardown.
#awaitPromptsendssession/cancel, waits for settlement, and force-terminates only on the 15s timeout;PluginExecutorService's retirement path aborts active executions and awaits settlement, and the effect cleanup disposes the provider (process trees included). - Antigravity launch policy matches the live setup path in
packages/runtime-host/src/server/acp/antigravity.ts(BROWSER=/usr/bin/true,PYTHONUNBUFFERED=1,ANTIGRAVITY_HARNESS_PATH,cwd = dirname(executable),localharness_externalhelper precheck). - Generic boundary changes are additive and validated symmetrically —
PluginExecutorToolResultContentis a bounded discriminated union,file_diffalready exists in the canonicalToolResultEventshape, andnormalizePermissionRequest/normalizePermissionResultvalidate both directions with the form withdrawn on abort. - Concurrency. One prompt per conversation is enforced by
session.active(acp_busy), andsession.initializationde-dupes concurrent initialization.
Test coverage vs. the PR checklist
The checklist says tests cover "lifecycle, continuity, cancellation, permission bridging, diff projection, and adapter registration". acp-executor-plugin.test.ts has 3 tests (retention, history-only, cancellation); permission bridging and file_diff are asserted only incidentally inside the first. The "Behavior and safety" claims with no test: workspace containment including symlink escape for fs/readTextFile / fs/writeTextFile, the 8 MiB file cap, the diff-size degrade path, setConfigOption validation, adapter/config validation, process-tree termination on dispose, and the durable pluginStateStore (only a fake store is exercised). Containment and the diff-degrade path are the two I'd add first, since they're the security/robustness claims.
Coordination
#5283 is the merged base and #5222/#5385/#5386 are CLI-side ACP work that doesn't touch these packages, so I don't see a conflict. The one seam worth aligning is mcpServers: [] above: #5386 adds session-scoped ACP MCP on the CLI side, and the runtime plugin currently opts out entirely. Similarly, this PR's "Set C" (agent questions, unsupported-input presentation) overlaps #5385's interaction mapping — worth a quick sync so the two don't land incompatible contracts.
The two existing PR commits were already incorporated on the newer main baseline. Preserve their ancestry while retaining the fully tested PR2 tree.
e53fc4e to
421ca51
Compare
8691bc9 to
7d7f4f5
Compare
7d7f4f5 to
f555e4b
Compare
717c84b to
82b5696
Compare
Generated-by: OpenAI Codex
Generated-by: OpenAI Codex
Connect provider catalogs and exact model configuration to Desktop, preserve question option identities and drafts, and project process loss as history-only. Add controlled-process coverage and official Antigravity Desktop acceptance evidence. Generated-by: OpenAI Codex
Generated-by: OpenAI Codex
Separate executor selection, share native model rows, keep model panels the same height, and show full external model names without plugin icons. Generated-by: OpenAI Codex
Include both ACP packages in the incremental build graph and build the exact Host plugin entrypoints before launch. Exercise replacement of stale bundles and clean adapter output. Generated-by: OpenAI Codex
Preserve executor-specific models, reasoning effort and client model slots from main. Keep confirmed ACP configuration distinct, reject conflicting model targets, and align relocated selection state and protocol compatibility. Generated-by: OpenAI Codex
Canonicalize catalog-managed executor model inputs across main and PR2 entrypoints. Retain the provider stop reason through cancellation and the durable runtime ledger, with regression coverage for settlement races. Generated-by: OpenAI Codex
Generated-by: OpenAI Codex
Generated-by: OpenAI Codex
Render Maka models directly in the shared searchable panel and keep native thinking beside the composer model trigger. Preserve selection and retry behavior, cover thinking placement, and refresh attachment-only UI evidence. Generated-by: OpenAI Codex
Match main native model formatting, reuse Gemini provider icons, and remove the synthetic Agent default option. Generated-by: OpenAI Codex
Keep the panel helper import separate from the wheel helper removed on main, preventing a clean textual merge from leaving an unresolved reference. Generated-by: OpenAI Codex
Expose verified Antigravity model families and opaque variant IDs as structured catalog capabilities. Select the highest supported intensity when choosing a base model, keep all intensity selection in the composer, and display only confirmed external configuration. Preserve native model presentation and protect rollback, retry, scope changes and execution locks with regression tests. Generated-by: OpenAI Codex
Preserve main's external-session workspace import contract and native model formatting. Remove the obsolete model description helper dependency exposed by PR merge CI, and advance the combined protocol and compatible declaration to epoch 181. Record clean builds, all workspace suite results and serial release validation. Generated-by: OpenAI Codex
Generated-by: OpenAI Codex
Reconcile the renderer architecture and Astryx inventories with current main, and keep the Antigravity entry visible while ACP discovers live models. Generated-by: OpenAI Codex
a6c35a9 to
48eb1c2
Compare
Generated-by: OpenAI Codex
Record process continuity as soon as ACP creates a session, before model configuration can fail. Emit correct whole-file hunk counts and newline markers, with regression coverage. Generated-by: OpenAI Codex
Reconcile confirmed executor models after failed Host commits, retain terminal tool text beside diffs, and reserve continuity before ACP session creation. Generated-by: OpenAI Codex
Generated-by: OpenAI Codex
Keep built-in setup placeholders in the Host catalog and let registered Plugin entries replace them. Show a generic loading state in the shared picker, with regression coverage and current UI stories. Generated-by: OpenAI Codex
a5fe2ef to
bb05a3f
Compare
Resolve the renderer architecture ledger against the merged source and retain the generic executor catalog loading boundary. Generated-by: OpenAI Codex
Generated-by: OpenAI Codex
Generated-by: OpenAI Codex
Generated-by: OpenAI Codex
|
I reviewed this from the problem/mechanism/scope perspective at head The problem and execution boundary are well chosen: Maka needs to act as an ACP client for an agent that owns its own session and tools, rather than presenting Antigravity as an ordinary LLM connection or duplicating the conversation UI. Reusing the generic Plugin executor path from #5283, keeping ACP transport/lifecycle in the shared runtime, and limiting the Antigravity adapter to launch policy and verified catalog interpretation is a coherent solution. The real-agent acceptance evidence for model confirmation, tool/diff projection, interactions, follow-up and cancellation supports that mechanism. I would retain this architecture rather than replace it with an Antigravity-specific Host path. From an Occam’s razor perspective, the remaining question is whether every retained state and shared contract is necessary for this vertical slice. My main actionable concern is process ownership: I would also keep the acceptance language precise: PR 2 provides readable history, not cross-process continuation; I am not repeating the earlier projection/metadata/diff findings as current blockers: the head now bounds tool metadata, degrades oversized diff projection and sets terminal state after emission. My conclusion is that the boundary design is convergent, while process retention and scope claims need one more pass before calling the implementation fully converged. |
Astro-Han
left a comment
There was a problem hiding this comment.
On the merge-vs-split question: merge this as one PR. I traced the dependency edges across all 126 files — the chain is strictly linear (core/runtime contracts → host + protocol + storage + IPC → plugin packages + UI), and every candidate cut produces a worse intermediate state:
- Plugins first doesn't compile (
@maka/core/executor-catalogand the newPluginSessionExecutorsurface don't exist on main). - Infra first either crashes Host boot —
execution-composition.ts:421unconditionally constructs the builtin coordinator whose constructorimport.meta.resolves the two package specifiers — or ships aplugin.executor.query+ 3 IPC channels with no producer or consumer. - UI first renders a picker whose every request fails against a nonexistent op.
- Protocol/storage first is dead schema.
This is already the minimal end-to-end slice: the executor seam landed earlier (#5283, #5164), and this PR is its first real consumer — which is exactly what #5103 requires ("every production contract must ship with its real producer and consumer in the same PR"). The one technically-landable two-PR cut (infra, then packages+UI) would require making the coordinator's package resolution lazy first and ships a placeholder-only catalog in between — dead surface bought only for review size. Recommend against it.
Correctness: verified clean on the risky paths. I followed selection → IPC → coordinator → executor service → ACP process → events back, through failure/retry/cancellation/recovery. The invariants all close: external changes commit before durable state, uncertain outcomes → history_only, process loss → readable history with explicit "start a new task" gating, permission/question identity binding through admitFormRequest, bounded cancellation drain preserving real stop reasons, filesystem callbacks with realpath/O_NOFOLLOW/dev-ino containment, epoch 181 covering the new wire surface. The prior-round fixes (bounded tool metadata, retained lifecycle events on projection failure, retry before session exists, fs/process ownership checks) all verified.
Three P3 findings inline — none blocking. Also worth a cleanup pass at some point (not merge gates):
PluginExecutorService.execute()/list()are test-only public methods that bypassbind()'s scope check;ExecutorModelPickerProps.scopeKeyis a dead prop (thekey={scopeKey}on the JSX does the remount);ExecutorModelPickerBoundaryis a pure forwarding wrapper.docs/antigravity-acp-pr2-acceptance.mdreads as a run log —docs/README.mdkeeps those on the PR thread or indocs/archive/.plugin.platform.query's'executors'view and theexecutorConfignested channel on session create appear to have no production consumer yet — fine if they're PR3/PR4 groundwork, worth a comment saying so.- The ACP-runtime vs Antigravity-adapter package split is a real boundary with a documented second-adapter plan — keep it.
Nice work — this is a large diff but the layering is disciplined, the staged plan is honest about what PR3/PR4 still owe, and the verification story (real ACP 1.1.1, real Electron) is solid.
中文
关于拆分:建议一次合入。 126 个文件的依赖链严格线性,每个候选切点都会产生更差的中间态:先落插件不编译、先落 infra 要么 Host 启动即抛要么落一堆无消费者的查询面、先落 UI 会渲染一个永远报错的 picker、先落协议是纯死 schema。main 上的 executor seam(#5283/#5164)已经在了,本 PR 是它的第一个真实消费者——这正是 #5103 明文要求的"契约与生产消费者同 PR"。唯一可行的二切也得先把 coordinator 的包解析改 lazy,买到的只是评审体量,不解锁任何阻塞。
正确性:沿真实生产路径(选择 → IPC → coordinator → executor service → ACP 进程 → 事件回流)走完失败/重试/取消/恢复全链,核心不变量全部闭合——进程丢失留可读历史、取消有界且保留真实 stopReason、权限身份精确绑定、fs 回调容器校验完整、epoch 181 覆盖新 wire 面。三条 P3 在行内,均不阻塞。
可删小件(不挡合并):execute()/list() 仅测试调用、scopeKey 死 prop、纯转发 wrapper、acceptance 文档按治理应进 archive 或 PR 正文、executors 视图与 executorConfig 通道疑似暂无生产者(若是 PR3/PR4 地基请注明)。ACP 双包拆分建议保留。
Review generated with AI assistance (Devin); verified against head 7fc76eb.
|
|
||
| async discover(input: { cwd: string; signal: AbortSignal }): Promise<ExecutorCatalogEntry> { | ||
| if (this.#disposed) return this.#catalogEntry('unavailable'); | ||
| if (this.#catalog) return this.#catalog; |
There was a problem hiding this comment.
P3 — trigger ② (failure/staleness path): the catalog caches permanently after the first successful discover(). #catalog is never invalidated on the success path — if the agent's model catalog changes without an executable-path change (Antigravity's models are server-side, so this is the expected case), the picker won't see new models for the life of the Host process, and assertExecutorAvailable (execution-composition.ts) will reject a valid new model as unavailable. Failure direction is safe (unavailable/auth entries aren't cached and can retry). A TTL, or invalidating on acp_config_invalid/policy change, would close it.
中文
discover() 成功后 #catalog 在 executor 生命周期内不再刷新——agent 服务端下发新模型时(Antigravity 正是这类)picker 长期看不到,assertExecutorAvailable 还会把有效新模型误判 unavailable,直到 Host 重启。失败路径不缓存可重试,方向安全;加个 TTL 或在配置失效时清缓存即可。
There was a problem hiding this comment.
I agree that the cache can become stale when Antigravity changes its server-side models. I am keeping the successful catalog cached for PR 2 because #5103 assigns account/catalog invalidation and refresh reconciliation to PR 4. Adding a TTL here would introduce refresh timing and selected-model reconciliation behavior before that slice. I have made the current Entry-lifetime cache and Plugin-reload workaround explicit in the README and PR scope, so the limitation is visible rather than presented as live refresh. I would leave the lifecycle change to PR 4.
|
|
||
| function normalizeResult(result: PluginExecutorResult): PluginExecutorResult { | ||
| if (!result || typeof result !== 'object') throw new TypeError('Executor result is invalid'); | ||
| if (result.status === 'completed' && typeof result.text === 'string') { |
There was a problem hiding this comment.
P3 — trigger ③ (trust boundary): completed.text passes through unbounded. Every other event text goes through isSafeEventText (≤8192), but a completion's accumulated text is emitted as-is — a runaway or misbehaving ACP agent can write an arbitrarily large single durable event, which may hit a store/transport per-event limit and end the turn in error. Same exposure class as existing streaming aggregation, but the single-event shape adds a real boundary. A cap in normalizeResult (or chunking text_complete in the backend) covers it.
中文
completed.text 无界:其他事件文本都过 ≤8192 的 isSafeEventText,唯独 completion 的累积全文原样落 durable event——失控 agent 可写任意大单条事件。在 normalizeResult 加上限或后端分片即可。
There was a problem hiding this comment.
Fixed in 9483a78. normalizeResult now rejects completed text above 256 KiB in UTF-8 bytes, so the backend emits a bounded failure instead of a single oversized text_complete durable event. The focused backend test verifies that a streamed delta remains followed by error/complete, with no large completion event. The test also passed after the latest main merge.
| const info = await file.stat(); | ||
| if (info.size > MAX_TEXT_FILE_BYTES) throw new Error('ACP text file is too large'); | ||
| const text = await file.readFile('utf8'); | ||
| const start = line ? line - 1 : 0; |
There was a problem hiding this comment.
P3 — trigger ③ (trust boundary): negative line returns the file's tail instead of an error. start = line - 1 with line: -3 slices from the end. Only affects files the agent could already read in full — no boundary escape — but it's accidental semantics. Reject line < 1 or limit < 0; also fine to leave.
中文
line 为负时 slice 返回文件尾部而非报错。agent 对本就有权整读的文件,无边界突破;顺手加个 line < 1 校验即可,不改也能接受。
There was a problem hiding this comment.
Fixed in 9483a78. File reads now reject a non-positive or non-integer line and a negative or non-integer limit before opening the file. limit: 0 now returns an empty range. A focused filesystem test covers the invalid values and a normal slice, and passed after the latest main merge.
|
Thanks for the focused review. I agree that retaining one process per conversation creates an unbounded resource cost over a long-running Host. For PR 2, I would take the documentation option you suggested. The #5103 checklist explicitly treats a session waiting for its next turn as active and requires its process to remain available while idle; closing the UI does not retire it. Evicting that process now would make an otherwise usable task history-only until PR 3 provides restoration. I will make the current limit and tradeoff explicit in the PR description and README: there is no idle timeout or process-count cap, and owned processes are cleaned up on task retirement and Plugin/Host teardown. I will track a bounded-retention policy separately, informed by PR 3's restoration behavior; I am not assuming PR 3 itself will implement that policy. I will also tighten the scope statements: PR 2 keeps history readable after process loss but cannot continue the same external session; |
Generated-by: OpenAI Codex
|
Follow-up on my reply above: commit I opened #5620 to decide bounded process retention separately after the PR 3 restoration behavior is established; PR 3 itself is not being assigned that policy. In the shared-contract pass, I removed the unused optional Full workspace typecheck and the focused Core executor-catalog tests pass locally. Would the documented PR 2 tradeoff address your process-ownership concern? |
Document catalog cache scope and archive PR2 acceptance evidence. Generated-by: OpenAI Codex
Resolve the Host compatibility epoch collision at 182. Generated-by: OpenAI Codex
|
Thanks for the approval and the dependency review. I kept this as one vertical PR, addressed the three P3 comments inline, and merged current main ( For the optional cleanup notes: I removed the unused picker After the main merge, the full workspace build and typecheck, the merge-result protocol epoch guard, and 162 focused tests passed locally. Fresh GitHub checks are pending. |
Regenerate the Astryx surface inventory for the combined UI file set. Generated-by: OpenAI Codex
Summary
Implements the Antigravity ACP PR 2 execution slice from #5103: authenticated catalog discovery, executor/model selection, retained sessions, hosted permissions and questions, cancellation, and readable history after process loss.
gemini-pro-agent, not an inferred suffix. The Antigravity adapter owns label recognition and exposes structured capabilities to generic UI.Refs #5103.
CI fixes
The earlier protocol declaration mismatch and Astryx raw-control inventory failures are fixed. The four failing checks on
def615305shared a later cause: GitHub's synthetic merge with main removedmodelChoiceDescription, leaving references in the PR's model panel. The final integration removes that obsolete dependency while retaining searchable catalog metadata and main's native formatting.Current main
b004473edis incorporated, including Host-owned WorkHub result turns and the running-state UI update. The compatibility epoch is 182, and the merge-result protocol guard passes against main at epoch 178. The Astryx surface inventory was regenerated for the combined file set. Earlier Storybook smoke passed 392 stories / 424 theme renders after its loading-state timing fix. Fresh GitHub checks for the new merged head are pending.Verification
After the
b004473edmerge: full workspace typecheck and format passed; Astryx inventory coverage passed for 295 files, its 19 generator tests passed, and 18 affected UI tests passed. GitHub reports the PR as mergeable; fresh CI is running.After the
bc0786ee6merge: full workspacebuild:testand typecheck passed; 162 focused ACP process, Runtime executor, Host composition/protocol, and UI picker tests passed. The protocol epoch guard passed against current main. Earlier Storybook, lint/format, Host catalog, and renderer architecture checks belong to the previous integration checkpoint.After integration with main
0052f1cfd: all 13 workspace suites passed after rechecks, 12,995 Node tests passed / 38 skipped. The initial run had two intermittent failures; focused checks and complete Host/Storage suite reruns passed without code changes.Clean build, production renderer, all workspace type checks, lint/format, ASF/locale, architecture, Desktop/UI Knip, Astryx/Windows inventories, shell hooks and protocol guards passed. Release checks passed 203 tests serially; an earlier concurrent run hit test-worker deserialization and shutdown-timing failures.
Independent linkage checkpoint: 9,916 tests passed, 26 skipped, across Core, Runtime, Runtime Host, Desktop, UI, ACP executor and Antigravity adapter. The UI suite contains 633 tests.
Regression cases cover full/partial/unsorted levels, opaque IDs, unknown/ambiguous models, automatic highest selection, failed confirmation and rollback/retry, same-model confirmation, scope races, catalog notifications, execution locks, native reselection and executor switching.
Official ACP 1.1.1 returned 11 real variants forming four base models. Actual configuration responses confirmed Flash Medium, Pro High (
gemini-pro-agent) and Pro Low. IDs are retained verbatim.Real Electron verification: native rows/search, automatic High when choosing an external model, footer-only thinking, Flash Low/Medium/High, Pro Low/High, and consistent selection on reopening. A real Pro High task returned
THINKING_UI_OK; an idle footer switch was acknowledged asgemini-3.1-pro-low, then displayed Low. Running tasks lock model changes.Full-window Electron screenshots below show the official ACP 1.1.1 catalog in Maka. The isolated native connection is a fixture. The generic loading state is covered by Storybook smoke and picker tests; it is not pictured below.
Detailed acceptance report
Full-window UI evidence
Full Maka application-window captures from the real Electron Desktop on 2026-09-22, using the official Antigravity ACP 1.1.1 catalog and an isolated native-connection fixture. Each image includes the whole 1162 × 768 Maka window rather than a cropped picker. Images are GitHub attachments; no screenshot binaries are committed. The generic discovery loading state is verified by Storybook smoke but not shown in these captures.
Catalog ready: Antigravity base models
Composer: Flash thinking control
The acceptance report also shows Pro thinking and Maka's native model list.
Scope and limits
Verified against official ACP 1.1.1 on macOS arm64. New-task choices are validated against the ready catalog; the real ACP session confirms the ID on its first prompt. Unrecognized future label shapes remain raw options until verified. If rollback cannot be confirmed or an established process is lost, the session stays history-readable and requires a new task. PR 3 owns cross-process restoration; PR 4 owns modes and expanded catalog lifecycle.
After its first prompt, each conversation retains one ACP process even while idle, as required by #5103's PR 2 continuity rule. Closing the Desktop UI does not retire the task. PR 2 has no idle timeout or process-count cap, so a long-running Host can accumulate processes. Task retirement and Plugin/Host teardown clean up owned process trees. This keeps the same Agent Session available for follow-up turns while PR 2 cannot restore it after process loss. The bounded-retention decision is tracked separately in #5620; PR 3 does not itself commit to implementing that policy. The first successful model catalog is cached for the executor Entry lifetime; server-side changes without an Entry replacement require a Plugin reload until PR 4 adds invalidation and refresh reconciliation.
session/newcurrently passesmcpServers: [], so Maka MCP servers are not forwarded to Antigravity. The PR 2 execution run reused an existing Google login; fresh interactive login was verified in PR 1, and recovery from expired authentication during execution has not been established here. Public CI and independent human review remain merge gates.AI use
Select exactly one:
Tool(s) and scope: OpenAI Codex contributed implementation, tests, review fixes, verification, documentation, screenshots, and PR preparation. The new implementation and merge commits, plus the two previously missing AI-authored commits, carry a
Generated-by: OpenAI Codextrailer.Checklist
Does this PR entail a change in behavior?