Skip to content

fix(devin): make SessionEnd installation non-destructive#162

Merged
Yif-Yang merged 1 commit into
microsoft:mainfrom
Yif-Yang:fix/devin-session-end-installer-safety
Jul 21, 2026
Merged

fix(devin): make SessionEnd installation non-destructive#162
Yif-Yang merged 1 commit into
microsoft:mainfrom
Yif-Yang:fix/devin-session-end-installer-safety

Conversation

@Yif-Yang

Copy link
Copy Markdown
Contributor

Summary

Follow-up hardening for #148:

  • merge hook arrays without replacing existing project events or SessionEnd hooks
  • make repeated installation idempotent and preserve malformed configs on failure
  • use a SkillOpt-specific hook filename so an existing on-session-end.sh is not overwritten
  • safely quote the printed MCP server path and make the hook succeed when HOME is unavailable
  • clarify that the activity marker is for inspection/automation; current harvesting uses transcript timestamps
  • add installer, marker, malformed-config, idempotency, existing-hook, and special-path tests

Validation

  • 368 passed, 6 skipped
  • bash -n plugins/devin/install.sh plugins/devin/hooks/on-session-end.sh
  • git diff --check
  • independent Claude Code review using claude-opus-4.8: MERGE, no blocking findings
  • official Devin hook contract checked for .devin/hooks.v1.json, SessionEnd, and DEVIN_PROJECT_DIR

Copilot AI review requested due to automatic review settings July 21, 2026 18:03
@Yif-Yang
Yif-Yang merged commit 61735e3 into microsoft:main Jul 21, 2026
1 check passed

Copilot AI 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.

Pull request overview

Hardens the Devin plugin installer and SessionEnd hook to be non-destructive and idempotent, ensuring existing project hooks/configs are preserved while still installing SkillOpt-Sleep’s integration safely.

Changes:

  • Update installer to merge hooks.v1.json event arrays without overwriting existing hooks and to avoid duplicating the SkillOpt entry on reinstall.
  • Install the SessionEnd hook under a SkillOpt-specific filename to avoid clobbering an existing project on-session-end.sh.
  • Improve robustness/UX: shell-quote the printed MCP server path and make the hook succeed when HOME is missing; add comprehensive installer/hook tests.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/test_devin_plugin.py Adds installer/hook tests covering idempotency, malformed configs, legacy hook preservation, and safe path quoting.
plugins/devin/README.md Clarifies marker purpose (inspection/automation) vs current harvesting mechanism; documents non-destructive installer behavior.
plugins/devin/install.sh Switches to Python-based merge (no overwrite of existing hooks), avoids duplicate installs, uses SkillOpt-specific hook filename, and prints a safely shell-quoted MCP path.
plugins/devin/hooks/on-session-end.sh Makes the marker hook non-blocking when HOME is unavailable; clarifies marker semantics.
plugins/devin/hooks/hooks.v1.json Updates hook command to point at the new SkillOpt-specific hook script name.
CHANGELOG.md Notes the new non-destructive Devin installer + SessionEnd marker behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread plugins/devin/install.sh
Comment on lines 18 to 19
# Merge into existing hooks.v1.json instead of overwriting, so we don't
# destroy other project hooks.
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.

2 participants