Skip to content

Commit .claude/rules, gate migration parity in CI, clear reference_only on re-verify - #23

Merged
rchildress87 merged 3 commits into
mainfrom
claude-rules-refonly-ci
Oct 3, 2026
Merged

rchildress87 merged 3 commits into
mainfrom
claude-rules-refonly-ci

Conversation

@rchildress87

Copy link
Copy Markdown
Contributor

What this changes

This PR closes the three items PR #22 left open, one commit each.

1. Commit the shared Claude rules (27b3e835)

  • The problem: .gitignore ignored all of .claude/, so .claude/rules/testing.instructions.md existed on only one machine.
  • The change: the entry is now .claude/* with !.claude/rules/ re-included. Settings, lock files and session memory stay ignored.
  • A side effect, fixed: .claude/* applies only at the repo root, whereas the old .claude/ matched at any depth. */**/.claude/ keeps any nested .claude/ folder out of git.
  • .claude/golang.md deleted, not committed:
    • It was an unattributed, byte-identical copy of github/awesome-copilot's MIT-licensed go.instructions.md.
    • Its advice to name tests Test_x_y and to consider testify contradicts this repo.
    • The rules file now states the repo's own conventions instead.

2. Gate migration parity in CI (19816253)

  • New CI step: the test job runs make check-migrations right after checkout, before the SDK fetch, so a mismatch fails in seconds.
  • Script gap: the check collected only CREATE TABLE and ADD COLUMN. A column or table dropped in one dialect only passed; the old script exits 0 on a one-sided DROP COLUMN.
  • Script fix: it now applies the migrations in order, drops included, and compares the final schemas. A one-sided add, column drop or table drop each exits 1, and the current schema still passes.
  • Script hardening:
    • It now really exits 2 when it cannot run; the header promised this but nothing did it.
    • Temp files move to a mktemp directory removed on any exit.

3. Clear reference_only when a scanner re-verifies a placeholder (84406405)

  • The bug: a scanner row identical to a resolver placeholder (same type, {} attributes and tags) compares equal, so it takes the verify path, and that UPDATE never touched the flag. The row stayed flagged, TypesForScan hid its type, and coverage verify could report the type as declared but not emitted.
  • The fix: the verify UPDATE now sets reference_only = FALSE, the same way it already clears the archive tombstone. Only UpsertResources (scanners) reaches that path.

Checklist

  • Branched from main and targeting main
  • CGO_ENABLED=0 go test ./... passes (full run with -tags grpcnotrace)
  • go vet ./... and golangci-lint run are clean (store: 0 issues; no other Go package changed)
  • golangci-lint fmt --diff is empty on the changed package
  • make check-migrations passes, and this PR's CI runs it for the first time
  • No new scanners or resolvers
  • No CGO dependency introduced

Notes for reviewers

  • The new test fails without the fix. TestUpsertResources_VerifyClearsReferenceOnly runs on SQLite and Postgres (the Postgres subtest ran, not skipped). With the new SET line removed, both fail.
  • Effect on disco-saas: it reads reference_only for a UI badge, a synthetic-inventory generator and a row count that does not filter on it. After this change a scanner-verified row shows as a real resource, which is how a row that took the split path already behaves.
  • Known limit of the migration check: RENAME is not modelled. A rename shows up as drift, never as a silent pass.

🤖 Generated with Claude Code

rchildress87 and others added 3 commits October 3, 2026 09:55
.gitignore ignored all of .claude/, so the test-quality rules in
.claude/rules/testing.instructions.md existed only on one machine. The
blanket entry becomes .claude/* with .claude/rules/ re-included; git
cannot re-include a path whose parent directory is excluded.

.claude/* contains a slash, so it is anchored to the repo root, whereas
the old .claude/ matched at any depth. */**/.claude/ keeps a .claude/
directory a session creates in a subdirectory out of git. Settings,
locks and session memory stay ignored.

The rules file linked to .claude/golang.md, a local, unattributed copy
of github/awesome-copilot's MIT-licensed go.instructions.md whose
test-naming and testify advice contradicts this repo. That file is
deleted rather than committed; the rules file now names the idioms it
relied on and the repo's own conventions (TestX_Scenario, stdlib
testing only).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
CI's test job now runs `make check-migrations` right after checkout,
before the SDK fetch, so schema drift fails in seconds. Until now the
check was manual and `go test` caught only version-ceiling skew.

The script collected CREATE TABLE and ADD COLUMN only, so a column or
table dropped in one dialect passed: the old script exits 0 with a
one-sided `DROP COLUMN zone`. It now replays each set in migration
order into a (table, column) set, applying DROP COLUMN and DROP TABLE,
and diffs the final sets. A one-sided add, column drop or table drop
each exits 1 (checked under mawk); the current schema still matches.

The header promised exit 2 on a tooling failure, but a failed extraction
exited with the awk status or not at all. It now exits 2 explicitly.
Temp files move from fixed /tmp names, removed only on success, to a
mktemp directory with an EXIT trap.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…lder

A resolver placeholder (InsertResourcesIfAbsent) is written with
reference_only = TRUE and `{}` attributes. When a scanner later emits
the same resource and the split path runs, the new row omits the column
and the flag ends up FALSE. But a scanner row identical to the
placeholder (same type, `{}` attributes and tags) compares equal and
takes the verify path, whose UPDATE never touched the column. The row
stayed flagged, so TypesForScan hid the type and `coverage verify`
could report it declared-not-emitted.

The verify UPDATE now sets reference_only = FALSE, unconditionally like
deleted_at/deleted_by. Only UpsertResources (scanners) reaches that
path; InsertResourcesIfAbsent never does, so a placeholder write can
neither clear the flag nor set it on a real row.

TestUpsertResources_VerifyClearsReferenceOnly (SQLite + Postgres)
re-emits an identical row under a second scan and checks the flag
clears, no split, verified_by advances, TypesForScan counts the type,
and an untouched placeholder keeps its flag. With the new SET line
removed it fails on both dialects (TypesForScan returns []).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@rchildress87
rchildress87 merged commit 8c0abbf into main Oct 3, 2026
2 checks passed
rchildress87 added a commit that referenced this pull request Oct 21, 2026
aws:ec2:ipam-prefix-list-resolver rows now carry a Rules key, read per
resolver with GetIpamPrefixListResolverRules (manual NextToken loop,
MaxResults 1000, shared errRepeatedNextToken) and sorted to a total order:
each rule's conditions first, then the rules, both by their encoding, so
unchanged rules never version-split the row. An empty read stores [].
Delete-complete resolvers are stored without reading rules.

A failed rules read never writes an empty or missing list in place of the
rules (decision #23): a denied or transient read skips that resolver with
one counted warning per phase; any other error, including the not-found
code EC2's model does not list, fails the phase after the resolvers that
were read are stored. Resolvers listed without an id or ARN are counted in
a warning.

The resolver phase moved out of scanEC2's concurrent categories into
scanEC2IPAMExtended, which runs after them, so one failed rules read
cannot cancel the other EC2 categories; this costs at least one round trip
per region. Existing resolver rows split once (new Rules key), and a role
without GetIpamPrefixListResolverRules now stores no resolver rows.

Reviewed adversarially over three rounds; 26/26 mutants killed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DTWFksHGT3LDTeYfqUYPRB
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