Replace session memory with holographic fact store - #2
Closed
ScriptedAlchemy wants to merge 16 commits into
Closed
Conversation
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e86ef80b7c
ℹ️ 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".
added 15 commits
June 7, 2026 03:23
This reverts commit fca9582.
Owner
Author
6 tasks
spa5k
pushed a commit
to spa5k/tracedecay
that referenced
this pull request
Jul 4, 2026
Flake ScriptedAlchemy#2: dashboard API tests (e.g. `automation_config_is_dashboard_controllable_and_persistent`, `automation_jobs_crud_and_manual_run_are_dashboard_controllable`) intermittently failed with `... failed: io: Peer disconnected` when the client raced the dashboard server's startup under parallel load. Fix (shared harness): the dashboard POST/PATCH/DELETE helpers now route through `common::http_call_with_retry`, so a request that fails at the connection level (peer disconnected / refused / reset) is retried with bounded backoff. Combined with `wait_for_dashboard` now polling for a real 2xx before the first request, all dashboard tests benefit. The local `git` helper also uses the resilient absolute-path lookup + ENOENT retry. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ScriptedAlchemy
added a commit
that referenced
this pull request
Aug 7, 2026
WIND-DOWN CHECKPOINT. Partial lane: the three files here are the only ones whose working-tree diff is exclusively mine over committed content. They DO NOT COMPILE STANDALONE — each is one half of a change whose other half lives in a file entangled with another lane's uncommitted hunks and untracked files, which this commit deliberately does not sweep. A successor must land the remaining hunks below in the same commit-or-revert unit as this one. WHAT LANDED HERE 1. src/application_surface/configuration_wire.rs New pub(super) fn configuration_invocation_payload(). Unwraps the serde `content` field of ConfigurationWireRequestV1 so the ApplicationRequest::Surface payload is the operation's own request body, not the tag/content envelope. No existing helper did this (searched: tracedecay_grep ConfigurationWireRequestV1 + full read of configuration_wire.rs; only result-side validate_configuration_outcome and payload_decodes exist). 2. src/mcp/server/requests.rs McpServer::execute_tool_dispatch now populates ToolCallRegistryOptions.daemon_user_profile_id from self.profile_identity -> profile_id().clone(). 3. src/mcp/tools/handlers/dispatch_groups.rs dispatch_session_workflow_tools passes options.daemon_user_profile_id into dashboard::handle_dashboard. Items 2 and 3 reference a struct field that is NOT yet committed (see REMAINING #2), so HEAD does not build until that lands. WHY (two product defects, one test drift) DEFECT A - configuration invocation payload was double-encoded. src/application_surface.rs:3286 (execute_application_surface) built the Surface payload as serde_json::to_value(request) over the whole ConfigurationSurfaceRequest = ConfigurationWireRequestV1, which is #[serde(rename_all="snake_case", tag="operation", content="request")] (crates/tracedecay-application/src/configuration.rs:197-198). That emits {"operation":"get","request":{"key":...}}. The receiving executor re-parses per-operation at src/daemon_client.rs:734 -> parse_application_surface_request (src/application_surface.rs:3091) -> from_value::<ConfigurationGetRequestV1>, which is deny_unknown_fields with only `key` (crates/tracedecay-application/src/configuration.rs: 45-49). Result: InvocationError::InvalidRequest -> invocation_contract_problem (src/application_surface.rs:4302) -> the observed application.surface.invalid_request / pre_admission. Same for Set/Unset/Batch, hence the dashboard PATCH 400 on configuration_batch. ConfigurationObservedState succeeded only because it is absent from the migrated_payload arm and never round-trips. src/daemon/invocation_executor.rs:288 carries the identical arm list. Present since introduction: git log -S migrated_payload -> fa8e1cf (2026-07-29); the enum was already tag/content there, so this vertical has never worked end to end. Plan: docs/plans/tracedecay-v2/20-configuration-control-plane.md:5-8 (control plane is the sole typed authority; CLI/MCP/HTTP/dashboard consume the same daemon-owned resolution) and :35-37 (ordinary valid mutations commit atomically with expected-revision CAS). DEFECT B - dashboard profile identity read from the wrong authority. patch_user_settings (crates/tracedecay-dashboard-api/src/settings_api.rs :337-341) fails closed with configuration_authority_unavailable_error() -> 503 (crates/tracedecay-api/src/configuration.rs:155-163) when runtime.user_profile_id() is None. src/mcp/tools/handlers/dashboard.rs :406 derived it from registered_project_session_db.binding().shard_id.profile_id. That store is None on the core daemon MCP server (src/mcp/server/construction.rs :279 daemon_owned_core -> registered_session_db: None), and the core server is published first and only later replaced by the full one (src/daemon/project_composition.rs:587 core construct/publish -> :907-975 full construct + replace_ready_if upgrade). Both servers always carry profile_identity (:590, :911), which exposes the canonical UserProfileId (src/daemon/profile_identity.rs:43-45). settings_api.rs :439-441 already states the intended contract: a profile write requires the exact profile identity bound by the daemon handshake. Plan: 20-configuration-control-plane.md:41-43 - scope bindings use existing typed identity; host profiles never create authority. TEST DRIFT C - configuration_set literals omit idempotency_key. ConfigurationSetRequestV1 requires it and is deny_unknown_fields (crates/tracedecay-application/src/configuration.rs:54-61); the landed contract is exercised by src/daemon/production_harness/ configuration_idempotency_journey_test.rs:397-406. Plan: 20-configuration-control-plane.md:18 - unreleased source-only request helpers and declarations change in place (fresh-store rule; the stale literal earns no compat). REMAINING WORK - NOT IN THIS COMMIT Four files hold my remaining hunks but also hold another lane's uncommitted hunks, and two of them depend on files that are UNTRACKED (src/mcp/tools/handlers/dashboard_git_correlation.rs, dashboard_graph.rs, dashboard_graph/queries.rs, crates/tracedecay-application/src/ work_run_control.rs). Committing them now would either sweep that lane's work or land a tree referencing nonexistent modules. Land these once those lanes have committed their untracked files: 1. src/application_surface.rs:112-116 - add configuration_invocation_payload to the `use configuration_wire::{..}` list. 2. src/application_surface.rs:3286 - replace ) => Some(serde_json::to_value(request) .map_err(|_| ApplicationSurfaceAdapterError::InvalidSurfaceRequest)?), with ) => Some(configuration_invocation_payload(request)?), (the ConfigurationGet|Set|Unset|Batch arm of migrated_payload). 3. src/mcp/tools/handlers/mod.rs:231-240 - add field to ToolCallRegistryOptions: pub(crate) daemon_user_profile_id: Option<tracedecay_domain::configuration::UserProfileId>, placed after dashboard_lcm_retrieval_service, with the doc comment explaining it must not come from the project-session store because that authority mounts behind the core project-open publication. 4. src/mcp/tools/handlers/mod.rs:289-292 - add `daemon_user_profile_id: None,` to impl Default. 5. src/mcp/tools/handlers/dashboard.rs:314-317 - add parameter `daemon_user_profile_id: Option<UserProfileId>,` to handle_dashboard, immediately after registered_project_session_db. UserProfileId is already imported at dashboard.rs:15-17. 6. src/mcp/tools/handlers/dashboard.rs:404-414 - delete the `let dashboard_profile_id = registered_project_session_db...` binding and pass daemon_user_profile_id to DashboardInvocationExecutorAdapter::new instead. 7. tests/runtime_surface_acceptance.rs:1068 - add "idempotency_key": "configuration.idempotency.mcp-set", to set_arguments. 8. tests/runtime_surface_acceptance.rs:1119 - add "idempotency_key": "configuration.idempotency.mcp-stale", to stale_arguments. The key must DIFFER from the mcp-set key so the stale write exercises a true revision CAS conflict rather than an idempotency-key conflict; both map to configuration.conflict at src/daemon/service/invocation/configuration.rs:848, so a shared key would make the test's stated intent unfalsifiable. VERIFICATION STATUS - NOT GREEN With all eight remaining hunks applied in the working tree, the run was: running 3 tests mcp_configuration_write_persists_and_rejects_stale_cas ... FAILED (now panics at tests/runtime_surface_acceptance.rs:1137, was :734) dashboard_user_settings_replay_through_application_restart ... FAILED (now :993, was :930) dashboard_project_settings_commit_through_the_daemon_control_plane ... FAILED (now :817, was :812) test result: FAILED. 0 passed; 3 failed; 10 filtered out; in 10.75s All three moved off the pre-admission refusals (400 / 503 / invalid_request) onto later assert_eq! value comparisons, so the control plane now reaches the daemon and commits. The left/right diffs for those three asserts were never captured: the run that would have shown them was killed by the cargo-suspension protocol change, and an earlier run was filtered in a way that stripped the indented detail lines. NEXT STEPS 1. Land the eight hunks above once the entangled lanes commit. 2. Queue (per /tmp/codex-mine/buildq/PROTOCOL.md): cargo test --test runtime_surface_acceptance --features test-transport -- --test-threads=1 --nocapture --exact mcp_configuration_write_persists_and_rejects_stale_cas dashboard_project_settings_commit_through_the_daemon_control_plane dashboard_user_settings_replay_through_application_restart 3. Read the three assert_eq! left/right diffs and fix; expected to be small, since every pre-admission refusal is already resolved. 4. Separately: tests/core_cli_suite/tool_daemon_test.rs:1159-1169 and :1203-1213 carry the same stale configuration_set shape as drift C and are almost certainly failing for the same reason. Out of this lane's scope; not investigated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Aug 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
amari-holographicFHRR vectors.tokensave_fact_store,tokensave_fact_feedback, andtokensave_memory_status, plus v11 migration/backfill from legacy memory tables.Verification
cargo testcargo clippy --workspace --all-targetscargo fmt --all -- --checkgit diff --checkact pull_request -W .github/workflows/ci.yml ...attempted; format passed aftercargo fmt, Linux test exposed an act/Docker-sensitive git config test which was fixed and then verified locally.