fix: stop attributing machine plumbing to the user peer - #77
Conversation
|
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:
WalkthroughHoncho adds configurable Git event persistence, filters harness-injected and terse prompts during uploads, and replaces remote session-end marker handling with local logging, cleanup, and immediate exit. ChangesHoncho filtering and lifecycle handling
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant UserPromptHook
participant SaveUserMessageHook
participant HonchoAPI
UserPromptHook->>SaveUserMessageHook: Provide prompt
SaveUserMessageHook->>SaveUserMessageHook: Detect harness or terse content
SaveUserMessageHook->>HonchoAPI: Skip upload or post with reasoning disabled
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CHANGELOG.md`:
- Around line 13-15: Update the changelog entry for harness-injected turns and
terse acknowledgements to clarify that terse acknowledgements remain uploaded
for conversational continuity but no longer generate content-free conclusions;
avoid implying they are excluded from user memory or persistence.
🪄 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: CHILL
Plan: Pro
Run ID: 0d0e8a49-c232-4633-b2f8-76cc3f7bc95e
📒 Files selected for processing (4)
CHANGELOG.mdplugins/honcho/.claude-plugin/plugin.jsonplugins/honcho/package.jsonplugins/honcho/src/hooks/user-prompt.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- plugins/honcho/src/hooks/user-prompt.ts
akattelu
left a comment
There was a problem hiding this comment.
lgtm.
since hooks accept input from stdin, it should be possible to write some tests (not bundled with the plugin) that verify behavior like this, but not blocking as requirement of this PR.
/loop wakeups re-submit literal sentinels like <<autonomous-loop-dynamic>> through the user slot; matched only when the whole trimmed prompt is one <<token>>, so prompts that merely start with << are never dropped. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
250 markers in the audit dataset derived exactly 17 conclusions, all
lifecycle exhaust ('claude's session ended at ...'); nothing reads the
marker back. Messages upload live elsewhere, so session-end now only
writes the local summary and clears state. Supersedes #37.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
# Conflicts: # plugins/honcho/src/hooks/session-end.ts # plugins/honcho/src/hooks/user-prompt.ts
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
akattelu
left a comment
There was a problem hiding this comment.
i think you need to address merge conflicts but lgtm otherwise!
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/honcho/src/hooks/user-prompt.ts (1)
312-312: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDo not write prompt-derived queries to activity logs.
logApiCallembedsdetailsin its activity message, so these calls retain the first 60 characters of user input (or a template containing it). Prompts can contain credentials or PII; log query source/length instead.
plugins/honcho/src/hooks/user-prompt.ts#L312-L312: replace the dialectic query preview with non-sensitive metadata.plugins/honcho/src/hooks/user-prompt.ts#L360-L360: replace the context search-query preview with non-sensitive metadata.Proposed fix
- logApiCall("peer.chat (dialectic)", "POST", `${reasoning}: ${query.slice(0, 60)}`, elapsedMs, true); + logApiCall("peer.chat (dialectic)", "POST", `${reasoning}; query length: ${query.length}`, elapsedMs, true); ... - logApiCall(contextLabel, "GET", `search: ${searchQuery.slice(0, 60)}`, Date.now() - startTime, true); + logApiCall(contextLabel, "GET", `search; query length: ${searchQuery.length}`, Date.now() - startTime, true);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/honcho/src/hooks/user-prompt.ts` at line 312, Update both logApiCall sites in user-prompt.ts at lines 312 and 360 to remove prompt-derived query previews from activity details. Replace the dialectic and context search details with non-sensitive metadata such as query source and length, without including any query content or templates containing it.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@plugins/honcho/src/hooks/user-prompt.ts`:
- Line 312: Update both logApiCall sites in user-prompt.ts at lines 312 and 360
to remove prompt-derived query previews from activity details. Replace the
dialectic and context search details with non-sensitive metadata such as query
source and length, without including any query content or templates containing
it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cc90c29f-6d84-4960-8d07-66317d98d5be
📒 Files selected for processing (5)
plugins/honcho/src/config.tsplugins/honcho/src/hooks/save-user-message.tsplugins/honcho/src/hooks/session-end.tsplugins/honcho/src/hooks/session-start.tsplugins/honcho/src/hooks/user-prompt.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- plugins/honcho/src/hooks/session-start.ts
- plugins/honcho/src/hooks/session-end.ts
- plugins/honcho/src/config.ts
Main (#77) already removed the [Session ended] marker upload and cooldown animation; the transcript parse survived only to put a message count in a log line. Remove it too — SessionEnd now just logs and clears session state (~20ms, zero I/O beyond stdin), so on /exit the harness abort window can never surface 'SessionEnd hook failed: Hook cancelled'. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Summary by CodeRabbit
New Features
Bug Fixes