Inject state DB, agent graph store#20689
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5f2fcd2754
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
dev/rasmus/agent_store_interface_plumbing |
| Arc::new(LocalAgentGraphStore::new(state_db)) | ||
| } | ||
|
|
||
| pub fn agent_graph_store_from_config( |
There was a problem hiding this comment.
same q about deciding store based on experimental_thread_store
There was a problem hiding this comment.
Yeah, we should have a separate flag here. Can we take in a follow-up to make the scope of this PR more manageable?
3216b5f to
73f6fcb
Compare
7a4638f to
0fe0569
Compare
0fe0569 to
134a4df
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 134a4df6f3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| chrono = { workspace = true, features = ["serde"] } | ||
| clap = { workspace = true, features = ["derive"] } | ||
| codex-analytics = { workspace = true } | ||
| codex-agent-graph-store = { workspace = true } |
There was a problem hiding this comment.
Regenerate the Bazel module lockfile
Adding codex-agent-graph-store to codex-core changes the Rust dependency graph, but MODULE.bazel.lock was not updated. just bazel-lock-check reports the lockfile is out of date, so Bazel CI will fail until just bazel-lock-update is run and committed.
Useful? React with 👍 / 👎.
Why
We want the agent graph store to be passed down the stack as a real dependency, the same way we already treat the thread store.
This will let us inject the agent graph store as a real dependency and support implementations other than the local SQLite-backed one. Right now most code instantiates a state DB and an agent graph store just-in-time. Ideally, we would not depend on the state DB directly but only read through the higher-level interfaces.
This change makes the dependency boundaries explicit and moves state DB initialization to process bootstrap instead of hiding it inside local store implementations.
What changed
ThreadManagernow requires aStateDbHandleand anAgentGraphStoreat construction time instead of treating them as optional internals.LocalThreadStoreLocalAgentGraphStoreapp-server,mcp-server,prompt_debug, and the thread-manager sample) now initialize the state DB up front and inject the resulting handle down the stack.app-servernow consistently uses its process-scoped state DB handle instead of reopening SQLite or trying to recover it from loaded threads.AgentGraphStoreinstead of reaching through local thread-store-specific state.Verification
cargo check -p codex-core -p codex-thread-store -p codex-app-server -p codex-mcp-server -p codex-thread-manager-sample --testscargo test -p codex-thread-storecargo test -p codex-core thread_manager_accepts_separate_agent_graph_store_and_thread_store -- --nocapturecargo test -p codex-app-server thread_archive_archives_spawned_descendants -- --nocapture