Skip to content

Fix refreshed host instructions in reused ACP sessions - #493

Merged
sambitcreate merged 10 commits into
mainfrom
feature/acp-host-instructions
Oct 17, 2026
Merged

sambitcreate merged 10 commits into
mainfrom
feature/acp-host-instructions

Conversation

@sambitcreate

@sambitcreate sambitcreate commented Oct 15, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • Fingerprint host guidance for ACP session reuse, ignoring only the generation-local UUID in a complete AGENTS.md wrapper. The wrapper header and JSON instruction records remain part of the comparison.
  • Before each turn, rebuild the native session with current instructions and bounded conversation history when guidance changes. Keep warm and resumed sessions when it does not.
  • If instructions refresh while a bridged tool is pending, cancel the old ACP prompt and rebuild before sending the tool result.

Fixes #398.

Validation

  • npm run build:worktree-file-io and npm run build:subagent-file-mutator
  • npm run test:acp (146 passed)
  • npx tsx --test main/services/agents-instructions.test.ts (19 passed)
  • npm run test:subagent-file-mutator:native (31 passed, 3 skipped)
  • npm run type-check
  • npm run lint
  • git diff --check

The fake agent runs through the real ACP process, connection, and runtime. Tests inspect the prompts for unchanged instructions, A to B to A, saved-session resume, and a mid-turn tool continuation. The review follow-up uses separately created real AGENTS instruction refreshers with identical file content to verify warm and persisted reuse; edited content still rebuilds. A boundary test changes AGENTS text at raw index 64,010 while the delivered 64,000-character prefix stays identical, then verifies warm and persisted session reuse.

The earlier isolated worktree run of agents-instructions.test.ts failed its native reader case with io_failed. After merging main and building the newly required subagent file mutator helper, that suite passes 19 of 19 and the native helper suite passes 31 cases with 3 skipped. A signed-in Antigravity account was not used for live-provider acceptance.

Hosted CI finding

  • Exact-head runtime-subagents job 116707994916 on 544e0af6e995ee563120f8cdf00181e7dcd2dc78 failed main/services/bot-runtime/live-projection.test.ts case while classifying fails, a cached routine stays hidden and the person's classified message stays listed; the new one waits, then appears. Its retries are throttled, saw 3 calls assertion failed; 3,411 of 3,412 tests passed. This PR changes no Bot runtime or test files. PR #502 recorded the same spec and symptom on its first head. The focused spec passes locally. This is evidence of a pre-existing timing-sensitive failure, though the underlying timing cause remains unproven. A passing rerun would not erase the flaky-test finding. No retry or timeout policy was changed.

A single failed-job rerun of workflow 38818022683, attempt 2 was requested after attempt 1 completed, on the same head 544e0af6e995ee563120f8cdf00181e7dcd2dc78. No further rerun is permitted by the one-rerun rule. Attempt 2 completed successfully, including the runtime-subagents job. The first-attempt Bot retry-throttling failure remains recorded as a flaky-test finding; the passing rerun does not prove its timing cause is fixed.

The later exact-head workflow 38852569389 on 6d262043967b4d339d3d64ce7556e27f342248bb ended cancelled: Electron E2E shard 2 exceeded the configured 20-minute job execution limit while its deterministic E2E test step was running. GitHub's check-run annotation states The job has exceeded the maximum execution time of 20m0s; its job-log API returned 404. Shards 1 and 3 passed. The exact stuck test is unknown, so this remains an unresolved CI gate on that head. No timeout or retry policy was changed.

The latest-head runtime-subagents job 116931184168 on b34432b86a66bd68d6995a02fa8d58b1c2367ec6 failed the same unchanged main/services/bot-runtime/live-projection.test.ts case and retries are throttled, saw 3 calls assertion. This job had one failed test among 3,412. All three E2E shards passed. Main's runtime-subagents job on base 515de67389c5d08ccc78ad215997a0bfc8a1206e passed, and the focused spec passed locally on this PR head; the timing cause remains unproven. After the workflow became terminal, I requested one same-head failed-job rerun, attempt 2. The rerun passed, including runtime-subagents and CI required, on unchanged head b34432b86a66bd68d6995a02fa8d58b1c2367ec6. The first-attempt failure remains an open flaky-test finding; no further rerun is permitted.

Latest-main synchronization

Merged origin/main at 3473089fb2ae2cbe0db4e4ed7fef163057972137 into the PR branch without conflicts, rebase, or force-push. The current head is fd0993d245c70256d49e6f82096c53251a4f5be7. Relative to main, the feature diff remains six ACP/project-memory files. The root package test chain and Remote revision 33 protocol and fixtures match main; this PR changes no Remote or mobile contract.

After installing the Pi 1.3.0 dependency lock, local checks passed: npm run test:acp (146), focused AGENTS/Pi catalog tests (120), npm run test:subagent-file-mutator:native (31 pass, 3 skip), npm run test:ci-policy (63), npm run type-check, npm run lint, and git diff --check. Earlier focused Bot/Remote and simulator E2E checks passed on the preceding merge.

Two broader local runs on an earlier main merge did not fully pass. npm run test:cli failed its unchanged extension-binding release test in bind-reject mode (shutdowns 0, expected 1) after a long run; the same test passed alone. A combined Remote test run and an isolated rerun both timed out in the unchanged identity reads and healthy LAN wake do not await optional Tailscale inspection case at its one-second within() fence. Their causes are unresolved; no timeout, retry, or unrelated production code was changed.

Exact-head CI run 38985972307 and bot reviews are pending.

Hermes review gate

Earlier full and follow-up reviews failed in the AGY engine before a verdict. Read-only bot-host logs identified RESOURCE_EXHAUSTED (code 429): Individual quota reached; the reported reset was approximately 15:34 UTC on 2026-10-16. Hermes later posted a successful full-review status on an earlier head, but that does not review this commit. I requested a fresh full review for fd0993d245c70256d49e6f82096c53251a4f5be7. A successful verdict on this exact head and resolution of any actionable threads remain required.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

Unchanged AGENTS.md instructions currently invalidate reused sessions because their generation-local framing changes.

Reviewed changes Reviewed the host-instruction fingerprinting, session replacement and persistence paths, and real-process regression tests.

  • Instruction identity: Hashes the trimmed, bounded host instructions used to construct fresh ACP prompts.
  • Session reconciliation: Rebuilds warm or resumed sessions on instruction changes and persists the fingerprint with the native session record.
  • Tool continuation: Cancels an in-flight bridged-tool prompt when instructions change and reconstructs under the refreshed instructions, with fake-agent coverage and updated project memory.

Validation: npm run test:acp passed all 143 tests. An additional probe using the real AGENTS instruction refresher and fake ACP process reproduced the inline finding. Live, signed-in Antigravity acceptance was not exercised.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using gpt-6.1-sol | 𝕏

Comment thread main/services/acp/runtime.ts

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

Nonce normalization can still discard a reused session when only undelivered instruction text changes near the size limit.

Reviewed changes Reviewed the follow-up since 7ab6f8e, including the fix for generation-local AGENTS markers and its integration coverage.

  • Normalized AGENTS identity: Ignored generated tag UUIDs while retaining the wrapper header, JSON records, and other host guidance in the fingerprint.
  • Extended real-refresher coverage: Verified unchanged warm and persisted session reuse across separately created refreshers, plus reconstruction after A to B to A edits.
  • Updated project memory: Documented the stable-guidance comparison and refreshed test coverage.

Validation: npm run test:acp passed all 145 tests. An additional real-refresher/fake-process probe reproduced the boundary issue inline. Live, signed-in Antigravity acceptance was not exercised.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using gpt-6.1-sol | 𝕏

Comment thread main/services/acp/prompt.ts
@very-hermes-bot

very-hermes-bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Collaborator

Hermes Review Bot

Confidence: 5

Reviewed the whole PR again, with the previous Hermes checkpoint and prior reviews as context.

Engine: agy/gemini-3.8-flash-high
Review mode: full
Scope: full
Head: fd0993d245c70256d49e6f82096c53251a4f5be7
Generated: 2026-10-17T00:27:40+00:00
Reviews: 2

Summary

Aiden's ACP runtime now fingerprints bounded host instructions (MAX_INSTRUCTION_CHARS = 64_000) and validates that fingerprint on every ordinary turn and mid-turn bridged-tool continuation. By replacing generation-local UUID nonces in the <agents-instructions-*> framing with equal-length stable placeholders (STABLE_AGENTS_NONCE), character offsets before the 64,000-character delivery cutoff are preserved, preventing spurious session invalidations on unedited AGENTS.md files. When host guidance changes—or when resuming legacy saved sessions lacking a fingerprint—the runtime removes the stale session record, disposes of the native binding, and reconstructs context under the updated guidance. Maintainers should double-check that if the outer framing emitted by createAgentsInstructionRefresher in main/services/agents-instructions.ts:155 is ever altered, the AGENTS_FRAME pattern in main/services/acp/prompt.ts:34 is updated in lockstep.

Confidence Score: 5/5

Full trace of prompt construction, session lifecycle reconciliation, disk persistence, and tool cancellation confirmed end-to-end correctness with zero regressions against latest main.

📁 Important Files Changed
  • main/services/acp/prompt.ts: Introduces deliveredInstructions and hostInstructionsFingerprint, normalizing generation-local AGENTS.md nonces with a fixed-width placeholder before computing the SHA-256 hash over the delivered 64,000-character prefix.
  • main/services/acp/runtime.ts: Tracks hostInstructionsFingerprint on bindings, checks for instruction changes on turn execution and tool continuation, drops outdated sessions, and rebuilds context with updated instructions.
  • main/services/acp/session-store.ts: Persists optional hostInstructionsFingerprint in session records and maintains backward compatibility for legacy records.
  • main/services/acp/runtime.test.ts: Adds integration tests covering instruction updates (including A-B-A transitions), distinct AGENTS generation nonces, edits beyond the 64k cutoff, disk resume, and bridged-tool continuations.
  • main/services/acp/fixtures/fake-acp-agent.mjs & .memory/trusted-agents-refresh.md: Test fixture logging support for prompt context and updated project memory tracking validation on the latest base.

Findings

No findings.

📊 Sequence Diagram

Unchanged from the previous review.

Machine-Readable Findings

[]

🕑 Review History
Round Scope Commit Confidence Status
1 initial 544e0af6e995 5/5 completed
1 initial b34432b86a66 5/5 completed
1 initial fd0993d245c7 5/5 completed
2 full fd0993d245c7 5/5 latest

Last reviewed commit: fd0993d245c7
Reviews (2) · Comment /hermes review to trigger a new review · /hermes review full for full re-review

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes Reviewed the follow-up since 4320864, focusing on instruction-cutoff preservation and session reuse.

  • Preserved instruction offsets: Replaced generated AGENTS tag UUIDs with equal-length stable placeholders so fingerprinting respects the original 64,000-character delivery cutoff.
  • Added boundary coverage: Verified that AGENTS edits beyond the delivered prefix preserve both warm session reuse and saved-session resume through the real refresher and fake ACP process.
  • Updated project memory: Documented fixed-width normalization and the boundary behavior.

Validation: npm run test:acp passed all 146 tests; npm run type-check passed. Live, signed-in Antigravity acceptance was not exercised.

Pullfrog  | View workflow run | Using gpt-6.1-sol | 𝕏

@sambitcreate

Copy link
Copy Markdown
Owner Author

/hermes review full

@sambitcreate

Copy link
Copy Markdown
Owner Author

/hermes review full

@sambitcreate

Copy link
Copy Markdown
Owner Author

/hermes review full

@sambitcreate

Copy link
Copy Markdown
Owner Author

/hermes review full

@sambitcreate
sambitcreate merged commit 7ba2fde into main Oct 17, 2026
27 checks passed
@sambitcreate
sambitcreate deleted the feature/acp-host-instructions branch October 17, 2026 01:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

P2: Reused Antigravity sessions ignore refreshed host instructions

2 participants