You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maka.apache.org by "Sun-GLiang (via GitHub)" <gi...@apache.org> on 2026/09/24 03:07:55 UTC
[PR] feat(acp): restore Antigravity sessions after restart [maka]
Sun-GLiang opened a new pull request, #5670:
URL: https://github.com/apache/maka/pull/5670
## Summary
Maka could read an Antigravity task after its ACP process or Host restarted, but could not continue the same external Session. This PR persists the external Session ID and a versioned prompt checkpoint in Plugin-private storage, then restores a committed task through the Agent's negotiated `session/resume` method. The original Session and saved model are retained; no earlier prompt is resent and restoration never falls back to `session/new`.
A completed turn becomes committed only after Maka has consumed its canonical terminal event. If a prompt may have progressed beyond saved history, `session/load` replay is kept outside the canonical conversation and the task shows an explicit history gap. Desktop now shows restorable, restoring, restore-failed, and history-gap states with Restore, retry, or New Task actions as appropriate. Tasks created before this change remain history-only because they have no saved external Session ID.
Refs #5103
## Verification
- `npm run build`, `npm run typecheck`, `npm run lint`, and `npm run format:check` passed.
- `npm run check:renderer-architecture`, `npm run check:locale-hygiene`, and the protocol compatibility epoch guard passed. The epoch advances from 183 to 184.
- All workspace suites passed with `node scripts/run-workspace-tests-parallel.mjs --concurrency=1` on the branch rebased to current `main`.
- Controlled real-stdio tests cover a forcibly killed process after a durable acknowledgement, same-Session continuation in a fresh process, and uncertain load replay without canonical duplication.
- Official Antigravity ACP 1.1.1 on macOS arm64 resumed the same external Session across processes and recalled a synthetic token. A loaded Session also recalled a synthetic token previously read by a tool after the toy file was removed. The built production Plugin bundles reported `restorable` → `ready` and recalled prior context in a fresh process. Sanitized evidence and the replay decision are in `docs/archive/antigravity-acp-pr3-acceptance.md`.
An earlier serial test run hit one intermittent Runtime Host Goal handoff assertion unrelated to ACP. The complete Host workspace passed independently, that test passed in isolation, and the final serial run passed.
## Review focus
The official Agent replay did not expose stable canonical event identities for every observed turn. This PR deliberately reports a history gap for an uncertain prompt instead of guessing which replay notifications to append. The Plugin-private record binds the saved Session to the exact executable and helper bytes, workspace, and adapter identity.
## Remaining draft verification
- [ ] Exercise a full signed-in Desktop UI restart and capture the restored task surface. The real Agent protocol probe, built Plugin smoke, and Desktop picker/controller tests already pass separately.
- [ ] Review CI results.
## AI use
- [ ] No generative tool made a substantive contribution
- [x] Generative tooling made a substantive contribution
Tool(s) and scope: OpenAI Codex implemented the ACP restoration path, tests, Desktop state, and acceptance documentation. Affected commits include `Generated-by: OpenAI Codex` trailers.
## Checklist
- [x] Tests cover the change and fail without it
- [x] Lint, format, typecheck and the affected suites pass locally
Does this PR entail a change in behavior?
- [x] Yes — described under Summary above
- [ ] No
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] feat(acp): restore Antigravity sessions after restart [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han commented on code in PR #5670:
URL: https://github.com/apache/maka/pull/5670#discussion_r4122585680
##########
packages/acp-executor-plugin/src/__tests__/acp-executor-plugin.test.ts:
##########
@@ -42,6 +43,448 @@ const adapter: AcpAgentAdapter<{ executable: string; model?: string }> = {
}),
};
+function durableState() {
+ const values = new Map<string, unknown>();
+ let rejectCommit = false;
+ const state: AcpConversationStateStore = {
+ has: async (key) => values.has(key),
+ mark: async (key, cwd) => {
+ values.set(key, { version: 1, cwd });
+ },
+ read: async (key) => structuredClone(values.get(key)),
+ write: async (key, record) => {
+ if (rejectCommit && record.phase === 'committed') throw new Error('Commit unavailable');
+ values.set(key, structuredClone(record));
+ },
+ };
+ return {
+ state,
+ values,
+ rejectCommit: () => {
+ rejectCommit = true;
+ },
+ record: (key = 'session-a') => values.get(key) as AcpContinuityRecord,
+ };
+}
+
+test('durably acknowledged ACP Session resumes the same external ID without replay or session/new', async () => {
+ const fixture = await executableFixture();
+ const protocol = fakeProtocol();
+ protocol.supportsRestore = true;
+ const storage = durableState();
+ const make = () =>
+ new AcpExecutor(
+ adapter,
+ { executable: fixture.executable },
+ {
+ state: storage.state,
+ createConnection: protocol.factory,
+ },
+ );
+ const first = make();
+ try {
+ assert.equal((await first.execute(request('first'), executorContext([]))).status, 'completed');
+ assert.equal(storage.record().phase, 'prompt_pending');
+ await first.acknowledgeExecution('session-a', 'turn-first');
+ assert.equal(storage.record().phase, 'committed');
+ assert.equal(storage.record().committedPrompts, 1);
+ await first.dispose();
+ const restored = make();
+ try {
+ assert.equal(
+ (await restored.inspectConversation({ conversationKey: 'session-a', cwd: process.cwd() }))
+ .readiness,
+ 'restorable',
+ );
+ await restored.configureConversation(
+ { conversationKey: 'session-a', cwd: process.cwd(), configuration: { model: 'default' } },
+ new AbortController().signal,
+ );
+ assert.equal(protocol.sessions, 1);
+ assert.equal(protocol.resumes, 1);
+ assert.equal(protocol.loads, 0);
+ assert.equal(
+ (await restored.inspectConversation({ conversationKey: 'session-a', cwd: process.cwd() }))
+ .readiness,
+ 'ready',
+ );
+ assert.equal(
+ (await restored.execute(request('second'), executorContext([]))).status,
+ 'completed',
+ );
+ await restored.acknowledgeExecution('session-a', 'turn-second');
+ assert.equal(storage.record().committedPrompts, 2);
+ assert.equal(protocol.sessions, 1);
+ } finally {
+ await restored.dispose();
+ }
+ } finally {
+ await first.dispose();
+ await rm(fixture.root, { recursive: true, force: true });
+ }
+});
+
+test('unacknowledged prompt loads replay as restoration and exposes history gap without a second prompt', async () => {
+ const fixture = await executableFixture();
+ const protocol = fakeProtocol();
+ protocol.supportsRestore = true;
+ const storage = durableState();
+ const make = () =>
+ new AcpExecutor(
+ adapter,
+ { executable: fixture.executable },
+ {
+ state: storage.state,
+ createConnection: protocol.factory,
+ },
+ );
+ const first = make();
+ try {
+ assert.equal((await first.execute(request('first'), executorContext([]))).status, 'completed');
+ await first.dispose();
+ const restored = make();
+ const events: unknown[] = [];
+ try {
+ assert.equal(
+ (await restored.inspectConversation({ conversationKey: 'session-a', cwd: process.cwd() }))
+ .readiness,
+ 'history_gap',
+ );
+ const result = await restored.execute(request('new-input'), executorContext(events));
+ assert.equal(result.status, 'failed');
+ if (result.status === 'failed') assert.equal(result.code, 'acp_history_gap');
+ assert.deepEqual(events, []);
+ assert.equal(protocol.loads, 1);
+ assert.equal(protocol.resumes, 0);
+ assert.equal(protocol.sessions, 1);
+ assert.equal(protocol.prompts, 1);
+ assert.equal(storage.record().phase, 'history_gap');
+ assert.equal(storage.record().gapEvidence?.replayedUserChunks, 1);
+ } finally {
+ await restored.dispose();
+ }
+ const reopened = make();
+ try {
+ const repeated = await reopened.execute(request('still-blocked'), executorContext(events));
+ assert.equal(repeated.status, 'failed');
+ if (repeated.status === 'failed') assert.equal(repeated.code, 'acp_history_gap');
+ assert.equal(protocol.loads, 1);
+ assert.equal(protocol.prompts, 1);
+ } finally {
+ await reopened.dispose();
+ }
+ } finally {
+ await first.dispose();
+ await rm(fixture.root, { recursive: true, force: true });
+ }
+});
+
+test('late restored-session updates stay quarantined while the history gap is persisted', async () => {
+ const fixture = await executableFixture();
+ const protocol = fakeProtocol();
+ protocol.supportsRestore = true;
+ const storage = durableState();
+ let releaseGapWrite!: () => void;
+ const gapWriteBlocked = new Promise<void>((resolve) => {
+ releaseGapWrite = resolve;
+ });
+ let gapWriteStarted!: () => void;
+ const gapWriteReached = new Promise<void>((resolve) => {
+ gapWriteStarted = resolve;
+ });
+ const state: AcpConversationStateStore = {
+ ...storage.state,
+ write: async (key, record) => {
+ if (record.phase === 'history_gap') {
+ gapWriteStarted();
+ await gapWriteBlocked;
+ }
+ await storage.state.write!(key, record);
+ },
+ };
+ const make = () =>
+ new AcpExecutor(
+ adapter,
+ { executable: fixture.executable },
+ { state, createConnection: protocol.factory },
+ );
+ const first = make();
+ const blockedWrite = 'node_modules/.maka-acp-restore-write';
+ try {
+ assert.equal((await first.execute(request('first'), executorContext([]))).status, 'completed');
+ await first.dispose();
+ const restored = make();
+ const events: unknown[] = [];
+ try {
+ const execution = restored.execute(request('unsent-new-turn'), executorContext(events));
+ await gapWriteReached;
+ protocol.notifyUpdate({
+ sessionUpdate: 'agent_message_chunk',
+ content: { type: 'text', text: 'LATE OLD EXTERNAL OUTPUT' },
+ });
+ protocol.notifyUpdate({
+ sessionUpdate: 'tool_call',
+ toolCallId: 'late-old-tool',
+ title: 'Historical tool',
+ });
+ assert.deepEqual(await protocol.requestPermission(), { outcome: { outcome: 'cancelled' } });
+ const workspaceAccess = await Promise.allSettled([
+ protocol.readTextFile('package.json'),
Review Comment:
**P3.** `'package.json'` is a relative path. The fs layer rejects it as "must be absolute" whether or not the restore gate exists, so removing the gate at `index.ts:875-883` still passes this test. Use an absolute workspace path, and assert the specific gate rejection.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] feat(acp): restore Antigravity sessions after restart [maka]
Posted by "Sun-GLiang (via GitHub)" <gi...@apache.org>.
Sun-GLiang commented on PR #5670:
URL: https://github.com/apache/maka/pull/5670#issuecomment-5869544413
Review follow-up on current head `9ac0387`:
Fixed in `3ecca4d`:
- Filesystem reads and writes are now rejected while an ACP Session is restoring, has a history gap, or is lost. The regression exercises both operations during the blocked history-gap checkpoint window.
- A settled terminal result that is not durably consumed now takes an explicit abandonment path to `history_gap` instead of leaving the conversation wedged in `acp_busy`.
- Concurrent restore retries re-check and reuse the winning replacement Session instead of orphaning a process.
- A fresh pre-creation configuration failure remains retryable instead of becoming `history_only`.
- Desktop rejects Restore while selection is already in flight, before applying the optimistic `restoring` snapshot.
The protocol conflict was resolved by merging current main, moving the live epoch from 197 to 198, and restoring the two historical compatible-change declarations to epoch 189 (`d0cba03`, `9ac0387`).
I am not changing the two remaining binding/restore points:
- `restore_failed` is intentional when restore capability is unavailable or the binding changed. Retry can become valid again after restoring a compatible installation/capability; `history_gap` would incorrectly claim uncertain replay, while `history_only` would discard a potentially recoverable external Session ID. Launching is required because ACP restore capability is negotiated at initialization.
- Hashing the executable/helper on every non-probe initialization is also intentional fail-closed identity binding. A fresh `session/new` must record a known-good binding for future resume, and changed bytes must prevent the same opaque Session ID from being resumed against a different implementation.
Validation on the pushed head: full build passed; Runtime, ACP, Runtime Host, and Desktop renderer typechecks passed; Biome passed; 206 targeted tests passed; and the merge-result protocol guard passed for 197 → 198. CI is currently running.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] feat(acp): restore Antigravity sessions after restart [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han commented on PR #5670:
URL: https://github.com/apache/maka/pull/5670#issuecomment-5857892065
**Correction to my earlier review (pullrequestreview-5331195535).** My instruction about the two `protocol-compatible-changes/*.json` files was wrong: do **not** move their `epoch` to the new epoch.
`packages/runtime-host/protocol-compatible-changes/executor-catalog-browser-safe.json` and `issue-4032-composition-identity.json` record a compatibility argument against the epoch that was current **when the argument was made** — their own text says "Against the cumulative protocol through epoch 189, …". That number is the baseline of that argument, not the live epoch, so an unrelated PR must not refresh it. Revert both JSON changes, and take only `packages/runtime-host/src/protocol/index.ts` to the next free epoch after `main` (currently `196`).
Where this conflicts with my earlier review, the revert wins.
---
> **Automated review notice:** This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] feat(acp): restore Antigravity sessions after restart [maka]
Posted by "Sun-GLiang (via GitHub)" <gi...@apache.org>.
Sun-GLiang commented on code in PR #5670:
URL: https://github.com/apache/maka/pull/5670#discussion_r4121952668
##########
packages/runtime-host/src/protocol/index.ts:
##########
@@ -103,7 +103,8 @@ export const RUNTIME_HOST_REGISTRATION_SCHEMA_VERSION = 1 as const;
export const RUNTIME_HOST_PROTOCOL_VERSION = 0 as const;
// Increment when the same protocol version no longer guarantees safe Client-Host
// interoperability. Mismatches are rejected before domain commands are admitted.
-export const RUNTIME_HOST_COMPATIBILITY_EPOCH = 192 as const;
+export const RUNTIME_HOST_COMPATIBILITY_EPOCH = 193 as const;
Review Comment:
Agreed and fixed by d0cba03 and 9ac0387. The branch is merged with current main (epoch 197), the live protocol epoch is now 198, and both historical compatible-change declarations are restored to main's original epoch 189. The merge-result protocol guard passes for 197 → 198.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] feat(acp): restore Antigravity sessions after restart [maka]
Posted by "Sun-GLiang (via GitHub)" <gi...@apache.org>.
Sun-GLiang merged PR #5670:
URL: https://github.com/apache/maka/pull/5670
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] feat(acp): restore Antigravity sessions after restart [maka]
Posted by "Sun-GLiang (via GitHub)" <gi...@apache.org>.
Sun-GLiang commented on code in PR #5670:
URL: https://github.com/apache/maka/pull/5670#discussion_r4121899862
##########
packages/acp-executor-plugin/src/index.ts:
##########
@@ -311,12 +373,17 @@ export class AcpExecutor implements PluginExecutorProvider {
{ model: request.configuration.model },
context.signal,
);
+ await this.#beginPrompt(session, request.turnId);
const prompt = session.connection!.agent.request(methods.agent.session.prompt, {
sessionId: session.acpSessionId!,
prompt: [{ type: 'text', text: promptText(request) }],
});
const response = await this.#awaitPrompt(session, prompt, context.signal);
if (!response) return { status: 'cancelled', reason: 'timeout' };
+ // A settled prompt can be checkpointed even when it was cancelled or failed.
+ // Timeouts and transport failures never reach this point, and the Runtime
+ // must still durably consume the terminal event before acknowledging it.
+ if (session.record) session.awaitingAck = request.turnId;
Review Comment:
Confirmed and fixed in 3ecca4d. The Runtime now calls a Session- and generation-scoped `abandonExecution` when a settled result's terminal event is not consumed or acknowledgement fails. ACP then transitions the matching `prompt_pending` record to `history_gap`, clears `awaitingAck`, persists the gap when possible, and closes the process. Regressions cover detached consumers, failed acknowledgements, and the durable ACP gap state.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] feat(acp): restore Antigravity sessions after restart [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han commented on code in PR #5670:
URL: https://github.com/apache/maka/pull/5670#discussion_r4116130623
##########
packages/acp-executor-plugin/src/index.ts:
##########
@@ -311,12 +373,17 @@ export class AcpExecutor implements PluginExecutorProvider {
{ model: request.configuration.model },
context.signal,
);
+ await this.#beginPrompt(session, request.turnId);
const prompt = session.connection!.agent.request(methods.agent.session.prompt, {
sessionId: session.acpSessionId!,
prompt: [{ type: 'text', text: promptText(request) }],
});
const response = await this.#awaitPrompt(session, prompt, context.signal);
if (!response) return { status: 'cancelled', reason: 'timeout' };
+ // A settled prompt can be checkpointed even when it was cancelled or failed.
+ // Timeouts and transport failures never reach this point, and the Runtime
+ // must still durably consume the terminal event before acknowledging it.
+ if (session.record) session.awaitingAck = request.turnId;
Review Comment:
**P2** — `awaitingAck` is only cleared by a successful `acknowledgeExecution` (L697). If the backend is detached before acking (e.g. the kernel throws while persisting the terminal, or the service wrapper rejects as retired/aborted), this conversation returns `acp_busy` on every later send for the rest of the Host lifetime, while `inspectConversation` still reports `ready`. Consider an abandon path that marks `historyGap` and loses the session, or reporting `history_gap` from inspect when `awaitingAck` is set with no active prompt.
##########
packages/acp-executor-plugin/src/index.ts:
##########
@@ -373,18 +443,27 @@ export class AcpExecutor implements PluginExecutorProvider {
if (failures.length) throw new AggregateError(failures, 'ACP process cleanup failed');
}
- #session(request: Readonly<PluginExecutorRequest>): RetainedSession {
+ async #session(request: {
+ readonly conversationKey: string;
+ readonly cwd: string;
+ readonly configuration?: ExecutorConfiguration;
+ }): Promise<RetainedSession> {
const cwd = resolve(request.cwd);
const existing = this.#sessions.get(request.conversationKey);
if (existing) {
if (existing.cwd !== cwd)
throw new AcpRuntimeError('ACP conversation cannot change workspace', 'acp_cwd_changed');
- if (existing.lost)
+ if (!existing.lost) return existing;
+ if (existing.active || existing.configuring)
+ throw new AcpRuntimeError('ACP Session is busy', 'acp_busy');
+ if (!existing.record)
throw new AcpRuntimeError(
'ACP conversation is history-only because its external process was lost',
'acp_history_only',
);
- return existing;
+ await existing.loss;
+ if (this.#sessions.get(request.conversationKey) === existing)
+ this.#sessions.delete(request.conversationKey);
Review Comment:
**P2** — After `await existing.loss`, two concurrent callers can both fall through and each install their own `created` session. The second `set` overwrites the first, whose spawned process is then unreachable by `dispose`, and both resume the same ACP session. Re-check `this.#sessions.get(request.conversationKey)` after the await and reuse the replacement if one exists, or serialise the lost→new transition per conversation.
##########
packages/runtime-host/src/protocol/index.ts:
##########
@@ -103,7 +103,8 @@ export const RUNTIME_HOST_REGISTRATION_SCHEMA_VERSION = 1 as const;
export const RUNTIME_HOST_PROTOCOL_VERSION = 0 as const;
// Increment when the same protocol version no longer guarantees safe Client-Host
// interoperability. Mismatches are rejected before domain commands are admitted.
-export const RUNTIME_HOST_COMPATIBILITY_EPOCH = 192 as const;
+export const RUNTIME_HOST_COMPATIBILITY_EPOCH = 193 as const;
Review Comment:
**P2** — main is already at 195 (193/194 are taken), so this needs to become 196 on rebase. The edits to `executor-catalog-browser-safe.json` and `issue-4032-composition-identity.json` (189→193) don't affect the epoch guard, which only reads added declarations, and they rewrite when those changes actually landed. Suggest reverting them.
##########
packages/acp-executor-plugin/src/index.ts:
##########
@@ -215,27 +240,59 @@ export class AcpExecutor implements PluginExecutorProvider {
}): Promise<ExecutorCatalogEntry> {
const session = this.#sessions.get(input.conversationKey);
if (this.#disposed) return this.#catalogEntry('unavailable');
- if (session?.lost || (!session && (await this.#state?.has(input.conversationKey, input.cwd))))
- return this.#catalogEntry('history_only');
- return this.#catalogEntry('ready', session?.configOptions ?? [], input.configuration?.model);
+ if (session && session.cwd !== resolve(input.cwd))
+ return this.#catalogEntry('history_only', [], input.configuration?.model);
+ if (session?.restoring)
+ return this.#catalogEntry('restoring', session.configOptions, input.configuration?.model);
+ if (session?.restoreFailed)
+ return this.#catalogEntry(
+ 'restore_failed',
+ session.configOptions,
+ input.configuration?.model,
+ );
+ if (session?.historyGap)
+ return this.#catalogEntry('history_gap', session.configOptions, input.configuration?.model);
+ if (session && !session.lost)
+ return this.#catalogEntry('ready', session.configOptions, input.configuration?.model);
+ if (session?.lost && !session.record)
+ return this.#catalogEntry('history_only', session.configOptions, input.configuration?.model);
+ const stored = this.#state?.read
+ ? decodeContinuity(await this.#state.read(input.conversationKey))
+ : (await this.#state?.has(input.conversationKey, input.cwd))
+ ? 'legacy'
+ : undefined;
+ if (!stored) return this.#catalogEntry('ready', [], input.configuration?.model);
+ if (stored === 'invalid' || stored === 'legacy' || stored.cwd !== resolve(input.cwd))
+ return this.#catalogEntry('history_only', [], input.configuration?.model);
+ if (stored.phase === 'reserved')
+ return this.#catalogEntry('history_only', [], input.configuration?.model);
+ return this.#catalogEntry(
+ stored.phase === 'prompt_pending' || stored.phase === 'history_gap'
+ ? 'history_gap'
+ : 'restorable',
+ [],
+ input.configuration?.model ?? stored.confirmedModel,
+ );
}
async configureConversation(
input: { conversationKey: string; cwd: string; configuration?: ExecutorConfiguration },
signal: AbortSignal,
): Promise<void> {
- const session = this.#sessions.get(input.conversationKey);
- if (
- !session?.connection ||
- session.lost ||
- session.active ||
- session.configuring ||
- !input.configuration?.model
- )
- throw new Error('ACP model change is unavailable');
+ if (!input.configuration?.model) throw new Error('ACP model change is unavailable');
+ const session = await this.#session(input);
+ if (session.active || session.configuring) throw new Error('ACP model change is unavailable');
session.configuring = true;
+ const wasConnected = !!session.connection;
try {
+ await this.#ensureInitialized(session, signal);
await this.#applyInitialConfig(session, { model: input.configuration.model }, signal, true);
+ } catch (error) {
+ if (!wasConnected || session.lost) {
Review Comment:
**P2** — On a fresh conversation (no stored record, not yet reserved), a failure here only calls `#lose`. The session stays in `#sessions` as `lost && !record`, so `#session` and `inspect` then report `acp_history_only` ("external process was lost") for the rest of the Host lifetime, even though no external session ever existed. `execute` deletes such sessions on retryable failures; configure should do the same.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] feat(acp): restore Antigravity sessions after restart [maka]
Posted by "Sun-GLiang (via GitHub)" <gi...@apache.org>.
Sun-GLiang commented on code in PR #5670:
URL: https://github.com/apache/maka/pull/5670#discussion_r4121935567
##########
packages/acp-executor-plugin/src/index.ts:
##########
@@ -373,18 +443,27 @@ export class AcpExecutor implements PluginExecutorProvider {
if (failures.length) throw new AggregateError(failures, 'ACP process cleanup failed');
}
- #session(request: Readonly<PluginExecutorRequest>): RetainedSession {
+ async #session(request: {
+ readonly conversationKey: string;
+ readonly cwd: string;
+ readonly configuration?: ExecutorConfiguration;
+ }): Promise<RetainedSession> {
const cwd = resolve(request.cwd);
const existing = this.#sessions.get(request.conversationKey);
if (existing) {
if (existing.cwd !== cwd)
throw new AcpRuntimeError('ACP conversation cannot change workspace', 'acp_cwd_changed');
- if (existing.lost)
+ if (!existing.lost) return existing;
+ if (existing.active || existing.configuring)
+ throw new AcpRuntimeError('ACP Session is busy', 'acp_busy');
+ if (!existing.record)
throw new AcpRuntimeError(
'ACP conversation is history-only because its external process was lost',
'acp_history_only',
);
- return existing;
+ await existing.loss;
+ if (this.#sessions.get(request.conversationKey) === existing)
+ this.#sessions.delete(request.conversationKey);
Review Comment:
Confirmed and fixed in 3ecca4d. After awaiting the lost Session's teardown, `#session` now re-checks the conversation map. If another caller has already installed the replacement, it reuses that winner instead of overwriting it. The regression runs concurrent restore retries and verifies that only one resume/replacement process is created and remains tracked.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] feat(acp): restore Antigravity sessions after restart [maka]
Posted by "Sun-GLiang (via GitHub)" <gi...@apache.org>.
Sun-GLiang commented on code in PR #5670:
URL: https://github.com/apache/maka/pull/5670#discussion_r4121947069
##########
packages/acp-executor-plugin/src/index.ts:
##########
@@ -215,27 +240,59 @@ export class AcpExecutor implements PluginExecutorProvider {
}): Promise<ExecutorCatalogEntry> {
const session = this.#sessions.get(input.conversationKey);
if (this.#disposed) return this.#catalogEntry('unavailable');
- if (session?.lost || (!session && (await this.#state?.has(input.conversationKey, input.cwd))))
- return this.#catalogEntry('history_only');
- return this.#catalogEntry('ready', session?.configOptions ?? [], input.configuration?.model);
+ if (session && session.cwd !== resolve(input.cwd))
+ return this.#catalogEntry('history_only', [], input.configuration?.model);
+ if (session?.restoring)
+ return this.#catalogEntry('restoring', session.configOptions, input.configuration?.model);
+ if (session?.restoreFailed)
+ return this.#catalogEntry(
+ 'restore_failed',
+ session.configOptions,
+ input.configuration?.model,
+ );
+ if (session?.historyGap)
+ return this.#catalogEntry('history_gap', session.configOptions, input.configuration?.model);
+ if (session && !session.lost)
+ return this.#catalogEntry('ready', session.configOptions, input.configuration?.model);
+ if (session?.lost && !session.record)
+ return this.#catalogEntry('history_only', session.configOptions, input.configuration?.model);
+ const stored = this.#state?.read
+ ? decodeContinuity(await this.#state.read(input.conversationKey))
+ : (await this.#state?.has(input.conversationKey, input.cwd))
+ ? 'legacy'
+ : undefined;
+ if (!stored) return this.#catalogEntry('ready', [], input.configuration?.model);
+ if (stored === 'invalid' || stored === 'legacy' || stored.cwd !== resolve(input.cwd))
+ return this.#catalogEntry('history_only', [], input.configuration?.model);
+ if (stored.phase === 'reserved')
+ return this.#catalogEntry('history_only', [], input.configuration?.model);
+ return this.#catalogEntry(
+ stored.phase === 'prompt_pending' || stored.phase === 'history_gap'
+ ? 'history_gap'
+ : 'restorable',
+ [],
+ input.configuration?.model ?? stored.confirmedModel,
+ );
}
async configureConversation(
input: { conversationKey: string; cwd: string; configuration?: ExecutorConfiguration },
signal: AbortSignal,
): Promise<void> {
- const session = this.#sessions.get(input.conversationKey);
- if (
- !session?.connection ||
- session.lost ||
- session.active ||
- session.configuring ||
- !input.configuration?.model
- )
- throw new Error('ACP model change is unavailable');
+ if (!input.configuration?.model) throw new Error('ACP model change is unavailable');
+ const session = await this.#session(input);
+ if (session.active || session.configuring) throw new Error('ACP model change is unavailable');
session.configuring = true;
+ const wasConnected = !!session.connection;
try {
+ await this.#ensureInitialized(session, signal);
await this.#applyInitialConfig(session, { model: input.configuration.model }, signal, true);
+ } catch (error) {
+ if (!wasConnected || session.lost) {
Review Comment:
Confirmed and fixed in 3ecca4d. A fresh configuration failure now removes the retained Session only when no ACP Session ID was obtained, continuity was never reserved, and no continuity record exists. That keeps the retryable pre-creation case at `ready` while preserving fail-closed behavior after any uncertain external creation. The regression verifies that a subsequent configuration retry succeeds.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] feat(acp): restore Antigravity sessions after restart [maka]
Posted by "jackwener (via GitHub)" <gi...@apache.org>.
jackwener commented on code in PR #5670:
URL: https://github.com/apache/maka/pull/5670#discussion_r4110472442
##########
packages/acp-executor-plugin/src/index.ts:
##########
@@ -449,11 +561,70 @@ export class AcpExecutor implements PluginExecutorProvider {
owner.failed,
]);
if (initialized.protocolVersion !== 1) throw new Error('Unsupported ACP protocol version');
+ if (stored) {
+ session.replay = { updates: 0, userChunks: 0 };
+ const pending = stored.phase === 'prompt_pending';
+ const supported = pending
+ ? initialized.agentCapabilities?.loadSession
+ : initialized.agentCapabilities?.sessionCapabilities?.resume;
+ if (!supported)
+ throw new AcpRuntimeError(
+ 'ACP Agent cannot restore this Session',
+ 'acp_restore_unsupported',
+ );
+ const restored = await Promise.race([
+ owner.connection.agent.request(
+ pending ? methods.agent.session.load : methods.agent.session.resume,
+ { sessionId: stored.sessionId!, cwd: session.cwd, mcpServers: [] },
+ { cancellationSignal: startupSignal },
+ ),
+ owner.failed,
+ ]);
+ session.configOptions = restored.configOptions ?? [];
+ const restoredModel = currentAcpModel(session.configOptions);
+ if (restoredModel) await this.#persistConfirmedModel(session, restoredModel);
+ session.restoring = false;
Review Comment:
[P2] Keep history-gap sessions quarantined until the connection is closed
For a saved `prompt_pending` record, `session/load` can return while the restored external session still has late output to deliver. This line clears `restoring` before the awaited history-gap write at line 602. `execute()` has already installed the *new* request's `session.active` (lines 365–367). During that storage wait, `#acceptUpdate` no longer takes its restoration return (lines 818–824), and for an `agent_message_chunk` with text and an active context it calls **`emitText(active.context, 'output_delta', update.content.text)` at line 835**; it does not check `historyGap`. The backend maps that output to `text_delta` using the new Turn ID (`plugin-executor-backend.ts:392–400`).
I reproduced this with a real stdio child Agent plus production Plugin/Service/Backend: after an unacknowledged first prompt, hold the `history_gap` store write and deliver one old-session text notification after the load response. The backend yields `text_delta(turnId="unsent-new-turn", text="LATE OLD EXTERNAL OUTPUT")`, then `error` and `complete`; the Agent history confirms only the original prompt was sent. The history-gap error therefore does not prevent old text from entering the new Turn's canonical event stream. The callback-level probe also leaks a tool start.
Keep updates quarantined throughout the gap/failure teardown, e.g. gate on `historyGap` as well as `restoring`, and add a late-notification regression. Adding just that guard in an isolated compiled copy makes the same stdio probe emit only `error, complete`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] feat(acp): restore Antigravity sessions after restart [maka]
Posted by "Sun-GLiang (via GitHub)" <gi...@apache.org>.
Sun-GLiang commented on code in PR #5670:
URL: https://github.com/apache/maka/pull/5670#discussion_r4111516030
##########
packages/acp-executor-plugin/src/index.ts:
##########
@@ -449,11 +561,70 @@ export class AcpExecutor implements PluginExecutorProvider {
owner.failed,
]);
if (initialized.protocolVersion !== 1) throw new Error('Unsupported ACP protocol version');
+ if (stored) {
+ session.replay = { updates: 0, userChunks: 0 };
+ const pending = stored.phase === 'prompt_pending';
+ const supported = pending
+ ? initialized.agentCapabilities?.loadSession
+ : initialized.agentCapabilities?.sessionCapabilities?.resume;
+ if (!supported)
+ throw new AcpRuntimeError(
+ 'ACP Agent cannot restore this Session',
+ 'acp_restore_unsupported',
+ );
+ const restored = await Promise.race([
+ owner.connection.agent.request(
+ pending ? methods.agent.session.load : methods.agent.session.resume,
+ { sessionId: stored.sessionId!, cwd: session.cwd, mcpServers: [] },
+ { cancellationSignal: startupSignal },
+ ),
+ owner.failed,
+ ]);
+ session.configOptions = restored.configOptions ?? [];
+ const restoredModel = currentAcpModel(session.configOptions);
+ if (restoredModel) await this.#persistConfirmedModel(session, restoredModel);
+ session.restoring = false;
Review Comment:
**Automated follow-up (OpenAI Codex):** Confirmed and fixed in e0eccd0c7. Late Session updates are now dropped while the restored Session has a history gap or is being torn down; late permission requests are cancelled as well. The new regression blocks the `history_gap` checkpoint write, injects old text, tool activity, and a permission request, and verifies that none reaches the new Turn. The ACP Plugin suite passes (58/58).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] feat(acp): restore Antigravity sessions after restart [maka]
Posted by "Sun-GLiang (via GitHub)" <gi...@apache.org>.
Sun-GLiang commented on PR #5670:
URL: https://github.com/apache/maka/pull/5670#issuecomment-5846692822
**Automated follow-up (OpenAI Codex):** A correction to the Windows checkpoint note in @zhiiw's review: the absence of `fsync` does not guarantee that a lost checkpoint becomes `history_only` or `history_gap`. For example, `#beginPrompt` writes `prompt_pending` before sending the ACP prompt. A power loss could discard that unflushed write and leave the previous `committed` record, even if the Agent received the prompt. Restart would then take the `session/resume` path and could treat the history as aligned. The current process-kill and Host-restart tests do not establish power-loss durability, so I have made that boundary explicit in the PR description. The epoch note is also corrected to 189 → 190.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org