Skip to content

fix(ci): verify packaged fdfind identity - #6785

Merged
cv merged 2 commits into
mainfrom
codex/fix-advisor-fdfind-version
Jul 13, 2026
Merged

fix(ci): verify packaged fdfind identity#6785
cv merged 2 commits into
mainfrom
codex/fix-advisor-fdfind-version

Conversation

@prekshivyas

@prekshivyas prekshivyas commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • validate the pinned Ubuntu fd-find binary using its actual fdfind <version> identity
  • keep the trusted workflow-boundary validator aligned with the executable check
  • make the workflow test fixture reproduce the package output seen on hosted runners

Why

PR review advisors currently fail during SDK bootstrap before analysis. The pinned Ubuntu package reports fdfind 9.0.0, while the workflow expects fd 9.0.0. This was reproduced on both advisor lanes for #6711.

Validation

  • npx vitest run --project integration test/pr-review-advisor-workflow-boundary.test.ts (19 passed)
  • npm run checks
  • pre-commit and pre-push hooks, including CLI typecheck

Summary by CodeRabbit

  • Bug Fixes
    • Corrected verification of the fdfind search tool version during Pi SDK installation.
    • Improved workflow boundary validation to align with the tool’s actual version output and mismatch messaging.
    • Ensured YAML is explicitly pinned and validated during Pi SDK installation and related workflow jobs, improving consistency and reliability of the PR review automation.

@prekshivyas prekshivyas self-assigned this Jul 13, 2026
@prekshivyas
prekshivyas requested a review from cv July 13, 2026 21:14
@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 24deefc0-b206-4b6c-b08f-07f5be786fac

📥 Commits

Reviewing files that changed from the base of the PR and between 838c726 and 069b0a5.

📒 Files selected for processing (3)
  • .github/workflows/pr-review-advisor.yaml
  • test/pr-review-advisor-workflow-boundary.test.ts
  • tools/pr-review-advisor/workflow-boundary.mts
🚧 Files skipped from review as they are similar to previous changes (1)
  • tools/pr-review-advisor/workflow-boundary.mts

📝 Walkthrough

Walkthrough

The workflow now pins the YAML runtime dependency, constrains its installation date, and consistently expects fdfind version output across workflow validation and tests.

Changes

Pi SDK dependency and verification pinning

Layer / File(s) Summary
Update Pi SDK installation constraints
.github/workflows/pr-review-advisor.yaml
The workflow pins YAML_VERSION, installs yaml@${YAML_VERSION} with a date constraint, and verifies the fdfind-prefixed binary version.
Align boundary validation and tests
tools/pr-review-advisor/workflow-boundary.mts, test/pr-review-advisor-workflow-boundary.test.ts
Boundary checks require YAML_VERSION and the YAML package installation, while tests update fake fdfind output and validate both runtime dependency pins.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Suggested labels: bug-fix, area: ci

Suggested reviewers: cv, dependabot[bot]

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title matches the main CI change: verifying the packaged fdfind binary identity.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/fix-advisor-fdfind-version

Comment @coderabbitai help to get the list of available commands.

@github-code-quality

github-code-quality Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage remains at 96%, unchanged from the main branch.


Updated July 13, 2026 21:30 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.

@cjagwani cjagwani self-assigned this Jul 13, 2026
@cjagwani

cjagwani commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator

Maintainer gate note (updated after head movement): current exact head is 069b0a5. Both commits are GitHub-verified. Local exact-head verification passed the workflow-boundary integration test (19/19), build:cli, and typecheck:cli; exact-head hosted CI and CodeRabbit are still settling, so do not merge yet. The two Advisor model jobs still fail during Install Pi SDK because pull_request_target executes the trusted workflow from current main ac53164, which contains the incorrect fd 9.0.0 identity this PR repairs; no model evidence was produced and rerunning before this lands cannot succeed. Once exact-head required checks finish and a fresh exact-head human approval is recorded, a human merge will be needed to restore the shared Advisor lane.

@cv
cv merged commit da86438 into main Jul 13, 2026
52 of 60 checks passed
@cv
cv deleted the codex/fix-advisor-fdfind-version branch July 13, 2026 21:36
@cv cv mentioned this pull request Jul 13, 2026
21 tasks
cv added a commit that referenced this pull request Jul 13, 2026
## Summary

This follow-up to #6745 binds `E2E / PR Gate` to the exact pull request
number, head SHA, and base SHA, so base retargets and stale workflow
events cannot reuse or overwrite another diff's result. Unrelated `main`
advances no longer force a full CI rerun when GitHub proves the trusted
E2E control plane is unchanged; ambiguous or changed trust boundaries
still fail closed.

## Changes

- Give canonical PR CI a strict run identity and require the trusted
`.github/workflows/pr.yaml` path before the privileged coordinator
accepts it.
- Isolate metadata-only edits from eligible CI/advisor concurrency while
rerunning both advisors for real base retargets.
- Bind check external IDs, controller state, child dispatch inputs,
evidence finalization, and manual exceptions to both head and base SHAs;
serialize seed/coordinator mutations and revalidate the live open PR
before every security-sensitive transition.
- Pass the accepted workflow SHA to the child E2E workflow and reject a
dispatch race before matrix generation or secret-bearing jobs.
- Accept a newer `main` only when GitHub proves it is a stable
descendant with fewer than 300 fully enumerated changed files and no
`e2e-control-plane` match. This compatibility path is required by the
trusted workflow-run controller because exact tip equality made
unrelated merges invalidate completed PR CI; `test/pr-e2e-gate.test.ts`
covers safe advances, control-plane paths and renames, divergence,
truncation, and second advances.
- Add regression coverage for out-of-order base events, A→B→A retargets,
failed-CI provenance, closed/retargeted finalization, late
manual-exception races, workflow source identity, and concurrency
cancellation boundaries. Update the internal E2E and maintainer
contracts.
- Preserve the packaged `fdfind` fix from #6785 without duplicating it
in this diff.

## 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 changes internal CI and
maintainer behavior only; the internal E2E and merge-gate contracts were
updated.
- [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 correctness
and security review covered workflow-event provenance, cancellation
domains, exact-diff check identity, compare completeness, and race
windows; every finding was addressed with a regression test.
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Verification

- [x] PR description includes the DCO sign-off declaration 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/pr-e2e-gate.test.ts test/pr-e2e-gate-lifecycle.test.ts
test/pr-e2e-gate-exceptions.test.ts test/pr-e2e-gate-workflow.test.ts
test/pr-review-advisor-workflow-boundary.test.ts
test/pr-workflow-contract.test.ts` (104 passed); `npx vitest run
--project e2e-support
test/e2e/support/e2e-operations-workflow-boundary.test.ts` (23 passed)
- [ ] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — command/result:
- [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: Carlos Villela <cvillela@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **New Features**
* PR E2E gates now track exact head and base revisions, preventing
results from being applied after retargeting or new commits.
* Added secure workflow and base revision validation before E2E jobs are
dispatched.
* Added controlled manual resolution paths for eligible no-secret
exceptions.

* **Bug Fixes**
  * Improved handling of edited and retargeted pull requests.
  * Prevented stale or mismatched CI results from completing gates.
* Strengthened cancellation and failure handling for incomplete or
invalid E2E runs.

* **Documentation**
* Updated maintainer guidance for gate exceptions, ruleset rollouts, and
validation requirements.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@wscurran wscurran added area: ci CI workflows, checks, release automation, or GitHub Actions bug-fix PR fixes a bug or regression labels Jul 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: ci CI workflows, checks, release automation, or GitHub Actions bug-fix PR fixes a bug or regression

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants