webgrip/unfold
5
0
Fork 0
Code Issues 1 Pull requests 2 Projects Releases 54 Packages 8 Wiki Activity Actions

fix(ploeg): read a writer's branch on the forge before it counts as no change or updated #188

Merged
ryangr0 merged 1 commit from ryangr0/ploeg-writer-branch-delivery-guard into development 2026-10-03 15:12:20 +00:00 AGit
Conversation 0 Commits 1 Files changed 5 +395 -42
ryangr0 commented 2026-10-03 12:25:38 +00:00
Owner
Copy link

Follow-up to #177 (merged), which replaced #159. Covers the EXEC-06 acceptance criteria #177 left open.

Problem

#177 compares a writer's checkout with its starting commit, but only when the Run resolves to no_change_needed, and it trusts an empty pull request lookup. Still complete on development:

  • A writer whose pull request was already open is credited pr_updated while its edits or commits stay in the clone.
  • A failed pull request lookup reads as "no pull request", so a clean checkout becomes no_change_needed and the Work Item done.
  • A writer that pushed its branch but opened no pull request gets the wrong reason. The depth-limited clone only tracks the base, so its pushed commits count as "not on the forge".
  • Nothing tells "the branch moved during this Run" from "the branch was already there".

Invariant

A writing Run ends no_change_needed only when the worker read the forge successfully, the Run's branch did not move on the forge, and the clone holds nothing the forge does not have. It ends pr_updated only when the branch moved, or when the clone holds nothing unpublished (the case where nothing at all was pushed is left to VIK-1732 / ADR-0059). An unknown read never completes a Run.

Change

  • Before the harness runs, the worker records the starting commit and the Run branch's head on the forge (git ls-remote).
  • When a writer would end no_change_needed or pr_updated, it reads the branch again:
    • a failed pull request lookup or branch read → stuck, "whether this Run pushed or opened a pull request is unknown";
    • the branch moved and there is no pull request → stuck, naming the branch and both heads;
    • pr_updated and the branch moved → unchanged;
    • otherwise uncommitted paths, or local commits that neither the remote branch nor the base contains → stuck, naming them.
  • Generated-output policy: everything git status lists counts as a change, so the repository's .gitignore decides what is build output. This is tested both ways.
  • pr_opened, failed, stuck and every reader are untouched. Usage and failure reasons keep their meaning.
  • docs/concepts/inside-a-run.md step 8 describes the rule.

No automatic push or pull request (EXEC-06 non-goal). The stuck reason names the paths and commits. The content itself is not saved off the pod.

Tests (worker + real ACP adapter + git-http-backend forge)

New: an edit-tool change, a commit on the base branch, a pushed branch without a pull request, local-only changes under an open pull request (3 variants), a real push to the open pull request (stays pr_updated), a new pull request with a stray file (stays pr_opened), a failed forge read (2 variants), untracked output the repository does not ignore, a reader that leaves files (keeps its approval), and a harness failure or pod termination that also edited the checkout (keeps agent_error / infra_node). Usage cost from the ACP agent survives the conversion to stuck.

Before the fix, 6 of these failed on #177's head (the pr_updated variants, the forge-read variants, and the wrong reason for a pushed branch). TestOpenSpecWorkItem_WriterBriefedAndGated is split. The gate still checks the pushed branch, and a fix left only in the working tree is now caught earlier as unpublished.

Commands (at 7fc6f8d9, Go 1.27.1, Node 24.21.0)

  • mise exec -- go test ./pkg/worker/ -count=1 -v: 154 top-level tests passed, 0 failed
  • mise exec -- go test ./... -count=1 in apps/ploeg: 34 packages ok, 0 failed
  • mise run verify: all gates passed (vloer 7, demo-replay 1, vloer-extension 4, ploeg 7 incl. gofmt/vet/build/test/openspec, brand 1, site 5, site-demo 1, helm 15, release 3, integration 1, docs 1)
  • docs-check ran inside verify (docs gate).

Still open (tracked)

  • pr_updated when nothing at all was pushed and the clone is clean; a failed pre-run pull request lookup (VIK-1732, ADR-0059).
  • Verifying the pushed candidate rather than the clone (VIK-1738).

Refs VIK-1737

🤖 Generated with Claude Code

Follow-up to #177 (merged), which replaced #159. Covers the EXEC-06 acceptance criteria #177 left open. ## Problem #177 compares a writer's checkout with its starting commit, but only when the Run resolves to `no_change_needed`, and it trusts an empty pull request lookup. Still complete on `development`: - A writer whose pull request was already open is credited `pr_updated` while its edits or commits stay in the clone. - A failed pull request lookup reads as "no pull request", so a clean checkout becomes `no_change_needed` and the Work Item `done`. - A writer that pushed its branch but opened no pull request gets the wrong reason. The depth-limited clone only tracks the base, so its pushed commits count as "not on the forge". - Nothing tells "the branch moved during this Run" from "the branch was already there". ## Invariant A writing Run ends `no_change_needed` only when the worker read the forge successfully, the Run's branch did not move on the forge, and the clone holds nothing the forge does not have. It ends `pr_updated` only when the branch moved, or when the clone holds nothing unpublished (the case where nothing at all was pushed is left to VIK-1732 / ADR-0059). An unknown read never completes a Run. ## Change - Before the harness runs, the worker records the starting commit and the Run branch's head on the forge (`git ls-remote`). - When a writer would end `no_change_needed` or `pr_updated`, it reads the branch again: - a failed pull request lookup or branch read → `stuck`, "whether this Run pushed or opened a pull request is unknown"; - the branch moved and there is no pull request → `stuck`, naming the branch and both heads; - `pr_updated` and the branch moved → unchanged; - otherwise uncommitted paths, or local commits that neither the remote branch nor the base contains → `stuck`, naming them. - Generated-output policy: everything `git status` lists counts as a change, so the repository's `.gitignore` decides what is build output. This is tested both ways. - `pr_opened`, `failed`, `stuck` and every reader are untouched. Usage and failure reasons keep their meaning. - `docs/concepts/inside-a-run.md` step 8 describes the rule. No automatic push or pull request (EXEC-06 non-goal). The stuck reason names the paths and commits. The content itself is not saved off the pod. ## Tests (worker + real ACP adapter + git-http-backend forge) New: an edit-tool change, a commit on the base branch, a pushed branch without a pull request, local-only changes under an open pull request (3 variants), a real push to the open pull request (stays `pr_updated`), a new pull request with a stray file (stays `pr_opened`), a failed forge read (2 variants), untracked output the repository does not ignore, a reader that leaves files (keeps its approval), and a harness failure or pod termination that also edited the checkout (keeps `agent_error` / `infra_node`). Usage cost from the ACP agent survives the conversion to `stuck`. Before the fix, 6 of these failed on #177's head (the `pr_updated` variants, the forge-read variants, and the wrong reason for a pushed branch). `TestOpenSpecWorkItem_WriterBriefedAndGated` is split. The gate still checks the pushed branch, and a fix left only in the working tree is now caught earlier as unpublished. ## Commands (at 7fc6f8d9, Go 1.27.1, Node 24.21.0) - `mise exec -- go test ./pkg/worker/ -count=1 -v`: 154 top-level tests passed, 0 failed - `mise exec -- go test ./... -count=1` in apps/ploeg: 34 packages ok, 0 failed - `mise run verify`: all gates passed (vloer 7, demo-replay 1, vloer-extension 4, ploeg 7 incl. gofmt/vet/build/test/openspec, brand 1, site 5, site-demo 1, helm 15, release 3, integration 1, docs 1) - docs-check ran inside verify (docs gate). ## Still open (tracked) - `pr_updated` when nothing at all was pushed and the clone is clean; a failed pre-run pull request lookup (VIK-1732, ADR-0059). - Verifying the pushed candidate rather than the clone (VIK-1738). Refs VIK-1737 🤖 Generated with [Claude Code](https://claude.com/claude-code)
fix(ploeg): read a writer's branch on the forge before it counts as no change or updated
Some checks failed
[Workflow] On Pull Request / release-policy (pull_request) Successful in 34s
Details
[Workflow] On Pull Request / checks (pull_request) Successful in 10m21s
Details
[Workflow] On Pull Request / warnings (pull_request) Has been cancelled
Details
7fc6f8d988
 
The unpublished-work guard from #177 compared a writer's checkout with
its starting commit, but only when the Run resolved to no_change_needed,
and it trusted an empty pull request lookup. Four cases still ended as
complete:

- a writer whose open pull request already existed was credited
  pr_updated while its edits or commits stayed in the clone;
- a failed pull request lookup read as "no pull request", so a clean
  checkout became no_change_needed and the Work Item done;
- a writer that pushed its branch but opened no pull request was
  reported with a wrong reason (its pushed commits counted as not on
  the forge, because the depth-limited clone tracks only the base);
- nothing distinguished "the branch moved during the Run" from "the
  branch was already there".

The worker now records where the Run's branch stands on the forge
(git ls-remote) next to the starting commit, before the harness runs.
When a writer would end no_change_needed or pr_updated it reads the
branch again. A failed pull request lookup or branch read is stuck
("unknown"); a branch that moved without a pull request is stuck; a
pr_updated whose branch moved keeps pr_updated; otherwise uncommitted
paths (anything git status lists, so the repository's .gitignore
decides what is build output) or local commits that neither the remote
branch nor the base contains are stuck, naming them. pr_opened,
failed and stuck outcomes, and readers, are untouched.

Tests drive the real worker with a fake ACP agent against a
git-http-backend forge: edit-tool, shell and base-branch commits, a
pushed branch without a pull request, local-only changes under an open
pull request, a real push to it, a failed forge read, untracked output
the repository does not ignore, a reader that leaves files, and a
harness failure or pod termination that also edited the checkout.

VIK-1737

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
ryangr0 merged commit 45ebdce5ce into development 2026-10-03 15:12:20 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
webgrip/unfold!188
No description provided.