Skip to content

perf(agent): bound and coalesce SWM expiry cleanup - #2518

Closed
branarakic wants to merge 76 commits into
testnet-canaryfrom
perf/issue-1961-bounded-swm-expiry
Closed

branarakic wants to merge 76 commits into
testnet-canaryfrom
perf/issue-1961-bounded-swm-expiry

Conversation

@branarakic

@branarakic branarakic commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

SWM expiry cleanup processes at most 250 operations per page and four nonempty pages per physical pass. Continuations visit remaining targets fairly and stop when no operations are removed. Manual cleanup awaits the drain; periodic maintenance yields between bounded passes. Each pass uses one explicit cutoff, and retention changes invalidate active generations.

Cleanup and publisher writes share entity/KA lock identities. Operations are revalidated under those locks before deletion, and newer KA heads survive expiry of old operations. Storage resolves complete graph families, preserving staging data and root/subgraph identities. Four concurrent operation pipelines drain every admitted sibling; counted mutations remain serialized per target. A targeted post-delete metadata check drives progress, so unrelated writes that offset graph-wide deletion counts cannot strand later expired pages.

TTL, LLM and telemetry settings use one live DkgConfigStore per resolved file. The generic ConfigFileStore owns only atomic publication and serialization. Opening the live owner drains previously admitted local writes, acquires a process-wide SQLite file lock, and loads the initial deeply immutable snapshot while holding that lock. Standalone configuration writes use the same lock; a CLI such as publisher enable is explicitly rejected while the daemon owns the file. The permanent empty sidecar carries no configuration data, and the operating system releases ownership after normal closure or process death. A completed pre-claim save is authoritative as a full snapshot, so deleted optional fields cannot be restored from stale startup state. Direct paths, file symlinks and directory aliases share that owner.

Each live update rebases inside the publication queue and explicitly declares either configuration-only semantics or a prepared runtime apply/rollback pair. The file candidate publishes before activation. Failed publication leaves runtime untouched; failed activation invokes compensation and restores the exact prior file before admitting another update. Both recovery steps are attempted independently, with aggregated errors and a retained backup if file restoration fails. If either recovery step fails, the live owner rejects queued and later updates before candidate preparation and retains its write claim until closure; settings changes require a daemon restart. Ordinary activation failures with successful recovery keep the lane usable. TTL compensation restores retention, LLM compensation restores the memory manager, and telemetry compensation restores the previous gate and signals.

Slow local-agent connect and refresh preparation remains outside the commit queue. Producers return explicit attach-state patches alongside response records; connect also returns normalized registration intent and carries failed-probe state in a typed error. Routes rebase that intent and those patches through the live owner, preserving concurrent operator metadata/capability edits, applying the probe result even when it matches the starting snapshot, and honoring newer operator disconnects. Deferred attach completion uses the same explicit patch contract. Read helpers accept immutable inputs; mutations use explicit drafts and owner updates. Daemon startup uses an explicitly immutable startupConfig; telemetry callbacks read the current committed settings at each activation while resource identity remains tied to startup. Request contexts expose the canonical store directly, and Kafka reads its namespaced settings from that live readonly projection on each dispatch. The create and subscribe --save CLI flows persist always-on subscriptions through the existing daemon subscription owner instead of writing a second config snapshot.

Validation:

  • 773 tests pass across 48 CLI configuration, process ownership, settings, route, authentication, relay, telemetry, logging and integration suites. Eight cases run the built daemon against a mock chain; these include a separate publisher CLI process followed by a TTL update and both --save subscription commands, with an independent read of the persisted subscription record.
  • Cross-process cases cover direct paths and file/directory aliases, ordinary release, abrupt process death, startup racing an external publication, and failed-startup cleanup. The telemetry regression disables and re-enables the real settings lifecycle after a committed endpoint/token change, checking current settings and unchanged immutable boot identity.
  • Removing the process lock fails eight cross-process regressions. Caching the first telemetry snapshot fails the live-settings regression. Source and built artifacts were restored before the final passing checks.
  • Clean nonincremental CLI typechecking, a build without prior incremental state, public type fixtures, strict settings/process fixture types and lint pass. An additional 84 doctor/metrics/telemetry checks pass (overlapping the suites above). The metrics reader applies recursive readonly mapping only after selecting telemetry, preserving mutable and immutable callers without expanding unrelated recursive configuration types.
  • The full runtime package build, including the downstream Kafka plugin, and cold Kafka typechecking pass. All 177 Kafka tests pass, including 11 real-daemon/local-chain cases and a regression for replacing the configuration projection between requests. Eight older CLI route fixture factories now provide the canonical store projection; an additional 217 tests pass across 11 route suites. Final lint passes.
  • The focused configuration/settings/subscription suite passes 59 tests. Explicit LLM clear and the legacy empty-key request remove the persisted secret, committed setting and runtime key; failed clear activation restores all three. Shutdown tests block both publication and activation, fence local and separate-process writers until the admitted update commits, then verify another process can write. Negative controls for early lease release and retaining a cleared secret fail the new regressions. The subscribe --save command test verifies daemon-owned always-on persistence without a second raw config write.
  • The recovery-fence follow-up passes 62 focused tests, cold CLI typechecking, CLI build/public checks, strict fixture typechecking and lint. Runtime-only, file-only and combined recovery failures reject already-queued and later updates; closing and reopening the owner permits a new startup. Removing the fence makes all three new regressions fail.
  • Local-agent concurrency checks preserve edits during blocked connect/refresh, explicit transport-field deletion and newer disconnects during successful/failed preparation. Removing the refresh patch, bypassing the disconnect guard or substituting copied connect state makes the corresponding regressions fail. After restoring sources, the full runtime package build, cold CLI types, strict fixtures and final lint pass; the final Hermes suite also verifies the actual failed-probe producer (100 tests, overlapping the CLI total).
  • The 1,851-file test inventory, SPARQL diff gate and merge compatibility with testnet-canary pass. The runtime dependency closure was built before the CLI checks. Remote CI and reviewer confirmation remain pending for the new commit.

Closes #1961.

Comment thread packages/agent/src/swm-expiry-cleanup.ts Outdated
Comment thread packages/agent/src/swm-expiry-cleanup.ts Outdated
Comment thread packages/agent/src/dkg-agent-lifecycle.ts Outdated
Comment thread packages/agent/test/swm-expiry-batching.test.ts Outdated
Comment thread packages/agent/src/swm-expiry-cleanup.ts
Comment thread packages/agent/src/swm-expiry-cleanup.ts Outdated
Comment thread packages/agent/src/swm-expiry-cleanup-worker.ts Outdated
Comment thread packages/agent/src/swm-expiry-cleanup.ts Outdated
Comment thread packages/agent/src/swm-expiry-cleanup-worker.ts Outdated
Comment thread packages/agent/src/swm-expiry-cleanup.ts Outdated
Comment thread packages/agent/test/workspace-ttl.test.ts Outdated
Comment thread packages/agent/src/dkg-agent-lifecycle.ts Outdated
Comment thread bench/swm-expiry-cleanup.mjs Outdated
Comment thread packages/agent/src/swm-expiry-cleanup.ts Outdated
Comment thread packages/agent/src/swm-expiry-cleanup-worker.ts Outdated
Comment thread packages/agent/src/swm-expiry-cleanup.ts Outdated
Comment thread packages/agent/src/swm-expiry-cleanup-worker.ts Outdated
Comment thread packages/agent/test/swm-expiry-cleanup-worker.test.ts
Comment thread packages/agent/src/shared-memory-graphs.ts
@branarakic

Copy link
Copy Markdown
Contributor Author

Updated this PR with current testnet-canary in 736104d after CI run34356103537 failed the selected-private durable catalog restart integration. The branch now includes the already-landed catalog replay and empty identity-response retry fixes (#2521/#2508). The exact failing restart case passes locally, as do 198 cleanup/responder/shutdown tests and all five additional TTL cases on this merged head. Dependency/agent builds, public/fixture types, lint, 1797-file inventory and SPARQL checks pass. Fresh CI is running.

Comment thread packages/agent/src/dkg-agent-base.ts Outdated
Comment thread packages/agent/test/swm-expiry-batching.test.ts Outdated
Comment thread packages/agent/src/swm-expiry-cleanup-worker.ts Outdated
Comment thread packages/agent/src/swm-expiry-cleanup.ts
Comment thread packages/agent/src/shared-memory-graphs.ts
Comment thread packages/agent/src/dkg-agent.ts
Comment thread packages/agent/src/swm-expiry-cleanup.ts Outdated
Comment thread packages/agent/src/swm-expiry-cleanup.ts Outdated

@otReviewAgent otReviewAgent left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Operational Notice: Review Agent could not complete this review.

Business logic reviewer failed: Selected model is at capacity. Please try a different model.

Comment thread packages/cli/src/daemon/routes/context.ts
Comment thread packages/cli/src/daemon/local-agent-connectors/hermes.ts

@otReviewAgent otReviewAgent left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Operational Notice: Review Agent could not complete this review.

Business logic reviewer failed: retry_exhausted

Review asked that the connector strategy own the full lifecycle or be
removed, because extracting only the connect phase left the other phases as
hard-coded connector checks in two already busy modules and created several
authoritative places for connector behaviour. Teardown and cancellation had
already moved behind the strategy; refresh had not.

`refreshLocalAgentIntegrationFromUi` was an if/else cascade over
'prime-agent', 'hermes', 'openclaw' and a bare fallback. Each branch body
moves 1:1 onto its connector as `createRefreshPlan`, the generic connector
keeps the empty-patch fallback, and the orchestrator collapses to resolving
the integration and dispatching through `localAgentConnectorFor`. Probe
behaviour, notices, metadata pinning and the OpenClaw bridge-health reset
are unchanged, and refresh still exposes no injection point, so the
arity guard that pins "Refresh never runs setup" keeps holding.

`createRefreshPlan` is required on the contract, so a connector that
implements only part of the lifecycle no longer compiles. A type test
asserts that, and a routes test drives connect, refresh, cancellation and
disconnect for every registered connector plus an unregistered id through
the one contract.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@otReviewAgent otReviewAgent left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Operational Notice: Review Agent could not complete this review.

Business logic reviewer failed: retry_exhausted

Comment thread packages/cli/src/daemon/local-agent-connectors/index.ts Outdated
Comment thread packages/cli/src/fs-utils.ts Outdated

@otReviewAgent otReviewAgent left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Operational Notice: Review Agent could not complete this review.

Business logic reviewer failed: You've hit your usage limit. Visit https://chatgpt.com/codex/settings/usage to purchase more credits or try again at Sep 20th, 2026 9:00 AM.

@branarakic

Copy link
Copy Markdown
Contributor Author

I reran the failed jobs from the previous CI attempt after reproducing the cleanup path locally. The affected SWM expiry suites pass 55/55 locally, including bounded batches, single-flight cleanup, fairness, publisher locking, external-store draining, and TTL behavior. The rerun is in progress; no code change was needed for the prior timing-sensitive failures.

@branarakic

Copy link
Copy Markdown
Contributor Author

The rerun of Tornado: agent [3/10] reproduced the same two timing-sensitive failures (context-graph-discovery storage-scan abort observation and the cross-node e2e-join phase timing); the other 511/513 tests passed. I ran both failing cases locally against the PR head: the full e2e-join.test.ts passed 7/7 and the storage-scan timeout case passed; the PR's focused expiry/TTL suites remain 55/55. The failing files are outside the expiry-focused change, so I am treating this as CI timing noise unless another rerun produces a deterministic failure.

@branarakic

Copy link
Copy Markdown
Contributor Author

CI follow-up pushed at a4cc6cf87 (with 80ed75dca). I applied the canary's deterministic RFC-64 join completion check and stabilized the bounded storage-scan timeout test so it overrides both supported store scan entry points. Local validation on the refreshed PR head: the focused storage-timeout case passes, e2e-join.test.ts passes 7/7, and git diff --check is clean. The prior CI failures were the scan-abort observation and the asynchronous RFC-64 join phase; both now have deterministic coverage. Fresh CI is running.

@otReviewAgent otReviewAgent left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Operational Notice: Review Agent could not complete this review.

Business logic reviewer failed: You've hit your usage limit. Visit https://chatgpt.com/codex/settings/usage to purchase more credits or try again at Sep 20th, 2026 9:00 AM.

@branarakic

Copy link
Copy Markdown
Contributor Author

CI follow-up: the red agent shard reproduced the storage-scan abort assertion on rerun. The test used a 1 ms budget and 20 ms delay, which can resolve in the wrong order under the sharded runner. It now uses a 50 ms budget with a 250 ms delayed scan, preserving the abort assertion while removing the timing race.

Validation: focused context-graph-discovery.test.ts timeout tests pass (2/2); git diff --check passed. Pushed as cefd957ec.

@otReviewAgent otReviewAgent left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Operational Notice: Review Agent could not complete this review.

Business logic reviewer failed: You've hit your usage limit. Visit https://chatgpt.com/codex/settings/usage to purchase more credits or try again at Sep 20th, 2026 9:00 AM.

@branarakic

Copy link
Copy Markdown
Contributor Author

Superseded by #2623, the current-canary focused implementation for #1961. #2623 retains bounded batching, single-flight cleanup, and no-progress protection without the unrelated configuration changes in this older branch.

@branarakic branarakic closed this Sep 15, 2026
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.

3 participants