diff --git a/tasks/done/02-start-cleanly-when-bench-is-the-project.md b/tasks/archive/02-start-cleanly-when-bench-is-the-project.md similarity index 99% rename from tasks/done/02-start-cleanly-when-bench-is-the-project.md rename to tasks/archive/02-start-cleanly-when-bench-is-the-project.md index 06b6a64..2330b6b 100644 --- a/tasks/done/02-start-cleanly-when-bench-is-the-project.md +++ b/tasks/archive/02-start-cleanly-when-bench-is-the-project.md @@ -1,6 +1,6 @@ # 02 — Start cleanly when bench is the project itself -**Status:** Done +**Status:** Archived **PR:** https://github.com/12vectors/bench/pull/1 **Priority:** High — ./start.sh is broken in this repo, and one of the three defects breaks every fresh install anywhere **Type:** Bug diff --git a/tasks/in-progress/03-focus-checks-come-from-the-project.md b/tasks/archive/03-focus-checks-come-from-the-project.md similarity index 61% rename from tasks/in-progress/03-focus-checks-come-from-the-project.md rename to tasks/archive/03-focus-checks-come-from-the-project.md index a6671a3..fc488ba 100644 --- a/tasks/in-progress/03-focus-checks-come-from-the-project.md +++ b/tasks/archive/03-focus-checks-come-from-the-project.md @@ -1,6 +1,7 @@ # 03 — Focus checks come from the project, not from core -**Status:** In Progress +**Status:** Archived +**PR:** https://github.com/12vectors/bench/pull/6 **Priority:** Medium — cosmetic on bench's own board, but every adopting project sees a stranger's definition of done **Type:** Feature @@ -83,3 +84,20 @@ lint-imports / frontend` as bench's checks, all "not run" — they are the origin project's, frozen at extraction time (the same fossil layer as the `.task-manager/` path assumption in `../backlog/02-start-cleanly-when-bench-is-the-project.md`). + + +--- + +## Work report — 2026-07-30 07:18 (Basil) + +The work is complete: implemented, tested and committed in two commits on `task/03-focus-checks-come-from-the-project` (`7df286b` the mechanism, `fe4872d` the doc rewording), with the full suite green — 69 tests, 20 of them new, via `python3 -m unittest discover -s tests` — and a clean tree. + +**What changed.** What counts as a definition-of-done check now lives in one project-owned file instead of being hardcoded into core. `manager/core/checks` ships the old three rows (pytest / lint-imports / frontend) as the default; a file named `checks` in `manager/local/` replaces it wholesale, exactly like prompt overrides. The claude adapter (`manager/core/adapters/claude/emit.py`) classifies each Bash command against the resolved file and carries the entry's label into the event summary; `config.checks()` reads the same file and `httpd.py` serves it in `/api/state`; the Focus panel in `manager/core/board.html` renders one row per served entry and matches events with the same patterns — no fixed rows, no duplicated regexes. Bench's own `manager/local/checks` defines `unittest`, so the self-hosted Focus view finally shows a check that can actually run here. `CLAUDE.md` and `tasks/task-template.md` now describe the definition of done generically. + + +**For the reviewer, in order of interest:** + +- One judgment call to sanction: the acceptance bans naming pytest anywhere in `manager/core/` outside the checks default, so the shipped `BOARD_AGENT_COMMANDS` default shrank from `python3 -m unittest,python3 -m pytest` to `python3 -m unittest` (`manager/core/config.py`, `.env.example`). A pytest project relying on the old default must now set one line in `local/.env` — the `.env.example` comment already warns that a missing runner is a test agents cannot run. +- A deliberate behaviour shift: the claude adapter no longer emits kind `test` (every matched check is kind `check`, which the schema keeps for other adapters), and the Sessions header's "N test runs" counter became "N check runs" counting both kinds. +- `emit.py`'s runtime moved under a `__main__` guard so the classifier is importable by tests; hook invocation (`python3 emit.py`) is unchanged. +- `tests/test_checks_definition.py` pins the acceptance directly, including a repo-walk test asserting no fossil stack names in `manager/core/`, a parser-agreement test between adapter and board, and a label-edit-flows-through test. diff --git a/tasks/done/05-agent-permission-modes-forbid-the-agents-own-contract.md b/tasks/archive/05-agent-permission-modes-forbid-the-agents-own-contract.md similarity index 99% rename from tasks/done/05-agent-permission-modes-forbid-the-agents-own-contract.md rename to tasks/archive/05-agent-permission-modes-forbid-the-agents-own-contract.md index af2b43e..fc1b1a4 100644 --- a/tasks/done/05-agent-permission-modes-forbid-the-agents-own-contract.md +++ b/tasks/archive/05-agent-permission-modes-forbid-the-agents-own-contract.md @@ -1,6 +1,6 @@ # 05 — Launch headless agents with permissions that allow their own contract -**Status:** Done +**Status:** Archived **Priority:** High — the work loop's core promise (implement, test, commit) is currently impossible for the agent asked to keep it **Type:** Bug diff --git a/tasks/done/06-agent-reports-written-for-humans.md b/tasks/archive/06-agent-reports-written-for-humans.md similarity index 99% rename from tasks/done/06-agent-reports-written-for-humans.md rename to tasks/archive/06-agent-reports-written-for-humans.md index bee5908..31d9679 100644 --- a/tasks/done/06-agent-reports-written-for-humans.md +++ b/tasks/archive/06-agent-reports-written-for-humans.md @@ -1,6 +1,6 @@ # 06 — Agent closing reports written for humans, not for the parser -**Status:** Done +**Status:** Archived **PR:** https://github.com/12vectors/bench/pull/2 **Priority:** Medium — every report lands in a task file and a PR body that a person must act on **Type:** Feature diff --git a/tasks/done/07-richer-task-template.md b/tasks/archive/07-richer-task-template.md similarity index 99% rename from tasks/done/07-richer-task-template.md rename to tasks/archive/07-richer-task-template.md index 4e0e472..7bfa9b9 100644 --- a/tasks/done/07-richer-task-template.md +++ b/tasks/archive/07-richer-task-template.md @@ -1,6 +1,6 @@ # 07 — Enrich the task template (new tasks only; existing cards untouched) -**Status:** Done +**Status:** Archived **PR:** https://github.com/12vectors/bench/pull/3 **Priority:** Medium — template quality compounds: every future card inherits it **Type:** Chore diff --git a/tasks/done/08-focus-report-well-expands.md b/tasks/archive/08-focus-report-well-expands.md similarity index 99% rename from tasks/done/08-focus-report-well-expands.md rename to tasks/archive/08-focus-report-well-expands.md index 50478e5..53f8f02 100644 --- a/tasks/done/08-focus-report-well-expands.md +++ b/tasks/archive/08-focus-report-well-expands.md @@ -1,6 +1,6 @@ # 08 — Let the agent's report open from the Focus view -**Status:** Done +**Status:** Archived **PR:** https://github.com/12vectors/bench/pull/4 **Priority:** Medium — the report is the payoff of a whole agent run, and Focus dead-ends exactly there **Type:** Bug diff --git a/tasks/in-progress/09-fit-the-board-on-small-laptops.md b/tasks/archive/09-fit-the-board-on-small-laptops.md similarity index 54% rename from tasks/in-progress/09-fit-the-board-on-small-laptops.md rename to tasks/archive/09-fit-the-board-on-small-laptops.md index 2c8d596..0c6c36b 100644 --- a/tasks/in-progress/09-fit-the-board-on-small-laptops.md +++ b/tasks/archive/09-fit-the-board-on-small-laptops.md @@ -1,6 +1,7 @@ # 09 — Fit all five columns on small laptop screens -**Status:** In Progress +**Status:** Archived +**PR:** https://github.com/12vectors/bench/pull/8 **Priority:** Medium — daily-driver hardware; the Done column is cut off on a 13" MacBook **Type:** Bug @@ -67,3 +68,29 @@ Measured from a screenshot on the owner's 13" MacBook (2026-07-29): Done column clipped mid-card at the right viewport edge. Related in spirit to 04 (the activity log grip) — both are the board's chrome not keeping up with real usage; unrelated in code. + + +--- + +## Work report — 2026-07-30 07:32 (Ada) + +Committed and tested. The work is complete — here is the closing report. + +--- + +The fix is committed on this branch (`e1b9a8f`) and the full suite (`python3 -m unittest discover -s tests`, 80 tests including 5 new ones) passes. Two files changed: `manager/core/board.html` and a new `tests/test_column_flex.py`. + +**What changed.** The kanban columns no longer demand a fixed 296px each. In `manager/core/board.html` the `.kcol` rule went from `flex:0 0 var(--col); width:var(--col)` to `flex:1 1 0; min-width:var(--col-min); max-width:var(--col)`, and a new `--col-min:240px` design token sits beside `--col` in the `:root` block. The effect, per viewport width: + +- **≥1556px** — every column caps at `--col` (296px) and the row stays left-aligned, so the board is pixel-identical to before. +- **1280–1556px** — all five columns shrink evenly (equal flex factors from a zero basis). At the 240px floor the total footprint is 5×240 + 4×10px gap + 36px board padding = 1,276px, so 1440, 1366 and 1280 viewports all fit with no horizontal scroll. +- **below ~1276px** — the pre-existing `overflow-x:auto` on `#board` takes over, unchanged. + +Card internals were already width-tolerant (`min-width:0`, ellipsis, `overflow-wrap`), so no card CSS needed touching, and drag-and-drop is element-based, unaffected by column width. I also checked the header, Sessions and Focus views for 1280px cutoffs as the task asked: the Sessions rail is fixed 250px beside a `min-width:0` fluid pane, the Focus grid uses `minmax(0,…)` fractions with a 900px breakpoint, and the header flex-wraps — all pass as-is, so nothing speculative was changed. + +**How it was verified.** This repo's convention for `board.html` (no frontend runner) is source-level invariant tests; `tests/test_column_flex.py` follows the pattern of `tests/test_focus_well.py` and pins: the floor is a token the `.kcol` rule actually uses; columns grow/shrink with a `--col` cap still at 296px; the arithmetic *five floors + four gaps + board padding ≤ 1280* (recomputed from the live CSS values, so a drive-by token edit that breaks the fit fails the test); a floor no lower than 240px; and the `overflow-x:auto` fallback surviving. + +For the reviewer: + +- Look first at the two-line CSS change in `manager/core/board.html` (lines 23 and 94) — everything else is tests. +- The one thing tests cannot prove is how it *looks*: worth a 30-second window-resize on a real 13" MacBook before merging, since that screenshot is what motivated the card. diff --git a/tasks/in-progress/12-choose-agent-models-per-intent.md b/tasks/archive/12-choose-agent-models-per-intent.md similarity index 58% rename from tasks/in-progress/12-choose-agent-models-per-intent.md rename to tasks/archive/12-choose-agent-models-per-intent.md index 5243c54..92307dd 100644 --- a/tasks/in-progress/12-choose-agent-models-per-intent.md +++ b/tasks/archive/12-choose-agent-models-per-intent.md @@ -1,6 +1,7 @@ # 12 — Let the board choose agent models, per launch intent -**Status:** In Progress +**Status:** Archived +**PR:** https://github.com/12vectors/bench/pull/10 **Priority:** Medium — works today by inheritance, but invisibly and uncontrollably **Type:** Feature @@ -88,3 +89,29 @@ that defined" and the honest answer was "nowhere, and it depends whose machine the board runs on" — with every agent so far silently riding the owner's personal CLI default. Inheritance stays the default; this card makes it a choice instead of an accident. + + +--- + +## Work report — 2026-07-30 07:38 (Juno) + +The work is complete and committed. Here is the closing report: + +--- + +The task is implemented, tested and committed as a single commit (`9fed08d`) on `task/12-choose-agent-models-per-intent`; the full suite (87 tests, including 11 new ones) passes with `python3 -m unittest discover -s tests`, which is this project's entire definition of done. + +**What changed** + +- `manager/core/config.py` — four new defaults-empty settings (`BOARD_AGENT_MODEL` plus `_WORK`/`_ACT_PR`/`_REVIEW`) and an `agent_model(mode)` resolver: the per-intent setting wins, then the general one, and empty means inherit the vendor default. Core treats the name as an opaque string throughout. +- `manager/core/agents.py` — `_launch` puts the resolved model in the adapter env as `AGENT_MODEL` only when one is configured; when none is, it also *removes* any `AGENT_MODEL` the board process itself inherited, so a shell variable can no longer hijack launches silently. All four launch kinds store the resolved model (or `None` for inherited) on the agent record, exposed through `_agent_public`. +- `manager/core/adapters/claude/run` — appends `--model "$AGENT_MODEL"` when set (using the bash-3.2-safe empty-array idiom, since macOS's `/bin/bash` runs these scripts under `set -u`). +- `manager/core/adapters/opencode/permission_config.py` and `run` — the generated config gains a top-level `"model"` key when set. I verified the key and its `"provider/model-id"` format against opencode.ai/docs/config today, as the task asked. +- `manager/core/.env.example` and `manager/core/adapters/README.md` — the four settings and the `AGENT_MODEL` contract (optional env in; absent = vendor default; value = opaque, passed through untranslated, never sent empty) are documented. +- `manager/core/board.html` — the Sessions header line and the Focus reference line now show the model an agent run was given, or "model inherited", for exactly the honesty the task's Notes asked for. Interactive (human) sessions show nothing, since the board never launched them. +- `tests/test_agent_model.py` — new. Covers intent→model resolution in a fresh interpreter per env combination, the `_launch` env seam (including the stray-variable strip), the record field, and both adapters end-to-end via stub binaries, asserting launches with nothing set are argv/config-identical to before this change. + +**For the reviewer** + +- Start with `tests/test_agent_model.py` — the two `test_unset_launches_byte_identical_to_today` tests are the acceptance criterion that matters most, and the `LaunchEnv` class pins the "empty means the variable is simply absent" rule. +- One judgment call worth a look: `_launch` popping `AGENT_MODEL` from the inherited environment. The task's motivation section called out silent environment hijack for `ANTHROPIC_MODEL`; I stripped only the board's own contract variable (`AGENT_MODEL`), not arbitrary vendor variables, since filtering vendor env would be vendor knowledge core must not have. diff --git a/tasks/backlog/13-agents-md-as-the-canonical-brief.md b/tasks/archive/13-agents-md-as-the-canonical-brief.md similarity index 60% rename from tasks/backlog/13-agents-md-as-the-canonical-brief.md rename to tasks/archive/13-agents-md-as-the-canonical-brief.md index 339374a..7e533c2 100644 --- a/tasks/backlog/13-agents-md-as-the-canonical-brief.md +++ b/tasks/archive/13-agents-md-as-the-canonical-brief.md @@ -1,6 +1,7 @@ # 13 — AGENTS.md as the canonical brief; CLAUDE.md becomes a pointer -**Status:** Backlog +**Status:** Archived +**PR:** https://github.com/12vectors/bench/pull/9 **Priority:** Medium — correctness gap for the opencode adapter today, naming debt everywhere else **Type:** Refactor @@ -96,3 +97,31 @@ a vendor's name was baked into core-owned, cross-vendor machinery. - Host projects may have their own root AGENTS.md; bench's lives inside `.task-manager/`, so no collision — but verify nested-file pickup for both vendors rather than assuming it. + + +--- + +## Work report — 2026-07-30 07:35 (Milo) + +The task is complete. Closing report: + +--- + +The rename is implemented, tested and committed as `e10b5cf` on this branch: the workflow brief now lives in `AGENTS.md` (root and `manager/local/`), each `CLAUDE.md` is a five-line compatibility pointer, and the full suite — 78 tests including a new end-to-end `update.sh` round-trip test — passes. + +What changed: + +- `AGENTS.md` now holds the full brief verbatim; the only content edits are its own tree diagram (both files listed, pointer labelled) and one "checks it against the task and AGENTS.md" self-reference. `CLAUDE.md` is now only an `@AGENTS.md` import plus a comment explaining it is load-bearing for Claude Code CLIs without native AGENTS.md support. The same pair exists in `manager/local/`. +- `update.sh` copies both names in its core-owned file list, with a comment stating why: on an old-layout install, `AGENTS.md` arrives and the pointer *replaces* the old full `CLAUDE.md`, so no vendor-named brief is resurrected or shadows the real one. +- All four prompts (`manager/core/prompts/work.md`, `review.md`, `review-pr.md`, `act-pr.md`), `README.md`, the `manager/core/board.py` and `manager/core/taskfiles.py` docstrings, and `tasks/task-template.md` (three "repo CLAUDE.md" mentions — same naming debt, so included) now say AGENTS.md. `README.md`'s install section gained the promised line: the brief is `AGENTS.md`, `CLAUDE.md` a compatibility pointer, both inside `.task-manager/` so a host repo's own root AGENTS.md is untouched. +- `manager/core/adapters/README.md` documents in the `run` contract that vendors reading `AGENTS.md` from the working tree natively (opencode, current Claude Code) pick it up in every worktree with no adapter work. I confirmed the opencode adapter runs `opencode run` with the worktree as cwd, so no adapter change was needed. + +How it was verified: + +- New `tests/test_update_round_trip.py` builds an old-layout install (full vendor-named `CLAUDE.md`, no `AGENTS.md`) and a scratch distribution repo from this repo's real top-level files, runs the real `update.sh` as a subprocess, and asserts the brief arrives as `AGENTS.md`, the pointer replaces the old full copy, and `manager/local/` and `tasks/` are untouched. `python3 -m unittest discover -s tests`: 78 tests, all pass. +- The acceptance grep across core, prompts, scripts, README returns only the pointer files' own copy lines and deliberate compatibility mentions. + +For the reviewer: + +- One acceptance item is only partially verifiable from inside this run: "a Claude Code session still loads the full brief". This session itself proves the pre-rename load; the pointer uses Claude Code's standard `@path` import, but the sandbox denied `claude --version`, so please open one fresh interactive session in the repo and confirm the brief is in context. +- First files to look at: `update.sh` (the round-trip guarantee), `CLAUDE.md` (the pointer wording), and the new test. diff --git a/tasks/archive/14-work-agents-branch-from-latest-origin-main.md b/tasks/archive/14-work-agents-branch-from-latest-origin-main.md new file mode 100644 index 0000000..3b82871 --- /dev/null +++ b/tasks/archive/14-work-agents-branch-from-latest-origin-main.md @@ -0,0 +1,136 @@ +# 14 — Work agents branch from the latest origin/main, not from whatever HEAD is + +**Status:** Archived +**PR:** https://github.com/12vectors/bench/pull/7 +**Priority:** Medium — invisible today because one person pushes from one machine; first teammate or GitHub-side merge makes it bite +**Type:** Feature + +A fresh work launch creates the task branch from the main checkout's +local `HEAD`. Nothing fetches first, so the agent builds on the board +machine's possibly-stale view of the world: anything merged on GitHub, +pushed by a teammate, or landed by act-pr from elsewhere is missing, +and the resulting PR is born conflicted or re-solves solved problems. +Starting work should mean starting from the newest main that exists. + +## Context + +- `manager/core/agents.py:150-176` — the worktree setup. + Fresh branch: `base = rev-parse HEAD`, `worktree add -b` from it + (`:175-176`). No `fetch` anywhere in core. +- The board already treats origin as the source of truth on the *other* + side of the flow: PR opening refuses when local main is ahead of the + remote, and merge & clean-up pushes after merging. Launching is the + one leg that never looks at origin. +- Continuing paths (existing worktree `:158-168`, existing branch + `:171-173`) resume prior work — their base is history, not a choice, + so they are unaffected; freshness for an in-flight branch is act-pr's + problem, prompted by reviews, not the launcher's. +- Bench must keep working remoteless: `git remote` may list nothing, + and the network may be down. Today's behaviour is the only correct + one in that world. + +**Affected areas:** `agents.py` (worktree setup) only; the ticker line +it emits. No adapter or prompt involvement. + +## What to build + +- Before a *fresh* `worktree add -b`: if a remote exists, `git fetch + origin main` with a short timeout; on success, base the new branch on + `origin/main` instead of `HEAD`. On no-remote, fetch failure, or + timeout: fall back to `HEAD` exactly as today — launching must never + be blocked by network weather. +- Never touch the main checkout itself: no fast-forwarding, no pulling + — the user may be sitting in it with uncommitted work. The fetched + ref is used only as the branch point. (Local main catches up + naturally at merge & clean-up time, which already merges and pushes.) +- Narrate honestly in the ticker: when the branch point is origin/main + and local main is behind it, say so ("branched from origin/main, + N ahead of this checkout"); when the fetch failed and HEAD was used, + say that too. Silent freshness is as bad as silent staleness. + +**Out of scope** — tempting neighbours left alone: + +- Rebasing or updating existing task branches (act-pr territory, + review-driven). +- Fast-forwarding the user's main checkout. +- Any remote other than `origin`, or configurable base branches — main + is the flow's spine everywhere else in core. + +## Acceptance + +- [ ] Given origin/main ahead of local main, when ▸ start work fires, + then the new worktree's merge-base with origin/main is + origin/main's tip, and the ticker names the branch point. +- [ ] Given no remote configured, when ▸ start work fires, then the + launch behaves byte-identically to today (base = HEAD, no fetch + attempted, nothing new in the ticker). +- [ ] Edge case — network down with a remote configured: launch + proceeds from HEAD after the timeout, ticker says the fetch was + skipped, and total launch delay stays within the timeout bound. +- [ ] Relaunch onto an existing worktree/branch is unchanged. + +## Open questions + +- None. + +## Notes + +Requested by the owner (2026-07-30) after watching origin advance +independently of the checkout during the first multi-lane day — board +merges pushing while other work was in flight made "which main did this +agent start from?" a live question for the first time. + +**Risks** + +- A fetch on every launch adds seconds and a network dependency to the + hottest path on the board — the timeout-and-fall-back behaviour is + the load-bearing part, not the fetch. +- Basing on origin/main while local main is behind means the worktree + contains commits the main checkout lacks; anything comparing the two + (diff stats in Focus read from the worktree — verify) must not + assume worktree ⊆ local main. + + +--- + +## Work report — 2026-07-30 07:27 (Juno) + +ing report. + +--- + +The work is committed on this branch (two commits, `4a91e5a` and `92b0c54`) and verified: the full suite — 56 tests, including 7 new ones for this behaviour — passes with `python3 -m unittest discover -s tests`. + +**What changed.** Fresh work launches now branch from the newest main that exists instead of the board checkout's HEAD. A new helper `_fresh_branch_point()` in `manager/core/agents.py` checks for an `origin` remote, fetches `origin/main` under a timeout, and hands `start_agent` the branch point plus a ticker note; the fresh-branch path then runs `git worktree add --no-track -b` from `origin/main` (no-track so the task branch doesn't adopt origin/main as its upstream, matching today's untracked branches). No remote, a failed fetch, a missing `origin/main` ref, or a timeout all fall back to branching from HEAD exactly as before — the launch is never blocked. The main checkout is never touched; the fetched ref is only the branch point. The timeout is a new setting, `BOARD_FETCH_TIMEOUT` (default 10s), added to `manager/core/config.py` and documented in `manager/core/.env.example`. The launch line in the ticker now narrates when it matters: "branched from origin/main, N ahead of this checkout" when local main is behind, and "fetch of origin/main timed out/failed; branched from local HEAD" when the network lost. An in-sync origin and the no-remote case stay quiet, as today. The canonical brief in `CLAUDE.md` was updated to stop saying fresh worktrees branch "from current HEAD". + +**How it was verified.** `tests/test_fresh_branch_point.py` exercises the helper against real git repositories: origin ahead by two commits yields `origin/main` with the "2 ahead" note and a worktree whose merge-base with origin/main is origin's tip; no remote and a non-origin remote return HEAD silently and fast; an unreachable origin falls back with the failure note; and a genuinely hanging fetch (simulated with git's `ext::` transport running `sleep`) returns within the timeout bound with the timed-out note. The continuing-worktree and existing-branch paths were not modified, and the pre-existing suites (empty-branch guard, adapter permissions, self-hosting) still pass. + +For the reviewer: + +- Read `manager/core/agents.py` first — `_fresh_branch_point()` and the fresh-branch arm of `start_agent` are the whole change. +- Run `python3 -m unittest discover -s tests` to confirm (the hanging-fetch test adds ~1s). + +One interaction worth knowing, left as-is deliberately: a *relaunch* onto an existing worktree still computes its diff base as `merge-base(main, branch)`. If the branch was born from an origin/main that local main hadn't caught up to, a relaunch's Focus diff stats will include those origin commits until local main catches up (which merge & clean-up already does). The acceptance criteria pin relaunch behaviour as unchanged, so I did not touch it — it is a display-accuracy nuance, not a correctness issue, but a fair candidate for a follow-up card. + + +--- + +## PR update — 2026-07-30 07:37 (Basil) + +ADDRESSED: The "N ahead" ticker count now compares origin/main against HEAD instead of the local main branch, so it stays accurate when the board runs from another branch. + +The fix is committed as `0111a9f` and pushed, so the PR is updated; the full suite — 57 tests, including one new one for this case — passes with `python3 -m unittest discover -s tests`. + +- **Copilot's line comment on `manager/core/agents.py` (the only review point):** the note "branched from origin/main, N ahead of this checkout" computed N as `rev-list --count main..origin/main`, which is only correct when the checkout is actually on `main` — from another branch or a detached HEAD the number could be wrong or misleading. Fixed by counting `HEAD..origin/main` instead. HEAD is what the fallback base would have been, so the count is now literally "what launching from this checkout would have missed", matching the note's wording regardless of which branch the board runs from. A new test, `test_ahead_count_is_relative_to_the_checkout_not_local_main` in `tests/test_fresh_branch_point.py`, pins the behaviour: a checkout already holding origin/main's tip on a side branch (with local main behind) now correctly gets no "ahead" narration, where the old count would have claimed "1 ahead". + +The review was a COMMENTED verdict (no approve/request-changes), and Copilot's PR overview raised nothing beyond that one line comment, so there is nothing else outstanding from this round. + + +--- + +## PR update — 2026-07-30 07:39 (Juno) + +ADDRESSED: Nothing new to change — the sole review point (Copilot's "ahead" count) was already fixed in pushed commit 0111a9f; verified in code and by the 57-test suite passing. + +- **Copilot's line comment on `manager/core/agents.py` (count "ahead" relative to the checkout, not local `main`):** already addressed before this round by commit `0111a9f`, which is on the remote branch, so the PR is current. I re-verified rather than assuming: `_fresh_branch_point()` now counts `HEAD..origin/main`, which is precisely "commits since the merge-base of the checkout and origin/main" — the semantics Copilot suggested — and the regression test `test_ahead_count_is_relative_to_the_checkout_not_local_main` (`tests/test_fresh_branch_point.py:101`) covers the misleading case (origin/main's tip already on a side branch, local main behind → no "ahead" note). Full suite: 57 tests, all passing. +- **No other feedback exists:** the review verdict was COMMENTED (no approve/request-changes), the PR overview raised nothing beyond that one line comment, and there are no issue comments or human reviews. Nothing was skipped or disagreed with; there was simply nothing else outstanding, so no new commits were needed. diff --git a/tasks/review/01-install-ships-pristine-board.md b/tasks/done/01-install-ships-pristine-board.md similarity index 77% rename from tasks/review/01-install-ships-pristine-board.md rename to tasks/done/01-install-ships-pristine-board.md index ed9c597..85a57b3 100644 --- a/tasks/review/01-install-ships-pristine-board.md +++ b/tasks/done/01-install-ships-pristine-board.md @@ -1,6 +1,6 @@ # 01 — Ship a pristine board on install, not bench's own cards -**Status:** Review +**Status:** Done **PR:** https://github.com/12vectors/bench/pull/5 **Priority:** High — every install made after we start dogfooding inherits our cards **Type:** Bug @@ -92,3 +92,17 @@ The fix is implemented, committed (`44537ae`), and verified: the full test suite - Start with `first_boot_leftovers` and `first_boot_clean` in `install.py` — the guard logic and the stamp ordering are the whole risk surface. - Run `python3 -m unittest discover -s tests` to reproduce the green run. - One acceptance step I could not run literally: the sandbox denies executing `install.py` directly in this checkout, so "bench itself is untouched" is proven by `test_self_hosted_repo_is_never_cleaned` (which builds a git-initialised self-hosted layout) rather than by a live run here. The scratch-host tests cover the clone → first-run → second-run sequence the other acceptance points describe. + + +--- + +## PR update — 2026-07-30 07:22 (Cleo) + +ADDRESSED: All four Copilot review points fixed — symlink-safe removal, stamp gated on the first-boot guard, and hermetic tests with two new assertions/cases; 55 tests pass. + +- **Symlink deletion could follow a link into `rmtree` (install.py:94).** Fixed: removal now calls `shutil.rmtree` only for real directories (`is_dir() and not is_symlink()`) and unlinks everything else, so a symlinked leftover is removed as a link and its target survives. A new test, `test_symlinked_leftover_is_unlinked_not_followed` in `tests/test_install_first_boot.py`, proves a directory pointed to from `tasks/backlog/` is left intact. +- **The `local/state/` stamp was created even when the guard was disarmed (install.py:96–97).** Fixed by restructuring: the guard is now its own predicate, `first_boot()`, computed once (with `resolve()` for the self-hosted comparison, as the reviewer asked) and gating both the clean and the stamp. A run where `local/.env` already exists, or a self-hosted repo, now touches nothing at all — not even the stamp directory. `first_boot_leftovers()` shrank to just the listing. +- **`test_existing_env_file_disarms_the_guard` didn't assert the stamp stays absent.** Added `assertFalse((local / "state").exists())` to that test; it fails against the old behaviour and passes now. +- **`run_install()` leaked the caller's `BOARD_*` environment into the subprocess.** Fixed: it now filters `BOARD_*` variables out of the environment, matching the existing pattern in `tests/test_self_hosting.py`, so a CI- or user-set `BOARD_AGENT_ADAPTER` can't redirect the test to an adapter that isn't in the scratch host. + +To verify: `python3 -m unittest discover -s tests` — 55 tests, all passing (54 before, plus the new symlink case). diff --git a/tasks/done/15-distribute-bench-as-packaged-releases.md b/tasks/done/15-distribute-bench-as-packaged-releases.md new file mode 100644 index 0000000..beef081 --- /dev/null +++ b/tasks/done/15-distribute-bench-as-packaged-releases.md @@ -0,0 +1,203 @@ +# 15 — Distribute bench as packaged releases; update.sh consumes them + +**Status:** Done +**PR:** https://github.com/12vectors/bench/pull/11 +**Priority:** Medium — correctness by construction for every future adopter; urgency rises with the first real second install +**Type:** Feature +**Depends on:** 13 — soft: the release manifest names top-level files, and 13 renames the brief; land 13 first or the manifest churns immediately + +This repo is two things at once: bench-the-project (its own cards, its +own `local/checks`, its own wiring) and bench-the-distribution. Install +and update currently conflate them by cloning the repo, then subtracting +what shouldn't have come along (card 01's first-boot cleaning). Invert +it: build a curated release artifact that never contained bench's own +state in the first place, publish it as a GitHub Release, and point +`update.sh` (and first-time install) at releases. No release published → +a clear message, not a silent fallback. + +## Context + +- `update.sh:33` — updates today are `git clone --depth 1` of + `BENCH_SOURCE`, then rsync of `manager/core/` plus a hardcoded + top-level file list (`:40`). The clone carries everything; curation + happens by picking files out of it. +- Install (README "Install into a repo") is the same clone with + `rm -rf .git` — which is why `../done/01-install-ships-pristine-board.md` + had to teach `install.py` to scrub bench's cards on first boot. That + cleaning works, but it is subtraction after the fact; a release is + correct by construction. (01's cleaner stays as defense-in-depth for + people who install from a clone anyway — developers will.) +- What must NOT ship, present in the repo today: bench's task cards, + `manager/local/` content (`checks`, workflow notes — the project's + half, actively diverging from a neutral starter), `local/state/`, + `.claude/`, `tests/`, `.worktrees/`, plans/reference content. +- What a fresh install DOES need: `manager/core/`, the top-level + scripts and docs, `tasks/task-template.md` + empty stage dirs, and a + **starter `local/`** — empty `adapters/ commands/ driver/ prompts/`, + a stub notes file pointing at `core/.env.example`. First-time install + and update then share one artifact: unpack-as-`.task-manager/` the + first time, selective replace afterwards. +- `manager/core/VERSION` exists (currently `1`) and update.sh already + reports version transitions — releases give that number somewhere + real to live: tag `v`. + +**Affected areas:** `update.sh` (rewrite), README install section, a new +release-builder script at top level; `install.py` untouched (wiring is +orthogonal); core code untouched. + +## What to build + +- **A manifest, one place**: the list of what ships. The release + builder reads it; a test asserts the artifact contains exactly the + manifest and none of the excluded classes above. Today update.sh's + hardcoded file list is that manifest in disguise — promote it. +- **`release.sh`** (top level, core-owned): builds the tarball from the + manifest at `manager/core/VERSION`, tags `v`, publishes via + `gh release create` with the tarball as the asset. Refuses if the tag + exists or the tree is dirty. Stdlib/stock-tools only (tar, gh). +- **`update.sh` rewritten**: resolve the latest release of + `BENCH_SOURCE` (or `BENCH_REF` as an exact tag) via `gh release + download` with a curl fallback for tokenless use; unpack; replace + `manager/core/` wholesale and the manifest's top-level files; touch + nothing else — same survival guarantees as today (tasks/, local/, + .env, state), now also guaranteed by the artifact's own contents. + No release found → exit with a message naming the repo, saying no + release is published yet, and pointing developers at cloning as the + dev-mode alternative. No silent git fallback. +- **README**: the first instruction becomes one tokenless command + against GitHub's stable latest-release URL: + + mkdir .task-manager && curl -L \ + https://github.com///releases/latest/download/bench.tar.gz \ + | tar -xz -C .task-manager + ./.task-manager/start.sh + + Two obligations fall out of that one-liner and belong to release.sh: + the asset name is stable across releases (`bench.tar.gz` — the + `releases/latest/download/` URL depends on it), and the artifact's + contents sit at the tarball root (no version-named wrapper directory, + or the extraction lands one level too deep). The clone path moves to + a "working on bench itself" note. +- **Stamp the source at build time**: release.sh knows the repo it + publishes from; bake that origin into the artifact (a default inside + the shipped update.sh) so a fresh install can update on day 30 with + zero configuration. `BENCH_SOURCE` in `local/.env` remains as the + override, no longer a prerequisite — today it is required and + update.sh errors without it. + +**Out of scope** — tempting neighbours left alone: + +- Auto-update checks or the board nagging about new versions. +- Semantic versioning, changelogs, release notes automation — tag = + VERSION, notes are whatever the releaser writes. +- Removing 01's first-boot cleaning — it stays as the clone-install + safety net. +- Migration logic between core versions (nothing versioned enough to + migrate yet). + +## Acceptance + +- [ ] The README one-liner, run verbatim in a scratch repo against a + real published release, yields a working `.task-manager/` — files + at the right depth, `start.sh` serves, and `update.sh` works with + no `BENCH_SOURCE` configured (the stamped default). +- [ ] Given a published release, when `update.sh` runs in an installed + project, then core/ and manifest top-level files match the + release, and tasks/, `manager/local/`, `.env` and state survive + byte-identical. +- [ ] Given NO published release, when `update.sh` runs, then it exits + non-zero with the no-release-yet message and changes nothing. +- [ ] The artifact test proves the tarball contains no task cards, no + `local/` content beyond the starter, no state/tests/.claude — + exactly the manifest. +- [ ] Unpacking a release as `.task-manager/` in a scratch repo and + running `start.sh` yields a working, empty board with no + first-boot scrubbing needed (01's cleaner finds nothing to do). +- [ ] Edge case — release asset present but VERSION inside disagrees + with the tag: update.sh refuses loudly rather than installing + ambiguity. +- [ ] `release.sh` refuses on dirty tree and on an already-published + tag. + +## Open questions + +- None. + +## Notes + +Owner's framing (2026-07-30): "do the work to package a clear release +(without the local tasks, adapters of bench itself) and the update +script updates from that. If there is no release it will let users +know." The first-time-install question resolves inside the same +artifact: the release ships the starter `local/`, so install = unpack + +start, and update = selective replace — one artifact, two verbs. + +**Risks** + +- Tokenless `gh`/API rate limits on the download path — the curl + fallback must work anonymously against public release assets, or + private-repo adopters need `gh auth` documented. +- The manifest drifting from reality as top-level files come and go + (13 renames the brief; future cards add scripts) — the + artifact-contents test is the tripwire, and release.sh failing dirty + keeps releases honest. +- Bench itself updates from its own releases only in dev-clone mode; + the self-hosted board must not "update" its own working tree from a + release and clobber in-flight work — update.sh's existing refusal to + touch tasks/local covers it, but verify against the self-hosted + layout where core/ IS the working tree. + + +--- + +## Work report — 2026-07-30 07:50 (Nell) + +ectories with `.gitkeep`), and `seed` (a generated starter `manager/local/CLAUDE.md` — never bench's own). +- `release.sh` (new, top level, not shipped in the artifact) — stages the manifest into `bench.tar.gz` with contents at the tarball root and a stable asset name (both required by the README's `releases/latest/download/` one-liner), stamps the origin repo into the shipped `update.sh` as `BENCH_SOURCE_DEFAULT`, refuses on a dirty tree or an existing tag, then tags `v` and publishes via `gh release create`. A `--tarball ` mode builds without publishing (used by tests). +- `update.sh` (rewritten) — resolves the source as environment > `local/.env` > build stamp, downloads the latest release (or `BENCH_REF` as an exact tag) via `gh` with an anonymous `curl` fallback, refuses when the asset's `VERSION` disagrees with its tag, replaces `manager/core/` wholesale plus the downloaded artifact's own `copy` list, and touches nothing else. No release published → non-zero exit, a message naming the repo and pointing developers at cloning; no silent git fallback. In an unstamped dev checkout with no `BENCH_SOURCE` it refuses with instructions. +- Docs — `README.md` install is now the tokenless `curl | tar` one-liner; the clone path moved to a "working on bench itself" section; `CLAUDE.md`'s Updating section and `manager/core/.env.example`'s `BENCH_SOURCE` comment describe the release flow. +- Tests — `tests/test_release_artifact.py` proves the tarball equals exactly the manifest (the drift tripwire), ships no cards/local-content/state/tests/`.claude`, sits at the root, carries the stamp, installs pristine (first boot finds nothing to scrub) and boots the board from an unpack; plus `release.sh`'s two publish refusals. `tests/test_update_from_release.py` runs `update.sh` hermetically against PATH-stubbed `gh`/`curl`: core+top-level replacement, byte-identical survival of tasks/`local`/`.env`/state, the no-release refusal changing nothing, the VERSION/tag-mismatch refusal, and the stamped-default/`BENCH_SOURCE`-override precedence. + +To do (reader actions): + +- After merging, cut the first real release: `./release.sh` from a clean checkout of `main`. The first acceptance box — the README one-liner against a *real published* release — can only be checked after that; everything up to the network is simulated in tests. +- Review first: `update.sh` (the whole flow is there) and the manifest. + +To know: + +- One environment fix worth a glance: macOS's bash 3.2 mis-parsed `$repo` followed by a literal `…` as one variable name, so `update.sh` uses `${repo}…` (`update.sh:111`). +- The `curl` fallback learns the latest tag from GitHub's `releases/latest` redirect (no API, no rate limit); private repos need `gh auth`, as the no-release message says. +- This worktree shows `release.sh` as modified only because my sandbox could not `chmod` the working-tree file; the committed mode is `100755` (set via `git add --chmod=+x`), so fresh checkouts get an executable script. + + +--- + +## PR update — 2026-07-30 07:57 (Nell) + +ADDRESSED: Path-safety guards added to update.sh, bare setUpClass asserts replaced with explicit checks; curl comment was a false positive, left unchanged. + +- **State of the work:** all points are resolved in commit e515c82, pushed to the PR; the full suite (92 tests, including two new refusal tests) passes. +- **Copilot line comment — `latest_tag_curl` "does not follow redirects, missing `-L`" (update.sh):** deliberately unchanged, because the claim is factually wrong. Both curl invocations already pass `-L` inside their combined flags — `curl -fsSLo` (`update.sh:79`) and `curl -fsSL -o` (`update.sh:103`) each expand to include `--location`, so `%{url_effective}` does resolve the `releases/latest` redirect. The hermetic tests exercise this path indirectly and it also works against real GitHub. +- **Copilot line comments — bare `assert` in `setUpClass` (tests/test_update_from_release.py, tests/test_release_artifact.py ×2):** fixed in all three places. The build-the-artifact check is now an explicit `if result.returncode != 0: raise RuntimeError(...)` with the script's output in the message, so the suites fail deterministically even under `python -O`. +- **Suppressed comment — tarball extracted without validating member paths (update.sh):** fixed. Before extraction, `update.sh` now lists the asset's members and refuses any absolute path or `..` component ("escape its own root"), exiting non-zero with nothing changed. Covered by the new `test_asset_with_escaping_member_paths_is_refused`. +- **Suppressed comment — manifest `copy` paths trusted in `cp`, no parent dirs (update.sh):** fixed. The manifest's copy paths are validated (no absolute paths, no `..`, no empty segments) *before* the core rsync, so a poisoned manifest refuses cleanly rather than after a partial update; the copy loop now also creates parent directories, so a future release adding a nested top-level file installs correctly. Covered by the new `test_unsafe_manifest_copy_path_is_refused`, which asserts the install is byte-identical after the refusal. +- To know: the worktree still shows `release.sh` as locally modified — that is the pre-existing mode-only sandbox artifact explained in the original work report (committed mode is 755), intentionally left uncommitted. + + +--- + +## PR update — 2026-07-30 08:09 (Nell) + +ADDRESSED: Verified every review point resolved in pushed commit e515c82; no new feedback since, all 92 tests pass, nothing further to change. + +- **State of the work:** the branch is fully pushed (local and remote heads both at e515c82) and the complete test suite — 92 tests, including the two path-safety refusal tests added for this review — passes. The only PR feedback is the single Copilot review submitted on commit 7a7ad39; commit e515c82 addressed it, and this pass verified each fix is actually present in the code rather than trusting the earlier report. No human reviews, no issue comments, and no new line comments exist on the PR. +- **Copilot line comment — `latest_tag_curl` "does not follow redirects, missing `-L`" (update.sh):** deliberately unchanged, because the claim is factually wrong. Both curl invocations already include `-L` inside their combined flags — `curl -fsSLo` (update.sh:79) and `curl -fsSL` (update.sh:103) — so the `releases/latest` redirect is followed and `%{url_effective}` resolves the tag. The comment's "also lines 121 and 142" refer to positions in the pre-e515c82 diff; no curl call exists at those lines in the current file. +- **Copilot line comments — bare `assert` in `setUpClass` (tests/test_update_from_release.py:63, tests/test_release_artifact.py:71 and :192):** confirmed fixed in all three places. Each now uses an explicit `if result.returncode != 0: raise RuntimeError(...)` carrying release.sh's output, so the suites fail deterministically even under `python -O`. +- **Suppressed comment — tarball extracted without validating member paths (update.sh):** confirmed fixed. Before extraction the asset's member list is checked for absolute paths or `..` components (update.sh:124) and the script refuses with nothing changed; covered by `test_asset_with_escaping_member_paths_is_refused`. +- **Suppressed comment — manifest `copy` paths trusted in `cp`, no parent dirs (update.sh):** confirmed fixed. Copy paths are validated for absolute/`..`/empty segments before the core rsync (update.sh:147-153), so a poisoned manifest refuses cleanly with no partial update, and the copy loop creates parent directories (update.sh:162); covered by `test_unsafe_manifest_copy_path_is_refused`. + +To know: + +- No new commit was made this pass — there was nothing left to change; verification only. +- The worktree still shows `release.sh` as modified; it is the known mode-only artifact (sandbox could not `chmod` the working-tree file). The committed mode is 100755, so fresh checkouts get an executable script. Intentionally left uncommitted. +- The card is ready for the next step: another **◔ review PR** pass, or merge. After merging, cut the first real release with `./release.sh` from a clean checkout of `main` — the README one-liner acceptance box can only be verified against a genuinely published release. diff --git a/tasks/done/16-conflicted-prs-first-class-with-an-agent-path.md b/tasks/done/16-conflicted-prs-first-class-with-an-agent-path.md new file mode 100644 index 0000000..ea37b5d --- /dev/null +++ b/tasks/done/16-conflicted-prs-first-class-with-an-agent-path.md @@ -0,0 +1,132 @@ +# 16 — Surface PR conflicts on the card and give act-on-PR the tools to resolve them + +**Status:** Done +**PR:** https://github.com/12vectors/bench/pull/12 +**Priority:** High — the first real conflict (PR #7, card 14) had no bench path at all; parallel lanes make the next one soon +**Type:** Feature + +A conflicted PR is invisible to the board and unactionable through it: +the poller doesn't read mergeable state, the act-pr prompt knows +nothing about stale branches, and the act-pr allowlist would deny +`git fetch`/`git merge` the moment an agent tried. Resolution is +currently entirely by hand. Make conflict a state the card wears, and +mechanical resolution a job ↻ act on PR can do — while teaching it to +refuse the conflicts that are really product decisions. + +## Context + +- Observed on PR #7 (`task/14-…`): GitHub says CONFLICTING/DIRTY; the + board said nothing; no action chip helps. +- `manager/core/github.py` — the poller folds reviews + CI into one + verdict; `mergeable`/`mergeStateStatus` (available via + `gh pr view --json`) is not fetched, so the card cannot know. +- `manager/core/prompts/act-pr.md` — entirely review-comment-driven; + no concept of the branch being behind or conflicted with main. +- `manager/core/adapters/claude/hook_settings.py:37-38` — act-pr + prefixes: add/commit/status/diff/push + gh pr view/diff/api. No + `git fetch`, no `git merge`: resolution is permission-denied today. + (The opencode adapter mirrors the same stances — change both.) +- Board merge & clean-up already aborts cleanly on conflict — correct, + and unchanged by this card; it just stops being the first time + anyone hears about the problem. +- `../review/14-work-agents-branch-from-latest-origin-main.md` reduces + how often fresh branches start stale; this card handles the + conflicts that parallel lanes produce anyway. + +**Affected areas:** `github.py` (poll mergeable), `board.html` (chip), +`prompts/act-pr.md`, both adapters' permission stances + their tests. + +## What to build + +- **See it**: poller fetches mergeable state with the fields it already + reads; a conflicted PR puts an alarm-coloured `conflicts` chip on the + card (a destination-style chip in the footer row, per the design + system) and folds into the card's verdict as changes-needed-by-you, + not as CI failure. +- **Arm the agent**: act-pr allowlist gains `git fetch` and `git merge` + (both adapters). Deliberately NOT `git rebase` and no force-push — + the branch is public; resolution must be additive. +- **Teach the prompt**: act-pr.md gains a conflicts section — if the PR + conflicts with main: fetch, `git merge origin/main`, resolve honouring + both sides' intent, run the project's tests until green, put the + resolution in its own commit whose message names the files and the + choices, push, and cover the resolution explicitly in the ADDRESSED + report. If both intents cannot hold simultaneously — main has made + the branch's premise false — resolve nothing: abort the merge, and + report that a human must decide, stating the specific collision. + Guessing at semantic conflicts is the one forbidden move. +- **Keep the human at the merge**: nothing auto-merges (unchanged); + the resolved PR flows into the existing ◔ review loop, which now also + judges the resolution commit. + +**Out of scope** — tempting neighbours left alone: + +- Auto-resolving on the board's own merge & clean-up path (it keeps + aborting cleanly; the fix is upstream, before you drag to done). +- Rebase workflows, force-push, and history rewriting of any kind. +- Proactively merging main into healthy, non-conflicted branches. + +## Acceptance + +- [ ] Given a PR GitHub marks CONFLICTING, when the poller next runs, + then the card wears the conflicts chip and drops any + approved-green state until resolved. +- [ ] Given a mechanically conflicted PR, when ↻ act on PR runs, then + the branch gains a merge-resolution commit, tests pass, the push + updates the PR to mergeable, and the ADDRESSED report names the + conflicted files and the choices made. +- [ ] Given a semantic conflict (a fixture branch whose premise main + has removed), when ↻ act on PR runs, then no resolution is + committed and the report says a human must decide, naming the + collision. +- [ ] Edge case — permission proof: the act-pr stance test shows + `git fetch`/`git merge` allowed, `git rebase` and force-push + absent, in both adapters. +- [ ] Merge & clean-up behaviour on conflict is unchanged. + +## Open questions + +- None. + +## Notes + +Prompted by PR #7 (card 14, 2026-07-30): the card about branching from +fresh main was itself the first stale-base conflict — it edits +`agents.py`, which cards 12 and others had just changed. Until this +card lands, conflicted PRs are hand-work in the task's worktree. + +**Risks** + +- An agent's "faithful to both intents" is a judgment call; the + guardrails (own commit, tests green, review loop judges it, human + merges) bound the blast radius, but a plausible-wrong resolution + surviving lazy review remains the real hazard. The refuse-on-semantic + rule in the prompt is the load-bearing mitigation — watch its + false-confidence rate in early use. +- `mergeable` from GitHub is computed lazily and can read UNKNOWN for + a while after pushes; the chip must tolerate the unknown state + without flapping between colours. + + +--- + +## Work report — 2026-07-30 07:58 (Sage) + +ed** + +- `manager/core/github.py` — the poller now asks `gh pr view` for `mergeable` alongside the fields it already read. A CONFLICTING PR drops any approved-green verdict as changes-needed-by-you (the CI chip is untouched — a conflict is not a CI failure), and the flip narrates in the ticker both ways. GitHub computes mergeability lazily, so an UNKNOWN reading keeps the previous state instead of flapping the chip. To make this testable without `gh`, the poll logic was extracted into a pure function `_fold(data, prev)`; `_poll_pr` now just fetches, folds, and narrates. +- `manager/core/board.html` — a conflicted PR puts an alarm-coloured `conflicts` chip in the card's footer row, beside CI/copilot/PR, per the design system's destination-chip pattern. +- Both adapters (`manager/core/adapters/claude/hook_settings.py`, `manager/core/adapters/opencode/permission_config.py`) — the act-pr stance gains `git fetch` and `git merge`. One thing worth a reviewer's attention: I went slightly beyond "absent" for force-push, because the existing `git push` allow already covers `git push --force` by prefix. Both adapters now carry explicit deny rules for the force spellings and `git rebase` — deny-beats-allow in the Claude settings, last-match-wins globs in the opencode config. Prefix rules can't catch every exotic flag placement; the prompt and review loop remain the guard there, as the code comments say. +- `manager/core/prompts/act-pr.md` — a conflicts section: fetch, `git merge origin/main`, honour both intents, tests green, the resolution in its own commit naming files and choices, covered explicitly in the report — and the refusal rule: if main has made the branch's premise false, `git merge --abort`, resolve nothing, and name the collision for a human. `manager/core/adapters/README.md` and the root `AGENTS.md` document the widened contract and the new chip. +- Merge & clean-up (`complete_task`) is untouched, per the task's out-of-scope list. + +**How it was verified** + +- New `tests/test_pr_conflicts.py`: conflict drops green even when approved, conflict ≠ CI failure, UNKNOWN keeps the previous reading both ways (no flapping), first-sight UNKNOWN alarms nobody, resolution lets green return, and the `conflicts` key reaches `public_state()` and the chip markup. +- New stance tests in `tests/test_adapter_permissions.py`: fetch/merge allowed in act-pr only, rebase and `--force` absent from every allowlist, the denies present in both adapters, and the opencode denies ordered after the push allow so they win. +- The live-agent acceptance criteria (a real mechanical resolution, a real semantic refusal) are exercised only by an actual ↻ act on PR run — worth watching on the first conflicted PR this meets. + +**To check first as a reviewer** + +- The deny-rule decision described above — it's the one place I interpreted the spec rather than transcribed it. +- The `_fold` extraction diff in `manager/core/github.py` (the logic move is mechanical, but it's the load-bearing change). diff --git a/tasks/done/17-card-actions-click-solid.md b/tasks/done/17-card-actions-click-solid.md new file mode 100644 index 0000000..7a25133 --- /dev/null +++ b/tasks/done/17-card-actions-click-solid.md @@ -0,0 +1,129 @@ +# 17 — Make card actions click-solid: stable targets, instant feedback + +**Status:** Done +**PR:** https://github.com/12vectors/bench/pull/13 +**Priority:** Medium — the actions are the board's hands; every launch goes through this wobble +**Type:** Bug + +The hover actions (▸ start work, ‖ hold, ◔ still true?, …) are easy to +miss and mute when hit. Two root causes, both in `board.html`: the +click target is small and *changes geometry mid-interaction* (appears +on hover with a slide animation, then the arm step swaps the label to a +different-width string under the cursor), and firing gives zero +feedback until the server's SSE update arrives a second or more later — +so the click reads as "did nothing", inviting a double-fire the server +then refuses. + +## Context + +- `board.html:176-183` — `.hoveracts button`: `padding:2px 8px`, + 11px font → ~20px hit target. +- `board.html:170-171` — the slot is `display:none` until `.card:hover` + and enters with `animation:rise .12s` — the target is moving as the + pointer arrives. +- `board.html:976-985` — the arm handler replaces the label with + `act.confirm` ("start work" → "start it?"): different text width, so + the button reshapes between first and second click; a missed second + click bubbles to the card and opens the sheet. The 3.5s disarm timer + means a slow second click silently re-arms instead of firing. +- `act.run()` (same block) fires the POST with no state change; the + card redraws only when the server's SSE lands (worktree + spawn ≈ + seconds for start-work). Double-clicks in the gap hit the server's + one-agent-per-task refusal and surface as an error toast for a click + the user thinks failed. +- Design-system vocabulary already exists for the fix: breathe = agent + alive, `--alarm` = armed, and actions already own the pill's slot. + +**Affected areas:** `board.html` only (CSS + the action-slot builder); +no server change — the server's refusals stay as the backstop, they +just stop being reachable by honest clicking. + +## What to build + +- **Stable geometry.** Reserve each button's width for the wider of + label/confirm (measure both, fix `min-width`), so arming restyles but + never reshapes. Hit target to ≥24px tall (padding, not font size). + Drop the entry animation or animate opacity only — the target must + not travel while the pointer approaches it. +- **Unmissable armed state.** Keep the alarm colour, add the remaining + window visibly (a subtle draining underline/边 on the 3.5s timer — + and lengthen it to ~5s). A second click after expiry re-arms, which + is correct, but the user must *see* the state they are clicking in. +- **Instant busy state on fire.** The moment `act.run()` is called the + button locks: disabled, breathe animation, label → present participle + ("starting…", "holding…", "checking…"), clicks swallowed. It stays + locked until the SSE redraw replaces the card, or an N-second + timeout restores it with an error toast naming the action — never a + silent revert. +- **Misses fall harmlessly.** While the slot is visible, clicks in its + padding/gap zone must not bubble into card selection — pad the slot's + hitbox, stopPropagation at the slot, not per-button. + +**Out of scope** — tempting neighbours left alone: + +- Changing the arm-then-fire model itself (it guards token spend and is + right); this card makes its states legible, not optional. +- Server-side idempotency/debouncing of launch requests. +- Touch support, keyboard activation of hover actions. + +## Acceptance + +- [ ] Given an unarmed action, when clicked twice briskly, then exactly + one launch happens and the button visibly walked unarmed → armed + → busy with no geometry change throughout. +- [ ] The armed button shows its remaining window; after expiry it + reads as unarmed again (visual state, not just internal class). +- [ ] Given a fired action, when the server takes seconds, then the + button reads busy immediately (breathe, participle label) and + resolves into the card's new state without a flash of the old. +- [ ] Edge case — server error or timeout on fire: button returns to + rest and a toast names the failed action; no stuck busy state. +- [ ] Edge case — click landing in the slot but between buttons: + nothing happens; the card sheet does not open. +- [ ] All existing actions (start work, hold, back, reopen, still + true?, review PR, act on PR, copilot, local commands) inherit the + behaviour from the one slot builder — no per-action forks. + +## Open questions + +- None. + +## Notes + +Owner's report (2026-07-30): "easy to miss a click" and "takes a second +for bench to react so it's not clear if you clicked" — both reproduced +from the code paths above; the width-swap on arming is the likely main +miss-click culprit, and the feedback gap is structural (no optimistic +state at all between click and SSE). + +**Risks** + +- The busy state is client-side optimism: if the SSE connection is + dead the timeout-and-toast path is the only honest exit — test it + with the server stopped, not just slow. +- Local commands arm through a separate handler today + (`board.html:961-965`); unifying into the slot builder must not + change their arm/run contract. + + +--- + +## Work report — 2026-07-30 08:03 (Hazel) + + + action, because the board tears cards down and rebuilds them on every SSE render: a rebuild mid-window now re-applies the same armed or busy picture instead of silently disarming, and a second click keeps its meaning across a redraw. + +- **Stable geometry.** Each button renders its rest, confirm and busy labels stacked in one CSS grid cell (`.actlbl`), so the button is born as wide as its widest state and arming restyles without reshaping. The slot now fades in with an opacity-only animation instead of sliding up, buttons are 24px tall, and the card's top row permanently reserves that height so hovering never grows the card or shifts its neighbours. +- **Visible armed window.** The window is lengthened from 3.5s to 5s and wears a draining underline animated over exactly that duration; a button rebuilt mid-window rejoins the drain partway through via a negative animation delay rather than restarting it. +- **Instant busy state.** Firing immediately disables the button, breathes the glyph in accent colour, and shows a present-participle label ("starting…", "holding…", "checking…", …). It unlocks when the SSE redraw replaces the card, when the request helper reports failure (they all now return success/failure), or when a 15s timeout restores it with an error toast naming the action. A thrown fetch (server down, dead SSE) is caught and toasted the same way — no silent revert, no stuck busy state. +- **Misses fall harmlessly.** The slot pads its own hitbox with padding cancelled by negative margin and swallows clicks itself (`stopPropagation` at the slot, not per button), so a click in the gap between buttons neither fires anything nor opens the card sheet. + +**To do (reviewer):** +- Run `python3 -m unittest discover -s tests` to confirm the suite. +- Eyeball the arm→busy walk in a live board: hover an in-progress card, click "start work" twice briskly, and watch unarmed → armed (draining underline) → "starting…" with no width or position change. +- For the honest-exit path, stop the board server mid-session and fire an action: the button should return to rest after 15s with a toast naming the action. + +**To know:** +- The commit deliberately does not change the arm-then-fire model, server-side idempotency, or touch/keyboard support — all named out of scope by the task. +- The local-command chips keep their exact arm/run contract (first click arms "run it?", second runs) but now inherit the drain bar, the busy lock and the 5s window from the shared machine; their old 3.5s bespoke handler is gone. +- Because the tests are source-level regex invariants (this repo's convention for `board.html`), they pin the contract — label ghost-stack, matching CSS/JS timer durations, slot-level click guard, `return res.ok` in every request helper — rather than executing the JS; the second "to do" item above is the behavioural check. +- One visual side effect worth knowing: every card's top row is 4px taller than before (20px → 24px), the price of a hit target that never moves.