Add warp plugin to plugin marketplace#137
Conversation
Move the Warp terminal integration plugin from the standalone augmentmoogi/auggie-warp repository into plugin_marketplace/warp, alongside the existing code-review plugin.
There was a problem hiding this comment.
Thanks for porting this plugin over — the structured/legacy split and version gating make the overall shape easy to follow. I left two high-confidence comments: one on the broken homepage URL in the metadata, and one on the Stop hook path, which currently won't populate the completion summary from the documented hook payload.
🔖 Automated review complete. Please react with 👍 or 👎 on the individual review comments to provide feedback on their usefulness.
Add metadata.includeConversationData to the Stop hook entry and read from .conversation.userPrompt / .conversation.agentTextResponse instead of the undocumented ._exchange path.
augmentmoogi
left a comment
There was a problem hiding this comment.
Code Review Agent🛡️ with @augmentmoogi's authorization
Posted as a COMMENT review because GitHub does not allow @augmentmoogi (the PR author) to self-approve via either the bot integration or their personal token. The verdict below is APPROVE — please apply the GitHub approval manually.
Verdict: APPROVE
Summary: A self-contained, well-tested addition of a Warp terminal-notification plugin to the official Auggie marketplace. The structured/legacy split is sensible, JSON construction is safe against user-controlled values (verified by the JSON-special-character tests), and the two prior github-actions findings (broken homepage URL, undocumented Stop-hook fields) were correctly addressed in follow-up commits. No blockers.
Blockers: None
Suggestions: Lexicographic version comparison in should-use-structured.sh; inconsistent legacy-fallback gating in on-session-start.sh vs. on-stop.sh/on-notification.sh.
Nits: Stale tool_input.command assertion in the permission_request test; modern-only-hook routing test only covers one of three scripts.
Reasoning: The change is additive, isolated to a new plugin_marketplace/warp/ directory plus a single registry entry in .augment-plugin/marketplace.json. All 37 tests pass locally. The remaining concerns are either future-proofing (version comparison) or minor consistency cleanups (gating, test coverage), none of which warrant blocking.
| esac | ||
|
|
||
| # If we matched a channel and the version is <= the broken threshold, fall back. | ||
| if [ -n "$threshold" ] && [[ ! "$WARP_CLIENT_VERSION" > "$threshold" ]]; then |
There was a problem hiding this comment.
Code Review Agent🛡️ on behalf of @augmentmoogi
This comparison relies on lexicographic ordering of vYYYY.MM.DD.HH.MM.<channel>_NN strings. It works today because the build counter is zero-padded to two digits — but if Warp ever ships ..stable_100, lexicographic comparison will incorrectly classify it as older than ..stable_99. Consider either documenting the zero-padding assumption near the LAST_BROKEN_* constants or parsing the date + counter into integers (IFS=. read plus a numeric compare on the counter) so the gate keeps working as Warp's release numbering evolves.
|
|
||
| # Legacy fallback for old Warp versions | ||
| if ! should_use_structured; then | ||
| exec "$SCRIPT_DIR/legacy/on-session-start.sh" |
There was a problem hiding this comment.
Code Review Agent🛡️ on behalf of @augmentmoogi
The legacy-fallback gating is inconsistent across the three hooks that have legacy variants: on-stop.sh and on-notification.sh only exec their legacy script when TERM_PROGRAM=WarpTerminal, but this one execs unconditionally and lets legacy/on-session-start.sh self-guard. The end behavior is the same today, but the divergence makes it easy to break the invariant on the next change to either side. Consider applying the same [ "$TERM_PROGRAM" = "WarpTerminal" ] && exec ...; exit 0 pattern in all three modern-path scripts.
| if [ "$TOOL_NAME" = "ask-user" ]; then | ||
| BODY=$(build_payload "$INPUT" "permission_request" \ | ||
| --arg tool_name "$TOOL_NAME") | ||
| "$SCRIPT_DIR/warp-notify.sh" "warp://cli-agent" "$BODY" |
There was a problem hiding this comment.
Code Review Agent🛡️ on behalf of @augmentmoogi
The permission_request test in tests/test-hooks.sh builds a payload with tool_input.command, but this hook only forwards --arg tool_name "$TOOL_NAME" — never tool_input. The test is exercising build-payload.sh's arg-forwarding generically, not what the hook actually emits, so the asserted tool_input.command field will not appear in real notifications. Either drop tool_input from that test case (since the hook doesn't emit it) or, if you do want Warp to receive the tool input on permission_request, plumb it through here with --argjson tool_input "$(echo "$INPUT" | jq -c '.tool_input // {}')".
| echo "" | ||
| echo "--- Modern-only hooks exit silently without protocol version ---" | ||
|
|
||
| for HOOK in on-post-tool-use.sh; do |
There was a problem hiding this comment.
Code Review Agent🛡️ on behalf of @augmentmoogi
The "modern-only hooks exit silently without protocol version" loop only iterates over on-post-tool-use.sh, so the same invariant for on-pre-tool-use.sh and on-prompt-submit.sh is asserted only by inspection. Expanding the list to on-post-tool-use.sh on-pre-tool-use.sh on-prompt-submit.sh is a one-line change that locks in the contract for all three modern-only hooks.
Add Warp terminal integration to the plugin marketplace
warp://cli-agentpayloads for newer Warp releases and legacy OSC notifications for older versions, including protocol negotiation and version-based fallback behavior.🤖 This description was generated automatically. Please react with 👍 if it's helpful or 👎 if it needs improvement.