fix(e2e): reconcile uncertain workflow dispatches#7594
Merged
Conversation
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Contributor
📝 WalkthroughWalkthroughThe PR adds structured GitHub API errors, bounded workflow-dispatch reconciliation, validated ChangesDispatch reconciliation and retry hardening
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant PRGate
participant GitHubAPI
participant WorkflowInventory
participant CoordinationCheck
PRGate->>GitHubAPI: dispatch workflow with correlation
GitHubAPI-->>PRGate: response metadata or uncertain result
PRGate->>WorkflowInventory: poll bounded correlated-run inventory
WorkflowInventory-->>PRGate: exact child, zero matches, or contradiction
PRGate->>CoordinationCheck: publish child binding or receipt-marked failure
CoordinationCheck-->>PRGate: persisted check state for retry or authorization
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 4b1a142 in the TypeScript / code-coverage/cliThe overall coverage in commit 4b1a142 in the Show a code coverage summary of the most impacted files.
Updated |
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
tools/e2e/pr-e2e-dispatch-reconciliation.mts (1)
696-724: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winUnbounded recheck window can make old receipts permanently non-retryable.
recheckUpperBoundMsgrows with wall-clock time, so thecreatedrange for the recheck spans from the originalsentAtMsto now. The filter (branch=main,event=workflow_dispatch,head_sha=workflowSha) can accumulate more thanMAX_WORKFLOW_RUNS(100) entries over a long gap, andvalidateWorkflowRunInventorythen throws "invalid or incomplete", turning every subsequent retry into a terminal reconciliation error rather than a clean recheck.Consider capping the recheck upper bound (for example
deadlineAtMs + clockSkewMs + a bounded grace) or paginating/narrowing the query so a stale receipt cannot exhaust the page cap.🤖 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 `@tools/e2e/pr-e2e-dispatch-reconciliation.mts` around lines 696 - 724, The recheck window built in the reconciliation flow around recheckUpperBoundMs is unbounded and can exceed the workflow inventory page cap for stale receipts. Bound the upper time range with a finite grace period after options.receipt.deadlineAtMs (or otherwise narrow/paginate the readInventory query) so validateWorkflowRunInventory can process stale receipts without treating accumulated historical runs as an invalid or incomplete inventory.tools/e2e/pr-e2e-retry-receipt.mts (1)
9-13: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated validation constants risk silent drift.
GITHUB_REQUEST_ID_PATTERNis redefined here and intools/advisors/github.mts(Line 26), andMAX_DISPATCH_RECONCILIATION_WINDOW_MSmirrorsMAX_RECONCILIATION_WINDOW_MSintools/e2e/pr-e2e-dispatch-reconciliation.mts(Line 24). If either side changes, previously valid receipts start failing validation. Consider exporting one canonical definition and importing it.🤖 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 `@tools/e2e/pr-e2e-retry-receipt.mts` around lines 9 - 13, Consolidate the duplicated validation constants by exporting canonical definitions from their existing owning modules and importing them here. Update GITHUB_REQUEST_ID_PATTERN to reuse the definition from github.mts and MAX_DISPATCH_RECONCILIATION_WINDOW_MS to reuse MAX_RECONCILIATION_WINDOW_MS from pr-e2e-dispatch-reconciliation.mts, removing the local duplicates while preserving current validation behavior.test/pr-e2e-gate.test.ts (1)
401-416: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the repeated evaluating-check output literal.
The
{ title: "Evaluating PR commit", summary: "Validating the PR SHA and selecting..." }pair is repeated four times here (and in sibling gate tests). A smallevaluatingOutput()fixture keeps the pre-dispatch title contract in one place.Also applies to: 504-519, 1254-1265, 1308-1314
🤖 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/pr-e2e-gate.test.ts` around lines 401 - 416, Extract the repeated evaluating-check output object into a shared evaluatingOutput() fixture in the test module, then replace the duplicated literals in existingPrGateCheckRunsRoute, directPrGateCheckRoute, and the sibling gate test cases at the other referenced locations. Preserve the exact title and summary values and use the fixture for every pre-dispatch evaluating output.tools/e2e/pr-e2e-gate.mts (1)
2190-2198: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCentralize the pre-dispatch title constants.
The three accepted titles are duplicated as literals at the call sites (Line 2749, Line 3290, Line 3425) and in
markCheckInProgresscalls. A shared constant/helper (e.g.authorizedCheckTitle(maintainer)) keeps the validator and producers from drifting silently.🤖 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 `@tools/e2e/pr-e2e-gate.mts` around lines 2190 - 2198, Centralize the accepted pre-dispatch titles used by isValidExpectedPreDispatchTitle and the markCheckInProgress call sites. Define shared constants or an authorizedCheckTitle(maintainer) helper for the three title forms, then update the validator and producers at the referenced call sites to reuse them instead of literal strings, preserving the existing validation behavior.
🤖 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 `@test/pr-e2e-gate-dispatch-recovery.test.ts`:
- Around line 1079-1083: Update urlRunId to remove its explicit if guard and use
the router’s matching-URL invariant by asserting the regex match before
accessing capture group 1; preserve the existing invalid-URL failure behavior
through the assertion.
In `@test/pr-e2e-gate-runner-loss-dispatch-recovery.test.ts`:
- Around line 356-384: Move the per-case error regex and marker expectations out
of the test body into the it.each case objects, adding fields for the expected
rejection pattern and marker behavior. Replace the candidateIds.length === 0
ternary and if branch with assertions driven by those case fields, preserving
the existing request and retryCheck assertions.
---
Nitpick comments:
In `@test/pr-e2e-gate.test.ts`:
- Around line 401-416: Extract the repeated evaluating-check output object into
a shared evaluatingOutput() fixture in the test module, then replace the
duplicated literals in existingPrGateCheckRunsRoute, directPrGateCheckRoute, and
the sibling gate test cases at the other referenced locations. Preserve the
exact title and summary values and use the fixture for every pre-dispatch
evaluating output.
In `@tools/e2e/pr-e2e-dispatch-reconciliation.mts`:
- Around line 696-724: The recheck window built in the reconciliation flow
around recheckUpperBoundMs is unbounded and can exceed the workflow inventory
page cap for stale receipts. Bound the upper time range with a finite grace
period after options.receipt.deadlineAtMs (or otherwise narrow/paginate the
readInventory query) so validateWorkflowRunInventory can process stale receipts
without treating accumulated historical runs as an invalid or incomplete
inventory.
In `@tools/e2e/pr-e2e-gate.mts`:
- Around line 2190-2198: Centralize the accepted pre-dispatch titles used by
isValidExpectedPreDispatchTitle and the markCheckInProgress call sites. Define
shared constants or an authorizedCheckTitle(maintainer) helper for the three
title forms, then update the validator and producers at the referenced call
sites to reuse them instead of literal strings, preserving the existing
validation behavior.
In `@tools/e2e/pr-e2e-retry-receipt.mts`:
- Around line 9-13: Consolidate the duplicated validation constants by exporting
canonical definitions from their existing owning modules and importing them
here. Update GITHUB_REQUEST_ID_PATTERN to reuse the definition from github.mts
and MAX_DISPATCH_RECONCILIATION_WINDOW_MS to reuse MAX_RECONCILIATION_WINDOW_MS
from pr-e2e-dispatch-reconciliation.mts, removing the local duplicates while
preserving current validation behavior.
🪄 Autofix (Beta)
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: df3dab9b-f404-49fc-8376-640f4f52e4d7
📒 Files selected for processing (19)
.agents/skills/nemoclaw-maintainer-day/MERGE-GATE.mdtest/e2e/README.mdtest/github-api.test.tstest/pr-e2e-dispatch-reconciliation.test.tstest/pr-e2e-gate-dispatch-recovery.test.tstest/pr-e2e-gate-fork-approval.test.tstest/pr-e2e-gate-internal-approval.test.tstest/pr-e2e-gate-lifecycle.test.tstest/pr-e2e-gate-runner-loss-dispatch-recovery.test.tstest/pr-e2e-gate-runner-loss-retry.test.tstest/pr-e2e-gate-typed-target.test.tstest/pr-e2e-gate.test.tstest/pr-e2e-required.test.tstest/pr-e2e-retry-receipt.test.tstools/advisors/github.mtstools/e2e/pr-e2e-dispatch-reconciliation.mtstools/e2e/pr-e2e-gate.mtstools/e2e/pr-e2e-required.mtstools/e2e/pr-e2e-retry-receipt.mts
Comment on lines
+356
to
+384
| const attempt = retryRunnerLossPrGate(context.command); | ||
| const result = expect(attempt).rejects.toThrow( | ||
| candidateIds.length === 0 | ||
| ? /not observed after bounded reconciliation/u | ||
| : /multiple correlated runs/u, | ||
| ); | ||
| await vi.runAllTimersAsync(); | ||
| await result; | ||
|
|
||
| expect(retryCheck).toMatchObject({ | ||
| id: 18, | ||
| status: "completed", | ||
| conclusion: "failure", | ||
| output: { title }, | ||
| }); | ||
| expect(requests.filter((request) => request.url.endsWith("/dispatches"))).toHaveLength(1); | ||
| expect( | ||
| requests.filter((request) => /\/actions\/runs\/(?:24|25)\/cancel$/u.test(request.url)), | ||
| ).toHaveLength(candidateIds.length); | ||
| expect( | ||
| requests.filter( | ||
| (request) => request.url.endsWith("/check-runs/17") && request.method === "PATCH", | ||
| ), | ||
| ).toHaveLength(0); | ||
| if (candidateIds.length === 0) { | ||
| expect(JSON.stringify(retryCheck)).toContain("nemoclaw-pr-e2e-dispatch:v1:"); | ||
| } else { | ||
| expect(JSON.stringify(retryCheck)).not.toContain("dispatch-not-observed"); | ||
| } |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Guardrail failure: move the per-case branch into the it.each table.
CI reports the test-conditionals scan for this file (1 if statement(s), up from 0). The if (candidateIds.length === 0) branch — and the error-regex ternary above it — are per-case data, so encode them in the case objects instead of branching inside the body.
🛠️ Table-driven cases
it.each([
{
label: "zero candidates",
candidateIds: [] as number[],
title: "Workflow dispatch was not observed",
+ error: /not observed after bounded reconciliation/u,
+ expectedMarker: "nemoclaw-pr-e2e-dispatch:v1:",
+ forbiddenMarker: undefined as string | undefined,
},
{
label: "multiple candidates",
candidateIds: [24, 25],
title: "Runner-loss retry could not start",
+ error: /multiple correlated runs/u,
+ expectedMarker: undefined as string | undefined,
+ forbiddenMarker: "dispatch-not-observed",
},
- ])("fails closed after $label without a second dispatch", async ({ candidateIds, title }) => {
+ ])(
+ "fails closed after $label without a second dispatch",
+ async ({ candidateIds, title, error, expectedMarker, forbiddenMarker }) => {Then assert with expect(JSON.stringify(retryCheck)).toEqual(expectedMarker ? expect.stringContaining(expectedMarker) : expect.not.stringContaining(forbiddenMarker!)) — or simply two expect calls driven by the optional fields.
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 384-384: 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.readFileSync(context.outputPath, "utf8")
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/pr-e2e-gate-runner-loss-dispatch-recovery.test.ts` around lines 356 -
384, Move the per-case error regex and marker expectations out of the test body
into the it.each case objects, adding fields for the expected rejection pattern
and marker behavior. Replace the candidateIds.length === 0 ternary and if branch
with assertions driven by those case fields, preserving the existing request and
retryCheck assertions.
Source: Pipeline failures
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Contributor
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
Nemotron output stays in workflow artifacts and does not change the assessment above. E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
cv
approved these changes
Jul 26, 2026
cv
enabled auto-merge (squash)
July 26, 2026 20:37
cv
disabled auto-merge
July 26, 2026 20:37
23 tasks
cv
pushed a commit
that referenced
this pull request
Jul 26, 2026
<!-- markdownlint-disable MD041 --> ## Summary Follow up on #7594 so an old `dispatch-not-observed` receipt can be rechecked after more than one page of matching workflow runs. The recheck now reads a complete, bounded inventory without hiding late children or permitting a duplicate dispatch. ## Related Issue Follow-up to #7594 for #7593. Part of #7140. ## Changes - Paginate stale-receipt workflow inventories through GitHub's documented 1,000-result filtered-search limit under one shared timeout. - Require a stable total count, complete intermediate pages, exact final count, and globally unique run IDs before replacement can proceed. - Preserve fail-closed behavior for changing, duplicate, short, oversized, timed-out, or otherwise incomplete inventories. - Share the request-ID validator and reconciliation-window limit between receipt production and validation. - Centralize the exact pre-dispatch titles consumed by the producer and fail-closed validator. The direct time-window cap suggested during review was not used because it could hide a genuinely late child. Tests cover a clean 101-run inventory, a correlated child appearing only on page two, and every incomplete-inventory rejection boundary. ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [ ] Docs updated for user-facing behavior changes - [x] Docs not applicable — justification: this is an internal E2E control-plane reliability follow-up; the operational guidance added by #7594 remains accurate. - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: independent Codex security and code reviews passed at `0693c168d`; pagination remains bounded, complete, and fail-closed before any replacement dispatch. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `no-docs-needed` - Evidence: #7594's `test/e2e/README.md` and `.agents/skills/nemoclaw-maintainer-day/MERGE-GATE.md` guidance remains accurate; no user-facing `docs/` behavior changes. - Agent: Codex Desktop <!-- docs-review-head-sha: 0693c16 --> <!-- docs-review-agents-blob-sha: be20a09 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: - Station profile/scenario: - Result: - Supporting evidence: ## Verification - [x] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — `npx vitest run --project integration test/github-api.test.ts test/pr-e2e*.test.ts`: 19 files, 311 tests passed - [x] Applicable broad gate passed — CLI type-check and `npx prek run --from-ref origin/main --to-ref HEAD` passed - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- Signed-off-by: Apurv Kumaria <akumaria@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved dispatch reconciliation when workflow runs appear late or across multiple inventory pages. * Added safeguards for incomplete, duplicated, inconsistent, or oversized workflow-run listings. * Improved handling of stale and zero-match dispatch receipts, including replacement checks. * Standardized validation of GitHub request identifiers. * Improved consistency of PR gate status titles during evaluation, authorization, and retry flows. * **Tests** * Expanded end-to-end coverage for paginated inventory and delayed workflow-run scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
23 tasks
cv
pushed a commit
that referenced
this pull request
Jul 26, 2026
<!-- markdownlint-disable MD041 --> ## Summary Add the exact upper-bound regression requested during review of #7595. A complete stale-receipt inventory containing 1,000 workflow runs across all 10 allowed pages must remain retryable rather than failing from an off-by-one pagination error. ## Related Issue Follow-up to #7595 and #7594 for #7593. Part of #7140. ## Changes - Exercise the inclusive 1,000-run filtered-search cap. - Require the receipt recheck to consume exactly 10 complete pages. - Preserve the existing zero-correlation success contract at the boundary. ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [ ] Docs updated for user-facing behavior changes - [x] Docs not applicable — justification: this is a test-only boundary follow-up with no runtime or user-facing behavior change. - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: Exact product-scope and nine-category security review PASS at `871b933840300068aef3f462a8ecf1d0873209dc` (tree `4e2d18d2833a353776d6936fd22d28f3c95ad2d9`, base `17a8ad1a6979de0b92b25eb93453dbbdf03cf61a`). The diff is test-only; it exercises the exact fail-closed pagination boundary and changes no production code, dependency, permission, credential path, or supported product surface. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `no-docs-needed` - Evidence: Test-only regression coverage for the existing GitHub filtered-result pagination cap; no user-visible behavior changed and #7594's operational guidance remains accurate. - Agent: Codex Desktop <!-- docs-review-head-sha: 871b933 --> <!-- docs-review-agents-blob-sha: be20a09 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: - Station profile/scenario: - Result: - Supporting evidence: ## Verification - [x] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — exact head `871b933840300068aef3f462a8ecf1d0873209dc`: `npx vitest run --project integration test/pr-e2e-dispatch-reconciliation.test.ts` passed 35/35. - [x] Applicable broad gate passed — CLI build and exact-head `npm run check:diff` passed after generating the required `dist/` artifacts. - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- Signed-off-by: Apurv Kumaria <akumaria@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Added end-to-end coverage for reconciling complete stale receipt inventories at GitHub’s filtered-result limit. * Verified reconciliation succeeds with clock skew and across paginated inventory reads, including the page-cap boundary (ensuring calls span the expected pages). <!-- end of auto-generated comment: release notes by coderabbit.ai -->
23 tasks
ericksoa
pushed a commit
that referenced
this pull request
Jul 27, 2026
## Summary GitHub may canonicalize a PR gate check details URL to the check own /runs/<check-id> URL even when the exact child-bound summary is persisted. The controller introduced by #7594 rejected that valid response, revoked authorization, and prevented the selected #7590 Hermes lanes from starting. This change accepts only the exact child URL or the exact canonical check URL while preserving every authorization identity check. ## Related Issue Part of #7140. Follow-up to #7593 and #7594; aligns the parent controller with the canonicalization behavior already protected by #7515. ## Changes - Accept the selected child Actions URL or the exact canonical /runs/<check-id> URL after the check ID, GitHub Actions app, PR head/base identity, status, title, and child-bound summary all validate. - Continue rejecting foreign repositories, wrong check IDs, alternate schemes, query or fragment variants, and unrelated URLs. - Cover both allowed URL forms after a lost PATCH response and prove a foreign canonical URL still revokes authorization before child cancellation. ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [ ] Docs updated for user-facing behavior changes - [x] Docs not applicable — justification: existing E2E operational documentation already states that GitHub may canonicalize details_url to /runs/<check-id>; no user-facing command or workflow changes. - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: independent Codex Desktop code and security reviews verified the exact URL allowlist and the retained check ID, app, PR/head/base, status, plan, jobs, targets, and child-run summary binding; no findings remained. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes. - Result: `no-docs-needed` - Evidence: Exact head `d4f53a19618b4d810494e2094c6aff6b9f6602cc` changes only the E2E controller’s canonical check-URL validation and focused tests. `test/e2e/README.md` already documents direct-read recovery, exact persisted child binding, and GitHub canonicalization to `/runs/<check-id>`. Changed test text is behavior-oriented and introduces no issue references. No docs build was needed because no documentation source changed. - Agent: Codex Desktop <!-- docs-review-head-sha: d4f53a1 --> <!-- docs-review-agents-blob-sha: be20a09 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: not applicable - Station profile/scenario: not applicable - Result: not applicable; scripts/prepare-dgx-station-host.sh is unchanged. - Supporting evidence: ## Verification - [x] PR description includes a Signed-off-by line and every commit appears as Verified in GitHub - [x] Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run check:diff passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — 19 PR-E2E files, 313 tests passed; the E2E workflow boundary added 39 passing tests. - [x] Applicable broad gate passed — CLI typecheck, source-shape, test-size, title-style, project isolation, Biome, and the complete changed-file hook set passed. Post-rebase focused validation passed 41 tests. - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] npm run docs builds without warnings (doc changes only) - [ ] Doc pages follow the style guide (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new doc pages only) Failure evidence: controller run 30221481816 and child run 30221620499. The parent published the exact child-bound summary but GitHub returned the canonical check URL, so the old strict URL comparison revoked the otherwise valid authorization. Self-hosting evidence: #7599 controller run 30224207359 checked out trusted base `2f66902b`, rejected the canonical URL for check 89851619986, and could not cancel child run 30224558462 after GitHub returned HTTP 500. The orphan child then failed closed because trusted controller authorization had not been published. A same-revision retry is not supported after dispatch; the exact fix requires the repository's approved controller-bootstrap governance path. --- Signed-off-by: Apurv Kumaria <akumaria@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved pull request gate recovery when an authorization update response is lost, ensuring check details URLs are set correctly after retry behavior. * Updated gate check validation to accept both dispatched child run URLs and canonical controller check URLs. * **Tests** * Expanded end-to-end coverage with parameterized published-details URL variants and scenarios where lost updates result in a foreign check URL. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
apurvvkumaria
pushed a commit
that referenced
this pull request
Jul 27, 2026
<!-- markdownlint-disable MD041 --> ## Summary Add the canonical `docs/changelog/2026-07-25.mdx` release entry with the exact `## v0.0.96` heading. The entry reconciles all 90 first-parent commits since v0.0.95 with all 92 merged PRs in the live `v0.0.96` label ledger and groups the user-visible changes by operator journey. ## Changes - Add the parser-safe dated MDX changelog entry for v0.0.96 with root-absolute links to the focused user guides. - Source summary: - [#7194](#7194) -> `docs/changelog/2026-07-25.mdx`: Document persistent baseline network policy exclusions and their inspection, rebuild, and snapshot behavior. - [#7188](#7188), [#7427](#7427), and [#7546](#7546) -> `docs/changelog/2026-07-25.mdx`: Document DNS-backed HTTPS inference routing, keyless loopback endpoints, and provider-marker isolation. - [#7238](#7238) -> `docs/changelog/2026-07-25.mdx`: Document blueprint sandbox and provider identifier validation before state writes or OpenShell calls, with bounded terminal-safe rejection previews. - [#7319](#7319), [#7274](#7274), [#7528](#7528), [#7353](#7353), and [#7560](#7560) -> `docs/changelog/2026-07-25.mdx`: Document the managed default gateway service, onboarding readiness, and container-runtime identity safeguards. - [#7349](#7349), [#7498](#7498), [#7406](#7406), [#7196](#7196), [#7559](#7559), [#7421](#7421), [#7510](#7510), [#7295](#7295), and [#7565](#7565) -> `docs/changelog/2026-07-25.mdx`: Document gateway-scoped status, lifecycle diagnostics, managed MCP recovery, delete-edge safeguards, and fail-closed CLI prompt and command output. - [#7591](#7591) -> `docs/changelog/2026-07-25.mdx`: Document opt-in authenticated MCP tool-name discovery, its bounded and names-only contract, probe interaction, and rebuild requirement. - [#7305](#7305), [#7480](#7480), [#7471](#7471), [#7365](#7365), and [#7541](#7541) -> `docs/changelog/2026-07-25.mdx`: Document installer version checks, version-tag reporting, license guidance, WSL Ollama selection, and DGX Station vLLM detection. - [#7482](#7482), [#7466](#7466), [#7208](#7208), [#7434](#7434), and [#7586](#7586) -> `docs/changelog/2026-07-25.mdx`: Document Ollama resource details, reasoning precedence, Hermes onboarding behavior, and preserved managed Hermes BuildKit failures. - [#6830](#6830), [#7492](#7492), [#7563](#7563), and [#7582](#7582) -> `docs/changelog/2026-07-25.mdx`: Document the authoritative OpenClaw production lock, fixed managed-image dependencies, immutable Hermes base adoption, and Hermes image-size reduction. - [#7505](#7505), [#7530](#7530), [#7547](#7547), [#7508](#7508), [#7548](#7548), [#7549](#7549), [#7537](#7537), [#7534](#7534), [#7515](#7515), [#7511](#7511), [#7551](#7551), [#7562](#7562), [#7575](#7575), [#7496](#7496), [#7594](#7594), [#7595](#7595), and [#7599](#7599) -> `docs/changelog/2026-07-25.mdx`: Summarize release validation, transient and bounded dispatch reconciliation, exact pre-tag qualification, identity revalidation, npm-audit retry, sharding, image reuse, timeout, telemetry, and workflow-hardening changes. - Reconciled without separate changelog prose: - [#7539](#7539), [#7526](#7526), [#7507](#7507), [#7506](#7506), [#7519](#7519), [#7516](#7516), [#7396](#7396), [#7254](#7254), [#7583](#7583), [#7596](#7596), and [#7598](#7598): Test-harness or fixture-only changes. - [#7403](#7403), [#7161](#7161), [#6877](#6877), [#7531](#7531), [#7525](#7525), [#7522](#7522), [#7536](#7536), [#7552](#7552), [#7566](#7566), [#7553](#7553), [#7561](#7561), [#7577](#7577), [#7569](#7569), [#7585](#7585), [#7584](#7584), [#7592](#7592), [#7580](#7580), [#7571](#7571), [#7517](#7517), [#7589](#7589), [#7402](#7402), [#7558](#7558), [#7544](#7544), and [#7601](#7601): Dependency, internal recovery, validation, contributor-workflow, E2E optimization, telemetry, or CI trust changes with no separate user-facing release claim. - [#7556](#7556), [#7573](#7573), [#7576](#7576), and [#7578](#7578): Experimental repository-maintainer conflict automation with no canonical user documentation surface. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [x] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [ ] Tests added or updated for changed behavior - [x] Existing tests cover changed behavior — justification: `test/changelog-docs.test.ts` validates dated changelog structure, version headings, and published links. - [ ] Tests not applicable — justification: - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [ ] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `docs-updated` - Evidence: Reviewed `docs/changelog/2026-07-25.mdx` at exact head `0f5dedb47` against 90 first-parent release commits and 92 merged PRs labeled `v0.0.96`. Verified parser-safe MDX SPDX, the exact version heading, literal CLI names, writing style, skip terms, all 20 root-absolute published links, and the accepted #7591 opt-in authenticated discovery bounds. #7544, #7599, and #7601 remain internal or CI-only release-ledger entries. Changelog tests passed 6/6, the docs build passed with 0 errors and two pre-existing Fern warnings, and `npm run check:diff` plus the final diff check passed. - Agent: Codex Desktop documentation-writer subagent <!-- docs-review-head-sha: 0f5dedb --> <!-- docs-review-agents-blob-sha: be20a09 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: - Station profile/scenario: - Result: - Supporting evidence: ## Verification - [x] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — `npx vitest run test/changelog-docs.test.ts`: 6/6 passed. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: Not applicable to this prose-only changelog entry. - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) — the build passed with 0 errors and 2 existing Fern warnings; the published-route check passed. - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) — native changelog files use the required parser-safe MDX SPDX comment and no frontmatter. --- Signed-off-by: Carlos Villela <cvillela@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Persistent network policy exclusions with consistent restore/exclusion reporting across rebuilds/snapshots. * Opt-in MCP tool discovery via `mcp status --tools` with bounded, redacted authenticated traffic. * Improved HTTPS inference switching for custom endpoints and refreshed onboarding/model menu details. * Refined OpenShell gateway defaults for port `8080`, including more reliable readiness checks. * **Bug Fixes** * Prevent incorrect provider/model restoration after compatible-provider update failures. * Preserve managed MCP state after exec loss and tighten gateway/doctor status scoping. * **Tests** * Stronger, fail-closed release validation with hardened evidence/artifact handoff and bounded timeouts/retries. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Co-authored-by: Prekshi Vyas <prekshiv@nvidia.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
An uncertain GitHub workflow-dispatch response can mean either “no child started” or “the child started but the response was lost.” This change sends exactly one dispatch request, reconciles the result through bounded read-only inventory checks, and prevents a retry from starting duplicate credential-bearing E2E work.
Related Issue
Fixes #7593.
Part of #7140.
Changes
The shared reconciliation path is required because retrying the dispatch POST directly cannot distinguish a rejected request from a successfully accepted request whose response was lost. The PR-E2E reconciliation, gate recovery, protected approval, and runner-loss suites cover this boundary.
Type of Change
Quality Gates
docs/is unchanged.1876c7683, including duplicate-dispatch prevention, exact authorization binding, bounded reads, cancellation collection, and failure ordering;bc1ba2071and4b1a1427care reviewed guardrail and redundant-import cleanups with no behavior change.Documentation Writer Review
docs-updatedtest/e2e/README.md;.agents/skills/nemoclaw-maintainer-day/MERGE-GATE.md. No user-facingdocs/update is needed because this is internal E2E control-plane behavior.DGX Station Hardware Evidence
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailablenpx vitest run --project integration test/github-api.test.ts test/pr-e2e*.test.ts: 19 files, 306 tests passednpx prek run --from-ref origin/main --to-ref HEADpassednpm run docsbuilds without warnings (doc changes only) — passed with 0 errors and 2 existing Fern warningsSigned-off-by: Apurv Kumaria akumaria@nvidia.com
Summary by CodeRabbit
Improvements
Documentation
Tests