Skip to content

fix(ci): repair main branch regressions - #11170

Merged
cv merged 4 commits into
mainfrom
fix/main-ci-regressions-34114747276
Sep 7, 2026
Merged

fix(ci): repair main branch regressions#11170
cv merged 4 commits into
mainfrom
fix/main-ci-regressions-34114747276

Conversation

@sandl99

@sandl99 sandl99 commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

Outcome

Restores the two paths failing in main CI: registry-only fresh onboarding reconstructs retained sandbox identity before gateway preflight, and Hermes legacy-layout startup accepts the trusted pre-migration directory posture.

Reason

Main CI run 34114747276 repeated both failures across attempts. The onboarding coverage added by #11124 kept its verified create checkpoint only in memory, while the Hermes hardening from #11110 rejected the safe legacy 0750 directory mode before startup could repair it.

Changes

  • Capture the verified pending-create entry when the generated fixture persists it and pass that entry only to the registry-only recovery child.
  • Include the gateway port in the shared verified-create reservation fixture so the durable pending entry matches the production registry authority.
  • Accept sandbox-owned 0750 as a trusted transitional Hermes root posture; the existing startup path remains responsible for repairing it to the canonical mode.
  • Reject the adjacent 0755 posture and retain the exact ownership, group, and mode boundary.
  • Refresh the Dockerfile integrity pin for the changed security validator.

Verification

  • Contributor validation: npm run validate:pr passed for e080d1f.
  • Focused tests: the Hermes 0750 acceptance and 0755 rejection tests passed; the validator digest-binding probe passed; the Hermes startup mode-repair test passed; the onboarding recovery unit suite passed 40/40; npm run build:cli passed.
  • Repository checks: test-title, growth-guardrail, formatting, lint, type-check, hadolint, and secret-scan checks passed.
  • The Linux-only Docker-backed onboarding regression case is delegated to PR CI because this macOS checkout skips that lane by design.
  • Secrets review: the diff contains no secrets, API keys, or credentials.

Signed-off-by: San Dang sdang@nvidia.com

Summary by CodeRabbit

  • Bug Fixes
    • Improved environment validation to recognize valid sandbox directory permissions while continuing to reject insecure configurations.
    • Strengthened startup checks to prevent unauthorized or incorrectly scoped environment access.
    • Improved recovery flows by reliably using the retained verified checkpoint when current registry data is unavailable.
    • Added safeguards for recovery readback, helping preserve verified identity information across process boundaries and preventing recovery when required checkpoint data is missing.

Signed-off-by: San Dang <sdang@nvidia.com>
@sandl99 sandl99 self-assigned this Sep 7, 2026
@copy-pr-bot

copy-pr-bot Bot commented Sep 7, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@coderabbitai

coderabbitai Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 317b93b1-b593-4392-bc67-b61cc914836f

📥 Commits

Reviewing files that changed from the base of the PR and between 274ce89 and e080d1f.

📒 Files selected for processing (2)
  • test/helpers/onboard-script-mocks.cjs
  • test/onboarding/onboard-fresh-create-identity.test.ts

📝 Walkthrough

Walkthrough

The PR updates Hermes environment validation to accept mode 0750 and pins the corresponding validator hash. It adds boundary tests and adjusts onboarding recovery tests to retain and restore verified registry checkpoints.

Changes

Hermes environment boundary

Layer / File(s) Summary
Hermes mode validation and integrity pin
agents/hermes/validate-env-secret-boundary.py, agents/hermes/Dockerfile, test/agents/hermes/hermes-env-secret-boundary-hardening.test.ts
The validator accepts sandbox-owned mode 0750. The Dockerfile updates the validator hash. Tests cover accepted 0750 and rejected 0755 configurations.

Onboarding recovery

Layer / File(s) Summary
Recovery checkpoint handling
test/onboarding/onboard-fresh-create-identity.test.ts, test/helpers/onboard-script-mocks.cjs
Recovery tests retain and restore the verified checkpoint. Registry-only recovery requires checkpoint data. Reservation fixtures include the gateway port.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 274ce

This change restores onboarding recovery fixture behavior and permits the intended transitional Hermes directory mode without allowing the adjacent world-readable posture. No actionable merge-blocking risk remains.

Suggested reviewers: prekshivyas, cv

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the primary change: repairing CI regressions on the main branch. It is concise and consistent with the changeset.
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.
  • Fix all pre-merge checks with AI
✨ 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 fix/main-ci-regressions-34114747276

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

@github-code-quality

github-code-quality Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall line coverage in commit e080d1f in the fix/main-ci-regressi... branch remains at 96%, unchanged from commit 5ab3341 in the main branch.


Updated September 07, 2026 14:50 UTC

Signed-off-by: San Dang <sdang@nvidia.com>
@sandl99
sandl99 marked this pull request as ready for review September 7, 2026 13:57
Signed-off-by: San Dang <sdang@nvidia.com>
Signed-off-by: San Dang <sdang@nvidia.com>
@cv
cv merged commit 1166a1e into main Sep 7, 2026
83 of 84 checks passed
@cv
cv deleted the fix/main-ci-regressions-34114747276 branch September 7, 2026 14:51
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor finished for commit e080d1f. Include the Advisor findings in the complete PR feedback collection. Verify and group valid findings before repair.

All previous runs

sandl99 added a commit that referenced this pull request Sep 8, 2026
<!-- markdownlint-disable MD041 -->
## Outcome

Hermes creates its fresh Kanban schema in one SQLite transaction. This
avoids one synchronous disk commit for each schema statement during
managed-image startup.

## Reason

Hermes 0.20.6 opens the Kanban database in autocommit mode with
`synchronous=FULL`, then calls `conn.executescript(SCHEMA_SQL)`. Without
an explicit transaction, SQLite commits each `CREATE` statement
separately. Slow overlay storage can therefore keep the gateway outside
the three-minute health gate.

#11170 fixed permission validation. It did not change this Kanban
initialization path, so rebuilding the image exposed the existing
startup delay.

## Changes

- Require the pinned Hermes source to contain one
`conn.executescript(SCHEMA_SQL)` call.
- Wrap that call with `BEGIN IMMEDIATE` and `COMMIT` during the image
build, then compile the module.
- Make the legacy root-entrypoint fixture clear inherited directory
special bits with GNU `chmod`'s five-digit form.

## Verification

- `npm run validate:pr` passed against canonical `main` at
`efd56a372999acc9d97936e9b30b2164743255a5`.
- `npm run checks:repository` and the exact E2E mock/live parity gate
passed.
- Focused `e2e-support` validation passed: 13 tests.
- GNU coreutils 8.32 in Ubuntu reproduced `0750 -> 2750` and verified
`00750 -> 750` from an inherited `03770` directory.
- The diff contains no secrets, API keys, or credentials.

---

Signed-off-by: San Dang <sdang@nvidia.com>

---------

Signed-off-by: San Dang <sdang@nvidia.com>
@wscurran wscurran added the bug-fix PR fixes a bug or regression label Sep 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug-fix PR fixes a bug or regression

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants