From 1eb3dc13e7005cc9fa6e043cc8f9aaed7f0e2eaf Mon Sep 17 00:00:00 2001 From: Sungyong Cho Date: Mon, 7 Sep 2026 20:43:43 +0200 Subject: [PATCH 1/5] feat(workflow): preserve worker succession across conversations Summary Prepare Commit It 3.0.0 for user-approved worker succession independent of chat retention. Changes - Preserve assignment lineage, checkpoints and legacy worker records through approved takeover. - Define OPS delivery receipts and synchronized purpose and work-state labels. - Name conversations through supported client tools and preserve honest review ownership. - Align package documentation and prepare version 3.0.0 without publishing a release tag. Verification - 27 package and installer tests passed; shell syntax and v3.0.0 release checks passed. - Independent eight-scenario review completed; readiness and review ownership ambiguities corrected. - git diff --check passed. --- CHANGELOG.md | 13 ++ README.md | 17 ++ SKILL.md | 47 +++-- references/conflict-resolution.md | 6 +- references/pr-review.md | 10 +- references/worker-coordination.md | 279 +++++++++++++++++++----------- tests/test_package.py | 18 ++ 7 files changed, 274 insertions(+), 116 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5982e50..a56658b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,19 @@ ## Unreleased +## 3.0.0 - 2026-09-07 + +- Replace predecessor-acknowledged ownership transfer with explicit user-approved succession, + preserved checkpoints and project assignment lineage independent of disposable chats. +- Allocate collision-checked worker IDs while retaining existing IDs and v1 work-state records; + record successor/model verification without implying independent review or merge authority. +- Define OPS-attributed ownership, commit/PR receipts and synchronized managed labels while + retaining personal development authorship and existing repository permissions. +- Name conversations through available official client tools, with verified results and + a capability-based fallback that never edits internal conversation storage. +- This major version changes the coordination and handoff contract. Existing records remain + readable; label provisioning, tags, releases and account activation are not implicit. + ## 2.9.0 - 2026-09-07 - Add explicitly assigned conflict resolution, a durable merge sequence, exact review diff --git a/README.md b/README.md index ebfed14..df01a52 100644 --- a/README.md +++ b/README.md @@ -114,6 +114,23 @@ Read [worker coordination](references/worker-coordination.md) for named workers [PR review](references/pr-review.md) for review work. Reload the real source when another conversation may have updated it; existing context is not rewritten by filesystem changes. +### Worker succession in 3.0.0 + +Workers are replaceable execution owners. Project assignments, commits, PRs and tested +checkpoints preserve continuity when chats disappear or a later model takes over. +Explicit user approval transfers named work without waiting for a predecessor response; +existing IDs and v1 records remain valid. The changed transfer contract is the reason +for the major version, not an incompatible storage migration. + +The coordination reference defines collision-checked IDs, assignment lineage, OPS +ownership/commit receipts, and `DEV`/`OPS` plus `OCCUPIED`/`REVIEW_READY` label synchronization. +Personal accounts still author development commits and create PRs when the repository +separates those actions from OPS records. Labels do not grant ownership or merge authority. + +Conversation names follow ` | | `. +Use supported official tools and verify the new title; otherwise provide the suggested +title without blocking development. Chat titles and IDs are optional lookup aids. + For an explicitly assigned resolver of a named PR set, read [conflict resolution](references/conflict-resolution.md). Ordinary worker authority is unchanged. diff --git a/SKILL.md b/SKILL.md index 5a9ce32..94544f5 100644 --- a/SKILL.md +++ b/SKILL.md @@ -2,7 +2,7 @@ name: commit-it description: Track requested features, fixes and GitHub delivery, prepare authorized issue-linked commits, perform scoped PR reviews, and resolve an explicitly assigned PR set through verified integration and authorized ordered merges. metadata: - version: "2.9.0" + version: "3.0.0" --- # Commit It @@ -51,6 +51,11 @@ permission for scoped issue/PR tracking. Without that permission or a direct req prepare the proposed external update and obtain the missing authorization. Tracking permission alone never implies commit, push, PR publication, merge or branch deletion. An explicitly approved delivery sequence should proceed without repeated approval. +Where operational writes use a configured OPS account or guarded command, route +claims, labels, progress, commit receipts and review comments through that interface. +The `gh` examples below describe operations, not permission to bypass account routing +or worker restrictions on issue-body edits. Development commits and PR creation keep +the repository's personal author identity, including both commit author and committer. 1. **Start:** use `gh issue list/view` and `gh pr list/view` for the current task only. Reuse the related issue/PR. When creation is authorized and no match exists, create @@ -175,16 +180,30 @@ work; intake alone does not start implementation or create a background service. ### Coordinator and worker ownership -When the user adopts a coordinator with named workers, read -[worker coordination](references/worker-coordination.md). Use the user-assigned IDs, -Draft/Ready PR stages, an authoritative PR work-state block and issue status mirrors. -Separate ownership from verification and authenticate messages with the intended -account/App; a commit email does not set the issue/PR author. This opt-in mode does -not expand repository permissions or create accounts, labels, Projects or background jobs. - -When a repository separates development authorship from operational records, -follow its repository or global identity policy. Keep personal account mappings, -local paths, credentials and deployment configuration outside this reusable skill. +When the user assigns workers or requests succession, read +[worker coordination](references/worker-coordination.md). Workers are replaceable +execution owners, not permanent conversations. Preserve continuity through project +assignments, issues, PRs, commits and reproducible verification. Later models improve +inherited work through tests; a model change alone is neither new evidence nor a reason +to discard valid work. Archived or deleted chats must not prevent approved succession. + +Preserve supplied worker IDs. Otherwise allocate `--<8 hex digits>` +and check project records for collisions. Keep assignment lineage and handoff revisions; +explicit user approval is sufficient to transfer named work without predecessor acknowledgement. +Preserve checkpoints, reconcile current ownership and stop only actual conflicting writers. +This does not expand merge, deployment, credential or published-history authority. + +Use the reference's Draft/Ready stages, authoritative work-state record, issue mirrors, +OPS commit/PR receipts and managed `DEV`/`OPS`, `OCCUPIED`/`REVIEW_READY` labels. Provision +missing labels only within setup authorization. Separate personal development commits +and PR creation from configured OPS ownership/status/review writes; authenticate each +operation independently. Keep personal mappings, host paths and credentials outside this +package. No account, App, Project or background job creation is implied. + +At assignment or handoff, use available official tools to name the current conversation +` | | `. Verify the result; capability +absence only requires a suggested title, not blocked implementation. See the reference +for platform-specific paths and fallback rules. ### Explicit conflict-resolution mode @@ -353,7 +372,8 @@ implementation, issue management, merging, or unrelated cleanup. - Default to a brief review of the exact diff, direct contracts/callers, and relevant verification. Expand only for a concrete risk or finding. Reuse valid test evidence. - When assigned to review PRs made by others, identify work this agent actually created; - account authorship alone cannot distinguish workers sharing one GitHub account. + account authorship alone cannot distinguish workers sharing one GitHub account. A successor + who modifies inherited code uses `Self-review: LGTM`; changing workers is not independent review. - If review publication is authorized and the scope has no blockers, the first line must be exactly `Self-review: LGTM` for work this agent authored or `Review: LGTM` for another worker's work. Only an explicitly assigned conflict resolver may instead use @@ -365,7 +385,8 @@ implementation, issue management, merging, or unrelated cleanup. - A shared author account cannot formally approve or request changes on its own PR. Publish the result as a comment review in that case and describe its actual state. - For an ordinary worker's own completed PRs, reuse the implementation and tester's passing - evidence, briefly review the final diff, and post `Self-review: LGTM` immediately if unblocked. + evidence, briefly review the final diff, and post `Self-review: LGTM` after required + record, label and Draft/Ready synchronization succeeds and no blocker remains. Do not add another audit or repeat valid tests. Recheck only a relevant change or failure. Do not present self-review as independent or human approval. Unfinished work and pending required checks must not receive any approval heading. diff --git a/references/conflict-resolution.md b/references/conflict-resolution.md index 2894f06..76439b3 100644 --- a/references/conflict-resolution.md +++ b/references/conflict-resolution.md @@ -8,8 +8,10 @@ this mode. User and repository authority still govern every action. - Record role `conflict-resolver` with the resolver's actual worker ID, named input PRs, base branch and allowed actions. A role is not a new identity. Preserve source - authorship, earlier worker records, unrelated changes and active worktrees. Confirm - affected writers have stopped before editing their branches; do not take over an + authorship, earlier worker records, unrelated changes and active worktrees. Follow + [user-approved succession](worker-coordination.md#user-approved-succession) for a + transfer: explicit user approval suffices without predecessor acknowledgement. + Preserve checkpoints and pause actual conflicting writes; do not take over an unassigned PR or infer authority from a shared account. - Resolve the named PRs' accepted requirements, defects and cross-PR interactions. Ordinary workers retain their existing PR-only boundaries. Resolver assignment diff --git a/references/pr-review.md b/references/pr-review.md index 1292fb1..050246a 100644 --- a/references/pr-review.md +++ b/references/pr-review.md @@ -30,8 +30,8 @@ unpublished personal design. A further improvement is blocking only when necessa the authorized outcome; otherwise keep it explicitly optional. For an ordinary worker's completed PR, reuse the implementation and tester's passing evidence. -Briefly inspect the final diff and immediately post `Self-review: LGTM` if it has no -blocker; do not add a second audit or rerun valid tests. Recheck only a relevant new +After required record, label and Draft/Ready synchronization succeeds, briefly inspect +the final diff and post `Self-review: LGTM` if it has no blocker; do not add a second audit or rerun valid tests. Recheck only a relevant new change or failure and name the reviewed head. An unfinished scope, known blocker or pending required check must not receive any approval heading. @@ -47,6 +47,12 @@ These are the only approval headings; never use bare `OK`, bare `LGTM`, or anoth variant. All require the same completed scope and verified evidence for the recorded head/base; none claims independent human approval or guarantees that every behavior was tested. Determine authorship from actual work, not the shared GitHub account. +A successor who edits inherited code uses `Self-review: LGTM`, preserves earlier +worker attribution and links the reused verification. A new worker/model/account is +not independent review by itself. A separate review-only assignment may use `Review: LGTM` only +without taking implementation ownership or implementing the reviewed work. A successor +accepting implementation ownership reports completion as self-review, including inherited work. Publish through the configured OPS review +identity when required; the record still identifies the actual reviewing worker. A review never grants permission to merge or deploy. A conflict resolver preserves original authorship and follows the additional tree/sequence requirements in [conflict resolution](conflict-resolution.md); ordinary reviews do not activate it. diff --git a/references/worker-coordination.md b/references/worker-coordination.md index fc031c4..57d6a04 100644 --- a/references/worker-coordination.md +++ b/references/worker-coordination.md @@ -1,123 +1,204 @@ # Coordinator and worker ownership -Use this mode when the user explicitly adopts a coordinator plus named execution -workers. Ordinary tasks do not require this protocol. Repository role boundaries -still apply; prepare a record rather than publish it if status writes are unauthorized. - -## Roles and identity - -- The coordinator handles issue intake/scope, assigns work and normally performs - authorized merges or integration-checkout synchronization. Fetch and fast-forward - a stable main when possible; do not rewrite main just because a request says rebase. - Published-history rewrites and foreign checkout changes require explicit authority. -- Workers implement their approved assignment in isolated persistent worktrees, - verify it, publish/update its PR and provide the review result. The user assigns - stable IDs such as `worker-1`; retain the same ID across the assignment. Ask for a - missing ID before publishing ownership. Never allocate another worker's identity. -- An assignment has one writer. Multiple scopes on one issue need an explicit, - disjoint coordinator allocation. A bundle of issues shares one assignment ID. - Shared account authorship, issue assignees and PR labels do not identify a worker. - The user determines these workers and their count; this mode does not automatically - spawn tasks or give subagents separate worker identities. +Use this mode for explicitly assigned workers or user-approved succession. Ordinary +unassigned tasks do not require it. Repository role boundaries still apply; prepare +records rather than publish them when status writes are unauthorized. + +## Project continuity and identity + +Workers are replaceable execution owners, not permanent conversations. Preserve project +continuity in assignments, issues, PRs, commits and reproducible tests. A later model +may inherit and improve work incrementally; model novelty does not prove improvement +or invalidate existing checks. Chats may be archived or deleted. Their identifiers and +titles are optional navigation aids, never the only location of checkpoints or approval. + +- The coordinator handles authorized intake, scope and allocation. Ordinary workers + implement an approved assignment in persistent isolated worktrees, verify it and + deliver its PR and review. They do not inherit coordinator merge or checkout rights. +- Preserve user-supplied IDs, including legacy names such as `worker-1`. If none is + supplied, allocate `--<8 lowercase hex digits>`, using UTC + and a random suffix; for example `codex-20260907T190000Z-a1b2c3d4`. Verify against + existing project worker records before registration and regenerate on collision. + Use the actual client family such as `codex` or `claude`, not an inferred model. +- A new independent execution owner gets a new ID. Resume, context compaction and + renaming the same execution retain its ID. Record verified model/client information + separately when useful; an in-place model switch records its verification interval. + Never infer a model from a user-visible title or claim unsupported runtime metadata. +- Keep the existing Assignment across succession. Link every original Assignment when + combining previously separate scopes; do not overwrite lineage with a new bundle ID. + One assignment has one writer. Multiple scopes on one issue require disjoint explicit + allocation. An account, label or GitHub assignee is not a worker identity or lock. +- This protocol does not launch agents or increase staffing. Honor the authorized + capacity and keep shared contracts and Git mutations under one owner. ### Explicit resolver assignment -The user may assign role `conflict-resolver` for a named PR set under -[conflict resolution](conflict-resolution.md). Preserve the actual worker ID, source -authors and earlier worker history; the role does not create a new identity or confer -coordinator authority. Confirm stopped writers and the authorized assignment before -branch edits. An explicitly authorized resolver may merge that set, but ordinary -workers remain PR-only and a review heading never grants merge or checkout-sync rights. +An explicitly assigned `conflict-resolver` follows +[conflict resolution](conflict-resolution.md) for only its named PR set. Retain its +actual worker ID, prior workers and original authors. Apply the succession safeguards +below when taking over writers; the role does not silently grant coordinator rights. +Only separate named merge authority permits that sequence's merges. Ordinary workers +remain PR-only, and no review heading grants merge or integration-checkout authority. -Keep the existing `Work status` transitions below. The resolver separately owns the -canonical `commit-it:merge-sequence:v1` comment and its `Merge status`, including after -the anchor PR merges. Update only records within the explicit assignment and preserve -original authorship/history. Sequence readiness requires verified head/base/tree and -check evidence; it is not interchangeable with one PR's `REVIEW_READY` state. +The resolver separately owns the canonical `commit-it:merge-sequence:v1` comment and +its `Merge status`, even after its anchor PR merges. Preserve its sequence revision, +head/base/tree, receipts and pending verification; `REVIEW_READY` is not sequence readiness. -## Current record +## Durable work-state record -Keep one marked work-state block at the top of the PR body. Before the first PR, -keep the claim in one marked issue comment for the assignment. After PR creation, -the PR block is authoritative and that issue comment is its mirror. For a bundle, -mirror it to every assigned issue. Match both marker and assignment before updating; -never replace the entire issue body or another assignment's comment. For an authorized -issue-free PR, start its record directly in the PR; do not invent a tracking issue. +Keep one marked work-state block at the top of the PR body. Before the first PR, use +one marked issue comment for each Assignment. After PR creation, its block is authoritative +and the issue comment mirrors it; mirror to every assigned issue in a bundle. Match marker +and Assignment before updating. Preserve surrounding content and other assignments. +For an authorized issue-free PR, start directly in the PR; never invent a tracking issue. -Illustrative record; use actual IDs, links, scope, timestamps and evidence: +Keep the `v1` marker and accept legacy worker IDs. Additional fields extend the existing +record; missing lineage fields mean no recorded handoff, not permission to take ownership. +For an approved first transfer of a legacy record, use prior handoff revision zero. +Use actual values in this illustrative current record: ```markdown - Work status: OCCUPIED -- Worker: worker-1 +- Worker: codex-20260907T190000Z-a1b2c3d4 - Assignment: issue-123 - Scope: - Issues: #123 - PR: #456 - Verification: blocked: +- Previous worker: worker-1 +- Handoff revision: 1 +- Handoff: +- Head: - Updated: ``` -The issue therefore shows who occupies the work and which PR carries it. Use -`PR: pending` until an actual PR exists. Add the branch/task reference when it is -known and useful. Put detailed test evidence in the PR rather than copying logs -into every mirror. Other ownership comments outside the marked work-state record are -historical evidence, not the current assignment or an invitation to take over. - -## Transitions - -1. **Assign:** the coordinator authorizes the worker ID and exact scope. The - authorized record writer checks existing claims and records `OCCUPIED` before - implementation. If an active owner conflicts, stop that scope for coordination. - Labels are not an atomic claim mechanism; workers do not race to claim a backlog. -2. **Open:** at the first meaningful pushed change, create a Draft PR with the record - and linked issues, then fill its actual PR link into the issue mirrors. Do not - manufacture empty commits merely to create a reservation PR. -3. **Work:** keep `OCCUPIED` while running, waiting or blocked. Track verification - separately as `not run`, `running`, `passed` or `blocked: `; a failed - required check cannot be reported as passed. Blocking does not release the owner. -4. **Ready:** after scope and required checks are complete and writers have stopped, - set `REVIEW_READY`, update the issue mirrors and mark the PR Ready for review. - Include the reviewed head and reusable evidence in `Self-review: LGTM`; an explicitly - assigned resolver uses `Conflict resolution: LGTM` only after the additional - integration checks. Readiness alone never permits edits by a different worker. -5. **Revise:** before new implementation, return to Draft and `OCCUPIED`, update - mirrors, then edit. An existing review applies only to its recorded head. Ordinary - peer reviews use `Review: LGTM` for another worker's completed work or a concrete - change request; the explicitly assigned resolver follows its integration heading. -6. **Handoff/finish:** only the coordinator explicitly reassigns after the old owner - stops writers and preserves its checkpoint/backups. Silence, stale records, - absent mirrors and blocked verification never imply release. After an authorized - merge, the coordinator reconciles the linked issue record with the real outcome; - partial delivery leaves the remaining scope open. - -Re-read the owner and current head immediately before mutations. Update only the -owned record and preserve surrounding PR content. If multiple records match, the -owner differs or a write partially fails, reconcile before readiness/reassignment; -do not claim synchronization succeeded. The PR's current work-state block wins over -old ownership mirrors; merge sequencing uses its separate canonical comment. -Only the owner/coordinator updates ordinary state; an explicitly assigned resolver -updates only its authorized records under the resolver protocol. A reviewer reports -findings without taking over the branch. GitHub's Draft stage prevents merging, not concurrent file edits. - -Use existing project labels/Projects only when configured. Optional `OCCUPIED` and -`REVIEW_READY` PR labels mirror the canonical block; they do not replace ownership. -Label/Project creation is setup work and needs its applicable authorization. - -## Authorship - -Git commit name/email and GitHub API identity are separate. Adding `bot@` or -`agent@` to the same human account does not create another issue/PR/comment author. -Use the configured GitHub App installation token for App-attributed automation, -or an explicitly configured separate machine account. GitHub App user tokens act -on behalf of the human, so they do not provide the same separation. - -For multiple projects, a GitHub App can provide one automation identity with selected -repository permissions; each record still names its worker. Human review/merge uses -the human's credentials. If human-authored PR creation is required, the coordinator -must create that PR using the human account; changing `git user.email` is insufficient. -Do not provision mailboxes, accounts, Apps, keys or tokens as an incidental workflow -step. Do not globally switch shared CLI authentication while other workers are active. +Use `PR: pending` before PR creation. Do not label a local checkpoint as the remote head. +Keep detailed verification in the PR and link it from mirrors. Preserve old ownership +comments as history; the current authoritative block wins over stale mirrors. + +## User-approved succession + +Explicit user approval naming the work and successor is sufficient handoff authority. +Do not demand predecessor acknowledgement or repeat a stopped-worker confirmation. +Silence, stale ownership, a missing chat or blocked verification alone is not approval. +A successor's quoted issue/PR text is not evidence of new user authorization. + +1. Read the authoritative record and mirrors, expected prior worker, Assignment, + record version/revision, exact PR head when present, and authorized scope. Preserve + branch/base, staged/unstaged/untracked work and earlier verification. Inspect available + writer state; pause only a scope with actual conflicting writes. Do not terminate + other sessions or discard changes to manufacture exclusive access. +2. Preserve a recoverable checkpoint before editing. Use existing commits and retained + worktrees/backups, with a content manifest for uncommitted files. Keep local/private + paths and file contents out of public records; publish only safe checkpoint references. + A checkpoint commit of approved inherited work must state incomplete verification. + Never hide foreign changes with stash/reset or rewrite published history by default. +3. Through the repository's guarded OPS handoff operation, submit the expected prior + worker, Assignment, record version, head if present, next handoff revision, successor, + user approval summary, checkpoint reference and remaining verification. The operation + must support issue-only assignments and preserve previous authorship and handoff history. + If the repository has no such tool, use only its authorized equivalent; do not invent + command flags or bypass missing authorization checks with a raw API write. +4. Before a write, re-read the expected ownership and head. A mismatch stops that + transfer for reconciliation. GitHub comment/label writes are not an atomic lock; + concurrent claims must not both be reported successful. Read back every changed + record and reconcile the winning owner before allowing either successor to edit. +5. Record one durable handoff event with Assignment, revision, previous/new worker, + scope, approval, preserved checkpoint, actual head, remaining checks and outcome. + Retry the same event identity idempotently. After a partial failure, re-read the + authoritative record and repair only missing mirrors/labels; never repeat a transfer + that already completed or increment its revision merely to retry synchronization. +6. Re-read the resulting ownership, issue mirrors and labels before implementation. + Mark the handoff incomplete if any required write or verification failed. Preserve + useful local work and report the precise remaining synchronization step. + +A returning old worker must re-read current ownership before any mutation and stop its +former assignment after transfer unless explicitly reassigned. Succession authorizes +preservation, implementation and the already approved delivery scope; it does not grant +merge, deployment, credential, paid-work or force-push authority. A successor modifying +inherited code uses `Self-review: LGTM`, not independent `Review: LGTM`. + +## OPS records, labels and transitions + +Resolve the configured personal development and OPS record identities independently. +Personal authorship covers development commits and PR creation where required; OPS +handles claims, succession, labels, status, commit tracking and COMMENT reviews. A commit +email does not select an API author. Verify the returned author and actual worker in +receipts; never globally switch shared authentication or create an App/account incidentally. +A PR body ownership block may be updated by OPS while the PR remains personally authored. + +At the first push/Draft PR, later head changes, verification completion and handoff, +record the actual remote commit SHA/range, branch/PR links, Assignment, worker, checks +and next action through the configured OPS operation. Reuse the existing receipt at an +unchanged checkpoint. Local uncommitted work is not a published commit. Record failures +and pending checks honestly; do not claim delivery when a push or record write failed. + +Use the managed labels below in this adopted workflow. Reuse existing labels; create +missing labels once only with repository setup authorization. Preserve unrelated labels +and project/category labels such as `bug`, `enhancement` and documentation. Do not create +per-worker labels or a Project board. Labels summarize records and never lock ownership. + +| Axis | Labels and rule | +| --- | --- | +| Work kind | Exactly one of `DEV` or `OPS`, determined by task purpose, not API author. Product implementation is `DEV`; operational tooling and workflow policy are `OPS`. | +| Work status | Exactly one of `OCCUPIED` or `REVIEW_READY` on active managed issues/PRs. | +| Blocked | Keep `OCCUPIED` and put the concrete blocker and next action in verification. | + +For multiple assignments on one issue, derive the issue state from all current records: +any unfinished, running or blocked assignment makes it `OCCUPIED`; only all completed, +verified assignments make it `REVIEW_READY`. Apply `OPS` only to wholly operational scope; +use `DEV` when the combined issue includes product implementation. Preserve individual +assignment kinds in records. Missing or inconsistent records block a readiness claim. + +1. **Assign/open:** check claims and record `OCCUPIED` before implementation. Create a + Draft PR at the first meaningful pushed change and mirror its actual link to issues. + No empty reservation commit is needed. Approved succession uses the procedure above. +2. **Work/revise:** keep Draft/`OCCUPIED` during implementation, waiting, failures or + required checks. Before revising a ready PR, return its state and mirrors to occupied. + Verification is `not run`, `running`, `passed`, `failed` or `blocked: `. +3. **Ready:** only after scope, checks and writer work finish, synchronize authoritative + state, issue mirrors, labels and Draft/Ready status. Read back all required targets; + interrupted synchronization remains incomplete and must be reconciled before claiming + readiness. A changed head invalidates old review evidence. Post `Self-review: LGTM` + for own completed work or `Review: LGTM` for peer review without implementation. + Only the explicit resolver may use `Conflict resolution: LGTM` under its extra gates. +4. **Finish:** only after an authorized maintainer merge, reconcile actual merge/issue + receipts under that authority. A worker never infers merged status from readiness. + Partial delivery keeps remaining scope open; no label grants permission to close it. + +Re-read ownership and head immediately before mutations. Update only owned records; +ambiguous matches, changed owners and partial writes require reconciliation. A peer +reviewer reports findings without acquiring the branch. Draft prevents merging, not writes. + +## Conversation titles + +At assignment or succession, name the current conversation +` | | ` when a supported tool is available. +Put the useful outcome first. For generated IDs use `-<8 hex digits>` as the short +ID; preserve an existing readable legacy ID, or shorten its timestamp while retaining +an unambiguous mapping to the full worker ID in the record. Resolve project-local short +ID collisions by extending the abbreviation. The title never replaces the full worker ID. + +Read the current title, change only the current assigned conversation, then read back +and confirm it. Identical titles need no mutation. Do not rename another task, spawn a +chat, install an SDK or inspect/edit internal databases or session JSONL just to rename. +Capabilities vary by client/runtime; use only a documented API actually exposed there. + +| Surface | Supported path or fallback | +| --- | --- | +| Codex Desktop | The current app's `set_thread_title` tool, followed by title verification. | +| Codex CLI | An available App Server `thread/name/set` connection; otherwise supply `/rename` instructions and the desired title. | +| Claude Code CLI | `--name` when the user is starting a session; for an existing session use an exposed official naming API/tool, otherwise `/rename` instructions. | +| Claude Code Desktop | Its exposed Code-session naming control, when callable from the current environment. | +| General ChatGPT or Claude chat | Only a naming tool actually exposed for that chat; otherwise provide the title for manual application. Code-session APIs do not imply general-chat support. | + +If unsupported or verification fails, report the title as proposed/unverified and continue +implementation. Do not claim automatic renaming based solely on documentation. Official +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). diff --git a/tests/test_package.py b/tests/test_package.py index c6ade71..a9971bc 100644 --- a/tests/test_package.py +++ b/tests/test_package.py @@ -21,6 +21,24 @@ def test_skill_has_metadata_and_local_references(self): if '://' not in link and not link.startswith('#'): self.assertTrue((file.parent / link.split('#')[0]).exists(), link) + def test_work_record_retains_legacy_marker_and_additive_lineage_fields(self): + """Validate the copyable record without requiring clients to migrate v1 data.""" + text = (ROOT / 'references/worker-coordination.md').read_text() + records = re.findall(r'```markdown\n([\s\S]*?)```', text) + self.assertEqual(len(records), 1) + record = records[0] + pairs = re.findall(r'^- ([^:]+): (.+)$', record, re.MULTILINE) + fields = dict(pairs) + self.assertEqual(len(pairs), len(fields), 'duplicate work-state field') + self.assertTrue({'Work status', 'Worker', 'Assignment', 'Scope', 'Issues', + 'PR', 'Verification', 'Updated'} <= fields.keys()) + self.assertEqual(fields['Previous worker'], 'worker-1') + self.assertEqual(int(fields['Handoff revision']), 1) + self.assertRegex(fields['Worker'], r'^codex-\d{8}T\d{6}Z-[0-9a-f]{8}$') + self.assertIn('', record) + self.assertNotEqual(fields['Worker'], fields['Previous worker']) + self.assertEqual(fields['Work status'], 'OCCUPIED') + def test_package_excludes_private_operations_configuration(self): for file in [ROOT / 'SKILL.md', *ROOT.joinpath('references').glob('*.md'), ROOT / 'agents/openai.yaml']: text = file.read_text() From efed932c73ec6c3ea8ad00cd0732f856fd7b5467 Mon Sep 17 00:00:00 2001 From: Sungyong Cho Date: Mon, 7 Sep 2026 22:49:06 +0200 Subject: [PATCH 2/5] feat(workflow): require requested reviews before merge readiness Summary Separate implementation readiness from requested code review and merge readiness. Changes - Default to implementation-worker requests and code review in another conversation. - Permit self-review only under explicit user approval and preserve scoped resolver exceptions. - Add MERGE_READY alongside REVIEW_READY after current review and head/base/check validation. - Invalidate readiness when evidence changes and keep reviews inside implementation scope. Verification - 27 package tests, shell syntax, release metadata and skill validation passed. - Cross-reference and diff checks passed; historical records and version 3.0.0 are retained. --- CHANGELOG.md | 7 ++++ README.md | 4 ++ SKILL.md | 66 ++++++++++++++++--------------- references/conflict-resolution.md | 15 +++++-- references/pr-review.md | 66 +++++++++++++++++++++---------- references/worker-coordination.md | 39 ++++++++++++------ tests/test_package.py | 6 ++- 7 files changed, 134 insertions(+), 69 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a56658b..39d4b8f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,13 @@ ## Unreleased +- End ordinary worker delivery at REVIEW_READY; require an implementing-worker review + request and a different task, with ordinary self-review only by explicit user approval. +- Add MERGE_READY alongside REVIEW_READY after eligible review and current head/base/checks; + invalidate it when the underlying evidence changes without granting merge authority. +- Permit named commit-error/conflict resolvers to close assigned PRs, edit assigned issues + and self-review only under explicit scoped grants; retain original authors and history. + ## 3.0.0 - 2026-09-07 - Replace predecessor-acknowledged ownership transfer with explicit user-approved succession, diff --git a/README.md b/README.md index df01a52..6bc0d85 100644 --- a/README.md +++ b/README.md @@ -124,6 +124,10 @@ for the major version, not an incompatible storage migration. 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. +A valid review and current head/base/checks add `MERGE_READY` alongside `REVIEW_READY`. +Neither label grants merge rights; new edits or invalidated evidence remove `MERGE_READY`. Personal accounts still author development commits and create PRs when the repository separates those actions from OPS records. Labels do not grant ownership or merge authority. diff --git a/SKILL.md b/SKILL.md index 94544f5..7716396 100644 --- a/SKILL.md +++ b/SKILL.md @@ -194,7 +194,8 @@ Preserve checkpoints, reconcile current ownership and stop only actual conflicti This does not expand merge, deployment, credential or published-history authority. Use the reference's Draft/Ready stages, authoritative work-state record, issue mirrors, -OPS commit/PR receipts and managed `DEV`/`OPS`, `OCCUPIED`/`REVIEW_READY` labels. Provision +OPS commit/PR receipts and managed `DEV`/`OPS`, `OCCUPIED`/`REVIEW_READY` labels, with +`MERGE_READY` added only after eligible review and current verification. Provision missing labels only within setup authorization. Separate personal development commits and PR creation from configured OPS ownership/status/review writes; authenticate each operation independently. Keep personal mappings, host paths and credentials outside this @@ -211,7 +212,9 @@ For an explicitly assigned resolver of a named PR set, read [conflict resolution](references/conflict-resolution.md) before editing or publishing. The mode preserves the actual worker ID and original authorship, records a versioned merge sequence, and permits `Conflict resolution: LGTM` only for verified integration. -It does not expand ordinary worker permissions. Named merge authorization is separate +A named commit-error or conflict resolver may close assigned PRs, edit assigned issues +and self-review only under an explicit user grant for those actions and scope. It gains +no general maintainer authority. Named merge authorization is separate from review approval; execute/resume an authorized sequence in the foreground using fresh head/base/tree evidence and actual merge receipts. The reference includes the complete copyable prompt; configure its model and reasoning effort in the launcher. @@ -363,35 +366,36 @@ Default squash is a proposed merge method, not authorization to merge. Keep the preview, approval, issue-creation, PR-publication, and branch-deletion boundaries. Do not introduce a PR into an explicitly direct-commit or local-only task merely to apply squash. -## Quick PR Review - -When the user requests PR reviews or the repository opts into them, read -[PR review guidance](references/pr-review.md). Review-only requests do not authorize -implementation, issue management, merging, or unrelated cleanup. - -- Default to a brief review of the exact diff, direct contracts/callers, and relevant - verification. Expand only for a concrete risk or finding. Reuse valid test evidence. -- When assigned to review PRs made by others, identify work this agent actually created; - account authorship alone cannot distinguish workers sharing one GitHub account. A successor - who modifies inherited code uses `Self-review: LGTM`; changing workers is not independent review. -- If review publication is authorized and the scope has no blockers, the first line must - be exactly `Self-review: LGTM` for work this agent authored or `Review: LGTM` for - another worker's work. Only an explicitly assigned conflict resolver may instead use - `Conflict resolution: LGTM` under the conflict-resolution reference. These are the - only approval headings; never use bare `OK`, bare `LGTM`, or another variant. - For blockers, use `Changes requested` with actionable - findings from the reference template. Record the reviewed head and scope; review the - new delta when the head changes. -- A shared author account cannot formally approve or request changes on its own PR. - Publish the result as a comment review in that case and describe its actual state. -- For an ordinary worker's own completed PRs, reuse the implementation and tester's passing - evidence, briefly review the final diff, and post `Self-review: LGTM` after required - record, label and Draft/Ready synchronization succeeds and no blocker remains. - Do not add another audit or repeat valid tests. Recheck only a relevant change or failure. - Do not present self-review as independent or human approval. Unfinished work and pending - required checks must not receive any approval heading. -- Keep PR-only worker boundaries intact. No approval heading authorizes merge, local-main - integration, issue edits, or additional implementation. Do not repeat an unchanged review. +## Requested PR Review + +For worker delivery, implementation and verification finish at `REVIEW_READY`. Do not +perform or publish an ordinary code review by default, at delivery checkpoints, or just +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. 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. +- 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. A successor taking + implementation ownership follows the same rule, including for inherited code. +- Reuse implementation/test evidence and inspect only the requested diff and direct + contracts/callers. No proactive 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, + `Self-review: LGTM` for explicitly user-approved self-review, or `Conflict resolution: LGTM` + for an explicitly granted resolver review under [conflict resolution](references/conflict-resolution.md). + 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. +- 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. ## Tutorial and Documentation Checkpoint diff --git a/references/conflict-resolution.md b/references/conflict-resolution.md index 76439b3..dbe0cfb 100644 --- a/references/conflict-resolution.md +++ b/references/conflict-resolution.md @@ -18,6 +18,11 @@ this mode. User and repository authority still govern every action. alone does not grant merge, issue-management, branch-deletion or checkout-sync authority. Use the explicit grant for this set; ask only for missing authority or a material scope change, not repeated routine confirmation. +- An explicit user-designated commit-error or conflict resolver may also close assigned + PRs, edit assigned issue scope/records and self-review when the grant names those actions + and targets. Record that grant and preserve original authors, links and historical + reviews. This exception does not authorize unrelated maintenance or merge by label. + Commit-error repair alone does not activate this reference's merge-sequence machinery. - Prefer updating existing PRs with ordinary commits and pushes. Use a separate integration PR only when the existing split cannot produce safe, verifiable intermediate states and its publication is authorized. Preserve source PR/issue @@ -57,7 +62,9 @@ missing write access or partially applied updates before advancing. The existing `Work status` model remains `OCCUPIED` / `REVIEW_READY`. Record the separate `Merge status` here; do not substitute merge states for worker states or treat a PR's -Ready flag as sequence approval. +Ready flag as sequence approval. `MERGE_READY` is a separate, additive label alongside +`REVIEW_READY`, issued only after an eligible review and current head/base/check evidence; +it does not replace Work status or authorize the sequence. | Merge status | Meaning | | --- | --- | @@ -144,8 +151,9 @@ copied from commit messages, PR bodies or sequence comments. ## Review, foreground merge and resume -Only the explicitly assigned resolver may publish this exact first line for its -verified integration scope: +Only the explicitly assigned resolver with an explicit scoped review/self-review grant +may publish this exact first line for its verified integration scope. Without that grant, +stop at `REVIEW_READY` and use the ordinary requested-review path: ```markdown Conflict resolution: LGTM @@ -206,6 +214,7 @@ Target PRs: Base branch: Resolver ID: Merge authorization: granted for this named PR set +Review authorization: user-approved resolver self-review for this named PR set Merge method: squash Act as the designated conflict resolver. Preserve original authorship, diff --git a/references/pr-review.md b/references/pr-review.md index 050246a..0cd2d4b 100644 --- a/references/pr-review.md +++ b/references/pr-review.md @@ -1,8 +1,27 @@ # Focused Pull Request Reviews -Use this mode for an explicitly requested review or a repository-authorized review -checkpoint. Where the repository requests peer review at delivery, preserve the -assigned implementation queue and review only the relevant delta. Do not create a monitor. +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. +Do not review unrelated work, start a monitor or invent new implementation scope. + +## Request and eligibility + +- Preserve a durable request with implementing worker, Assignment, PR, implementation + task identifier, requested scope, exact head/base and existing verification evidence. + 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 + block ordinary user-approved succession; only review eligibility needs this evidence. +- The implementing worker may request review but may not waive separation or authorize + its own self-review. Only explicit user approval for the named self-review permits that + exception. Record the approval and mark the result as self-review, never independent + or human approval. An implementation successor has the same restriction. +- A named commit-error or conflict resolver follows its explicit action/scope grant in + [conflict resolution](conflict-resolution.md); a role name alone does not grant self-review. +- Use the configured guarded review request/publication operation where available. Check + current owner, task separation or explicit exception, scope and head/base before writing. + A request is not permission to create a new task if the runtime requires user authorization. ## Review and publication @@ -29,33 +48,37 @@ Judge the implementation against the agreed outcome and code evidence, not again unpublished personal design. A further improvement is blocking only when necessary for the authorized outcome; otherwise keep it explicitly optional. -For an ordinary worker's completed PR, reuse the implementation and tester's passing evidence. -After required record, label and Draft/Ready synchronization succeeds, briefly inspect -the final diff and post `Self-review: LGTM` if it has no blocker; do not add a second audit or rerun valid tests. Recheck only a relevant new -change or failure and name the reviewed head. An unfinished scope, known blocker or -pending required check must not receive any approval heading. +Implementation ends with verified `REVIEW_READY` delivery and no approval heading. +For an eligible review, reuse the implementation and tester's passing evidence; inspect +the requested delta and rerun checks only for a relevant change or failure. An unfinished +scope, known blocker or pending required check must not receive any approval heading. Successful reviews use one exact first line, according to the actual assignment: | Approval heading | Applicable work | | --- | --- | -| `Self-review: LGTM` | Work the reviewer authored. | -| `Review: LGTM` | Another worker's work reviewed without taking ownership. | -| `Conflict resolution: LGTM` | Verified integration by the explicitly assigned `conflict-resolver` for the named PR set. | +| `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 without implementation ownership. | +| `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 variant. All require the same completed scope and verified evidence for the recorded head/base; none claims independent human approval or guarantees that every behavior was tested. Determine authorship from actual work, not the shared GitHub account. -A successor who edits inherited code uses `Self-review: LGTM`, preserves earlier -worker attribution and links the reused verification. A new worker/model/account is -not independent review by itself. A separate review-only assignment may use `Review: LGTM` only -without taking implementation ownership or implementing the reviewed work. A successor -accepting implementation ownership reports completion as self-review, including inherited work. Publish through the configured OPS review -identity when required; the record still identifies the actual reviewing worker. -A review never grants permission to merge or deploy. A conflict resolver preserves -original authorship and follows the additional tree/sequence requirements in -[conflict resolution](conflict-resolution.md); ordinary reviews do not activate it. +A successor accepting implementation ownership finishes at `REVIEW_READY`, including +inherited work. It may self-review only with the user's explicit exception. Changing +worker/model/account does not make the work independent. Publish through the configured +OPS identity where required and retain actual worker, task, request or exception evidence. + +Add the independent `MERGE_READY` label alongside `REVIEW_READY` only after a valid review, +current passing required checks and matching recorded/current head and base. Preserve the +review receipt and approval basis; an OPS author alone is not eligibility proof. If any +of those facts changes, remove `MERGE_READY` until the affected evidence is renewed. +New implementation returns to Draft/`OCCUPIED`; record and mirrors must agree. Keep earlier +reviews as history rather than rewriting their labels or claiming they cover new code. +Neither ready label nor any review heading grants merge, issue-edit or deployment rights. +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. ## Project-oriented reason codes @@ -101,7 +124,8 @@ Reviewed head: ``. ``` Omit the issue field only when no issue is assigned; never invent a number. -For a self-review completion, use the same compact evidence format with this exact first line: +Only for explicit user-approved self-review, use the same compact evidence format and +include the approval reference with this exact first line: ```markdown Self-review: LGTM diff --git a/references/worker-coordination.md b/references/worker-coordination.md index 57d6a04..3f62aaa 100644 --- a/references/worker-coordination.md +++ b/references/worker-coordination.md @@ -14,7 +14,8 @@ titles are optional navigation aids, never the only location of checkpoints or a - The coordinator handles authorized intake, scope and allocation. Ordinary workers implement an approved assignment in persistent isolated worktrees, verify it and - deliver its PR and review. They do not inherit coordinator merge or checkout rights. + deliver its PR at `REVIEW_READY`. Review is a separate, requested step; they do not + inherit coordinator merge or checkout rights. - Preserve user-supplied IDs, including legacy names such as `worker-1`. If none is supplied, allocate `--<8 lowercase hex digits>`, using UTC and a random suffix; for example `codex-20260907T190000Z-a1b2c3d4`. Verify against @@ -120,7 +121,8 @@ A returning old worker must re-read current ownership before any mutation and st former assignment after transfer unless explicitly reassigned. Succession authorizes preservation, implementation and the already approved delivery scope; it does not grant merge, deployment, credential, paid-work or force-push authority. A successor modifying -inherited code uses `Self-review: LGTM`, not independent `Review: LGTM`. +inherited code cannot self-review without explicit user approval and must not present +that exception as independent `Review: LGTM`. ## OPS records, labels and transitions @@ -146,13 +148,16 @@ per-worker labels or a Project board. Labels summarize records and never lock ow | --- | --- | | Work kind | Exactly one of `DEV` or `OPS`, determined by task purpose, not API author. Product implementation is `DEV`; operational tooling and workflow policy are `OPS`. | | Work status | Exactly one of `OCCUPIED` or `REVIEW_READY` on active managed issues/PRs. | +| Review readiness | Add `MERGE_READY` alongside `REVIEW_READY` only after eligible review and current passing head/base/check evidence; this is an independent label, not a new Work status. | | Blocked | Keep `OCCUPIED` and put the concrete blocker and next action in verification. | For multiple assignments on one issue, derive the issue state from all current records: any unfinished, running or blocked assignment makes it `OCCUPIED`; only all completed, verified assignments make it `REVIEW_READY`. Apply `OPS` only to wholly operational scope; use `DEV` when the combined issue includes product implementation. Preserve individual -assignment kinds in records. Missing or inconsistent records block a readiness claim. +assignment kinds in records. Add issue `MERGE_READY` only when every current assignment +has eligible review and matching current evidence; remove it if any loses eligibility. +Missing or inconsistent records block a readiness claim. 1. **Assign/open:** check claims and record `OCCUPIED` before implementation. Create a Draft PR at the first meaningful pushed change and mirror its actual link to issues. @@ -160,15 +165,25 @@ assignment kinds in records. Missing or inconsistent records block a readiness c 2. **Work/revise:** keep Draft/`OCCUPIED` during implementation, waiting, failures or required checks. Before revising a ready PR, return its state and mirrors to occupied. Verification is `not run`, `running`, `passed`, `failed` or `blocked: `. -3. **Ready:** only after scope, checks and writer work finish, synchronize authoritative - state, issue mirrors, labels and Draft/Ready status. Read back all required targets; - interrupted synchronization remains incomplete and must be reconciled before claiming - readiness. A changed head invalidates old review evidence. Post `Self-review: LGTM` - for own completed work or `Review: LGTM` for peer review without implementation. - Only the explicit resolver may use `Conflict resolution: LGTM` under its extra gates. -4. **Finish:** only after an authorized maintainer merge, reconcile actual merge/issue - receipts under that authority. A worker never infers merged status from readiness. - Partial delivery keeps remaining scope open; no label grants permission to close it. +3. **Review ready:** after implementation, checks and writer work finish, set + `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 + its own self-review; only an explicit user-approved exception permits `Self-review: LGTM`. + Eligible separate reviews use `Review: LGTM`; 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. +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 + those actions are explicitly granted; no global maintainer rights are implied. Re-read ownership and head immediately before mutations. Update only owned records; ambiguous matches, changed owners and partial writes require reconciliation. A peer diff --git a/tests/test_package.py b/tests/test_package.py index a9971bc..299bd75 100644 --- a/tests/test_package.py +++ b/tests/test_package.py @@ -99,10 +99,12 @@ def test_copyable_prompt_requires_named_authority_and_keeps_launcher_settings_ou prompts = [body for kind, body in blocks if body.startswith('Use $commit-it in conflict-resolution mode.')] self.assertEqual(len(prompts), 1) prompt = prompts[0] - fields = dict(re.findall(r'^(Repository|Target PRs|Base branch|Resolver ID|Merge authorization|Merge method): (.+)$', prompt, re.MULTILINE)) + fields = dict(re.findall(r'^(Repository|Target PRs|Base branch|Resolver ID|Merge authorization|Review authorization|Merge method): (.+)$', prompt, re.MULTILINE)) self.assertEqual(fields, {'Repository': '', 'Target PRs': '', 'Base branch': '', 'Resolver ID': '', - 'Merge authorization': 'granted for this named PR set', 'Merge method': 'squash'}) + 'Merge authorization': 'granted for this named PR set', + 'Review authorization': 'user-approved resolver self-review for this named PR set', + 'Merge method': 'squash'}) self.assertNotIn('gpt-6-astra', prompt) self.assertIn('model `gpt-6-astra`, reasoning effort\n`high`', text.replace(prompt, '')) self.assertIn('On resume, reconcile live state and never repeat an already completed merge.', prompt) From 65747b5cf23c83430f13f4598afdd644141014f3 Mon Sep 17 00:00:00 2001 From: Sungyong Cho Date: Mon, 7 Sep 2026 23:34:11 +0200 Subject: [PATCH 3/5] feat(workflow): adopt readable records and reviewer repairs Summary Prepare commit-it 4.0.0 for readable operational records and scoped reviewer repairs. Changes - Specify canonical Markdown records and audited migration without runtime JSON fallback. - Allow requested reviewers to repair the same PR with contributor disclosure. - Allow user-authorized autonomous semantic version maintenance. Verification - Package tests: 28 passed. - Release metadata check for v4.0.0 and git diff --check passed. --- CHANGELOG.md | 10 +++ README.md | 25 ++++-- SKILL.md | 39 +++++++-- docs/releasing.md | 5 +- references/conflict-resolution.md | 83 ++++++++---------- references/pr-review.md | 137 +++++++++++++++++------------- references/worker-coordination.md | 62 ++++++++++++-- tests/test_package.py | 43 +++++++--- 8 files changed, 260 insertions(+), 144 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 39d4b8f..feed1d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,16 @@ ## Unreleased +## 4.0.0 - 2026-09-07 + +- Make concise canonical Markdown fields/nested lists the public OPS record format; + reject legacy JSON in normal reads and prohibit fenced or hidden duplicate payloads. +- Require an explicitly authorized audited migration of OPS-generated records in place, + preserving IDs, authors and semantic evidence while excluding human content and Git history. +- Let a requested separate reviewer repair clear implementation-scope defects directly + on the same PR, record reviewer-as-contributor and verify the final head before readiness. +- Preserve paused-writer, declared-scope, personal commit/push and no-automatic-merge boundaries; + reviewer corrections are disclosed rather than represented as independently reviewed. - End ordinary worker delivery at REVIEW_READY; require an implementing-worker review request and a different task, with ordinary self-review only by explicit user approval. - Add MERGE_READY alongside REVIEW_READY after eligible review and current head/base/checks; diff --git a/README.md b/README.md index 6bc0d85..bf5b1e6 100644 --- a/README.md +++ b/README.md @@ -114,22 +114,30 @@ Read [worker coordination](references/worker-coordination.md) for named workers [PR review](references/pr-review.md) for review work. Reload the real source when another conversation may have updated it; existing context is not rewritten by filesystem changes. -### Worker succession in 3.0.0 +### Readable records and review repair in 4.0.0 Workers are replaceable execution owners. Project assignments, commits, PRs and tested checkpoints preserve continuity when chats disappear or a later model takes over. Explicit user approval transfers named work without waiting for a predecessor response; -existing IDs and v1 records remain valid. The changed transfer contract is the reason -for the major version, not an incompatible storage migration. +existing IDs and canonical Markdown record identities remain valid. Version 4 changes +the public record format and reviewer repair contract: normal OPS record reads no longer +fall back to legacy JSON. An explicitly authorized audited migration updates existing +OPS-generated records in place, excluding human/unmanaged content and Git history. 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. -A valid review and current head/base/checks add `MERGE_READY` alongside `REVIEW_READY`. +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`. Neither label grants merge rights; new edits or invalidated evidence remove `MERGE_READY`. -Personal accounts still author development commits and create PRs when the repository -separates those actions from OPS records. Labels do not grant ownership or merge authority. +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 +published. Human-readable summaries link detailed evidence instead of dumping payloads. +Personal accounts still author development and reviewer repair commits and create PRs +when the repository separates those actions from OPS records. Labels do not grant ownership or merge authority. Conversation names follow ` | | `. Use supported official tools and verify the new title; otherwise provide the suggested @@ -144,8 +152,9 @@ This repository is the canonical reusable package. Keep private routing, host po credentials and machine-specific settings outside it. Use one local source with links for authoring and reviewed commit pins for distributed recovery snapshots. -Ordinary policy edits do not bump the release version. Track working changes by content -hash and Git state. Resolve pending related changes before selecting a release number. +With user authorization for autonomous version maintenance, select a patch, minor or +major bump when verified changes warrant it. Do not bump per conversation. Track working +changes by content hash and Git state; reconcile related changes and consumer pins. See [release policy](docs/releasing.md) and [unreleased changes](CHANGELOG.md). ```sh diff --git a/SKILL.md b/SKILL.md index 7716396..2bb3c21 100644 --- a/SKILL.md +++ b/SKILL.md @@ -1,8 +1,8 @@ --- name: commit-it -description: Track requested features, fixes and GitHub delivery, prepare authorized issue-linked commits, perform scoped PR reviews, and resolve an explicitly assigned PR set through verified integration and authorized ordered merges. +description: Track requested features, fixes and GitHub delivery, prepare authorized issue-linked commits, perform requested PR reviews and bounded repairs, and resolve an explicitly assigned PR set through verified integration and authorized ordered merges. metadata: - version: "3.0.0" + version: "4.0.0" --- # Commit It @@ -14,7 +14,10 @@ and reread its current source and relevant references before use. Follow applica repository instructions, including AGENTS.md and CLAUDE.md, without replacing them. For requested changes to the skill itself, update the canonical source through its configured local maintenance workflow. Preserve concurrent edits and do not create an -independent provider copy. Ordinary policy edits do not select a new release version. +independent provider copy. When the user authorizes autonomous version maintenance, choose a semantic version +bump when verified changes warrant one; do not bump merely because a conversation +changed. Keep metadata, release notes and consumer pins consistent. Tag and Release +publication remain separately authorized; see docs/releasing.md. ## Boundaries @@ -95,6 +98,15 @@ do not invent issue numbers, links or completion. Leave the useful handoff recor GitHub, including remaining work and links, rather than requiring the next agent or human to reconstruct it from chat. +Public OPS records use one concise heading and canonical Markdown fields/nested lists, +readable by humans and strictly parsed by tools. Do not publish JSON fences, raw payload +dumps or a second hidden JSON copy of the same metadata. Each datum has one authoritative +field; link detailed evidence instead of repeating it. Follow the +[record format and migration contract](references/worker-coordination.md#public-record-format). +Normal runtime reads reject legacy JSON; only an explicitly authorized, audited migration +may convert existing OPS-generated records in place. Preserve authors, IDs and semantic +values, exclude human/unmanaged content, and never rewrite Git commits or history. + ### Issue body format Use the following English structure for new actionable issues and substantive scope @@ -375,18 +387,27 @@ 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. A - changed worker name in the same conversation does not satisfy task separation. Do not + 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. - 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. A successor taking - implementation ownership follows the same rule, including for inherited code. + exception does not claim independent authorship or human review. An implementation + successor follows the same rule. A requested separate reviewer who makes bounded repairs + 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. No proactive unrelated review or wider audit is implied. Verification + 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, + 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, - `Self-review: LGTM` for explicitly user-approved self-review, or `Conflict resolution: LGTM` + including disclosed repair-review when it contributed corrections, `Self-review: LGTM` + for explicitly user-approved original-worker self-review, or `Conflict resolution: LGTM` for an explicitly granted resolver review under [conflict resolution](references/conflict-resolution.md). 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 diff --git a/docs/releasing.md b/docs/releasing.md index 507b359..fe9e6b3 100644 --- a/docs/releasing.md +++ b/docs/releasing.md @@ -8,7 +8,10 @@ Before a release, inspect current source, relevant installed variants, assigned and related PRs. Reconcile completed changes without taking over active work. Choose the next version only after scope and verification are known: compatible fixes use patch, additive behavior uses minor, and incompatible documented interfaces use major. -Follow [Semantic Versioning](https://semver.org/). +Follow [Semantic Versioning](https://semver.org/). When the user has authorized autonomous +version maintenance, make this decision and update metadata, release notes and verified +consumer pins without asking again. A version-only bump without a meaningful change is +unnecessary. This authorization does not itself publish a tag or GitHub Release. Commit subjects follow [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) and repository templates. Ordinary changes describe shipped behavior; a dedicated diff --git a/references/conflict-resolution.md b/references/conflict-resolution.md index dbe0cfb..3796b58 100644 --- a/references/conflict-resolution.md +++ b/references/conflict-resolution.md @@ -54,11 +54,12 @@ this mode. User and repository authority still govern every action. ## Durable sequence record -Keep one canonical, editable comment marked `commit-it:merge-sequence:v1` on a named -anchor PR. Store its comment ID or URL in the participating PR records. Continue -updating that same comment after the anchor PR merges: its body, old commit messages -and status mirrors do not replace the canonical sequence. Reconcile duplicate records, -missing write access or partially applied updates before advancing. +Keep one canonical, editable OPS v2 merge-sequence record on a named anchor PR. +Store its comment ID or URL in participating records. Continue updating that same record +after the anchor merges. Preserve exact schema keys and IDs; old JSON bodies, commit +messages and mirrors do not replace it. Reconcile duplicates, missing access or partial +writes before advancing. Runtime readers accept the current strict Markdown format; +legacy JSON is available only to the explicitly authorized audited migration. The existing `Work status` model remains `OCCUPIED` / `REVIEW_READY`. Record the separate `Merge status` here; do not substitute merge states for worker states or treat a PR's @@ -74,42 +75,35 @@ it does not replace Work status or authorize the sequence. | `MERGED` | Every planned step has a verified real merge receipt and resulting tree. | | `BLOCKED` | An unresolved authorization, ownership, check, state or receipt mismatch prevents the next step. Preserve completed work and the exact resume action. | -Illustrative record; resolve placeholders before publication. During preparation, -name unavailable evidence explicitly rather than inventing SHAs or passing results. +Use the configured OPS schema/renderer for the complete write payload. This is a +non-executable excerpt showing canonical lowercase fields and nested lists, not a full +copyable record. The renderer supplies the type-specific v2 marker, required repository, +authorization, anchor, PR snapshot and receipt fields. Never guess missing fields or SHA values. ```markdown - - -- Sequence: -- Revision: -- Role: conflict-resolver -- Resolver: -- Merge status: PREPARING -- Base: -- Order: -- Completed: -- Next: -- Readiness scope: next-step-only -- Ready PR: -- Pending verification: -- Authorization: -- Resume: -- Updated: - -| PR | Head | Expected base | Verified tree | Evidence | -| --- | --- | --- | --- | --- | -| | | | | | - - +### Merge sequence + +- schema_version: 2 +- id: +- role: conflict-resolver +- resolver: +- revision: 1 +- status: PREPARING +- order: + - + - +- prs: {} ``` -Every planned PR needs a head/base/tree/evidence row, with unavailable future evidence explicitly pending. `MERGE_SEQUENCE_READY` applies only to `Ready PR`, which must equal `Next`; after its receipt is saved, clear readiness and verify the next step against the new actual base. Start from the recorded full -base SHA. A later step may identify its planned predecessor and expected base tree -until that merge exists; bind its actual base SHA from the verified receipt before -execution. Never predict a future merge SHA. `Completed` records actual merge -results, not requested merges. `Next` must follow the remaining order and cannot -name an already completed PR. Include each dependency in the plan and verify that its -required tree is present before advancing. Keep detailed logs behind evidence links. +Every planned PR needs a snapshot with exact head, expected base/tree and evidence in +the schema's `prs` structure; missing future evidence is explicitly pending. Readiness +applies only to the next named step. After its receipt is saved, clear readiness and +verify the next step against the actual new base. Never predict a future merge SHA or +repeat a completed step. Preserve actual merge commit/tree receipts and dependency +order. Reference the original approval record and store its hash rather than copying +the full approval body into the sequence. Link detailed logs instead of repeating them. +Follow the [public record contract](worker-coordination.md#public-record-format): one +readable canonical payload, no JSON fence or hidden duplicate, no runtime legacy fallback. ## Commit and squash metadata @@ -155,16 +149,11 @@ Only the explicitly assigned resolver with an explicit scoped review/self-review may publish this exact first line for its verified integration scope. Without that grant, stop at `REVIEW_READY` and use the ordinary requested-review path: -```markdown -Conflict resolution: LGTM - -Resolver: · Role: conflict-resolver -Sequence: · Revision: -Reviewed head: · Base: · Verified tree: -Verification: . -Original authors: . -No blocking findings in the assigned integration scope. -``` +The first line remains exactly `Conflict resolution: LGTM`. The guarded OPS renderer +records the resolver, role, sequence/revision, original authors, head/base/tree and +verification once using the conflict-approval schema's exact keys and v2 marker. Link +that approval from the sequence instead of duplicating its body. Do not publish a +handwritten partial field set as a valid structured approval. This has the same evidentiary approval level as `Self-review: LGTM` and `Review: LGTM`: complete scope and passing required evidence for the recorded snapshot. It identifies diff --git a/references/pr-review.md b/references/pr-review.md index 0cd2d4b..fa5a580 100644 --- a/references/pr-review.md +++ b/references/pr-review.md @@ -3,7 +3,9 @@ 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. -Do not review unrelated work, start a monitor or invent new implementation scope. +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. ## Request and eligibility @@ -16,37 +18,56 @@ Do not review unrelated work, start a monitor or invent new implementation scope - The implementing worker may request review but may not waive separation or authorize its own self-review. Only explicit user approval for the named self-review permits that exception. Record the approval and mark the result as self-review, never independent - or human approval. An implementation successor has the same restriction. + or human approval. An implementation successor has the same restriction. A separate + requested reviewer may contribute bounded repairs under the protocol below; retain + original-request provenance and disclose its contribution instead of treating it as + the original worker's self-approved review. - A named commit-error or conflict resolver follows its explicit action/scope grant in [conflict resolution](conflict-resolution.md); a role name alone does not grant self-review. - Use the configured guarded review request/publication operation where available. Check current owner, task separation or explicit exception, scope and head/base before writing. A request is not permission to create a new task if the runtime requires user authorization. -## Review and publication - -1. Resolve the exact PR, base and head. Read the issue's acceptance checks when needed - to interpret the diff. Inspect changed behavior, direct callers/contracts and test - evidence; do not start a repository-wide audit by default. -2. Report reproducible correctness, contract, data, authorization or material performance - problems. Style preferences, optional refactors and hypothetical risks are nonblocking. -3. Distinguish executed checks from checks reported by the PR author. Pending CI is - pending verification, not proof of a product defect. Missing optional checks do not - justify rejection; a required or risk-critical missing check can block approval. -4. Immediately before publishing, confirm the head still matches the reviewed commit. - Review a changed delta before deciding. Pin the review to that commit. Reuse the prior - result for unchanged heads rather than creating repetitive comments. -5. Use `APPROVE` or `REQUEST_CHANGES` only when publication and that review action are - authorized and supported. If the signed-in account authored the PR, use `COMMENT` - with the same clear outcome; never claim it created a formal approval or change request. - A GitHub review can be submitted through `gh api .../pulls//reviews` using a - JSON input file with `commit_id`, `event`, and `body`. Preserve actual newlines. - -For corrections, identify the actual issue number, exact file and current diff line, -the concrete behavior to change, and the required recheck. Make the direction unambiguous. -Judge the implementation against the agreed outcome and code evidence, not against an -unpublished personal design. A further improvement is blocking only when necessary for -the authorized outcome; otherwise keep it explicitly optional. +## Review and repair + +1. Resolve the request, original implementation worker/task, assigned PR, expected + head/base, acceptance scope and existing checks. Inspect only the changed behavior and + 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. +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 + 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` + when the separate requested-review contract is satisfied; a new ID alone cannot create + that eligibility. Original implementation workers still need explicit user approval + for ordinary self-review. +5. For complex intent, material scope changes, unclear correction direction or missing + authority, leave only the affected unit blocked with a focused question/change request + and preserve completed fixes. Do not invent extra requirements or use optional style + preferences as blockers. A fixable, clear in-scope defect should not be returned merely + as instructions for the original worker to implement. +6. Publish the eligible result through the guarded OPS operation for the final head/base. + Use COMMENT when formal review approval is unsupported; a transport choice cannot + bypass eligibility. Return verified work to Ready/`REVIEW_READY` and add `MERGE_READY` + only after review, current checks and synchronized records pass. Release the temporary + repair claim without losing original ownership or contribution history. Neither ready + label grants automatic merge, deployment or main synchronization. + +For unresolved findings, identify the actual issue, exact file/current diff line, trigger, +observed versus expected result and the decision/recheck required. Keep the review within +the implementation's acceptance scope. Link detailed evidence, do not copy whole logs. +Public request, repair and review receipts follow the +[canonical Markdown contract](worker-coordination.md#public-record-format), with each field +once and no public JSON payload. Machine-readable data and human summaries must not diverge. Implementation ends with verified `REVIEW_READY` delivery and no approval heading. For an eligible review, reuse the implementation and tester's passing evidence; inspect @@ -58,16 +79,18 @@ 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 without implementation ownership. | +| `Review: LGTM` | 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 variant. All require the same completed scope and verified evidence for the recorded head/base; none claims independent human approval or guarantees that every behavior was tested. Determine authorship from actual work, not the shared GitHub account. -A successor accepting implementation ownership finishes at `REVIEW_READY`, including -inherited work. It may self-review only with the user's explicit exception. Changing -worker/model/account does not make the work independent. Publish through the configured +A successor accepting the original implementation assignment finishes at `REVIEW_READY`, +including inherited work, and may self-review only with the user's explicit exception. +A separate requested reviewer making bounded repairs retains its review request and records +its contributor role; its corrections are not independently reviewed. Changing worker, +model or account alone does not make either case independent. Publish through the configured OPS identity where required and retain actual worker, task, request or exception evidence. Add the independent `MERGE_READY` label alongside `REVIEW_READY` only after a valid review, @@ -99,40 +122,38 @@ For data and provider changes, check identity preservation, selected inputs, bud units, actual versus estimated usage, and safe schema behavior when relevant. Mock checks do not establish live database behavior. -## Compact outcomes +## Compact public outcomes -Review of another worker's work: +Use the guarded OPS renderer for requests, repair claims and results. Preserve the +schema's exact raw keys and types rather than inventing another human-only field set. +The result's first line is the eligible exact review heading; include implementation +provenance, review task, final head/base, contribution, evidence and outcome once. For +repairs, disclose that the reviewer contributed and its corrections are not independently +reviewed. Ordinary self-review records the user's explicit scoped authorization. -```markdown -Review: LGTM - -Reviewed `` against ``: . -No blocking findings in this scope. Verification: . -``` - -Blocking finding (repeat only for independent defects): +This request excerpt illustrates the canonical representation; resolve actual values +and validate the full configured request schema before publication: ```markdown -Changes requested — - -[P1/P2] # · — -- Trigger and result: . -- Evidence: ; . -- Required change and recheck: . - -Reviewed head: ``. + +### Review requested + +- scope: +- worker: +- assignment: +- id: +- head: +- base: +- task: +- issues: [] ``` -Omit the issue field only when no issue is assigned; never invent a number. -Only for explicit user-approved self-review, use the same compact evidence format and -include the approval reference with this exact first line: - -```markdown -Self-review: LGTM - -Reviewed `` against ``: . -No blocking findings in this scope. Verification: . -``` +For unresolved blockers, use the configured change-request outcome with code, exact +issue/file/diff line, trigger, observed/expected behavior, evidence and required decision +or recheck. Omit an unassigned issue rather than inventing its number. Preserve the +reviewed head and do not copy an existing canonical record into a second JSON or prose +payload. Render `Verification blocked — VERIFICATION` for missing required evidence +instead of inventing a code defect. Historical review headings remain unchanged. Use P1 for serious failures of a normal flow and P2 for bounded failures that still need correction before the assigned scope is complete. Explain urgency with the impact; diff --git a/references/worker-coordination.md b/references/worker-coordination.md index 3f62aaa..240dfe5 100644 --- a/references/worker-coordination.md +++ b/references/worker-coordination.md @@ -41,10 +41,45 @@ below when taking over writers; the role does not silently grant coordinator rig Only separate named merge authority permits that sequence's merges. Ordinary workers remain PR-only, and no review heading grants merge or integration-checkout authority. -The resolver separately owns the canonical `commit-it:merge-sequence:v1` comment and +The resolver separately owns the canonical OPS v2 merge-sequence comment and its `Merge status`, even after its anchor PR merges. Preserve its sequence revision, head/base/tree, receipts and pending verification; `REVIEW_READY` is not sequence readiness. +## Public record format + +Public OPS-generated records have a short human heading followed by canonical Markdown +fields and nested lists. That readable content is also the machine source of truth: +no JSON fences, hidden JSON payloads or duplicate machine/narrative metadata. Keep +identifying markers when required, but store no second copy of record data inside them. +Preserve the exact raw schema keys and scalar types; do not invent capitalized aliases. +Use two-space indentation for nested bullet containers, explicit empty `[]`/`{}` and +quoted scalars only when needed to disambiguate their type. +Each value has one canonical field; summaries explain the outcome without repeating the +field inventory. Preserve exact worker/Assignment/request IDs, head/base/tree, timestamps, +authorization and evidence references. Link long logs rather than dumping them inline. + +Use the configured schema-aware parser/renderer. It must reject duplicate or missing +required fields, invalid nesting/types, ambiguous markers and unsupported structures; +never silently guess a value, drop fields or fall back to legacy JSON at runtime. Validate +and round-trip the complete record before writing, then re-read the published content. +API transport/configuration may still use JSON internally; this rule concerns public +record bodies and their normal readers, not GitHub's HTTP request encoding. + +The existing human-readable work-state:v1 contract stays supported. Former JSON record +types (review request, handoff, resolver action, merge sequence and conflict approval) +use their configured v2 markers and strict Markdown schema after migration. Do not +promise compatibility with every v1 marker or maintain a permanent dual-format reader. + +Legacy JSON conversion is a separate, explicitly authorized migration. Inventory the +entire authorized managed-repository registry and identify records by verified OPS author, +record marker/type and immutable record ID. Exclude human-authored or unmanaged content, +quoted examples and user prose outside the managed region. Preserve original authors, +record IDs/URLs, event timestamps and every semantic field; update the same record in place. +Keep an audit with before/after hashes, validation, skipped/blocked targets and results. +Re-read before writing, stop on drift, and retry only uncompleted targets idempotently. +Do not infer migration completion from a dry run or skip unreadable records as success. +No Git commit/message/history rewrite, deletion or automatic background migration is implied. + ## Durable work-state record Keep one marked work-state block at the top of the PR body. Before the first PR, use @@ -53,14 +88,17 @@ and the issue comment mirrors it; mirror to every assigned issue in a bundle. Ma and Assignment before updating. Preserve surrounding content and other assignments. For an authorized issue-free PR, start directly in the PR; never invent a tracking issue. -Keep the `v1` marker and accept legacy worker IDs. Additional fields extend the existing -record; missing lineage fields mean no recorded handoff, not permission to take ownership. +Keep the identifying `v1` marker and legacy worker IDs; neither requires a legacy JSON +reader. Existing canonical Markdown fields remain valid. Missing lineage fields mean no +recorded handoff, not permission to take ownership. For an approved first transfer of a legacy record, use prior handoff revision zero. Use actual values in this illustrative current record: ```markdown +### Work state + - Work status: OCCUPIED - Worker: codex-20260907T190000Z-a1b2c3d4 - Assignment: issue-123 @@ -121,8 +159,10 @@ A returning old worker must re-read current ownership before any mutation and st former assignment after transfer unless explicitly reassigned. Succession authorizes preservation, implementation and the already approved delivery scope; it does not grant merge, deployment, credential, paid-work or force-push authority. A successor modifying -inherited code cannot self-review without explicit user approval and must not present -that exception as independent `Review: LGTM`. +inherited implementation cannot self-review without explicit user approval. Requested +review repair is a distinct bounded role: retain the original implementation worker and +request provenance and disclose the reviewer-as-contributor rather than claiming +independent review of its corrections. ## OPS records, labels and transitions @@ -173,7 +213,11 @@ Missing or inconsistent records block a readiness claim. 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 its own self-review; only an explicit user-approved exception permits `Self-review: LGTM`. - Eligible separate reviews use `Review: LGTM`; an explicitly granted resolver may use + 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 + 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 @@ -186,8 +230,10 @@ Missing or inconsistent records block a readiness claim. those actions are explicitly granted; no global maintainer rights are implied. Re-read ownership and head immediately before mutations. Update only owned records; -ambiguous matches, changed owners and partial writes require reconciliation. A peer -reviewer reports findings without acquiring the branch. Draft prevents merging, not writes. +ambiguous matches, changed owners and partial writes require reconciliation. A requested +reviewer may hold bounded repair write ownership while the original writer pauses; +this does not transfer unrelated implementation assignments. Draft prevents merging, +not writes. Preserve requested scope and provenance when recording the final repair head. ## Conversation titles diff --git a/tests/test_package.py b/tests/test_package.py index 299bd75..0f02c36 100644 --- a/tests/test_package.py +++ b/tests/test_package.py @@ -64,22 +64,39 @@ def test_resolver_protocol_is_reachable_and_review_headings_are_complete(self): row = next(line for line in review.splitlines() if line.startswith('| `Conflict resolution: LGTM`')) self.assertIn('explicitly assigned `conflict-resolver`', row) - def test_sequence_record_carries_snapshot_receipts_and_separate_merge_states(self): + def test_sequence_excerpt_has_canonical_fields_and_separate_merge_states(self): + """Keep the illustrative schema lowercase and avoid publishing a duplicate payload.""" text, blocks = self.conflict_blocks() - records = [body for kind, body in blocks if body.startswith('')] - self.assertEqual(len(records), 1) - record = records[0] - fields = re.findall(r'^- ([^:]+): (.+)$', record, re.MULTILINE) - self.assertEqual(len(fields), len(dict(fields)), 'duplicate sequence record field') - self.assertEqual(set(dict(fields)), {'Sequence', 'Revision', 'Role', 'Resolver', - 'Merge status', 'Base', 'Order', 'Completed', 'Next', 'Readiness scope', 'Ready PR', 'Pending verification', 'Authorization', 'Resume', 'Updated'}) - self.assertEqual(dict(fields)['Role'], 'conflict-resolver') - self.assertNotIn('Work status', dict(fields)) - self.assertIn('| PR | Head | Expected base | Verified tree | Evidence |', record) - self.assertIn('', record) + excerpts = [body for kind, body in blocks if body.startswith('### Merge sequence\n')] + self.assertEqual(len(excerpts), 1) + record = excerpts[0] + pairs = re.findall(r'^- ([^:]+):([^\n]*)$', record, re.MULTILINE) + fields = {name: value.strip() for name, value in pairs} + self.assertEqual(len(pairs), len(fields), 'duplicate canonical field') + self.assertEqual(set(fields), {'schema_version', 'id', 'role', 'resolver', + 'revision', 'status', 'order', 'prs'}) + self.assertEqual(fields['schema_version'], '2') + self.assertEqual(fields['role'], 'conflict-resolver') + self.assertEqual(fields['prs'], '{}') + self.assertEqual(len(re.findall(r'^ - ', record, re.MULTILINE)), 2) + self.assertNotIn('```json', record) + self.assertNotIn('Work status', fields) states = set(re.findall(r'^\| `([A-Z_]+)` \|', text, re.MULTILINE)) self.assertEqual(states, {'PREPARING', 'MERGE_SEQUENCE_READY', 'MERGING', 'MERGED', 'BLOCKED'}) - self.assertIn(dict(fields)['Merge status'], states) + self.assertIn(fields['status'], states) + + def test_review_request_excerpt_has_one_public_payload(self): + """Preserve schema keys and a current record marker without a hidden JSON shadow.""" + text = (ROOT / 'references/pr-review.md').read_text() + excerpts = re.findall(r'```markdown\n(', excerpts[0]) + self.assertEqual(excerpts[0].count('