refactor(skills): fold Hermes upgrade guidance - #8179
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit e444c33 in the TypeScript / code-coverage/cliThe overall coverage in commit e444c33 in the Show a code coverage summary of the most impacted files.
Updated |
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
3 terminology differences from the second opinionAdvisory only. These are normalized differences from the primary terminology receipt.
Second-opinion terminology and E2E selections are advisory. They do not change the primary assessment or E2E / PR Gate. 3 semantic terminology decisionsTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: None This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
apurvvkumaria
left a comment
There was a problem hiding this comment.
Reviewed the exact stacked delta at head ec7a804. Hermes trigger routing now lands in the general dependency workflow; the conditional variant preserves the CalVer collection and immutable base-image publication gates, while the parent contract audit retains the removed security, lifecycle, state, provenance, platform, and E2E coverage. No stale old-skill references remain. The moved collector now publishes through a mode-0600 temporary file with fsync, atomic no-clobber linking, and cleanup. Focused validation passed 36 tests across the variant, collector, and frontmatter suites, and the diff check is clean. Current red image and npm-audit checks concern unchanged runtime advisories; the aggregate CodeQL annotation is on a test-fixture permission assertion, not production behavior. No blocking correctness, security, compatibility, or regression defect found.
| if created and not complete: | ||
| try: | ||
| os.unlink(output.name, dir_fd=directory) | ||
| except FileNotFoundError: |
apurvvkumaria
left a comment
There was a problem hiding this comment.
Re-reviewed exact head 89e7524. The follow-up opens and validates the output directory once, creates the report with exclusive no-follow semantics and private permissions, flushes file and directory state, and removes partial output on failure. This closes directory and leaf race paths without weakening the stacked skill refactor. Focused validation passes 40 of 40 tests on macOS. The CodeQL empty-except note is an intentional missing-file cleanup case, and current red audit and image checks concern unchanged stacked-base runtime dependencies. No blocking defect found.
…ermes-upgrade-variant
56845f3
into
codex/reduce-contributor-skill-drift
Summary
Remove the dedicated Hermes implementation skill and route Hermes release work through the general dependency-upgrade workflow. Keep only the Hermes-specific CalVer collection and base-image publication gates in a conditional reference so implementation details continue to come from source and tests.
This PR is stacked on #8159 and should be reviewed and merged after it. Once #8159 merges, this PR can target
mainwithout carrying duplicate skill-refactor changes.Changes
0600writes, no replacement or symlink following, unsafe-directory rejection, and failure cleanup.Type of Change
Quality Gates
89e75241bwith the repository's nine-category security checklist. All categories passed with no remaining findings; evidence covers descriptor anchoring, ownership and mode checks, files and symlinks, hostile directories, permissive umask, and injected fsync failure cleanup.Documentation Writer Review
docs-updateddocs/change is needed because this changes contributor-agent routing, documented in the skills catalog.DGX 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 unavailablenpm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result: Not applicable to this narrow agent-skill and standalone-helper refactor;npm run checks:repositorypassed.npm run docsbuilds without warnings (doc changes only)Signed-off-by: Julie Yaunches jyaunches@nvidia.com