Repository navigation
Conversation
agent_revisions.admitted_at came from the controller clock while controller_work.created_at comes from PostgreSQL's, and four worker sites plus the cleanup SQL compared them. With the controller ahead, a stop could read the running revision as admitted after it and finish STOP_SUPERSEDED without stopping it. The PostgreSQL store now stamps admitted_at with clock_timestamp() and returns the stored stamp; the worker compares through one helper, revisionAdmittedBeforeWork. Closes finding 1195.
…dmission The controller's clock starts the repository credential deadline when it admits a deployment, and PostgreSQL now stamps the revision's createdAt a moment later, after runtime credentials are provisioned. The platform test asserted they were equal; it now checks the deadline base precedes the stored admission by less than a minute.
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
GitHub Actions
Failure
GitHub Actions
Failure
clawsweeper
Bot
added
P2
|
Codex review: blocked before merge. What this changesThis PR uses PostgreSQL timestamps to order revision admissions against stop and deletion work. Example: PostgreSQL admits a revision at 12:00:00 while the controller clock reads 12:05:00, then records a stop at 12:00:01.
Review scores
ProductKind: Bug fix · Worth it: Yes · Fix scope: Complete Merge readiness⛔ Blocked before merge - 1 item remains Keep this PR open: it fixes a source-confirmed stop-ordering bug within the existing lifecycle contract, and no definite introduced bug was found. Priority: P2 Before merge
FindingsNone. Tests
Agent review detailsHow this fits togetherOCC admission persists immutable Agent revisions and durable work; the controller worker reads their ordering, checks ownership and authorization, and invokes Compute and repository-credential cleanup. flowchart TD
A[Authorized deployment] --> B[PostgreSQL revision admission]
C[Authorized stop or deletion] --> D[PostgreSQL controller work]
B --> E[Controller worker ordering]
D --> E
E --> F[Compute lifecycle]
E --> G[Repository credential cleanup]
Technical reviewBest possible solution: Keep database-owned admission timestamps and verify the explicitly retained old-state behavior without rewriting immutable revision history. Do we have a high-confidence way to reproduce the issue? Current-main source establishes the failure with an ahead controller clock; the added PostgreSQL scenario targets that path, but a base-versus-head failing execution was not supplied. Is this the best way to solve the issue? Stamping admission at its persistence owner fixes the clock mismatch across both worker and SQL comparisons while preserving lifecycle authorization and cleanup ownership. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against dc92ed5c8292. Provenance checked
TestingProof path: in-process harness. SecurityNone. EvidenceWhat I checked:
Review metrics
LabelsLabel changes:
Label justifications:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior. WorkflowClawSweeper edits this one comment on every review. Comment Reviewed October 13, 2026, 4:16 AM ET / 08:16 UTC. |
Problem
A stop, an Agent deletion and a Namespace deletion own the revisions admitted before they were requested. The worker decided that ordering by comparing two clocks:
agent_revisions.admitted_atwas stamped by the controller (OpenClawController.timestamp()), andcontroller_work.created_atis stamped by PostgreSQL (clock_timestamp()). Four worker sites compared them (worker.tsSTOP_SUPERSEDED check and stop cleanup set, the Namespace-deletion credential close inobserve(), andbeforeSourceinrepository-credentials.ts), and so did the cleanup SQL inpostgres-work-queue.ts.With the controller clock ahead of PostgreSQL's by d, an active revision admitted less than d before a stop read as admitted after it. The stop then finished
STOP_SUPERSEDED(success) without stopping anything:desiredRuntimeStatebecamestoppedwhile the runtime kept running. Finding 1195, the same family as finding 1187 (#2281).Change
PostgresPlatformState.createRevisionstampsadmitted_atwithclock_timestamp()and returns the stored stamp ascreatedAt(RETURNING admitted_at). Deploy responses already use the returned revision, so the API's revision timestamps are on the database clock too. The caller'screatedAtis no longer stored. The in-memory store is unchanged.revisionAdmittedBeforeWork(controller-work.ts), replaces the four comparisons and documents the tie rule: both reads are truncated to milliseconds, and a tie counts as earlier, so the stop still stops the revision.repositoryCredentials.deadlineWallMsstays on the controller's clock, which the worker checks it against. It is taken at admission, before runtime credentials are provisioned, so it no longer equalscreatedAtplus the session duration exactly.repository-credentials-platform.test.mjsnow checks that the deadline base comes before the stored admission and less than a minute earlier.reconciliation.mdsays the stop ordering uses PostgreSQL's clock.Verification
postgres-application.json: "a stop orders the running revision by the database clock, not the controller clock". For one Agent the admitting clock runs 5 minutes ahead of PostgreSQL's, for another 5 minutes behind (newcontrollerSkewMsfixture knob). It checks the storedadmitted_atis on the database clock and equals the returnedcreatedAt, then deploys, stops and expectsAGENT_STOPPED, a ComputestopRevisioncall, and a cleared active pointer. Before the fix the ahead case finishesSTOP_SUPERSEDEDwithout stopping. Hosted CI is the proof for PostgreSQL tests.createdAt(assertCreatedRevision). The memory runs pass locally:platform-state-memory,repository-sessions-memory(48/48).tsc --buildclean, eslint and prettier clean on touched files,repository-credentials-source-boundary,repository-credentials-packageandconfiguration-occpass (126/126).Risks
deadlineWallMswithclock_timestamp()are an existing cross-clock spot, unchanged here.desiredRuntimeStatetorunning, and the stop rechecks that before each effect, so this cannot stop a newer deployment. IncloseRevisionsuch a tie can make the cleanup registration refuse and roll back the pass; this was already possible before the change.