Skip to content

feat: per-agent persistent memory (#62) - #63

Merged
nutt-adam merged 12 commits into
mainfrom
feat/issue-62-agent-memory
Mar 18, 2026
Merged

feat: per-agent persistent memory (#62)#63
nutt-adam merged 12 commits into
mainfrom
feat/issue-62-agent-memory

Conversation

@nutt-adam

@nutt-adam nutt-adam commented Mar 17, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add memory: Option<String> field to AgentConfig for pointing to a persistent memory file
  • On tt up, claude-code agents get memory contents appended to CLAUDE.md in their worktree under # Agent Memory
  • Non-Claude runtimes (codex, aider, openclaw) get memory contents prepended to their prompt
  • ensure_tutti_dir now creates .tutti/memory/ directory
  • Memory paths validated: must be relative, no .. traversal
  • Default template updated with commented memory example

Test plan

  • AgentConfig parses with memory field present
  • AgentConfig parses without memory field (backward compat, defaults to None)
  • Validation rejects absolute memory paths
  • Validation rejects path traversal (..)
  • ensure_tutti_dir creates .tutti/memory/ directory
  • Memory file contents appended to existing CLAUDE.md
  • CLAUDE.md created with memory when no existing CLAUDE.md
  • No-op when memory config is None
  • No-op when memory file doesn't exist (graceful)
  • prepend_memory_to_prompt adds context for non-Claude runtimes
  • prepend_memory_to_prompt is a no-op for claude-code (handled via CLAUDE.md)
  • All 261 tests pass, clippy clean, fmt clean

Closes #62

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Per-agent persistent memory support with runtime-aware injection and prompt augmentation; default memory enabled for pr-monitor and memory storage created.
  • Improvements

    • Memory path validation and safety checks; config validation errors for invalid memory paths.
    • Atomic multi-line prompt pasting for terminal runtimes.
    • Non-blocking runtime health heuristics with non-fatal status/auth warnings.
  • Tests

    • Expanded coverage for memory handling, injection idempotence, prompt composition, worktree and runtime flows.
  • Documentation

    • Added dogfooding guidance.

Add memory field to AgentConfig that points to a markdown file. On tt up,
claude-code agents get memory appended to CLAUDE.md in their worktree;
other runtimes get memory prepended to their prompt. The memory directory
(.tutti/memory/) is created by ensure_tutti_dir. Memory paths are validated
to be relative and non-traversing.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Mar 17, 2026

Copy link
Copy Markdown

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds an optional per-agent persistent memory field (AgentConfig.memory: Option<String>) with validation and default; creates .tutti/state/memory; implements CLI memory handling (file injection for claude-code, prompt prepend for others); updates tests and helpers; several tmux/session and worktree tests refactored/added.

Changes

Cohort / File(s) Summary
Config (struct, defaults, validation)
src/config/mod.rs, src/config/defaults.rs
Add public memory: Option<String> to AgentConfig (serde default + doc); add default pr-monitor memory path; validate memory paths (non-empty, relative, no .. components); add/adjust parsing & validation tests.
CLI: up (memory injection & prompt composition)
src/cli/up.rs
Add inject_agent_memory, prepend_memory_to_prompt, validate_no_symlink, MEMORY_SECTION markers; compute effective_prompt and use it for non-claude-code runtimes; tests for injection, idempotence, creation, noop, and prompt augmentation.
State dir creation
src/state/mod.rs
Create .tutti/state/memory in ensure_tutti_dir(); update tests to expect it.
Agent config usage in tests & fixtures
src/automation/mod.rs, src/budget/mod.rs, src/cli/doctor.rs, src/cli/handoff.rs, src/cli/watch.rs
Update test fixtures to include memory: None in AgentConfig initializations to reflect the new field.
Session tmux behavior
src/session/tmux.rs
Switch to buffer-based paste via send_text_via_tmux_buffer, add send_enter helper, adjust bracketed paste handling and error messages.
Runtime/send heuristics
src/cli/send.rs, src/cli/up.rs
Inspect runtime adapter/pane output after auto-up and emit non-fatal yellow warnings for Unknown status or auth failures; propagate effective_prompt into launch path for non-claude runtimes.
Worktree tests & helpers
src/worktree/mod.rs
Add comprehensive worktree test suite and TestRepo helper exercising worktree lifecycle; tests only, no public API changes.
Integration tests / tmux helper
tests/send_auto_up_integration.rs
Add tmux_is_usable() helper to check tmux availability/operability and replace inline checks in integration tests.
Docs
CLAUDE.md
Add "Dogfooding" subsection describing dogfooding workflow.
Other tests updated
src/cli/..., src/...
Various tests updated to initialize new memory field and cover memory-related behaviors across modules.

Sequence Diagram(s)

sequenceDiagram
    participant CLI as CLI (tt up)
    participant Config as Config Loader
    participant FS as File System
    participant Runtime as Agent Runtime

    CLI->>Config: load agent configs (may include memory path)
    Config-->>CLI: return AgentConfig

    alt agent has memory path
        CLI->>FS: validate path (relative, no "..", not symlink)
        FS-->>CLI: validation result
        CLI->>FS: read memory file
        FS-->>CLI: memory contents

        alt runtime == claude-code
            CLI->>FS: inject MEMORY_SECTION into CLAUDE.md (idempotent)
            FS-->>CLI: CLAUDE.md updated
            CLI-->>Runtime: launch runtime (uses injected CLAUDE.md)
        else other runtime
            CLI->>CLI: prepend memory to prompt → effective_prompt
            CLI-->>Runtime: launch with effective_prompt
        end
    else no memory
        CLI-->>Runtime: launch with original prompt
    end

    Runtime-->>CLI: runtime started / status (warnings if Unknown/auth issues)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰 I nibble paths and tuck a tiny note,
Into .tutti/state/memory I wrote.
CLAUDE reads the page, others feel the nudge,
Prompts hum softer from the crumbs I budge.
A rabbit’s little log — steady, snug, and woke.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description covers the main changes, implementation details, and includes a comprehensive test plan with checked items. However, the Versioning section is incomplete: version selected is blank, and version bump checkboxes are not marked. Complete the Versioning section by selecting a SemVer version (likely MINOR for new capability), updating Cargo.toml, adding CHANGELOG.md entry, and checking off completed items.
Out of Scope Changes check ❓ Inconclusive Most changes are directly related to the memory feature. However, tmux send_text refactoring (src/session/tmux.rs) and worktree test suite expansion (src/worktree/mod.rs) appear tangential to the core memory feature objective and may represent scope creep. Clarify whether the tmux buffer-paste refactoring and worktree test suite additions are essential to the memory feature or should be separated into distinct PRs to maintain focused scope.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat: per-agent persistent memory (#62)' clearly and specifically describes the main feature addition: support for per-agent memory persistence. It directly aligns with the core change throughout the codebase.
Linked Issues check ✅ Passed The PR implements per-agent persistent memory (parsing, validation, injection for claude-code and non-Claude runtimes, directory creation, and test coverage) which directly addresses Phase 2 acceptance criteria: per-agent memory support is implemented and memory is integrated into startup workflows. Some Phase 2 scope items (workspace-shared memory, CLI ergonomics, summarization hooks, mailbox/handoff semantics) remain as future work, but the PR fulfills the immediate objective of enabling per-agent memory injection.
Docstring Coverage ✅ Passed Docstring coverage is 98.36% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/issue-62-agent-memory
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot]
coderabbitai Bot previously requested changes Mar 17, 2026

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/cli/up.rs`:
- Around line 173-178: run_all() currently bypasses the memory injection and
prompt-prepend logic present in run(); update run_all() to call
inject_agent_memory(project_root, &working_dir, agent, &runtime_name)? before
launching each agent and use prepend_memory_to_prompt(project_root, agent,
&runtime_name, agent.prompt.as_deref())? to build the effective_prompt (same as
run()), replacing direct uses of agent.prompt when spawning agents; ensure you
reference inject_agent_memory and prepend_memory_to_prompt and handle their
Result returns the same way run() does.
- Around line 447-452: The inject_agent_memory function currently always appends
to working_dir/CLAUDE.md which can mutate the workspace root and duplicate
entries for reused worktrees; update inject_agent_memory to first detect whether
working_dir is a real worktree path (e.g., compare to project_root or check for
a worktree marker) and only proceed when it’s an actual worktree; write to an
agent-scoped file (include agent.id or AgentConfig name instead of unconditional
CLAUDE.md in the project root) and implement logic to find and replace or update
a managed memory section (bounded markers) inside that file instead of always
appending to avoid duplicate memory on relaunch (apply the same fix to the other
append block referenced near the second occurrence).

In `@src/config/mod.rs`:
- Around line 571-594: The current check uses trimmed.contains("..") which
falsely rejects filenames like "notes..md"; instead, parse the path and reject
only when it contains a ParentDir component: in the loop over self.agents
(checking agent.memory and the trimmed variable) replace the contains("..")
check with inspecting Path::new(trimmed).components() and testing for
Component::ParentDir (use std::path::Component), and keep returning
TuttiError::ConfigValidation with the same message when such a component is
found.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: ace596fe-8984-4585-9e7b-4f87eef99513

📥 Commits

Reviewing files that changed from the base of the PR and between 44f1cd5 and 1b22c0d.

📒 Files selected for processing (9)
  • src/automation/mod.rs
  • src/budget/mod.rs
  • src/cli/doctor.rs
  • src/cli/handoff.rs
  • src/cli/up.rs
  • src/cli/watch.rs
  • src/config/defaults.rs
  • src/config/mod.rs
  • src/state/mod.rs

Comment thread src/cli/up.rs Outdated
Comment thread src/cli/up.rs Outdated
Comment thread src/config/mod.rs
send_text previously split text into lines and sent each with a separate
Enter keypress. This caused multi-line prompts to be interpreted as
individual shell commands when the target was a non-TUI runtime (e.g.,
codex that had exited to zsh).

Fix: paste the entire text as a single tmux buffer with bracketed paste
mode (-p flag), then send one Enter. Also add a pre-send status check
in tt send to warn when the runtime appears to have exited.

Discovered during dogfood run of issue #62 orchestration.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
coderabbitai[bot]
coderabbitai Bot previously requested changes Mar 17, 2026

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/cli/send.rs`:
- Around line 76-88: The pre-send health check currently only warns on
AgentStatus::Unknown; update the logic around runtime::get_adapter /
adapter.detect_status to also treat AgentStatus::AuthFailed(_) as a
non-fatal-but-actionable warning when not launched_via_auto_up: match
AuthFailed(err) alongside Unknown and eprintln a clear, user-facing message
referencing agent_name and the auth error (err) with guidance (e.g.,
re-authenticate or check runtime credentials), so users know the runtime is
unusable and how to fix it.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: aea7eac2-2018-4f84-8379-ef6b29573d35

📥 Commits

Reviewing files that changed from the base of the PR and between 1b22c0d and dc4cc8b.

📒 Files selected for processing (3)
  • CLAUDE.md
  • src/cli/send.rs
  • src/session/tmux.rs
✅ Files skipped from review due to trivial changes (1)
  • CLAUDE.md

Comment thread src/cli/send.rs
1. Wire memory injection through run_all() so tt up --all injects memory
2. Make inject_agent_memory idempotent with bounded markers — no
   duplicate memory sections on relaunch of reused worktrees
3. Skip CLAUDE.md mutation when working_dir is the project root
   (no worktree) to avoid modifying the workspace's own file
4. Use Component::ParentDir for path traversal detection instead of
   contains("..") which falsely rejected filenames like notes..md
5. Handle AuthFailed status in pre-send health check (warn user)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
coderabbitai[bot]
coderabbitai Bot previously requested changes Mar 17, 2026

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/cli/up.rs`:
- Around line 471-475: When prepare_agent_working_dir detects working_dir ==
project_root and currently returns early (skipping CLAUDE.md injection), change
that branch so Claude agents don't lose memory: for agents whose engine/id
equals "claude-code" call prepend_memory_to_prompt(...) to add the memory into
the prompt (instead of skipping), or alternatively emit a clear warning/return
Err if injection cannot occur; update the same logic at the other occurrence
(the block around lines 530-533) so both code paths treat "claude-code"
consistently rather than silently returning Ok().
- Around line 478-513: Reject symlinked source and destination paths by
canonicalizing and validating them before any file IO in inject_agent_memory and
prepend_memory_to_prompt: for the source (resolved =
project_root.join(memory_path)), use std::fs::canonicalize and ensure the
canonical path starts with the canonical project_root (or otherwise refuse) and
also reject if the original metadata shows a symlink (symlink_metadata +
file_type().is_symlink()); for the destination (claude_md_path =
working.join("CLAUDE.md")), refuse if the destination itself is a symlink or if
canonicalizing the destination would place it outside the canonical working dir
(and similarly check parent path canonicalization), returning an error instead
of reading or writing when validation fails. Ensure these checks reference
memory_path, project_root, resolved, claude_md_path, inject_agent_memory, and
prepend_memory_to_prompt.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 08d183e3-bd1e-4499-a871-823ba456ca5d

📥 Commits

Reviewing files that changed from the base of the PR and between dc4cc8b and ea1e8ae.

📒 Files selected for processing (3)
  • src/cli/send.rs
  • src/cli/up.rs
  • src/config/mod.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/cli/send.rs

Comment thread src/cli/up.rs Outdated
Comment thread src/cli/up.rs
nutt-adam and others added 4 commits March 18, 2026 07:14
…le Enter

Bracketed paste mode (-p) caused codex TUI to consume the first Enter
as "close paste bracket" rather than "submit prompt", leaving the text
in the input field without processing. Fix: only use -p for multi-line
text (where it's needed to prevent line-by-line shell interpretation),
and send a second Enter after bracketed pastes to trigger submission.

Single-line commands (unset, export, shell commands) skip bracketed
paste entirely to avoid the double-Enter issue.

Validated via tt send to codex with multi-line prompt — codex
immediately started working without needing manual Enter.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1. Claude agents no longer lose memory when worktrees are disabled:
   inject_agent_memory returns bool indicating whether file injection
   succeeded; prepend_memory_to_prompt falls back to prompt injection
   for claude-code when file_injected=false

2. Reject symlinked memory source and CLAUDE.md destination paths:
   validate_no_symlink checks symlink_metadata and canonicalize to
   prevent arbitrary file reads (symlinked source) and writes
   (symlinked CLAUDE.md) outside the project root

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
coderabbitai[bot]
coderabbitai Bot previously requested changes Mar 17, 2026

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/cli/up.rs`:
- Around line 478-514: The code currently reads and writes files using resolved
(from project_root.join(memory_path)) and claude_md_path
(working.join("CLAUDE.md")) without validating symlinks or ensuring they point
inside the intended directories; update the logic to canonicalize and validate
both paths (use std::fs::canonicalize or equivalent) before any read/write:
canonicalize resolved and ensure it has project_root.canonicalize() as a prefix,
and canonicalize claude_md_path and ensure it has working.canonicalize() as a
prefix (reject or error if canonicalization fails or the paths escape their base
directories), then proceed to read memory_contents and write combined only after
these checks, keeping the MEMORY_SECTION_START / MEMORY_SECTION_END handling
unchanged.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 44d59287-0956-4dc9-afec-6a60db89f737

📥 Commits

Reviewing files that changed from the base of the PR and between ea1e8ae and eee7cdf.

📒 Files selected for processing (7)
  • src/cli/up.rs
  • src/config/defaults.rs
  • src/config/mod.rs
  • src/session/tmux.rs
  • src/state/mod.rs
  • src/worktree/mod.rs
  • tests/send_auto_up_integration.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/state/mod.rs

Comment thread src/cli/up.rs
@nutt-adam

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

All 7 findings have been addressed across commits ea1e8ae, eee7cdf, and 6301cef:

  1. run_all() now wires memory injection
  2. Idempotent managed sections with bounded markers
  3. Skips project root CLAUDE.md mutation
  4. Component::ParentDir for traversal detection
  5. AuthFailed handled in pre-send check
  6. Claude-code fallback to prompt prepending when file injection skipped
  7. Symlink validation via validate_no_symlink() on source and destination

@coderabbitai

coderabbitai Bot commented Mar 17, 2026

Copy link
Copy Markdown

@nutt-adam Thanks for the detailed summary of all 7 fixes! I'll kick off a fresh review now covering the latest commits.


🧠 Learnings used
Learnt from: CR
Repo: nutthouse/tutti PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T19:46:08.576Z
Learning: Applies to src/state/**/*.rs : Organize code in `src/state/` for .tutti/ directory and agent state persistence
✅ Actions performed

Full review triggered.

@nutt-adam
nutt-adam dismissed stale reviews from coderabbitai[bot], coderabbitai[bot], coderabbitai[bot], and coderabbitai[bot] March 17, 2026 20:45

All findings addressed.

@nutt-adam

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Mar 17, 2026

Copy link
Copy Markdown
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 2

♻️ Duplicate comments (1)
src/cli/up.rs (1)

522-525: ⚠️ Potential issue | 🔴 Critical

New CLAUDE.md writes still bypass the symlink guard.

Line 523 only validates the destination when CLAUDE.md already exists. On the first launch, Line 551 creates the file without ever checking the path, so a symlinked working_dir or parent directory can still redirect this write outside the intended agent tree. Please validate the destination path before fs::write, not just the already-existing file.

Also applies to: 551-551

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/cli/up.rs` around lines 522 - 525, The symlink guard only runs when
claude_md_path.exists(); to fix, call validate_no_symlink(&claude_md_path,
"CLAUDE.md", working) (or a helper that verifies all path components) before any
fs::write that creates CLAUDE.md so the destination is validated on first launch
as well; update the code paths around claude_md_path and the fs::write that
creates the file to invoke validate_no_symlink (or a new validate_path_parents
helper) prior to writing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/cli/up.rs`:
- Around line 457-468: Update the ConfigValidation error messages in the
validation branch that checks symlinks and canonical path ownership (the block
using path, allowed_root, canonical, root and returning
TuttiError::ConfigValidation) to include the allowed location and an actionable
remediation hint; for the symlink case (where path.is_symlink()) include
guidance to remove or replace the symlink with a real file under the
workspace/worktree and show the offending path, and for the canonicalization
case include the canonicalized allowed_root value and a short instruction to
move the file into that worktree/allowed location or update configuration so the
path is under the shown allowed_root.
- Around line 1492-1499: Don’t silently drop memory by using unwrap_or_else;
instead propagate the error from prepend_memory_to_prompt so failures are
surfaced (and handled by run()) — replace the unwrap_or_else call that assigns
effective_prompt with a fallible handling (use the ? operator or map_err to add
context) so prepend_memory_to_prompt(...) returns an Err up the stack rather
than quietly returning agent.prompt.clone(); reference prepend_memory_to_prompt,
effective_prompt and run() when making the change.

---

Duplicate comments:
In `@src/cli/up.rs`:
- Around line 522-525: The symlink guard only runs when claude_md_path.exists();
to fix, call validate_no_symlink(&claude_md_path, "CLAUDE.md", working) (or a
helper that verifies all path components) before any fs::write that creates
CLAUDE.md so the destination is validated on first launch as well; update the
code paths around claude_md_path and the fs::write that creates the file to
invoke validate_no_symlink (or a new validate_path_parents helper) prior to
writing.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 40ac13f0-252e-4160-8eb8-225483d7f2c8

📥 Commits

Reviewing files that changed from the base of the PR and between eee7cdf and 6301cef.

📒 Files selected for processing (1)
  • src/cli/up.rs

Comment thread src/cli/up.rs Outdated
Comment thread src/cli/up.rs Outdated
nutt-adam and others added 2 commits March 18, 2026 09:51
… run_all

- Include allowed directory and remediation hints in symlink/path validation
  errors per project convention ("User-facing errors should include actionable
  guidance")
- Replace silent unwrap_or_else in run_all() with explicit error handling that
  warns and skips the agent, matching run() behavior

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…efore first write

- run_all() now propagates prepend_memory_to_prompt errors with ? (matching
  run() behavior) instead of silently continuing
- Validate CLAUDE.md destination and its parent directory for symlinks before
  any write, not just when the file already exists

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@nutt-adam

Copy link
Copy Markdown
Contributor Author

Addressed all CodeRabbit findings from the latest review:

2e54777:

  • run_all() now propagates prepend_memory_to_prompt errors with ? (matching run() behavior) instead of silently dropping memory
  • validate_no_symlink is now called on the CLAUDE.md path and its parent directory before any write, not just when the file already exists — this closes the first-launch symlink bypass

5723b82 (prior commit):

  • Validation error messages now include the allowed directory and actionable remediation hints

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Mar 18, 2026

Copy link
Copy Markdown

@nutt-adam Thanks for the thorough follow-up! The fixes look well-considered — particularly closing the first-launch symlink bypass by validating the CLAUDE.md parent directory before any write (not just when the file already exists), and ensuring run_all() propagates prepend_memory_to_prompt errors consistently with run(). Kicking off a fresh review now.

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/cli/up.rs`:
- Around line 539-543: The current removal logic for the managed section uses
existing.find(MEMORY_SECTION_END) which searches from the file start and can
match an unrelated end marker; change it to search for MEMORY_SECTION_END
relative to the found start (e.g. search within &existing[start..] and compute
the absolute end index by adding the start offset) so that the slice for
before/after uses the correct end position; update the code around existing,
MEMORY_SECTION_START, MEMORY_SECTION_END and the calculation of
before/after/base to use this relative find and handle the case where the end
marker is not found.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 31a1c935-ada9-424a-b8d4-ca6dc3972239

📥 Commits

Reviewing files that changed from the base of the PR and between eee7cdf and 2e54777.

📒 Files selected for processing (1)
  • src/cli/up.rs

Comment thread src/cli/up.rs
nutt-adam and others added 2 commits March 18, 2026 13:57
Avoids matching an unrelated end marker that appears before the start
marker in CLAUDE.md.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1. validate_no_symlink: check parent directory when destination path
   does not yet exist, preventing symlink-based directory traversal on
   first launch before fs::write creates the file.

2. inject_agent_memory: search for MEMORY_SECTION_END relative to the
   found start position (existing[start..]) instead of skipping past
   the start marker, ensuring correct absolute offset computation.

3. run_all: propagate inject_agent_memory errors with ? instead of
   silently dropping them via unwrap_or_else and a warning log.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@nutt-adam

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Mar 18, 2026

Copy link
Copy Markdown
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@nutt-adam
nutt-adam merged commit 715c39c into main Mar 18, 2026
10 checks passed
@nutt-adam
nutt-adam deleted the feat/issue-62-agent-memory branch March 18, 2026 18:42
nutt-adam added a commit that referenced this pull request Mar 19, 2026
…anges

Cover startup grace window (#67), persistent memory (#62/#63), merge gate
enforcement (#59), permissions suggest (#53), orchestration state machine
(#54/#55), and all fixes shipped since 0.2.0.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
nutt-adam added a commit that referenced this pull request Mar 19, 2026
…choir runs (#71)

* feat(health): add startup grace window to wait_for_agent_idle (#67)

Prevent fresh prompt steps from falsely completing before the agent
has consumed the prompt. The startup grace period (default 30s) gates
completion detection until real working activity is observed.

Key changes:
- wait_for_agent_idle accepts a startup_grace Duration parameter
- AgentStatus::Working counts as activity even without pane hash change,
  requiring 2+ consecutive polls to avoid flicker false positives
- First pane capture no longer counts as a hash "change"
- Completion signals before any activity are held until grace expires
- "Unravelling" added to claude-code working patterns
- startup_grace_secs field threaded through config and automation

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* docs: add 0.3.0 changelog entry for issue #67 and prior unreleased changes

Cover startup grace window (#67), persistent memory (#62/#63), merge gate
enforcement (#59), permissions suggest (#53), orchestration state machine
(#54/#55), and all fixes shipped since 0.2.0.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: reduce startup grace to 10s and validate wait settings

- Reduce DEFAULT_STARTUP_GRACE_SECS from 30 to 10 so the
  completion-before-activity path fires before typical wait timeouts
- Validate that wait_timeout_secs/startup_grace_secs are only set when
  wait_for_idle is true, failing fast with actionable guidance

Addresses CodeRabbit feedback on PR #71.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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.

phase 2: shared memory and memory update hooks

1 participant