Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/ROADMAP_WORKFLOW.md
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ These are all the keys of the job record, by the job that writes them:
| Stage | `stage.groups`, `stage.needs_split`, `stage.file_cap` | The task groups with their size, the group over the budget, and the cap of that measure |
| Stage | `stage.ready` | The plan pull request that the Service has seen leave the draft state |
| Stage | `stage.needs_fix` | The plan pull request that the Service told about the label `plan/needs-fix` |
| Stage | `stage.pending` | The intent mark of the stage job, as JSON with the branch name and the time. The job writes it before the push and clears it with the record after the draft. An open Item with the mark, a pushed branch and no `stage.pr` is an orphan: a round at least 1 hour later closes its draft, deletes its branch and clears the record, unless another author wrote to the branch |
| Stage | `stage.text_hash` | The hash of the text of the Item that the plan came from |
| Stage | `stage.rejected` | A plan that a person rejected, as JSON. It holds the pull request, the time of the close, the files that the reason names, and the text hash. The job leaves the Item out until the text or one of those files changes. A staged plan clears it |
| Stage | `stage.failed_at`, `stage.failed_error` | When the last plan or write of the Item failed and the class of its error. The job leaves the Item out for `roadmap.stage.retry_hours` hours; a staged Item or an Item stopped on the budget clears both |
Expand Down
12 changes: 12 additions & 0 deletions docs/roadmap/stage.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,18 @@ The reply of the planner has no key to say that the premise of the issue is fals

Each run also removes the draft and the branch of a closed item, when only the identity of the session wrote to them.

## The orphan draft of a failed write

The job writes the intent mark `stage.pending` before it pushes the branch of an item. The mark is JSON with the branch name and the time. The write of the record after the draft clears it. When that write fails, or the worker dies between the push and the record, the item keeps the mark and has a pushed branch and a draft but no `stage.pr`. This is an orphan.

In each run the job judges an open item that holds the mark and no `stage.pr`. A mark younger than 1 hour is left alone, because another run can be in the middle of its publish. For an older mark the job does these things:

- When the branch of the mark is gone, it clears the record of the item.
- When only the identity of the session wrote to the branch, it closes the draft pull request that has this branch as its head, deletes the branch and clears the record. The job finds the draft with one read of the open pull requests for the round. It makes this read only when it has a pushed branch to remove. The next round plans the item again.
- When a commit past the default branch has another author, it keeps the branch, and the plan lists it.

A pushed branch with no mark is never an orphan, because a person or another agent can own it. A retitle between the failure and the next round does not matter, because the job uses the branch of the mark and not the slug of the title. A closed item that holds the mark is not judged. A bad value of the mark counts as no mark.

## A plan that a person rejected

A person rejects a plan when they close its pull request with no merge. The reason stays on the closed pull request, so the stage job copies it to the issue. In each run the job reads the plan pull request of each staged item. A staged item is an open New or Ready item that holds `stage.pr` and has no plan on the default branch. With `--item` the job reads the pull request of that item only. For a pull request that is closed and not merged, the job does these things in order:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
schema: spec-driven
created: 2026-11-08
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# Design

## Decisions

- **Write an intent mark before the push.**: The mark `stage.pending` holds the branch name and the time. The job can then tell its own orphan from a branch of a person or of an agent, which has no mark.
- **Store the mark as JSON with the fields `branch` and `time`, as `stage.rejected` does.**: The pass uses the branch of the mark. A retitle between the failure and the next round changes the slug of the branch, but not the mark.
- **Write the mark with the `now` of the `StageWriter`.**: The tests set the clock, and the Service passes its own clock. The pass in `plan_stage` uses the `now` of the plan to judge the age.
- **Judge the orphans in `plan_stage`, like `_reconcile`, and read the open pull requests in the writer.**: `plan_stage` stays read-only, and a dry run makes no request. The plan holds a `Removal` for each orphan. `StageWriter.begin` reads `store.open_pulls()` once, only when a removal with a pushed branch exists, and finds the draft by its head.
- **Keep the age as the constant `CONST_ROADMAP_STAGE_ORPHAN_AGE_SECONDS` of 3600 seconds.**: A run in another clone can be in the middle of its publish. A setting would add a key to the ConfigMap for a value that nobody needs to tune.
- **Judge a mark that is as old as the limit.**: The rule is "younger than 1 hour is left alone", so a mark of exactly 1 hour is judged.
- **Judge the pass after the selection of the round.**: The item is still held by its pushed branch in the round that removes the branch. The next round plans it again. The plan facts (the groups with measured tokens and the hash of the text) were in the marks write that failed, so the planner must run again.
- **A bad record of the mark counts as no mark.**: A value that is not JSON, or that holds no branch or time, is logged and the pass leaves the item alone, as `recorded_rejection` does.
- **Do not widen to a closed item.**: A closed item that holds `stage.pending` and no `stage.pr` is not in the issue. It is a known gap.

## Rejected alternatives

- **Delete every pushed branch of an open item that has no `stage.pr`, found by issue number and branch pattern.**: `issue_holder` in `src/devops_cli/git/worktree_add.py` finds a branch by the issue number only. `OpenPull` in `src/devops_cli/roadmap/stores/store.py` has no author field, and `_foreign_commits` compares the commit authors with the `user.name` of the clone. A clone that has the identity of a person passes the guard, so the rule could delete the work of a person or of another agent. It would also defeat `_Judge._held`.
- **Write the branch in `stage.branch` before the push and use that key.**: `_reconcile` and the Service read `stage.branch` as the record of a finished stage. An early write would make a half-staged item look staged.
- **Search the pushed branches for the author `devops-roadmap[bot]`.**: The clone commits as the identity of the App, so a branch of an agent in the same clone has the same author. The mark is the only proof of intent.
- **Order the steps differently so that the marks write comes before the draft.**: The marks write needs the number of the draft pull request, and the record must not name a draft that does not exist.
- **Read the open pull requests in `plan_stage`.**: This adds a request to every round, and a dry run must make none.
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
# Proposal

## Why

The issue #4051 reports an orphan draft. `_stage_one` in `src/devops_cli/roadmap/stage/stage.py` pushes the branch, opens the draft pull request, and then writes the marks in one `set_marks` call. When that write fails, or when the worker dies between the push and the write, the branch and the draft stay on GitHub and the item records no `stage.pr`. The item then stays out of every later round, because `_Judge._held` holds an item for any pushed branch that names its issue number. `_reconcile` reads closed items only. A person must close the draft and delete the branch by hand.

A rule that deletes every pushed branch of an open item with no `stage.pr` is not safe. A branch of a person or of an agent for the same issue number looks the same, and `OpenPull` has no author field. The job needs a mark that proves the branch is its own.

## What Changes

- Add the key `stage.pending` to `StageRecordKey` in `src/devops_cli/roadmap/stores/store.py`. Its value is JSON with the branch name and the time: `{"branch": "<name>", "time": "<ISO 8601>"}`.
- `_stage_one` writes `stage.pending` with one `set_marks` before the push. The time is the `now` of the `StageWriter`. The marks write after the draft sets `stage.pending` to None with the other keys.
- Add a pass `_orphans` next to `_reconcile`. `plan_stage` calls it with its `now` and adds its results to `StagePlan.removals` and `StagePlan.kept`. The pass reads the open items that have `stage.pending` and no `stage.pr`, and it uses the branch name of the mark:
- A mark younger than a constant age of 1 hour gives nothing. The age is the constant `CONST_ROADMAP_STAGE_ORPHAN_AGE_SECONDS` in `src/devops_cli/config/constants.py`, and no setting.
- A branch that is not on `origin` gives `Removal(..., gone=True)`: only the record is cleared.
- A pushed branch with `_foreign_commits` equal to 0 gives a `Removal`. The writer reads `store.open_pulls()` once per round, only when such a removal exists, closes the open pull request whose head is the branch, deletes the branch and clears the record.
- A pushed branch with a commit of another author gives a `KeptBranch`, and costs no request.
- A pushed branch with no `stage.pending` mark is never touched.
- The dry run plan (`stage_requests` in `src/devops_cli/roadmap/request_plan.py`) lists the write of the mark before the push and the read of the open pull requests.
- Fix the documents that the new key makes wrong: `docs/ROADMAP_WORKFLOW.md` (the table of keys), `docs/roadmap/stage.md` (the orphan pass) and the docstring of `StageRecordKey`.
- Add a requirement to the `roadmap-stage` capability, because the stage job removing an orphan is behavior that a consumer observes.
- No setting is added, renamed, removed or tightened. The example and the live ConfigMap need no change. The test `tests/k8s` key-walk test of #4093 still proves that the example ConfigMap loads into `Settings`.
- The change fits one pull request.

## Capabilities

### New Capabilities

None.

### Modified Capabilities

- `roadmap-stage`: A failed marks write leaves no orphan draft

## Impact

- Code: `src/devops_cli/roadmap/stage/stage.py`, `src/devops_cli/roadmap/stores/store.py`, `src/devops_cli/roadmap/request_plan.py`, `src/devops_cli/config/constants.py`, `src/devops_cli/lang/en/messages.py`
- Tests: `tests/roadmap/stage/test_roadmap_stage_orphan.py`
- Documents: `docs/ROADMAP_WORKFLOW.md`, `docs/roadmap/stage.md`
- Request cost: each staged item adds one `set_marks` call (2 requests: one read and one PATCH). A round with an orphan to remove adds one read of the open pull requests. Other rounds add none.
- Open point for review: the owner has not yet confirmed the intent-mark approach. It replaced the plain rule "delete the pushed branch by issue number and branch pattern", because that rule could delete the branch of a person or of another agent.
- Known gap, not widened: a closed item that holds `stage.pending` and no `stage.pr` is not an orphan of this pass. `_reconcile` clears a closed item only when it holds `stage.branch`.

## Feasibility

I checked the premise at `origin/main` by reading the code. I ran no GitHub request.

- `src/devops_cli/roadmap/stage/stage.py`: `_stage_one` calls `_commit_and_push`, then `_open_pull`, then one `set_marks` with BRANCH, PR, SHA, GROUPS and TEXT_HASH, then the labels, then the comment. `_open_pull` deletes the branch when the draft fails. `_label_pull` and `_label_needs_fix` catch `GitHubOperationError`. So only a failed marks write, or a death of the worker between the push and the write, leaves a pushed branch and a draft with no `stage.pr`.
- `_reconcile` reads the closed items with `stage.branch`. `_Judge._held` uses `issue_holder` in `src/devops_cli/git/worktree_add.py`, which holds an item for any pushed branch named with the issue number. This confirms that the item stays out of every round.
- `OpenPull` in `src/devops_cli/roadmap/stores/store.py` has `head_ref`, `number` and `draft`, and no author. `_foreign_commits` is the only author guard. This confirms why the plain rule is unsafe.
- `_remove_staged` clears `{key: None for key in StageRecordKey}`, so a new key is cleared with the rest. `RejectedPlan` in `src/devops_cli/roadmap/stage/stage_rejected.py` is the pattern for a JSON mark.
- `set_marks` in `src/devops_cli/roadmap/stores/github_store.py` makes one read and one PATCH, so the cost of 2 requests holds.
- The tests in `tests/roadmap/stage/test_roadmap_stage_orphan.py` run on the unchanged code, and 11 of 17 fail for the reasons above (`devops ci fail-to-pass --base origin/main`). The 6 that pass are guards: they prove that a mark of a young age, a recorded pull request and a closed item are left alone.
- I did not check how GitHub treats the open pull request of a deleted head branch. The pass closes the draft itself, so it does not depend on that behavior.

## Existing Tool Considered

No open, maintained tool or library repairs the state of one job in the record of an issue. The change reuses `_remove_staged`, `Removal`, `KeptBranch`, `_foreign_commits` and the JSON mark pattern of `stage.rejected`, all in the repository. The change builds one key, one write and one pass. The written cost: about 50,000 tokens and 2.5 hours to build, about 2,000 tokens each quarter to keep, and about 10,000 tokens and 0.5 hour to fix. No internal code is deleted.
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
## ADDED Requirements

### Requirement: A staged item records its intent before the push
The job SHALL write the mark `stage.pending`, the branch name and the time as JSON, with one write of marks before it pushes the branch of an item. The write of the marks after the draft SHALL clear `stage.pending` with the other keys.

#### Scenario: The request cost of a staged item
- **WHEN** a round stages one item
- **THEN** the round writes the marks twice for that item, and the record holds no `stage.pending` at the end

### Requirement: An old intent mark with no recorded pull request gives an orphan
In each run the job SHALL look at the open items that hold `stage.pending` and no `stage.pr`, and SHALL use the branch name of the mark. A mark younger than 1 hour SHALL give no action. A branch that is not on `origin` SHALL give only the clear of the record.

#### Scenario: The marks write fails
- **WHEN** the write of the marks after the draft fails, and a later round runs after the mark is 1 hour old
- **THEN** the round closes the draft, deletes the branch and clears the record, and a following round plans the item again

#### Scenario: The worker dies after the push
- **WHEN** the worker dies after the push and before the draft, and a later round runs after the mark is 1 hour old
- **THEN** the round deletes the branch, makes no request to close a draft and clears the record

#### Scenario: A retitle changes the slug
- **WHEN** a person changes the title of the item between the failure and the next round
- **THEN** the round still finds the orphan by the branch name of the mark

#### Scenario: A young mark
- **WHEN** the mark is younger than 1 hour
- **THEN** the job makes no request and keeps the mark, and a round that runs when the mark is 1 hour old judges it

#### Scenario: The branch is gone
- **WHEN** the branch of the mark is not on `origin`
- **THEN** the job clears the record and makes no request

### Requirement: The job removes an orphan only when it wrote the branch alone
When every commit of the pushed branch of an orphan past the default branch is by the identity of the clone or is a clean merge, the job SHALL close the open pull request whose head is the branch, delete the branch and clear the record. The job SHALL read the open pull requests once in a round, and only when such a branch exists. When any other commit exists, the job SHALL keep the branch and the draft and say so in the plan.

#### Scenario: A person added a commit
- **WHEN** the pushed branch of the mark holds a commit by another author
- **THEN** the job keeps the branch and the draft, makes no request and the plan says why

#### Scenario: The request cost of a removal
- **WHEN** a round removes two orphans
- **THEN** the round reads the open pull requests once

### Requirement: A branch with no intent mark is never touched
The job SHALL NOT close, delete or change a pushed branch or a pull request whose item holds no `stage.pending`.

#### Scenario: A branch with no mark
- **WHEN** a person or an agent pushed a branch for another item that holds no `stage.pending`
- **THEN** the job does not touch the branch or its pull request
Loading
Loading