Skip to content

Commit 182a590

Browse files
aksOpsclaudePaperclip-Paperclip
committed
fix(bootstrap): R5 reviewer findings 4–7 (RAN-47 fd559a54)
Reviewer's updated PR comment fd559a54 surfaced 4 additional blockers beyond R5-1..3 already fixed in `a4dee7c`. R5-4 — spotbugs-maven-plugin not lifecycle-bound. pom.xml declared the plugin but with no `<executions>` block, so `mvn verify` (and therefore CI on every PR) did not actually run SpotBugs — the engineering-standards.md "zero High/Critical findings" gate was a documented claim, not an enforced one. Bound the `check` goal to the verify phase, set explicit threshold=High + failOnError=true so the gate matches the documented semantic and cannot silently relax under future config edits. R5-5 — rollback.md branch-protection GET→PUT schema mismatch. GitHub's GET /protection returns a denormalized payload (nested `{enabled: bool}` envelopes, `checks[].context` strings, `*.url` fields) that PUT does not accept verbatim. Replaced the naive cat-into-PUT with a documented jq filter that unwraps the envelopes, projects `checks[].context` into the flat `contexts[]` PUT expects, drops `*.url` fields, and forces `restrictions: null` for this repo. R5-6 — engineering-standards.md §1 unenforced branch coverage claim. Quality-gate table claimed "≥ 85% line, ≥ 75% branch (project-wide)" but `pom.xml`'s jacoco rule only enforces LINE COVEREDRATIO 0.85. Aligned the doc to reality (LINE only). Adding a branch-coverage rule is a separate decision — not in scope here. R5-7 — release.md SSH-key claims contradict GPG-via-Actions reality. Two stale SSH-signing references: "Source tag (annotated, ssh-signed)" and pre-release checklist item "Local signing key present: ssh-add -L | grep ...". The actual GA path is GPG/OpenPGP-signed by release-java.yml using the imported MAVEN_GPG_PRIVATE_KEY — no local SSH key required. Updated both: the source-tag descriptor now reads "GPG/OpenPGP-signed by release-java.yml", and the checklist item now verifies the GHA secrets (MAVEN_GPG_PRIVATE_KEY, MAVEN_GPG_PASSPHRASE) are present via `gh secret list`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-Authored-By: Paperclip <noreply@paperclip.ing>
1 parent a4dee7c commit 182a590

4 files changed

Lines changed: 63 additions & 4 deletions

File tree

pom.xml

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -410,7 +410,32 @@
410410
<version>${spotbugs.version}</version>
411411
<configuration>
412412
<excludeFilterFile>spotbugs-exclude.xml</excludeFilterFile>
413+
<!-- Hard gate semantics per engineering-standards.md §1:
414+
"Zero High/Critical findings". Threshold High filters
415+
out Medium/Low findings (so the gate doesn't trip on
416+
non-blocking findings); failOnError is the
417+
spotbugs-maven-plugin default for `check`, made
418+
explicit so a future config edit doesn't silently
419+
relax the gate. -->
420+
<threshold>High</threshold>
421+
<failOnError>true</failOnError>
413422
</configuration>
423+
<executions>
424+
<!-- Reviewer finding fd559a54 (RAN-47, R5-4):
425+
spotbugs-maven-plugin was declared but had no
426+
lifecycle binding, so `mvn verify` (and therefore
427+
CI on every PR) did not actually run SpotBugs.
428+
Bind `check` to the verify phase so the gate
429+
claimed in engineering-standards.md §1 is
430+
actually enforced on every build. -->
431+
<execution>
432+
<id>spotbugs-check-on-verify</id>
433+
<phase>verify</phase>
434+
<goals>
435+
<goal>check</goal>
436+
</goals>
437+
</execution>
438+
</executions>
414439
</plugin>
415440

416441
<plugin>

shared/runbooks/engineering-standards.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ The rule of last resort: **`/home/dev/.claude/rules/*.md` wins.** This file does
1111
| Gate | Threshold | Where it runs | Failure action |
1212
|---|---|---|---|
1313
| Unit + integration tests | All pass | `mvn verify` (CI + local) | Block merge |
14-
| JaCoCo coverage | ≥ 85% line, ≥ 75% branch (project-wide, post-exclusions) | `jacoco-maven-plugin` rule in `pom.xml` | Block merge |
14+
| JaCoCo coverage | ≥ 85% line (project-wide, post-exclusions) | `jacoco-maven-plugin` rule in `pom.xml` | Block merge |
1515
| SonarCloud Quality Gate | `Passed` (`Sonar way` profile + 80% new-code coverage) | `ci-java.yml` | Block merge |
1616
| SpotBugs | Zero High/Critical findings; `spotbugs-exclude.xml` justified per-entry | `mvn spotbugs:check` | Block merge |
1717
| OWASP Dependency-Check | No High/Critical CVEs (`failBuildOnCVSS=7`); Medium tracked | `mvn -B -ntp clean verify` (the `dependency-check:check` execution is bound to the `verify` phase in `pom.xml`); `ci-java.yml` runs on every PR + push to `main` | Block merge |

shared/runbooks/release.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ codeiq ships in three forms. A "release" updates **all three** in lockstep:
1414
|---|---|---|
1515
| Library JAR | `io.github.randomcodespace.iq:code-iq` (POM packaging) | Maven Central via Sonatype Central Portal |
1616
| Executable CLI JAR | `target/code-iq-<version>-cli.jar` | GitHub Release asset |
17-
| Source tag | `v<version>` (annotated, ssh-signed) | Git tag pushed to `RandomCodeSpace/codeiq` |
17+
| Source tag | `v<version>` (annotated, GPG/OpenPGP-signed by `release-java.yml`) | Git tag pushed to `RandomCodeSpace/codeiq` |
1818

1919
Versioning is [SemVer](https://semver.org/). Pre-`1.0.0` releases use `0.MINOR.PATCH`; breaking changes bump MINOR.
2020

@@ -33,7 +33,7 @@ Run BEFORE creating the tag:
3333
5. SpotBugs clean: `mvn spotbugs:check` exits 0.
3434
6. CHANGELOG entry drafted under `[Unreleased]` and ready to promote.
3535
7. Working copy of `main` is clean (`git status --porcelain` empty).
36-
8. Local signing key present: `ssh-add -L | grep -F "$(cat ~/.ssh/id_ed25519.pub | awk '{print $2}')"`required for the annotated tag.
36+
8. GPG release-signing secrets present in repo settings: `MAVEN_GPG_PRIVATE_KEY` and `MAVEN_GPG_PASSPHRASE` (verify via `gh secret list`). The workflow signs both the release commit and the annotated tag with the imported OpenPGP key — no local SSH or GPG key is required on the maintainer's machine for the GA path (Reviewer finding fd559a54, R5-7).
3737

3838
---
3939

shared/runbooks/rollback.md

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,41 @@ Do NOT delete the bad git tag. Yanking tags after they have been seen by consume
6262

6363
These are driven by `gh api` calls (see RAN-46 inventory). They are not in version control by themselves, so rollback is by re-running the prior call.
6464

65-
- **Branch protection**: snapshot before any change with `gh api /repos/RandomCodeSpace/codeiq/branches/main/protection > /tmp/bp-before.json`. To roll back: `cat /tmp/bp-before.json | gh api -X PUT /repos/RandomCodeSpace/codeiq/branches/main/protection --input -`.
65+
- **Branch protection**: snapshot before any change with `gh api /repos/RandomCodeSpace/codeiq/branches/main/protection > /tmp/bp-before.json`. The GET payload is a denormalized view that GitHub's PUT endpoint does **not** accept verbatim (PUT flattens the nested objects: `enforce_admins.enabled` → bare boolean, `required_status_checks.checks[].context` strings → flat `contexts[]`, `*.url` fields are rejected). Reshape with the jq filter below before piping into PUT (Reviewer finding fd559a54, R5-5):
66+
67+
```bash
68+
jq '{
69+
required_status_checks: (
70+
if .required_status_checks == null then null
71+
else {
72+
strict: .required_status_checks.strict,
73+
contexts: ([.required_status_checks.checks[]?.context] // [])
74+
}
75+
end
76+
),
77+
enforce_admins: (.enforce_admins.enabled // false),
78+
required_pull_request_reviews: (
79+
if .required_pull_request_reviews == null then null
80+
else {
81+
dismiss_stale_reviews: (.required_pull_request_reviews.dismiss_stale_reviews // false),
82+
require_code_owner_reviews: (.required_pull_request_reviews.require_code_owner_reviews // false),
83+
required_approving_review_count: (.required_pull_request_reviews.required_approving_review_count // 1)
84+
}
85+
end
86+
),
87+
restrictions: null,
88+
required_linear_history: (.required_linear_history.enabled // false),
89+
allow_force_pushes: (.allow_force_pushes.enabled // false),
90+
allow_deletions: (.allow_deletions.enabled // false),
91+
block_creations: (.block_creations.enabled // false),
92+
required_conversation_resolution: (.required_conversation_resolution.enabled // false),
93+
lock_branch: (.lock_branch.enabled // false),
94+
allow_fork_syncing: (.allow_fork_syncing.enabled // false)
95+
}' /tmp/bp-before.json \
96+
| gh api -X PUT /repos/RandomCodeSpace/codeiq/branches/main/protection --input -
97+
```
98+
99+
The transform unwraps the `{enabled: bool}` envelopes, projects `checks[].context` strings out into the flat `contexts[]` PUT expects, drops `*.url` fields, and forces `restrictions: null` (apps/teams/users restrictions are out of scope for this repo). If you need to *change* a field instead of rolling back, edit the transformed payload before piping.
66100
- **CodeQL default setup**: re-toggle via Repository Settings → Code security → Code scanning. The disabled state is the safe default.
67101
- **Dependabot security updates**: `gh api -X PUT /repos/RandomCodeSpace/codeiq/automated-security-fixes` to enable, `-X DELETE` to disable.
68102
- **Workflow files** (`.github/workflows/*.yml`): revert via §2 — they are version-controlled.

0 commit comments

Comments
 (0)