Skip to content

fix(agent): verify synchronous state acknowledgements - #202

Merged
khaliqgant merged 1 commit into
mainfrom
fix/guardian-sync-ack-readback
Jul 25, 2026
Merged

fix(agent): verify synchronous state acknowledgements#202
khaliqgant merged 1 commit into
mainfrom
fix/guardian-sync-ack-readback

Conversation

@khaliqgant

@khaliqgant khaliqgant commented Jul 25, 2026

Copy link
Copy Markdown
Member

Summary

  • accept a hosted synchronous 2xx state-write acknowledgement only after the same exact SDK readback and revision-advance proof used for queued writes
  • preserve operation polling whenever a non-empty opId is returned
  • keep uncommitted synchronous acknowledgements terminal through readback mismatch

Production evidence

Factory guardian run ebc40a61-8192-41a3-829f-5548fba326a5 loaded all 308 manifest features, then a successful non-queued response reached the unconditional opId assertion before readback. PR #200 did not introduce the assertion; it removed the earlier Slack receipt blockage that had masked this latent hosted-response compatibility bug.

RED first

The two new fixtures initially failed at the old unconditional operation-ID guard: 47 pass, 2 fail.

GREEN

  • guardian + production manifest: 55/55
  • production build: green
  • Prettier and diff check: green
  • cloud deploy dry-run: green, 2 integrations and 1 schedule, no side effects
  • full local Vitest: 1401/1403; only the two pre-existing Node 26 fleet-launcher temp-fixture failures resolving esbuild, unchanged from the prior reviewed hotfix

Safety

Missing opId is never sufficient for success. Both response branches require canonical state readback; updates also require revision advancement. CAS conflicts, HTTP errors, invalid or terminal operation status, uncommitted writes, unchanged revision, and timeouts remain terminal. No deploy performed.


Summary by cubic

Verify hosted synchronous 2xx state-write acknowledgements using the same exact SDK readback and revision-advance proof as queued writes. Only poll operations when a non-empty opId is returned; uncommitted sync acks remain terminal via readback mismatch.

  • Bug Fixes
    • In agent.ts, always perform exact readback; poll only when opId is non-empty.
    • Tests: added committed/uncommitted synchronous-ack cases and operation-poll counter; removed invalid queued-response path.
    • Error handling unchanged: readback mismatches, unchanged revisions, HTTP errors, CAS conflicts, and timeouts remain terminal.

Written for commit f65ca18. Summary will update on new commits.

Review in cubic

@cursor

cursor Bot commented Jul 25, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@coderabbitai

coderabbitai Bot commented Jul 25, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: e544e6d9-7ef5-4f0a-921f-b7a913aeab1f

📥 Commits

Reviewing files that changed from the base of the PR and between 3d669ce and f65ca18.

📒 Files selected for processing (2)
  • .agentworkforce/agents/factory-feature-guardian/agent.test.ts
  • .agentworkforce/agents/factory-feature-guardian/agent.ts

📝 Walkthrough

Walkthrough

The Relayfile test server now simulates committed and uncommitted synchronous acknowledgements while tracking operation polls. SDK saves conditionally wait for queued operations and always perform read-back verification.

Changes

Relayfile synchronous acknowledgement flow

Layer / File(s) Summary
Synchronous acknowledgement test harness
.agentworkforce/agents/factory-feature-guardian/agent.test.ts
RelayfileStateServer tracks operation polls and returns immediate committed or uncommitted 204 responses.
Conditional operation waiting and verification
.agentworkforce/agents/factory-feature-guardian/agent.ts, .agentworkforce/agents/factory-feature-guardian/agent.test.ts
save waits only when a queued operation ID exists, then verifies the write through exact read-back; tests cover polling and synchronous acknowledgement outcomes.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant createSdkProgressStore.save
  participant RelayfileStateServer
  participant ReadBackVerification
  createSdkProgressStore.save->>RelayfileStateServer: PUT SDK content
  RelayfileStateServer-->>createSdkProgressStore.save: 204 acknowledgement
  createSdkProgressStore.save->>ReadBackVerification: Read back and compare content
  ReadBackVerification-->>createSdkProgressStore.save: Exact match or rejection
Loading

Poem

“A 204 came hopping by,
With etag bright or none,” said I.
“We’ll read it back to check the track,
Then safely call it done!”
— A rabbit in the test stack 🐇

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the main change: verifying synchronous state acknowledgements.
Description check ✅ Passed The description is directly aligned with the changes to synchronous acknowledgements, polling, and readback validation.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/guardian-sync-ack-readback

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@khaliqgant
khaliqgant merged commit 565690e into main Jul 25, 2026
7 checks passed
@khaliqgant
khaliqgant deleted the fix/guardian-sync-ack-readback branch July 25, 2026 23:23
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.

1 participant