fix(kyberforge): fix HOME/git scope-walkup false-FAILs in agent-audit
validate.sh's detect_scope() and validate-provenance.sh's find_plugin_root() disagreed with new-agent.sh's already-correct, documented walk-up semantics on three points, each causing validate.sh to false-FAIL a legitimately-scaffolded project-scope agent pair: - a marker-less directory walked up into $HOME (no .git/apm.yml of its own) was classified as user scope instead of project scope - the .git-boundary branch returned the walked-to .git location instead of the conventional scope root, breaking any <root> that is a subdirectory of a larger git-tracked tree (monorepo package dirs) - the new conventional-root arithmetic introduced to fix the above two cases had no guard against non-conventional/hand-placed file paths, which could point it at the wrong ancestor Also adds scripts/check-scope-walkup-sync.sh, a behavioral drift-guard (per ADR-0014's no-cross-skill-path precedent) that cross-checks the four independently hand-ported walk-up implementations (validate.sh, validate-provenance.sh, new-agent.sh, new-skill.sh) against real fixture scaffolds, wired into .pre-commit-config.yaml at pre-push so future drift between the ports is caught automatically. Verified via bash tests/run-tests.sh (13/13) and targeted before/after reproduction of each bug this closes.
This commit is contained in:
@@ -64,9 +64,11 @@ TYPE_RE = re.compile(r"^type:\s*(['\"]?)(instructions|skill|hybrid|prompts)\1(?:
|
||||
# --- Find package root: walk up for the nearest ancestor apm.yml that
|
||||
# declares a top-level type: field. An apm.yml with no type: field is a
|
||||
# marketplace-only manifest (see monorepo-and-repo-shapes.md) — skip it and
|
||||
# keep walking. Stop at a .git boundary or the filesystem root: neither is
|
||||
# plugin/APM scope, so this script has nothing to check there.
|
||||
# keep walking. Stop at a $HOME boundary, a .git boundary, or the filesystem
|
||||
# root: none of these is plugin/APM scope, so this script has nothing to
|
||||
# check there.
|
||||
def find_plugin_root(start_dir):
|
||||
home = os.path.expanduser('~')
|
||||
current = os.path.abspath(start_dir)
|
||||
while True:
|
||||
apm_yml = os.path.join(current, 'apm.yml')
|
||||
@@ -74,6 +76,15 @@ def find_plugin_root(start_dir):
|
||||
with open(apm_yml) as f:
|
||||
if any(TYPE_RE.match(line) for line in f):
|
||||
return current
|
||||
# $HOME is a non-plugin-scope boundary — checked before the .git test
|
||||
# below (mirrors validate.sh's detect_scope ordering), so a
|
||||
# dotfiles-managed $HOME (yadm, chezmoi bare-repo, etc.) can't shadow
|
||||
# this check by being its own .git repo. Without this, the walk could
|
||||
# continue past $HOME toward the filesystem root looking for a
|
||||
# type-bearing apm.yml, misclassifying a user/project-scope file as
|
||||
# plugin scope in rare ancestor layouts.
|
||||
if current == home:
|
||||
return None
|
||||
# .git is a directory in a normal checkout but a file (`gitdir: ...`)
|
||||
# in a git worktree — exists() covers both.
|
||||
if os.path.exists(os.path.join(current, '.git')):
|
||||
|
||||
@@ -152,23 +152,68 @@ def find_apm_package_root(apm_yml_path):
|
||||
|
||||
def detect_scope(start_dir):
|
||||
home = os.path.expanduser('~')
|
||||
current = os.path.abspath(start_dir)
|
||||
original_start = os.path.abspath(start_dir)
|
||||
# Agent files conventionally live exactly two path segments below their
|
||||
# scope root — <root>/.claude/agents, <root>/.github/agents,
|
||||
# <root>/.copilot/agents, or <root>/.apm/agents (see new-agent.sh's
|
||||
# CC_DIR/CP_DIR and user-scope dirs). Stripping those two segments
|
||||
# recovers the same root new-agent.sh would have been invoked with to
|
||||
# produce this exact file, independent of how far the walk below has to
|
||||
# travel to find (or fail to find) a marker — mirrors new-agent.sh's
|
||||
# `root` vs `current` distinction even though validate.sh is handed a
|
||||
# file's directory, not the scope root itself.
|
||||
#
|
||||
# That arithmetic is only trustworthy when the path actually has this
|
||||
# shape: parent directory literally named "agents", grandparent one of
|
||||
# the four known scope-dir names. A hand-placed or otherwise
|
||||
# non-conventional agent file (never produced by new-agent.sh) has no
|
||||
# such guarantee — blindly trusting two-segments-up there could point at
|
||||
# an unrelated ancestor. conventional_shape gates every use of
|
||||
# conventional_root below; when it's false, the walked-to `current`
|
||||
# directory is used instead, the same fallback this function used before
|
||||
# conventional_root existed.
|
||||
scope_dir_name = os.path.basename(os.path.dirname(original_start))
|
||||
conventional_shape = (
|
||||
os.path.basename(original_start) == 'agents'
|
||||
and scope_dir_name in ('.claude', '.github', '.copilot', '.apm')
|
||||
)
|
||||
conventional_root = os.path.dirname(os.path.dirname(original_start))
|
||||
current = original_start
|
||||
while True:
|
||||
apm_yml = os.path.join(current, 'apm.yml')
|
||||
if os.path.isfile(apm_yml) and find_apm_package_root(apm_yml):
|
||||
return 'plugin', current
|
||||
# $HOME is the user-scope boundary — checked before the .git test
|
||||
# below, so a dotfiles-managed $HOME (yadm, chezmoi bare-repo, etc.)
|
||||
# can't shadow user scope by being its own .git repo.
|
||||
# can't shadow user scope by being its own .git repo. 'user' scope
|
||||
# requires EITHER start_dir to BE $HOME itself (no walk-up — the
|
||||
# new-agent.sh "root exactly $HOME" case) OR start_dir to sit at the
|
||||
# conventional two-segments-below-root depth (i.e. $HOME IS that
|
||||
# root, matching the real ~/.claude/agents or ~/.copilot/agents
|
||||
# shape). Any other walk-up into $HOME — a marker-less directory
|
||||
# nested deeper than that convention — resolves to project scope
|
||||
# instead: a stray directory under $HOME can't be silently
|
||||
# redirected into the shared global ~/.claude or ~/.copilot agent
|
||||
# directories.
|
||||
if current == home:
|
||||
return 'user', home
|
||||
if original_start == home or (conventional_shape and conventional_root == home):
|
||||
return 'user', home
|
||||
return 'project', conventional_root if conventional_shape else current
|
||||
# .git is a directory in a normal checkout but a file (`gitdir: ...`)
|
||||
# in a git worktree — exists() covers both.
|
||||
# in a git worktree — exists() covers both. Returns conventional_root,
|
||||
# not current: new-agent.sh's project-scope file placement always
|
||||
# uses its `$ROOT` argument directly, never the walked-up `.git`
|
||||
# location, so a <root> one or more levels below the repo's .git
|
||||
# (a subdirectory of a larger git-tracked tree — explicitly a
|
||||
# supported case per new-agent.sh's usage text) must resolve to the
|
||||
# same root new-agent.sh actually wrote to, not to the .git dir —
|
||||
# unless the path lacks the conventional shape, in which case that
|
||||
# arithmetic isn't trustworthy and current is used instead.
|
||||
if os.path.exists(os.path.join(current, '.git')):
|
||||
return 'project', current
|
||||
return 'project', conventional_root if conventional_shape else current
|
||||
parent = os.path.dirname(current)
|
||||
if parent == current:
|
||||
return 'user', home
|
||||
return 'project', conventional_root if conventional_shape else current
|
||||
current = parent
|
||||
|
||||
agent_dir = os.path.dirname(agent_file)
|
||||
|
||||
@@ -183,6 +183,36 @@ EOF
|
||||
assert_output --partial "FAIL"
|
||||
}
|
||||
|
||||
@test "non-plugin scope: \$HOME boundary stops the walk before reaching an ancestor apm.yml above \$HOME" {
|
||||
# A type-bearing apm.yml sits ABOVE the fake $HOME — if find_plugin_root
|
||||
# didn't stop at $HOME, it would walk past it and misclassify this
|
||||
# user/project-scope file as plugin scope, which would then FAIL on
|
||||
# Check 0 (source_keys declared but sources.md absent) since sources.md
|
||||
# doesn't exist at that ancestor apm.yml's location either.
|
||||
local dir="$TMPDIR/anc"
|
||||
mkdir -p "$dir"
|
||||
cat > "$dir/apm.yml" <<EOF
|
||||
name: outer-package
|
||||
version: 0.1.0
|
||||
type: skill
|
||||
EOF
|
||||
local fake_home="$dir/fakehome"
|
||||
mkdir -p "$fake_home/.apm/agents"
|
||||
cat > "$fake_home/.apm/agents/my-agent.agent.md" <<EOF
|
||||
---
|
||||
name: my-agent
|
||||
description: A valid agent description.
|
||||
source_keys:
|
||||
- my-source
|
||||
---
|
||||
|
||||
You are a test agent.
|
||||
EOF
|
||||
run env HOME="$fake_home" bash "$SCRIPT" "$fake_home/.apm/agents/my-agent.agent.md"
|
||||
assert_success
|
||||
assert_output ""
|
||||
}
|
||||
|
||||
@test "non-plugin scope: .git between the agent file and an ancestor apm.yml stops the walk first" {
|
||||
local dir="$TMPDIR/repo2"
|
||||
mkdir -p "$dir/.apm/agents"
|
||||
|
||||
@@ -92,6 +92,174 @@ EOF
|
||||
refute_output --partial "FAIL"
|
||||
}
|
||||
|
||||
@test "user scope: agent file directly in \$HOME (start dir IS exactly \$HOME, no walk-up) resolves to user scope" {
|
||||
local fake_home="$TMPDIR/fakehome-direct"
|
||||
mkdir -p "$fake_home" "$fake_home/.copilot/agents"
|
||||
cat > "$fake_home/my-agent.md" <<EOF
|
||||
---
|
||||
name: my-agent
|
||||
description: A valid agent description.
|
||||
---
|
||||
|
||||
You are a test agent. When invoked, do the thing.
|
||||
EOF
|
||||
cat > "$fake_home/.copilot/agents/my-agent.agent.md" <<EOF
|
||||
---
|
||||
name: my-agent
|
||||
description: A valid agent description.
|
||||
---
|
||||
|
||||
You are a test agent. When invoked, do the thing.
|
||||
EOF
|
||||
run env HOME="$fake_home" bash "$SCRIPT" "$fake_home/my-agent.md"
|
||||
assert_success
|
||||
refute_output --partial "FAIL"
|
||||
refute_output --partial "counterpart"
|
||||
}
|
||||
|
||||
@test "project scope: a nested marker-less directory walked up into \$HOME resolves to project scope, not user scope (live repro of new-agent.sh's stray-directory case)" {
|
||||
local fake_home="$TMPDIR/fakehome-nested"
|
||||
local nested="$fake_home/scratch/testdir"
|
||||
mkdir -p "$nested/.claude/agents" "$nested/.github/agents"
|
||||
cat > "$nested/.claude/agents/my-agent.md" <<EOF
|
||||
---
|
||||
name: my-agent
|
||||
description: A valid agent description.
|
||||
---
|
||||
|
||||
You are a test agent. When invoked, do the thing.
|
||||
EOF
|
||||
cat > "$nested/.github/agents/my-agent.agent.md" <<EOF
|
||||
---
|
||||
name: my-agent
|
||||
description: A valid agent description.
|
||||
---
|
||||
|
||||
You are a test agent. When invoked, do the thing.
|
||||
EOF
|
||||
run env HOME="$fake_home" bash "$SCRIPT" "$nested/.claude/agents/my-agent.md"
|
||||
assert_success
|
||||
refute_output --partial "FAIL"
|
||||
refute_output --partial "counterpart"
|
||||
}
|
||||
|
||||
@test "project scope: nested marker-less dir under \$HOME does NOT look for a counterpart under the shared \$HOME/.copilot or \$HOME/.github dirs" {
|
||||
local fake_home="$TMPDIR/fakehome-nested2"
|
||||
local nested="$fake_home/scratch/testdir"
|
||||
mkdir -p "$nested/.claude/agents" "$fake_home/.copilot/agents"
|
||||
cat > "$nested/.claude/agents/my-agent.md" <<EOF
|
||||
---
|
||||
name: my-agent
|
||||
description: A valid agent description.
|
||||
---
|
||||
|
||||
You are a test agent. When invoked, do the thing.
|
||||
EOF
|
||||
# Decoy counterpart at the *user*-scope location — if scope were
|
||||
# misclassified as 'user' (the pre-fix bug), validate.sh would find this
|
||||
# unrelated file and (wrongly) pass.
|
||||
cat > "$fake_home/.copilot/agents/my-agent.agent.md" <<EOF
|
||||
---
|
||||
name: my-agent
|
||||
description: A valid agent description.
|
||||
---
|
||||
|
||||
You are a test agent. When invoked, do the thing.
|
||||
EOF
|
||||
run env HOME="$fake_home" bash "$SCRIPT" "$nested/.claude/agents/my-agent.md"
|
||||
assert_failure
|
||||
assert_output --partial "counterpart file not found"
|
||||
}
|
||||
|
||||
@test "project scope: filesystem-root fallback (no \$HOME in path, no markers found) resolves to project scope, not user scope" {
|
||||
local unrelated_home="$TMPDIR/unrelated-home-never-reached"
|
||||
local root="$TMPDIR/no-home-relation/deep/proj"
|
||||
mkdir -p "$root/.claude/agents" "$root/.github/agents"
|
||||
cat > "$root/.claude/agents/my-agent.md" <<EOF
|
||||
---
|
||||
name: my-agent
|
||||
description: A valid agent description.
|
||||
---
|
||||
|
||||
You are a test agent. When invoked, do the thing.
|
||||
EOF
|
||||
cat > "$root/.github/agents/my-agent.agent.md" <<EOF
|
||||
---
|
||||
name: my-agent
|
||||
description: A valid agent description.
|
||||
---
|
||||
|
||||
You are a test agent. When invoked, do the thing.
|
||||
EOF
|
||||
run env HOME="$unrelated_home" bash "$SCRIPT" "$root/.claude/agents/my-agent.md"
|
||||
assert_success
|
||||
refute_output --partial "FAIL"
|
||||
refute_output --partial "counterpart"
|
||||
}
|
||||
|
||||
@test "project scope: <root> one level below a .git ancestor resolves scope to <root>, not to wherever .git was found (subdirectory of a larger git-tracked tree)" {
|
||||
local repo="$TMPDIR/repo-with-subdir"
|
||||
local root="$repo/subdir"
|
||||
mkdir -p "$repo/.git" "$root/.claude/agents" "$root/.github/agents"
|
||||
cat > "$root/.claude/agents/my-agent.md" <<EOF
|
||||
---
|
||||
name: my-agent
|
||||
description: A valid agent description.
|
||||
---
|
||||
|
||||
You are a test agent. When invoked, do the thing.
|
||||
EOF
|
||||
cat > "$root/.github/agents/my-agent.agent.md" <<EOF
|
||||
---
|
||||
name: my-agent
|
||||
description: A valid agent description.
|
||||
---
|
||||
|
||||
You are a test agent. When invoked, do the thing.
|
||||
EOF
|
||||
# new-agent.sh, invoked with <root> as its root argument, would place the
|
||||
# counterpart at <root>/.github/agents — not at the repo root's
|
||||
# .github/agents, even though .git lives at the repo root one level up.
|
||||
run bash "$SCRIPT" "$root/.claude/agents/my-agent.md"
|
||||
assert_success
|
||||
refute_output --partial "FAIL"
|
||||
refute_output --partial "counterpart"
|
||||
}
|
||||
|
||||
@test "project scope: a non-conventional path (agent file not directly under a literal 'agents' dir) falls back to the nearest .git boundary instead of two-segments-up arithmetic" {
|
||||
local outer="$TMPDIR/outer-repo"
|
||||
local pkg="$outer/pkgA"
|
||||
mkdir -p "$pkg/.git" "$pkg/.github/agents" "$pkg/extra"
|
||||
# Misplaced file: sits two path segments below $outer (pkgA/extra), which
|
||||
# matches the conventional_root arithmetic by coincidence, but its
|
||||
# immediate parent dir is "extra", not "agents" — conventional_shape is
|
||||
# false, so the fix must fall back to the nearest .git boundary (pkgA),
|
||||
# not trust $outer.
|
||||
cat > "$pkg/extra/my-agent.md" <<EOF
|
||||
---
|
||||
name: my-agent
|
||||
description: A valid agent description.
|
||||
---
|
||||
|
||||
You are a test agent. When invoked, do the thing.
|
||||
EOF
|
||||
# Counterpart at the nearest-.git root (pkgA), not at $outer — if the
|
||||
# arithmetic were trusted here, validate.sh would look for a counterpart
|
||||
# at $outer/.github/agents/my-agent.agent.md, which doesn't exist, and
|
||||
# false-FAIL.
|
||||
cat > "$pkg/.github/agents/my-agent.agent.md" <<EOF
|
||||
---
|
||||
name: my-agent
|
||||
description: A valid agent description.
|
||||
---
|
||||
|
||||
You are a test agent. When invoked, do the thing.
|
||||
EOF
|
||||
run bash "$SCRIPT" "$pkg/extra/my-agent.md"
|
||||
assert_success
|
||||
refute_output --partial "counterpart file not found"
|
||||
}
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Failing cases — project/user scope: CC/Copilot pair checks
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user