Repository navigation
fix(global-db): skip retention-released Codex alias owners - #4379
Conversation
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
|
ScriptedAlchemy
left a comment
ScriptedAlchemy
left a comment
There was a problem hiding this comment.
Pushed 5498869 for the rekey test race exposed by the current-head fleet run.
The replacement worker starts immediately and may consume the handed-off request before the test reads the queue length. Four affected journeys now join the worker through the retained wake route and require a canonical Joined result plus a durable Complete receipt. A dropped request yields Started and fails; the empty fixture sessions have no observation history that could reconstruct it. The readmission case holds its existing history fixture until the pending request is handed off. Production code and budgets are unchanged. Pinned rustfmt and diff checks pass; native execution remains for CI.
The same older run also had 29 inherited store-runtime memory-admission refusals at exactly 10 GiB observed/limit with an 8 GiB watermark. Source inspection makes that consistent with zero shared cgroup headroom; the isolated test sampler was not yet installed. Dynamic local/remote TestRunner execution and the logs do not identify the responsible worker/cgroup. I have not raised memory budgets or changed fleet concurrency without that evidence. Those failures remain a separate CI resource issue to verify on the new run.
The spool admits staged envelopes at least once, so a commit-lock timeout can append another frame after the original was already acknowledged. Sequence counts therefore cannot identify distinct captured callbacks. Verify each concurrent Cursor session has exactly one completed capture outcome with accepted_for_replay status. This still fails for an unbound callback and detects missing or duplicated outcomes per session.
Text admission sized from shared cgroup headroom, then reserved an extra watermark band against that same room. A 1.5 GiB build was refused with more than 1.5 GiB available because the reserve requested another 0.6 GiB. Add ceiling-aware reservation to the canonical process authority and use it for text artifacts. The ledger and measured process still obey the watermark, while the cgroup is charged only for the actual allocation. Remove the synthetic margin charge and its immediate shrink. Exercise actual concurrent reservations, a cgroup-constrained admission, and text builds with exactly their required cgroup headroom. Native regressions run in CI; local rustfmt and diff checks pass.
The first-generation and dirty-branch journeys reported only a generic convergence park while their daemon stderr was discarded. Keep a separate log for each existing journey and include it on failure so CI names the refused operation and its actual cause. Keep the existing success assertions, readiness deadlines and polling.
ScriptedAlchemy
left a comment
ScriptedAlchemy
left a comment
There was a problem hiding this comment.
Pushed a follow-up chain for the concrete failures from the Linux fleet:
- Text builds now reserve their actual allocation below the existing admission watermark through ProcessResidentMemory. The old synthetic watermark band charged cgroup headroom twice. Added exact-headroom, over-headroom and overlapping-ledger regressions; no limits increased.
- The mixed-client capture test verifies one accepted_for_replay completion for each expected callback/session. Spool sequence numbers count at-least-once append attempts and are not a unique callback count.
- The two parked-index CLI journeys retain their daemon logs, and the explorer journey recognizes the existing timed_out terminal state and reports the last full source-progress payload. Their timing limits and success assertions are unchanged. These two changes improve diagnosis; they do not claim the remaining convergence failures are fixed.
Pinned rustfmt, whitespace and commitlint pass. Native tests are delegated to the new CI run, and I will continue from its failure evidence. Cross-checked with the PR's sibling-owner issue and the current merged implementation.
Reuse the reviewed reserve_up_to implementation from 863e868 so text builders can reserve a supported smaller budget when headroom changes. Keep sizing, charging, minimum-floor validation, and the bounded shedding retry on one admission path, then park only for the minimum working set. Remove the unused reserve_below facade. Preserve cgroup headroom and real artifact floor regressions, and carry the existing sized-admission and shared-ledger behavior tests.
ScriptedAlchemy
left a comment
ScriptedAlchemy
left a comment
There was a problem hiding this comment.
Pushed the remaining memory-admission convergence fix, using the reviewed implementation from #4393. Text builders now size and charge under the same canonical ledger lock, adapt to a smaller supported reservation, and keep actual admission/minimum validation inside the existing single shed-and-trim retry. Final refusal waits for the minimum working set. Removed the unused reserve_below API; no memory ceiling or retry budget was increased.
Both production files match #4393 exactly. Existing cgroup, artifact floor/refusal, charge release and reader transfer tests are retained, and the sized-admission/overlap regressions exercise the converged path. Imports are now at module scope. Pinned formatting, diff checks and commitlint pass; native behavior remains for CI. Unrelated spool, parked-owner and explorer fixes are preserved.
…dex-sibling-released-owner Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> # Conflicts: # crates/tracedecay-agent-hosts/src/agents/claude/tests.rs # crates/tracedecay-code-index-runtime/tests/code_index_scheduler/serving.rs
…dex-sibling-released-owner Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> # Conflicts: # crates/tracedecay-session-runtime/src/session_temporal_refresh_scheduler/registry_tests.rs
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
Devin Review found 1 new potential issue.
⚠️ 1 issue in files not directly in the diff
⚠️ Rekey tests miss unprocessed refreshes
When rekey_project hands off a request, queued_request_count passes before the replacement worker processes it. The tests no longer require a completed refresh receipt, so lost refreshes pass unnoticed.
2 flags not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)
|
Re Devin Review "Rekey tests miss unprocessed refreshes" ( After the master merge in d93ac28, this file is identical to I tried adding the durable-completion check back on top of master's structure. |
Fixes #4375
Base is
master. The alias-owner preload was added in #4366, which has since merged. The branch merged master in 22ec39b, which also skips alias lookups for writes that already have a terminal admission refusal.Summary
read_retrieval_aliases_from_snapshotrecords an alias owner only when the owner's observation is live (json_extract(owner.observation_json, '$.__retention_released') IS NULL). Alias rows are still returned.RetrievalAnchorAliasCollision. Host admission maps that to the deterministic refusalobservation_retrieval_anchor_alias_collision. Before this change the write got a retryableStorageerror, so the rollout failed again on every pass.Motivation / root cause
#4366 preloads
observation_retrieval_anchors.observation_idfor every alias anchor behind a keyed Codex write, then decodes those observations inread_stored_observations_from_snapshot. When retention has released an owner, itsobservation_jsonis{"__retention_released":"observation"}. Decoding that marker fails the whole preflight withStorage { operation: "load observation batch preflight", source: Serde("unknown field__retention_released…") }. As the issue says, production does not write Deleted/Superseded dispositions for Codex anchors today, so this is hardening.One correction to the issue text. A typed collision still fails its whole batch segment.
persist_observation_segment_bodypropagates every per-write prepare error, so an unrelated write in the same segment is not committed. Every other alias collision behaves the same way, and this PR does not change that. The test pins this behaviour, then shows the unrelated write commits when sent on its own and the sibling write alone is still refused.Changes
crates/tracedecay-global-db/src/observation_adapter.rs: added aLEFT JOIN observations AS owner … AND … IS NULLand selectowner.observation_id.crates/tracedecay-global-db/src/observation_adapter.rs(merge resolution, 22ec39b):read_retrieval_aliases_from_snapshotreceivesadmission_refusalsand requests no aliases for writes that already have a terminal admission refusal.crates/tracedecay-global-db/src/observation_batch_tests.rs: addedcodex_sibling_over_a_retention_released_owner_is_a_typed_alias_collision. It commits the thread rollout, appends a productionDeleteddisposition dated 40 days back (append_disposition), runsrun_observation_retentionwith the default config in Apply mode and asserts that one anchor and one observation were released. Then it callspersist_observations([unrelated, revert]).Current CI status
The Linux fleet run on
90776a5lost its hosted runner during test-binary compilation. GitHub reported: "The hosted runner lost communication with the server." Test execution never started and no diagnostic artifact was uploaded. Clippy, shipped CLI build, repository gates, feature gates, and Dashboard passed. This is an infrastructure interruption, not a failing test result. The branch includes current master.GitHub refused an individual job retry while the original workflow was still running its aggregate, so a fresh default CI run was dispatched on the unchanged head. Full CI must pass before merge; no code, assertions, or limits were changed for this interruption.
Test plan
cargo test -p tracedecay-global-db --lib observation_batch_tests::codex_sibling_over_a_retention_released_owner_is_a_typed_alias_collision -- --exactfailed withgot Storage { operation: "load observation batch preflight", source: Serde(Error("unknown field__retention_released" ….cargo test -p tracedecay-global-db --lib observation_batch_tests::: 18 passed.cargo test -p tracedecay-global-db --lib retention: 23 passed.cargo fmt --all -- --checkandcargo clippy -p tracedecay-global-db --all-targets -- -D warningsboth passed.Checklist
CHANGELOG.mdnot updated (generated by release-please).envfiles includedLink to Devin session: https://app.devin.ai/sessions/abd0328c18d54cb39bcc989a2dd2ab5b
Open in Devin Desktop: https://app.devin.ai/desktop/session/abd0328c18d54cb39bcc989a2dd2ab5b?variant=devin
Requested by: @ScriptedAlchemy