Fix TUI sync and receipt-backed activation reporting - #68
Merged
Merged
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (9)
📝 WalkthroughWalkthroughChangesDOX sync and activation reporting
Estimated code review effort: 4 (Complex) | ~60 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant TUI
participant sync_dox
participant SQLiteMemoryStore
TUI->>sync_dox: request dry-run preview
sync_dox-->>TUI: return DoxSyncReport
TUI->>SQLiteMemoryStore: apply reviewed report events
SQLiteMemoryStore-->>TUI: return persistence result
sequenceDiagram
participant integrations_activate
participant verify_activation_receipts
participant IntegrationLifecycleActionReport
integrations_activate->>verify_activation_receipts: verify matching receipt
verify_activation_receipts-->>integrations_activate: return receipt state or no proof
integrations_activate->>IntegrationLifecycleActionReport: set state and next step
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PR Summary by QodoFix TUI sync and receipt-backed activation reporting
AI Description
Diagram
High-Level Assessment
Files changed (10)
|
Code Review by Qodo
1. Custom store sync misses project rules
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+801
to
+803
| let lines = wrap_preview_text( | ||
| &format!("{}\n\n{detail}", pending.summary), | ||
| regions[0].width, |
There was a problem hiding this comment.
2. A crafted guide controls the terminal 🐞 Bug ⛨ Security
wrap_preview_text copies control characters from pending.summary and the selected DOX event into raw terminal lines without sanitization. When an AGENTS.md summary or source path contains escape, bell, or similar control characters, opening /sync emits them during confirmation rendering and can alter the display or trigger terminal control behavior.
Agent Prompt
## Issue description
The DOX confirmation preview renders source-derived text without stripping terminal control characters, allowing crafted guidance or paths to affect the user's terminal.
## Fix Focus Areas
- crates/tree-ring-memory-cli/src/tui/render.rs[801-813]
- crates/tree-ring-memory-cli/src/tui/render.rs[864-880]
## Recommended Fix
Sanitize every explicit preview row before constructing its `Line`, removing or visibly replacing control characters while preserving the intended line boundaries. Add a rendering regression test containing escape and bell characters in both a candidate summary and source reference, and verify those bytes never reach the rendered buffer.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+131
to
+133
| sensitive_count, | ||
| preview.skipped_secret_count, | ||
| preview.warnings.len(), |
There was a problem hiding this comment.
3. Sync understates discarded guidance 🐞 Bug ≡ Correctness
PendingAction::sync_dox labels skipped_secret_count as a number of secret sections even though the collector increments it once for an entirely discarded file. When any section trips the secret guard, events_from_agents_file abandons every otherwise eligible candidate from that AGENTS.md, so the preview and empty-state status describe a narrower omission than occurred.
Agent Prompt
## Issue description
The TUI describes the file-level `skipped_secret_count` as skipped sections, concealing that all guidance from each affected `AGENTS.md` was omitted.
## Fix Focus Areas
- crates/tree-ring-memory-cli/src/tui/actions.rs[124-133]
- crates/tree-ring-memory-cli/src/tui/app.rs[576-583]
- README.md[677-683]
## Recommended Fix
Change the confirmation, empty-state, and documentation wording to identify affected `AGENTS.md` files rather than sections, preferably stating that files containing detected secret content are skipped entirely. Add a test with one ordinary and one secret section in the same file to lock the displayed semantics to the collector's behavior.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+557
to
+558
| let source_root = std::path::absolute(project_root_for_memory_root(&self.root)) | ||
| .map_err(|error| error.to_string())?; |
There was a problem hiding this comment.
1. Custom store sync misses project rules 🐞 Bug ≡ Correctness
preview_dox_sync derives source_root from project_root_for_memory_root(&self.root), whose non-.tree-ring branch returns the memory-store root rather than the project from which the TUI was launched. Thus a project using a custom --root scans that store for AGENTS.md—including the store's initialization guidance when present—instead of the project's instructions, yielding an empty preview or importing unrelated rules.
Agent Prompt
### Issue description
`/sync` treats every custom memory-store root as the project source root. The global `--root` option is documented as a memory-store location, so a TUI launched from a project with `--root /some/store` must discover that project's `AGENTS.md`, not `/some/store/AGENTS.md`.
### Fix Focus Areas
- crates/tree-ring-memory-cli/src/tui/app.rs[555-572]
- crates/tree-ring-memory-cli/src/tui/app.rs[717-725]
### Recommended Fix
Carry or derive the TUI launch project directory independently from the memory-store root, and use that directory as the DOX source root. Retain the existing `.tree-ring` compatibility behavior, but do not infer that arbitrary custom store paths are project directories; add coverage for launching from a project with a custom `--root`.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+567
to
+572
| let preview = sync_dox( | ||
| None, | ||
| DoxSyncActionRequest { | ||
| source_root, | ||
| project: Some(project.clone()), | ||
| dry_run: true, |
There was a problem hiding this comment.
4. Shared stores overwrite project rules 🐞 Bug ≡ Correctness
The new TUI flow persists DOX events whose IDs are derived solely from a relative source reference, while the supplied project value is only the source root's basename and is not part of that ID. When two projects with the same relative AGENTS.md section sync into one configured store, the batch upsert replaces the first project's summary, source, and project fields with the second project's record.
Agent Prompt
### Issue description
The TUI creates source-linked DOX candidates for a configured store, but core DOX IDs hash only the source path relative to the scanned project. Distinct projects with the same relative `AGENTS.md` paths and headings therefore produce identical primary keys when they share a custom memory store, causing one project's sync to overwrite the other's records.
### Fix Focus Areas
- crates/tree-ring-memory-cli/src/tui/app.rs[559-575]
- crates/tree-ring-memory-core/src/dox.rs[171-189]
- crates/tree-ring-memory-core/src/dox.rs[321-347]
### Recommended Fix
Include a stable, unambiguous project identity in the DOX stable-ID input, such as a canonical-root fingerprint plus the relative source reference. Preserve stable IDs for repeated syncs of the same project, and add a shared-store test proving identical relative source paths from two projects remain separate records.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
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.
The TUI accepted
/syncand displayed a confirmation, but confirming only changed a status message: no source adapter ran and an empty dashboard stayed empty./syncnow previews source-linked DOX candidates from the current project and saves exactly that preview after confirmation, using the shared CLI action and existing coordinated-write policy. Source/store paths, candidate review, cancellation, failures, and empty or filtered states are visible, including in compact terminals. Repeated imports retain stable source identities.Successful activation also checks existing valid recall receipts and gives a concrete session/status next step when proof is missing, instead of asking users to apply an already-applied bridge plan. Failed/review/trust states retain their existing behavior.
Validation: all 570 workspace tests passed, formatting/package validation and shell syntax checks passed, and independent review found no remaining issues. Native terminal checks verified preview, cancel, dashboard refresh after save, and repeat import without duplicates. Compact rendering regressions cover long preview scrolling at 60x18 and actionable errors/command input at 60x14.
The affected project was also checked with a real preview and cancel; its protected files, policy, and empty store remain unchanged. Populating its shared guidance still requires the owner-configured coordinator capability.
High-level PR Summary
This PR fixes the TUI's
/synccommand to properly preview and save DOX summaries from source files, replacing a broken placeholder implementation. The command now displays source paths, candidate summaries, and project context before confirmation, then saves using the existing CLI action with coordinated-write policy. The PR also improves activation reporting by checking for valid recall receipts and providing concrete next steps (like "open a new session") instead of asking users to reapply already-completed bridge plans. Includes comprehensive test coverage for preview scrolling, compact terminal rendering, coordinated-mode authorization, cancellation, and repeat imports without duplicates.⏱️ Estimated Review Time: 30-90 minutes
💡 Review Order Suggestion
docs/protocol/harness-activation.mdcrates/tree-ring-memory-cli/src/actions/adapters.rscrates/tree-ring-memory-cli/src/actions/integrations.rscrates/tree-ring-memory-cli/src/tui/actions.rscrates/tree-ring-memory-cli/src/tui/app.rscrates/tree-ring-memory-cli/src/tui/render.rscrates/tree-ring-memory-cli/tests/activation_outcome_acceptance.rsREADME.mdCargo.tomlCargo.lockSummary by CodeRabbit
New Features
/syncpreview in the TUI, including candidate selection, sensitive-content filtering, scrolling, and confirmation before saving.Bug Fixes
Documentation