Repository navigation
fix(dsh): preserve config and forward daemon credentials - #1606
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 2 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly summarizes the main changes: preserving DSH configuration and forwarding daemon credentials. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| if (wantHooks && (!hasHooks || opts.force)) { | ||
| const hooksPath = installHooksFile(home); | ||
| next = stripBlock(next, HOOKS_MARKER); | ||
| next = appendBlock(next, hooksBlock(hooksPath)); | ||
| next = upsertBlock(next, HOOKS_MARKER, hooksBlock(hooksPath)); | ||
| p.log.info(`Hook manifest: ${hooksPath}`); |
There was a problem hiding this comment.
🟡 Partial reconnect leaves hooks inactive
When the MCP row is missing but the hooks row remains, --with-hooks skips installHooksFile. A missing manifest stays missing, so reconnect reports success without restoring automatic capture.
Learn more
The hooks row points to a separate manifest containing the commands that launch capture scripts. If the MCP row has been removed but the hooks row remains, install proceeds instead of returning already-wired. The new condition skips installHooksFile in that path, even when the manifest has been deleted. The earlier install path regenerated the manifest whenever a reconnect proceeded with hooks enabled.
Example: A user retains an agentmemory-hooks insert row, removes the MCP insert row, and loses agentmemory.hooks.json. Running connect dsh --with-hooks writes the MCP row and reports success, but the bridge still cannot read its manifest.
Recommended fix: When wantHooks is true and the install proceeds because hasMcp is false, restore the manifest even if hasHooks is true. Keep the existing hooks insert row intact unless --force is set or the row is absent.
Was this helpful? React with 👍 or 👎 to provide feedback.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Preserve patch operations and neighboring Harness plugins and customized MCP settings when reconnecting AgentMemory. Forward the daemon URL and secret at Harness startup without storing credentials, and document the optional hooks bridge dependencies and authentication limits.
Validated with 24 adapter tests, the full repository suite (3,868 passed, 4 skipped), and repeated forced installs through the built CLI. Earlier integration testing used real dsh 0.2.1-rc.2 against iii 0.22.1 and 0.24.6. The scripted model exercised MCP save/recall and local hook capture; a freshly packed runtime also preserved the project filter.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation