feat(journal): add tempyr-journal capture layer + journal_log MCP/CLI#20
Merged
Conversation
New tempyr-journal crate with append-only JSONL writer, 8-kind taxonomy (plan/finding/assumption/question/decision/dead_end/risk/outcome), per-kind validation, gitleaks-style redaction with Shannon-entropy fallback, sticky publisher state, and rotating event log. Capture entry points: journal_log MCP tool and `tempyr journal log` CLI. Entries land in `<git-common-dir>/tempyr/journals/open/` so they survive worktree abandonment via the shared Git directory; publisher and search come in subsequent phases. 78 new unit tests (317 workspace-wide), end-to-end smoke test verified. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Single-pass cleanup driven by /simplify and a manual review: - Lift `parse_confidence`/`parse_severity`/`parse_polarity` into `FromStr` impls on `Confidence`/`Severity`/`Polarity`; both MCP and CLI use one `parse_opt::<T>` helper. - Replace local Levenshtein with `strsim::levenshtein` (already a workspace member crate dep; promoted to workspace). - Use `default_redactor()` static (`OnceLock<Redactor>`) instead of rebuilding 9 regexes per write. - Add `Entry::for_session(kind, summary, &session)` so production paths and test fixtures stop hand-constructing 25-field literals. - Restructure CLI: `JournalAction::Log(Box<LogArgs>)` via `#[derive(Args)]` — removes a 23-positional `run_log` and the 22-field destructure in `main.rs`, also fixes a clippy::large_enum_variant. - Drop dead `TestResults` schema and `SessionMeta.normalized_worktree_path` (the latter duplicated `repo_root`). - Replace hand-rolled byte-class checks for `is_valid_session_id` with a cached regex. - Fix two TOCTOU patterns in `Session::resume` and `PublisherState::load`. - Fix misleading `--reversible/--no-reversible` docstring (clap-derive only supports `--reversible <true|false>` for `Option<bool>`). - Improve `UnknownKind` error format so close-match and full-list cases both read naturally. - Strengthen `worktree_common_dir_points_to_primary` test to actually verify the worktree-vs-primary differentiation it claims. - Update `CLAUDE.md` and `AGENTS.md` to document the new crate. 317 tests still pass; clippy and fmt clean. Net -60 LOC. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as "tempyr CLI"
participant MCP as "Tempyr MCP"
participant Git as "git"
participant Redactor as "default_redactor()"
participant Session as "Session"
participant Writer as "writer::append"
participant FS as "Filesystem"
CLI->>Git: discover common-dir, worktree top, branch/HEAD
CLI->>Session: open_or_resume(common_dir, worktree_top, agent)
CLI->>Redactor: enforce(entry)
alt not redacted
CLI->>Writer: write_entry(session, draft)
Writer->>FS: open/lock JSONL, write newline JSON, sync, release lock
Writer-->>CLI: success (entry id, session id, finalized?)
else redacted and mode=Block
Redactor-->>CLI: return Redacted error
end
MCP->>Git: discover common-dir, worktree top
MCP->>Session: cached open_or_resume or replace on repo change
MCP->>Redactor: enforce(entry)
MCP->>Writer: write_entry(session, draft)
Writer->>FS: lock/write/sync
Writer-->>MCP: success (entry id, session id, finalized?)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/tempyr-cli/src/commands/journal_cmd.rs`:
- Around line 95-108: The CLI file paths in entry.files must be repo-normalized
before redaction: for each path coming from args.files (used after
Entry::for_session and assigned to entry.files), convert absolute paths under
worktree_top into relative paths (use the same logic as
cwd.strip_prefix(&worktree_top).ok().map(|p| p.to_string_lossy().replace('\\',
"/'))), normalize separators, and leave non-worktree paths unchanged (or keep
as-is) so that redact.rs sees normalized repo-relative entries instead of raw
home/absolute paths; assign the resulting normalized Vec<String> to entry.files
instead of copying args.files directly.
In `@crates/tempyr-journal/src/kind.rs`:
- Around line 126-181: Trim surrounding whitespace before measuring or
emptiness-checking user strings in validate_entry: use trimmed lengths for
summary and detail (e.g., compute len from entry.summary.trim().chars().count()
and detail_len from entry.detail.as_ref().map(|d|
d.trim().chars().count()).unwrap_or(0)) and replace the blank closure to treat
strings that are only whitespace as empty (e.g., blank should use
as_deref().map(|s| s.trim().is_empty()).unwrap_or(true)); keep the existing
error branches (Decision, DeadEnd, Assumption) but perform these trimmed checks
so fields like chosen, rationale, approach, and failure_mode composed only of
spaces fail validation.
In `@crates/tempyr-journal/src/path.rs`:
- Around line 165-170: The function canonicalize_or_keep currently returns
Result<PathBuf> but never returns Err; change its signature to return PathBuf
and update its body to return the PathBuf directly (return strip_unc(c) or
p.to_path_buf()); then update all callers that use canonicalize_or_keep
(references in the diff: calls that currently use the `?` operator) to remove
the `?` and accept the PathBuf return type (adjust any surrounding code that
expected a Result accordingly).
In `@crates/tempyr-journal/src/session.rs`:
- Around line 182-208: On collision when OpenOptions::create_new returns
AlreadyExists, don't silently ignore it: read the existing file at meta_path,
parse it with serde_json::from_str (or from_reader) and replace the in-memory
meta variable so the returned Session reflects the persisted state; propagate
any read/parse errors instead of returning success with the original meta.
Update the match in the function that writes the metadata (the block using
meta_path, serde_json::to_string_pretty, and OpenOptions) to handle Err(e) if
e.kind() == std::io::ErrorKind::AlreadyExists by opening meta_path,
deserializing into meta, and then continue to construct and return Session {
common_dir: common_dir.to_path_buf(), meta } (or alternatively return a distinct
collision result if you prefer).
- Around line 210-224: The resume function should validate that the deserialized
SessionMeta corresponds to the requested session_id to detect tampering or
corruption: after parsing SessionMeta (symbol SessionMeta) in resume, compare
meta.session_id (or the field that holds the id inside SessionMeta) to the
session_id parameter used to build jpath::session_meta_path and return an Err
describing the mismatch (rather than silently returning a Session) if they
differ; update the return path in resume (which constructs Session { common_dir,
meta }) to only proceed when the IDs match.
In `@crates/tempyr-mcp/src/handler.rs`:
- Around line 1724-1727: The code assigns entry.files =
p.files.unwrap_or_default() without normalizing repo-relative paths, which lets
absolute user-home paths slip through and later cause redact enforcement to
fail; update the assignment to map/convert each path in p.files (when present)
into a worktree-relative string (using worktree_top and the existing
relative_cwd/relative path helper logic) before setting entry.files so that
default_redactor().enforce sees repo-local paths; ensure you reuse the same
normalization used for cwd (relative_cwd or equivalent) and handle
unwrap_or_default() to avoid panics.
- Around line 1716-1719: The code currently uses env::current_dir() (cwd) and
jpath::git_common_dir / jpath::repo_toplevel directly, bypassing find_project()
and the anchoring logic; change it to resolve the repository from the anchored
project instead: call find_project() (and ensure
project_anchor_state.wait_ready() has completed) to obtain the anchored project
root, then compute common_dir and worktree_top from that anchored root (instead
of cwd) so journal_log and this handler use the same root/env resolution as
other tools; update any uses of the cwd variable to reference the anchored
project's path and preserve the same error mapping semantics.
- Around line 732-748: The current journal_session_or_open returns and caches a
single Session in self.journal_session, causing sessions to be reused across
different repositories/worktrees; change the cache to be keyed by repo/worktree
and update journal_session_or_open to lookup by a key derived from common_dir
and worktree_top (e.g., a PathBuf or string composite), returning the existing
Session if present or creating one via Session::open and inserting it into the
map before returning; adjust references to self.journal_session (replace
Option<Session> with a Mutex<HashMap<Key, Session>>), and ensure clones are used
when returning a Session so multiple callers get owned instances.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: cb9173cf-64ec-4fff-b3d2-25fcef4a9683
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (19)
AGENTS.mdCLAUDE.mdCargo.tomlcrates/tempyr-cli/Cargo.tomlcrates/tempyr-cli/src/commands/journal_cmd.rscrates/tempyr-cli/src/commands/mod.rscrates/tempyr-cli/src/main.rscrates/tempyr-interview/Cargo.tomlcrates/tempyr-journal/Cargo.tomlcrates/tempyr-journal/src/entry.rscrates/tempyr-journal/src/kind.rscrates/tempyr-journal/src/lib.rscrates/tempyr-journal/src/path.rscrates/tempyr-journal/src/redact.rscrates/tempyr-journal/src/session.rscrates/tempyr-journal/src/state.rscrates/tempyr-journal/src/writer.rscrates/tempyr-mcp/Cargo.tomlcrates/tempyr-mcp/src/handler.rs
CodeRabbit flagged 8 inline issues; verified each against current code and applied fixes plus tests: - `path.rs`: simplify `canonicalize_or_keep` (always-Ok) to return `PathBuf`, drop `?` at callers; add `repo_relative_path(path, worktree_top)` helper. - `path.rs`: tests covering normalized abs path, relative pass-through, and out-of-worktree preservation. - `kind.rs`: trim user strings before length/empty checks so a string of pure whitespace can no longer satisfy `summary >= 20`, `detail >= 50`, or the per-kind required `chosen`/`rationale`/`approach`/`failure_mode` fields. Tests cover the three previously-permissive cases. - `session.rs`: on `Session::open_at` AlreadyExists collision, read+parse the persisted meta so the returned `Session` reflects disk (was silently returning the new caller's args). Existing test extended to assert agent comes from disk. - `session.rs`: `Session::resume` validates `meta.session_id` matches the requested id; refuses to return a Session built from a tampered/corrupt meta written under the wrong filename. New test. - `journal_cmd.rs`: normalize `--file` paths through `repo_relative_path` before redaction so absolute repo paths under `/Users/<n>/` or `C:\Users\<n>\` aren't blocked by the `user_home_path` rule. - `handler.rs`: same normalization for `journal_log` MCP `files` param. - `handler.rs`: resolve `common_dir`/`worktree_top` via `find_project()` instead of raw `env::current_dir()`, matching every other tool's anchored root resolution (waits on `project_anchor_state` too). - `handler.rs`: cache journal session keyed by `(common_dir, worktree_top)` in a `JournalSessionCache` struct, so a server that ends up serving more than one repo/worktree doesn't keep appending to the wrong session id. Smoke-tested: `tempyr journal log finding ... --file <abs path>` now records `crates/tempyr-journal/src/path.rs` instead of being blocked. All 457 tests pass; clippy and fmt clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/tempyr-cli/src/commands/journal_cmd.rs`:
- Around line 92-93: Currently the code always calls Session::open(&common_dir,
&worktree_top, &args.agent) which creates a new timestamped session_id; instead,
before creating a new session, look up or resume any active session for this
worktree/agent and only call Session::open when none exists. Implement a small
discovery step that queries the common_dir/worktree_top for an active session
(e.g. Session::find_active or Session::load_active for the given agent/worktree)
and reuse that Session (preserving its session_id) when found; fall back to
Session::open(...) only if no active session is discovered. Ensure the journal
command uses the discovered Session instance for subsequent logging and for
--final to close the correct session.
In `@crates/tempyr-journal/src/path.rs`:
- Around line 104-107: The function repo_relative_path currently returns early
for non-absolute inputs leaving Windows backslashes intact; update
repo_relative_path so relative paths are normalized the same as absolute ones:
remove the early return, compute the repo-relative path (for absolute inputs
strip the worktree_top prefix, for relative inputs use the input path as-is),
then normalize path separators by replacing backslashes with forward slashes (or
use a to_slash equivalent) before returning the String; reference
repo_relative_path to locate the change.
In `@crates/tempyr-journal/src/session.rs`:
- Around line 182-203: The meta sidecar write is racy because create_new reports
AlreadyExists while the winner may still be writing, causing partial-JSON reads;
fix by writing the JSON to a temporary file next to meta_path (e.g., meta_path +
".tmp"), fsync the temp file, then atomically rename (fs::rename) it to
meta_path so readers never see partial content; additionally, in the
AlreadyExists branch (where you read meta_path into bytes and call
serde_json::from_slice into meta), wrap the read/deserialize in a short retry
loop (with a small sleep/backoff) that retries until deserialization succeeds or
a timeout is reached to handle the rare window before the atomic rename
completes — reference symbols: meta_path, meta, session_id, OpenOptions (current
use), and resume()/the AlreadyExists branch.
In `@crates/tempyr-mcp/src/handler.rs`:
- Around line 678-686: The function relative_cwd currently falls back to
returning an absolute cwd string when cwd is outside worktree_top; change this
so it returns None instead (matching CLI behavior) when strip_prefix fails.
Update the relative_cwd function to only map the strip_prefix result to a
normalized forward-slash string and remove the or_else fallback that returns
cwd.to_string_lossy(); ensure the function returns Option<String> with None for
out-of-repo paths.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3c5b42b5-dec5-44b5-b93e-0597d48da044
📒 Files selected for processing (5)
crates/tempyr-cli/src/commands/journal_cmd.rscrates/tempyr-journal/src/kind.rscrates/tempyr-journal/src/path.rscrates/tempyr-journal/src/session.rscrates/tempyr-mcp/src/handler.rs
CodeRabbit flagged 4 more inline issues; verified each and applied fixes plus tests: - session.rs: add `Session::find_active(common_dir, worktree_top, agent)` + `Session::open_or_resume(...)` so back-to-back CLI invocations land in one session instead of spawning a new timestamped session per call. CLI and MCP both switch from `Session::open` to `open_or_resume`. Tests cover empty-journals, agent filtering, finalized skip, newest-first ordering, and reuse-then-fallback semantics. - path.rs: `repo_relative_path` now normalizes backslashes to forward slashes for *all* inputs (relative ones too) — Windows-style relative paths from `--file crates\foo\bar.rs` no longer survive into JSONL. Existing test renamed/updated; new test for the relative case. - session.rs: replace the create_new + read-on-AlreadyExists dance with an atomic-write pattern: write JSON to a unique `<id>.meta.json.tmp.<n>` via create_new, fsync, then `hard_link` it into place. hard_link fails cleanly with AlreadyExists if a concurrent opener won the race (rename would silently replace), and `read_meta_with_retry` absorbs the microsecond window where the loser sees the link mid-publish. - handler.rs: drop the `or_else(|| Some(cwd.to_string_lossy()))` fallback in `relative_cwd` so an MCP server invoked outside the worktree no longer leaks an absolute home-dir path that the redactor would block. Now matches the CLI's behavior. Three new unit tests. Smoke-tested: two `tempyr journal log` calls separated by 2s now share the same `session_id` (find_active works). All 468 tests pass; clippy and fmt clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/tempyr-cli/src/commands/journal_cmd.rs`:
- Around line 129-135: The commit sets entry.is_final but never calls
session.finalize(), so --final never actually closes the session; after
successfully appending the final entry (the append(&session, &entry) call) add a
conditional that if args.is_final is true you call session.finalize() and
map/propagate any error into anyhow (similar to the append error mapping) so the
.ready marker is written and the session is closed; place this after
default_redactor().enforce(...) and append(&session, &entry) succeed.
In `@crates/tempyr-journal/src/session.rs`:
- Around line 167-169: SessionId currently only uses date+worktree hash+second
which allows two different agents to generate the same SessionId and collide;
modify the open flow around SessionId::new, find_active and meta_path handling
so that if a collision is detected but the persisted metadata agent differs from
the requesting agent (i.e., the loaded meta indicates agent != requested agent),
treat it as a retry: loop to generate a new SessionId (waiting or spinning until
the clock advances to a new second) and retry find_active/open until either you
get the matching agent/session or a fresh SessionId is created; update the logic
in the functions/methods that call SessionId::new, find_active, and the open()
path to perform this retry-on-agent-mismatch, and add a regression test that
simulates two agents (e.g., "claude" and "codex") opening the same worktree
within the same second to assert the second caller gets a distinct session and
does not append to the first agent's session.
In `@crates/tempyr-mcp/src/handler.rs`:
- Around line 1727-1730: The tool description on the #[tool(name =
"journal_log", ...)] attribute references "journal_search" which doesn't exist;
remove or replace that reference so clients aren't directed to a non-existent
tool (locate the description string inside the journal_log tool attribute and
delete the phrase "for past reasoning, use journal_search" or reword to omit any
call-to-action to journal_search while keeping the existing safe guidance such
as "For curated knowledge, use graph_add_node").
- Around line 1773-1778: The code currently sets entry.is_final and persists the
entry (via default_redactor().enforce and append) but never finalizes the
session, so add logic after append(&session, &entry) to finalize when
entry.is_final is true: call the existing session-finalization routine (create
the session ".ready" marker or invoke the function that writes the ready marker
/ marks the session ready) so the publisher will be triggered and
Session::find_active() will stop resuming this session; ensure the
finalize/ready-create step runs only after successful append and propagates
errors as map_err(|e| e.to_string()) consistent with surrounding error handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: b5aa0a78-cf4b-42c9-a1db-4fb2ff398f50
📒 Files selected for processing (4)
crates/tempyr-cli/src/commands/journal_cmd.rscrates/tempyr-journal/src/path.rscrates/tempyr-journal/src/session.rscrates/tempyr-mcp/src/handler.rs
CodeRabbit flagged 4 more issues; all verified and fixed:
- session.rs: two agents in the same wall-clock second on the same
worktree previously got the same SessionId and the loser silently
reused the winner's session under the wrong agent name. Now `open_at`
returns a new `JournalError::AgentMismatch { existing, requested }`
variant on collision, and `open()` retries up to 30 times with brief
sleeps, advancing `Utc::now()` each pass until the second ticks over.
Same-agent same-second is still a clean reuse. Replaced the previous
`open_twice_with_same_ts_does_not_clobber_meta` test with three
tighter cases:
- same-agent reuse preserves disk and id
- different-agent strict-form errors with AgentMismatch
- live `open()` advances past a same-second collision so the two
agents end up with distinct ids
- journal_cmd.rs (CLI): `--final` set `entry.is_final` but never wrote
the `.ready` marker, so `find_active` would keep resuming the session
forever. Now calls `session.finalize()` after a successful append. CLI
output reports "Session finalized: <id>" and the `--json` payload gets
a `finalized` field.
- handler.rs (MCP `journal_log`): same finalize bug — `is_final` on the
entry never closed the session. Same fix; the JSON response now
carries `finalized: bool`.
- handler.rs: tool description previously pointed clients at
`journal_search`, which doesn't exist (Phase 3). Replaced with
graph_add_node-only routing guidance.
Smoke tests:
$ tempyr journal log outcome "..." --final --json
→ writes `<id>.ready`; the next CLI call gets a fresh session id
instead of resuming the finalized one.
All 470 tests pass; clippy and fmt clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
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 `@crates/tempyr-cli/src/commands/journal_cmd.rs`:
- Around line 83-170: The run_log implementation duplicates the
transport-independent journal-write pipeline found in handler.rs (e.g.,
Kind::parse_helpful, cwd_rel computation, Entry::for_session, populating entry
fields, default_redactor().enforce, append, and Session::finalize); extract that
shared logic into a new API in tempyr-journal (e.g., a function like
write_entry_or_finalize(session: &Session, args: &LogArgs) -> Result<(Entry,
bool)>) and have both run_log and handler::journal_log call it; move
path-normalization (cwd_rel), entry population (all entry.* assignments),
redaction (default_redactor().enforce), append(entry), and finalize(session)
into that helper so Session::open_or_resume / Kind::parse_helpful remain caller
responsibilities while transport-specific output (JSON/cli messages) stays in
run_log and handler.rs.
In `@crates/tempyr-mcp/src/handler.rs`:
- Around line 757-761: The code currently hardcodes the agent string "claude"
when opening a journal session via Session::open_or_resume in handler.rs; change
this to use the runtime/server-config agent identity instead (e.g., pull the
agent id from the TempyrServer configuration or MCP runtime context used by this
handler) so the call becomes Session::open_or_resume(common_dir, worktree_top,
agent_id) where agent_id is obtained from the surrounding TempyrServer struct or
handler context (not a string literal), and propagate or validate that agent
value as needed before calling Session::open_or_resume.
- Around line 751-756: The current fast-path returns a cached session when
guard.as_ref() matches common_dir and worktree_top, but it doesn't check whether
that cached Session is already finalized; this can cause reopened journal_log
calls to append to a closed JSONL. Update the conditional around guard.as_ref()
so that after matching cache.common_dir == common_dir && cache.worktree_top ==
worktree_top you also check cache.session.is_ready() (or
!cache.session.is_ready() depending on API) before returning; if the cached
session is finalized call Session::find_active() (reopen) and replace the
cache.session with the newly opened session, then return that active session.
Ensure you reference the same guard/cache variables and maintain existing
cloning semantics for cache.session.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4e1d66b7-8357-4a6f-85c2-564e6b374e65
📒 Files selected for processing (4)
crates/tempyr-cli/src/commands/journal_cmd.rscrates/tempyr-journal/src/lib.rscrates/tempyr-journal/src/session.rscrates/tempyr-mcp/src/handler.rs
CodeRabbit flagged 3 more issues; all verified and fixed:
- writer.rs: extracted the duplicated CLI/MCP write pipeline into
`EntryDraft` + `write_entry(session, worktree_top, draft)`. The helper
owns: Entry::for_session population, file-path normalization (via
jpath::repo_relative_path), cwd relativization, redaction enforcement,
append, and conditional Session::finalize. Returns a `WriteOutcome
{ entry, finalized }` for transport-specific output formatting.
Added 3 direct tests (path/cwd normalization + finalize, no-finalize
path, redactor blocks pre-append).
- journal_cmd.rs (CLI): switched run_log to populate an EntryDraft and
call write_entry; deleted the now-redundant inline cwd-relativization,
redactor invocation, append, and finalize. Output formatting (JSON +
text) stays here.
- handler.rs (MCP): same refactor; the journal_log tool now builds
EntryDraft from JournalLogParams and calls write_entry. Deleted the
local `relative_cwd` helper and its three unit tests — the same logic
now lives once inside write_entry. Removed unused imports of Entry,
default_redactor, and append.
- handler.rs: hardcoded "claude" agent string replaced with a new
`agent_id: String` field on TempyrServer (default
`DEFAULT_AGENT_ID = "claude"`, configurable via
`with_agent_id(impl Into<String>)`). The journal_session_or_open path
passes `&self.agent_id` instead of the literal so non-Claude embedders
can attribute journals correctly.
- handler.rs: cached journal session is now also discarded when
`cache.session.is_ready()` — i.e., the previous entry triggered
`finalize()`. Without this check a follow-up `journal_log` call would
silently keep appending to a session the publisher had already taken
ownership of. The cache hit now requires `(common_dir, worktree_top,
!is_ready)` to match.
Smoke-tested:
$ tempyr journal log finding "..." --json
→ resumes the active session via open_or_resume, writes through the
new shared pipeline, prints the same shape as before.
All 470 tests pass; clippy and fmt clean. Net +176 LOC (the EntryDraft
struct + WriteOutcome + 3 new tests on the journal side; CLI and MCP
each lost ~30 lines of duplication).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
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 `@crates/tempyr-journal/src/writer.rs`:
- Around line 149-158: The current flow calls append(session, &entry) and
session.finalize() separately, which allows races where a "final" entry is
appended but another append slips in before session.finalize() sets the ready
marker; fix by introducing a session-level critical section that atomically
performs the ready-check, append, and (if entry.is_final) finalize. Implement a
new Session method (e.g., Session::append_with_finalization or
Session::atomic_append) that acquires the session mutex/lock, checks if the
session is already ready (rejecting further writes), performs the append
operation, and then, if entry.is_final, calls session.finalize() before
releasing the lock; replace the two-step append + session.finalize() code paths
(the block around default_redactor()/append()/session.finalize() and the similar
block at lines 175-198) to call this atomic method so the ready-marker and JSONL
append cannot be interleaved by concurrent callers.
- Around line 123-132: entry.files are being repo-normalized before resolving
relative inputs against the draft's cwd, causing relative paths like
"src/lib.rs" to remain relative to the CLI working dir; change the mapping that
sets entry.files (using draft.files and jpath::repo_relative_path) to first
resolve each file against the draft.cwd (use the same relative_cwd(draft.cwd,
worktree_top) logic or the already computed entry.cwd) when the file is
relative, then call jpath::repo_relative_path on the resulting absolute or
cwd-resolved path so repo-normalization yields repo-root-relative paths; keep
using draft.files, jpath::repo_relative_path, draft.cwd, relative_cwd and
worktree_top to locate the correct spots to change.
In `@crates/tempyr-mcp/src/handler.rs`:
- Around line 685-693: The cached journal session key currently ignores agent
identity, so Session::open_or_resume() can return a Session tied to the wrong
agent when TempyrServer::with_agent_id() is used or when instances are cloned;
update the cache key and cache-access logic to include agent_id (the agent
string used by Session::open_or_resume()) so JournalSessionCache is keyed by
(common_dir, worktree_top, agent_id), and adjust all places that construct or
lookup journal_session (the journal_session:
Arc<Mutex<Option<JournalSessionCache>>> field and its accessors/fast-path
checks) to use the new triplet key when creating, matching, or resuming sessions
to ensure per-agent isolation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 65d33bc3-7991-488c-ad7a-7a22594692f0
📒 Files selected for processing (4)
crates/tempyr-cli/src/commands/journal_cmd.rscrates/tempyr-journal/src/lib.rscrates/tempyr-journal/src/writer.rscrates/tempyr-mcp/src/handler.rs
CodeRabbit flagged 3 more issues; all verified and fixed:
- writer.rs: `append(session, entry)` is now atomic with respect to
session finalization. Under the JSONL exclusive lock it (1) checks
`session.is_ready()` and refuses with `JournalError::InvalidEntry` if
the session was finalized between caller's open and this write,
(2) writes one full line and fsyncs, (3) on `entry.is_final` calls
`session.finalize()` to drop the `.ready` marker — all before the
lock is released. Prevents two real races:
- a concurrent writer slipping an append in between the final
entry and the marker write
- a stale CLI/MCP cache writing to a session the publisher has
already taken ownership of
Added two tests: post-final straggler is refused (and the JSONL only
has the two valid entries), and `append_validated` (the publisher
escape hatch) still works on finalized sessions because it's the
documented bypass. write_entry's separate `session.finalize()` call
is gone — append handles it.
- path.rs: new `resolve_file_path(path, worktree_top, cwd)` joins
relative inputs onto `cwd` before normalizing. `tempyr journal log
--file src/lib.rs` from `<repo>/crates/foo/` now records as
`crates/foo/src/lib.rs` instead of `src/lib.rs` (the previous
behavior would have made the path ambiguous to a reader).
Absolute inputs and the no-cwd fallback both delegate to
`repo_relative_path`. write_entry switched to this helper.
Three new tests; updated the existing write_entry normalization test
to express the realistic CLI scenario.
- handler.rs: cache key now includes `agent_id` — the field is
`JournalSessionCache { common_dir, worktree_top, agent_id, session }`
and the fast-path `if let Some(cache) = guard.as_ref()` check
requires `cache.agent_id == self.agent_id` along with the existing
predicates. This matters because `TempyrServer` is `Clone` and
`with_agent_id(...)` is a builder that consumes `self`: a clone with
a different agent_id shares the `Arc<Mutex<...>>` cache, and without
the agent_id key one server's session would silently leak into the
other.
Smoke-tested:
$ cd crates/tempyr-journal && tempyr journal log finding "..." \
--file src/path.rs --json
→ entry.files = ["crates/tempyr-journal/src/path.rs"]
entry.cwd = "crates/tempyr-journal"
All 475 tests pass; clippy and fmt clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
10 tasks
This was referenced Apr 28, 2026
3 tasks
This was referenced Apr 28, 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
Adds a new
tempyr-journalcrate that captures agent reasoning (decisions, dead ends, findings, etc.) as append-only JSONL under<git-common-dir>/tempyr/journals/open/. Survives worktree abandonment because it lives in the shared Git directory. Exposed viajournal_logMCP tool andtempyr journal logCLI.This is the capture layer of a 4-phase plan. Publisher (Git refs + push), search/index, and integration with the existing graph follow in subsequent PRs.
What's in here
plan | finding | assumption | question | decision | dead_end | risk | outcomewith per-kind structured field validation (e.g.,decisionrequireschosen/rationale/reversible;dead_endrequiresapproach/failure_mode).std::fs::File::lock(Rust 1.89+, no third-party crate). Windows-correct open flags (.read(true).append(true)); 8-thread concurrent-write stress test passes with no torn lines.git rev-parse --git-common-dirresolution so journals are written to the primary repo's.git/, not a worktree's.gitgitfile.v: 1) for forward compat; readers ignore unknown fields.<YYYYMMDD>-<wt_hash>-<HHMMSS>ID, atomicmeta.jsonsidecar (create_new),.readymarker for the publisher..git/tempyr/journals/.Crate layout
entryEntrystruct +Confidence/Severity/Polarityenums withFromStrimplskindKindenum,parse_helpfulwith Levenshtein typo suggestions, per-kind validationpathgit_common_dir/repo_toplevel/worktree_hash(Windows case-folded) + journal directory layoutsessionSessionId(regex-validated, path-injection-safe),Sessionlifecyclewriterappend(session, entry)— validate → lock → singlewrite_all→ fsyncredactRedactorwith Block/Redact/Warn modes; staticdefault_redactor()statePublisherState(sticky last-error/counters, atomic temp+rename) + rotating event logOut of scope (subsequent PRs)
tempyr journal flush, in-process tokio ticker, Git tree/commit/push torefs/tempyr/journals/..., public-repo detection at init, refspec config, pack-refs cadence)journal_searchMCP tool,tempyr journal searchCLI).claude/settings.jsonhook template forSessionStart/SessionEndTest plan
cargo test --workspace --lib— 317 passcargo clippy --workspace --all-targets— cleancargo fmt --check— cleantempyr journal log decision "..." --chosen X --rationale Y --reversible truewrites a valid JSONL line to the primary repo's.git/tempyr/journals/open/, with branch and HEAD auto-capturedtempyr journal log baddkindd "..."returnsUnknown kind "baddkindd". Valid kinds: plan, finding, ...tempyr journal log desicion "..."returnsUnknown kind "desicion". Did you mean+ "decision?"🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Chores