feat(agents): add native plugins for Codex Claude and Cursor - #73
Conversation
roodboi
left a comment
roodboi
left a comment
There was a problem hiding this comment.
Requesting changes because the migration currently has no safe cutover boundary: a normal interactive Hack command can remove the working standalone integration before the native plugin is installed or enabled.
What I verified on this head:
- full
bun testexits successfully - the focused plugin/setup suite passes (48 tests, 193 assertions)
bun run typecheck,bun run check, andgit diff --checkpass- hosted CI is green across test, runtime-images, Docker E2E, and secret scan
- live
codex plugin list --jsonandclaude plugin list --jsonoutput match the new parsers
What still needs direct verification before merge:
- Fresh marketplace add + plugin install for Codex, Claude Code, and Cursor using current stable clients.
- A cutover matrix for plugin missing / disabled / enabled × project / user legacy artifacts × generated / customized artifacts. Missing or disabled must retain the working legacy integration; enabled may remove only exact generated copies.
- After a new session, prove both skills, the Claude hooks / Cursor rule, and
hack mcp serveare actually loaded from each installed plugin. - Exercise interactive
hack initandhack setupoutput so missing/disabled plugins are warnings with non-success status.
Please also complete the PR description's Summary, Verification, Release Signal, Semantic Surfaces, and Risks sections. This is a user-facing feat and the release decision must be explicit per repository policy.
| installClaudeHooks({ scope: "user" }), | ||
| installCodexSkill({ scope: "project", projectRoot: opts.projectRoot }), | ||
| installCodexSkill({ scope: "user" }), | ||
| removeDeprecatedHackCursorIntegration({ |
There was a problem hiding this comment.
[P1] Gate legacy cleanup on native-plugin readiness. maybeEnsureAgentIntegrations() reaches this auto-sync from any normal interactive project command, and this block removes project and user Cursor/Claude/Codex artifacts without first proving the corresponding plugin is installed and enabled. After upgrading Hack, a user's first hack up can therefore delete working rules, hooks, skills, and MCP config while leaving only install guidance. Make auto-sync warn-only until checkHack*Plugin() returns noop, or otherwise make the cutover atomic. Please add a matrix test for plugin missing / disabled / enabled × project / user × generated / customized legacy artifacts, asserting that missing or disabled retains every working artifact.
| } | ||
|
|
||
| logger.success({ message: `Updated ${opts.label} at ${opts.path}` }); | ||
| logger.success({ |
There was a problem hiding this comment.
[P1] Do not render a missing plugin as a successful install. Each prepareHack*Plugin() returns missing when the user selects an integration but has not installed the native plugin; this fallthrough sends that result through logger.success, potentially immediately after legacy artifacts were removed. Handle missing, stale, and deprecated as warnings/failures here, and test interactive onboarding with the client executable absent, the marketplace absent, and the plugin disabled so the flow never claims the integration was updated.
| cursor-agent plugin marketplace add hack-dance/hack | ||
| ``` | ||
|
|
||
| Open `/plugin` in Cursor, choose the **Hack Dance** marketplace, and install **Hack**. Cursor bundles |
There was a problem hiding this comment.
[P2] Validate and document the current Cursor installation surface. Cursor's current official plugin documentation says to install in the editor with /add-plugin (or Settings → Plugins), while /plugin is not documented: https://cursor.com/changelog/2-5. Because this is the only recovery path printed after cleanup, an invalid slash command strands the user without either integration. Update the canonical guidance and all generated copies, then verify on current stable Cursor and Cursor Agent CLI that the marketplace is accepted, Hack installs/enables, and its rules, skills, and MCP server appear after a new session.
roodboi
left a comment
roodboi
left a comment
There was a problem hiding this comment.
Re-reviewed the two new commits (e21feee and 6808a21). The original unsafe-cleanup, missing-plugin success, Cursor guidance, and PR-description findings are substantially addressed.
What I verified on this head:
- focused plugin/cutover/setup suite: 36 tests, 217 assertions, 0 failures
- direct CLI TypeScript check and direct Ultracite check on all changed TS/test files
- CLI build and
git diff --check - the readiness matrix preserves legacy content while plugins are missing/disabled
Two false-green cases remain when the plugin is enabled but customized legacy content is preserved; see the inline comments. Please test enabled-plugin cutover with a customized primary artifact and customized MCP entry for all three clients, both scopes, through hack setup <client>, hack setup sync --all-scopes, automatic sync, and interactive hack init. Every path must preserve the customization, warn, and exit nonzero until the duplicate legacy integration is manually reconciled.
Hosted CI has not executed: run 243 is action_required with zero jobs, consistent with the fork workflow awaiting maintainer approval. Please approve/run CI and get it green. A live current-stable Cursor + Cursor Agent CLI marketplace/install/load check also remains outstanding before approval.
| let status: AgentPluginResult<TScope>["status"] = "absent"; | ||
| if (error) { | ||
| status = "error"; | ||
| } else if (results.some((result) => result.status === "removed")) { |
There was a problem hiding this comment.
[P1] Let preserved content outrank successful removals. In every enabled/customized matrix case there can be both outcomes—for example, a customized Cursor rule is preserved while its generated MCP sibling is removed. This branch selects removed first, so prepareNativeAgentPlugin() returns cleanupStatus: "removed"; hack setup sync exits 0 and automatic sync reports a full repair even though the customized legacy integration remains and the next check is still stale. Use precedence error > preserved > removed > absent, and add mixed removed + preserved assertions for customized rules/skills and customized MCP entries across all three clients and both scopes.
| return 1; | ||
| } | ||
|
|
||
| if (outcome === "unchanged") { |
There was a problem hiding this comment.
[P1] Treat a preserved cleanup as an incomplete cutover here and in interactive init. Even after the cleanup aggregator reports preserved, the plugin result remains status: "noop", cleanupStatus: "preserved"; this block only special-cases removed and therefore logs info and exits 0. src/commands/project.ts::logInstallResult() drops cleanupStatus entirely and has the same false-success behavior. Propagate the cleanup outcome, warn, and return nonzero for preserved. Please exercise enabled plugin + customized rule/skill and customized MCP through each direct hack setup <client> command and interactive hack init, asserting preserved bytes and non-success status.
roodboi
left a comment
roodboi
left a comment
There was a problem hiding this comment.
CI is now running, and the main test/typecheck/build, runtime-image, and secret-scan jobs pass. Docker E2E fails deterministically in agent-docs-sync because this PR changed missing native plugins to a non-success sync result without updating the existing E2E contract. See the inline comment for the required coverage. After updating it, run bun run test:e2e:local:docker and keep the earlier customized-artifact cutover cases in scope.
| return true; | ||
| } | ||
| return ( | ||
| entry.requiresReadyPlugin && |
There was a problem hiding this comment.
[P1] Update the agent-docs-sync E2E for this new failure contract. CI run 243 now reaches the Docker E2E job but fails at tests/e2e/scenarios/agent-docs-sync.ts:194-202: the fixture has no Cursor, Claude, or Codex client, so these branches make hack setup sync --all-scopes exit 1 while the unchanged scenario still requires exit 0 and then a clean --check. Please make the scenario assert the missing-plugin warning/non-success path while also proving the generated AGENTS/CLAUDE docs and deprecated Tickets artifacts are still reconciled; add a ready-plugin fixture (or split the scenario) to retain positive coverage for exit 0 followed by a clean check. Run bun run test:e2e:local:docker and get the hosted docker-e2e job green.
2 participants
Summary
Verification
Release Signal
Semantic Surfaces
Risks / Follow-up