From 11b8823d94bd8b2715bd15fa3f9026f29c2f51ec Mon Sep 17 00:00:00 2001 From: Andrii Pasternak Date: Sun, 26 Apr 2026 00:12:13 +0100 Subject: [PATCH 1/2] fix(validate-architecture): add stale-citation filter + issue dedupe guard (#511) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The /validate-architecture skill produced false-positive issue #479 by: 1. citing file paths the report's snapshot saw, but `main` no longer has (process engine deleted in #430 the same day); 2. running `gh issue create` with no check for existing open issues with the same finding fingerprint. Two targeted edits to .claude/skills/validate-architecture/SKILL.md: - New Step 2c "Filter Stale Citations" — `git ls-files --error-unmatch` every cited path before report. Drop ghosts. Downgrade FAIL → PASS when an invariant has zero remaining real citations. - Modified Step 4 — fingerprint = sorted invariant numbers; query open `automated,priority-p1` issues with `--search "in:body validate-architecture fingerprint="`; comment on existing issue instead of creating duplicate. Issue body now stamps the current commit SHA for evidence binding. Closes #511. Co-Authored-By: Claude Opus 4.7 (1M context) --- .claude/skills/validate-architecture/SKILL.md | 59 +++++++++++++++++-- 1 file changed, 54 insertions(+), 5 deletions(-) diff --git a/.claude/skills/validate-architecture/SKILL.md b/.claude/skills/validate-architecture/SKILL.md index e9b8b5b5e..04cda3d0a 100644 --- a/.claude/skills/validate-architecture/SKILL.md +++ b/.claude/skills/validate-architecture/SKILL.md @@ -130,6 +130,22 @@ architecture.md:L — Process Engine marked "OUT OF SCOPE" but routers/proces Suggested edit: either remove the OUT OF SCOPE tag (if the module is in fact live), or remove the routers (if it is truly dormant). ``` +### Step 2c: Filter Stale Citations + +Before generating the report, validate every `file:line` citation produced in Steps 2a and 2b against the current working tree. The skill is sometimes run on a snapshot taken just before a large deletion PR lands; without this filter, the report (and any auto-created issue) cites paths that no longer exist. + +For each cited path: + +```bash +git ls-files --error-unmatch >/dev/null 2>&1 && echo exists || echo dropped +``` + +- **If `dropped`**: remove the citation from the violation. Do not retain "ghost" line numbers from a previous tree. +- **After filtering**, if an invariant's violation list is empty, downgrade its status from `FAIL` to `PASS (after stale-citation filter)` and record the dropped citation count in the report so the reader can see what was removed. +- **If the only violations cited paths that no longer exist**, do not propagate this invariant to Step 4's issue-creation trigger. + +This step exists because of issue #479: a 2026-04-24 run cited 11 paths that were deleted by commit e901108 (#430) the same day. None of those P1-critical citations were real on `main`, but the unfiltered report produced a `priority-p1` issue against `main`. + ### Step 3: Generate Report Output two sections: @@ -162,9 +178,9 @@ Output two sections: - architecture.md:L — "
" marked out-of-scope but . Suggested edit: . ``` -### Step 4: Create Issue if Critical +### Step 4: Create or Update Issue if Critical -Create a GitHub issue when any of these fire: +Create or update a GitHub issue when any of these fire (after the Step 2c stale-citation filter has run): **P0-P1 invariants** (critical — break runtime or security): - #1 Three-Layer Backend (layer violations cause maintenance debt) @@ -176,7 +192,32 @@ Create a GitHub issue when any of these fire: - D1 count mismatches with >25% divergence - D2 any scope contradiction (dormant-but-live modules) -If any fire, create issue: +**Dedupe guard — required before any `gh issue create`:** + +Compute a fingerprint over the post-filter findings, then check for an existing open issue with the same fingerprint: + +```bash +COMMIT_SHA=$(git rev-parse --short HEAD) +# Sorted, comma-separated list of invariant numbers that fired post-filter, +# e.g. "1,3" or "8,14" +FINGERPRINT=$(printf '%s\n' "${FIRED_INVARIANTS[@]}" | sort -n | paste -sd, -) + +# Find any open automated arch-validation issue +EXISTING=$(gh issue list --repo abilityai/trinity \ + --label "automated,priority-p1" --state open \ + --search "in:body validate-architecture fingerprint=$FINGERPRINT" \ + --json number,title --jq '.[0].number') + +if [ -n "$EXISTING" ]; then + # Same invariant set already tracked — comment instead of duplicating + gh issue comment "$EXISTING" --repo abilityai/trinity --body "Re-run on \`$COMMIT_SHA\` ($(date -u +%Y-%m-%d)): same invariants still failing (\`$FINGERPRINT\`). See attached fresh report. + +[fresh report body]" + exit 0 +fi +``` + +Only if no existing issue matches the fingerprint, create a new one: ```bash gh issue create \ @@ -185,10 +226,12 @@ gh issue create \ --body "## Automated Architecture Validation Report **Date**: $(date -u +%Y-%m-%d) +**Commit**: $COMMIT_SHA +**Fingerprint**: validate-architecture fingerprint=$FINGERPRINT ### Critical Invariant Violations (P0-P1) -[List each P0-P1 violation with invariant number, file:line, description] +[List each P0-P1 violation with invariant number, file:line, description — Step 2c-filtered, no stale paths] ### Doc Drift — Suggested architecture.md Edits @@ -198,12 +241,18 @@ gh issue create \ 1. [Prioritized fix for each finding] +### Dedupe Notes + +- This issue is keyed by \`fingerprint=$FINGERPRINT\` (sorted invariant numbers). +- Future skill runs with the same fingerprint will comment on this issue rather than open a new one. +- Close this issue once the cited invariants pass on \`main\`. + --- *Generated by scheduled /validate-architecture run*" \ --label "type-bug,priority-p1,automated" ``` -If nothing critical fires, skip issue creation — report only logged to execution history. +If nothing critical fires after Step 2c's filter, skip issue creation — report only logged to execution history. **Do not create an issue solely on pre-filter results.** ## Outputs From 6aba51b3db73fd8cb1424c1e1d5fd4a49f6bd580 Mon Sep 17 00:00:00 2001 From: Andrii Pasternak Date: Sun, 26 Apr 2026 00:23:38 +0100 Subject: [PATCH 2/2] fix(validate-architecture): clarify dedupe branching, distinct fingerprint marker, quote paths (#511) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to review feedback on PR #513. Three small skill-prose hardenings: - I2 (LLM-driven flow control): the dedupe branch previously relied on `if [ -n "$EXISTING" ]; then ...; exit 0; fi` followed by a separate create block. `exit 0` halts a bash subshell, not an LLM walking the markdown — a future runner could execute both blocks. Replace with explicit "Path A — COMMENT, then STOP" / "Path B — CREATE" prose branching and an explicit DO-NOT note. - I4 (fingerprint collision): replace free-text body search `validate-architecture fingerprint=$FP` with HTML-comment marker `` plus a quoted-phrase search. Self-evidently programmatic; won't collide with prose. - I3 (path quoting): the Step 2c example now uses `"$path"` and a note about shell metachars, so implementers don't strip the quotes. - Add concurrency caveat documenting that the dedupe is best-effort, not atomic (no GitHub primitive provides this). Co-Authored-By: Claude Opus 4.7 (1M context) --- .claude/skills/validate-architecture/SKILL.md | 40 ++++++++++++------- 1 file changed, 26 insertions(+), 14 deletions(-) diff --git a/.claude/skills/validate-architecture/SKILL.md b/.claude/skills/validate-architecture/SKILL.md index 04cda3d0a..ff4a0c125 100644 --- a/.claude/skills/validate-architecture/SKILL.md +++ b/.claude/skills/validate-architecture/SKILL.md @@ -134,10 +134,10 @@ architecture.md:L — Process Engine marked "OUT OF SCOPE" but routers/proces Before generating the report, validate every `file:line` citation produced in Steps 2a and 2b against the current working tree. The skill is sometimes run on a snapshot taken just before a large deletion PR lands; without this filter, the report (and any auto-created issue) cites paths that no longer exist. -For each cited path: +For each cited path (always quote `"$path"` — citations may contain spaces or shell metacharacters): ```bash -git ls-files --error-unmatch >/dev/null 2>&1 && echo exists || echo dropped +git ls-files --error-unmatch "$path" >/dev/null 2>&1 && echo exists || echo dropped ``` - **If `dropped`**: remove the citation from the violation. Do not retain "ghost" line numbers from a previous tree. @@ -194,30 +194,39 @@ Create or update a GitHub issue when any of these fire (after the Step 2c stale- **Dedupe guard — required before any `gh issue create`:** -Compute a fingerprint over the post-filter findings, then check for an existing open issue with the same fingerprint: +Compute a fingerprint over the post-filter findings, then check for an existing open issue with the same fingerprint. The fingerprint is wrapped in an HTML comment marker (``) inside the issue body so the dedupe key is self-evidently programmatic and won't collide with prose mentions of "fingerprint" in unrelated issues. ```bash COMMIT_SHA=$(git rev-parse --short HEAD) # Sorted, comma-separated list of invariant numbers that fired post-filter, # e.g. "1,3" or "8,14" FINGERPRINT=$(printf '%s\n' "${FIRED_INVARIANTS[@]}" | sort -n | paste -sd, -) +FINGERPRINT_MARKER="validate-architecture::fingerprint=$FINGERPRINT" -# Find any open automated arch-validation issue +# Find any open automated arch-validation issue with the exact marker EXISTING=$(gh issue list --repo abilityai/trinity \ --label "automated,priority-p1" --state open \ - --search "in:body validate-architecture fingerprint=$FINGERPRINT" \ + --search "in:body \"$FINGERPRINT_MARKER\"" \ --json number,title --jq '.[0].number') +``` + +**Branch on `$EXISTING`. The two paths are mutually exclusive — execute exactly one.** + +**Path A — `$EXISTING` is non-empty (matching open issue found): COMMENT, then STOP.** + +```bash +gh issue comment "$EXISTING" --repo abilityai/trinity --body "Re-run on \`$COMMIT_SHA\` ($(date -u +%Y-%m-%d)): same invariants still failing (\`$FINGERPRINT\`). See attached fresh report. -if [ -n "$EXISTING" ]; then - # Same invariant set already tracked — comment instead of duplicating - gh issue comment "$EXISTING" --repo abilityai/trinity --body "Re-run on \`$COMMIT_SHA\` ($(date -u +%Y-%m-%d)): same invariants still failing (\`$FINGERPRINT\`). See attached fresh report. +[fresh report body] -[fresh report body]" - exit 0 -fi +" ``` -Only if no existing issue matches the fingerprint, create a new one: +After commenting, **DO NOT** execute Path B. The skill workflow ends here for this run. + +**Path B — `$EXISTING` is empty (no matching open issue): CREATE a new issue.** + +Only run this block when Path A did not run. ```bash gh issue create \ @@ -227,7 +236,6 @@ gh issue create \ **Date**: $(date -u +%Y-%m-%d) **Commit**: $COMMIT_SHA -**Fingerprint**: validate-architecture fingerprint=$FINGERPRINT ### Critical Invariant Violations (P0-P1) @@ -243,15 +251,19 @@ gh issue create \ ### Dedupe Notes -- This issue is keyed by \`fingerprint=$FINGERPRINT\` (sorted invariant numbers). +- This issue is keyed by \`$FINGERPRINT_MARKER\` (sorted invariant numbers). - Future skill runs with the same fingerprint will comment on this issue rather than open a new one. - Close this issue once the cited invariants pass on \`main\`. + + --- *Generated by scheduled /validate-architecture run*" \ --label "type-bug,priority-p1,automated" ``` +**Concurrency caveat**: this dedupe is best-effort, not atomic. Two runners executing simultaneously could both see `$EXISTING` empty and both create issues. GitHub provides no atomic compare-and-create primitive. The next run with the same fingerprint will detect both open issues and comment on the first; the duplicate can be closed manually with `Closes #`. In practice this is rare because the skill is scheduler-driven (single runner). + If nothing critical fires after Step 2c's filter, skip issue creation — report only logged to execution history. **Do not create an issue solely on pre-filter results.** ## Outputs