fix: address the 0.7.0 pre-release review findings - #65
Merged
Conversation
The app tests opened App against the developer's own Plannotator data directory, and the send test's Discard delivery succeeds, so every run appended a record to the real feedback archive. Point each app under test at a fresh temp data dir and assert the send lands there.
The Claude Code and Copilot resolvers walk a ladder whose first rung is the session registered for the agent's pid, yet every result was labelled a folder guess, so the footer said "showing the newest transcript for this folder" even when the pid had identified the session. The resolvers now report which rung matched; a pid-registered session is exact, cwd and folder rungs stay the folder's newest transcript, and Copilot's unscoped rungs read as the newest session.
message_source put the transcript path into Provenance::AgentMessage's session, so an archived message review carried the path in both target.agent.session and target.agent.transcript, where the shared feedback-archive contract wants the host-assigned session id in session. Carry the real id instead: the one Herdr or --session-id gave, else the one a uuid-named Claude Code, Droid, Codex or Copilot transcript carries in its name, else nothing. The transcript path stays in the app's own field for the archive's transcript.
The README said reply reviews are never written to disk, which stopped being true when the feedback archive landed: a successful send or copy appends the submitted feedback to the shared archive for every review kind. Say what is written, where, when, and how to turn it off.
A session id given next to a transcript path is validated before any reader runs, and it is recorded only for hosts whose transcript names carry no id of their own. A uuid-named transcript is the authority for its own id, so a contradicting id can no longer be archived as the reviewed message's session.
Merged
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.
Fixes the four actionable findings from the 0.7.0 pre-release compatibility review, all in code that landed since v0.6.0.
sessionfield, so archive records had the path in bothtarget.agent.sessionandtarget.agent.transcript, against the frozen contract.Locatednow carries a real session id: an explicit--session-idor Herdr'sPLANNOTATOR_TUI_SESSION_IDwins, else the id derived from a uuid-shaped transcript name (Claude Code, Droid, Codex rollouts, Copilot session dirs), else absent. Paths are never written tosession.Exactand gets no hint; cwd and folder rungs say "newest transcript for this folder"; Copilot's unscoped rungs say "newest session".cargo test. EveryAppunder test now uses a scratch data dir; verified by checksumming the realfeedback/tree before and after a full run with no env override (120 files, identical).Gate:
cargo fmt --all --check,cargo clippy --locked --workspace --all-targets -- -D warnings,cargo test --locked --workspace(195 passed). Production behavior changes are limited to items 1 and 3.