ci(qualification): bootstrap trusted draft gate - #8656
Conversation
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
📝 WalkthroughWalkthroughAdds the OpenShell 0.0.101 qualification manifest, secure contract and file validation, pull-request verifier, trusted-base workflow, and comprehensive tests for draft qualification enforcement. ChangesOpenShell qualification gate
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant PullRequest
participant classify
participant verifyDraft
participant report
PullRequest->>classify: submit pull-request metadata
classify->>classify: classify qualification-sensitive paths
classify-->>verifyDraft: required and same-repository outputs
verifyDraft->>verifyDraft: validate trusted and candidate draft data
verifyDraft-->>report: verification result
report->>report: report final qualification status
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 4d5104c in the TypeScript / code-coverage/cliThe overall coverage in commit 4d5104c in the Show a code coverage summary of the most impacted files.
Updated |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
12 terminology differences from the second opinionAdvisory only. These are normalized differences from the primary terminology receipt.
Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests. 3 semantic terminology decisionsTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
E2E guidanceAdvisory only. A maintainer can dispatch the default E2E suite against this exact revision. Recommended E2E: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (13)
scripts/checks/openshell-qualification-io.mts (1)
236-238: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSimplify the redundant root symlink check.
fs.lstatSyncon a symlink that points to a directory returnsisDirectory() === false. The!rootStats.isDirectory()clause already rejects that case, sorootStats.isSymbolicLink()never changes the outcome. The behavior is correct; the second clause is dead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/checks/openshell-qualification-io.mts` around lines 236 - 238, Simplify the root validation condition in the relevant qualification check by removing the redundant rootStats.isSymbolicLink() clause, leaving the isDirectory() check to reject non-directory roots while preserving the existing failure behavior.scripts/checks/openshell-qualification-bootstrap-contract.mts (2)
277-279: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDerive the pinned version from
FIXED_FIELDSinstead of repeating the literal.
"0.0.99"duplicatesFIXED_FIELDS.openshellRepositoryBaselineVersionon line 63. If the baseline moves, the manifest and this guard drift apart silently, and the gate then rejects every valid transition. Read the value from the canonical constant.As per path instructions for
scripts/checks/**: "Derive inventories and limits from a canonical source where possible; flag duplicated lists that can silently drift."♻️ Proposed change
- if (versions.baseVersion !== "0.0.99" || versions.candidateVersion !== "0.0.99") { + const pinnedVersion = FIXED_FIELDS.openshellRepositoryBaselineVersion; + if (versions.baseVersion !== pinnedVersion || versions.candidateVersion !== pinnedVersion) { failQualificationGate("draft bootstrap cannot change the pinned OpenShell version"); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/checks/openshell-qualification-bootstrap-contract.mts` around lines 277 - 279, Update the pinned-version check in the qualification gate to compare both version fields against FIXED_FIELDS.openshellRepositoryBaselineVersion instead of repeating the "0.0.99" literal. Preserve the existing rejection behavior when either version differs.Source: Path instructions
257-261: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReturn fresh empty arrays instead of the shared
FIXED_FIELDSinstances.
...FIXED_FIELDScopies theartifactsarray by reference. Every validated contract returned by this function shares one array instance. A consumer that mutatesartifactswould corrupt later validations. Theas consttype prevents this at compile time only, so astructuredCloneboundary or an explicitartifacts: []keeps the runtime guarantee.♻️ Proposed change
return { ...FIXED_FIELDS, + artifacts: [], requiredWorkflowGate: validateRequiredWorkflowGate(value.requiredWorkflowGate), tests, };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/checks/openshell-qualification-bootstrap-contract.mts` around lines 257 - 261, Update the validated contract return construction to avoid reusing the `artifacts` array from `FIXED_FIELDS`; explicitly provide a fresh empty `artifacts` array while preserving the other fixed fields and validation results. Ensure each invocation of the surrounding validation function returns an independently mutable array.test/openshell-qualification-paths.test.ts (1)
21-41: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the qualification manifest and the path module itself to this table.
The table omits
ci/openshell-0.0.101-qualification-v1.json. That file is the object the whole gate protects, andverifyDraftPullRequestGatecallsclassifyQualificationbefore it accepts a draft verification request. The table also omitsscripts/checks/openshell-qualification-paths.mtsandscripts/checks/openshell-qualification-io.mts, which are the self-protection entries. Both additions are one line each and lock the most important entries ofSENSITIVE_EXACT_PATHS.💚 Proposed change
it.each([ + "ci/openshell-0.0.101-qualification-v1.json", "nemoclaw-blueprint/blueprint.yaml", "scripts/install-openshell.sh", @@ "scripts/checks/openshell-qualification-bootstrap-contract.mts", "scripts/checks/openshell-qualification-contract.mts", "scripts/checks/openshell-qualification-github.mts", + "scripts/checks/openshell-qualification-io.mts", + "scripts/checks/openshell-qualification-paths.mts", "scripts/checks/verify-openshell-qualification-producer-workflow.mts",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/openshell-qualification-paths.test.ts` around lines 21 - 41, Add the qualification manifest path ci/openshell-0.0.101-qualification-v1.json and the self-protection modules scripts/checks/openshell-qualification-paths.mts and scripts/checks/openshell-qualification-io.mts to the it.each table for isOpenShellQualificationSensitivePath. Keep the existing assertions and path list unchanged otherwise.test/openshell-qualification-bootstrap-contract.test.ts (1)
101-103: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the exact authority-rewrite message.
"cannot change"is also a substring of"draft bootstrap cannot change the pinned OpenShell version". The assertion therefore does not prove that the authority-rewrite branch on line 291 ofscripts/checks/openshell-qualification-bootstrap-contract.mtsrejected the input. Match the specific message so the test fails if the rejection moves to another branch.As per path instructions for
**/*.test.{ts,js,mts,mjs,cts,cjs}: "Flag ... conditionals that make a test pass without exercising its claim."💚 Proposed change
expect(() => validateBootstrapDraftTransition(governedBase, rewritten, VERSIONS)).toThrow( - "cannot change", + "established required workflow authority cannot change", );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/openshell-qualification-bootstrap-contract.test.ts` around lines 101 - 103, Update the assertion around validateBootstrapDraftTransition to match the exact authority-rewrite rejection message, rather than the ambiguous "cannot change" substring. Preserve the existing test input and ensure the expectation specifically identifies the authority-rewrite branch so another validation error cannot satisfy the test.Source: Path instructions
test/openshell-qualification-pr-gate-workflow.test.ts (1)
29-39: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove the duplicated workflow-step helpers into the shared contract helper. Both test files define near-identical
stepandsparsePathsfunctions becausetest/helpers/e2e-workflow-contract.tsexposes onlyreadYamland the types. The two copies already differ in signature and will drift.
test/openshell-qualification-pr-gate-workflow.test.ts#L29-L39: delete the localstepandsparsePaths, and import them from./helpers/e2e-workflow-contract.test/openshell-qualification-sparse-bootstrap-bundle.test.ts#L23-L35: delete the localstepandsparsePaths, and import the same shared helpers.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/openshell-qualification-pr-gate-workflow.test.ts` around lines 29 - 39, Move the duplicated step and sparsePaths helpers into test/helpers/e2e-workflow-contract.ts, exporting shared implementations that preserve the required signatures and behavior. In test/openshell-qualification-pr-gate-workflow.test.ts lines 29-39 and test/openshell-qualification-sparse-bootstrap-bundle.test.ts lines 23-35, delete the local step and sparsePaths definitions and import the shared helpers instead.test/openshell-qualification-pr-gate.test.ts (2)
191-201: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove or retarget the duplicate byte-identity assertion.
Both blueprint mutations exercise the same branch in
validateBlueprintPair, and both assert the same message. The second case adds no new behavior. The test title claims coverage of "OpenShell version movement", but the verifier never parses the version; it compares bytes and a pinned digest. Either delete the second case or assert a distinct failure, such as a candidate blueprint that exceedsMAX_BLUEPRINT_BYTES.As per path instructions: "Flag copied production algorithms, broad mocks that bypass the behavior under test, and conditionals that make a test pass without exercising its claim."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/openshell-qualification-pr-gate.test.ts` around lines 191 - 201, Remove the second blueprint mutation and duplicate byte-identity assertion from the test, or retarget it to a distinct behavior such as exceeding MAX_BLUEPRINT_BYTES. Ensure the test title and assertions correspond to behavior actually exercised by validateBlueprintPair rather than unparsed OpenShell version changes.Source: Path instructions
76-86: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winResolve repository files from the module URL, not the working directory.
writeDraftRootanddraftContractreadnemoclaw-blueprint/blueprint.yamlandci/openshell-0.0.101-qualification-v1.jsonthrough paths relative toprocess.cwd().test/openshell-qualification-sparse-bootstrap-bundle.test.tsderivesREPO_ROOTfromimport.meta.urlfor the same repository files. Use the same approach here so the tests do not depend on the Vitest project root.♻️ Proposed change
+import { fileURLToPath } from "node:url"; + +const REPO_ROOT = path.join(path.dirname(fileURLToPath(import.meta.url)), ".."); + function writeDraftRoot(root: string, contract: Record<string, unknown>): void { fs.mkdirSync(path.join(root, "ci"), { recursive: true }); fs.mkdirSync(path.join(root, "nemoclaw-blueprint"), { recursive: true }); fs.writeFileSync( path.join(root, "ci/openshell-0.0.101-qualification-v1.json"), JSON.stringify(contract), ); fs.writeFileSync( path.join(root, "nemoclaw-blueprint/blueprint.yaml"), - fs.readFileSync("nemoclaw-blueprint/blueprint.yaml"), + fs.readFileSync(path.join(REPO_ROOT, "nemoclaw-blueprint/blueprint.yaml")), ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/openshell-qualification-pr-gate.test.ts` around lines 76 - 86, Update writeDraftRoot and draftContract to resolve blueprint.yaml and the qualification contract from the repository root derived from import.meta.url, matching the REPO_ROOT approach in openshell-qualification-sparse-bootstrap-bundle.test.ts; remove their dependence on process.cwd() while preserving the existing file contents and parsing behavior..github/workflows/openshell-0.0.101-pr-gate.yaml (1)
19-27: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAdd a concurrency group for superseded pull-request events.
Each
synchronizeevent starts a new gate run. The verifier fails when the head SHA moves, so stale runs waste runner time and report confusing failures. A per-pull-request concurrency group withcancel-in-progress: trueremoves the stale runs.♻️ Proposed change
permissions: contents: read pull-requests: read +concurrency: + group: openshell-qualification-${{ github.event.pull_request.number }} + cancel-in-progress: true + jobs:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/openshell-0.0.101-pr-gate.yaml around lines 19 - 27, Add a workflow-level or job-level concurrency configuration for the `classify` job using a per-pull-request group key, and set `cancel-in-progress: true` so newer events cancel superseded runs while keeping separate pull requests isolated.scripts/checks/verify-openshell-qualification-pr-gate.mts (3)
370-390: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winReject output values that contain newlines.
appendGitHubOutputwriteskey=valuelines without escaping. Boolean values andJSON.stringifyoutput are safe today because JSON escapes control characters. A future caller that passes a raw filename can inject extra output keys. Add an explicit guard so the invariant does not depend on the caller.🛡️ Proposed guard
const source = Object.entries(values) - .map(([key, value]) => `${key}=${value}\n`) + .map(([key, value]) => { + if (/[\r\n]/u.test(key) || /[\r\n]/u.test(value)) { + fail("GitHub output entry contains a line break"); + } + return `${key}=${value}\n`; + }) .join("");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/checks/verify-openshell-qualification-pr-gate.mts` around lines 370 - 390, Update appendGitHubOutput to validate every output key and value before building the source string, rejecting any that contain newline characters. Preserve the existing key=value line format and fail through the established fail function so raw caller-provided values cannot inject additional GitHub output entries.
263-296: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueShare the blueprint read and digest logic.
readBlueprintVersionandvalidateBlueprintPairrepeat the same bounded read, the same digest comparison, and the same hardcoded"0.0.99"return. A single helper keeps the pinned baseline in one place.♻️ Proposed refactor
+function readTrustedBlueprint(root: string, label: string): Buffer { + return readBoundedRegularFileFromRoot(root, "nemoclaw-blueprint/blueprint.yaml", label, { + maximumBytes: MAX_BLUEPRINT_BYTES, + minimumBytes: 1, + }); +} + export function readBlueprintVersion(root: string): string { - const bytes = readBoundedRegularFileFromRoot( - root, - "nemoclaw-blueprint/blueprint.yaml", - "OpenShell version blueprint", - { maximumBytes: MAX_BLUEPRINT_BYTES, minimumBytes: 1 }, - ); + const bytes = readTrustedBlueprint(root, "OpenShell version blueprint"); if (createHash("sha256").update(bytes).digest("hex") !== REVIEWED_BLUEPRINT_SHA256) { fail("trusted OpenShell version blueprint does not match the reviewed baseline"); } - return "0.0.99"; + return PINNED_OPENSHELL_VERSION; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/checks/verify-openshell-qualification-pr-gate.mts` around lines 263 - 296, Extract the shared trusted-blueprint read, SHA-256 validation against REVIEWED_BLUEPRINT_SHA256, and version return from readBlueprintVersion into a helper. Update both readBlueprintVersion and validateBlueprintPair to reuse that helper while preserving the candidate byte-identity check in validateBlueprintPair.
298-327: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConfirm the draft transition version arguments.
validateBootstrapDraftTransitionfails unless both versions equal"0.0.99", andvalidateBlueprintPairalways returns"0.0.99". The pinned constant is therefore duplicated across two modules. Confirm that both sites stay in sync when the program moves to 0.0.101, and consider exporting one constant from the contract module.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/checks/verify-openshell-qualification-pr-gate.mts` around lines 298 - 327, Centralize the pinned qualification blueprint version used by validateBlueprintPair and validateBootstrapDraftTransition by exporting a shared version constant from the contract module. Update both consumers, including verifyDraftPullRequestGate, to use that constant so they remain synchronized when advancing from 0.0.99 to a future version such as 0.0.101.test/openshell-qualification-sparse-bootstrap-bundle.test.ts (1)
101-107: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueInclude stderr in the failure message.
Line 88 passes
result.stderras the assertion message. Line 105 does not. Add the same diagnostic so a bundle failure reports the child-process error.♻️ Proposed change
- expect(importVerifier(root).status).toBe(0); + const result = importVerifier(root); + + expect(result.status, result.stderr).toBe(0); expect(bundleFiles(root)).toEqual([...paths].sort());🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/openshell-qualification-sparse-bootstrap-bundle.test.ts` around lines 101 - 107, Update the assertion in the “constructs the verify bundle with contract and blueprint data (`#8590`)” test to pass the verify process’s stderr as the assertion failure message, matching the existing diagnostic pattern at line 88 and preserving the status expectation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/checks/openshell-qualification-paths.mts`:
- Around line 117-130: Update the status handling around the validation logic to
treat both “renamed” and “copied” entries as requiring previous_filename
validation and movement checks, while preserving rejection of previous_filename
for other statuses. Apply the same classification change in the duplicate
validator in verify-openshell-e2e-qualification.mts, and add regression coverage
for copied entries with valid and invalid previous_filename values.
In `@test/openshell-qualification-io.test.ts`:
- Around line 112-133: Update the test around readBoundedRegularFileBytes to
make the overwrite detectable without filesystem timestamp granularity: after
creating source.txt and before invoking the read, explicitly set its
modification time to an older value using the existing fs utilities. Preserve
the mocked fstatSync sequence and assertion that the read throws when the file
changes.
In `@test/openshell-qualification-pr-gate-workflow.test.ts`:
- Around line 42-106: Move the inline assertions from the spawned --eval program
into the test body of the workflow contract test. Reuse the existing job, step,
and sparsePaths helpers defined earlier in the file, remove the spawnSync
child-process setup and unused-helper condition, and preserve all current
structural assertions against the parsed workflow.
- Around line 199-208: Update the parameterized “enforces $disposition behavior”
test around job() and spawnSync() to assert the emitted ::error:: message as
well as result.status. Add the expected failure message to each disposition
case, including distinct text for branches such as malformed applicability
output and sensitive fork, and verify it from result.stdout or result.stderr
according to where the report script emits errors.
In `@test/openshell-qualification-pr-gate.test.ts`:
- Around line 204-219: Update the test around readBlueprintVersion to assert the
complete, owning-module failure messages for both the symlink and oversized-file
cases. Use the relevant message constants from openshell-qualification-io.mts if
available, ensuring the assertions remain distinct rather than matching the
shared substring.
In `@test/openshell-qualification-sparse-bootstrap-bundle.test.ts`:
- Around line 117-123: Remove the tautological fs.existsSync exclusion assertion
following the exact-set assertion in the sparse bootstrap bundle test. Keep the
existing exact-set assertion as the verification that the constructed bundle
contains only the declared sparse paths and no receipt files; do not add
unrelated checks against paths constructBundle cannot copy.
---
Nitpick comments:
In @.github/workflows/openshell-0.0.101-pr-gate.yaml:
- Around line 19-27: Add a workflow-level or job-level concurrency configuration
for the `classify` job using a per-pull-request group key, and set
`cancel-in-progress: true` so newer events cancel superseded runs while keeping
separate pull requests isolated.
In `@scripts/checks/openshell-qualification-bootstrap-contract.mts`:
- Around line 277-279: Update the pinned-version check in the qualification gate
to compare both version fields against
FIXED_FIELDS.openshellRepositoryBaselineVersion instead of repeating the
"0.0.99" literal. Preserve the existing rejection behavior when either version
differs.
- Around line 257-261: Update the validated contract return construction to
avoid reusing the `artifacts` array from `FIXED_FIELDS`; explicitly provide a
fresh empty `artifacts` array while preserving the other fixed fields and
validation results. Ensure each invocation of the surrounding validation
function returns an independently mutable array.
In `@scripts/checks/openshell-qualification-io.mts`:
- Around line 236-238: Simplify the root validation condition in the relevant
qualification check by removing the redundant rootStats.isSymbolicLink() clause,
leaving the isDirectory() check to reject non-directory roots while preserving
the existing failure behavior.
In `@scripts/checks/verify-openshell-qualification-pr-gate.mts`:
- Around line 370-390: Update appendGitHubOutput to validate every output key
and value before building the source string, rejecting any that contain newline
characters. Preserve the existing key=value line format and fail through the
established fail function so raw caller-provided values cannot inject additional
GitHub output entries.
- Around line 263-296: Extract the shared trusted-blueprint read, SHA-256
validation against REVIEWED_BLUEPRINT_SHA256, and version return from
readBlueprintVersion into a helper. Update both readBlueprintVersion and
validateBlueprintPair to reuse that helper while preserving the candidate
byte-identity check in validateBlueprintPair.
- Around line 298-327: Centralize the pinned qualification blueprint version
used by validateBlueprintPair and validateBootstrapDraftTransition by exporting
a shared version constant from the contract module. Update both consumers,
including verifyDraftPullRequestGate, to use that constant so they remain
synchronized when advancing from 0.0.99 to a future version such as 0.0.101.
In `@test/openshell-qualification-bootstrap-contract.test.ts`:
- Around line 101-103: Update the assertion around
validateBootstrapDraftTransition to match the exact authority-rewrite rejection
message, rather than the ambiguous "cannot change" substring. Preserve the
existing test input and ensure the expectation specifically identifies the
authority-rewrite branch so another validation error cannot satisfy the test.
In `@test/openshell-qualification-paths.test.ts`:
- Around line 21-41: Add the qualification manifest path
ci/openshell-0.0.101-qualification-v1.json and the self-protection modules
scripts/checks/openshell-qualification-paths.mts and
scripts/checks/openshell-qualification-io.mts to the it.each table for
isOpenShellQualificationSensitivePath. Keep the existing assertions and path
list unchanged otherwise.
In `@test/openshell-qualification-pr-gate-workflow.test.ts`:
- Around line 29-39: Move the duplicated step and sparsePaths helpers into
test/helpers/e2e-workflow-contract.ts, exporting shared implementations that
preserve the required signatures and behavior. In
test/openshell-qualification-pr-gate-workflow.test.ts lines 29-39 and
test/openshell-qualification-sparse-bootstrap-bundle.test.ts lines 23-35, delete
the local step and sparsePaths definitions and import the shared helpers
instead.
In `@test/openshell-qualification-pr-gate.test.ts`:
- Around line 191-201: Remove the second blueprint mutation and duplicate
byte-identity assertion from the test, or retarget it to a distinct behavior
such as exceeding MAX_BLUEPRINT_BYTES. Ensure the test title and assertions
correspond to behavior actually exercised by validateBlueprintPair rather than
unparsed OpenShell version changes.
- Around line 76-86: Update writeDraftRoot and draftContract to resolve
blueprint.yaml and the qualification contract from the repository root derived
from import.meta.url, matching the REPO_ROOT approach in
openshell-qualification-sparse-bootstrap-bundle.test.ts; remove their dependence
on process.cwd() while preserving the existing file contents and parsing
behavior.
In `@test/openshell-qualification-sparse-bootstrap-bundle.test.ts`:
- Around line 101-107: Update the assertion in the “constructs the verify bundle
with contract and blueprint data (`#8590`)” test to pass the verify process’s
stderr as the assertion failure message, matching the existing diagnostic
pattern at line 88 and preserving the status expectation.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: a017d661-8377-4dc7-9b19-9bfcce50f9de
📒 Files selected for processing (12)
.github/workflows/openshell-0.0.101-pr-gate.yamlci/openshell-0.0.101-qualification-v1.jsonscripts/checks/openshell-qualification-bootstrap-contract.mtsscripts/checks/openshell-qualification-io.mtsscripts/checks/openshell-qualification-paths.mtsscripts/checks/verify-openshell-qualification-pr-gate.mtstest/openshell-qualification-bootstrap-contract.test.tstest/openshell-qualification-io.test.tstest/openshell-qualification-paths.test.tstest/openshell-qualification-pr-gate-workflow.test.tstest/openshell-qualification-pr-gate.test.tstest/openshell-qualification-sparse-bootstrap-bundle.test.ts
| if (typeof value.status !== "string" || !KNOWN_FILE_STATUSES.has(value.status)) { | ||
| fail(`pull-request file ${filename} has unknown status`); | ||
| } | ||
| if (value.status === "renamed") { | ||
| const previousFilename = validateRepositoryPath( | ||
| value.previous_filename, | ||
| "renamed pull-request previous_filename", | ||
| ); | ||
| if (previousFilename === filename) fail(`renamed pull-request file ${filename} did not move`); | ||
| return { filename, previousFilename, status: value.status }; | ||
| } | ||
| if (value.previous_filename !== undefined) { | ||
| fail(`non-renamed pull-request file ${filename} unexpectedly has previous_filename`); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
GitHub REST API list pull request files previous_filename copied status
💡 Result:
The GitHub REST API endpoint for listing files in a pull request is GET /repos/{owner}/{repo}/pulls/{pull_number}/files [1][2]. The response for each file includes a status field, which indicates the nature of the change, and a previous_filename field [1][3]. Key details regarding these fields include: status: A string representing the file's status in the pull request [1][4]. It can take one of the following values: added, removed, modified, renamed, copied, changed, or unchanged [1][2][5]. previous_filename: A string that is returned when the status is renamed or copied [1][3]. This field contains the original filename of the file before it was renamed or copied, providing necessary context for tracking file movements or duplications [1][4]. When the status is not renamed or copied, this field may be null or omitted depending on the specific API implementation details, but it is explicitly defined in the Diff Entry schema to support these status types [1][5].
Citations:
- 1: https://docs.github.com/en/rest/pulls/pulls
- 2: https://docs.github.com/en/rest/pulls/pulls?apiVersion=2026-03-10
- 3: https://docs.github.com/en/enterprise-cloud@latest/rest/pulls/pulls
- 4: https://docs.github.com/en/enterprise-server@3.17/rest/pulls/pulls
- 5: https://docs.github.com/en/enterprise-server@3.18/rest/pulls/pulls
🏁 Script executed:
#!/bin/bash
set -eu
file=$(git ls-files | grep -F 'scripts/checks/openshell-qualification-paths.mts' | head -n1)
printf '%s\n' "$file"
sed -n '1,180p' "$file"
printf '\n--- related status handling ---\n'
rg -n --glob '*.mts' --glob '*.ts' 'KNOWN_FILE_STATUSES|previous_filename|status === "copied"|status === "renamed"' scriptsRepository: NVIDIA/NemoClaw
Length of output: 8255
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- relevant tests and fixtures ---'
rg -n -C 5 --glob '*.{test,spec}.{ts,mts,js}' --glob '*.json' \
'validatePullRequestFile|pathsForFile|copied|previous_filename|non-renamed pull-request file' .
printf '%s\n' '--- duplicate implementation context ---'
sed -n '340,400p' scripts/checks/verify-openshell-e2e-qualification.mts
printf '%s\n' '--- standalone behavior probe ---'
python3 - <<'PY'
from dataclasses import dataclass
`@dataclass`
class File:
filename: str
previous_filename: str | None
status: str
def paths_for_file(file):
return [file.previous_filename, file.filename] if file.previous_filename else [file.filename]
for status in ("renamed", "copied", "modified"):
item = File("new/path", "old/path", status)
print(status, paths_for_file(item))
PYRepository: NVIDIA/NemoClaw
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- focused path-check tests ---'
sed -n '1,115p' test/openshell-qualification-paths.test.ts
printf '%s\n' '--- duplicate validator and its tests ---'
sed -n '350,395p' scripts/checks/verify-openshell-e2e-qualification.mts
sed -n '255,305p' test/openshell-e2e-qualification.test.ts
printf '%s\n' '--- callers ---'
rg -n 'validatePullRequestFile|loadPullRequestFiles|verifyOpenShellE2EQualification' \
scripts/checks/openshell-qualification-paths.mts \
scripts/checks/verify-openshell-e2e-qualification.mts \
test/openshell-qualification-paths.test.ts \
test/openshell-e2e-qualification.test.tsRepository: NVIDIA/NemoClaw
Length of output: 11949
Handle copied files with previous_filename
GitHub documents previous_filename for both renamed and copied entries. The current check rejects copied entries before classifying both paths. Handle copied entries with the same validation path as renamed entries, update the duplicate validator in scripts/checks/verify-openshell-e2e-qualification.mts, and add regression coverage.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/checks/openshell-qualification-paths.mts` around lines 117 - 130,
Update the status handling around the validation logic to treat both “renamed”
and “copied” entries as requiring previous_filename validation and movement
checks, while preserving rejection of previous_filename for other statuses.
Apply the same classification change in the duplicate validator in
verify-openshell-e2e-qualification.mts, and add regression coverage for copied
entries with valid and invalid previous_filename values.
| it("rejects same-size input overwritten after descriptor authentication (#8590)", () => { | ||
| const root = tempRoot(); | ||
| const source = path.join(root, "source.txt"); | ||
| fs.writeFileSync(source, "SAFE"); | ||
| const fstatSync = fs.fstatSync.bind(fs); | ||
| vi.spyOn(fs, "fstatSync") | ||
| .mockImplementationOnce((descriptor, options) => { | ||
| const stats = fstatSync(descriptor, options as { bigint: true }); | ||
| fs.writeFileSync(source, "EVIL"); | ||
| return stats; | ||
| }) | ||
| .mockImplementation((descriptor, options) => | ||
| fstatSync(descriptor, options as { bigint: true }), | ||
| ); | ||
|
|
||
| expect(() => | ||
| readBoundedRegularFileBytes(source, "overwritten input", { | ||
| maximumBytes: 8, | ||
| minimumBytes: 1, | ||
| }), | ||
| ).toThrow("changed while it was being read"); | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
This test depends on filesystem timestamp granularity and can flake.
The overwrite keeps dev, ino, mode, nlink, and size identical, because fs.writeFileSync truncates the same inode and writes 4 bytes again. The only fields that differ are mtimeNs and ctimeNs. Linux updates inode timestamps from the coarse-grained kernel clock, whose granularity is typically one timer tick. The mocked fstatSync and the overwrite run microseconds apart, so both stats can report the same mtimeNs and ctimeNs. The read then succeeds and the test fails.
Make the change detectable without relying on clock resolution. Set an explicit older mtime on the file before the read, so the post-write timestamp is always different.
💚 Proposed change
fs.writeFileSync(source, "SAFE");
+ const stale = new Date(Date.now() - 60_000);
+ fs.utimesSync(source, stale, stale);
const fstatSync = fs.fstatSync.bind(fs);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it("rejects same-size input overwritten after descriptor authentication (#8590)", () => { | |
| const root = tempRoot(); | |
| const source = path.join(root, "source.txt"); | |
| fs.writeFileSync(source, "SAFE"); | |
| const fstatSync = fs.fstatSync.bind(fs); | |
| vi.spyOn(fs, "fstatSync") | |
| .mockImplementationOnce((descriptor, options) => { | |
| const stats = fstatSync(descriptor, options as { bigint: true }); | |
| fs.writeFileSync(source, "EVIL"); | |
| return stats; | |
| }) | |
| .mockImplementation((descriptor, options) => | |
| fstatSync(descriptor, options as { bigint: true }), | |
| ); | |
| expect(() => | |
| readBoundedRegularFileBytes(source, "overwritten input", { | |
| maximumBytes: 8, | |
| minimumBytes: 1, | |
| }), | |
| ).toThrow("changed while it was being read"); | |
| }); | |
| it("rejects same-size input overwritten after descriptor authentication (`#8590`)", () => { | |
| const root = tempRoot(); | |
| const source = path.join(root, "source.txt"); | |
| fs.writeFileSync(source, "SAFE"); | |
| const stale = new Date(Date.now() - 60_000); | |
| fs.utimesSync(source, stale, stale); | |
| const fstatSync = fs.fstatSync.bind(fs); | |
| vi.spyOn(fs, "fstatSync") | |
| .mockImplementationOnce((descriptor, options) => { | |
| const stats = fstatSync(descriptor, options as { bigint: true }); | |
| fs.writeFileSync(source, "EVIL"); | |
| return stats; | |
| }) | |
| .mockImplementation((descriptor, options) => | |
| fstatSync(descriptor, options as { bigint: true }), | |
| ); | |
| expect(() => | |
| readBoundedRegularFileBytes(source, "overwritten input", { | |
| maximumBytes: 8, | |
| minimumBytes: 1, | |
| }), | |
| ).toThrow("changed while it was being read"); | |
| }); |
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 114-114: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(source, "SAFE")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename-typescript)
[warning] 119-119: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(source, "EVIL")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename-typescript)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/openshell-qualification-io.test.ts` around lines 112 - 133, Update the
test around readBoundedRegularFileBytes to make the overwrite detectable without
filesystem timestamp granularity: after creating source.txt and before invoking
the read, explicitly set its modification time to an older value using the
existing fs utilities. Preserve the mocked fstatSync sequence and assertion that
the read throws when the file changes.
| it("validates the parsed trust boundary through an executable contract (#8590)", () => { | ||
| const validation = spawnSync( | ||
| process.execPath, | ||
| [ | ||
| "--input-type=module", | ||
| "--eval", | ||
| ` | ||
| import assert from "node:assert/strict"; | ||
| import fs from "node:fs"; | ||
| import YAML from "yaml"; | ||
| const workflow = YAML.parse(fs.readFileSync( | ||
| ".github/workflows/openshell-0.0.101-pr-gate.yaml", | ||
| "utf8", | ||
| )); | ||
| assert.deepEqual(Object.keys(workflow.jobs), [ | ||
| "classify", "verify-draft", "openshell-qualification", | ||
| ]); | ||
| assert.deepEqual(workflow.on.pull_request_target.types, [ | ||
| "opened", "synchronize", "reopened", "edited", | ||
| ]); | ||
| assert.deepEqual(workflow.permissions, { | ||
| contents: "read", "pull-requests": "read", | ||
| }); | ||
| const steps = Object.values(workflow.jobs).flatMap((job) => job.steps ?? []); | ||
| const setups = steps.filter((step) => step.uses?.startsWith("actions/setup-node@")); | ||
| const checkouts = steps.filter((step) => step.uses?.startsWith("actions/checkout@")); | ||
| assert.equal(setups.length, 2); | ||
| assert(setups.every((step) => | ||
| step.uses === "actions/setup-node@820762786026740c76f36085b0efc47a31fe5020" | ||
| )); | ||
| assert.equal(checkouts.length, 3); | ||
| assert(checkouts.every((step) => | ||
| step.uses === "actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1" | ||
| )); | ||
| const byName = (jobName, stepName) => { | ||
| const found = workflow.jobs[jobName].steps.find((step) => step.name === stepName); | ||
| assert(found); | ||
| return found; | ||
| }; | ||
| const paths = (jobName, stepName) => | ||
| byName(jobName, stepName).with["sparse-checkout"].trim().split("\\n"); | ||
| assert.deepEqual(paths("classify", "Checkout base-trusted qualification verifier"), [ | ||
| "scripts/checks/openshell-qualification-bootstrap-contract.mts", | ||
| "scripts/checks/openshell-qualification-io.mts", | ||
| "scripts/checks/openshell-qualification-paths.mts", | ||
| "scripts/checks/verify-openshell-qualification-pr-gate.mts", | ||
| ]); | ||
| assert.deepEqual(paths("verify-draft", "Checkout candidate qualification data"), [ | ||
| "ci/openshell-0.0.101-qualification-v1.json", | ||
| "nemoclaw-blueprint/blueprint.yaml", | ||
| ]); | ||
| const candidate = byName("verify-draft", "Checkout candidate qualification data"); | ||
| assert.equal(candidate.with["persist-credentials"], false); | ||
| assert.equal(candidate.with["allow-unsafe-pr-checkout"], true); | ||
| assert.equal(workflow.jobs["openshell-qualification"].if, "always()"); | ||
| assert(steps.filter((step) => step.run).every((step) => | ||
| !step.run.includes(".candidate-openshell-qualification/scripts/") | ||
| )); | ||
| `, | ||
| ], | ||
| { cwd: process.cwd(), encoding: "utf8" }, | ||
| ); | ||
|
|
||
| expect(validation.status, validation.stderr).toBe(0); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Assert the workflow contract in the test, not in a spawned string program.
The inline --eval program re-reads the YAML and re-implements job, step, and sparsePaths, which the file already defines at Lines 23-39. The string program gets no type checking, no lint coverage, and no formatting, and sparsePaths becomes unused as a result. The assertions are pure structural checks over parsed YAML, so the child process adds no isolation value. Move the assertions into the test body and use the existing helpers.
As per path instructions: "Flag copied production algorithms, broad mocks that bypass the behavior under test, and conditionals that make a test pass without exercising its claim."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/openshell-qualification-pr-gate-workflow.test.ts` around lines 42 - 106,
Move the inline assertions from the spawned --eval program into the test body of
the workflow contract test. Reuse the existing job, step, and sparsePaths
helpers defined earlier in the file, remove the spawnSync child-process setup
and unused-helper condition, and preserve all current structural assertions
against the parsed workflow.
Source: Path instructions
| ])("enforces $disposition behavior (#8590)", ({ env, expectedStatus }) => { | ||
| const report = step(job("openshell-qualification"), "Report qualification decision"); | ||
| assert(report.run); | ||
| const result = spawnSync("bash", ["-c", report.run], { | ||
| encoding: "utf8", | ||
| env: { ...process.env, ...env }, | ||
| }); | ||
|
|
||
| expect(result.status).toBe(expectedStatus); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Bind each disposition to its own failure message.
The test asserts only the exit status. Seven cases expect status 1 and pass through different branches of the report script. A branch that fires for the wrong reason still satisfies the assertion. For example, "malformed applicability output" and "sensitive fork" are indistinguishable today. Assert the emitted ::error:: text for each failing case.
💚 Proposed change
- ])("enforces $disposition behavior (`#8590`)", ({ env, expectedStatus }) => {
+ ])("enforces $disposition behavior (`#8590`)", ({ env, expectedStatus, expectedMessage }) => {
const report = step(job("openshell-qualification"), "Report qualification decision");
assert(report.run);
const result = spawnSync("bash", ["-c", report.run], {
encoding: "utf8",
env: { ...process.env, ...env },
});
expect(result.status).toBe(expectedStatus);
+ expect(result.stdout).toContain(expectedMessage);
});As per path instructions: "Prefer observable outcomes through the public boundary over source-text, private-shape, or mock-call assertions."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/openshell-qualification-pr-gate-workflow.test.ts` around lines 199 -
208, Update the parameterized “enforces $disposition behavior” test around job()
and spawnSync() to assert the emitted ::error:: message as well as
result.status. Add the expected failure message to each disposition case,
including distinct text for branches such as malformed applicability output and
sensitive fork, and verify it from result.stdout or result.stderr according to
where the report script emits errors.
Source: Path instructions
| it("rejects linked and oversized OpenShell version blueprints (#8590)", () => { | ||
| const root = tempRoot(); | ||
| const blueprintDirectory = path.join(root, "nemoclaw-blueprint"); | ||
| const blueprintPath = path.join(blueprintDirectory, "blueprint.yaml"); | ||
| const targetPath = path.join(root, "target.yaml"); | ||
| const source = fs.readFileSync("nemoclaw-blueprint/blueprint.yaml"); | ||
| fs.mkdirSync(blueprintDirectory, { recursive: true }); | ||
| fs.writeFileSync(blueprintPath, source); | ||
| expect(readBlueprintVersion(root)).toBe("0.0.99"); | ||
| fs.renameSync(blueprintPath, targetPath); | ||
| fs.symlinkSync(targetPath, blueprintPath); | ||
| expect(() => readBlueprintVersion(root)).toThrow("regular non-link file"); | ||
| fs.unlinkSync(blueprintPath); | ||
| fs.writeFileSync(blueprintPath, Buffer.alloc(1024 * 1024 + 1)); | ||
| expect(() => readBlueprintVersion(root)).toThrow("bounded regular non-link file"); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert the failure messages from the owning module.
The test asserts "regular non-link file" and "bounded regular non-link file". Those strings come from scripts/checks/openshell-qualification-io.mts, not from the verifier under test. The second string contains the first as a substring, so the first assertion also passes for the oversized case. Assert the full message, or assert through a message constant, so the two cases stay distinguishable.
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 210-210: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(blueprintPath, source)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename-typescript)
[warning] 216-216: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(blueprintPath, Buffer.alloc(1024 * 1024 + 1))
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename-typescript)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/openshell-qualification-pr-gate.test.ts` around lines 204 - 219, Update
the test around readBlueprintVersion to assert the complete, owning-module
failure messages for both the symlink and oversized-file cases. Use the relevant
message constants from openshell-qualification-io.mts if available, ensuring the
assertions remain distinct rather than matching the shared substring.
| expect( | ||
| [ | ||
| ".github/workflows/openshell-0.0.101-qualification.yaml", | ||
| "scripts/checks/verify-openshell-qualification-producer-workflow.mts", | ||
| "scripts/release-cut-tag.sh", | ||
| ].every((relativePath) => !fs.existsSync(path.join(root, relativePath))), | ||
| ).toBe(true); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Remove the tautological exclusion assertion.
constructBundle copies only the paths it receives. It cannot create .github/workflows/openshell-0.0.101-qualification.yaml, scripts/checks/verify-openshell-qualification-producer-workflow.mts, or scripts/release-cut-tag.sh. The assertion therefore passes regardless of the workflow contents. The exact-set assertion at Lines 113-116 already proves the candidate bundle is declarative and receipt-free. Delete the second assertion, or assert against the declared sparse list instead of the constructed directory.
💚 Proposed change
expect(bundleFiles(root)).toEqual([
"ci/openshell-0.0.101-qualification-v1.json",
"nemoclaw-blueprint/blueprint.yaml",
]);
- expect(
- [
- ".github/workflows/openshell-0.0.101-qualification.yaml",
- "scripts/checks/verify-openshell-qualification-producer-workflow.mts",
- "scripts/release-cut-tag.sh",
- ].every((relativePath) => !fs.existsSync(path.join(root, relativePath))),
- ).toBe(true);
+ expect(paths).not.toContain(".github/workflows/openshell-0.0.101-qualification.yaml");
+ expect(paths.some((entry) => entry.startsWith("scripts/"))).toBe(false);As per path instructions: "Flag copied production algorithms, broad mocks that bypass the behavior under test, and conditionals that make a test pass without exercising its claim."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| expect( | |
| [ | |
| ".github/workflows/openshell-0.0.101-qualification.yaml", | |
| "scripts/checks/verify-openshell-qualification-producer-workflow.mts", | |
| "scripts/release-cut-tag.sh", | |
| ].every((relativePath) => !fs.existsSync(path.join(root, relativePath))), | |
| ).toBe(true); | |
| expect(bundleFiles(root)).toEqual([ | |
| "ci/openshell-0.0.101-qualification-v1.json", | |
| "nemoclaw-blueprint/blueprint.yaml", | |
| ]); | |
| expect(paths).not.toContain(".github/workflows/openshell-0.0.101-qualification.yaml"); | |
| expect(paths.some((entry) => entry.startsWith("scripts/"))).toBe(false); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/openshell-qualification-sparse-bootstrap-bundle.test.ts` around lines
117 - 123, Remove the tautological fs.existsSync exclusion assertion following
the exact-set assertion in the sparse bootstrap bundle test. Keep the existing
exact-set assertion as the verification that the constructed bundle contains
only the declared sparse paths and no receipt files; do not add unrelated checks
against paths constructBundle cannot copy.
Source: Path instructions
3 participants
Summary
Bootstrap a base-trusted, fail-closed pull-request gate and an inert draft qualification inventory for the OpenShell v0.0.101 program. This prerequisite records no compatibility result and does not activate the organization ruleset; every qualification mapping remains pending until separately accepted evidence and governance are available.
Related Issue
Related #8590
Changes
pull_request_targetworkflow that classifies qualification-sensitive paths using executable code checked out from the exact base SHA.requiredWorkflowGate, retirement evidence, and artifacts unset and all selector/final mappings pending.Type of Change
Quality Gates
4d5104c5127346bd08d9fa64c491786e66d1bd3dverified read-only workflow permissions, immutable action pins, base-trusted executable code, declarative-only candidate checkout, disabled persisted credentials, bounded file/path/JSON parsing, and inert draft/null state. The reviewed bootstrap was rebound across a path-disjoint base move; 76 focused tests, the 10-test affected workflow suite after removing an unused helper, typecheck, repository gates, and gitleaks pass.Documentation Writer Review
no-docs-neededDGX Station Hardware Evidence
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run validate:prpassed after refreshingorigin/mainwhen hooks were skipped or unavailablenpx vitest run --project integrationover the six OpenShell bootstrap/gate suites: 76/76 pass;npm run typecheck:cli, source-shape, test-size, title, and project-membership gates pass.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result:npm run docsbuilds without warnings (doc changes only)Signed-off-by: Apurv Kumaria akumaria@nvidia.com
Summary by CodeRabbit
New Features
Tests