diff --git a/README.md b/README.md index bf5b1e6..589cf1c 100644 --- a/README.md +++ b/README.md @@ -126,12 +126,15 @@ OPS-generated records in place, excluding human/unmanaged content and Git histor The coordination reference defines collision-checked IDs, assignment lineage, OPS ownership/commit receipts, and `DEV`/`OPS` plus `OCCUPIED`/`REVIEW_READY` label synchronization. -Ordinary implementation ends at `REVIEW_READY`. Code review requires the implementing -worker's request and a different task; ordinary self-review needs explicit user approval. -The separate reviewer directly fixes clear in-scope defects on the same PR, pauses the -original writer, and records its contribution. It does not claim independent review of -its own corrections. A valid review of the final head plus current checks adds -`MERGE_READY` alongside `REVIEW_READY`. +Ordinary implementation completes at `REVIEW_READY`. The user can open a different task +and explicitly request review, or the implementer can record a request. A user request +covers autonomous in-scope review/repair, personal commits, ordinary pushes, guarded rebase +and expected-SHA lease publication, and task/OPS coordination through `MERGE_READY`. +The reviewer can register its user origin without an implementer round trip. Ready/paused +scopes need no predecessor acknowledgement; actual overlapping writers block repair. +Ordinary self-review still needs explicit user approval. Reviewer repairs are disclosed, +not independently reviewed. Rebase onto the current base and verify final head/base before +adding `MERGE_READY`; foreground base drift invalidates it until affected evidence is renewed. Neither label grants merge rights; new edits or invalidated evidence remove `MERGE_READY`. Public records use concise headings and one authoritative set of Markdown fields and nested lists, strictly parsed by tools. No JSON fences or hidden metadata duplicates are @@ -175,3 +178,10 @@ list without executing an unreviewed installer. Tests, CI, `.git` and local main instructions remain in the authoring repository, outside copied release packages. A copy records a source commit only when all packaged bytes match that repository's HEAD; unpublished edits are not mislabeled as the committed release. + +### Intake coordination + +A user-designated moderator can triage report batches using the repository's MODERATOR.md +and the [intake guidance](references/worker-coordination.md#intake-moderators). It reuses +related issues, records scoped additions/evidence and keeps held drafts in chat. Product +implementation ownership and merge/closure authority remain separate. diff --git a/SKILL.md b/SKILL.md index 2bb3c21..37a3cda 100644 --- a/SKILL.md +++ b/SKILL.md @@ -34,9 +34,11 @@ publication remain separately authorized; see docs/releasing.md. of the scoped delivery sequence. Still render concrete paths, messages and evidence before execution. Reassess material scope changes or foreign work; do not repeatedly ask for approval already granted for the same actions. -- Never amend, reset, stash, rebase, force-push, or expose secrets unless separately and - explicitly authorized. Approval of the rendered preview authorizes its exact ordinary - push as part of the commit workflow. +- Never amend, reset, stash, rebase, force-push, or expose secrets without explicit authority. + A scoped delivery preview covers its ordinary push. An explicit user review request in a + separate task covers the bounded rebase/expected-SHA lease exception in + [PR review guidance](references/pr-review.md#review-rebase-and-publication), not blanket force. + Repository and higher-priority runtime/tool approval policies still apply. ## GitHub Tracking for Humans and Agents @@ -190,6 +192,14 @@ work; intake alone does not start implementation or create a background service. them. Fetch before integration, preserve active workers' checkouts and dirty work, and sync the correct integration checkout. No idle polling or unassigned backlog work. +### Intake moderators + +When the user assigns intake or coordination, follow the repository's MODERATOR.md if +present and the [intake guidance](references/worker-coordination.md#intake-moderators). +Reuse related work, classify each report once and size by coherent outcomes. Keep held +drafts in chat unless the user requests another destination. Moderator notes do not claim +product implementation ownership; merge, close and publication authority remain scoped. + ### Coordinator and worker ownership When the user assigns workers or requests succession, read @@ -372,7 +382,10 @@ would happen. State both resolved settings in the preview. Contradictory explici for the same setting require one focused clarification. If the requested method is unavailable or conflicts with a protection, report that constraint rather than silently choosing another method or modifying repository settings. Rebase is not an automatic -fallback and remains subject to separate explicit authorization. +fallback and requires explicit authority. The branch rebase covered by an +[explicit user review request](references/pr-review.md#review-rebase-and-publication) +already has that scoped authority; do not request it again. That exception does not +authorize a merge or changing the requested merge method. Default squash is a proposed merge method, not authorization to merge. Keep the existing preview, approval, issue-creation, PR-publication, and branch-deletion boundaries. Do not @@ -385,13 +398,14 @@ perform or publish an ordinary code review by default, at delivery checkpoints, because a repository enables tracking. Read [PR review guidance](references/pr-review.md) only for an eligible request or an explicit user-approved exception. -- Ordinary review requires the implementing worker's durable request and takes place - in a different conversation/task, strictly within that implementation scope. Record the - requesting worker, implementation task, reviewer task, PR, head/base and outcome. The - requested separate reviewer directly repairs clear defects within that implementation - scope on the same PR by default, then verifies and publishes the final result. A changed - worker name in the same conversation does not satisfy task separation. Do not - spawn a new task without the runtime's required user authority; record a pending request. +- Ordinary review requires a durable user-origin or implementing-worker request in a + different conversation/task. REVIEW_READY completes implementation; the user can open a + separate task and request the named PR's review. That explicit request authorizes autonomous + in-scope review, repair, verification, personal commits, ordinary push, guarded rebase/lease + publication and user-visible task/OPS coordination through MERGE_READY, without repeated + permission. The reviewer can register the user-origin request directly while preserving + original worker/task provenance, scope and head/base. A changed worker name alone does not + satisfy task separation. Do not create a new task without the runtime's required authority. - Self-review is prohibited for ordinary workers unless the user explicitly approves it for the named scope. An implementing worker cannot approve its own exception. This exception does not claim independent authorship or human review. An implementation @@ -399,10 +413,11 @@ only for an eligible request or an explicit user-approved exception. is instead disclosed as reviewer-as-contributor under the repair protocol; it cannot claim independent review of its own corrections. - Reuse implementation/test evidence and inspect only the requested diff and direct - contracts/callers. Before repair, pause the original writer, declare files/scope and - expected head/base, remove `MERGE_READY`, and use Draft/`OCCUPIED`. Commit and ordinarily - push scoped fixes personally to the same PR; OPS records the contribution and final - head. Leave change requests/questions only for complex intent, material scope changes, + contracts/callers. Before repair, declare files/scope and expected head/base, remove + `MERGE_READY`, and use Draft/`OCCUPIED`. Verified ready/paused scopes need no predecessor + acknowledgement; actual overlapping writers block the repair. Coordinate only this PR's + work through authorized task messages and OPS records. Commit/publish scoped fixes personally + to the same PR; OPS records the contribution and final head. Leave change requests/questions only for complex intent, material scope changes, unresolved authority or ambiguity. No unrelated review or wider audit is implied. Verification and final diff/scope checks remain part of implementation; they are not a PR approval. - Authorized successful reviews start exactly with `Review: LGTM` for a separate reviewer, @@ -412,8 +427,9 @@ only for an eligible request or an explicit user-approved exception. Preserve historical headings; use a concrete `Changes requested` outcome for blockers. - A valid review plus current passing checks and matching PR head/base permits adding `MERGE_READY` alongside `REVIEW_READY`; it does not replace the work status. Reconcile - the record and labels before reporting readiness. Remove `MERGE_READY` when edits, - head/base changes, a required check failure or an invalid review invalidate its evidence. + the record and labels before reporting readiness. Rebase onto the current base before final + review readiness. At a foreground checkpoint, changed base/head invalidates MERGE_READY: + reconcile, rebase and renew affected evidence before restoring it. No scheduler is implied. - Publish through the configured OPS identity where required and use COMMENT when formal self-approval is unavailable. No heading or either ready label grants merge, issue-edit, deployment or local-main authority. Do not repeat an unchanged review. @@ -561,9 +577,11 @@ branch. matching local branch destination can be established; otherwise treat it as a blocker. - Fetch the previewed destination before execution and stop if the remote changed in a way that makes the push non-fast-forward or invalidates the preview. -- Use an ordinary push only. Never force-push, use `--force-with-lease`, rewrite remote - history, bypass branch protection, or change remote configuration. -- Treat detached HEAD, ambiguous or missing remotes, non-fast-forward history, failed +- Use an ordinary push by default. Only the explicit user-requested review exception in + [PR review guidance](references/pr-review.md#review-rebase-and-publication) permits a + rebase publication with an exact branch and expected remote SHA in `--force-with-lease`. + Never use plain force, rewrite other remote history, bypass protection or change remotes. +- Treat detached HEAD, ambiguous or missing remotes, unexpected non-fast-forward history, failed hooks or verification, secret detection, authentication failure, protected-branch rejection, and newly discovered foreign work as blockers. - After pushing, verify the remote-tracking ref contains the final local commit and diff --git a/references/pr-review.md b/references/pr-review.md index fa5a580..a14d026 100644 --- a/references/pr-review.md +++ b/references/pr-review.md @@ -1,16 +1,26 @@ # Focused Pull Request Reviews -Ordinary worker reviews are off by default. The implementing worker must request a -review of its exact implementation scope, and a different conversation/task performs it. -A delivery checkpoint, ready label or repository tracking opt-in is not a request. -The requested separate reviewer fixes clear defects within that scope on the same PR -by default, then verifies the final head and records readiness. Do not review unrelated -work, start a monitor or invent new implementation scope. +Ordinary worker reviews are off by default. REVIEW_READY completes implementation. +The user may open a separate task and explicitly request review of the named PR, or the +implementing worker may record a review request. A ready label or tracking opt-in alone +is not a request. The separate reviewer fixes clear defects within that implementation +scope on the same PR, verifies the final head/base and records MERGE_READY. Do not review +unrelated work, start a monitor or invent new implementation scope. + +An explicit user review request grants standing authority for that PR's in-scope review, +repair, verification, personal commits, ordinary push, guarded rebase/expected-SHA lease +publication and user-visible task/OPS coordination through MERGE_READY. Do not ask again +for those routine actions. Repository and higher-priority runtime/tool approval policies +still apply; this document cannot override a tool restriction or missing runtime permission. +It grants no implicit merge, issue-scope edit, user-main update, credentials or deployment rights. ## Request and eligibility -- Preserve a durable request with implementing worker, Assignment, PR, implementation - task identifier, requested scope, exact head/base and existing verification evidence. +- Preserve a durable request with its origin and authorization evidence, implementing worker, + Assignment, PR, implementation task identifier, scope, exact head/base and existing checks. + For an explicit user request, the reviewer may register the user-origin request itself; + no additional implementing-worker request is needed. Keep the request's user origin separate + from original implementation provenance; never claim the reviewer was the original worker. The reviewer records a different task identifier and its actual worker ID. A new model or ID in the same task is not a separate review. If task identity cannot be established, leave review pending instead of asserting separation. Missing chat history does not @@ -35,15 +45,24 @@ work, start a monitor or invent new implementation scope. direct contracts/callers; no unrelated audit is authorized. 2. Correct clear, reproducible implementation-scope defects directly. Before any edit, declare the bounded repair scope/files and expected head/base in the OPS record, - ensure the original writer is paused, preserve its changes and use the same PR branch. - Set Draft/`OCCUPIED` and remove `MERGE_READY`. Re-read the claim and head/base immediately - before writing; actual concurrent writes or changed expectations stop that repair. + preserve changes and use the same PR branch. A verified ready/paused scope needs no + predecessor acknowledgement or another user confirmation. Use the authorized user-visible + task messages and OPS records to coordinate pause/resume within this reviewed PR; actual + overlapping writers block repair. Set Draft/`OCCUPIED` and remove `MERGE_READY`. Re-read + the claim and head/base immediately before writing; reconcile drift before continuing. + If the runtime rejects only an optional notification message, report it as unsent and + continue only when exclusive write ownership and every required OPS record are already + verified. If it rejects required coordination or a required record, stop the affected + stage. Never bypass a rejection or retry the rejected action indirectly through another + tool/account/channel; higher-priority runtime restrictions remain binding. 3. Make the smallest coherent fix and meaningful regression test. Personal identity authors/commits/pushes the repair to the same PR; OPS records original author/request, reviewer-as-contributor, claimed files, repair commits and resulting head. Preserve - authorship/history. Do not force-push, create a replacement PR, merge, edit issue scope - or acquire unrelated ownership merely because review was requested. -4. Reuse valid prior tests; rerun affected checks after corrections and verify the final + authorship/provenance and a recoverable checkpoint. Apply only the guarded rebase/lease + exception below; do not create a replacement PR, merge, edit issue scope or acquire + unrelated ownership merely because review was requested. +4. Rebase the reviewed branch onto its current base before final readiness under the protocol + below. Reuse valid prior tests; rerun affected checks after corrections and verify the final PR head/base and accepted implementation scope. Pending CI remains pending. Record the actual repair range and state explicitly that the reviewer contributed corrections and is not an independent reviewer of those corrections. This remains `Review: LGTM` @@ -79,7 +98,7 @@ Successful reviews use one exact first line, according to the actual assignment: | Approval heading | Applicable work | | --- | --- | | `Self-review: LGTM` | Own or inherited implementation only under explicit user-approved self-review for this scope. | -| `Review: LGTM` | Implementing-worker-requested review in a different task, including bounded same-PR repairs with reviewer-as-contributor disclosure. | +| `Review: LGTM` | User-origin or implementing-worker-requested review in a different task, including bounded same-PR repairs with reviewer-as-contributor disclosure. | | `Conflict resolution: LGTM` | Verified integration by the explicitly assigned `conflict-resolver` with review authority for the named PR set. | These are the only approval headings; never use bare `OK`, bare `LGTM`, or another @@ -103,6 +122,32 @@ Neither ready label nor any review heading grants merge, issue-edit or deploymen An explicit resolver preserves original authors and follows the extra tree/sequence gates in [conflict resolution](conflict-resolution.md); ordinary reviews do not activate them. +## Review rebase and publication + +For an explicit user review request, complete this procedure within the named PR's isolated +owned worktree. An implementing-worker request alone does not invent history-rewrite authority; +use existing explicit or repository standing permission for that operation. + +1. Fetch the target base and reviewed branch, record their full SHAs, inspect divergence and + current write ownership, and preserve a recoverable pre-rebase checkpoint. Rebase onto the + current target base before final MERGE_READY; if already current, no rewrite is needed. + Preserve original commit authors, repair contribution and scope. Do not absorb foreign work. +2. Run affected verification and inspect the resulting review delta. Keep old/new head and base + evidence in OPS; historical review receipts retain their original SHAs and meaning. +3. Prefer ordinary push. For the necessary rebase of the published reviewed branch, the explicit + user review request authorizes only + `--force-with-lease=refs/heads/:` to that exact branch. + The expected full SHA is captured before rebasing and rechecked immediately before publication. + A changed remote head, overlapping writer, failed lease or protection rejection blocks the + push: reconcile, never refresh the lease blindly to overwrite new work. Plain force, unpinned + leases, base-branch rewrites and protection bypass are forbidden. Verify the remote result. +4. Recheck the current remote head and target base immediately before readiness. Publish an + eligible review and synchronize Ready/REVIEW_READY plus MERGE_READY only for that verified + final head/base. If the base advances after readiness at a foreground checkpoint, + invalidate MERGE_READY, return to Draft/OCCUPIED, then rebase/reverify the affected scope and + renew the records. Reuse unchanged evidence; do not rewrite historical reviews. This is + foreground continuation of the authorized task, not a background monitor or merge grant. + ## Project-oriented reason codes These are practical project categories, not an externally certified standard. Use a diff --git a/references/worker-coordination.md b/references/worker-coordination.md index 240dfe5..fe96a3c 100644 --- a/references/worker-coordination.md +++ b/references/worker-coordination.md @@ -209,21 +209,28 @@ Missing or inconsistent records block a readiness claim. `REVIEW_READY`, synchronize authoritative state, mirrors, labels and PR Ready status. Verify all updates before claiming readiness. The ordinary implementing worker is done; do not automatically review its own or other workers' code or post an approval heading. -4. **Requested review:** the implementing worker records its exact scope and requests review - in a different conversation/task under [review eligibility](pr-review.md#request-and-eligibility). - Record both task IDs, actual workers and head/base evidence. A worker cannot authorize +4. **Requested review:** after completed REVIEW_READY delivery, the user may open a different + task and request the named PR's review, or the implementing worker may record a request + under [review eligibility](pr-review.md#request-and-eligibility). The reviewer may register + an explicit user-origin request directly, preserving user authorization evidence, original + worker/task provenance, both task IDs, scope and head/base; no implementer round trip is needed. A worker cannot authorize its own self-review; only an explicit user-approved exception permits `Self-review: LGTM`. The separate reviewer repairs clear in-scope defects on the same PR by default. Follow - the [repair protocol](pr-review.md#review-and-repair) with paused original writer, - declared files, expected head/base and Draft/`OCCUPIED`. Record reviewer-as-contributor + the [repair protocol](pr-review.md#review-and-repair) with declared files, expected head/base + and Draft/`OCCUPIED`. Ready/paused scopes need no predecessor acknowledgement; actual + overlapping writers block repair. The explicit user request covers bounded task messages, + OPS coordination, repairs, checks, commits/pushes and guarded rebase/expected-SHA lease + publication through MERGE_READY. Record reviewer-as-contributor without replacing original provenance. Eligible separate reviews use `Review: LGTM` with correction disclosure; an explicitly granted resolver may use `Conflict resolution: LGTM` under its additional gates. 5. **Merge ready:** add `MERGE_READY` alongside `REVIEW_READY` only with a valid eligible - review and current passing checks for the matching head/base. Re-read all records and - labels; partial synchronization is not readiness. Remove `MERGE_READY` on new edits, - changed head/base, failed required checks or invalid review. Return to Draft/`OCCUPIED` - before further implementation. Neither readiness label grants merge authority. + review and current passing checks for the matching head/base after the requested review's + [rebase/publication procedure](pr-review.md#review-rebase-and-publication). Re-read records + and labels; partial synchronization is not readiness. At foreground checkpoints, invalidate + MERGE_READY on base/head drift and return to Draft/OCCUPIED, then rebase/reverify the affected + scope before restoring it. Failed checks or invalid review also invalidate readiness. + No scheduler is implied. Neither readiness label grants merge authority. 6. **Finish:** only after an authorized maintainer or resolver merge, reconcile actual merge/issue receipts under that authority. Partial delivery leaves scope open. Named commit-error/conflict resolvers may close assigned PRs or edit assigned issues only if @@ -263,3 +270,32 @@ references: [Codex App Server](https://learn.chatgpt.com/docs/app-server), [Codex CLI commands](https://learn.chatgpt.com/docs/developer-commands?surface=cli), [Claude Code sessions](https://code.claude.com/docs/en/sessions), and [Claude Code Desktop](https://code.claude.com/docs/en/desktop#work-across-sessions). + +## Intake moderators + +A user-designated moderator owns intake and coordination, not product implementation. +Use the repository's MODERATOR.md for project-specific routes and evidence locations. + +- Reuse the round's related issue/ownership snapshot, then recheck affected owner/head/body + before writes. Avoid scanning every PR or repeating unchanged dispositions. +- Classify each report as duplicate, additive, new or held. Add only new evidence to a + duplicate; reconcile additive scope through the authorized issue note/body workflow. + A demonstrated regression after a completed fix needs a linked active record, not an + automatic reopen. Preserve acceptance intent and valid verification when criteria change. +- Size by independently verifiable outcomes and shared contracts. A small actionable issue + can stay small; research or a check count alone does not justify splitting. Preserve a + user-declared single-worker epic. Held drafts stay in the chat/handoff summary and are + revisited next round; never create automatic persistent-memory copies. +- Notify a product owner through its linked issue and the round summary. Ready work normally + receives a linked follow-up for added scope unless the user directs a scope change. + Do not claim its assignment or bypass a non-owner PR-write restriction. An evidence or + policy PR actually owned by the moderator follows ordinary owned delivery records. +- Prefer existing durable evidence; check privacy, preserve originals and distinguish + historical captures from reproductions. Publish only under the relevant authority, then + link the immutable artifact. The role itself grants no merge, close or checkout rights. +- Batch scoped records, serialize writes and use bounded tool-guided backoff. Missing + capabilities or runtime refusals never authorize another account/tool bypass. + +Report one compact table per batch: report, disposition, destination, next action. +Anything the implementation owner must act on belongs in the authorized GitHub record; +chat carries decisions and held drafts, not a competing repository backlog.