Skip to content

feat: atomically supersede workspace decisions with provenance - #52

Open
outlier27-cell wants to merge 4 commits into
tt-a1i:mainfrom
outlier27-cell:feat/issue-16-decision-ledger
Open

outlier27-cell wants to merge 4 commits into
tt-a1i:mainfrom
outlier27-cell:feat/issue-16-decision-ledger

Conversation

@outlier27-cell

Copy link
Copy Markdown
Contributor

When an Orchestrator records a changed storage decision, the old decision can remain active alongside its replacement. Add team memory add "Use PostgreSQL" --kind decision --supersedes <old-id> to archive the old workspace decision and save the new entry plus its provenance in one SQLite transaction.

Refs #16. This implements the replacement-history portion using existing team memory; it does not implement Journal, automatic session rotation, a separate decision ledger, or proof of user confirmation. Orchestrator writes continue to be attributable Agent writes. Workers, cross-workspace references, inactive/non-decision targets and user-scoped replacements are rejected. Old text remains queryable; the existing Memory source display shows its excerpt and ID. Dream revert accepts and preserves the new source type.

The concrete problem is changing a previously saved project constraint while continuing a project. After this change the default active-memory retrieval returns the replacement, while history retains the old constraint. Without an atomic replacement, separate archive/add requests can leave conflicting active facts or lose the active constraint after a failed write.

Validation on Windows / Node 24.14.1:

  • pnpm check, pnpm build, TypeScript no-emit and diff whitespace checks passed (existing 5 Biome infos).
  • Memory HTTP/store/digest suite: 30 passed, including real HTTP/SQLite/PTY permissions and isolation.
  • Dream/store parser and replacement suite: 61 passed before adding the dedicated Dream-history regression.
  • Final targeted regression: 4 passed (HTTP replacement; DB insertion-failure rollback and database reopen; CLI argument handling; real Dream rewrite/revert history preservation). Other cases were deselected by the name filter.
  • No full-suite pass claimed; validation focused on the affected persistence/retrieval/Dream paths. The small Memory source display was typechecked/built; manual browser acceptance is not claimed.

Self-Review

Four independent reviewers covered architecture, failure boundaries, verification and protocol. The first round found an incompatible Dream revert whitelist and missing frontend source type; both fixed, with a dedicated revert regression. Rollback/reopen coverage and visible source history were also added. Rechecks of boundaries/testing/protocol scored B+/B+/A- with no blocking findings; architecture review identified the same resolved compatibility issues. This PR is scoped to decision replacement and leaves #16 open.

@tt-a1i tt-a1i left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Decision supersession is not preserved when reverting a Dream that predates the supersession.

Reproduction against head 8571598 with real SQLite:

  1. Create active decision A: "Use SQLite."
  2. Run a Dream that rewrites A; its revert data saves A's earlier active snapshot.
  3. Add decision B: "Use PostgreSQL." with supersedes_id=A.id. A becomes archived and B is active.
  4. Revert the Dream from step 2.

Observed: A becomes active again, while B remains active. Active-memory search returns both conflicting decisions, so the superseded constraint can be injected again. Expected: reverting an earlier Dream must not undo a later decision replacement.

The new archive at src/server/team-memory-store.ts:620-622 is overwritten by restorePriorEntry in src/server/team-memory-dream-reverter.ts:190-248 (status restoration at lines 205/228), called by the revert loop at line 291. That restoration has no guard for later supersession/mutations.

Please preserve the subsequent supersession when reverting, or reject a stale revert before making changes. Verify the Dream -> supersede -> revert ordering with actual SQLite, asserting that only B remains active/searchable and its provenance is intact. The new test currently exercises the opposite ordering (supersede -> Dream on B -> revert) and therefore misses this failure. Also assert that the Dream rewrite actually took effect before reverting; otherwise a no-op rewrite can pass the current test.

This is a P2 correctness blocker for this PR's promise that the replacement is the sole active decision. The workspace/user-scope isolation checks were reviewed and are not part of this finding.

@outlier27-cell

Copy link
Copy Markdown
Contributor Author

Addressed in e938a98.

The new real-SQLite regression follows Dream rewrite -> decision supersession -> Dream revert. It first asserts that the rewrite took effect, then verifies the old decision remains archived, only the replacement is returned by active-memory retrieval, and the replacement retains its memory provenance link. The regression failed against the previous head because the old decision was revived.

Revert now preserves entries that have a durable subsequent workspace supersession link, rather than overwriting that later replacement with the Dream's earlier snapshot.

Validation: the complete Dream runner test file passed (59/59), and the build passed. Biome reports only the five existing informational diagnostics in unrelated scripts; diff whitespace validation passed. One intermediate run hit the existing test-server fetch failed / bad port setup failure; the complete rerun passed.

Please re-review the supersession/revert fix. This does not claim a full-suite local pass.

@outlier27-cell

Copy link
Copy Markdown
Contributor Author

Follow-up validation in 1ffafb2: the original supersede -> Dream -> revert test now asserts that the Dream rewrite actually took effect before reverting. The new Dream -> supersede -> revert regression directly searches both PostgreSQL and SQLite: only B is searchable, while A stays archived and B retains its provenance. Both targeted cases passed; Biome passed for the two changed files.

Self-Review

  • Architecture: A-, no new blocking findings; the prior resurrection issue is resolved.
  • Bugs/boundaries: A-, no new blocking findings. Timestamp ordering relies on the existing wall clock; a system clock rollback is not covered.
  • Test quality: original rewrite and direct-search assertion gaps fixed; re-review A-, no remaining actionable findings.
  • Protocol/spec: A, no new blocking deviations.

The full Dream runner passed 59/59 before this assertion-only follow-up; both strengthened cases passed afterward. Local build passed. Latest-head CI remains the source of remote validation status.

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.

2 participants