Skip to content

docs(#3508): add AGENTS.md review guidance for smoke-tests-native - #3510

Open
fullsend-ai-coder[bot] wants to merge 1 commit into
mainfrom
agent/3508-smoke-tests-native-guidance
Open

docs(#3508): add AGENTS.md review guidance for smoke-tests-native#3510
fullsend-ai-coder[bot] wants to merge 1 commit into
mainfrom
agent/3508-smoke-tests-native-guidance

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

Add a new "Reviewing smoke-tests-native Changes" section to AGENTS.md after the E2E Nightly Fix Conventions section. This gives the review agent domain-specific criteria for evaluating changes to the smoke-tests-native validation harness, covering three areas:

  • Untrusted input handling: adversarial filesystem paths (NUL bytes,
    overlong names, empty strings), typeof-array gotcha, and per-plugin
    error isolation to prevent sweep-level crashes
  • Consumer-path verification: ensuring validation targets match what
    the RHDH runtime actually reads (e.g. dist-scalprum/configSchema.json
    vs the CLI default dist/.config-schema.json)
  • Error propagation: distinguishing I/O errors from "not declared"
    or "not found" to avoid swallowing real problems

This parallels the existing "Major Version Bumps in Patches" section which provides equivalent domain-specific review criteria for dependency patches. The guidance is derived from 12 bugs self-found during PR #3486 review, where the review bot had zero correctness findings across 5 review runs due to lack of subsystem context.


Closes #3508

Post-script verification

  • Branch is not main/master (agent/3508-smoke-tests-native-guidance)
  • Secret scan passed (gitleaks — f0072e14593191839651d4f8a5bd3b27a604d860..HEAD)
  • Pre-commit hooks passed (authoritative run on runner)
  • Tests ran inside sandbox

Add a new "Reviewing smoke-tests-native Changes" section to AGENTS.md
after the E2E Nightly Fix Conventions section. This gives the review
agent domain-specific criteria for evaluating changes to the
smoke-tests-native validation harness, covering three areas:

- Untrusted input handling: adversarial filesystem paths (NUL bytes,
  overlong names, empty strings), typeof-array gotcha, and per-plugin
  error isolation to prevent sweep-level crashes
- Consumer-path verification: ensuring validation targets match what
  the RHDH runtime actually reads (e.g. dist-scalprum/configSchema.json
  vs the CLI default dist/.config-schema.json)
- Error propagation: distinguishing I/O errors from "not declared"
  or "not found" to avoid swallowing real problems

This parallels the existing "Major Version Bumps in Patches" section
which provides equivalent domain-specific review criteria for
dependency patches. The guidance is derived from 12 bugs self-found
during PR #3486 review, where the review bot had zero correctness
findings across 5 review runs due to lack of subsystem context.

Closes #3508
@openshift-ci

openshift-ci Bot commented Sep 1, 2026

Copy link
Copy Markdown

Hi @fullsend-ai-coder[bot]. Thanks for your PR.

I'm waiting for a redhat-developer member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@sonarqubecloud

sonarqubecloud Bot commented Sep 1, 2026

Copy link
Copy Markdown

@fullsend-ai-review

fullsend-ai-review Bot commented Sep 1, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 8:13 PM UTC · Completed 8:17 PM UTC

Commit: 2460e92 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Cost: $0.78

@fullsend-ai-review

Copy link
Copy Markdown

Review — approve

Confidence: high

This PR adds a well-structured "Reviewing smoke-tests-native Changes" section to AGENTS.md, providing domain-specific review guidance for the smoke-tests-native/ validation harness. The addition is motivated by 12 self-found bugs in PR #3486 that the review bot missed across 5 review runs, and every technical claim in the guidance has been verified against the current codebase.

Verification of technical claims

All six key assertions in the new guidance were cross-referenced against smoke-tests-native/src/loader.ts and smoke-tests-native/src/paths.ts:

Claim Verified at Status
statSync throws on NUL-byte/overlong paths; throwIfNoEntry: false only suppresses ENOENT isFile() at loader.ts:610-622 — wraps in try/catch with detailed comment
Empty-string loadScripts resolves to parent dir via resolveContained("") readScalprumFields at loader.ts:649-653 — filters with explanatory comment
typeof [] === "object" does not exclude arrays readConfigSchemaFile at loader.ts:785-793 — uses !Array.isArray(properties) with comment
Single artifact failure must not crash the sweep isFile() comment at loader.ts:614 — explains workspace-level collapse risk
RHDH reads dist-scalprum/configSchema.json, not dist/.config-schema.json Constants at loader.ts:743-746; ConfigSchemaConsumer type at loader.ts:277-298
nfsFeaturesError pattern for distinguishing I/O errors from "not declared" MfRemoteInfo.nfsFeaturesError at loader.ts:196; ConfigSchemaInfo.declaredError at loader.ts:337

Structure and placement

The new section is inserted between "E2E Nightly Fix Conventions" and "Documentation" — a logical location that keeps domain-specific review guidance grouped. The three subsections (untrusted input handling, consumer-path verification, error propagation) parallel the structure of the existing "Major Version Bumps in Patches" section, which provides equivalent criteria for dependency patches.

Scope assessment

The change is documentation-only (20 additions, 0 deletions, single file). It does not modify any code, configuration, or CI workflows. The guidance is accurate, actionable, and appropriately scoped to the smoke-tests-native/ subsystem.

No findings.


Protected paths detected — this PR modifies files under one or more
protected paths. The review agent cannot approve PRs that touch these paths.
A human reviewer must approve this PR.

Protected files in this PR:

  • AGENTS.md

@fullsend-ai-review fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Sep 1, 2026
@github-actions github-actions Bot added the non-workspace-changes PR changes files outside workspace directories label Sep 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ok-to-test non-workspace-changes PR changes files outside workspace directories requires-manual-review Review requires human judgment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add AGENTS.md review guidance for smoke-tests-native validation code

0 participants