diff --git a/docs/data-model.md b/docs/data-model.md index e6863f94..9161d154 100644 --- a/docs/data-model.md +++ b/docs/data-model.md @@ -13,7 +13,7 @@ Three directories, different owners: - **Project** (`projects.json`, single JSON array) — git repo path (which need NOT be the repository ROOT: pointing termic at `packages/app` of a monorepo makes that directory the project, and git then phrases its own paths differently from termic's — see [gotchas.md](gotchas.md) "git speaks repo-root paths") + scripts + `preview_url` template + `preview_browser` (GH #245, an `Option` **on purpose**: absent = follow the global `Settings.preview_browser`, `Some("")` = force the OS default for this project even when the global names a browser, `Some(cmd)` = override. A plain `String` cannot express that middle state, since empty is already spoken for by "inherit" — which is why `tasks_path`, the other project override, gets away with being one. Personal, never `.termic.yaml`: a launch command is machine-specific, so a committed `open -a "Google Chrome"` would be a silently dead link for a teammate on Linux, whereas a `preview_url` is portable) + `files_to_copy` globs (personal list wins when non-empty, else the repo's committed `.termic.yaml` one — `effective_files_to_copy`) + `default_cli` + `extra_named_ports` (personal env-var-name list for GH #196, unioned with the repo's committed `.termic.yaml` `extra_named_ports`; yaml order first, deduped, invalid/reserved names dropped — see `effective_extra_named_ports`) + optional `group` label (UI-only collapsible folder in the sidebar; no filesystem effect; a group exists iff ≥1 project carries the label. All group reads go through `groupOf()` in `src/lib/projectGroups.ts`, THE normalization point: trim + ALL-CAPS, so mixed-case labels on disk converge to one group. Collapse state + folder color live in `localStorage` keyed by normalized name, pruned when a group disappears). - **ProjectMember** (inline in `projects.json`, multi-repo projects only) — one repo mounted inside every task under a multi-repo project. Self-contained (`root_path` + `name` + `base_branch`), never a reference to a registered Project. Carries its own `setup_script` / `run_script` / `archive_script` and its own `files_to_copy` globs, all with the same resolution rule: the value here wins when non-empty, otherwise **that member repo's OWN committed `.termic.yaml`** (`member_effective_script`, `member_effective_files_to_copy`). Which gitignored files a repo needs is a property of that repo, which is why the list sits here and not on the host — the host project's own `files_to_copy` covers the task ROOT (the host worktree) and nothing else. Frozen onto each task's `composition` at create (`TaskMember`), so editing a member only affects future tasks. The copy runs for worktree members only: a repo-root member IS the live checkout and already holds its files. GH #264. - **Profile** (`profiles.json`, GH #280) — `slug` (frozen at creation; keys the data dir, the worktrees base AND the window label, so a rename never touches it), `name`, `accent` (a palette KEY from `src/lib/accents.ts`, not a hex), `order`, `last_focused_at` (Chrome's tie-break for a project living in several profiles), `open_at_quit` (launch restore). `Registry.root_slug` names which profile owns the app data dir itself, and may be `None` once that profile is deleted: nothing is promoted and nothing moves. **Task and Project each carry an in-memory `profile` tag** (`#[serde(skip)]`, so nothing reaches disk and there is no schema bump) derived from the directory the record was read from — that tag is the whole mechanism, see [profiles.md](profiles.md). -- **Task** (`tasks/.json`) — git worktree branched from project's `base_branch`. Worktrees live at `~/termic/tasks///` by default (configurable per project and globally; a non-root profile is seeded with `~/termic/profiles//tasks`). `is_main_checkout=true` tasks point at the project's live checkout (no worktree, archive skips `rm -rf`). `agent_args` is the ordered per-task argv created by `termic new --arg/--model`; it is appended after the selected agent's Settings args on every default-tab spawn and resume, but never reaches secondary or different-agent tabs. Optional `order` holds the sidebar position within the project, written by drag-to-reorder (`task_reorder`). Projects get their order from the `projects.json` array; tasks are a file each, so they need the explicit key. `load_tasks` sorts on `(order, created)` with a missing `order` LAST, which is why a project nobody has dragged still reads oldest-first and a new task appends at the bottom of a reordered one. Each task also owns a consecutive **port block** (GH #196), allocated at create by `allocate_task_ports`: `port` ($TERMIC_PORT) + one port per composition member (base+1+i) + `extra_named_ports` (frozen name→port pairs from the project's effective list, injected wherever TERMIC_PORT is set and expanded in the preview URL) + a 5-port buffer. The block length is stored on the task (`port_block_len`) at allocation; blocks first-fit over non-archived tasks from the bottom of the configured port range (`task_port_min`/`task_port_max`, default 18100-65535, GH #271; archived blocks are reused; restoring re-homes a block another task claimed meanwhile). Occupancy means "another task owns it", never "the OS says it is free": termic does not probe, so picking a range nothing else on the machine uses is the user's call, and a server started on a port something else already holds fails to bind in its own run tab. A range with no room left fails the allocation loudly at task create; `top_up_extra_ports` instead logs and keeps the pairs it has, because failing a spawn over one missing named port would be worse. Note `PORT_ALLOC_MIN` (1024) is the "this record predates port blocks" sentinel and is deliberately NOT the configurable floor: sharing them meant raising the floor above an existing task made that task's block invisible to every occupancy scan. Every load-occupancy→allocate→persist sequence holds `PORT_ALLOC_LOCK`, so concurrent creates / restores / top-ups can't scan the same snapshot and claim the same ports. This replaced the old `18100 + task count` formula, which could collide with multi-repo member ports. Names added to the config LATER reach existing tasks lazily: every tab spawn / run-script launch calls `top_up_extra_ports`, which freezes missing names into the task's buffer slots, overflowing to the next free single port anywhere once the buffer is full (`task_port_intervals` counts those strays as occupied for all later allocations; a restore re-home re-compacts them into a fresh contiguous block). Frozen pairs never move; names removed from the config keep injecting. Pre-existing tasks deserialize with an empty pair list and pick names up the same way. +- **Task** (`tasks/.json`) — git worktree branched from project's `base_branch`. Worktrees live at `~/termic/tasks///` by default (configurable per project and globally; a non-root profile is seeded with `~/termic/profiles//tasks`). `is_main_checkout=true` tasks point at the project's live checkout (no worktree, archive skips `rm -rf`). `agent_args` is the ordered per-task argv created by `termic new --arg/--model`; it is appended after the selected agent's Settings args on every default-tab spawn and resume, but never reaches secondary or different-agent tabs. `last_opened_at` is an RFC3339 UTC stamp written by the app itself every time the user activates the task (`setActiveTask` -> `task_touch`, at most once a minute per task), never typed by a person and `None` on records written before the field existed, which is why anything rendering an age shows nothing rather than falling back to `created`. `started_at` is the RFC3339 UTC stamp of the FIRST prompt the user submitted into any of the task's terminals (`task_mark_started`, fired from the same input paths that arm the work-state detector); it is write-once, never cleared, and it is what separates "created, agent idling at its prompt" from "someone gave it work", since spawning alone cannot draw that line when every GUI create spawns. Records written before the field existed are backfilled once at startup (`migrate_started_at_backfill`, guarded by `Settings.data_migration_version` rather than `schema_version`, which would re-run the whole workspaces->tasks migration): a task with `spawn_count > 0` or `has_resumable_history` takes `last_opened_at`, falling back to `created`. `base_sha` is the commit the task's branch was cut from, written by every site that actually cuts that branch and nowhere else: `task_create_sync`, the host repo in `task_create_multi_sync`, and `restore_task_branch` when an archive-with-`delete_branch` removed the branch and restore cuts a new one, all three rev-parsing the resolved base immediately before `git branch --no-track`. The restore case REWRITES the value, because the recreated branch genuinely starts from wherever the base is now, and keeping the old sha would have `task_git_phase_state` measure against a base that was never this branch's. It is deliberately `None` when an existing branch is reused, and on `task_import_worktree` and `task_open_repo`: those adopt a branch that predates the task, and recording the base's CURRENT sha for one of them would invent a creation point that never existed. A restore that finds its branch intact leaves the stored value alone for the same reason. `task_git_phase_state` reads it, falling back to the branch reflog's creation entry, and once that reflog expires (git's default `gc.reflogExpire` is 90 days) the creation commit is simply unknown. Optional `order` holds the sidebar position within the project, written by drag-to-reorder (`task_reorder`). `goal`, `parked_at` and `park_reason` are the ONE manual half of the phase, and the only fields on this record a person types: everything else the phase reads is derived from a live signal, and these two facts have no live twin anywhere in git, in the forge or in the agent's state. `goal` is what the task is for, in the user's words (`task_set_goal`). It is also what makes noting a task down for later possible at all, since the only place to write an intention before was the agent's prompt box and submitting that stamps `started_at`; a task with a goal and no `started_at` reads as Planned. `parked_at` (RFC3339 UTC) says the user deliberately put the task down and `park_reason` optionally says why, both written by `task_set_parked`, which returns the resulting `parked_at` so the caller stores the real stamp rather than its own guess. There is no Blocked state: "blocked on the API key" is a parked task with a reason, one state with an optional note instead of two that differ only in why. Four rules keep the manual half from going stale. All three are stored TRIMMED and `None` rather than `""`, so clearing a box removes the field instead of leaving a value that renders as nothing. A re-park does NOT move `parked_at`, because that stamp is what "parked 3 days ago" is rendered from and editing the reason is the common way to park twice. The reason is editable without unparking, and unparking clears BOTH fields, since a reason for a park that is over is not a fact about anything. And `task_mark_started` clears the park on every call: submitting a prompt into a parked task means the user picked it back up, so the one flag a human sets is also the one thing the app un-sets for them. That is what stopped `task_mark_started` being write-once as a COMMAND, though the `started_at` stamp it writes still is. All three fields are absent on every record written before they existed, which is the correct answer for all of them (no goal, not parked), so there is no migration, unlike `started_at`. Projects get their order from the `projects.json` array; tasks are a file each, so they need the explicit key. `load_tasks` sorts on `(order, created)` with a missing `order` LAST, which is why a project nobody has dragged still reads oldest-first and a new task appends at the bottom of a reordered one. Each task also owns a consecutive **port block** (GH #196), allocated at create by `allocate_task_ports`: `port` ($TERMIC_PORT) + one port per composition member (base+1+i) + `extra_named_ports` (frozen name→port pairs from the project's effective list, injected wherever TERMIC_PORT is set and expanded in the preview URL) + a 5-port buffer. The block length is stored on the task (`port_block_len`) at allocation; blocks first-fit over non-archived tasks from the bottom of the configured port range (`task_port_min`/`task_port_max`, default 18100-65535, GH #271; archived blocks are reused; restoring re-homes a block another task claimed meanwhile). Occupancy means "another task owns it", never "the OS says it is free": termic does not probe, so picking a range nothing else on the machine uses is the user's call, and a server started on a port something else already holds fails to bind in its own run tab. A range with no room left fails the allocation loudly at task create; `top_up_extra_ports` instead logs and keeps the pairs it has, because failing a spawn over one missing named port would be worse. Note `PORT_ALLOC_MIN` (1024) is the "this record predates port blocks" sentinel and is deliberately NOT the configurable floor: sharing them meant raising the floor above an existing task made that task's block invisible to every occupancy scan. Every load-occupancy→allocate→persist sequence holds `PORT_ALLOC_LOCK`, so concurrent creates / restores / top-ups can't scan the same snapshot and claim the same ports. This replaced the old `18100 + task count` formula, which could collide with multi-repo member ports. Names added to the config LATER reach existing tasks lazily: every tab spawn / run-script launch calls `top_up_extra_ports`, which freezes missing names into the task's buffer slots, overflowing to the next free single port anywhere once the buffer is full (`task_port_intervals` counts those strays as occupied for all later allocations; a restore re-home re-compacts them into a fresh contiguous block). Frozen pairs never move; names removed from the config keep injecting. Pre-existing tasks deserialize with an empty pair list and pick names up the same way. - **Agent accounts** (on the agent entry in `settings.json`, GH #278) — `accounts` (names, in the order added), `default_account` (what new tasks use), `adopted_account` (the one that IS the agent's pre-existing login and therefore relocates NOTHING). Profile-scoped for free, since `settings.agents` is. `Task.accounts` (agent id -> account name) is the per-task override a switch writes; absent means "follow the agent's default". The login STORES are global and keyed by NAME (`logins///`, `docker-agents///`), so two profiles using the same name share one login. See [agent-accounts.md](agent-accounts.md). - **Settings** (`settings.json`) — `preview_browser` (GH #245: app-wide command template that opens preview URLs and terminal links; empty = OS default), `repos_dir`, `welcomed`, `agents[]` (claude/gemini/codex defaults + customs; each has `command`/`args`/`yolo_args`/`runtime_yolo_command`). Defaults seeded if `agents` is empty. `schema_version` gates one-time on-disk migrations. `task_port_min` / `task_port_max` (GH #271) are the window port blocks are allocated from; 0 on either means the default 18100-65535, so read the pair through `PortRange::from_settings` (Rust) or `resolvePortRange` (`src/lib/portRange.ts`), never raw. - **Scratchpad** (`scratch//index.json` + `scratch//.txt`, GH #244) — an untitled buffer that survives a relaunch, scoped to ONE task. Stored here rather than in the worktree so it never appears in `git status`, in the agent's review diff, or in a commit. The index record (`id`, `title`, `syntax`, `order`, `created_at`, `updated_at`) exists because a pad has no filename to re-derive a title or syntax from, and one index read beats stat-ing N files on launch. Pads are NOT part of `persisted_tabs`, which is agent-tabs-only by construction; they restore from this index when their task is first entered. diff --git a/docs/e2e-coverage.md b/docs/e2e-coverage.md index eba7e1ca..5ac1056b 100644 --- a/docs/e2e-coverage.md +++ b/docs/e2e-coverage.md @@ -185,6 +185,12 @@ until `make e2e` is green and this file reflects it. | ✅ Dashboard groups | A project group renders as a folder with its member cards INSIDE it (not merely adjacent) and a membership count; collapsing on the dashboard collapses the sidebar folder and expanding from the sidebar re-opens the dashboard one, since both read `collapsedGroups`; a group typed as "Infrastructure" renders, collapses and shares state under the NORMALIZED "INFRASTRUCTURE" on both surfaces, which is the sharing claim itself and which an all-caps fixture name cannot prove; a folder holding an active task floats above an idle section without reordering its own members | `projects.e2e.ts` | | ✅ Dashboard live signals | A task row carries the same work badge the sidebar does, with the same precedence (a seeded `done` shows, a later attention outranks it, clearing both removes it), read through `dashboardBadge()` because `work-badge` is no longer unique on the page; the PR chip is absent until the pr store holds a lookup and then reports its state | `projects.e2e.ts` | | ✅ Dashboard recents | A store with no history renders no Recent row at all; visiting a task adds its chip; archiving that task removes it, so the row never offers a dead link | `projects.e2e.ts` | +| ✅ Dashboard phases | **Todo until the first prompt, not until opening**, which is the claim this row exists to pin: a task created and never opened reads `todo` on its row and carries no `started_at`; activating it spawns fakeagent and it is STILL `todo`, asserted against a record the spawn REWROTE (`spawn_count` reaches 1 on disk while `started_at` on that same listing is still null), so the claim is "the record moved and the stamp was not in it" rather than "we looked too early"; the first `submitToAgent` through xterm's own input path moves the row to `in_progress`, drops the Todo pill count by exactly one, and lands a `started_at` on disk that travelled null -> minutes old. Every pill is always rendered, in the order all / todo / in_progress / in_review / done / parked (Parked last, after Done: it is not a stage of the life the first four spell out), so the counts are what is asserted and never a pill's presence. A seeded PR then drives a worktree task (Todo after its spawn too, In progress after one prompt) through the whole ladder: open -> `in_review`, draft -> `in_progress`, open + `changes_requested` -> `in_review`, open + failing checks -> `in_review`, closed -> `in_progress`, merged -> `done`. With the PR snapshot cleared, REAL git drives the rest against the task's own worktree and the fixture's bare origin, each step behind a forced `useTaskGit.refresh(id, true)` (the poller's 30s floor makes `taskGitPassNow` a no-op between two steps seconds apart): started with no commits and no remote branch is `in_progress`; one commit of its own is still `in_progress`, because `ahead` is null and null is not "nothing left to push"; `push -u origin ` makes it `in_review`; a single untracked file takes it back to `in_progress` and deleting it returns it to `in_review`, so the rule is proved in both directions; fast-forwarding the branch into the fixture's `main` (and pushing `origin/main` on behind it) makes it `done` with no PR anywhere. Selecting a pill hides every non-matching row while the pill counts stay put (they describe the fleet, not the view) and the Projects header still counts projects; a filter matching nothing replaces rows AND cards with the one `dashboard-phase-empty` line reading "Nothing done"; pressing the selected pill again hands the selection back to All and the rows return. Teardown restores the fixture's `main`, the bare origin's `refs/heads/main` and the branch on both sides from SHAs recorded in `before()` (never from what a case returned), then asserts in `after()` that the fixture is clean, that no `e2e-phase-*` branch survives and that `ls-remote origin` holds no such head, so a leak fails here instead of in `git.e2e.ts` on a base that moved | `projects.e2e.ts` | +| ✅ Dashboard age | A row whose task has no `last_opened_at` renders no age at all; a stamp three days old renders "3 days ago" in `task-age`; activating that task clears the label; and the never-opened, never-prompted task beside it reads `todo`, since a row's age and its phase are independent halves. Persistence is proven on a SECOND task nobody has opened, so the value has to travel null -> a stamp minutes old rather than being satisfied by one that was already there (Rust holds a stamp younger than `TOUCH_MIN_SECS` instead of rewriting the file, so the same assertion on an already-activated task would pass on its creation stamp): the record is read off disk through `tasks_list` before and after `setActiveTask`. That and the `started_at` null -> fresh check in the phases row are the two assertions in these rows with no DOM to read instead | `projects.e2e.ts` | +| ✅ Planned tasks (goal + Start later) | The New Task dialog's prompt box with **Start later** ticked writes the text down as the task's GOAL and sends nothing: the task is created, its agent still spawns (`spawn_count` reaches 1 on disk), the row reads `todo` with the goal rendered beside it, and `started_at` is still absent from the record, so Planned is a rendering of a goal plus an unstarted task rather than a phase of its own. The same text in the same box with the box UNCHECKED is delivered instead, so that task is `in_progress` from birth and carries no goal: the contrast is the case, since the first half alone would pass against a build that simply dropped the prompt. The checkbox is a styled button, not a native input, so its state is read from `aria-checked`, and it reads unticked on every open. The sidebar menu's "Start with goal" then delivers a planned task's goal through the same seeder: the task reaches `in_progress` with a fresh `started_at` on disk, the goal SURVIVES on the record and on the row (it is a record of intent, not a queue entry that delivering consumes), and the menu row retires itself once the task has started. "Edit goal…" (the label follows whether there is one) opens the goal dialog with focus in the textarea, and saving rewrites the goal on disk and on the row without moving the phase | `projects.e2e.ts` | +| ✅ Parking (the one hand-set value) | Driven through the real sidebar menu and the real dialog: Park writes `parked_at` plus the optional reason, the row reads `parked`, shows the word "Parked" and carries the reason in that node's tooltip. Re-parking rewrites the reason and deliberately does NOT move `parked_at`, which answers "since when". One menu row carries both labels, following the record ("Park task" / "Unpark task"), and unpark opens no dialog because there is nothing to ask: it clears the stamp and the reason together, on disk, and the row returns to its derived phase. The "Also stop the task" checkbox is offered only for a MOUNTED task and defaults to ticked, and it is `aria-checked` that says so. Both dialogs take focus when opened from the dropdown, measured rather than assumed: `document.activeElement` is read once the menu has UNMOUNTED (so Radix's focus-return to the trigger has already run), with the menu-open state read first as the control that the helper is not answering the same thing regardless. Cancelling the park dialog parks nothing. The reason-rewrite step goes through the menu's own "Edit park reason…" row, which exists because the park row flips to Unpark once `parked_at` is set: without it the dialog's pre-fill and the Rust rewrite-without-restamping path would be unreachable by hand. That step also asserts the "Also stop" checkbox is NOT rendered while editing and that the task is still mounted afterwards, because the checkbox is hidden there while its state still holds its ticked default, which made saving a reason kill a running task's agents until it was guarded | `projects.e2e.ts` | +| ✅ A park clears itself on the next prompt | The claim the whole design rests on, and the reason a hand-set value is allowed to sit beside derived ones at all: a started task is parked through the menu (with the stop option unticked so its agent survives), then ONE prompt is submitted into its terminal through xterm's own input path and nothing else. The row returns from `parked` to `in_progress`, the Parked word goes, and `parked_at` and `park_reason` are both gone from the record, with nobody having touched the menu, the palette or the store | `projects.e2e.ts` | +| ✅ Parked in the phase table | Done outranks Parked: a parked worktree task given a merged PR snapshot reads `done` and renders no Parked word, while `parked_at` stays ON the record (outranked, not wiped), and taking the snapshot away brings Parked back, which is what proves the PR did it. The filter row carries a sixth pill, Parked, last after Done; selecting it leaves exactly the parked tasks on the board (the pill's count and the row count agree, and every surviving row reads `parked`) and drops an In progress task that was there a moment earlier | `projects.e2e.ts` | | ✅ Agent settings | Disable/re-enable an agent CLI via agentsSave | `agent.e2e.ts` | | ✅ Run config modal | The #124 run-commands manager opens for a project | `run.e2e.ts` | | ✅ SVG source/preview toggle | An `.svg` opens on the rendered picture (the default stays "preview", so a file-tree click still shows the image), the same source / preview / split toolbar markdown uses switches to the editable source and to both at once, an UNSAVED edit re-renders the picture (the preview is fed by the editor buffer, not disk, so a disk-backed one could not move), and toggling writes the `svgDefaultView` pref for the next file (GH #247) | `editor.e2e.ts` | diff --git a/docs/gotchas.md b/docs/gotchas.md index 96c09df0..a1c81bb9 100644 --- a/docs/gotchas.md +++ b/docs/gotchas.md @@ -773,3 +773,47 @@ family as the "Reset to defaults" loss that first put these fields in the TS typ (a default entry spread over fields TypeScript did not know about) and as the clone-that-snapshots-its-parent trap in `agents.ts`. See [agent-accounts.md](agent-accounts.md). + +## Task record setters serialize on the main thread, and only there + +Every small per-task setter in `lib.rs` (`task_record_spawn`, +`task_set_has_history`, `task_set_tabs`, `task_set_yolo`, some thirty of them) +is an unlocked read-modify-write of the task's whole JSON file: load, find, +mutate one field, `save_task`. Nothing guards two of them against each other. +They are correct anyway, because they are all sync commands and Tauri runs +sync commands on the main thread one after another. That invariant was never +written down, and `task_touch` broke it by accident: it fires on every +activation, so it was made async + `spawn_blocking` to stay off the main +thread, which put it on another thread at the exact moment `task_set_tabs` +and `task_record_spawn` fire for the same task (the pane mounts and spawns +within milliseconds of the activation). The e2e run then found a task +activated seconds earlier with `last_opened_at: null` on disk: a sibling had +read the record before the touch wrote it and written its own copy back +after. The fix was to make the touch sync like its siblings, which costs one +small read and one atomic write on the main thread, strictly less than a +sibling's `load_tasks_all()`. + +So: a per-task setter that writes the record is sync, or it takes a lock +that every other writer of that record also takes. The existing async writers +(`task_archive_sync`, `task_restore_sync`, `pr_lookup_blocking`, +`task_pr_create`) are the known exposure: rare and user-paced, or a 30s +background poll whose read-to-write window is a few microseconds, so nobody +has seen them lose a write. Adding a frequent one is how the race stops being +theoretical. + +`task_mark_started` is the hard case: it fires on EVERY prompt submission, +which is exactly when the pane is spawning and `task_record_spawn` and +`task_set_tabs` are firing for the same task, so it is sync like `task_touch`. +It used to be write-once as well, which meant there was only ever one write to +lose; it is not any more, because it also clears the park (a prompt into a +parked task means the user has picked it back up). The STAMP is still written +once, but the command can write on any call, so the rule is doing real work +here rather than being belt-and-braces. It still skips the write when nothing +changed, which is the case on almost every call. The two setters beside it, +`task_set_goal` and `task_set_parked`, are sync for the same reason and are +user-paced on top of it. + +`task_git_phase_state` is the other half of the rule: it is IO-heavy enough to +need `spawn_blocking`, so it is strictly READ-ONLY on the record and must +never call `save_task`. If it ever needs to persist something, that write +goes through a sync command, not through the async one that computed it. diff --git a/docs/ideas/phase-automation.md b/docs/ideas/phase-automation.md new file mode 100644 index 00000000..ade7184d --- /dev/null +++ b/docs/ideas/phase-automation.md @@ -0,0 +1,95 @@ +# Driving work from the phase + +**Status: idea. Nothing here is built, and nobody has committed to building +it.** The phase it builds on is implemented and documented in +[ui.md](../ui.md); this doc is only about what could sit on top of it. + +## What exists, and why it makes this possible + +A task's phase is derived at render from signals already in memory: the task +record, the PR store and a git lookup. Todo, In progress, In review and Done +are computed, not typed. The one hand-set value is Parked, which exists +because "I put this down" has no live twin anywhere in git or the forge, and +it clears itself on the next prompt. + +The rule the design stands on: + +> A person may set the states the machine cannot see. The machine owns every +> state it can see. A manual state clears itself the moment evidence arrives. + +That rule is what makes automation cheap here. Walk a task through the +pipeline and every step leaves a real fact behind: + +| Step | The fact it writes | What the phase does | +| --- | --- | --- | +| goal submitted as a prompt | `started_at` | Todo to In progress | +| agent commits and pushes | clean tree, `ahead = 0` | In progress to In review | +| PR opens | forge state | stays In review | +| PR merges, or the branch reaches base | `merged_into_base` | Done | + +So an automated pipeline needs **no status field at all**. It does the work, +and the phase follows because the work happened. Compare that with syncing a +stored column on a PR webhook: the column is a second copy of something git +already knows, and a second copy is the thing PR #292 was rejected for. + +## The shape + +A Planned task (a goal, no `started_at`) is the entry point. Today you start +it by hand and the goal is delivered as the first prompt. The idea is that +the same transition can be asked to carry more: + +``` +Planned In progress In review Done ++-----------+ +-----------------+ +----------------+ +-----------+ +| rate- | -> | agent runs with | -> | second model | -> | PR merged | +| limits | | the goal as its | | reviews the | | | +| (goal set)| | prompt | | diff, pushes | | | ++-----------+ +-----------------+ | a PR | +-----------+ + +----------------+ + ^ ^ ^ + | | | + you write automatic automatic, and + the goal optional per task +``` + +You define the work once, in the goal. Starting it runs the implementation, +the review and the PR, and you are handed something to look at. The phase is +the readout throughout, never the mechanism. + +## What would have to exist first + +1. **An action attached to a transition.** Termic already has the pieces: the + prompt library, the review flow, the message queue and PR create. What is + missing is the binding from "this task entered In progress" to "run this + prompt in it". +2. **A place to configure it per project.** A repo whose review prompt is + special needs to say so, and `.termic.yaml` is where that already lives. +3. **Completion that is not a guess.** "The agent finished" has to be a real + signal. Agent hooks report done today, and the git state says whether + anything was committed. Both are needed: an agent that reports done having + written nothing has not finished, it has failed. + +## Where this should stop + +**No automatic merge.** The pipeline's job is to hand back a reviewable PR, +not to land it. This repo's own contributing rules say green suites are not a +manual test and that the one gate an agent-written change has to pass is a +human actually driving it. An automation that merges its own work walks +straight past that. Producing the PR is the valuable part anyway; the last +click is cheap and it is the one worth keeping. + +**Fan-out has to be deliberate and visible.** One transition could spend a lot +of money across a lot of agents. Whatever starts work from a transition should +say what it is about to do before it does it, and should be off by default. + +## Open questions + +1. Is the transition the right trigger, or is it a button on the card? The + trigger reads better on a board and worse everywhere else, and Termic has + no board today. +2. Should a failed run move the phase back, or park the task with the failure + as its reason? Parking is more honest and it reuses something that exists, + but a machine writing the one hand-set field needs thinking about. +3. Does the second-model review belong here at all, or is it just a prompt you + run like any other? The answer probably depends on whether its result has + to gate the PR. diff --git a/docs/ipc.md b/docs/ipc.md index d91a1799..74a058c6 100644 --- a/docs/ipc.md +++ b/docs/ipc.md @@ -2,7 +2,7 @@ ## Tauri commands -- **Tasks**: `task_create`/`task_create_multi` (async, spawn_blocking; the frontend never blocks on the returned promise — see "Non-blocking task creation" below) stream the WHOLE creation timeline — worktree add, file copy, port allocation, then the setup script — on one channel, `setup-output://` (`{ line }`) + `setup-done://` (`{ code, success }`), keyed by the client-generated task id the New Task dialog sends as `args.id` (so the frontend can subscribe before invoking). `task_archive`/`task_delete` (async, spawn_blocking), `task_open_repo`, `task_run_script_stream` + `task_stop_script` (PIDs in `RUNNING_SCRIPTS`, child has `process_group(0)` for clean SIGTERM tree-kill), `task_ensure_extra_ports` (GH #196: tops up frozen named ports from the current config, called by the frontend before every tab spawn). +- **Tasks**: `task_create`/`task_create_multi` (async, spawn_blocking; the frontend never blocks on the returned promise — see "Non-blocking task creation" below) stream the WHOLE creation timeline — worktree add, file copy, port allocation, then the setup script — on one channel, `setup-output://` (`{ line }`) + `setup-done://` (`{ code, success }`), keyed by the client-generated task id the New Task dialog sends as `args.id` (so the frontend can subscribe before invoking). `task_archive`/`task_delete` (async, spawn_blocking), `task_open_repo`, `task_run_script_stream` + `task_stop_script` (PIDs in `RUNNING_SCRIPTS`, child has `process_group(0)` for clean SIGTERM tree-kill), `task_ensure_extra_ports` (GH #196: tops up frozen named ports from the current config, called by the frontend before every tab spawn). `task_touch` stamps one task's `last_opened_at` and returns what is now on disk; it is fired by `setActiveTask` on EVERY activation, so unlike its siblings (`task_record_spawn`, `task_set_has_history`) it reads the single record out of whichever profile holds it rather than `load_tasks_all()`, and it skips the write entirely when the existing stamp is under 60s old. It is deliberately SYNC like every other per-task setter: they are unlocked read-modify-writes of one file that only stay correct because sync commands run one after another on the main thread, and an async first version of this one lost its stamp to `task_record_spawn` firing for the same task milliseconds later (see [gotchas.md](gotchas.md), "Task record setters serialize on the main thread"). `task_mark_started` records that work is happening on a task NOW and returns the `started_at` on disk; same single-record read and same SYNC discipline as `task_touch`. It does two things. The STAMP is write-once: a task that already carries one keeps it byte-for-byte, because the field records when work STARTED (the frontend bails too, but its copy is per-window state and a second window would otherwise re-submit). The PARK it clears on every call, which is why the COMMAND is no longer write-once even though the stamp is: submitting a prompt into a parked task means the user has picked it back up, and a manual flag only a human can clear is the stale signal the whole derived phase exists to avoid. It still skips the write when NOTHING changed (already started AND not parked), which is almost every call, and that bail is what keeps a command that fires on every prompt submission off the disk. `task_set_goal(id, goal)` writes (or clears) what the task is for, the one thing on the record a person types and the one half of the phase nothing live can supply. The goal is stored TRIMMED and a blank or whitespace-only value stores `None` rather than `""`, so emptying the box really removes the goal instead of leaving one that renders as nothing; an unchanged value skips the write. `task_set_parked(id, parked, reason)` parks or unparks, and RETURNS the resulting `parked_at` (`None` once unparked) so the caller folds the real stamp into its store rather than inventing a `Date.now()` that would disagree with the file after a reload. Parking an ALREADY-parked task does not move `parked_at`, because that stamp is what "parked 3 days ago" is rendered from and editing the reason is the common way to call it twice; the reason is always taken from the argument (trimmed, blank means `None`), so it can be edited without unparking. Unparking clears BOTH fields. There is no Blocked state: "blocked on the API key" is a parked task with a reason. Both are SYNC single-record read-modify-writes for the same reason as their siblings, and both skip the write when nothing changed. `task_git_phase_state` answers where a task's branch stands against its base: `own_commits` (`rev-list --count B..T`, 0 after any merge), `dirty` (anything staged, unstaged or untracked in the HOST worktree; composition members are out of scope), `ahead` (`None`, not 0, when the branch has no remote branch at all), `merged_into_base` and `base_known`. It is ASYNC + `spawn_blocking` because it shells out to git several times and writes one object, and therefore strictly READ-ONLY on the task record: an async writer would race the sync setters, which is the whole point of the gotchas entry above. `merged_into_base` has two tiers and both are BIASED TOWARD FALSE, because a missed Done costs nothing and a wrong Done tells the user to archive live work: tier 1 (the branch has commits of its own) asks whether every branch-side commit is patch-equivalent on the base, with a squash variant that replays the branch's tree as one commit on the merge base; tier 2 (a fast-forward or a merge commit left no own commits) needs the tip to be an ancestor of the base AND the creation commit to be known AND the tip to have moved off it AND the branch reflog to prove something was committed here AND that committed sha to be reachable from the base. The last two conditions each rule out a different false Done that the DAG cannot see: without the reflog check, a fresh branch an agent rebased or fast-forwarded onto a newer base is structurally identical to an ff-merged one; without the reachability check, a branch that committed and then ran `git reset --hard` onto a moved base satisfies everything else, and calling that merged would tell the user to archive work that was thrown away. It errors for an archived task and for a main-checkout task, neither of which has a branch of its own to answer about. - **PTYs**: `pty_spawn`/`pty_write`/`pty_resize`/`pty_kill`. Emits `pty://` (`PtyChunk { data: Vec }`) and `pty-exit://` (`PtyExit { code: Option }`). `SpawnArgs.role` (`{ task_id, kind: "agent"|"aux", is_default }`) is the CLI attach/logs identity and allocates the 256 KiB output ring; it is deliberately separate from `task_id`, which doubles as the sandbox trigger (the aux shell carries a role but never a task_id). `SpawnArgs.owner` (`{ task_id?, tab_id?, kind: "agent"|"shell"|"aux"|"run"|"setup"|"custom" }`) is a THIRD identity and a reporting field only: the Activity monitor groups rows by project → task → tab with it. Every spawn sets it, including the ones the other two must skip — a scratch shell pegging a core is exactly what the monitor exists to find. Nothing may branch on it. - **PTY attach ack**: `pty_attached { id }`, called by the webview the instant `listen("pty://")` resolves. Tauri events are fire-and-forget, so everything the flusher emits before that listener exists is dropped with no trace, and the child starts writing the moment it is forked. Rust therefore holds a PTY's FIRST flush (and the reader's final drain, for a process that exits immediately) until the ack lands or a 3s grace expires. **Every caller of `pty_spawn` must send it** (`TerminalPane`, `AuxTerminal` today), or that terminal shows nothing until the grace runs out. The gate itself is `wait_for_attach` in `lib.rs`, unit-tested for all three exits. - **Activity monitor**: `procmon_open_window` creates or re-focuses the `procmon` window; `procmon_start` → `ProcSnapshot { session, rows, sampleMs, webkitUnavailable }`, `procmon_sample { session }`, `procmon_stop { session }`, `procmon_signal { pid, signal }` (TERM/KILL/INT/STOP/CONT only, and only for a pid inside one of OUR PTY subtrees — the webview must not be an arbitrary `kill(2)` gadget). Sampling is PULL-based: there is no sampler thread, the Activity window's own interval is the clock, and `stop` leaves the module holding nothing. `session` is a guard, not decoration: a mismatched id errors so a reloaded webview restarts cleanly instead of reading another window's deltas. Only ever called from the Activity window (`activity.html`), never the main one. `mod procmon` in `lib.rs` is a 3-way `#[cfg(target_os = …)]` split resolving to `procmon.rs` (macOS, libproc/mach FFI, `ri_phys_footprint` for memory), `procmon_linux.rs` (`/proc`, plain text, `VmRSS` for memory — no phys_footprint equivalent, no WebKit-sidecar attribution), or `procmon_other.rs` (every other OS: a stub reporting "unsupported"). All three share row shapes + OS-agnostic logic (subtree walk, `cpu_ratio`, `label_for`, `signal_from_name`) from `procmon_common.rs`. The macOS FFI genuinely fails to LINK if it ends up compiled into a non-macOS build — this split exists because that shipped broken once (the Linux release build failing at link time with undefined libproc/mach symbols). diff --git a/docs/performance.md b/docs/performance.md index f9e168f5..2fb400c4 100644 --- a/docs/performance.md +++ b/docs/performance.md @@ -15,6 +15,8 @@ 10. **A per-line editor annotation is a layout cost; a per-cursor-line one is not.** CodeMirror's own rule (`EditorView.decorations`): sets provided as a FUNCTION are computed after the viewport and so may not introduce block widgets, while sets provided DIRECTLY may affect layout but cannot read the viewport. An every-line blame column therefore has only two shapes, and both are bad: viewport-scoped and rebuilt on every scroll frame, or directly-provided and height-relevant on all 15,742 lines of `lib.rs`. The view's `heightRelevant` getter is the line to read: `this.block || !!this.widget && (this.widget.estimatedHeight >= 5 || this.widget.lineBreaks > 0)`. Inline blame (`inlineBlameExt.ts`) keeps ONE widget on the cursor's line with the default `estimatedHeight` (-1) and `lineBreaks` 0, so moving the cursor never dirties the height map at all, and the DecorationSet REFERENCE is reused whenever the rendered text is unchanged (an unchanged directly-provided set short-circuits CodeMirror's height-map compare; VS Code suppresses the same way via `isResourceBlameInformationEqual`). Two more things that are load-bearing rather than tidy: the git fork happens ONCE per file and every later cursor move is an array index (a 15k-line file is ~200 ms of `git blame`, so a per-move fork would be unusable), and the fetch does not start until the cursor leaves position 0, which is what stops a stack of mounted-but-hidden editor tabs from each forking git on open. If you add an every-line mode, read the CodeMirror rule above first and measure the height map, not the frame rate. +11. **A poller that feeds one screen ticks only while that screen is mounted.** The dashboard's derived phase reads a git state per task (`task_git_phase_state`, `src/store/taskGit.ts`), which shells out to git: on a large monorepo a status walk is not free, and the fleet-wide version of that cost is paid per task. It is NOT a global poller like the PR one, and the difference is not squeamishness: the PR badge is on the sidebar, which is always mounted, while the phase is drawn only on the dashboard, and the dashboard is mounted only while no task is open (`MainArea` renders it as the overlay when `activeTaskId` is null). So "the screen that needs this is up" and "the user is not driving an agent" are the same condition, and `startDashboardGitPolling()` / `stopDashboardGitPolling()` run off the Dashboard's effect: nothing git-shaped runs behind a live agent turn. Inside a pass the controls mirror the PR poller: sequential (each `await`ed, never a fan-out), at most 6 tasks, stalest first, a 30s floor per task, and skips for archived, `is_main_checkout`, and any task whose PR is open or merged, since those decide the phase on their own and git could only agree. Draft and closed PRs are still polled because `merged_into_base` must be able to beat them. Two smaller rules that are load-bearing rather than tidy: a REJECTED lookup is recorded as `state: null` with a `fetchedAt`, or the broken task becomes the stalest thing in the fleet and is retried on every pass forever; and a resolved state that deep-equals the cached one reuses the cached OBJECT, so `state` keeps its identity across a steady poll of an unchanged branch. Do not read that second one as more than it is: `fetchedAt` must advance or the floor stops working, so a successful refresh always writes `byTask` (twice, counting the `loading` flip), and the Dashboard subscribes to `byTask` and re-derives every phase either way. It buys a stable reference for anything keying on `state`, not a skipped write. A literal no-write would mean `loading` and `fetchedAt` in module-level maps rather than on the entry, i.e. a different `byTask` shape from pr.ts's, which was not worth it for two writes per task per pass, cap 6, on the only page mounted at the time. The tick and the floor are both 30s, which makes one task's steady cadence nearer 60s; that is deliberate, since 30s is a floor rather than a target, and what it really guards is the burst when the user leaves a task and comes back, which runs a pass immediately. + ## The Activity monitor's own cost The process monitor ([ui.md](ui.md#activity-window-per-agent-cpu--memory), `src-tauri/src/procmon.rs`) measures agents' CPU and memory, so it is the one feature where being cheap is the feature. Four decisions, in the order they matter: diff --git a/docs/tech-debt.md b/docs/tech-debt.md index b484e968..4f27327e 100644 --- a/docs/tech-debt.md +++ b/docs/tech-debt.md @@ -16,6 +16,7 @@ layer above Project shipped as [docs/profiles.md](profiles.md). | 1 | `workspace` → `task` migration (schema v1) | v0.19.0 | a few minor releases after v0.19 | active | | 2 | `migrate_legacy_members()` (multi-repo) | pre-v0.19 | independent (likely already) | active | | 3 | `LEGACY_IDS` (pre-registry language ids) | v0.28.x | a few minor releases | active | +| 4 | `started_at` backfill (`data_migration_version` 1) | task-phase release | a few minor releases after it | active | --- @@ -193,3 +194,74 @@ losing its highlight is acceptable. A few minor releases. 3. The note on `ScratchTab.syntax` in `src/lib/types.ts`. Nothing on the Rust side changes: `ScratchRecord.syntax` is an opaque `String`. + +--- + +## 4. `started_at` backfill (`data_migration_version` 1) + +One-time, on-disk backfill that gives `Task.started_at` a value on every record +written before the field existed. Runs once at startup, after +`migrate_workspaces_to_tasks` (which is what puts the records in `tasks/` where +this can find them) and before the window, so the first `tasks_list` already +carries the stamps and no row flickers from Todo to In progress. + +### Why it exists + +`started_at` is the first prompt the user submitted into a task, and it is what +separates "created, agent idling at its prompt" (Todo) from "someone gave it +work" (In progress). Spawning alone cannot draw that line, because every GUI +create spawns. Every task on disk when the field shipped predates it, so +without the backfill a user's whole fleet reads Todo on the first launch after +upgrading, which is both wrong and loud: the dashboard filter would show +nothing but Todo. + +### The rule + +A record with `started_at == None` and either `spawn_count > 0` or +`has_resumable_history` takes `last_opened_at`, falling back to `created`. +Neither is when work really started, and both are an upper bound that is right +to within a session; `created` is the floor, and a task nobody ever opened has +nothing better. A record carrying neither timestamp is left alone rather than +stamped with an empty string nothing downstream could parse. + +### Complexity: low + +| Surface | Where | ~LOC | Role | +|---|---|---|---| +| `migrate_started_at_backfill()` | `src-tauri/src/lib.rs` | ~30 | The sweep: every profile's tasks dir, save each changed record, stamp the version last and only when every write landed. Single call site in `.setup()`. | +| `backfill_started_at()` | `lib.rs` | ~20 | The rule for one record, split out so "running it twice changes nothing" is an assertion about the rule rather than about the guard bailing. | +| `stamp_data_migration_version()` | `lib.rs` | ~8 | Ladder stamp, never downwards. | +| `Settings.data_migration_version` + `STARTED_AT_BACKFILL_VERSION` | `lib.rs` | ~4 | The guard. | +| Tests | `lib.rs` | ~80 | Four cases: stamps a task an agent ran in, falls back to `created`, leaves an untouched task alone, sweeps every profile and stamps the version last. | + +### Why it is NOT a `schema_version` bump + +`schema_version` gates the workspaces->tasks migration on +`>= TASKS_SCHEMA_VERSION`, so raising that constant would re-run the whole +rename migration (backup, stage, atomic rename) on every v1 profile on the next +launch. `data_migration_version` is a second, independent ladder for task-record +backfills, in the same spirit as `cli_default_migrated` (entry 1's "Renamed +persisted fields" note) but counted rather than boolean, because backfills +accumulate and a bool per step does not say which ones a profile has seen. + +### Safe to remove when + +Every realistically-active install has launched a build carrying it at least +once. A few minor releases after the release that introduces it. The cost of +removing it early is that a dormant install's older tasks all read Todo when it +finally launches, which is cosmetic and self-corrects the moment the user +prompts into one. + +### What to delete + +1. `migrate_started_at_backfill()` and `backfill_started_at()` in + `src-tauri/src/lib.rs`, plus the call site in `.setup()`. +2. `const STARTED_AT_BACKFILL_VERSION`. +3. The four `the_backfill_*` tests in `lib.rs`. +4. `Settings.data_migration_version` and `stamp_data_migration_version()` ONLY + if no later step has joined the ladder. If one has, keep both and delete + step 1 from the "Steps so far" list on the field's doc comment. +5. This entry and its table row. + +Nothing on the frontend side changes: `started_at` itself stays, and the +backfill is invisible to it. diff --git a/docs/ui.md b/docs/ui.md index a2cc87c9..dcdaa28c 100644 --- a/docs/ui.md +++ b/docs/ui.md @@ -800,6 +800,310 @@ own components (`TaskWorkBadge`, `TaskPrBadge`), fed by the same (attention > done > working). The PR chip renders what the poller already resolved and never starts a lookup, so listing every task costs nothing. +### Phase and age are derived, never stored + +A task's phase comes from `taskPhase()` (`src/lib/taskPhase.ts`): the task +record, plus the live PR snapshot in `usePr`, plus the live git state in +`useTaskGit`. Five values, first match wins: + +| Phase | When | +|---|---| +| **Done** | `archived`, or the PR is merged, or `merged_into_base` | +| **Parked** | `parked_at` is set | +| **In review** | the PR is open, or (no PR at all AND own commits AND clean AND nothing ahead) | +| **In progress** | the PR is draft or closed, or `started_at` is set | +| **Todo** | none of the above | + +**The rule the table stands on:** a person may set the states the machine +cannot see, the machine owns every state it can see, and a manual state clears +itself the moment evidence arrives. In progress, In review and Done are +derived only, and no UI may hand-set them: each has a live twin the app already +polls, and a second hand-kept copy beside one is the redundancy PR #292 was +rejected for. **Parked is the one hand-set value**, allowed because "I have +deliberately put this down" leaves no trace in git, in the forge or in any +process, so it has nothing to contradict. It does not need hand-clearing +either: `markStarted` wipes `parked_at` and `park_reason` on the next prompt +into any terminal of the task. Its optional free-text reason is where "blocked +on the API key" lives, and there is deliberately no Blocked phase, since +blocked is a reason for parking rather than a stage of the work. Done outranks +Parked (a parked task whose PR merged is finished either way), and Parked +outranks even an open PR, because it is the most specific and most recent thing +a person has said about the work, and the row's PR chip still says the PR is +open. + +**A `goal` is text, not a state.** It records what the task is for, feeds no +rule in the table, and exists because there was nowhere else to write one down: +the only place was the agent's prompt box, and submitting that stamps +`started_at`. A task with a goal and no `started_at` reads as **Planned**, and +that reading is rendered from those two fields rather than derived into a sixth +value: a Planned phase would store what `started_at` already answers. There is +no Planned pill and no `"planned"` in `PHASE_ORDER`; the dashboard row draws +the goal and that IS the reading. Where goals come from, and the four controls +that write these two fields, are in "Setting a goal, and parking" below. + +**Todo is where every new task starts, and that is the normal case, not a +rarity.** Creating a task spawns its agent, so a spawn is not evidence that +anybody has given it work: the agent is sitting at its prompt waiting for one. +**In progress** begins at `started_at`, the first prompt a human submits into +any terminal of the task, stamped write-once by `markStarted` in `useApp` at +each place user text reaches a terminal (the GUI's Enter, a queued prompt, the +New Task dialog's seed, a library prompt, sent review comments, and the CLI's +`termic send`). Enter in a plain shell tab counts too: someone running the +task's tests has started working on it in every sense this screen cares about. + +**The git rule** is the second half of In review, and it is what a task that +was worked on and handed off looks like when there is no PR: `own_commits >= 1` +(the branch has commits the base cannot reach), `dirty === false` (nothing +staged, unstaged **or untracked** in the worktree) and `ahead === 0` (the +remote branch exists and has everything). `ahead === null` means there is no +remote branch at all, which is not the same as nothing left to push, so it does +not qualify. `base_known` is deliberately not a condition: the commit count is +taken against the base branch, not the creation commit, so an imported +worktree or a reused branch (whose `base_sha` is None by design) qualifies like +any other. Only `merged_into_base` needs the creation commit, and Rust folds +that in on its own. A **draft or closed PR outranks this rule entirely**: both +are an explicit statement by a person about how ready the work is, and a clean +pushed branch underneath does not overrule it, which is why the rule requires +`pr` to be absent rather than merely not-open. + +**Stop is deliberately not an input.** "The user stopped the task" is the +obvious signal for handing off, and it is unusable: it is not persisted +anywhere, so it is every task's state after a relaunch, and a phase that read +it would move the whole fleet to In review on every launch. The git rule +answers the same question from facts that survive a restart. + +The decisions that table encodes, all of them argued in that file's header: +archived beats merged (a shelved task is finished whatever its PR did); a draft +PR is In progress, because a draft says outright that it is not ready to look +at; a closed unmerged PR falls back to In progress, not Todo, because the +branch has real work on it; `changes_requested` stays In review, so the phase +does not oscillate with every review round; a failing check does not move the +phase at all (CI is a property of the work, not a stage of it, and the PR chip +already turns red); a failed PR lookup has `pr === null` like "no PR" does and +therefore falls through, so a machine with no `gh`/`glab` still phases +correctly; an unknown git state does the same (`undefined` for a task nothing +has polled, `null` for one whose lookup failed, both "we do not know"); and a +main-checkout task never enters the git rules at all, because `pollableTasks` +skips `is_main_checkout`. + +**With no PR, the phase moves In progress <-> In review with each work +cycle**, and that is truthful rather than noisy: edit something and the tree is +dirty, so it drops back; commit and push and it returns. It is a different +thing from the review-round oscillation the design avoids, where +`changes_requested` deliberately does not move the phase because a reviewer's +opinion is not a change in where the work stands. One consequence on purpose: a +stray untracked file pins a task at In progress. Unfinished work in the +worktree is unfinished work, whatever the commits say. + +**Once a PR is open, none of that applies.** An open PR reads In review however +dirty the worktree is and however many commits are unpushed. The asymmetry is +real and deliberate: without a PR a single untracked file pins a task at In +progress, and with one, nothing local moves it at all. + +An open PR is an explicit act by a person saying the work is ready to be looked +at. `dirty` and `ahead` are PROXIES for that same statement, used only where +the person has not made it, and a proxy must not overrule the thing it stands +in for. The practical half matters as much: a dirty worktree under an open PR +is what addressing review comments looks like, so a phase that flipped on every +edit would be noise, for the same reason `changes_requested` is kept out of the +phase. A draft PR is the control that shows this is a rule rather than an +oversight: it is the person saying the opposite, so it reads In progress even +on a clean, fully pushed branch. + +**On upgrade**, existing tasks are backfilled: one that had ever spawned an +agent reads In progress, so nothing that was underway reappears as Todo, while +tasks created from here start in Todo and earn In progress at their first +prompt. + +**The git pass is scoped to this page.** `useTaskGit` +(`src/store/taskGit.ts`) is shaped like the PR store, with one deliberate +difference: `startDashboardGitPolling()` / `stopDashboardGitPolling()` are +mounted by the Dashboard's effect and nothing else ticks it. That effect is +gated on there being tasks, the same gate `initPrStatusPoller` has: on launch +the page mounts before `loadAll` resolves, and starting there would spend the +immediate pass on an empty store and leave every git-derived phase reading In +progress until the next tick. The phase is drawn +only here, the Dashboard is mounted only while no task is open, and +`task_git_phase_state` shells out to git, so nothing runs while the user is +driving an agent. Inside a pass: sequential, at most 6 tasks, stalest first, a +30s floor per task, skipping archived, main-checkout, and any task whose PR is +open or merged (those decide the phase on their own). Draft and closed PRs are +still polled, because `merged_into_base` has to be able to beat them: a +squash-merged branch whose PR was closed rather than merged would otherwise +never reach Done. See [performance.md](performance.md). + +**The filter row** (`data-testid="dashboard-phase-filter"`) sits between Recent +and the Projects header and renders only when at least one non-archived task +exists, so a fresh install sees the page it always saw, or while a filter is +selected, so archiving the last task cannot strand the empty line with no pill +to clear it. Pills are All then `PHASE_ORDER`, each a ` + + + + ); +} diff --git a/src/components/dialogs/TaskGoalDialog.tsx b/src/components/dialogs/TaskGoalDialog.tsx new file mode 100644 index 00000000..01b02fc6 --- /dev/null +++ b/src/components/dialogs/TaskGoalDialog.tsx @@ -0,0 +1,106 @@ +// Edit a task's GOAL: free text recording what the task is for. +// +// It is text, not a state. It feeds no rule in `taskPhase`, so setting one +// never moves a task off Todo; a task carrying a goal with no `started_at` is +// what the dashboard draws as Planned, and that reading is rendered from those +// two fields rather than derived into a phase of its own (see the header of +// src/lib/taskPhase.ts). +// +// The New Task dialog's "Start later" checkbox is where most goals come from. +// This dialog is the way to add one afterwards, or to change one, or to clear +// it: an emptied box means no goal, which is the same answer `null`, an absent +// field and a box holding only spaces all give. +// +// A TEXTAREA, not an input. A goal typically arrives from the New Task +// dialog's multi-line prompt box, and `` silently strips +// the newlines out of its own value, so editing a pasted ticket here would +// flatten it on the way back out. + +import { useEffect, useState } from "react"; +import { useUI } from "@/store/ui"; +import { useApp } from "@/store/app"; +import { AppDialog } from "@/components/ui/Dialog"; +import { Button } from "@/components/ui/Button"; +import { taskGoalText } from "@/lib/taskNotes"; +import { Target } from "lucide-react"; + +export function TaskGoalDialog() { + const taskId = useUI(s => s.taskGoalTaskId); + const close = useUI(s => s.closeTaskGoal); + const task = useApp(s => s.tasks.find(w => w.id === taskId) ?? null); + const setTaskGoal = useApp(s => s.setTaskGoal); + + const open = taskId !== null; + const [goal, setGoal] = useState(""); + + // Snapshot the record whenever the dialog opens for a new id, the same + // shape ResumeOverrideDialog uses: this component is permanently mounted + // from Dialogs.tsx, so nothing else resets its state between opens. + useEffect(() => { + if (!open) return; + setGoal(task?.goal ?? ""); + }, [open, task?.id]); // eslint-disable-line react-hooks/exhaustive-deps + + function save() { + if (!taskId) return; + // The store trims and collapses an empty box to `null` itself, and bails + // when nothing moved, so an unedited submit costs one array lookup and no + // disk write (docs/performance.md bear trap 8). + setTaskGoal(taskId, goal); + close(); + } + + const had = taskGoalText(task ?? { goal: null }); + const clearing = had !== "" && goal.trim() === ""; + + return ( + (v ? null : close())} + title={had ? "Edit goal" : "Set a goal"} + className="max-w-lg" + > +

+ What {task?.name ?? "this task"} is + for. It is a note, not a status: nothing is sent to the agent and the + task's phase does not move. A task with a goal that nobody has prompted + yet reads as planned on the dashboard. +

+ +