fix(github-app): use per-agent config directory via AGENT_NAME#410
fix(github-app): use per-agent config directory via AGENT_NAME#410jack-nsheaps[bot] wants to merge 5 commits intomainfrom
Conversation
Replace hardcoded ~/.config/agent/ paths with ~/.agents/${AGENT_NAME}/.config/
to support multiple agents on the same machine. Falls back to .default
when AGENT_NAME is not set.
Fixes: PEM keys, tokens, env files, and debounce files now go to
agent-specific directories, preventing agents from overwriting each
other's GitHub App credentials.
Breaking: agents must set AGENT_NAME env var (or accept .default path).
Plugin Version StatusVersions are auto-bumped in PRs. Manual bumps to higher versions are preserved.
|
| # --- Configuration --- | ||
|
|
||
| DEBOUNCE_FILE="${HOME}/.config/agent/github-app-last-check" | ||
| _AGENT_CONFIG_DIR="${HOME}/.agents/${AGENT_NAME:-.default}/.config" |
There was a problem hiding this comment.
Default agent name outside here so it's consistent and don't use .default, use "_UNKNOWN"
We want to drop the dot prefix for your names soon anyway
There was a problem hiding this comment.
Addressed in 8eaca8f — the default is now centralized in lib/agent-paths.sh:11 (AGENT_CONFIG_DIR="${HOME}/.agents/${AGENT_NAME:-_UNKNOWN}/.config") and sourced by all hook/bin scripts. One-line change to drop the dot prefix when ready.
… scripts - Create lib/agent-paths.sh as single source of truth for AGENT_CONFIG_DIR - Use _UNKNOWN instead of .default as fallback agent name - Remove eager mkdir -p from token-check.sh (let it happen lazily) - Update all 3 bin/ scripts to source agent-paths.sh - All files now derive from AGENT_CONFIG_DIR, no hardcoded paths
…agent paths
- Replace all ~/.config/agent/ references with ~/.agents/${AGENT_NAME}/.config/
across README.md, settings YAML, SKILL.md, and token-refresh-spec.md
- Fix bin/ scripts to use BASH_SOURCE[0] with readlink -f for symlink safety
- Addresses Henry's review feedback on PR #410
| # shellcheck source=../lib/agent-paths.sh | ||
| _self="${BASH_SOURCE[0]}" | ||
| while [ -L "$_self" ]; do _self="$(readlink -f "$_self")"; done | ||
| source "$(cd "$(dirname "$_self")/.." && pwd)/lib/agent-paths.sh" |
There was a problem hiding this comment.
BASH_SOURCE[0] + readlink -f resolution here makes the agent-paths.sh source call symlink-safe, but:
- L97
PLUGIN_DIR="$(cd "$(dirname "$0")/.." && pwd)"— used to sourcetoken-utils.shat L98. - L122
script_dir="$(cd "$(dirname "$0")" && pwd)"— used to invokegenerate-token.shat L130.
…both still use dirname "$0". If this script is ever invoked via a symlink, it succeeds sourcing agent-paths.sh here but then fails a few lines later with an unhelpful "No such file or directory" on token-utils.sh — a strictly worse failure mode than the pre-PR state (where all three would fail at the same spot).
Two reasonable options:
A — resolve once, reuse everywhere. Capture PLUGIN_DIR from the already-resolved $_self, then use ${PLUGIN_DIR}/... at L97, L98, L122, L130.
| source "$(cd "$(dirname "$_self")/.." && pwd)/lib/agent-paths.sh" | |
| # shellcheck source=../lib/agent-paths.sh | |
| _self="${BASH_SOURCE[0]}" | |
| while [ -L "$_self" ]; do _self="$(readlink -f "$_self")"; done | |
| PLUGIN_DIR="$(cd "$(dirname "$_self")/.." && pwd)" | |
| source "${PLUGIN_DIR}/lib/agent-paths.sh" | |
| TOKEN_FILE="${GITHUB_TOKEN_FILE:-${AGENT_CONFIG_DIR}/github-token}" | |
| META_FILE="${TOKEN_FILE}.meta" | |
| ENV_RUNTIME_FILE="${GITHUB_APP_ENV_FILE:-${AGENT_CONFIG_DIR}/github-app-env}" |
…then L97 can be deleted (already set above) and L122 becomes script_dir="${PLUGIN_DIR}/bin".
B — revert. In practice token-check.sh is only invoked by the hook at "$BIN_DIR/token-check.sh" using an absolute plugin path, so symlink safety isn't exercised. If that's the intended invariant, drop L36–38 back to the simple $(cd "$(dirname "$0")/.." && pwd) pattern and leave the whole file consistent. Reserve symlink-safe resolution for git-credential-github-app.sh, which is the one script genuinely expected to be invoked via symlink.
Either way: pick one. The current partial application is the worst of both worlds. Low priority vs. the README docs but worth resolving before un-drafting.
See also: related README thread.
There was a problem hiding this comment.
Partially addressed in da88564.
- ✅ L97:
PLUGIN_DIRnow uses$_self(permalink). - ❌ L122:
script_dirindo_generate_token()still usesdirname "$0"(permalink). Under a symlinked invocation,$script_dir/generate-token.shon L130 will point at the symlink's directory, not the plugin'sbin/. One-line fix:
script_dir="$(cd "$(dirname "$_self")" && pwd)"Or reuse the already-resolved $PLUGIN_DIR/bin/generate-token.sh and drop script_dir entirely.
…ths, symlink fix, README notes
There was a problem hiding this comment.
👍 The README blocker from Henry's last review is fixed; two small $0 → symlink-safe consistency nits remain (non-blocking).
✅ README migration note + AGENT_NAME-in-parent-shell gotcha added (addresses discussion_r3093104531)
✅ lib/agent-paths.sh double-source guard added (addresses discussion_r3093082520)
✅ github-token-check.sh:25 now sources via ${CLAUDE_PLUGIN_ROOT} (addresses top of discussion_r3093080366)
✅ bin/token-check.sh:97 now uses $_self (addresses top of discussion_r3093102509)
✅ _UNKNOWN fallback, version bump 0.1.14 → 0.2.0, docs sweep verified
⚠️ bin/token-check.sh:122 still uses dirname "$0" — fix suggestion inline
⚠️ github-token-check.sh:95 still derives PLUGIN_DIR from $0 — fix suggestion inline
🖱️ Click to expand for full details
$0 → symlink-safe consistency nits remain (non-blocking).AGENT_NAME-in-parent-shell gotcha added (addresses discussion_r3093104531)✅
lib/agent-paths.sh double-source guard added (addresses discussion_r3093082520)✅
github-token-check.sh:25 now sources via ${CLAUDE_PLUGIN_ROOT} (addresses top of discussion_r3093080366)✅
bin/token-check.sh:97 now uses $_self (addresses top of discussion_r3093102509)✅
_UNKNOWN fallback, version bump 0.1.14 → 0.2.0, docs sweep verifiedbin/token-check.sh:122 still uses dirname "$0" — fix suggestion inlinegithub-token-check.sh:95 still derives PLUGIN_DIR from $0 — fix suggestion inlineScope
First review pass from claude-review on da88564a. Re-verified each of Henry's four still-open threads from the previous CHANGES_REQUESTED review by reading current file contents. The new commit addresses the README blocker and three of the four $0-based pattern (both flagged inline with suggestion blocks so they can be applied in one click).
Code Quality (88%, up from 85%)
The lib/agent-paths.sh + _AGENT_PATHS_LOADED guard now matches sibling lib/token-utils.sh:7-8 and the .claude/rules/shared-libs.md convention. ${CLAUDE_PLUGIN_ROOT} is used at the top of both hooks for the new helper. The two remaining $0 derivations (bin/token-check.sh:122, github-token-check.sh:95-98) are the only reason this isn't 100% — they're each a single-line change and have concrete suggestions attached.
Also, the _self / while [ -L ... ] / readlink -f block is now duplicated verbatim across three bin/ scripts. Per the previous recommended follow-ups, this would collapse to a single source line each if folded into agent-paths.sh via BASH_SOURCE[1]. Non-blocking.
Correctness / Potential Bugs
The README gotcha that was the primary blocker in the previous review is now handled in README.md:194-215: migration note for orphaned ~/.config/agent/ files and the AGENT_NAME-in-login-shell requirement for git-credential-github-app.sh. Both are the exact scenarios that would have silently re-introduced the cross-agent bug under a different path.
The one residual functional risk is the $0 at bin/token-check.sh:122: if a user symlinks token-check.sh (e.g. into ~/.local/bin), script_dir resolves to the symlink's directory and the generate-token.sh call at L130 fails with a confusing "No such file or directory". $_self is already resolved at L36–38, so reusing it fixes this in one line.
Security (95%)
Unchanged from Henry's read — strictly improved over 0.1.x. Per-agent isolation at ~/.agents/<name>/.config/, chmod 600 preserved on token/env/PEM. The 5% note is still the _UNKNOWN collision between unconfigured agents, now documented in the upgrade section.
Simplicity (85%)
Generally good. The shared helper + the fact that all seven consumer sites derive AGENT_CONFIG_DIR from one place is the right level of abstraction. The drag is the duplicated symlink-resolution block in three bin/ scripts (listed under follow-ups below). Moving it into agent-paths.sh collapses three 3-line blocks into three one-liners.
Convention / Style
camelCasesettings keys (autoGitConfig,tokenFile) — compliant with.claude/rules/settings-key-naming.md.agent-paths.shnow has the_AGENT_PATHS_LOADEDguard per.claude/rules/shared-libs.md.- Docs consistently split
<agent-name>placeholders (config examples) from${AGENT_NAME}(runtime behaviour prose) — correct.
Test Coverage
PR test plan (4 manual items) is still unchecked. PR is still in draft, so that's expected. Recommend adding a fifth item from Henry's previous review: "git push via the credential helper picks up the per-agent token when AGENT_NAME is only in the login shell" — this directly exercises the newly-documented gotcha.
PR Description Accuracy
Body still matches the code changes, including the new files-changed list. No drift.
How I arrived at the scores
- Code Quality 88% (up from 85%): 3 of 4 prior
⚠️ s resolved; only the two$0derivations remain. - Security 95% (unchanged): README upgrade note now covers the
_UNKNOWNcollision caveat. - Simplicity 85% (unchanged): duplicated resolver block is the only simplicity drag.
- Confidence 90%: I re-read each file at
da88564a, cross-checked Henry's threads 8/9/10/11 line-by-line, grepped for~/.config/agent/(only matches are in the intentional README migration example). 10% uncertainty is around whether thereadlink -f/while [ -L ]pattern is intended as portable (it's GNU-only; stock macOS < 12.3 doesn't havereadlink -f) — not a blocker for this PR.
Recommended follow-ups (non-blocking, can ship after):
- Fold the
_self/readlink -fresolver intoagent-paths.sh(usingBASH_SOURCE[1]) so eachbin/script needs onlysource "$(dirname "${BASH_SOURCE[0]}")/../lib/agent-paths.sh"— collapses three duplicated blocks, and fixesbin/token-check.sh:122as a side-effect. - Promote the symlinked-script resolution pattern into
shared/lib/once settled — any plugin exposing scripts invoked outside the Claude harness (credential helpers, diff drivers, hook wrappers) will want the same primitive. - Log
${AGENT_NAME:-_UNKNOWN}in thehook_log "Authenticated as ..."line atgithub-token-init.sh:346so the active per-agent directory is visible at session start. - Drop the dot prefix on
~/.agents/<name>/.config/when ready (per @nsheaps's earlier note) — one-line change inagent-paths.shnow that it's centralized. - If macOS-bare support matters, switch
readlink -fto a portable resolver (e.g.,realpathwith fallback, or a tiny shell function) — GNUreadlink -fisn't in stock macOS < 12.3.
Footnotes
-
Workflow Run: https://github.com/nsheaps/ai-mktpl/actions/runs/24564118272/attempts/1 ↩
-
Repo convention for shared libs:
.claude/rules/shared-libs.md↩ -
Repo convention for hook output:
.claude/rules/hook-output-patterns.md↩
Summary
lib/agent-paths.shas single source of truth forAGENT_CONFIG_DIR~/.config/agent/paths with~/.agents/${AGENT_NAME:-_UNKNOWN}/.config/Files changed
plugins/github-app/lib/agent-paths.sh— shared helper exportingAGENT_CONFIG_DIRplugins/github-app/hooks/scripts/github-token-check.sh— source helper, remove eager mkdirplugins/github-app/hooks/scripts/github-token-init.sh— source helper, useAGENT_CONFIG_DIRplugins/github-app/bin/token-check.sh— source helperplugins/github-app/bin/token-status.sh— source helperplugins/github-app/bin/git-credential-github-app.sh— source helperplugins/github-app/.claude-plugin/plugin.json— version bump to 0.2.0Test plan
AGENT_NAME=henryin Agent-Henry ENVIRONMENT, restart HenryAGENT_NAME=jack~/.agents/_UNKNOWN/.config/Co-Authored-By: Jack Oat <jack-nsheaps[bot]@users.noreply.github.com>