fix(google-antigravity): persist thought-signature replay across restarts - #1434
fix(google-antigravity): persist thought-signature replay across restarts#1434Yuxin-Qiao wants to merge 10 commits into
Conversation
|
✅ Deterministic PR hygiene checks passed. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe Antigravity replay cache now persists validated ChangesAntigravity replay persistence
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant AntigravityReplayCache
participant ConfigDirectory
participant ServerLifecycle
Client->>AntigravityReplayCache: Observe tool-call signature
AntigravityReplayCache->>ConfigDirectory: Load validated snapshot
ConfigDirectory-->>AntigravityReplayCache: Return retained signature
AntigravityReplayCache-->>Client: Apply signature to continuation
ServerLifecycle->>AntigravityReplayCache: Flush pending replay writes
AntigravityReplayCache->>ConfigDirectory: Atomically persist snapshot
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
⏳ DRAFT
What to do
Review readiness checklist
0/4 boxes ticked. New commits were pushed after the checklist was completed on |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs-site/src/content/docs/reference/adapters.md`:
- Around line 93-96: Update the documentation text describing Vertex and
Antigravity so it explicitly calls preserved and replayed thoughtSignature
values opaque. Keep the wording clear that adapters preserve and replay these
values, without implying clients provide or reconstruct them.
In `@src/adapters/google-antigravity-replay.ts`:
- Around line 91-102: In the replay-entry admission logic around the call
validation and byCall.set, calculate the UTF-8 byte size from call.signature and
pair[0], reject entries whose persisted call.sizeBytes differs from that
calculated value, and use the calculated size for ReplayCall.sizeBytes and
entry.bytes accounting. Add a regression test covering an underreported
sizeBytes value.
- Around line 199-200: Update flushAntigravityReplay() so that after awaiting
replaySnapshotPersistGate when no timer is initially present, it rechecks
replaySnapshotPersistTimer and calls persistReplaySnapshotNow() once if a timer
was scheduled during the in-flight write, ensuring the final mutation is
persisted before shutdown completes.
In `@tests/google-antigravity-replay.test.ts`:
- Line 563: Remove the duplicate const raw declaration in the test’s
snapshot-loading scope, keeping a single declaration with the existing version
and sessions typing so the file parses successfully.
- Around line 33-42: Inside describe("durable antigravity replay snapshot", ...)
add a nested beforeEach hook that calls setAntigravityReplayLimitsForTests(),
ensuring replaySnapshotLoaded and replayCache are reset before each test while
preserving the existing environment setup and cleanup hooks.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e40946e5-1fd7-493d-83b4-49162bbc7829
📒 Files selected for processing (8)
docs-site/src/content/docs/ja/reference/adapters.mddocs-site/src/content/docs/ko/reference/adapters.mddocs-site/src/content/docs/reference/adapters.mddocs-site/src/content/docs/ru/reference/adapters.mddocs-site/src/content/docs/zh-cn/reference/adapters.mdsrc/adapters/google-antigravity-replay.tssrc/server/lifecycle.tstests/google-antigravity-replay.test.ts
Wibias
left a comment
Wibias
left a comment
There was a problem hiding this comment.
Request changes based on a full bugs/edge-cases/security review of the current head.
Merge blockers:
-
Snapshot loading trusts serialized
sizeBytesinstead of deriving it from the persisted signature. A corrupt/tampered snapshot can store a very largesignaturewithsizeBytes: 0/1, bypassingmaxSignatureBytes, per-session byte limits, replay byte accounting, and app-owned memory accounting. Recompute the authoritative UTF-8 byte length fromsignatureduring load, enforce the limits against that value, and use it for bookkeeping. Prefer not serializing derivedsizeBytesat all. Add a regression with a large signature plus falsified smallsizeBytes. -
flushAntigravityReplay()is not guaranteed to include the final mutation during shutdown. If a writer is already in flight and no debounce timer exists when flush starts, flush only awaits the current gate. A mutation during that write can schedule a new debounce timer; the first writer completes, flush returns, and shutdown can exit before the newly scheduled write runs. Use a dirty/mutation generation or loop until the persisted generation catches the current mutation generation and no writer/timer remains. Add a deterministic blocked-writer test with a second mutation during the first write.
Required hardening:
- Snapshot persistence errors are swallowed inside
persistReplaySnapshotNow(). This also makes the lifecycle'sPromise.allSettled(...flushAntigravityReplay())rejection handling ineffective for real disk/ACL/rename failures. The cache need not crash the service, but durability loss should at least produce a sanitized warning/metric or otherwise reach the shutdown diagnostic path.
Security-positive note: using guarded atomic writes and hardened local permissions is the right direction, and I did not find raw credentials/account ids in the snapshot.
I explicitly did NOT adopt the stale bot claims about a duplicate const raw declaration or cross-test snapshot leakage; those do not reproduce on this head.
Please fix the two durability/integrity blockers, then refresh onto current dev and obtain green exact-head CI.
4a8b045 to
5796144
Compare
|
Thanks for the detailed review. All three points are fixed on the refreshed head (rebased onto current 1. Snapshot loading no longer trusts serialized
Regression coverage in 2. I added a mutation generation ( Deterministic regression: "flush waits out a blocked writer and persists mutations that land during it" blocks the first snapshot write via the 3. Persistence failures are no longer swallowed The debounced background path now logs a sanitized static warning on failure (no error payload, no paths). Coverage: "flush surfaces snapshot write failures for shutdown diagnostics" and "background snapshot failures log a redacted warning and keep the service running". Verification on the new head: replay suite 51/51, adjacent suites 168/168 across |
|
Tip For best results, initiate chat on the files or code changes.
You are interacting with an AI system. |
|
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/adapters/google-antigravity-replay.ts`:
- Around line 214-226: Bound the retry loop in flushAntigravityReplay so it
cannot wait indefinitely while replay mutations continue arriving. Add a small
iteration limit around the existing persistReplaySnapshotNow and
replaySnapshotPersistGate flow, while preserving the current behavior of
persisting pending mutations and awaiting in-flight writes; after the bound is
reached, return with the latest observed state persisted.
In `@tests/google-antigravity-replay.test.ts`:
- Around line 727-739: Update the background snapshot failure test around the
console.warn spy to restore the captured spy after assertions complete. Ensure
cleanup runs even when the test fails, using the test’s existing setup or
teardown mechanism rather than leaving console.warn mocked for subsequent tests.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b3479356-ac60-4218-bb80-f2c61ea8b142
📒 Files selected for processing (7)
docs-site/src/content/docs/ja/reference/adapters.mddocs-site/src/content/docs/ko/reference/adapters.mddocs-site/src/content/docs/reference/adapters.mddocs-site/src/content/docs/ru/reference/adapters.mddocs-site/src/content/docs/zh-cn/reference/adapters.mdsrc/adapters/google-antigravity-replay.tstests/google-antigravity-replay.test.ts
|
Tip For best results, initiate chat on the files or code changes.
You are interacting with an AI system. |
|
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/adapters/google-antigravity-replay.ts (2)
73-130: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winPersist load-time removals.
The loader drops expired or over-limit entries from memory, but the load path does not mark the snapshot dirty. If the first access only reads metrics or performs a cache miss,
antigravity-replay.jsonremains unchanged. Expired signatures then remain on disk and are parsed again after every restart.Track whether loading removed entries and schedule one rewrite after loading. Extend the expiry test to flush and verify that the stale session key is absent from the file.
Also applies to: 132-160
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/adapters/google-antigravity-replay.ts` around lines 73 - 130, Track whether loadReplaySnapshotEntry or load-time trimming removes any snapshot data, mark the replay snapshot dirty, and schedule a rewrite once snapshot loading completes so stale entries are removed from disk. Preserve normal loading behavior when nothing is discarded, and extend the expiry test to flush persistence and verify the expired session key is absent from the snapshot file.
162-214: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winOrder replay sessions by activity before applying the snapshot cap.
At
src/adapters/google-antigravity-replay.ts:190-197, reverseMaporder does not represent session activity.observeAntigravityReplayrefreshesexpiresAtMs, butMap.setpreserves the key’s original position. A recently updated older session can therefore be excluded when a newer inactive session consumes the 24 MiB cap.applyAntigravityReplayalso updates onlytouchedAtMsat lines 635-642, soexpiresAtMsdoes not represent all session activity.Track a session-level activity timestamp on both paths, sort candidates by that timestamp before enforcing the cap, and add a regression test in
tests/google-antigravity-replay.test.tsfor an older session updated after a newer session.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/adapters/google-antigravity-replay.ts` around lines 162 - 214, Track a session-level activity timestamp in both observeAntigravityReplay and applyAntigravityReplay, updating it whenever either path refreshes a session. In persistReplaySnapshotNow, order session candidates by this activity timestamp, newest first, before applying REPLAY_SNAPSHOT_MAX_BYTES instead of relying on reversed Map order; persist and restore the timestamp as needed. Add a regression test covering an older session updated after a newer session so the recently active session is retained under the cap.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/google-antigravity-replay.test.ts`:
- Around line 734-736: Replace the fixed Bun.sleep in the debounce test with the
existing write seam, signaling when the failing background write is reached and
waiting for that signal with a bounded timeout before checking warn.mock.
Preserve the warning assertion and existing finally cleanup.
---
Outside diff comments:
In `@src/adapters/google-antigravity-replay.ts`:
- Around line 73-130: Track whether loadReplaySnapshotEntry or load-time
trimming removes any snapshot data, mark the replay snapshot dirty, and schedule
a rewrite once snapshot loading completes so stale entries are removed from
disk. Preserve normal loading behavior when nothing is discarded, and extend the
expiry test to flush persistence and verify the expired session key is absent
from the snapshot file.
- Around line 162-214: Track a session-level activity timestamp in both
observeAntigravityReplay and applyAntigravityReplay, updating it whenever either
path refreshes a session. In persistReplaySnapshotNow, order session candidates
by this activity timestamp, newest first, before applying
REPLAY_SNAPSHOT_MAX_BYTES instead of relying on reversed Map order; persist and
restore the timestamp as needed. Add a regression test covering an older session
updated after a newer session so the recently active session is retained under
the cap.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 43f1b4d3-ad6b-434f-910b-fb3b4f2cb761
📒 Files selected for processing (2)
src/adapters/google-antigravity-replay.tstests/google-antigravity-replay.test.ts
|
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/adapters/google-antigravity-replay.ts (1)
117-122: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReject duplicate call keys during snapshot loading.
A duplicate
pair[0]replaces the prior value inbyCall, butbytesstill includes both call sizes. This can overstatereplayBytes, trigger eviction of valid sessions, or drop the only retained call when per-session trimming runs.Reject duplicate keys and mark the snapshot for cleanup. Alternatively, subtract the replaced
ReplayCall.sizeBytesbefore adding the replacement size. Add a regression test with two serialized pairs that use the same call key.Proposed fix
const callBytes = utf8.encode(pair[0]).byteLength + signatureBytes; if (callBytes > replayLimits.maxBytesPerSession) continue; + if (byCall.has(pair[0])) { + replaySnapshotLoadDiscarded = true; + continue; + } byCall.set(pair[0], { signature: call.signature, sizeBytes: callBytes, touchedAtMs: call.touchedAtMs, });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/adapters/google-antigravity-replay.ts` around lines 117 - 122, Update snapshot loading around the byCall map insertion to detect an existing pair[0] before replacing it, reject duplicate call keys, and mark the snapshot for cleanup. Ensure duplicate entries cannot inflate bytes or interfere with per-session trimming, and add a regression test covering two serialized pairs with the same call key.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/adapters/google-antigravity-replay.ts`:
- Around line 222-225: Update the snapshot size accounting around persistEntry
and replaySnapshotMaxBytes to include the exact UTF-8 byte length of the
complete {"version":...,"sessions":[...]} framing, including commas between
entries, before accepting each session. Preserve the cap when writing the final
snapshot, and add a boundary test that verifies the written file byte length
never exceeds the configured limit.
---
Outside diff comments:
In `@src/adapters/google-antigravity-replay.ts`:
- Around line 117-122: Update snapshot loading around the byCall map insertion
to detect an existing pair[0] before replacing it, reject duplicate call keys,
and mark the snapshot for cleanup. Ensure duplicate entries cannot inflate bytes
or interfere with per-session trimming, and add a regression test covering two
serialized pairs with the same call key.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d9cc0732-3fe8-420f-b76e-3083d9cb9c0a
📒 Files selected for processing (2)
src/adapters/google-antigravity-replay.tstests/google-antigravity-replay.test.ts
Ingwannu
left a comment
Ingwannu
left a comment
There was a problem hiding this comment.
The persistence direction is valid, and the earlier size-accounting, in-flight mutation, and error-propagation blockers have been substantially addressed. I am still requesting changes on current head 244ade8393e4b201077ad5d02b3ba536b8881231 because two durability contracts remain incomplete:
persistReplaySnapshotNow()enforcesreplaySnapshotMaxBytesagainst only the sum ofJSON.stringify(persistEntry)values. The written document also contains the version/sessions wrapper, brackets, and inter-entry commas, so the final UTF-8 file can exceed the declared hard cap. Account for the exact complete document size before admitting each session (or construct an exactly bounded document), and add a boundary regression that assertsBuffer.byteLength(the entire written snapshot) <= configured capwith multiple small sessions.flushAntigravityReplay()now bounds retries, but after eight non-converging attempts it returns successfully even whenreplayMutationGeneration !== replayWrittenGenerationor a timer is still pending.drainAndShutdown()therefore sees a fulfilled flush and emits no failure diagnostic although the latest signature was not made durable. Exhausting the retry budget must return an explicit failure (preferably a fixed, non-sensitive error) so the existing shutdown warning is reachable. Add a deterministic test that mutates during every attempted write and proves the flush fails rather than silently claiming success.
The branch is also two commits behind current dev, and this fork head currently has only the lightweight PR gates rather than exact-head platform/full-suite CI. After the two contracts are fixed, please refresh onto current dev, rerun the focused persistence/lifecycle suites plus typecheck/privacy, and obtain the full exact-head CI before merge.
244ade8 to
edf6ef0
Compare
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/google-antigravity-replay.test.ts`:
- Around line 658-687: Update the test around antigravity replay loading to use
a multi-byte UTF-8 signature for the admitted call and assert totalBytes with
TextEncoder byte length rather than character length. Add an oversized
multi-byte signature whose character count is below the signature limit but
whose encoded byte length exceeds it, and verify that session is dropped. Keep
the regression coverage focused near the existing load recomputation test.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a30ea0e3-c64a-4a1a-ae8f-9cbeac73b2fc
📒 Files selected for processing (1)
tests/google-antigravity-replay.test.ts
edf6ef0 to
89a96ce
Compare
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
head_sha='89a96ce0ebf6707278bf83a3077ff9b845eb8b0e'
echo '=== Checkout and reported head ==='
git rev-parse HEAD
git cat-file -e "${head_sha}^{commit}"
git show -s --format='commit=%H%nparents=%P%nsubject=%s' "$head_sha"
echo '=== Changed files relative to dev merge base ==='
base_sha="$(git merge-base "$head_sha" origin/dev)"
git diff --name-status "$base_sha" "$head_sha"
echo '=== Replay module outline ==='
ast-grep outline src/adapters/google-antigravity-replay.ts --items all --view expanded
echo '=== Snapshot load and persistence at reported head ==='
git show "${head_sha}:src/adapters/google-antigravity-replay.ts" \
| nl -ba \
| sed -n '80,300p'
echo '=== Cache expiry, eviction, mutation, and flush paths at reported head ==='
git show "${head_sha}:src/adapters/google-antigravity-replay.ts" \
| nl -ba \
| sed -n '430,650p'
echo '=== Lifecycle shutdown integration at reported head ==='
git show "${head_sha}:src/server/lifecycle.ts" \
| nl -ba \
| sed -n '1,260p'
echo '=== Durable replay regression tests at reported head ==='
git show "${head_sha}:tests/google-antigravity-replay.test.ts" \
| rg -n -C 5 \
'(duplicate.*session|duplicate.*key|expired.*snapshot|snapshot.*expired|sweepExpiredAntigravityReplay|flush.*blocked|flush.*failure|retry|activity|sizeBytes|permission|replay)' \
|| true
echo '=== Full PR diff for replay module and lifecycle ==='
git diff --unified=50 "$base_sha" "$head_sha" -- \
src/adapters/google-antigravity-replay.ts \
src/server/lifecycle.ts \
tests/google-antigravity-replay.test.tsLength of output: 2235 🏁 Script executed: #!/bin/bash
set -euo pipefail
head_sha='89a96ce0ebf6707278bf83a3077ff9b845eb8b0e'
parent_sha="$(git rev-parse "${head_sha}^")"
echo '=== Commit range ==='
git show -s --format='head=%H%nparent=%P%nsubject=%s' "$head_sha"
echo '=== Changed files in head commit ==='
git diff --name-status "$parent_sha" "$head_sha"
echo '=== Replay module structure ==='
ast-grep outline src/adapters/google-antigravity-replay.ts --items all --view expanded
echo '=== Snapshot loading and write scheduling ==='
nl -ba src/adapters/google-antigravity-replay.ts | sed -n '1,330p'
echo '=== Cache mutation, expiration, eviction, and flush behavior ==='
nl -ba src/adapters/google-antigravity-replay.ts | sed -n '400,700p'
echo '=== Shutdown integration ==='
ast-grep outline src/server/lifecycle.ts --items all --view expanded
nl -ba src/server/lifecycle.ts | sed -n '1,280p'
echo '=== Relevant durable-cache tests ==='
rg -n -C 7 --type ts \
'(duplicate.*(session|key)|expired.*snapshot|snapshot.*expired|sweepExpiredAntigravityReplay|flush.*(blocked|failure|mutation|retry)|activity|sizeBytes|UTF-8|utf8|write.*warning)' \
tests/google-antigravity-replay.test.ts
echo '=== Head commit diff ==='
git diff --unified=45 "$parent_sha" "$head_sha" -- \
src/adapters/google-antigravity-replay.ts \
tests/google-antigravity-replay.test.ts \
src/server/lifecycle.tsLength of output: 7542 🏁 Script executed: #!/bin/bash
set -euo pipefail
numbered_range() {
local file="$1"
local first="$2"
local last="$3"
awk -v first="$first" -v last="$last" \
'NR >= first && NR <= last { printf "%6d %s\n", NR, $0 }' "$file"
}
echo '=== Snapshot load, mutation tracking, persistence, and flush ==='
numbered_range src/adapters/google-antigravity-replay.ts 80 275
echo '=== Expiration and eviction ==='
numbered_range src/adapters/google-antigravity-replay.ts 460 565
echo '=== Observe, apply, clear, metrics, and test reset ==='
numbered_range src/adapters/google-antigravity-replay.ts 565 790
echo '=== Shutdown flushing ==='
numbered_range src/server/lifecycle.ts 1 280
echo '=== All durable replay test names ==='
rg -n --type ts '^\s*(test|it)\(' tests/google-antigravity-replay.test.ts
echo '=== Test blocks for prior findings and later hardening ==='
rg -n -C 18 --type ts \
'(duplicate top-level|duplicate.*session|expired.*snapshot|snapshot.*expired|flush waits|flush.*failure|background.*failure|bounded.*flush|activity.*timestamp|UTF-8|utf8|sizeBytes)' \
tests/google-antigravity-replay.test.ts
echo '=== Test-only head commit ==='
git diff --unified=60 c6a3d709f84c7299cb80a68c7594384fda30f7a7 \
89a96ce0ebf6707278bf83a3077ff9b845eb8b0e -- tests/google-antigravity-replay.test.tsLength of output: 69299
You are interacting with an AI system. |
|
Tip For best results, initiate chat on the files or code changes.
Two findings from that review remain open at this head:
If a new commit addresses these two points, push it and I will review the new head. You are interacting with an AI system. |
Wibias
left a comment
Wibias
left a comment
There was a problem hiding this comment.
Request changes on current head 89a96ce0ebf6707278bf83a3077ff9b845eb8b0e.
Two durability contracts are still incomplete:
-
flushAntigravityReplay()bounds its convergence loop, but exhaustingREPLAY_FLUSH_MAX_ATTEMPTScurrently falls through and resolves successfully even whenreplayMutationGeneration !== replayWrittenGenerationor a debounce timer remains pending. In that statedrainAndShutdown()observes a successful flush even though the latest thought signature may not be durable. Keep the retry bound, but make budget exhaustion reject with a fixed, non-sensitive error so shutdown diagnostics remain truthful. Add a deterministic regression that forces a mutation during every attempted write and proves the flush rejects rather than silently succeeding. -
Snapshot write-cap accounting only sums
Buffer.byteLength(JSON.stringify(persistEntry), "utf8"). The actual persisted document also contains the{ "version": ..., "sessions": [...] }framing, array brackets, and commas between entries, so the final UTF-8 file can exceed the advertisedreplaySnapshotMaxByteshard cap. Account for the exact serialized document size before admitting each session, and add a boundary regression that assertsBuffer.byteLength(the complete written snapshot, "utf8") <= configured capwith multiple entries.
The earlier integrity, in-flight mutation, error-propagation, load-cleanup, duplicate-key, UTF-8 accounting, and redacted-warning fixes look good. After these two remaining contracts are fixed, refresh onto current dev and rerun exact-head CI.
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo='lidge-jun/opencodex'
pr='1434'
echo '=== Pull request head ==='
gh pr view "$pr" --repo "$repo" --json number,headRefOid,baseRefOid,state,isDraft \
--jq '"number=\(.number)\nhead=\(.headRefOid)\nbase=\(.baseRefOid)\nstate=\(.state)\ndraft=\(.isDraft)"'
echo '=== Checked-out commit ==='
git rev-parse HEAD
echo '=== Replay module structure ==='
ast-grep outline src/adapters/google-antigravity-replay.ts --items all --view expanded
echo '=== Snapshot loading ==='
nl -ba src/adapters/google-antigravity-replay.ts | sed -n '90,180p'
echo '=== Expiry cleanup and mutation scheduling ==='
nl -ba src/adapters/google-antigravity-replay.ts | sed -n '470,545p'
echo '=== Relevant regression tests ==='
rg -n -C 12 --type ts \
'(duplicate top-level|duplicate.*session|replayCache\.has\(key\)|expired.*snapshot|snapshot.*expired|sweepExpiredAntigravityReplay|deleteExpiredReplaySessions)' \
tests/google-antigravity-replay.test.ts || trueLength of output: 7104 🏁 Script executed: #!/bin/bash
set -euo pipefail
numbered_range() {
local file="$1"
local first="$2"
local last="$3"
awk -v first="$first" -v last="$last" \
'NR >= first && NR <= last { printf "%6d %s\n", NR, $0 }' "$file"
}
echo '=== Snapshot loading ==='
numbered_range src/adapters/google-antigravity-replay.ts 90 160
echo '=== Mutation scheduling and flushing ==='
numbered_range src/adapters/google-antigravity-replay.ts 180 275
echo '=== Runtime expiry cleanup ==='
numbered_range src/adapters/google-antigravity-replay.ts 465 530
echo '=== Regression-test matches ==='
rg -n -C 15 --type ts \
'(duplicate top-level|duplicate.*session|replayCache\.has\(key\)|expired.*snapshot|snapshot.*expired|sweepExpiredAntigravityReplay|deleteExpiredReplaySessions)' \
tests/google-antigravity-replay.test.ts || trueLength of output: 17652
GitHub reports PR Two previously reported replay-cache defects remain at this head.
The PR should remain unresolved until these two fixes are present on a new head. You are interacting with an AI system. |
…arts The antigravity replay cache was process-local, so a tool-call continuation on /v1/chat/completions 400'd with 'missing a thought_signature' once the cache was lost (proxy restart, TTL expiry, or eviction pressure). The OpenAI-format surface has no field for a caller to carry the signature back, so the conversation was stuck. Snapshot the cache to antigravity-replay.json in the config directory: debounced atomic writes on mutation, lazy bounded load on first access, drop expired sessions at load, and a shutdown flush wired into drainAndShutdown. The CCA session id is deterministic (anchored on the first user message), so a restarted proxy re-derives the same key and re-injects the signatures. Claude-on-Antigravity is unaffected (it uses inline sanitization, not the replay cache).
…tation, surface persist failures
…ap by activity, deterministic failure test
89a96ce to
3dad8a8
Compare
|
Tip For best results, initiate chat on the files or code changes.
The reported head is
The PR must remain unresolved until a new commit fixes both defects and includes the requested regression tests. You are interacting with an AI system. |
|
Landed on Your commits are on
The snapshot cap counted only per-entry JSON and ignored the You reached equivalent fixes independently on your newer head — Verified: |
4 participants
Summary
src/adapters/google-antigravity-replay.tswas process-local (1h TTL, 10,240 sessions, batch eviction). Afterocx service stop && ocx service start(or TTL/eviction pressure), a/v1/chat/completionscontinuation 400s withFunction call is missing a thought_signature, and the OpenAI-format surface has no field for the caller to echo the signature back — the conversation was stuck. The/v1/responsespath was unaffected because the signature travels in the payload there.antigravity-replay.jsonin the config directory (same durability pattern asresponses-state.json), lazy bounded load on first cache access with expired sessions dropped at load, and a shutdown flush wired intodrainAndShutdown. The CCA session id is deterministic (SHA-256 of the first user message text), so a restarted proxy re-derives the same key and re-injects the cached signatures.antigravityUsesReplayCacheis Gemini-only).atomicWriteFileAsync(0600 file, 0700 dir, real-home write guard under test).Verification
bun test tests/google-antigravity-replay.test.ts— 53 pass, including restart simulation (observe -> flush -> reset -> apply re-injects), flush before debounce, load-time expiry and cap cleanup, corrupt/unknown-version refusal, oversized-file refusal, clear-on-invalid, duplicate-key collapse, UTF-8 byte accounting, and the review regressions below.sizeBytes(including multi-byte signatures whose UTF-8 size differs from JavaScript string length), snapshot never serializessizeBytes, flush waits out a blocked writer and persists mutations that land during it (deterministicafterTempWriteseam), flush surfaces snapshot write failures for shutdown diagnostics, background snapshot failures log a redacted warning and keep the service running.bun test tests/responses-state.test.ts tests/server-background-lifecycle.test.ts tests/shutdown-drain.test.ts tests/google-antigravity-wire.test.ts tests/google-antigravity-oauth.test.ts tests/state-store-sweeper.test.ts tests/app-owned-memory.test.ts tests/storage-mutation-race.test.ts— 218 pass.bun run typecheck— clean.bun run privacy:scan— passed.cd docs-site && bun install --frozen-lockfile && bun run build— 221 pages built (English + ja/ko/ru/zh-cn updated).bun run test: 10,926 pass, 8 skipped, 20 baseline failures. Failures are confined to unrelated catalog/live-update/provider-management, crash-guard, and retained-root seam tests; none overlap with this change. The replay suite remains 53/53.Checklist
Review readiness checklist
This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:
All CI tests are green on my local testing.
I pushed my PR to the latest dev commit.
I resolved all correct Codex and CodeRabbit findings.
My PR is ready for review.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
thoughtSignaturesnapshots.