Skip to content

Skill step 7 would make the audit-pin test compare a value with itself #125

Description

@realmarcin

Found in the adversarial review of #120.

The audit-pin test becomes circular if step 7 of the skill is followed literally

Where: .claude/skills/update-xmech-page/SKILL.md:193

test_the_audit_pins_the_revisions_the_stats_counted exists to tie the stats to the pinned revisions. Step 7 of the skill tells the next run to 'Derive ... merged PRs and revisions from mech_stats.json' for site_audit.json. That makes the audit SHA a copy of source_revision, so the test compares a value with itself. It cannot catch a stats run that read the wrong revision, such as the live checkout instead of the pin, or an enclosing repo (next finding). This run's build_site_audit.py actually took the SHA from revisions.json and asserted it equal to the stats (line 56), which is the correct direction. The instruction for future runs says the opposite.

Failure scenario. A future refresh accidentally runs mech_stats.py with the default MECHS_ROOT (live HEADs, not the pins). Following step 7, it writes the audit SHAs from mech_stats.json. The audit test passes, and the page claims provenance at SHAs that were never pinned or site-checked.

Evidence. SKILL.md:193-196: 'Derive the mechanical fields rather than typing them: ... merged PRs and revisions from mech_stats.json ... The provenance tests require its SHAs to equal the stats' source_revision.' scratchpad/refresh/build_site_audit.py:56 if stats[mech]["source_revision"] != pin["sha"]: and :60 "sha": pin["sha"].

Verifier (partly, low). SKILL.md step 7 (lines 193-196) tells a future run to derive the site_audit.json revisions from mech_stats.json, while the run's own builder took them from the pins ($SNAP/revisions.json) and asserted that they equal the stats. Followed literally, the skill turns test_the_audit_pins_the_revisions_the_stats_counted into a check that the audit and stats files are consistent with each other, not a check that the stats were counted at the pinned revisions. The test would still catch a later partial rerun of one file. It would miss the case where the census and stats were both read from the same wrong but clean checkout set, such as a stale or mis-pinned snapshot. The reviewer's example (mech_stats.py alone run against the default live MECHS_ROOT) is already caught by the census-vs-stats and clean-revision tests, and step 4 separately asks for a manual _revisions == pins check. Fix: say that the audit sha and commit_date come from $SNAP/revisions.json, and that the audit step must assert mech_stats.json's source_revision (and the census _revisions) equal those pins.

Suggested fix. Have the skill take the audit SHAs from the pins ($SNAP/revisions.json) and assert that they equal mech_stats.json's source_revision, as this run's script did.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions