Skip to content

Commit c166756

Browse files
authored
ci(openshell): accept optional SDK package decision (#10568)
<!-- markdownlint-disable MD041 --> ## Outcome Pull request CI now treats a valid base-controlled `required: false` SDK package decision as a successful no-package result. Invalid inspector output still stops the locator before it writes workflow output. Unblocks PR #10562. ## Reason `jq -e` treated Boolean `false` as a command failure. This blocked PR #10562 before its dependency jobs could run. The locator must preserve `false` as valid data while keeping the trusted decision boundary fail closed. ## Changes - Require exactly one trusted inspector JSON object with a Boolean `required` field, then convert that Boolean to a string before `jq -e` evaluates it. - Add workflow-shell regression tests for valid `false`, non-Boolean, empty, and multiple-document inspector output. ## Verification - `/Users/rsliter/Projects/NemoClaw/node_modules/.bin/vitest run --project integration test/automation/pull-requests/pr-workflow-contract.test.ts`: 1 file passed, 39 tests passed. - Normal `pre-commit` and `commit-msg` hooks passed, including YAML validation, repository checks, source-shape budgets, growth guardrails, secret scanning, formatting, linting, and commitlint. - `git diff --check origin/main...HEAD`: passed. - The diff contains no secrets, API keys, or credentials. ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed implementation - Result: `no-docs-needed` - Evidence: The change is limited to internal pull request CI parsing and workflow-shell regression tests. It does not change user commands, configuration, supported product behavior, or documentation routes. - Agent: Codex Desktop <!-- docs-review-head-sha: f1f683d --> <!-- docs-review-agents-blob-sha: dd3528f --> ## Review notes Independent security review passed all nine categories. The base-controlled checkout, same-repository gate, artifact identity checks, job permissions, and credential isolation remain unchanged. Negative tests confirm that invalid inspector output does not write workflow state. --- Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Improved SDK package decision handling by requiring a single, valid JSON decision with a boolean requirement. - Invalid, empty, or ambiguous decisions now fail clearly instead of being misinterpreted. - Artifact lookup is skipped when no SDK package is required. - **Tests** - Added coverage for valid custom inspection results, non-boolean requirements, missing decisions, and multiple decision outputs. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com> Co-authored-by: Rebecca Sliter <571084+rsliter@users.noreply.github.com>
1 parent 6c255a0 commit c166756

2 files changed

Lines changed: 63 additions & 2 deletions

File tree

.github/workflows/pr.yaml

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,15 @@ jobs:
214214
NEMOCLAW_CI_TARGET_ROOT="$GITHUB_WORKSPACE" \
215215
node --experimental-strip-types "$trusted_inspector"
216216
)"
217-
required="$(jq -er '.required | select(type == "boolean")' <<<"$decision")"
217+
required="$(
218+
jq -ser '
219+
if length == 1 and (.[0] | type) == "object" and (.[0].required | type) == "boolean" then
220+
.[0].required | tostring
221+
else
222+
error("decision must contain one object with a boolean required field")
223+
end
224+
' <<<"$decision"
225+
)"
218226
if [ "$required" != "true" ]; then
219227
echo "required=false" >> "$GITHUB_OUTPUT"
220228
exit 0

test/automation/pull-requests/pr-workflow-contract.test.ts

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,8 @@ function runWorkflowShellStep(
160160
type SdkPackageLocatorFixture = Readonly<{
161161
artifactFailureRunId?: number;
162162
artifactsByRunId?: Readonly<Record<string, unknown>>;
163+
inspectorOutput?: string;
164+
inspectorRequired?: unknown;
163165
runs: readonly unknown[];
164166
step: WorkflowStep;
165167
workflowRunFailure?: boolean;
@@ -178,9 +180,15 @@ function runSdkPackageLocator(fixture: SdkPackageLocatorFixture): Readonly<{
178180
mkdirSync(inspectorDirectory, { recursive: true });
179181
mkdirSync(workflowDirectory, { recursive: true });
180182
mkdirSync(fakeBin);
183+
const inspectorDecision =
184+
fixture.inspectorOutput ??
185+
JSON.stringify({
186+
artifactName: "reviewed-sdk.tgz",
187+
required: fixture.inspectorRequired ?? true,
188+
});
181189
writeFileSync(
182190
join(inspectorDirectory, "prepare-ci-npm-install.mts"),
183-
'process.stdout.write(JSON.stringify({ required: true, artifactName: "reviewed-sdk.tgz" }));\n',
191+
`process.stdout.write(${JSON.stringify(inspectorDecision)});\n`,
184192
);
185193
writeFileSync(join(workflowDirectory, "openshell-sdk-package-pr.yaml"), "name: test\n");
186194
writeFileSync(join(fakeBin, "seq"), "#!/bin/sh\nprintf '1\\n'\n", { mode: 0o755 });
@@ -674,6 +682,51 @@ describe("pull request and main workflow contracts", () => {
674682
}
675683
});
676684

685+
it("skips SDK artifact lookup when the trusted inspector does not require a package", () => {
686+
const { githubOutput, result } = runSdkPackageLocator({
687+
inspectorRequired: false,
688+
runs: [],
689+
step: requiredWorkflowStep(
690+
prWorkflow.jobs["openshell-sdk-package"],
691+
"Locate exact base-controlled SDK package run",
692+
),
693+
});
694+
695+
expect(result).toMatchObject({ status: 0, stderr: "" });
696+
expect(githubOutput).toBe("required=false\n");
697+
});
698+
699+
it("rejects a non-boolean package requirement from the trusted inspector", () => {
700+
const { githubOutput, result } = runSdkPackageLocator({
701+
inspectorRequired: "false",
702+
runs: [],
703+
step: requiredWorkflowStep(
704+
prWorkflow.jobs["openshell-sdk-package"],
705+
"Locate exact base-controlled SDK package run",
706+
),
707+
});
708+
709+
expect(result.status).not.toBe(0);
710+
expect(githubOutput).toBe("");
711+
});
712+
713+
it.each([
714+
["empty output", ""],
715+
["multiple JSON documents", '{"required":false}\n{"required":true}'],
716+
])("rejects %s from the trusted inspector", (_description, inspectorOutput) => {
717+
const { githubOutput, result } = runSdkPackageLocator({
718+
inspectorOutput,
719+
runs: [],
720+
step: requiredWorkflowStep(
721+
prWorkflow.jobs["openshell-sdk-package"],
722+
"Locate exact base-controlled SDK package run",
723+
),
724+
});
725+
726+
expect(result.status).not.toBe(0);
727+
expect(githubOutput).toBe("");
728+
});
729+
677730
it("explains how to recover when the exact SDK package artifact expired", () => {
678731
const { githubOutput, result } = runSdkPackageLocator({
679732
artifactsByRunId: {

0 commit comments

Comments
 (0)