feat(agent): agent/shifts-docs #26
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "agent/shifts-docs"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Both executors now range over (team, Role) rather than team. A team with no plan yields exactly one Role-less workload whose rendered manifest is unchanged; a planned team yields one per distinct Role, each with its own image, harness, model, dind setting and replica cap. That is what makes "different agents, different harnesses, different images" literally true — pod shape is fixed at render time, so it cannot be a runtime decision. Role workloads scale on the role predicate, quoted beside the query it must match byte-for-byte: pending runs for (team, role), served index-only by the agent_runs_claimable index migration 0008 created verbatim for it. Overshoot wastes a pod that exits 0; undershoot stalls Work Items with no error anywhere, so this third copy of the predicate carries its leash in a comment. SECURITY — the PolicyException stops tracking workload names. The privileged DinD sidecar needs a Kyverno waiver, and the exception in webgrip/homelab-cluster matched `scaledjob.keda.sh/name` with one entry per team. Under role partitioning that meant a security-repo change for every new writer Role, which is how waivers rot. Two things were wrong with that shape, and both are fixed by keying the exception to the HAZARD instead: * exception-governance forbids wildcards in resources.names[] — it says nothing about label selectors, and this exception never used names. The per-team convention was self-imposed, not required. * the Pod-level match was `app.kubernetes.io/name: ploeg-worker`, the label EVERY worker pod carries. Copper (dind: false) is waived today for privileged-containers, run-as-non-root and drop-all-capabilities while running no privileged container at all, and every reader would have inherited the same over-grant. The chart now emits `ploeg.webgrip.dev/privileged-dind: "true"` only where it actually renders the privileged sidecar. The waiver follows the privilege: adding a Role costs no security-repo change, and readers — which run dind: false — fall outside it entirely. Strictly narrower than today. ADR-0013 tier 1 lands as far as this repo can take it: a reading Role draws AGENT_BUILDER_TOKEN from executor.forgejo.readTokenSecret when configured, so the reader/writer split is enforced by the forge and not only by scheduling. Unset is documented as a known gap rather than a safe default — the repos are private, so "no credential" cannot clone, and closing it is one OpenBao entry plus one ExternalSecret, no chart or code change. Also: LITELLM_KEY_BUDGET degrades to the Role's own cap rather than the team's budget, because for a planned team that value is the SHIFT POOL and handing one Run the whole pool would be wrong if the fallback ever applied. Guarded by committed golden renders (scripts/helm-golden.sh, wired into CI). The pod is a security boundary and the waiver is keyed on a label this chart emits, so a manifest change now has to appear in the diff. Verified in both directions: green as committed, and red when the privileged-dind label is widened to pods that take no privilege. Two render-time guards added with tests of their own: a Role defined twice with different settings fails the render (one Role is one workload, so its shape cannot change between rounds), and a workload name over 63 characters fails rather than being truncated into a collision. CI fixture gains a planned team covering the whole new branch — reader/writer credential split, the dind-less reader pods, and a Role recurring across rounds collapsing to one workload. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>provider.ForgeProvider has been declared since the SPI was carved with zero implementations. This gives it its first one and its first caller in the same change, because the two are the same story: the blackboard (ADR-0011) is a reading Run's findings travelling to where a human is already looking. pkg/provider/forgejo does two things and no more. Comment posts to a pull request through the ISSUES endpoint — Forgejo models a PR as an issue with a branch attached, and /pulls/{n}/comments would be a review comment on a diff hunk, which a round's findings are not. ParseWebhook verifies the raw body against X-Forgejo-Signature BEFORE any JSON parsing and normalizes review_submitted / check_failed / merge_state_dirty, so the Follow-Ups of R9 need no further provider work. Events Ploeg does not act on are dropped without error: a forge subscribes wider than the core consumes, and erroring on every unrelated push is how a webhook ends up disabled. The Vikunja provider's stubs become real. FetchItem gives the thin-payload rule its authoritative half; Comment creates with PUT, not POST (the trap recorded in docs/ops/board.md — a POST there silently does something else); SetStatus writes only `done`, because needs_human and stale are NOT done and marking them so would hide the item from the very board that has to act on it. Inventing a label mapping for needs_human would put a Ploeg concept inside the provider (R7); what a human needs — why it stopped and which PR to look at — travels in the comment instead. Both are opt-in by credential. Without a URL and token they keep the prototype's logging no-op, so a deployment that has not been given tracker credentials still finishes runs; it just does not update the board. The engine now publishes each reading Run's findings when its Round completes rather than at close, so a human watching the thread sees the review while the writer is still working from it, and writes back to the tracker when the Shift closes — the PR link plus a request to merge, which is what turns the handoff from something noticed into something announced. Publication is best-effort, everywhere and deliberately. A forge outage, a tracker outage, an unresolved Work Target or a Shift with no pull request yet must not stall the pipeline or lose an Outcome: every failure is logged and none is returned into the lifecycle, and the tracker write-back happens AFTER the state is durable so an outage can never leave a Shift open. Tests drive both outages through a full plan and assert the Shift still closes and the item still reaches needs_human. Round-1 readers routinely run before any PR exists; their findings are not lost, they reach the writer through the briefing on its claim. Duplicate comments are possible and accepted: two evaluators can both observe a completed Round before one wins the advance CAS. A duplicated comment is visible and harmless; a missing one loses a review a human is waiting for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>Implements ADR-0017. A reading Run may return approve or request_changes in its OutcomeReport; a request re-opens the plan's OWN last writing Round with the findings attached, then the review Round after it. Each pair is one fix round. Before this, a reviewer that found a real defect had nowhere to send it — the plan's next entry opened regardless, and if the reviewer was last the Shift closed with the defect recorded and unfixed. MERGING THIS RATIFIES ADR-0017, which is still `proposed`. It is the first place an agent's output influences what runs next, so the boundary is worth reading before it lands: a verdict names no Role, authors no Round, raises no cap and extends no budget. It is one bit that may re-run work the operator already configured. The bounds are checked in the record's order, and each closes with its own reason so "why did this item stop" stays a query rather than a reconstruction: 1. the pool — money is the limit that cannot be argued with, so no Run is ever spawned that the Shift cannot pay for; 2. maxFixRounds — the cap, default off, configured per Team; 3. the verdict — the only bound an agent influences, and checked last. A writer's verdict is ignored twice over: the store blanks it on write (CASE WHEN writes THEN ''), and the loop skips writing Runs when it looks. A writer approving its own work would be the loop grading itself, and one guard could be refactored away without the other noticing. The fix-round count is derived from the Shift's round counter against the plan's length, never stored — the same discipline ADR-0012 applies to `reserved`. It cannot drift from what happened, and it survives a restart mid-loop for free, which a test pins directly. pkg/plan refuses maxFixRounds > 0 on a plan with no writing Round at boot, rather than at the moment a reviewer first asks for changes: a plan that cannot fix anything would otherwise look healthy for hours and then quietly ignore its first real verdict. The reviewer prompt now asks for the verdict and says what each value does — including that request_changes sends work back to the writer, so it is for things that must change rather than for thoroughness. Migration 0010 adds the column with a CHECK constraining it to the two values plus empty; the schema enum, the Go type and the boundary validator change together. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>POST /webhooks/forge/{provider} gives ForgeProvider.ParseWebhook the caller it has never had. The signature is verified against the raw body before anything is parsed (backlog #2), the handler does no expensive work — Forgejo's DELIVER_TIMEOUT is 5 seconds and a slow endpoint becomes a disabled one (backlog #3) — and every accepted event lands in the audit log. It acts on nothing, deliberately. Routing a submitted review into a re-mandate needs the branch-to-Work-Item lookup backlog #107 owes it, and there are now TWO paths that mean "keep going" — an agent's verdict (ADR-0017) and a human's review. Reconciling them is a decision, not a merge order, and ADR-0017 names the arrival of this route as the trigger to make it. What lands now is the endpoint, so the events are recorded from the day the network path opens rather than from the day somebody notices it was never wired. Dedup is a table, not a cache (migration 0011). A forge retries what it thinks failed, and a retry that acts twice turns one review into two fix rounds; an in-memory set would forget across exactly the restart a redelivery is most likely to follow. The insert IS the check — ON CONFLICT DO NOTHING — so two concurrent deliveries of one id cannot both conclude they are first. Ids are swept with the leases after 48 hours, well past any forge's retry window. A missing delivery header is treated as fresh rather than as a duplicate: a forge that sends none must not have every event silently dropped. The event BODY is not stored. It is text written outside the factory (backlog #9), and an audit row is read by humans and future prompts alike; the metadata is what routing will need. Tests: verified event recorded, wrong and missing signatures rejected with nothing touched, three deliveries of one id acting once, no-delivery-header not deduped, unknown provider 404, and a push webhook creating neither an audit row nor a work item. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>agent/shifts-docsto feat(agent): agent/shifts-docs