fix: reconcile declared-key ownership on delete and null (ARN-238) - #401
fix: reconcile declared-key ownership on delete and null (ARN-238)#401rita-aga wants to merge 63 commits into
Conversation
Live local Postgres durability E2E (behavioral regression)Head under test: This used a fresh PostgreSQL 17 cluster and the real Start isolated Postgres and Temper$ /opt/homebrew/opt/postgresql@17/bin/initdb -D /private/tmp/arn238-pg-e2e --no-locale --encoding=UTF8 --auth=trust
The files belonging to this database system will be owned by user "seshendranalla".
...
Success. You can now start the database server using:
'/opt/homebrew/opt/postgresql@17/bin/pg_ctl' -D /private/tmp/arn238-pg-e2e -l logfile start
$ /opt/homebrew/opt/postgresql@17/bin/pg_ctl -D /private/tmp/arn238-pg-e2e -l /private/tmp/arn238-pg-e2e.log -o '-p 55440' start
waiting for server to start.... done
server started
$ /opt/homebrew/opt/postgresql@17/bin/createdb -p 55440 arn238e2e
$ DATABASE_URL=postgresql://127.0.0.1:55440/arn238e2e RUST_LOG=temper_server::registry=info,temper_server::entity_actor=info ./target/debug/temper serve --storage postgres --specs-dir test-fixtures/specs --tenant arn238-live --port 55441 --no-observe$ curl -sS -o /dev/null -w '%{http_code}\n' http://127.0.0.1:55441/tdata
200Establish a deleted entity and its durable sequence$ curl -sS -o /private/tmp/arn238-create.json -w '%{http_code}\n' -X POST http://127.0.0.1:55441/tdata/Directories -H 'content-type: application/json' --data '{"id":"arn238-deleted","Name":"shared-key","WorkspaceId":"ws-live","ParentId":"parent-live","Path":"/shared-key"}'
201
$ /opt/homebrew/opt/postgresql@17/bin/psql -p 55440 -d arn238e2e -Atc "SELECT tenant_id,entity_type,key_name,key_hash,entity_id,sequence FROM entity_key_index WHERE tenant_id='arn238-live' AND entity_type='Directory'"
arn238-live|Directory|name_parent|cac98e79e3395846657ee67fe44479191a8f48eae82977261a5ca8ae6f8d7250|arn238-deleted|1
$ curl -sS -o /dev/null -w '%{http_code}\n' -X DELETE "http://127.0.0.1:55441/tdata/Directories('arn238-deleted')"
204
$ /opt/homebrew/opt/postgresql@17/bin/psql -p 55440 -d arn238e2e -Atc "SELECT count(*) FROM entity_key_index WHERE tenant_id='arn238-live' AND entity_type='Directory' AND entity_id='arn238-deleted'"
0
$ /opt/homebrew/opt/postgresql@17/bin/psql -p 55440 -d arn238e2e -Atc "SELECT max(sequence) FROM events WHERE persistence_id='arn238-live:Directory:arn238-deleted'"
2Seed the faithful pre-v3 stale-owner fixture and observe the regression$ /opt/homebrew/opt/postgresql@17/bin/psql -p 55440 -d arn238e2e -c "INSERT INTO entity_key_index (tenant_id,entity_type,key_name,key_hash,entity_id,sequence) VALUES ('arn238-live','Directory','name_parent','cac98e79e3395846657ee67fe44479191a8f48eae82977261a5ca8ae6f8d7250','arn238-deleted',2)"
INSERT 0 1
$ curl -sS -o /private/tmp/arn238-before.json -w '%{http_code}\n' -X POST http://127.0.0.1:55441/tdata/Directories -H 'content-type: application/json' --data '{"id":"arn238-replacement","Name":"shared-key","WorkspaceId":"ws-live","ParentId":"parent-live","Path":"/shared-key"}'
500
$ cat /private/tmp/arn238-before.json
{"error":{"code":"CreateError","message":"Actor query failed: actor stopped"}}The server log recorded the exact cause and exhausted the normal retry budget: Restart through the upgrade repair, without directly deleting the stale rowThe fresh database had already recorded the current v3 watermark on first boot, so I changed only its contract metadata to the exact pre-v3 representation. The stale ownership row itself remained in place for startup reconciliation. $ /opt/homebrew/opt/postgresql@17/bin/psql -p 55440 -d arn238e2e -c "UPDATE key_index_backfill_watermark SET key_set='name_parent' WHERE tenant_id='arn238-live' AND entity_type='Directory'; UPDATE key_index_contract_state SET key_set='name_parent', revision=1 WHERE tenant_id='arn238-live' AND entity_type='Directory'"
UPDATE 1
UPDATE 1After restarting the same $ /opt/homebrew/opt/postgresql@17/bin/psql -p 55440 -d arn238e2e -Atc "SELECT count(*) FROM entity_key_index WHERE tenant_id='arn238-live' AND entity_type='Directory' AND entity_id='arn238-deleted'"
0
$ /opt/homebrew/opt/postgresql@17/bin/psql -p 55440 -d arn238e2e -Atc "SELECT w.key_set,c.key_set,c.revision FROM key_index_backfill_watermark w JOIN key_index_contract_state c USING (tenant_id,entity_type) WHERE w.tenant_id='arn238-live' AND w.entity_type='Directory'"
v3|11:name_parent[4:Name11:WorkspaceId8:ParentId]|v3|11:name_parent[4:Name11:WorkspaceId8:ParentId]|2
$ curl -sS -o /private/tmp/arn238-after.json -w '%{http_code}\n' -X POST http://127.0.0.1:55441/tdata/Directories -H 'content-type: application/json' --data '{"id":"arn238-replacement","Name":"shared-key","WorkspaceId":"ws-live","ParentId":"parent-live","Path":"/shared-key"}'
201
$ curl -sS -o /private/tmp/arn238-filter.json -w '%{http_code}\n' "http://127.0.0.1:55441/tdata/Directories?%24filter=Name%20eq%20%27shared-key%27%20and%20WorkspaceId%20eq%20%27ws-live%27%20and%20ParentId%20eq%20%27parent-live%27"
200
$ cat /private/tmp/arn238-filter.json
{"@odata.context":"http://127.0.0.1:55441/tdata/$metadata#Directories","value":[{"id":"arn238-replacement","Name":"shared-key","WorkspaceId":"ws-live","ParentId":"parent-live","Path":"/shared-key"}]}
$ curl -sS -o /dev/null -w '%{http_code}\n' "http://127.0.0.1:55441/tdata/Directories('arn238-replacement')"
200
$ /opt/homebrew/opt/postgresql@17/bin/psql -p 55440 -d arn238e2e -Atc "SELECT entity_id,sequence FROM entity_key_index WHERE tenant_id='arn238-live' AND entity_type='Directory' AND key_name='name_parent'"
arn238-replacement|1Second restart durability proofAfter one more stop/start of the same server command: $ curl -sS -o /dev/null -w '%{http_code}\n' "http://127.0.0.1:55441/tdata/Directories?%24filter=Name%20eq%20%27shared-key%27%20and%20WorkspaceId%20eq%20%27ws-live%27%20and%20ParentId%20eq%20%27parent-live%27"
200
$ curl -sS -o /dev/null -w '%{http_code}\n' "http://127.0.0.1:55441/tdata/Directories('arn238-replacement')"
200
$ /opt/homebrew/opt/postgresql@17/bin/psql -p 55440 -d arn238e2e -Atc "SELECT k.entity_id,k.sequence,c.key_set,c.revision FROM entity_key_index k JOIN key_index_contract_state c USING (tenant_id,entity_type) WHERE k.tenant_id='arn238-live' AND k.entity_type='Directory' AND k.key_name='name_parent'"
arn238-replacement|1|v3|11:name_parent[4:Name11:WorkspaceId8:ParentId]|2The Temper server and PostgreSQL cluster were then stopped normally. The isolated cluster remains intact at Unbypassed push gate$ RUST_TEST_THREADS=1 git push
...
test result: ok. 7 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 2397.01s
...
test persisted_wasm_modules_are_lazy_compiled_on_first_invoke ... ok
test persisted_wasm_modules_with_legacy_db_blob_fallback_execute_after_startup_restore ... ok
...
test result: ok. 6 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 7.44s
...
Pre-push: ALL GATES PASSED
To https://github.com/nerdsane/temper.git
bdbf6f6f..465370f8 codex/arn-238-key-ownership-cleanup -> codex/arn-238-key-ownership-cleanup |
|
Independent GPT-5.6 review of the complete GitHub PR diff at head [P1] Fence every repair row mutation against the type contract revision
This leaves a cross-stream race: a backfill begins for contract A; a live write on another entity advances the type to contract B; then the old pass repairs an unchanged entity whose sequence still matches. That repair can commit A rows after B is current. The final watermark CAS correctly returns false, but it does not roll back the already committed stale rows. Those rows are still authoritative for write-time uniqueness, because normal append checks Required fix: carry the expected contract signature/revision into All current CI checks are green, but they do not cover this interleaving. Verdict: FAIL |
|
Independent-review remediation for the current GitHub head de2394b. Initial independent review: #401 (comment) (Verdict: FAIL) Remediation sequence:
Unbypassed serial push gate:
This comment records remediation only. No Greptile request has been made; the mandated fresh independent GitHub-head review comes next. |
|
Reviewed the GitHub-hosted open PR diff at exact head de2394b.\n\n[P1] Do not publish key coverage from an events-plus-key-rows-only repair universe — crates/temper-store-postgres/src/store.rs:1046-1075 makes list_entity_ids_for_key_reconciliation select only events and entity_key_index. That is narrower than the durable entity sources recognized immediately above by list_entity_ids_by_type at lines 993-1035, which also includes entity_catalog and entity_field_index. The startup repair then treats this truncated list as the complete type population at crates/temper-server/src/state/projection_backfill/key_index.rs:107-119 and publishes the authoritative watermark whenever those listed rows have no failures at lines 224-240. After publication, crates/temper-server/src/odata/query_plane_read/keyed.rs:200-235 accepts an empty lookup as authoritative absence. A catalog/snapshot-only entity is therefore omitted from repair, yet a keyed read can return empty and a later claimant can take the same key while the original entity remains catalog-visible. This is a real compatibility shape: docs/adrs/0077-catalog-first-odata-materialization.md:65-69 documents migrated deployments populating entity_catalog directly from snapshots because migrated snapshots never replay through the live transition path. Include every durable entity source in repair enumeration and either reconstruct catalog/snapshot-only state or fail the type closed; add a Postgres regression that seeds such an entity and proves coverage cannot certify a miss.\n\nVerdict: FAIL |
|
Independent GPT-5.6 review of the complete GitHub PR diff and surrounding remote context at exact head [P1] Journal-fence resident actor materialization in the incomplete-key scanWhen exact declared-key coverage is incomplete, or a covered lookup loses its revision fence, Concrete failure: a local actor remains at sequence N with the old key and live state; another process or direct durable writer commits a rename or tombstone at N+1; v3 coverage is incomplete or invalidated during rollout/repair. An exact old-key read then scans, selects the retained ID, materializes actor state N, matches the old key, returns the stale live entity, and can re-upsert that stale projection. This exposes a renamed former owner or resurrects a deleted entity despite the authoritative-scan contract. The covered-key path explicitly prevents this same stale-resident shape by rebuilding from the journal and requiring the recovered sequence to equal the ownership-row sequence ( Verdict: FAIL |
rita-aga
left a comment
There was a problem hiding this comment.
Independent GPT-5.6 complete-open-PR-diff review of exact head 346681ab88e209f34344b502f7a55c52ef3f73a3 against base a28fdb2ed8b0e1e0ea15a99c3b4a1dfba20e160e.
No remaining actionable ARN-238 defect was found. The review covered declared-key replacement/delete/all-null behavior, repair and generation fences, field-update intent binding across live/restart and memory-only eviction/rebinding, authoritative materialization/replay coordinates, persistence/recovery, boundedness, concurrency, and deterministic behavior.
Independent validation: ownership write surfaces 27/27; reconciliation DST 9/9; snapshot materialization 2/2; sim store 29/29; keyed-read matches 51/51; formatting and diff checks PASS.
Verdict: PASS
|
@greptile review Please review the complete current PR diff at exact head |
|
@greptile-apps review Please bypass the file-count limit and review the complete current PR diff at exact head |
rita-aga
left a comment
There was a problem hiding this comment.
Independent GPT-5.6 complete-open-PR-diff review of exact head 975dbdf122be756f97c00be6b7c9064c30f74d34 against base a28fdb2ed8b0e1e0ea15a99c3b4a1dfba20e160e.
No actionable diff-caused findings remain. Declared-key deletion/nulling, ownership transfer, durability, restart recovery, concurrency fencing, and idempotency behavior are sound. The PR-caused default-thread-stack WASM regression is fixed by bounding the enlarged dispatch future through the boxed generation-context result; no WASM test or stack provisioning was modified.
Independent validation reviewed the complete open diff and confirmed default-stack wasm_dispatch 6/6, ownership-focused suites 38/38, and git diff --check PASS. The unbypassed local full workspace gate, strict clippy, and live E2E also passed on this exact head.
Verdict: PASS
|
@greptile review Please review the complete current PR diff at exact head |
|
@greptile-apps review Please bypass the file-count limit and perform a fresh review of the complete current PR diff at exact head |
rita-aga
left a comment
There was a problem hiding this comment.
Verdict: PASS
Fresh independent review at exact commit 6782e1d48580d6e13713ee1b642c5fbdf5ecabfa found no actionable ARN-238 diff-caused defects. Verified exact-set ownership atomicity across actor, data-only, file, composite, PostgreSQL, and Sim paths; source/revision/liveness/snapshot fencing for repair, coverage, tombstones, and audit-only stale-row purge; historical snapshot provenance and spawn-identity preservation; materialized zero-domain owners versus audit-only phantom classification; and default-stack WASM generation-context boxing. Validation included temper-server lib with sim (680 passed), key-ownership write surfaces (28), reconciliation DST (9), passivation/materialization, WASM dispatch, Sim, and PostgreSQL fencing suites—all green.
|
@greptile-apps review Please bypass the file-count limit and perform a fresh complete-diff review of PR #401 at exact head |
rita-aga
left a comment
There was a problem hiding this comment.
Verdict: PASS
Fresh independent GPT-5.6 review of the complete open PR diff at exact commit a703b7d536b0265ffe2ec728af7c73bbeca57c86 against base a28fdb2ed8b0e1e0ea15a99c3b4a1dfba20e160e found no actionable diff-caused defects.
The review rechecked exact-set ownership across actor, field-update, data-only, File, composite, PostgreSQL, and Sim paths; contract epoch/revision/liveness/snapshot fencing; bounded repair and authoritative keyed reads; historical snapshot provenance; materialized zero-domain owners versus audit-only phantom purge; replay-safe field-update intent binding; tenant-generation admission; and the default-stack WASM future boxing. The Greptile P1 remediation preserves durable contract-only tenants by excluding them from empty-registry activation while retaining them in hydration/backfill discovery, and still activates every tenant with live registered specs.
Independent focused validation passed: durable contract-only tenant regression 1/1; ownership write surfaces 28/28; reconciliation DST 9/9; WASM dispatch 6/6; complete PR git diff --check.
|
@greptile review Please perform a fresh complete-diff review of PR #401 at exact head |
|
@greptile-apps review Please bypass the file-count limit and perform a fresh complete-diff review of PR #401 at exact head |
rita-aga
left a comment
There was a problem hiding this comment.
Verdict: PASS
Fresh final independent GPT-5.6 review after Greptile of the complete open PR diff at exact commit a703b7d536b0265ffe2ec728af7c73bbeca57c86 against base a28fdb2ed8b0e1e0ea15a99c3b4a1dfba20e160e found no actionable diff-caused defects.
The review rechecked exact-set declared-key replacement, delete, and all-null release across actor, PATCH/PUT, data-only, File, composite, PostgreSQL, and Sim paths; type-contract activation/revision and stream/liveness/snapshot fencing; bounded repair enumeration and coverage publication; authoritative keyed hit/miss materialization; historical snapshot provenance; materialized zero-domain owners versus audit-only phantom purge; replay-safe field-update intent binding; tenant-generation admission; and the default-stack WASM future boxing.
The fresh Greptile Review check is PASS at this exact head, and all 3 review threads are resolved with 0 unresolved actionable threads. Independent current-head validation passed: durable contract-only tenant regression 1/1, ownership write surfaces 28/28, reconciliation DST 9/9, snapshot-only materialization 2/2, WASM dispatch 6/6, and complete PR git diff --check. The durable contract-only tenant remediation preserves unloaded tenants by excluding them from empty-registry activation while retaining them for hydration/backfill discovery.
ARENA SHIPPABLEARN-238 is shippable at exact OPEN PR head Frozen-head validation
Required published-head review order
RED/GREEN durability evidence
Current-head CIActions run 30194176518 is green at The protected untracked SQLite artifact remains untouched. |
Summary
Root cause
Key ownership was treated as additions-only state. Deletes and key removal could leave durable index rows behind; startup backfill trusted an incomplete name-only watermark and enumerated only live catalog entities, so deleted or orphaned owners could survive indefinitely and block a later valid claimant.
The repair defines one exact reconciliation primitive: the store atomically replaces an entity's declared-key ownership with the key set derived from the newly committed state. Startup compares a versioned property-aware signature, enumerates key-only owners as well as live entities, and advances a durable repair revision only after reconciliation succeeds.
Validation
c7ed59de52308b04,5bdf0a5d465370f8cargo fmt --all -- --checkgit diff --checkcargo clippy -p temper-runtime -p temper-server -p temper-store-postgres -p temper-store-sim -p temper-store-turso -p temper-platform --all-targets --all-features -- -D warningscargo test --workspaceRUST_TEST_THREADS=1 git push→Pre-push: ALL GATES PASSEDLive Postgres E2E evidence is posted as a PR comment. Independent GitHub diff review and Greptile evidence will be linked before
ARENA SHIPPABLE.Safety
This is a correctness and durability repair. It does not weaken uniqueness, drop working capability, or bypass the normal push gates. No files under
crates/temper-actor-runtimeare changed.Linear: ARN-238
Greptile Summary
This PR repairs declared-key ownership reconciliation across durable writes and startup repair. The main changes are:
Confidence Score: 5/5
This looks safe to merge.
Important Files Changed
Reviews (7): Last reviewed commit: "fix(cli): preserve unloaded tenant key c..." | Re-trigger Greptile
Context used (4)