Skip to content

feat(direnv): add direnv plugin for per-directory env loading - #736

Open
nsheaps wants to merge 2 commits into
mainfrom
add-direnv-plugin
Open

nsheaps wants to merge 2 commits into
mainfrom
add-direnv-plugin

Conversation

@nsheaps

@nsheaps nsheaps commented Aug 8, 2026

Copy link
Copy Markdown
Owner

Summary

Adds a new direnv plugin, mirroring the mise and github-app plugins' structure and conventions.

  • SessionStart hook (install-direnv.sh): resolves a direnv binary, runs direnv allow on the project's .envrc (and any git worktrees), then computes the .envrc diff once via direnv export bash and writes it as static export/unset statements to a runtime file sourced (once, idempotently) via CLAUDE_ENV_FILE.
  • PreToolUse hook (direnv-check.sh, matcher Bash): before each Bash call, checks (debounced, default 2s via checkIntervalSeconds) whether the nearest .envrc's fingerprint (path + mtime + size) changed, and if so re-runs the export and rewrites the runtime file in place — so vars removed from .envrc are correctly dropped, not just left stale.
  • Deliberately does not install direnv hook bash (the PROMPT_COMMAND-style shell hook) — same rationale documented at length in mise's install-mise.sh: stderr/stdout pollution on every command, eval-injection risk, unnecessary cd/pushd/popd overrides in a non-interactive agent shell.
  • Depends on shared-lib (tool-install.sh, plugin-config-read.sh, hook-logging.sh, env-file.sh) exactly like mise/github-app/1pass.
  • Includes direnv.settings.yaml, README.md, SPEC.md, .release-it.js (copied verbatim from mise's pattern), and a manual-operation skills/direnv/SKILL.md.
  • Registered in .claude-plugin/marketplace.json (alphabetical position; CD bumps versions on merge per this repo's versioning.md).

On "claude-utils agent-plugin helpers"

Correction from an earlier version of this PR description: I initially reported that no such thing existed in nsheaps/claude-utils, based on investigating a local checkout that turned out to be stale. That was wrong. packages/agent-plugin/ and packages/agent-hook/ are real, documented in that repo's README ("For plugin and hook authors"), and this PR now integrates with both:

  • agent-pluginagent-plugin ensure-dependency <cli> <mise-package> installs a missing CLI via mise use -g <mise-package>, gated on that plugin's own autoInstall setting (read from .claude/settings.<plugin>.yaml, defaulting to false). Also provides leveled logging (agent-plugin log-info/log-warn/etc, threshold via $AGENT_PLUGIN_LOG_LEVEL) and per-plugin settings read/write.
  • agent-hookagent-hook export-input parses a hook's JSON stdin into shell exports (HOOK_TOOL_NAME, HOOK_TOOL_INPUT_COMMAND, etc.) via eval, replacing hand-rolled jq parsing. Also provides allow/deny/halt/warn-user/post for emitting a hook's JSON decision.

How I verified this for real (not just reading source): built both binaries from a clean origin/main checkout in an isolated worktree (bun install && bun run build), confirmed mise use direnv@latest resolves through the registry's aqua:direnv/direnv backend and installs direnv v2.37.1, then ran install-direnv.sh and direnv-check.sh end-to-end in scratch project directories with the built binaries on PATH (and separately with them absent) to exercise every branch — see "Test plan" below.

Where I landed: enhancement-if-present, not a hard dependency. agent-plugin's settings file (.claude/settings.direnv.yaml) is a different file from this plugin's own direnv.settings.yaml/plugins.settings.yaml convention (the one shared-lib's plugin-config-read.sh reads, and what every other ai-mktpl plugin uses). Requiring both to be configured to get auto-install working would be a real regression, and no other ai-mktpl plugin (mise, github-app, 1pass) depends on claude-utils yet — brew's claude-utils formula on this machine is still 0.10.0 and doesn't even ship these binaries. So:

  • direnv.settings.yaml's autoInstall stays the single source of truth for whether to install at all.
  • install-direnv.sh tries agent-plugin ensure-dependency direnv "direnv@latest" first when agent-plugin is on PATH, and falls back to the direct GitHub-release download on any failure (agent-plugin absent, its own autoInstall declined, no mise, or the mise install failing) — zero behavior change for the 100% of users who don't have claude-utils installed today.
  • Logging routes through agent-plugin log-* when present, always also through this repo's hook-logging.sh lifecycle (SessionStart's summary output and failure diagnostics depend on it regardless).
  • direnv-check.sh uses agent-hook export-input when present, falling back to jq. It never calls agent-hook allow/deny for the "nothing changed" case — per this repo's own hook-output-patterns.md, a PreToolUse hook with no opinion must emit nothing, not an explicit allow (which would bypass the permission system for every Bash call). That rule takes precedence over reaching for every agent-hook feature just because it exists.
  • The debounce/throttle placeholder now links to the confirmed in-flight nsheaps/claude-utils#436 (branch feature/agent-hook-throttle), which generalizes this exact pattern into a reusable, sourceable helper. Swapping to it once merged is a one-function change (should_check/record_check are isolated for exactly this).

Full rationale (with more detail than fits here) is in the plugin's README.md under "claude-utils integration".

Test plan

  • plugin.json/hooks.json/marketplace.json JSON syntax and direnv.settings.yaml YAML syntax validated
  • claude plugin validate plugins/direnv/.claude-plugin/plugin.json — passes
  • mise run validate-marketplace — passes
  • shellcheck on all scripts — only info-level SC1091/SC2317/SC2034 findings, matching the same class of findings shellcheck reports on existing plugins (e.g. mise's install script)
  • prettier --check on all files — passes
  • install-direnv.sh end-to-end in a scratch project dir with a real .envrc, without claude-utils on PATH — confirmed the (unchanged) direct-download install path, direnv-env runtime file, and CLAUDE_ENV_FILE wiring are correct
  • install-direnv.sh end-to-end with locally-built agent-plugin/agent-hook on PATH and direnv deliberately kept off PATH (isolated MISE_DATA_DIR/MISE_CONFIG_DIR so nothing touched this machine's real mise state) — confirmed agent-plugin ensure-dependency installs direnv via mise use -g direnv@latest for real, resolves the binary via mise which when it isn't yet on the current process's PATH, and wires it into CLAUDE_ENV_FILE correctly
  • Also exercised the fallback: forced agent-plugin's own ensure-dependency to fail (untrusted mise config in the ambient directory) and confirmed install-direnv.sh correctly fell through to the direct GitHub-release download rather than aborting
  • direnv-check.sh — confirmed: (a) no-op when nothing changed (silent, exit 0, no stdout), (b) re-exports and correctly drops a var removed from .envrc (overwrite, not append), (c) debounce window actually throttles a same-second re-check, (d) cd subdir && prefix parsing correctly resolves a nested .envrc, (e) with agent-hook on PATH, agent-hook export-input correctly parses HOOK_TOOL_NAME/HOOK_TOOL_INPUT_COMMAND and drives the same directory-resolution and re-export logic, (f) with agent-hook absent, the jq fallback still works identically
  • Not tested: CI's full mise run check/mise run validate pipeline (local mise run setup hit an unrelated aqua:cli/cli attestation/network failure installing this repo's own dev toolchain — pre-existing, unrelated to this change)
  • Not tested: a live Claude Code session invoking these hooks through the real harness (SessionStart/PreToolUse wiring), since I don't have access to spin one up standalone
  • Not tested against a real Homebrew-installed claude-utils build of agent-plugin/agent-hook — the current brew formula (0.10.0) predates these binaries, so verification used binaries built directly from origin/main of that repo in an isolated worktree

Co-Authored-By: Claude Code (User Settings, in: /Users/nathan.heaps/src/nsheaps/ai-mktpl) noreply@anthropic.com

https://claude.ai/code/session_01GdW3bEd5bo54U5eKm3njWe

Installs direnv and exports .envrc into CLAUDE_ENV_FILE once at session
start (static export/unset statements, not a live shell hook), then
re-exports via a debounced PreToolUse(Bash) check whenever the .envrc
fingerprint changes. Mirrors mise's install-mise.sh discipline around
avoiding `direnv hook bash`'s PROMPT_COMMAND-style eval loop, and
github-app's PreToolUse debounce pattern.

Co-Authored-By: Claude Code (User Settings, in: /Users/nathan.heaps/src/nsheaps/ai-mktpl) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GdW3bEd5bo54U5eKm3njWe
@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Plugin Version Preview

Preview only — plugin versions and marketplace.json are bumped automatically on merge to main, not in this PR. Manual bumps to higher versions are preserved. See the file annotations for the pending change on each plugin.json.

Plugin Base Current Action
direnv 0.0.0 0.1.0 Already bumped

@nsheaps
nsheaps marked this pull request as ready for review August 8, 2026 21:44
henry-nsheaps[bot]

This comment was marked as outdated.

henry-nsheaps[bot]

This comment was marked as outdated.

Wires in nsheaps/claude-utils' agent-plugin and agent-hook binaries where
available, per the real (verified against a fresh build of packages/agent-plugin
and packages/agent-hook, and a live end-to-end scratch-dir test) contract in
that repo's README:

- install-direnv.sh: tries `agent-plugin ensure-dependency direnv "direnv@latest"`
  (mise-based install; verified the mise registry maps `direnv` to
  aqua:direnv/direnv and `mise use direnv@latest` installs v2.37.1 successfully)
  before falling back to the existing direct GitHub-release download. Also
  routes log messages through `agent-plugin log-*` alongside shared-lib's
  hook-logging.sh.
- direnv-check.sh: uses `agent-hook export-input` instead of hand-rolled jq
  parsing when agent-hook is on PATH, falling back to jq otherwise. Does NOT
  use agent-hook's allow/deny for the "nothing changed" case, per this repo's
  own hook-output-patterns.md (no opinion = no output, never an explicit
  allow).

Both are treated as enhancements, not hard dependencies: agent-plugin's own
settings file (.claude/settings.direnv.yaml) is separate from this plugin's
existing plugins.settings.yaml convention, and no other ai-mktpl plugin
depends on claude-utils yet. direnv.settings.yaml's autoInstall remains the
single source of truth for whether to install; agent-plugin/mise is only an
alternate mechanism.

Debounce logic's TODO now links to the confirmed in-flight
nsheaps/claude-utils#436 (agent-hook-throttle).

Co-Authored-By: Claude Code (User Settings, in: /Users/nathan.heaps/src/nsheaps/ai-mktpl) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GdW3bEd5bo54U5eKm3njWe

@henry-nsheaps henry-nsheaps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ Some changes need to be made — the two blocking findings from the last review are untouched, and the new commit adds a second code path rather than a fix

❌ Still open, re-reproduced on this revision: editing .envrc mid-session wipes every exported var and latches the fingerprint so it never retries
❌ Still open: PATH_add emits an absolute export PATH= that clobbers mise's and tool_ensure_path's PATH
⚠️ New: the agent-plugin install path logs an ERROR every session on the default configuration, for a non-error
⚠️ New: installToProject: true is silently ignored by the agent-plugin path, which does a global mise use -g
⚠️ Still open: .envrc secrets land at 0644 before chmod 600; DIRENV_* vars baked into every shell make SKILL.md's Verify steps lie
❔ New: the claude-utils adoption is dual-path with mandatory fallbacks and no tests — better as its own PR after the fixes
✅ The integration itself is correctly built: eval-safe parser, stderr-only logger, real upstream issue link — all verified against claude-utils source

🖱️ Click to expand for full details

State of the previous review

aaddf71 changed README.md, direnv-check.sh, install-direnv.sh and lib/direnv-export.sh, but none of the ten open threads were addressed. I re-ran the reproductions against direnv v2.37.1 on this exact revision rather than assuming they still held:

Finding Thread Status on aaddf71
❌ Blocked .envrc wipes the env lib/direnv-export.sh:247 Re-reproduced. Editing an allowed .envrcdirenv export bash exits 1 with is blocked; lines 245–247 blank the output and line 259 overwrites the snapshot with just its header.
❌ Re-export path never re-allows direnv-check.sh Unchanged, and line 194 still commits the new fingerprint after the failed export, so it never retries. GitHub has marked this thread outdated because the file moved underneath it — the code it describes is still there.
PATH_add clobbers PATH lib/direnv-export.sh:259 Re-reproduced. PATH_add ./bin renders as export PATH=$'/tmp/dproj2/bin:/opt/other-plugin/bin:/usr/bin:/bin' — a whole-PATH absolute assignment, sourced last.
⚠️ chmod 600 after create lib/direnv-export.sh:261 Unchanged.
⚠️ DIRENV_* baked into every shell SKILL.md:110 Unchanged; the export output above still carries DIRENV_DIFF/WATCHES/DIR/FILE.
❔ Docstring vs none sentinel lib/direnv-export.sh:199 Unchanged.
❔ README overstates the mise parallel README.md:104 Unchanged.
❔ No tests lib/direnv-export.sh:1 Unchanged — and this commit added two more branches to the file.
PreToolUse table in plugins/CLAUDE.md hooks.json:26 Unanswered.
❔ Hand-edited marketplace.json marketplace.json:322 Unanswered; entry still present at lines 339–358.

One secondary detail worth repeating because it makes the first finding self-sustaining: direnv allow leaves .envrc's mtime and size identical (re-verified: 1786411988:19 before and after). So once the fingerprint is latched on a blocked file, the recovery SKILL.md documents cannot move it.

The new commit

I read claude-utils' source directly rather than the README, and the integration is built correctly:

  • agent-hook export-input is genuinely eval-safe. export-input.ts wraps values in single quotes using the '\'' close/escape/reopen sequence, and rebuilds variable names character-by-character from [A-Z0-9_] — rejecting anything that can't form an identifier — rather than trying to escape the left-hand side of an assignment. I have no injection finding against direnv-check.sh:81, and the file's own security commentary is accurate.
  • agent-plugin's logger is stderr-only (log.ts: "Everything goes to stderr, never stdout"), so it can't corrupt hook JSON.
  • AGENT_PLUGIN_NAME is a documented equivalent of --plugin (plugin-name.ts).
  • The TODO(agent-hook-throttle) now links nsheaps/claude-utils#436, which is a real, open PR in a public repo. That's a concrete improvement and closes one of my round-1 follow-ups.

The problems are at the seams, not in the parsing:

  • install-direnv.sh:137ensure-dependency's autoInstall defaults to false upstream, and the decline branch logs at error level. Since the agent-plugin block is only reached when direnv is already known to be absent, every user with claude-utils installed and no .claude/settings.direnv.yaml gets an unfilterable ERROR [direnv] direnv is not installed… on every session start, immediately before the curl fallback succeeds. Same thread also covers the unredirected stdout on a function whose stdout is its return channel; one suggestion fixes both.
  • direnv.settings.yaml:18ensure-dependency installs with mise use -g. That's a persistent rewrite of ~/.config/mise/config.toml affecting every project on the machine, taken while this plugin's own installToProject: true says project-local. Either gate the path on that setting or document the divergence.
  • lib/direnv-export.sh:26 — scope. Both new branches keep mandatory fallbacks, so nothing was replaced; the agent-hook branch substitutes for two lines of jq that must stay anyway, and the agent-plugin branch is declined-by-default upstream. Net: ~120 lines and two runtime forks added to a file that still has no tests, in a PR whose core loop has two unfixed ❌.
  • direnv-check.sh:77 — nit only: the payload travels as argv, so it's visible in ps. Matches upstream's documented usage, so it's an upstream ask.

What's solid

Everything I praised last time still holds and isn't repeated here. New this round: the README.md section on the integration is unusually good — it states the fallback behaviour, names the settings-file divergence as the reason it's an enhancement, and links the upstream throttle PR. The failure shape on direnv-check.sh:77–80 (2>/dev/null || true plus a non-empty guard) degrades to jq cleanly for a malformed payload, an ARG_MAX overflow, or an older claude-utils. The commit message is precise about what was and wasn't verified.

Why 68% / 76% / 72% / 92%

  • Quality 68 (was 72) — down because two ❌ in the core loop are now two revisions old while the diff grew, and the new code introduces a default-path ERROR log and a settings-vs-behaviour conflict. The scaffolding and documentation remain strong, which is what keeps it this high.
  • Security 76 (was 78) — the chmod window is unchanged; new: a global mise config mutation the settings file doesn't disclose, and payload-in-argv. Offset upward by having verified the eval path is safe rather than assuming.
  • Simplicity 72 (was 88) — the honest cost of two conditional integrations that can't remove the code they sit in front of.
  • Confidence 92 (was 90) — up: I re-reproduced all three direnv behaviours against the shipped v2.37.1 binary on this revision, and read claude-utils' log.ts, dependencies.ts, cli.ts, plugin-name.ts and export-input.ts for the exit-code, log-stream and escaping contracts instead of trusting the commit message. Residual 8%: still no live Claude Code session, so CLAUDE_ENV_FILE source ordering across parallel SessionStart hooks stays reasoned rather than observed.

Recommended follow-ups (non-blocking):

  • Raise the payload-on-stdin request with nsheaps/claude-utils so export-input doesn't require argv, and consider asking upstream to log the declined outcome at debug/info rather than error — a caller with its own fallback shouldn't have to swallow stderr to stay quiet.
  • Add checksum verification to shared-lib's tool-install.sh download helper so mise, direnv and future plugins all get it — direnv publishes SHA256SUMS per release. (Repo-wide, not introduced here.)
  • Once nsheaps/claude-utils#436 ships, do the planned should_check/record_check swap and drop the duplicated debounce in github-app.

To improve future reviews on this repo I'd benefit from:

  • A mise run test task wired to plugins/*/tests/*.test.sh, so plugin test coverage is a checkable convention rather than a per-plugin judgement call.

Notes:12345

Footnotes

  1. Workflow Run: https://github.com/nsheaps/ai-mktpl/actions/runs/31449463970/attempts/1

  2. PR: nsheaps/ai-mktpl#736

  3. direnv v2.37.1 release — the binary used to re-reproduce the blocked-.envrc exit code, the direnv allow mtime behaviour, and the PATH_add export shape

  4. nsheaps/claude-utilspackages/agent-plugin/src/{log,dependencies,cli,plugin-name}.ts and packages/agent-hook/src/export-input.ts, read for the log-stream, exit-code, plugin-name and shell-escaping contracts

  5. direnv stdlib — PATH_add

# claude-utils or configured .claude/settings.direnv.yaml.
if [ -n "$(direnv_agent_plugin_bin)" ]; then
local via_agent_plugin
if via_agent_plugin="$(direnv_try_ensure_dependency_via_agent_plugin)" && [ -n "$via_agent_plugin" ]; then

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ On the default path this logs an ERROR on every SessionStart for something that is not an error — and ensure-dependency's stdout is unguarded inside a function whose stdout is the return channel.

I read claude-utils' actual source rather than relying on the README. Two things follow from it:

1. The decline is logged at error level. packages/agent-plugin/src/dependencies.ts defaults autoInstall to false (readAutoInstall returns false for undefined), and the decline branch is:

if (!deps.autoInstall) {
  deps.log(
    "error",
    `${cli} is not installed. Install it with \`mise use -g ${misePackage}\`, or enable ` +
      `automatic installation with \`agent-plugin settings autoInstall true\`.`,
  );
  return { status: "declined", cli, misePackage };
}

Note the ordering in resolve_direnv_bin: this block is only reached when tool_is_available direnv was already false (line 118), so hasCommand("direnv") is false too and the decline branch is the one that runs. For any user who installs claude-utils but has never created .claude/settings.direnv.yaml — i.e. the default for everyone — every session start emits

ERROR [direnv] direnv is not installed. Install it with `mise use -g direnv@latest`, or enable automatic installation with `agent-plugin settings autoInstall true`.

…immediately followed by this plugin successfully installing direnv via the curl fallback. error is above the default info threshold, so it is never filtered. That is a scary, actionable-looking message telling the user to fix something that isn't broken, and it directly undercuts the "enhancement, not a hard dependency" framing the surrounding comment argues for.

2. $(direnv_try_ensure_dependency_via_agent_plugin) captures that function's stdout as the direnv path. agent-plugin's logger writes to stderr only today (log.ts: "Everything goes to stderr, never stdout"), so this is currently latent rather than broken — but the invocation on lib/direnv-export.sh:109 leaves stdout unredirected, so any future stdout from ensure-dependency becomes part of $via_agent_plugin and then part of $direnv_bin. Every other command in that function is careful about this (mise which direnv 2>/dev/null); this one isn't.

Both are fixed by capturing stderr and discarding stdout at the call site in lib/direnv-export.sh:

  local ap_err
  if ! ap_err="$(AGENT_PLUGIN_NAME="direnv" "$ap_bin" ensure-dependency direnv "direnv@latest" 2>&1 >/dev/null)"; then
    # Declined (claude-utils' own autoInstall setting is unset/false — its
    # default), no mise, or the mise install itself failed. All three are
    # normal here: this plugin's own curl fallback handles every one of them,
    # so agent-plugin's error-level decline message must NOT reach the session
    # verbatim. Keep it as a debug breadcrumb instead.
    [ -n "$ap_err" ] && direnv_log "debug" "agent-plugin ensure-dependency did not provide direnv: ${ap_err}"
    return 1
  fi

(2>&1 >/dev/null — stderr to the capture, stdout to /dev/null — the order matters.)

Separately, and much smaller: the doc comment on direnv_log (lib/direnv-export.sh:71) says it routes through agent-plugin --plugin direnv log-<level>, but the code sets AGENT_PLUGIN_NAME=direnv instead. Both are supported (plugin-name.ts resolves --plugin then AGENT_PLUGIN_NAME), so this is a comment fix, not a behaviour one.

Comment on lines +18 to +20
# Install direnv to $CLAUDE_PROJECT_DIR/bin/.local. Only used when
# autoInstall is true.
installToProject: true

@henry-nsheaps henry-nsheaps Bot Aug 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ installToProject is silently ignored when the agent-plugin path wins, and that path mutates the user's global mise config.

This setting documents "Install direnv to $CLAUDE_PROJECT_DIR/bin/.local", and tool_resolve_install_dir in shared-lib honours it for the curl fallback. But agent-plugin ensure-dependency (tried first, at install-direnv.sh:137) installs via mise use -g — global. Its own help text is explicit:

ensure-dependency <cli> <mise-package> — Make sure <cli> is runnable, installing <mise-package> via mise use -g

So on a machine with claude-utils + mise + .claude/settings.direnv.yaml: autoInstall: true, this plugin persistently rewrites ~/.config/mise/config.toml to pin direnv@latest globally — a change that outlives the session, affects every other project on the machine, and happens even though the user asked for a project-local install here. INSTALL_DIR ends up unused, and resolve_direnv_bin returns a path under mise's install root instead.

That's a bigger side effect than anything else this plugin does, and it isn't mentioned in README.md's claude-utils section, SPEC.md, or here. Two ways out, either is fine:

  • Respect the setting — skip the agent-plugin path when installToProject is true (the default), since a global install is by definition not a project-local one; or
  • Document it — say plainly in this comment and in the README that the agent-plugin mechanism is a global mise use -g install and ignores installToProject.

If you keep the current ordering, this comment at minimum needs to stop claiming the install location:

Suggested change
# Install direnv to $CLAUDE_PROJECT_DIR/bin/.local. Only used when
# autoInstall is true.
installToProject: true
# Where the curl-based fallback installs direnv. When true, installs to
# $CLAUDE_PROJECT_DIR/bin/.local. Only used when autoInstall is true.
#
# NOTE: this does NOT apply to the claude-utils `agent-plugin
# ensure-dependency` path (tried first when `agent-plugin` is on PATH),
# which installs via `mise use -g` — a GLOBAL install that rewrites
# ~/.config/mise/config.toml and is not scoped to this project.
installToProject: true

Related: the log-noise and stdout-capture issues on the same agent-plugin call are in the sibling thread at #736 (comment).

Comment on lines +26 to +47
# --- claude-utils agent-plugin / agent-hook integration (enhancement-if-present) ---
#
# nsheaps/claude-utils ships two native binaries meant for exactly this kind
# of plugin (see that repo's README, "For plugin and hook authors"):
# agent-plugin — leveled logging + per-plugin settings + `ensure-dependency`
# agent-hook — parses a hook's JSON stdin into shell vars, emits decisions
# Installed via `brew install nsheaps/devsetup/claude-utils`.
#
# No other ai-mktpl plugin depends on these yet (mise/github-app/1pass predate
# them and use this repo's own shared-lib bash convention), and their settings
# story is genuinely different: agent-plugin reads a per-plugin autoInstall
# flag from `.claude/settings.direnv.yaml`, NOT ai-mktpl's own
# `plugins.settings.yaml` (read via shared-lib's plugin-config-read.sh) that
# this plugin's `direnv.settings.yaml` documents and every other ai-mktpl
# plugin's settings live in. Requiring users to configure autoInstall twice,
# in two different files, to get auto-install working would be a regression.
#
# So this plugin treats agent-plugin/agent-hook as an ENHANCEMENT, not a hard
# dependency: used opportunistically when on PATH, with ai-mktpl's own
# shared-lib-based logic remaining the source of truth and the fallback in
# every case. See install-direnv.sh and direnv-check.sh for where each is
# used.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

KISS/YAGNI: this integration doubles the number of code paths in a plugin that still has zero tests, and it does not change what any user gets today. Should it be its own PR, landed after the correctness fixes?

To be clear about what I verified rather than assumed — I read the upstream source, and the engineering here is sound on its own terms:

  • agent-hook's export-input really is eval-safe. packages/agent-hook/src/export-input.ts single-quotes values with the '\'' close/escape/reopen sequence and rebuilds key names from [A-Z0-9_] rather than escaping them, rejecting anything that can't form an identifier. I have no injection finding against the eval on direnv-check.sh:81.
  • agent-plugin's logger is stderr-only (log.ts), so it can't corrupt a hook's JSON stdout.
  • AGENT_PLUGIN_NAME is a documented equivalent of --plugin (plugin-name.ts).
  • ✅ The TODO(agent-hook-throttle) now points at nsheaps/claude-utils#436, which is real and open — that's a genuine improvement over the bare TODO in the previous revision, and it closes out one of my round-1 follow-ups.

The concern is scope and cost, not correctness of the integration:

  1. Every path needs a full fallback anyway, by design ("enhancement, not a hard dependency"). So the jq parsing, the curl download, and the hook_log lifecycle all have to stay and keep working. Nothing was removed; ~120 lines and two new runtime branches were added on top.
  2. The agent-hook branch replaces two lines of jq. direnv-check.sh needs exactly tool_name and tool_input.command. The jq that does that has to remain as the fallback regardless, so the branch buys no capability — it only adds a second way for the same two variables to get set, one of which is reachable only on machines that have claude-utils installed. That's the branch least likely to be exercised and most likely to rot.
  3. The agent-plugin install branch is declined-by-default upstream (autoInstall defaults to false in dependencies.ts), so for essentially every user it runs, logs, fails, and falls through — see the thread on install-direnv.sh:137 for the error-level noise that produces, and the one on direnv.settings.yaml:18 for the installToProject conflict.
  4. There are still no tests, and this commit added the two branches whose interaction is hardest to reason about — direnv_log now fans out to two sinks, and resolve_direnv_bin now has four ways to produce a binary path.

Meanwhile the three findings from the previous review — all of which I re-reproduced against direnv v2.37.1 on this exact revision and none of which this commit touches — sit in the plugin's core loop. My suggestion is to land the correctness fixes plus a tests/direnv-export.test.sh first (this file is pure and stub-friendly, per the thread at #736 (comment)), and bring the claude-utils adoption back as a follow-up PR — at which point it can be reviewed on its own merits, and a test suite already exists to cover both branches of each fork.

Not a blocker on its own; the blockers are the unaddressed round-1 threads. But if the integration stays in this PR, both branches need the fixes in the sibling threads before merge.

AGENT_HOOK_BIN="$(direnv_agent_hook_bin)"
AGENT_HOOK_EXPORTS=""
if [ -n "$AGENT_HOOK_BIN" ]; then
AGENT_HOOK_EXPORTS="$("$AGENT_HOOK_BIN" export-input "$INPUT" 2>/dev/null || true)"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit (non-blocking, and it matches upstream's own documented usage): passing the whole hook payload as an argv element puts it in the process table for the lifetime of the call, so any other local user's ps -ef sees the full Bash command the agent is about to run — which for this agent routinely includes tokens, gh invocations, and .env manipulation. The jq fallback below doesn't have this property (echo "$INPUT" | jq keeps it in a pipe).

export-input.ts's own doc block shows eval "$(agent-hook export-input "$HOOK_INPUT")", so you're following the contract as written — this is really an upstream request (accept the payload on stdin when no argument is given) rather than something to fix here. Worth raising on nsheaps/claude-utils alongside #436.

The 2>/dev/null || true + non-empty check on line 77–80 is a good failure shape, by the way: a malformed payload, an ARG_MAX overflow on a very large command, or a claude-utils version that doesn't have export-input all land silently in the jq branch rather than aborting the hook. ✅

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.

1 participant