Issue: duplicated terminal support code

[nborodikhin]

When Claude Code executes a child process it does so without allocating a PTY which revdiff, being an interactive program, requires.

There are workarounds for this issue that use a custom per-terminal approach to create a new pane or overlay and launch revdiff in a new environment. The issue is that there are many places that start revdiff and per-terminal workarounds have to be updated manually every time a new terminal support is implemented:

• revdiff diff review at .claude-plugin/skills/revdiff/scripts/launch-revdiff.sh |
• revdiff codex skill at plugins/codex/skills/revdiff/scripts/launch-revdiff.sh |
• revdiff plan review at plugins/revdiff-planning/scripts/launch-plan-review.sh |
• external projects, such as cc-thingz planning plugin

[umputun]

thx, but this is a deliberate design decision, not a bug.

Claude Code skills are supposed to be self-contained — unifying scripts across skills doesn't work reliably, at minimum you hit permission prompts on every invocation, and it also raises the question of how a skill with a "common" external dependency gets distributed and updated. Same story for the codex skill and the revdiff-planning plugin — each needs to stand on its own.

In practice what we have works well enough. Agents are aware of the duplication and CLAUDE.md has explicit instructions for keeping the copies in sync when terminal support changes. Not ideal, but the alternatives are worse.

Also per CONTRIBUTING.md, general ideas and brainstorming belong in Discussions, not Issues. Moving this to Discussions — pls continue there if you want to explore a concrete shape.

[nborodikhin]

unifying scripts across skills doesn't work reliably

I agree, but I actually thought of unification on a different level.

Here is my proposal:

• extract terminal hacks into a single script, say revdiff-claude
• ship the script alongside revdiff
• revdiff-claude would check for the terminal and choose appropriate execution method

This way there will the only source of truth

[umputun]

interesting idea. shipping an overlay launcher alongside revdiff (your overlaunch / revdiff-claude shapes) is a coherent proposal and I get the appeal, single source of truth for the terminal-detection ladder. but I'm not convinced it's worth doing and wanted to share the numbers I looked at before replying.

first framing point: I don't really see the duplication as a problem, it's an expected consequence of keeping each skill self-contained and decoupled. the two launcher copies are byte-identical by design and the cost is low.

most of the terminal-backend additions (tmux, kitty, wezterm, zellij, iTerm2, Ghostty, cmux, Kaku, emacs/vterm) happened during the initial development phase when we were catching up to what users actually run. the ongoing rate is much lower, I don't expect another wave of new terminals. the launcher-fix history (mktemp, EDITOR propagation, kitty matcher, cmux stdout, iTerm AppleScript quirks, tmux title version check, shell-quoting) has the same shape, mostly concentrated early and tapering off.

when those cross-file syncs do happen, they're a 5-minute mechanical copy between .claude-plugin and plugins/codex. an agent keeps them in sync today. if we want a belt-and-braces guarantee I can add a CI job that diffs them and fails on drift, that's a ~10 line change. launch-plan-review.sh isn't really duplication by the way, it has a different arg contract and output, it just happens to share the terminal-detection ladder.

what the shipped-alongside launcher would still cost, regardless of implementation shape (Go port, standalone shell script in releases/homebrew, or Go binary that embeds the shell and knows how to install it):

• another thing to ship, version, document, and keep aligned with revdiff's release cycle
• skill-side wrappers still exist for arg translation (--staged, --only, plan review mode, output-file handling). the wrapper gets thinner, it doesn't disappear
• if it's a Go port, every terminal quirk accrued in fix commits has to survive the port with tests. if it's a shipped shell script, the fast-iteration advantage of bash stays but we've still moved the fix cycle from "edit in the skill repo" to "edit, tag, release, wait for upgrade"

external projects like cc-thingz aren't really helped here either. whether they depend on a shipped-alongside launcher is a downstream packaging choice, and if they want their plugin to work without requiring users to also install the launcher separately, they'll still bundle their own copy.

bottom line: the "duplication" is expected decoupling, the ongoing maintenance cost is low and shrinking, and the proposed fix adds packaging + release coupling to the code path most likely to need quick patches. I'd rather add the CI diff check to formalize the "these two files stay in sync" invariant than extract a new tool.

happy to be wrong if there's a concrete pain point I'm missing, pls push back if so.

[nborodikhin]

My pain point is that I am not happy with an overlay approach since I don't have access to the conversation history while plan reviewer window is up. I often ask Claude questions when reviewing plains, but I don't always have a chance to see answers before it tries to exit plan mode again.

I would really like to have revdiff launched in a separate terminal window, but with the current approach it is not possible since the way it is launched is baked into Claude plugin. My only option is to install local modified plugin which is a bit of work to keep synced with the upstream.

Having the launcher on the revdiff level (/usr/bin in case of Linux) instead of on Claude skill/plugin level would make customization trivial: I could create my own ~/bin/revdiff-claude and use it to open another terminal.

what the shipped-alongside launcher would still cost, regardless of implementation shape ...:

• another thing to ship, version, document, and keep aligned with revdiff's release cycle

What I had in mind is that launcher could be shipped with revdiff itself, in the same homebrew/deb package. For example:

$ dpkg -L revdiff
/usr
/usr/bin
/usr/bin/revdiff
/usr/bin/revdiff-claude

Then shipping, versioning and alignment are not such a big problems. I agree that this is somewhat unnatural breakdown - part of Claude Code support is shipped with revdiff itself - but that is the most convenient structure I came up with that both allows customization and avoids code duplication.

[umputun]

Two separate threads here, let me address each.

On the "asking questions during review" pain: there's already a classify/explain loop for this, you may not have noticed it because the trigger is in the annotation text. If an annotation starts with explain/what is/how does/clarify — or contains ?? anywhere (language-neutral shortcut, added in 0.7.6) — the skill treats it as a question instead of a directive. It writes the answer to a temp markdown file and re-opens revdiff with --only=<answer>.md so you read it with TOC and scrollback, and can add follow-up annotations to refine. The loop only exits when you close revdiff without annotations. See Step 3.5 in .claude-plugin/skills/revdiff/SKILL.md and the codex revdiff-plan skill.

One concrete gap: the Claude-side plan-review uses a PreToolUse hook on ExitPlanMode, not a skill. The hook's deny text just tells Claude "adjust the plan to address each annotation". It doesn't reference the classify/explain loop, so questions on plans get treated as plan-edits. Probably fixable by extending the hook's deny-reason text to match what the codex revdiff-plan skill already says. Doesn't seem to be a big change. I'll track it separately.

On the "launch in a separate terminal" pain: the launcher doesn't have to move into the revdiff package for that. A Claude-side alternative is an override chain — not implemented here today, but the planning plugin in cc-thingz uses exactly that pattern for prompts and agents: .claude/<plugin>/... → $CLAUDE_PLUGIN_DATA/<plugin>/... → bundled default, first-match wins. Straightforward to adopt for the launcher scripts if we decide to go this route. You'd drop your own .claude/revdiff-planning/scripts/launch-plan-review.sh that opens a new kitty tab / tmux window / whatever, and the skill invokes that instead of the bundled overlay. No fork, no separate plugin to keep in sync. Same mechanism would work for the /revdiff:revdiff launcher too.

Would an override chain on the launcher scripts cover what you're after?

[nborodikhin]

On the "asking questions during review" pain

Thanks for pointing to the question loop I missed that. Probably because I am mostly using revdiff for plan review.

On the "launch in a separate terminal" pain

Yes, resolving launcher script via CLAUDE_PLUGIN_DATA fits my needs perfectly.

[umputun]

Shipped in #126 (revdiff plugin 0.8.0, revdiff-planning plugin 0.3.0).

The chain ended up two layers (user → bundled) instead of three. The project layer (.claude/<plugin-namespace>/scripts/) was dropped after security review — the revdiff-planning hook fires automatically on every ExitPlanMode in any repo, so a repo-controlled executable layer would auto-execute on routine actions without per-repo opt-in. The ${CLAUDE_PLUGIN_DATA} user layer covers the use cases discussed here either way.

To customize the plan-review launcher (e.g. open in a separate kitty window instead of an overlay):

mkdir -p "${CLAUDE_PLUGIN_DATA}/scripts"
cp "${CLAUDE_PLUGIN_ROOT}/scripts/launch-plan-review.sh" "${CLAUDE_PLUGIN_DATA}/scripts/launch-plan-review.sh"
chmod +x "${CLAUDE_PLUGIN_DATA}/scripts/launch-plan-review.sh"
# edit "${CLAUDE_PLUGIN_DATA}/scripts/launch-plan-review.sh" to taste

Same pattern for the diff-review skill: replace ${CLAUDE_PLUGIN_ROOT} with ${CLAUDE_SKILL_DIR} and launch-plan-review.sh with launch-revdiff.sh.

Override file must be executable. A non-executable file in the user layer is treated as absent and the resolver falls through to the bundled default. Full details in plugins/revdiff-planning/README.md and .claude-plugin/skills/revdiff/references/install.md.

The separate plan-review hook gap (PreToolUse deny text not referencing the classify/explain ?? loop) is still tracked separately and not part of this change.

[nborodikhin]

Thanks, it works as expected with my own scripts at (for the official plugins):

• revdiff-planning: ~/.claude/plugins/data/revdiff-planning-revdiff/scripts/launch-plan-review.sh
• revdiff: ~/.claude/plugins/data/revdiff-revdiff/scripts/launch-revdiff.sh