-
Notifications
You must be signed in to change notification settings - Fork 3.1k
fix(skills): lock PR repair scope #10939
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
0e881fc
1c0cd68
69da117
f3cee4e
896a40b
e81180b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -37,6 +37,30 @@ This fetch refreshes read-only comparison evidence. It does not authorize mergin | |
| `main` into the candidate. Follow [Integrate the base branch](../_shared/pr-follow-up.md#integrate-the-base-branch) | ||
| before changing candidate history. | ||
|
|
||
| ### Scope lock | ||
|
|
||
| For initial publication, record the accepted outcome, delivered behavior, permitted mechanisms, | ||
| changed paths, total additions and deletions, and deferred scope. Use the implementation handoff and | ||
| the candidate diff against the canonical comparison ref. This record is the scope lock. Its changed | ||
| paths and totals are immutable caps for that candidate. | ||
|
|
||
| For an open PR, require the scope lock from the original lifecycle handoff before review collection. | ||
| If it is unavailable, stop before implementation, commit, or push and ask the user or maintainer to | ||
| establish the record or defer the repair. Resume stabilization for the unchanged head after an | ||
| authorized record is available. | ||
|
|
||
| Preserve the scope lock and use each repair envelope returned by the shared follow-up contract. Route | ||
| one root-cause group per implementation handoff. Record the local state before each handoff, then | ||
| measure only that handoff's added, modified, renamed, or deleted paths and growth against its group's | ||
| envelope before routing another group. Require the complete accumulated repair to remain within the | ||
| semantic scope lock. Do not widen an envelope after its implementation starts. | ||
|
|
||
| A smaller diff is allowed when it still delivers the accepted outcome. Numeric headroom does not | ||
| authorize unrelated changes. A reviewer, bot, implementation worker, formatter, or validator cannot | ||
| expand the lock. A general request to address review findings does not authorize expansion. Remove | ||
| separable out-of-scope work. Otherwise, stop before commit or push and ask the user or maintainer to | ||
| approve the expansion or defer it to a separate change. | ||
|
|
||
| ### Validation | ||
|
|
||
| Normal `pre-commit`, `commit-msg`, and `pre-push` hooks provide early feedback, but a successful commit or push does not prove that they ran; hooks can be missing, stale, or redirected through `core.hooksPath`. | ||
|
|
@@ -47,9 +71,9 @@ Select review evidence for the publication state before every agent-managed push | |
| - Before updating an open PR: | ||
|
|
||
| 1. Follow [Stabilize](../_shared/pr-follow-up.md#stabilize-the-candidate), [Collect](../_shared/pr-follow-up.md#collect), and [Decide](../_shared/pr-follow-up.md#decide) for the recorded remote `headRefOid`. | ||
| 2. Route only returned in-scope root-cause groups to `nemoclaw-contributor-implement-issue` with their returned scope records. | ||
| 3. Inspect the returned change and test evidence because the shared contract cannot repair, validate, commit, or push. | ||
| 4. Create one local repair commit and record it as the expected publication SHA. | ||
| 2. Route one returned in-scope root-cause group at a time to `nemoclaw-contributor-implement-issue` with the original PR objective, accepted scope, deferred scope, and complete group. | ||
| 3. Inspect the returned change and test evidence. Measure its delta from the recorded pre-handoff local state and require it to fit only that group's envelope before routing another group. | ||
| 4. After every group-specific check passes, require the accumulated repair to fit the semantic scope lock. Then create one local repair commit and record it as the expected publication SHA. | ||
| 5. Mark each accepted repair group resolved by the inspected local repair, subject to trusted validation. | ||
| 6. Reread `headRefOid` before the canonical base fetch and restart collection only when it differs from the reviewed remote SHA. | ||
| 7. Do not push while the original collection is pending, a finding is unclassified, an accepted group lacks an inspected repair, or validation is unresolved. | ||
|
|
@@ -71,7 +95,12 @@ Confirm that the complete validation execution surface is byte-for-byte identica | |
|
|
||
| Do not infer executable identity from a package name or version. Do not use a branch-defined validator as independent evidence. If any surface differs, is unavailable, or cannot be traced, do not execute the candidate validator or publish. Report the path or executable and canonical base SHA. | ||
|
|
||
| Run `npm run validate:pr` before every agent-managed push only after that comparison succeeds. Do not push when it fails or is inconclusive. If it changes a tracked file, inspect and commit the validator-created local diff. Record the new commit as the expected publication SHA. Do not reuse review evidence from the earlier commit for that later change. Before the first push, repeat the initial-publication review step for the new commit, including a self-review of the validator-created diff. For an open PR, preserve the completed remote disposition record and inspect the validator-created local diff as new pre-publication review evidence without recollecting the unchanged remote candidate. Refresh and resolve the trusted base, reestablish the trusted validation surface, and rerun validation. Use `npm run check` for repository-wide validation changes, such as hooks, formatter configuration, generated-check scripts, or coverage baselines. | ||
| Run `npm run validate:pr` before every agent-managed push only after that comparison succeeds. Do not push when it fails or is inconclusive. If it changes a tracked file, inspect the validator-created local diff and repeat the complete scope comparison. Discard those changes and stop before commit or push when they exceed the scope lock or a repair envelope. For multiple repair groups, attribute each validator change to its group and remeasure that group's cumulative delta; discard changes that cannot be attributed. Otherwise, commit them and record the new commit as the expected publication SHA. Do not reuse review evidence from the earlier commit for that later change. Before the first push, repeat the initial-publication review step for the new commit, including a self-review of the validator-created diff. For an open PR, preserve the completed remote disposition record and inspect the validator-created local diff as new pre-publication review evidence without recollecting the unchanged remote candidate. Refresh and resolve the trusted base, reestablish the trusted validation surface, and rerun validation. Use `npm run check` for repository-wide validation changes, such as hooks, formatter configuration, generated-check scripts, or coverage baselines. | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P2] Define recovery after a validator-induced scope breach. A deterministic validator can recreate the same out-of-envelope tracked change on every required run, but this branch only discards it and stops. Record the affected paths, diff totals, and owning repair group, then require either an in-envelope source correction that leaves validation clean or deferral to a newly authorized candidate; keep the active envelopes frozen. |
||
|
|
||
| For initial publication, the scope lock's path and diff totals are immutable caps: accept a | ||
| validator-created change only when the complete candidate still uses the recorded paths and does not | ||
| exceed either total. After a permitted base integration, replace the candidate-specific SHA, paths, | ||
| and totals from the new diff baseline while preserving the semantic lock before validation resumes. | ||
|
|
||
| A maintainer may unblock unavailable trusted-base validation only with recorded evidence identifying the base and candidate SHAs, isolated environment, trusted validator entry point and resolved executables, exact command and result, and publication authorization. The environment must not give candidate code contributor-host credentials. | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P2] Keep envelope path authority inside the pre-review lock. This allows a review-derived envelope to introduce a previously unauthorized path, even though the lock above says bots and reviewers cannot expand it. A malicious or mistaken finding can therefore authorize edits to sensitive workflows, policies, installers, or credential paths. Record an authorized repairable-path set before collection, or require an explicit user or maintainer expansion—path, behavior, mechanism, and reason—before an envelope may add a path.