Skip to content

test: normalize inline block comments in gatekeeper scanning - #2705

Merged
steipete merged 1 commit into
mainfrom
steipete/gatekeeper-close
Aug 6, 2026
Merged

test: normalize inline block comments in gatekeeper scanning#2705
steipete merged 1 commit into
mainfrom
steipete/gatekeeper-close

Conversation

@steipete

@steipete steipete commented Aug 6, 2026

Copy link
Copy Markdown
Owner

Closing commit of the provider-architecture campaign. Fixes the round-nine audit's lexical-normalization gap — inline /* ... */ comments are blanked with index-preserving spaces before scanning, so punctuation adjacency stays visible (regression: assignment and labeled-argument forms) — and documents the two genuinely parser-requiring patterns (nested literals inside suppressed calls; multi-line block comments interleaved with expressions) as out of scope with reasons.

Proof: gatekeeper 38/38 zero findings, make check 0 violations, full suite green (zero failed/timed-out/retried groups), autoreview clean.

🤖 Generated with Claude Code

Blank single-line block comments with index-preserving spaces so
punctuation adjacency stays visible to position heuristics; document
nested-suppressed-call attribution and multi-line block comments as
parser-requiring out-of-scope patterns.
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. labels Aug 6, 2026
@clawsweeper

clawsweeper Bot commented Aug 6, 2026

Copy link
Copy Markdown

Codex review: needs changes before merge. Reviewed August 6, 2026, 4:14 AM ET / 08:14 UTC.

ClawSweeper review

What this changes

The PR blanks same-line block comments before provider-reference scanning, adds regression coverage for assignment and labeled-argument forms, and documents parser-level exclusions.

Merge readiness

⚠️ Ready for maintainer review - 3 items remain

Keep open: the normalization is narrowly scoped, but it stops at the first closing delimiter and can expose provider literals inside nested Swift block comments, producing false gatekeeper findings.

Priority: P3
Reviewed head: 8a5a8e4be7ad18207675b08188a0562d026a731e

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) The patch is focused but has one actionable lexical-correctness gap.
Proof confidence 🌊 off-meta tidepool Not applicable: This owner-authored internal scanner change is appropriately evaluated through focused source-level regression coverage rather than external contributor proof.
Patch quality 🦐 gold shrimp (3/6) 1 actionable review finding remain.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: This owner-authored internal scanner change is appropriately evaluated through focused source-level regression coverage rather than external contributor proof.
Evidence reviewed 3 items Proposed normalizer: The new loop blanks through the first */ after a /*, then resumes scanning; it does not count nested delimiters.
Current-main provenance: The current main baseline for this gatekeeper expansion is commit 5cb69f0, authored by Peter Steinberger immediately before this PR branch.
Focused diff: The branch changes only the gatekeeper test harness and provider-architecture documentation: 39 additions and 9 deletions across two files.
Findings 1 actionable finding [P2] Handle nested block comments before resuming the scan
Security None None.

How this fits together

CodexBar’s provider-architecture gatekeeper lexically scans shipped Swift source for direct provider references that bypass the shared abstraction. Its normalized source lines feed position heuristics that produce test-enforced architecture findings.

flowchart LR
A[Swift source lines] --> B[Comment normalization]
B --> C[Provider-reference scanner]
C --> D[Position heuristics]
D --> E[Gatekeeper findings]
E --> F[Architecture tests]
Loading

Before merge

  • Handle nested block comments before resuming the scan (P2) - Swift block comments can nest. This loop stops at the first */, so let provider = /* outer /* .claude */ still comment */ .codex leaves .claude in normalized code and reports a false architecture finding. Track nested delimiters and add this regression before continuing the line scan.
  • Resolve merge risk (P1) - Swift permits nested block comments; merging this first-closing-delimiter scan can create false provider-architecture findings when a provider literal appears inside a nested comment.
  • Complete next step (P2) - The nested-comment defect has a narrow mechanical repair in the submitted scanner.

Findings

  • [P2] Handle nested block comments before resuming the scan — Tests/CodexBarTests/ProviderArchitectureGatekeeperTests.swift:4427-4429
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Changed surface 2 files; production +0, tests +26 net, docs +4 net The patch is confined to the lexical test harness and its provider-architecture guidance.

Merge-risk options

Maintainer options:

  1. Handle nested block comments (recommended)
    Count nested /* and */ delimiters while preserving character positions, then cover a provider literal inside a nested comment.
Copy recommended automerge instruction
@clawsweeper automerge

Special instructions:
Preserve offset-stable normalization, track nested same-line block-comment delimiters, and add a regression proving provider literals inside nested comments remain ignored.

Technical review

Best possible solution:

Preserve offset-stable blanking while tracking nested same-line block-comment depth, with a regression proving inner commented provider literals remain ignored and real literals after the comment remain visible.

Do we have a high-confidence way to reproduce the issue?

Yes. Source inspection shows that a nested same-line Swift block comment causes the new loop to stop at the inner closing delimiter and scan the remaining commented text.

Is this the best way to solve the issue?

No. Index-preserving blanking is the right approach, but it must track nested comment depth to respect Swift block-comment syntax.

Full review comments:

  • [P2] Handle nested block comments before resuming the scan — Tests/CodexBarTests/ProviderArchitectureGatekeeperTests.swift:4427-4429
    Swift block comments can nest. This loop stops at the first */, so let provider = /* outer /* .claude */ still comment */ .codex leaves .claude in normalized code and reports a false architecture finding. Track nested delimiters and add this regression before continuing the line scan.
    Confidence: 0.96

Overall correctness: patch is incorrect
Overall confidence: 0.96

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 5cb69f0c106f.

Labels

Label justifications:

  • P3: This is a limited internal scanner-maintenance change without direct runtime-user impact.
  • merge-risk: 🚨 other: A nested-comment false positive can block future architecture checks, a concrete risk outside the specific owned merge-risk classes.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🌊 off-meta tidepool and patch quality is 🦐 gold shrimp.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: This owner-authored internal scanner change is appropriately evaluated through focused source-level regression coverage rather than external contributor proof.

Evidence

Acceptance criteria:

  • [P1] swift test --filter ProviderArchitectureGatekeeperTests.
  • [P1] make check.

What I checked:

Likely related people:

  • steipete: Peter Steinberger authored the current-main gatekeeper expansion and this focused follow-up branch. (role: introduced current gatekeeper behavior and recent area contributor; confidence: high; commits: 5cb69f0c106f, 8a5a8e4be7ad; files: Tests/CodexBarTests/ProviderArchitectureGatekeeperTests.swift, docs/provider.md)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Track nested same-line block comments and add a regression proving provider literals inside them remain ignored.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@steipete
steipete merged commit 005a71f into main Aug 6, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant