From d94ab166426495ee1278c50cc7a2e3df0cdce280 Mon Sep 17 00:00:00 2001 From: Vinodkumar Naidu <7994336+nvkvin@users.noreply.github.com> Date: Sun, 13 Sep 2026 22:58:24 +0530 Subject: [PATCH 01/10] feat(task): persist last_opened_at and touch it on activation Nothing on the task record says when the user last had it open. The dashboard's Recent row is localStorage, capped at eight and per machine, so "which of these am I still on" has no durable answer. This adds `last_opened_at`, stamped by the app itself in `setActiveTask` and persisted by a new `task_touch` command. Nothing is typed by a person. The command fires on every activation, so it reads the one record out of whichever profile holds it (re-tagging `profile`, which is serde(skip), so the save lands back in that profile rather than the root tree) instead of its siblings' `load_tasks_all()`, and it skips the write when the existing stamp is under 60s old. It is sync like those siblings, on purpose: every per-task setter is an unlocked read-modify-write of the same file and they only stay correct because sync commands run one after another on the main thread. An async first version raced `task_record_spawn` for the same task and lost its stamp on disk; docs/gotchas.md now records the invariant. The frontend applies the same 60s guard before the IPC and stamps its own copy inside the set() that setActiveTask was already doing, so a task switch costs no extra store notification. A future or unparseable stamp re-stamps on both sides, otherwise a clock that jumped backwards would suppress every activation for hours. Also fold `task_record_spawn`'s answer back into the store. The count was only refreshed by loadAll, so a task launched this session read as never-spawned until the next reload; the derived phase that follows reads it. Refs #292 --- docs/data-model.md | 2 +- docs/gotchas.md | 27 +++ docs/ipc.md | 2 +- src-tauri/src/lib.rs | 210 ++++++++++++++++++++- src/components/task/TerminalPane.tsx | 8 +- src/lib/cliAgentState.test.ts | 4 + src/lib/dirTabs.test.ts | 4 + src/lib/ipc.ts | 5 + src/lib/trayAttention.test.ts | 4 + src/lib/types.ts | 9 + src/store/agentHooksSync.test.ts | 4 + src/store/app.test.ts | 212 +++++++++++++++++++++- src/store/app.ts | 59 +++++- src/store/cliPrompts.integration.test.ts | 7 +- src/store/cliSend.integration.test.ts | 4 + src/store/cliTab.integration.test.ts | 4 + src/store/cliTabClose.integration.test.ts | 4 + src/store/pr.test.ts | 4 + src/store/race.integration.test.ts | 4 + src/store/recentPlaces.test.ts | 3 + src/store/resume.integration.test.ts | 4 + src/store/scratchpad.integration.test.ts | 4 + src/store/selectorFanout.test.ts | 4 + 23 files changed, 583 insertions(+), 9 deletions(-) diff --git a/docs/data-model.md b/docs/data-model.md index f07a4679..ba31286d 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`. 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. - **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/gotchas.md b/docs/gotchas.md index 96c09df0..91d12b59 100644 --- a/docs/gotchas.md +++ b/docs/gotchas.md @@ -773,3 +773,30 @@ 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. diff --git a/docs/ipc.md b/docs/ipc.md index cef30539..b1f1eb56 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"). - **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/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 08620064..983e20b5 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -440,6 +440,17 @@ pub struct Task { /// archived first. `None` on tasks archived before this field existed. #[serde(default)] pub archived_at: Option, + /// RFC3339 UTC timestamp of the last time the user ACTIVATED this task + /// (the frontend's `setActiveTask`, i.e. a sidebar click or a Cmd-number + /// switch). Written by the app itself, never typed by a person, so it is + /// not part of the New Task form and not editable anywhere. + /// `None` on records written before the field existed. + /// The dashboard's age label reads it and shows NOTHING for `None`, + /// rather than guessing from `created`: a task created months ago and + /// opened this morning is not a months-old task, and the two facts are + /// not interchangeable. + #[serde(default)] + pub last_opened_at: Option, /// True when this task points at the project's main repo checkout /// (no git worktree created). Used by the "open repo directly" feature: /// archive skips `git worktree remove`, and the UI shows a distinct icon. @@ -5733,6 +5744,7 @@ fn task_open_repo( right_split_tabs: Vec::new(), split_layout: None, archived_at: None, + last_opened_at: None, pr_url: None, pr_number: None, pr_provider: None, @@ -5993,6 +6005,7 @@ fn task_import_worktree( right_split_tabs: Vec::new(), split_layout: None, archived_at: None, + last_opened_at: None, pr_url: None, pr_number: None, pr_provider: None, @@ -6392,6 +6405,7 @@ fn task_create_sync(app: AppHandle, args: CreateTaskArgs) -> Result Result Result { Ok(w.spawn_count) } +/// How long a `last_opened_at` stamp stays fresh. A task switched away from +/// and back inside this window is one visit, not two, and rewriting the file +/// for the second one buys nothing the dashboard can render. +const TOUCH_MIN_SECS: i64 = 60; + +/// Stamp `w.last_opened_at` unless it is already younger than +/// [`TOUCH_MIN_SECS`]. Returns true iff the record changed and so needs +/// saving. +/// +/// `now` is a PARAMETER rather than a `Utc::now()` call inside, which is the +/// whole reason this is a free function: the bail is the interesting part and +/// it is untestable against a real clock. +/// +/// An unparseable stamp (hand-edited file, a record from some future schema) +/// re-stamps rather than bailing. So does a stamp in the FUTURE: a machine +/// whose clock jumped backwards would otherwise bail on every activation +/// until real time caught up, which can be hours. +fn touch_task_record(w: &mut Task, now: chrono::DateTime) -> bool { + let fresh = w + .last_opened_at + .as_deref() + .and_then(|s| chrono::DateTime::parse_from_rfc3339(s).ok()) + .map(|prev| { + let age = now.signed_duration_since(prev.with_timezone(&chrono::Utc)); + age >= chrono::Duration::zero() && age < chrono::Duration::seconds(TOUCH_MIN_SECS) + }) + .unwrap_or(false); + if fresh { return false; } + w.last_opened_at = Some(now.to_rfc3339()); + true +} + +/// Stamp `last_opened_at` on one task and return the stamp now on disk. +/// +/// Fires on EVERY activation (every sidebar click, every Cmd-number switch), +/// and two things follow from that. +/// +/// It reads exactly ONE record. The siblings above call `load_tasks_all()`, +/// re-parsing every task file of every profile, which is fine a handful of +/// times per session and wrong on a path the user walks dozens of times an +/// hour. +/// +/// It is SYNC on purpose, like those siblings. Every per-task setter in this +/// file is an unlocked read-modify-write of the whole record, and they get +/// away with it because sync commands run on the main thread and therefore +/// one after another. The first version of this command was async + +/// `spawn_blocking`, 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 that fires this), +/// and the e2e run turned up a task activated seconds earlier with +/// `last_opened_at: null` on disk: a sibling had read the record before this +/// wrote it and written its own copy back after. One small read and one +/// atomic write is not the IO-heavy case docs/ipc.md's long-running discipline +/// is about; the siblings do strictly more work on the same thread. See +/// docs/gotchas.md, "Task record setters serialize on the main thread". +#[tauri::command] +fn task_touch(id: String) -> Result { + task_touch_sync(id) +} + +fn task_touch_sync(id: String) -> Result { + // Sweep the profiles for the one file, the way `delete_task_file` does: + // the caller has only an id, and a `stat` per profile is nothing next to + // parsing every record in all of them. + for pid in profiles_registry().ids() { + let Ok(dir) = tasks_dir_in(&pid) else { continue }; + let Ok(s) = fs::read_to_string(dir.join(format!("{id}.json"))) else { continue }; + let Ok(mut w) = serde_json::from_str::(&s) else { continue }; + // `profile` is `serde(skip)`, so a record parsed straight from a file + // carries the DEFAULT profile, not the one it came from. Without this + // line `save_task` would write a second copy of a non-root profile's + // task into the root tree. + w.profile = pid.clone(); + if !touch_task_record(&mut w, chrono::Utc::now()) { + return Ok(w.last_opened_at.clone().unwrap_or_default()); + } + save_task(&w).map_err(|e| e.to_string())?; + return Ok(w.last_opened_at.clone().unwrap_or_default()); + } + Err("no such task".into()) +} + /// Set the persisted `has_resumable_history` flag for a task. /// Frontend calls this: /// - TRUE when a spawn has been alive past the rapid-failure window @@ -22206,7 +22303,7 @@ pub fn run() { repo_config_load, repo_config_load_at, repo_config_save, repo_config_scaffold, repo_config_add_allowed_host, repo_config_add_allowed_path, task_reorder, - task_restore, task_delete, task_run_script, task_run_script_stream, task_ensure_extra_ports, task_stop_script, task_record_spawn, task_set_has_history, task_set_agent_session_id, + task_restore, task_delete, task_run_script, task_run_script_stream, task_ensure_extra_ports, task_stop_script, task_record_spawn, task_set_has_history, task_touch, task_set_agent_session_id, task_set_tabs, task_set_tab_session_id, task_set_split_layout, task_set_right_tabs, task_set_right_tab_session_id, @@ -29599,6 +29696,117 @@ filename f.rs assert_eq!(back.agent_args, task.agent_args); } + // ── last_opened_at / task_touch ───────────────────────────────── + // + // The stamp is written on EVERY activation, so the 60s bail is the + // feature: without it a user flicking between two tasks with ⌘1/⌘2 + // rewrites two files per keystroke. The bail lives in + // `touch_task_record`, which takes `now` as an argument precisely so + // these cases can name a time instead of sleeping. + + // Every task record on disk today predates the field. They must + // deserialize with `None` rather than failing the whole load, and `None` + // has to survive the trip: it is what tells the dashboard to say nothing + // instead of guessing from `created`. + #[test] + fn a_task_written_before_last_opened_at_existed_reads_as_never_opened() { + let mut value = serde_json::to_value(Task::default()).unwrap(); + assert!(value.get("last_opened_at").is_some(), "the field stopped serializing"); + value.as_object_mut().unwrap().remove("last_opened_at"); + + let back: Task = serde_json::from_value(value).unwrap(); + assert_eq!(back.last_opened_at, None); + } + + #[test] + fn touching_a_never_opened_task_stamps_it_and_the_stamp_round_trips() { + let now = chrono::DateTime::parse_from_rfc3339("2026-01-01T12:00:00Z") + .unwrap() + .with_timezone(&chrono::Utc); + let mut task = Task::default(); + assert_eq!(task.last_opened_at, None); + + assert!(touch_task_record(&mut task, now), "a never-opened task must be stamped"); + let stamp = task.last_opened_at.clone().expect("stamped"); + + let back: Task = serde_json::from_value(serde_json::to_value(&task).unwrap()).unwrap(); + assert_eq!(back.last_opened_at.as_deref(), Some(stamp.as_str())); + // The value is a real RFC3339 instant, not just a string that survived. + assert_eq!( + chrono::DateTime::parse_from_rfc3339(&stamp).unwrap().with_timezone(&chrono::Utc), + now, + ); + } + + #[test] + fn a_second_touch_inside_the_window_does_not_rewrite_the_stamp() { + let now = chrono::DateTime::parse_from_rfc3339("2026-01-01T12:00:00Z") + .unwrap() + .with_timezone(&chrono::Utc); + let mut task = Task::default(); + touch_task_record(&mut task, now); + let first = task.last_opened_at.clone().unwrap(); + + // 59s later: the same visit, as far as anything that renders this is + // concerned. No write, and the value is byte-identical. + assert!(!touch_task_record(&mut task, now + chrono::Duration::seconds(59))); + assert_eq!(task.last_opened_at.as_deref(), Some(first.as_str())); + + // 61s later: a new visit. + assert!(touch_task_record(&mut task, now + chrono::Duration::seconds(61))); + assert_ne!(task.last_opened_at.as_deref(), Some(first.as_str())); + } + + // A stamp nobody can parse, and a stamp from the future, both re-stamp. + // Bailing on either would freeze the value: a clock that jumped backwards + // would otherwise suppress every activation for hours. + #[test] + fn an_unreadable_or_future_stamp_is_replaced_rather_than_trusted() { + let now = chrono::DateTime::parse_from_rfc3339("2026-01-01T12:00:00Z") + .unwrap() + .with_timezone(&chrono::Utc); + + let mut garbage = Task { last_opened_at: Some("yesterday".into()), ..Default::default() }; + assert!(touch_task_record(&mut garbage, now)); + assert_eq!(garbage.last_opened_at.as_deref(), Some(now.to_rfc3339().as_str())); + + let mut ahead = + Task { last_opened_at: Some("2027-01-01T00:00:00Z".into()), ..Default::default() }; + assert!(touch_task_record(&mut ahead, now)); + assert_eq!(ahead.last_opened_at.as_deref(), Some(now.to_rfc3339().as_str())); + } + + #[test] + fn task_touch_writes_back_to_the_profile_the_task_came_from() { + // The command reads ONE file rather than `load_tasks_all()`, which + // means it parses a record whose `profile` tag is `serde(skip)` and + // therefore default. If it forgets to re-tag, the save lands in the + // root profile and the task exists twice. + with_scratch_data_dir(|data| { + crate::profiles::save_registry(data, &two_profile_registry()).unwrap(); + crate::save_task(&a_task("t2", ProfileId::Slug("home".into()))).unwrap(); + + let stamp = crate::task_touch_sync("t2".into()).expect("the task is there"); + assert!(chrono::DateTime::parse_from_rfc3339(&stamp).is_ok(), "not RFC3339: {stamp}"); + assert!(!data.join("tasks/t2.json").exists(), "the touch moved t2 into the root"); + + let home = crate::load_tasks_in(&ProfileId::Slug("home".into())); + assert_eq!(home.len(), 1); + assert_eq!(home[0].last_opened_at.as_deref(), Some(stamp.as_str())); + + // Straight back in: inside the window, so the same stamp comes + // back and nothing is rewritten. + assert_eq!(crate::task_touch_sync("t2".into()).unwrap(), stamp); + }); + } + + #[test] + fn touching_a_task_that_does_not_exist_is_an_error() { + with_scratch_data_dir(|_data| { + assert_eq!(crate::task_touch_sync("nope".into()), Err("no such task".into())); + }); + } + // ── Extra named ports (GH #196) ───────────────────────────────── #[test] diff --git a/src/components/task/TerminalPane.tsx b/src/components/task/TerminalPane.tsx index a03a5148..e8790521 100644 --- a/src/components/task/TerminalPane.tsx +++ b/src/components/task/TerminalPane.tsx @@ -2308,9 +2308,11 @@ const captureArmedRef = useRef(false); // race possible). Render the warning chip immediately when the // cage degraded. setSandboxWarning(spawn.sandbox.warning || null); - // Fire-and-forget analytics. Real resume gating lives on the - // has_resumable_history flag below, not here. - ipc.taskRecordSpawn(task.id).catch(() => {}); + // Persist the spawn and fold the new count back into the store, so a + // task launched this session stops reading as never-spawned. Real + // resume gating still lives on the has_resumable_history flag below, + // not here. + useApp.getState().recordSpawn(task.id); // Launching a task is exactly the moment its PR/MR status is worth // knowing, not something to wait on the user opening the Git tab // for - only the primary agent tab counts as "the task launched", diff --git a/src/lib/cliAgentState.test.ts b/src/lib/cliAgentState.test.ts index ad7f7e69..9761a526 100644 --- a/src/lib/cliAgentState.test.ts +++ b/src/lib/cliAgentState.test.ts @@ -9,6 +9,10 @@ import { vi } from "vitest"; vi.mock("@tauri-apps/api/event", () => ({ listen: vi.fn().mockResolvedValue(() => {}) })); vi.mock("@tauri-apps/api/core", () => ({ invoke: vi.fn().mockResolvedValue(undefined) })); vi.mock("@/lib/ipc", () => ({ + // Every task activation stamps `last_opened_at` through these; a mock + // missing them throws on property access, not on call. + taskTouch: vi.fn().mockResolvedValue("2026-01-01T00:00:00Z"), + taskRecordSpawn: vi.fn().mockResolvedValue(1), projectsList: vi.fn().mockResolvedValue([]), tasksList: vi.fn().mockResolvedValue([]), settingsLoad: vi.fn().mockResolvedValue({ agents: [] }), diff --git a/src/lib/dirTabs.test.ts b/src/lib/dirTabs.test.ts index 43f05764..1628d2a1 100644 --- a/src/lib/dirTabs.test.ts +++ b/src/lib/dirTabs.test.ts @@ -3,6 +3,10 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; // Same mock set app.test.ts uses — the store pulls IPC in at import time. vi.mock("@/lib/ipc", () => ({ + // Every task activation stamps `last_opened_at` through these; a mock + // missing them throws on property access, not on call. + taskTouch: vi.fn().mockResolvedValue("2026-01-01T00:00:00Z"), + taskRecordSpawn: vi.fn().mockResolvedValue(1), ptyWrite: vi.fn(), ptyKill: vi.fn().mockResolvedValue(undefined), projectsList: vi.fn().mockResolvedValue([]), diff --git a/src/lib/ipc.ts b/src/lib/ipc.ts index a5c2b225..21e36d52 100644 --- a/src/lib/ipc.ts +++ b/src/lib/ipc.ts @@ -579,6 +579,11 @@ export const taskRecentDenials = (id: string, minutes?: number) => // miss the only emission. export const taskRename = (id: string, name: string) => invoke("task_rename", { id, name }); export const taskRecordSpawn = (id: string) => invoke("task_record_spawn", { id }); +/** Stamp the task's `last_opened_at` and resolve with whatever is now on disk + * (unchanged when the previous stamp is still inside the 60s window). Fired on + * every activation, so the Rust side reads ONE record rather than every + * profile's whole tasks dir. */ +export const taskTouch = (id: string) => invoke("task_touch", { id }); export const taskSetHasHistory = (id: string, value: boolean) => invoke("task_set_has_history", { id, value }); export const taskSetAgentSessionId = (id: string, cli: string, uuid: string) => diff --git a/src/lib/trayAttention.test.ts b/src/lib/trayAttention.test.ts index af211a13..8c89e5ed 100644 --- a/src/lib/trayAttention.test.ts +++ b/src/lib/trayAttention.test.ts @@ -7,6 +7,10 @@ import { vi } from "vitest"; vi.mock("@tauri-apps/api/event", () => ({ listen: vi.fn().mockResolvedValue(() => {}) })); vi.mock("@tauri-apps/api/core", () => ({ invoke: vi.fn().mockResolvedValue(undefined) })); vi.mock("@/lib/ipc", () => ({ + // Every task activation stamps `last_opened_at` through these; a mock + // missing them throws on property access, not on call. + taskTouch: vi.fn().mockResolvedValue("2026-01-01T00:00:00Z"), + taskRecordSpawn: vi.fn().mockResolvedValue(1), projectsList: vi.fn().mockResolvedValue([]), tasksList: vi.fn().mockResolvedValue([]), settingsLoad: vi.fn().mockResolvedValue({ agents: [] }), diff --git a/src/lib/types.ts b/src/lib/types.ts index 5b4d4a9a..ec5cdf57 100644 --- a/src/lib/types.ts +++ b/src/lib/types.ts @@ -282,6 +282,15 @@ export interface Task { created: string; archived: boolean; archived_at?: string; + /** RFC3339 UTC, written by the app on every task activation (`setActiveTask`, + * and `task_touch` behind it). Never typed by a person. + * + * Serde writes Rust's `None` as `null`, so this arrives as `null` on a record + * that has one and is ABSENT on a record written before the field existed. + * Both mean the same thing here, "never opened since this was recorded", so + * collapsing them with `??` is correct. (CLAUDE.md forbids that only where + * `null` is a distinct answer from "nothing there yet"; it is not, here.) */ + last_opened_at?: string | null; /** Manual sidebar position within the project, written by drag-to-reorder * (`taskReorder`). Undefined on tasks the user has never dragged, which * sort AFTER any ordered sibling — so untouched projects stay in creation diff --git a/src/store/agentHooksSync.test.ts b/src/store/agentHooksSync.test.ts index a59c2c15..890c0420 100644 --- a/src/store/agentHooksSync.test.ts +++ b/src/store/agentHooksSync.test.ts @@ -12,6 +12,10 @@ const agentHooksStatus = vi.fn(async (id: string) => { }); vi.mock("@/lib/ipc", () => ({ + // Every task activation stamps `last_opened_at` through these; a mock + // missing them throws on property access, not on call. + taskTouch: vi.fn().mockResolvedValue("2026-01-01T00:00:00Z"), + taskRecordSpawn: vi.fn().mockResolvedValue(1), agentHooksSync: (...a: unknown[]) => agentHooksSync(...(a as [])), agentHooksStatus: (id: string) => agentHooksStatus(id), ptyWrite: vi.fn(), ptyKill: vi.fn().mockResolvedValue(undefined), diff --git a/src/store/app.test.ts b/src/store/app.test.ts index 6cd0042e..45ead7c2 100644 --- a/src/store/app.test.ts +++ b/src/store/app.test.ts @@ -3,6 +3,10 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; // Mocks must be declared before the module under test is imported. vi.mock("@/lib/ipc", () => ({ + // Every task activation stamps `last_opened_at` through these; a mock + // missing them throws on property access, not on call. + taskTouch: vi.fn().mockResolvedValue("2026-01-01T00:00:00Z"), + taskRecordSpawn: vi.fn().mockResolvedValue(1), ptyWrite: vi.fn(), ptyKill: vi.fn().mockResolvedValue(undefined), projectsList: vi.fn().mockResolvedValue([]), @@ -29,7 +33,7 @@ vi.mock("@/lib/agents", () => ({ vi.mock("@tauri-apps/api/core", () => ({ invoke: vi.fn().mockResolvedValue(undefined) })); import { invoke } from "@tauri-apps/api/core"; -import { isTabOnScreenIn, isUserWatching, RECENT_TASKS_CAP, useApp } from "@/store/app"; +import { isTabOnScreenIn, isUserWatching, RECENT_TASKS_CAP, TOUCH_MIN_MS, useApp } from "@/store/app"; import * as ipc from "@/lib/ipc"; import { markUnattendedSpawn, takeUnattendedSpawn } from "@/lib/unattendedSpawns"; import type { QueueItem, PaneLeaf, Tab, TerminalTab, PersistedTab } from "@/lib/types"; @@ -1581,3 +1585,209 @@ describe("previewPlace", () => { expect(real).toBeGreaterThan(1); }); }); + +// ── last_opened_at (task activation stamp) ──────────────────────────── +// +// The stamp is written on EVERY activation, which is the hottest store path +// the sidebar has: a ⌘1/⌘2 flick between two tasks is two activations per +// keystroke. So both halves of the design are count assertions, the class that +// survives a 3-core CI runner (docs/perf-ci.md): +// +// 1. a second activation inside TOUCH_MIN_MS writes NOTHING, keeping the +// `tasks` array identity that every mounted task's selectors hang off +// (docs/performance.md bear trap 8); and +// 2. the write that does happen rides INSIDE the set() `setActiveTask` was +// making anyway, so the feature adds zero subscriber notifications. +describe("setActiveTask last_opened_at", () => { + const stamped = (id: string) => useApp.getState().tasks.find(w => w.id === id)?.last_opened_at; + + beforeEach(() => { + useApp.setState({ tasks: [makeTask({ id: "ws1" }), makeTask({ id: "ws2" })] }); + }); + + it("stamps the task and persists it exactly once", () => { + const before = Date.now(); + useApp.getState().setActiveTask("ws1"); + + const at = stamped("ws1"); + expect(at).toBeTruthy(); + expect(Date.parse(at!)).toBeGreaterThanOrEqual(before); + expect(ipc.taskTouch).toHaveBeenCalledTimes(1); + expect(ipc.taskTouch).toHaveBeenCalledWith("ws1"); + // The sibling is untouched: a stamp is per task, not per activation. + expect(stamped("ws2")).toBeUndefined(); + }); + + it("does not touch the store again inside the 60s window", () => { + // Both tasks stamped once, which is the only real work here. + useApp.getState().setActiveTask("ws1"); + useApp.getState().setActiveTask("ws2"); + const first = stamped("ws1"); + + // Now the ⌘1/⌘2 flick: straight back and forth, all inside the window. + const before = useApp.getState(); + useApp.getState().setActiveTask("ws1"); + useApp.getState().setActiveTask("ws2"); + useApp.getState().setActiveTask("ws1"); + const after = useApp.getState(); + + // Same ARRAY, not merely equal: a fresh `tasks` is what invalidates every + // selector in every mounted task. + expect(after.tasks).toBe(before.tasks); + expect(stamped("ws1")).toBe(first); + // Still the two opening touches: the flick added none. + expect(ipc.taskTouch).toHaveBeenCalledTimes(2); + expect(vi.mocked(ipc.taskTouch).mock.calls.map(c => c[0])).toEqual(["ws1", "ws2"]); + }); + + it("re-stamps once the window has passed", () => { + const old = new Date(Date.now() - TOUCH_MIN_MS - 1_000).toISOString(); + useApp.setState({ tasks: [makeTask({ id: "ws1", last_opened_at: old })] }); + + useApp.getState().setActiveTask("ws1"); + + expect(stamped("ws1")).not.toBe(old); + expect(ipc.taskTouch).toHaveBeenCalledTimes(1); + }); + + // Both halves of the freshness predicate's escape hatch, pinned to the same + // rule `touch_task_record` follows in lib.rs. They matter here more than + // there: a stamp THIS side calls fresh never reaches Rust to be judged. + // + // NaN because `Date.now() - Date.parse("nonsense")` is NaN and every + // comparison against NaN is false; a guard written as `>= TOUCH_MIN_MS` + // would freeze the value forever. Negative because a clock that jumped + // backwards would otherwise suppress every activation until it caught up. + it("replaces a stamp it cannot parse", () => { + useApp.setState({ tasks: [makeTask({ id: "ws1", last_opened_at: "yesterday" })] }); + + useApp.getState().setActiveTask("ws1"); + + expect(Date.parse(stamped("ws1")!)).not.toBeNaN(); + expect(ipc.taskTouch).toHaveBeenCalledTimes(1); + }); + + it("replaces a stamp from the future rather than waiting it out", () => { + const ahead = new Date(Date.now() + 3_600_000).toISOString(); + useApp.setState({ tasks: [makeTask({ id: "ws1", last_opened_at: ahead })] }); + + useApp.getState().setActiveTask("ws1"); + + expect(stamped("ws1")).not.toBe(ahead); + expect(Date.parse(stamped("ws1")!)).toBeLessThanOrEqual(Date.now()); + expect(ipc.taskTouch).toHaveBeenCalledTimes(1); + }); + + it("adds ZERO subscriber notifications to an activation", () => { + const notificationsFor = (id: string) => { + let n = 0; + const unsub = useApp.subscribe(() => { n++; }); + useApp.getState().setActiveTask(id); + unsub(); + return n; + }; + + // Warm both so the second measurement below is a BAILED touch, not a + // first visit. + useApp.getState().setActiveTask("ws2"); + useApp.getState().setActiveTask("ws1"); + + // An activation that does stamp, versus one inside the window that does + // not. The measurement is the comparison: the stamp rides inside a set() + // `setActiveTask` was making anyway, so the two must be equal. + useApp.setState({ tasks: [makeTask({ id: "ws1" }), makeTask({ id: "ws2" })] }); + useApp.getState().setActiveTask("ws2"); + vi.mocked(ipc.taskTouch).mockClear(); + const stamping = notificationsFor("ws1"); + const touchCallsWhileStamping = vi.mocked(ipc.taskTouch).mock.calls.length; + + useApp.getState().setActiveTask("ws2"); + const bailing = notificationsFor("ws1"); + + expect(touchCallsWhileStamping).toBe(1); + expect(bailing).toBe(stamping); + // 2 is the PRE-EXISTING cost of one activation with no tabs open: the + // main set(), plus the read-clearing set() under it. The stamp is inside + // the first of those, so this number must not move. + expect(stamping).toBe(2); + }); + + it("never touches when the active task is cleared", () => { + useApp.getState().setActiveTask("ws1"); + vi.mocked(ipc.taskTouch).mockClear(); + + useApp.getState().setActiveTask(null); + + expect(ipc.taskTouch).not.toHaveBeenCalled(); + }); + + // Agent Race mounts N tasks at once without focusing them. Mounting is not + // opening, and stamping there would backdate every task in the race to the + // moment the user pressed one button. + it("mountTasks never touches", () => { + useApp.getState().mountTasks(["ws1", "ws2"]); + + expect(ipc.taskTouch).not.toHaveBeenCalled(); + expect(stamped("ws1")).toBeUndefined(); + }); + + it("is a no-op for an id that resolves to no task", () => { + useApp.getState().setActiveTask("nope"); + + expect(ipc.taskTouch).not.toHaveBeenCalled(); + }); +}); + +// ── recordSpawn ─────────────────────────────────────────────────────── +// +// `task_record_spawn` always WROTE the count; nothing read the answer back, +// so a task created this session stayed at spawn_count 0 in the store until +// the next `loadAll`. The dashboard's derived phase reads that field. +describe("recordSpawn", () => { + const count = (id: string) => useApp.getState().tasks.find(w => w.id === id)?.spawn_count; + + beforeEach(() => { + useApp.setState({ tasks: [makeTask({ id: "ws1", spawn_count: 0 })] }); + }); + + it("folds the persisted count back into the store", async () => { + vi.mocked(ipc.taskRecordSpawn).mockResolvedValueOnce(3); + + useApp.getState().recordSpawn("ws1"); + + await vi.waitFor(() => expect(count("ws1")).toBe(3)); + expect(ipc.taskRecordSpawn).toHaveBeenCalledWith("ws1"); + }); + + it("leaves state identity alone when the count did not change", async () => { + vi.mocked(ipc.taskRecordSpawn).mockResolvedValueOnce(0); + const before = useApp.getState(); + + useApp.getState().recordSpawn("ws1"); + await vi.waitFor(() => expect(ipc.taskRecordSpawn).toHaveBeenCalled()); + await Promise.resolve(); + + expect(useApp.getState()).toBe(before); + expect(useApp.getState().tasks).toBe(before.tasks); + }); + + it("drops the answer for a task that is gone by the time it lands", async () => { + vi.mocked(ipc.taskRecordSpawn).mockResolvedValueOnce(2); + + useApp.getState().recordSpawn("ws1"); + // Archived and reloaded out from under the in-flight call. + useApp.setState({ tasks: [] }); + await vi.waitFor(() => expect(ipc.taskRecordSpawn).toHaveBeenCalled()); + await Promise.resolve(); + + expect(useApp.getState().tasks).toEqual([]); + }); + + it("survives a rejected write without throwing", async () => { + vi.mocked(ipc.taskRecordSpawn).mockRejectedValueOnce(new Error("disk full")); + + expect(() => useApp.getState().recordSpawn("ws1")).not.toThrow(); + await vi.waitFor(() => expect(ipc.taskRecordSpawn).toHaveBeenCalled()); + expect(count("ws1")).toBe(0); + }); +}); diff --git a/src/store/app.ts b/src/store/app.ts index e7f8b1dc..3df785ad 100644 --- a/src/store/app.ts +++ b/src/store/app.ts @@ -217,6 +217,18 @@ export interface AppState { * so opening the task again respawns the agents with their conversations * resumed. */ stopTask: (taskId: string) => void; + /** Persist one more agent spawn for the task and fold the new count back + * into the store's copy of it. + * + * `task_record_spawn` has always written the number to disk, but nothing + * read the answer: the store's `spawn_count` was only ever refreshed by + * `loadAll`, so a task created this session read as never-spawned until + * the next reload. The dashboard's derived phase (a follow-up commit) reads + * that field, and "created it, launched an agent, still says never + * spawned" is the bug that would follow. + * + * Bails when the count is unchanged, leaving state identity intact. */ + recordSpawn: (taskId: string) => void; setView: (page: View["page"]) => void; openSettings: (tab?: View["settingsTab"], repoId?: string, highlight?: string) => void; closeSettings: () => void; @@ -440,6 +452,11 @@ const LS_RECENT_TASKS = scoped("recentTasks"); // string[] of task ids, newest * you were just doing, not a second history view — `History` already lists * everything, and a long list here would push the projects off the screen. */ export const RECENT_TASKS_CAP = 8; +/** How long a task's `last_opened_at` stamp stays fresh. Mirrors + * `TOUCH_MIN_SECS` in lib.rs: the two guards are independent (this one skips + * the store write and the IPC, that one skips the disk write), and a task + * switched away from and back inside the window is one visit either way. */ +export const TOUCH_MIN_MS = 60_000; const initialCollapsed = (() => { try { return JSON.parse(localStorage.getItem(LS_COLLAPSED_PROJ) || "{}"); } catch { return {}; } })(); const initialCollapsedTask = (() => { try { return JSON.parse(localStorage.getItem(LS_COLLAPSED_TASK) || "{}"); } catch { return {}; } })(); const initialCollapsedGrp = (() => { try { return JSON.parse(localStorage.getItem(LS_COLLAPSED_GRP) || "{}"); } catch { return {}; } })(); @@ -879,8 +896,34 @@ export const useApp = create((set, get) => ({ // to a task under a collapsed project. let nextCollapsed = get().collapsedProjects; let nextCollapsedGroups = get().collapsedGroups; + let nextTasks = get().tasks; if (id) { - const task = get().tasks.find(w => w.id === id); + const task = nextTasks.find(w => w.id === id); + // Stamp "last opened" and persist it, at most once per TOUCH_MIN_MS. + // + // The stamp is computed HERE rather than taken from what `task_touch` + // resolves with: the two differ by the IPC round trip, nothing renders + // milliseconds, and writing the reply back would cost a second copy of + // the whole ~233-key state (docs/performance.md bear trap 8) on a path + // the user walks dozens of times an hour. So the promise is + // fire-and-forget and its value is dropped on purpose. + // + // Same predicate as `touch_task_record` in lib.rs, and it has to be: + // a stamp this side thinks is fresh never reaches the Rust side to be + // judged. So an age BELOW zero (clock jumped backwards, or a stamp from + // a machine that is ahead) re-stamps rather than bailing, which would + // otherwise suppress every activation until real time caught up. + // NaN re-stamps too: `NaN >= 0` is false, so a corrupt stamp read from + // disk cannot freeze the value forever. + const opened = task?.last_opened_at ? Date.parse(task.last_opened_at) : NaN; + const age = Date.now() - opened; + if (task && !(age >= 0 && age < TOUCH_MIN_MS)) { + const stamp = new Date().toISOString(); + // Rides along in the set() below, exactly like `recentTasks`: a + // separate write would re-run every mounted task's selectors. + nextTasks = nextTasks.map(w => (w.id === id ? { ...w, last_opened_at: stamp } : w)); + ipc.taskTouch(id).catch(() => {}); + } // Force the parent project expanded (explicit false) — covers the // case where it was either explicitly collapsed by the user OR // default-collapsed-because-empty after a worktree just got added. @@ -915,6 +958,7 @@ export const useApp = create((set, get) => ({ collapsedProjects: nextCollapsed, collapsedGroups: nextCollapsedGroups, recentTasks: nextRecent, + tasks: nextTasks, }); if (id) { // Mark the WHOLE task as read on activation. Previously we @@ -956,6 +1000,19 @@ export const useApp = create((set, get) => ({ } }, + recordSpawn: (taskId) => { + ipc.taskRecordSpawn(taskId).then(count => { + set(s => { + // Read `tasks` from the CALLBACK's state, never from a value captured + // before the await: a `loadAll` can land in between and replace the + // whole array. + const task = s.tasks.find(w => w.id === taskId); + if (!task || task.spawn_count === count) return s; + return { tasks: s.tasks.map(w => (w.id === taskId ? { ...w, spawn_count: count } : w)) }; + }); + }).catch(() => {}); + }, + setView: (page) => set({ view: { page }, activeTaskId: null }), // Opening Settings does NOT clear `activeTaskId` or change `view.page` // away from whatever the user was on — Settings renders as a fixed diff --git a/src/store/cliPrompts.integration.test.ts b/src/store/cliPrompts.integration.test.ts index 5763f385..0d0dd433 100644 --- a/src/store/cliPrompts.integration.test.ts +++ b/src/store/cliPrompts.integration.test.ts @@ -14,7 +14,12 @@ // localStorage once at module load. import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; -vi.mock("@/lib/ipc", () => ({})); +// Every task activation stamps `last_opened_at` through these; a mock missing +// them throws on property access, not on call. +vi.mock("@/lib/ipc", () => ({ + taskTouch: vi.fn().mockResolvedValue("2026-01-01T00:00:00Z"), + taskRecordSpawn: vi.fn().mockResolvedValue(1), +})); vi.mock("@tauri-apps/api/event", () => ({ listen: vi.fn().mockResolvedValue(() => {}) })); vi.mock("@tauri-apps/api/core", () => ({ invoke: vi.fn().mockResolvedValue(null) })); diff --git a/src/store/cliSend.integration.test.ts b/src/store/cliSend.integration.test.ts index b23e0d7e..8f274147 100644 --- a/src/store/cliSend.integration.test.ts +++ b/src/store/cliSend.integration.test.ts @@ -9,6 +9,10 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; vi.mock("@/lib/ipc", () => ({ + // Every task activation stamps `last_opened_at` through these; a mock + // missing them throws on property access, not on call. + taskTouch: vi.fn().mockResolvedValue("2026-01-01T00:00:00Z"), + taskRecordSpawn: vi.fn().mockResolvedValue(1), ptyKill: vi.fn().mockResolvedValue(undefined), ptyAlive: vi.fn().mockResolvedValue(true), taskSetTabs: vi.fn().mockResolvedValue(undefined), diff --git a/src/store/cliTab.integration.test.ts b/src/store/cliTab.integration.test.ts index 8ae89457..3ffee261 100644 --- a/src/store/cliTab.integration.test.ts +++ b/src/store/cliTab.integration.test.ts @@ -17,6 +17,10 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; vi.mock("@/lib/ipc", () => ({ + // Every task activation stamps `last_opened_at` through these; a mock + // missing them throws on property access, not on call. + taskTouch: vi.fn().mockResolvedValue("2026-01-01T00:00:00Z"), + taskRecordSpawn: vi.fn().mockResolvedValue(1), ptyKill: vi.fn().mockResolvedValue(undefined), taskSetTabs: vi.fn().mockResolvedValue(undefined), taskSetTabSessionId: vi.fn().mockResolvedValue(undefined), diff --git a/src/store/cliTabClose.integration.test.ts b/src/store/cliTabClose.integration.test.ts index 208c5580..03e2dc5c 100644 --- a/src/store/cliTabClose.integration.test.ts +++ b/src/store/cliTabClose.integration.test.ts @@ -15,6 +15,10 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; vi.mock("@/lib/ipc", () => ({ + // Every task activation stamps `last_opened_at` through these; a mock + // missing them throws on property access, not on call. + taskTouch: vi.fn().mockResolvedValue("2026-01-01T00:00:00Z"), + taskRecordSpawn: vi.fn().mockResolvedValue(1), ptyKill: vi.fn().mockResolvedValue(undefined), taskSetTabs: vi.fn().mockResolvedValue(undefined), taskSetTabSessionId: vi.fn().mockResolvedValue(undefined), diff --git a/src/store/pr.test.ts b/src/store/pr.test.ts index 869faac2..b377b8dd 100644 --- a/src/store/pr.test.ts +++ b/src/store/pr.test.ts @@ -3,6 +3,10 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; // Mocks must be declared before the modules under test are imported. vi.mock("@/lib/ipc", () => ({ + // Every task activation stamps `last_opened_at` through these; a mock + // missing them throws on property access, not on call. + taskTouch: vi.fn().mockResolvedValue("2026-01-01T00:00:00Z"), + taskRecordSpawn: vi.fn().mockResolvedValue(1), detectForges: vi.fn().mockResolvedValue([]), taskPrStatus: vi.fn(), taskPrComments: vi.fn().mockResolvedValue([]), diff --git a/src/store/race.integration.test.ts b/src/store/race.integration.test.ts index 0f2397c0..b12ef452 100644 --- a/src/store/race.integration.test.ts +++ b/src/store/race.integration.test.ts @@ -11,6 +11,10 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; vi.mock("@/lib/ipc", () => ({ + // Every task activation stamps `last_opened_at` through these; a mock + // missing them throws on property access, not on call. + taskTouch: vi.fn().mockResolvedValue("2026-01-01T00:00:00Z"), + taskRecordSpawn: vi.fn().mockResolvedValue(1), taskCreate: vi.fn().mockResolvedValue(undefined), taskSetYolo: vi.fn().mockResolvedValue(undefined), })); diff --git a/src/store/recentPlaces.test.ts b/src/store/recentPlaces.test.ts index 9be0cf34..2a1099c7 100644 --- a/src/store/recentPlaces.test.ts +++ b/src/store/recentPlaces.test.ts @@ -11,6 +11,9 @@ vi.mock("@/lib/ipc", () => ({ detectClis: vi.fn().mockResolvedValue([]), taskSetTabs: vi.fn().mockResolvedValue(undefined), taskSetTabSessionId: vi.fn().mockResolvedValue(undefined), + // `setActiveTask` stamps `last_opened_at` through this, so every file that + // mocks the ipc module and drives an activation needs it present. + taskTouch: vi.fn().mockResolvedValue(null), })); vi.mock("@/lib/tabFocus", () => ({ focusTerminalTab: vi.fn(), diff --git a/src/store/resume.integration.test.ts b/src/store/resume.integration.test.ts index 19c80054..02757ab6 100644 --- a/src/store/resume.integration.test.ts +++ b/src/store/resume.integration.test.ts @@ -8,6 +8,10 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; vi.mock("@/lib/ipc", () => ({ + // Every task activation stamps `last_opened_at` through these; a mock + // missing them throws on property access, not on call. + taskTouch: vi.fn().mockResolvedValue("2026-01-01T00:00:00Z"), + taskRecordSpawn: vi.fn().mockResolvedValue(1), ptyKill: vi.fn().mockResolvedValue(undefined), taskSetTabs: vi.fn().mockResolvedValue(undefined), taskSetTabSessionId: vi.fn().mockResolvedValue(undefined), diff --git a/src/store/scratchpad.integration.test.ts b/src/store/scratchpad.integration.test.ts index 020c54a2..ecf88c3f 100644 --- a/src/store/scratchpad.integration.test.ts +++ b/src/store/scratchpad.integration.test.ts @@ -11,6 +11,10 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; vi.mock("@/lib/ipc", () => ({ + // Every task activation stamps `last_opened_at` through these; a mock + // missing them throws on property access, not on call. + taskTouch: vi.fn().mockResolvedValue("2026-01-01T00:00:00Z"), + taskRecordSpawn: vi.fn().mockResolvedValue(1), ptyWrite: vi.fn(), ptyKill: vi.fn().mockResolvedValue(undefined), projectsList: vi.fn().mockResolvedValue([]), diff --git a/src/store/selectorFanout.test.ts b/src/store/selectorFanout.test.ts index 48372395..21cf739f 100644 --- a/src/store/selectorFanout.test.ts +++ b/src/store/selectorFanout.test.ts @@ -21,6 +21,10 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; // Same mocks as app.test.ts — importing the store pulls in the ipc layer. vi.mock("@/lib/ipc", () => ({ + // Every task activation stamps `last_opened_at` through these; a mock + // missing them throws on property access, not on call. + taskTouch: vi.fn().mockResolvedValue("2026-01-01T00:00:00Z"), + taskRecordSpawn: vi.fn().mockResolvedValue(1), ptyWrite: vi.fn(), ptyKill: vi.fn().mockResolvedValue(undefined), projectsList: vi.fn().mockResolvedValue([]), From afc55f4d24b1603f0026f8a96028ff3811dd4a51 Mon Sep 17 00:00:00 2001 From: Vinodkumar Naidu <7994336+nvkvin@users.noreply.github.com> Date: Mon, 14 Sep 2026 00:14:34 +0530 Subject: [PATCH 02/10] feat(dashboard): derive a task phase, filter by it, and show the age "Which of these am I still working on?" had nowhere to live. PR #292 tried a hand-set status and it was dropped in review: three of its five states repeated the PR chip, the work badge and archiving, and the two that did not had to be kept current by hand, so they went stale next to live signals. Same question, opposite mechanism: nothing here is typed by a person. `taskPhase()` derives Backlog / In progress / In review / Done at render from the task record and the PR store, and stores nothing, so it cannot disagree with the chip. Draft is In progress, a closed unmerged PR falls back to In progress, changes_requested stays In review, failing checks do not move it, archived beats merged, and a failed lookup falls through to the record. The decisions and their reasons are in the file header and docs/ui.md. The dashboard gets a filter row with fleet-wide counts (Backlog only while it has members: every GUI create activates and spawns, so it is rarely non-empty), cards and folders drop out under a filter, and a row shows a faint age from one day since it was last opened, nothing for records written before `last_opened_at` existed. Neither carries colour and the phase is not written on the row: the PR chip owns colour on this page, and a row saying "In review" beside a chip that already says open is the redundancy #292 was rejected for. The sidebar is untouched in this pass. `relativeDayLabel` is History's date ladder, extracted so the two surfaces cannot disagree about "3 weeks ago". e2e: dashboard phases and age in projects.e2e.ts, driven through real creates, opens and seeded PR snapshots and read off the row's `data-task-phase`; the persistence half travels null -> fresh on a task nothing had opened. Refs #292 --- docs/e2e-coverage.md | 2 + docs/ui.md | 73 +++++- e2e/specs/projects.e2e.ts | 361 ++++++++++++++++++++++++++++- src/components/views/Dashboard.tsx | 290 ++++++++++++++++++++--- src/components/views/History.tsx | 14 +- src/lib/relativeDay.test.ts | 66 ++++++ src/lib/relativeDay.ts | 31 +++ src/lib/taskPhase.test.ts | 234 +++++++++++++++++++ src/lib/taskPhase.ts | 115 +++++++++ src/store/app.ts | 13 +- src/store/ui.test.ts | 38 +++ src/store/ui.ts | 13 ++ 12 files changed, 1192 insertions(+), 58 deletions(-) create mode 100644 src/lib/relativeDay.test.ts create mode 100644 src/lib/relativeDay.ts create mode 100644 src/lib/taskPhase.test.ts create mode 100644 src/lib/taskPhase.ts diff --git a/docs/e2e-coverage.md b/docs/e2e-coverage.md index 89198ddb..cc5dca44 100644 --- a/docs/e2e-coverage.md +++ b/docs/e2e-coverage.md @@ -183,6 +183,8 @@ 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 | A task created but never opened reads `backlog` on its row and is counted by the Backlog pill; opening it once (the pane mounts, fakeagent spawns, `recordSpawn` folds the count back) moves the row to `in_progress` and drops the pill count by one, or removes the pill when that was the last one; a seeded PR then drives the row 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`); 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 | `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. 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`, which is the one assertion in these two rows with no DOM to read instead | `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/ui.md b/docs/ui.md index 2ec26f80..bb55393d 100644 --- a/docs/ui.md +++ b/docs/ui.md @@ -757,6 +757,70 @@ 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`, and nothing a person types. There +is no status field to set and none to go stale. Four values, first match wins: +**Done** when the task is archived or its PR is merged, **In review** when the +PR is open, **In progress** when the PR is draft or closed-unmerged or the task +has ever spawned (`spawn_count > 0`) or has resumable history, **Backlog** +otherwise. + +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 Backlog, 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 lookup has `pr === null` like "no PR" does and +therefore falls through to the record, so a machine with no `gh`/`glab` still +phases correctly; a shell spawn counts as progress, because `task_record_spawn` +fires for every spawn; and a main-checkout task is never polled at all +(`pollableTasks` skips `is_main_checkout`), so it only leaves In progress by +being archived. Backlog is rare in practice: every GUI create path activates +the new task and activation spawns its default tab, so a task is In progress +within a second of existing. + +**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 ` + ); +} + // A recently visited task, as a chip. Terse on purpose: this row is a way back // into what you were just doing, and anything wider than the name plus its -// project would push the projects list off the first screen. +// project would push the projects list off the first screen. That is why it +// carries no age label and is not filtered by the phase pills: these eight are +// where you just were, which is a different question from what state the fleet +// is in. function RecentChip({ task: w, ctx, projectName, onOpen }: { task: Task; ctx: TaskRowContext; projectName: string | undefined; onOpen: () => void; }) { diff --git a/src/components/views/History.tsx b/src/components/views/History.tsx index 7e14a913..eea77dc6 100644 --- a/src/components/views/History.tsx +++ b/src/components/views/History.tsx @@ -7,17 +7,7 @@ import { TaskLocationIcon } from "@/components/TaskLocationIcon"; import { cn } from "@/lib/utils"; import { ChevronRight, Search, Trash2 } from "lucide-react"; import type { Task } from "@/lib/types"; - -function groupLabel(iso: string): string { - const diffDays = Math.floor((Date.now() - new Date(iso).getTime()) / 86_400_000); - if (diffDays === 0) return "Today"; - if (diffDays === 1) return "Yesterday"; - if (diffDays < 7) return `${diffDays} days ago`; - if (diffDays < 14) return "Last week"; - if (diffDays < 21) return "2 weeks ago"; - if (diffDays < 28) return "3 weeks ago"; - return new Intl.DateTimeFormat(undefined, { month: "long", year: "numeric" }).format(new Date(iso)); -} +import { relativeDayLabel } from "@/lib/relativeDay"; function fmtDate(iso: string): string { const d = new Date(iso); @@ -58,7 +48,7 @@ export function HistoryView() { const groups = useMemo(() => { const map = new Map(); for (const w of archived) { - const key = groupLabel(w.archived_at ?? w.created); + const key = relativeDayLabel(w.archived_at ?? w.created); if (!map.has(key)) map.set(key, []); map.get(key)!.push(w); } diff --git a/src/lib/relativeDay.test.ts b/src/lib/relativeDay.test.ts new file mode 100644 index 00000000..db93cafb --- /dev/null +++ b/src/lib/relativeDay.test.ts @@ -0,0 +1,66 @@ +// One rung per boundary in the ladder, against a fixed `now` so the test +// never flips at midnight or a DST change. Mirrors the exact buckets that +// used to live only in History.tsx's `groupLabel()`. + +import { describe, it, expect } from "vitest"; +import { relativeDayLabel, daysSince } from "@/lib/relativeDay"; + +// A Wednesday, arbitrary but fixed. +const NOW = new Date("2026-09-16T12:00:00.000Z").getTime(); + +function isoDaysAgo(days: number): string { + return new Date(NOW - days * 86_400_000).toISOString(); +} + +describe("relativeDayLabel", () => { + it("Today at 0 days", () => { + expect(relativeDayLabel(isoDaysAgo(0), NOW)).toBe("Today"); + }); + + it("Yesterday at 1 day", () => { + expect(relativeDayLabel(isoDaysAgo(1), NOW)).toBe("Yesterday"); + }); + + it("counts days through the middle of the week", () => { + expect(relativeDayLabel(isoDaysAgo(6), NOW)).toBe("6 days ago"); + }); + + it("Last week at the 7 day boundary", () => { + expect(relativeDayLabel(isoDaysAgo(7), NOW)).toBe("Last week"); + }); + + it("still Last week at 13 days", () => { + expect(relativeDayLabel(isoDaysAgo(13), NOW)).toBe("Last week"); + }); + + it("2 weeks ago at the 14 day boundary", () => { + expect(relativeDayLabel(isoDaysAgo(14), NOW)).toBe("2 weeks ago"); + }); + + it("still 2 weeks ago at 20 days", () => { + expect(relativeDayLabel(isoDaysAgo(20), NOW)).toBe("2 weeks ago"); + }); + + it("3 weeks ago at the 21 day boundary", () => { + expect(relativeDayLabel(isoDaysAgo(21), NOW)).toBe("3 weeks ago"); + }); + + it("still 3 weeks ago at 27 days", () => { + expect(relativeDayLabel(isoDaysAgo(27), NOW)).toBe("3 weeks ago"); + }); + + it("falls to month + year at the 28 day boundary", () => { + const iso = isoDaysAgo(28); + const expected = new Intl.DateTimeFormat(undefined, { month: "long", year: "numeric" }).format(new Date(iso)); + expect(relativeDayLabel(iso, NOW)).toBe(expected); + }); +}); + +describe("daysSince", () => { + it("floors to whole 24h buckets, not calendar days", () => { + expect(daysSince(isoDaysAgo(0), NOW)).toBe(0); + expect(daysSince(isoDaysAgo(1), NOW)).toBe(1); + expect(daysSince(isoDaysAgo(9.5), NOW)).toBe(9); + expect(daysSince(isoDaysAgo(28), NOW)).toBe(28); + }); +}); diff --git a/src/lib/relativeDay.ts b/src/lib/relativeDay.ts new file mode 100644 index 00000000..14fa604a --- /dev/null +++ b/src/lib/relativeDay.ts @@ -0,0 +1,31 @@ +// One ladder shared by History and the dashboard's task age, so the two +// never disagree about what "3 weeks ago" means. Extracted from +// `groupLabel()` in History.tsx: same buckets, same wording, now callable +// from anywhere that needs to talk about a date relative to now. + +/** + * Today / Yesterday / `N days ago` / Last week / `N weeks ago` / month+year, + * for a timestamp against `now`. Identical output to History's original + * `groupLabel()`. + */ +export function relativeDayLabel(iso: string, now: number = Date.now()): string { + const diffDays = daysSince(iso, now); + if (diffDays === 0) return "Today"; + if (diffDays === 1) return "Yesterday"; + if (diffDays < 7) return `${diffDays} days ago`; + if (diffDays < 14) return "Last week"; + if (diffDays < 21) return "2 weeks ago"; + if (diffDays < 28) return "3 weeks ago"; + return new Intl.DateTimeFormat(undefined, { month: "long", year: "numeric" }).format(new Date(iso)); +} + +/** + * Whole 24h buckets between `iso` and `now`, floored. These are NOT calendar + * days (a timestamp from 11pm yesterday and one from 1am today can land in + * the same bucket, or a different one, depending on the hour `now` falls + * on), but History has always worked this way and nothing downstream expects + * calendar-day boundaries. + */ +export function daysSince(iso: string, now: number = Date.now()): number { + return Math.floor((now - new Date(iso).getTime()) / 86_400_000); +} diff --git a/src/lib/taskPhase.test.ts b/src/lib/taskPhase.test.ts new file mode 100644 index 00000000..1f4467b7 --- /dev/null +++ b/src/lib/taskPhase.test.ts @@ -0,0 +1,234 @@ +// The precedence table from docs/ideas/task-status.md, "The design", plus +// the decisions called out in this file's header comment. + +import { describe, it, expect } from "vitest"; +import { + taskPhase, phaseCounts, taskAgeLabel, PHASE_ORDER, PHASE_LABEL, PHASE_EMPTY_LABEL, +} from "@/lib/taskPhase"; +import { relativeDayLabel } from "@/lib/relativeDay"; +import type { PrStatus, Task } from "@/lib/types"; + +function makeTask(overrides: Partial = {}): Task { + return { + id: "t1", + project_id: "p1", + name: "Feature work", + branch: "feature/example", + base_branch: "main", + path: "/Users/u/code/acme/tasks/acme/feature-example", + cli: "claude", + port: 1420, + created: "2026-01-01T00:00:00.000Z", + archived: false, + ...overrides, + }; +} + +function makePr(overrides: Partial = {}): PrStatus { + return { + provider: "github", + number: 1, + url: "https://github.com/acme/widget/pull/1", + title: "Example change", + state: "open", + checks: "none", + review: "none", + base: "main", + head: "feature/example", + ...overrides, + }; +} + +describe("taskPhase", () => { + it("backlog: no PR, never spawned, no history", () => { + expect(taskPhase(makeTask(), null)).toBe("backlog"); + }); + + it("in_progress: no PR, spawn_count > 0", () => { + const task = makeTask({ spawn_count: 1 }); + expect(taskPhase(task, null)).toBe("in_progress"); + }); + + it("in_progress: no PR, has_resumable_history true, spawn_count 0", () => { + const task = makeTask({ spawn_count: 0, has_resumable_history: true }); + expect(taskPhase(task, null)).toBe("in_progress"); + }); + + it("in_review: PR open", () => { + const task = makeTask({ spawn_count: 1 }); + expect(taskPhase(task, makePr({ state: "open" }))).toBe("in_review"); + }); + + it("done: PR merged", () => { + const task = makeTask({ spawn_count: 1 }); + expect(taskPhase(task, makePr({ state: "merged" }))).toBe("done"); + }); + + it("done: archived, regardless of PR state", () => { + const task = makeTask({ archived: true, spawn_count: 1 }); + expect(taskPhase(task, makePr({ state: "open" }))).toBe("done"); + }); + + it("archived beats merged too (same outcome, both routes to done)", () => { + const task = makeTask({ archived: true }); + expect(taskPhase(task, makePr({ state: "merged" }))).toBe("done"); + }); + + it("archived wins even when there is no PR at all", () => { + const task = makeTask({ archived: true }); + expect(taskPhase(task, null)).toBe("done"); + }); + + it("draft PR is in_progress, not in_review", () => { + const task = makeTask(); + expect(taskPhase(task, makePr({ state: "draft" }))).toBe("in_progress"); + }); + + it("closed PR is in_progress even with spawn_count 0 and no history", () => { + const task = makeTask({ spawn_count: 0, has_resumable_history: false }); + expect(taskPhase(task, makePr({ state: "closed" }))).toBe("in_progress"); + }); + + it("changes_requested review on an open PR stays in_review", () => { + const task = makeTask(); + const pr = makePr({ state: "open", review: "changes_requested" }); + expect(taskPhase(task, pr)).toBe("in_review"); + }); + + it("failing checks on an open PR stay in_review", () => { + const task = makeTask(); + const pr = makePr({ state: "open", checks: "failing" }); + expect(taskPhase(task, pr)).toBe("in_review"); + }); + + it("pr === null with spawn_count 0 and no history is backlog", () => { + const task = makeTask({ spawn_count: 0, has_resumable_history: false }); + expect(taskPhase(task, null)).toBe("backlog"); + }); + + it("pr === null with has_resumable_history is in_progress", () => { + const task = makeTask({ spawn_count: 0, has_resumable_history: true }); + expect(taskPhase(task, null)).toBe("in_progress"); + }); + + it("pr === undefined behaves exactly like pr === null", () => { + // A failed PrLookup (cli-missing, no-remote, error, ...) also resolves + // to a null/undefined pr, and both must fall through to the agent + // signals rather than forcing backlog. + const task = makeTask({ spawn_count: 1 }); + expect(taskPhase(task, undefined)).toBe(taskPhase(task, null)); + expect(taskPhase(task, undefined)).toBe("in_progress"); + }); +}); + +describe("phaseCounts", () => { + it("tallies a mixed list, including a task whose prOf returns null", () => { + const tasks: Task[] = [ + makeTask({ id: "a", archived: true }), + makeTask({ id: "b", spawn_count: 1 }), + makeTask({ id: "c" }), + makeTask({ id: "d", spawn_count: 2 }), + ]; + const prById: Record = { + a: null, + b: makePr({ state: "open" }), + c: null, + d: makePr({ state: "merged" }), + }; + const counts = phaseCounts(tasks, id => prById[id] ?? null); + expect(counts).toEqual({ + backlog: 1, + in_progress: 0, + in_review: 1, + done: 2, + }); + }); + + it("returns every phase key at zero on an empty list", () => { + expect(phaseCounts([], () => null)).toEqual({ + backlog: 0, in_progress: 0, in_review: 0, done: 0, + }); + }); +}); + +describe("PHASE_ORDER / PHASE_LABEL", () => { + it("carries exactly the four phases, each with a label", () => { + expect(PHASE_ORDER).toHaveLength(4); + for (const phase of PHASE_ORDER) { + expect(PHASE_LABEL[phase]).toBeTruthy(); + } + }); +}); + +describe("PHASE_EMPTY_LABEL", () => { + it("covers every phase", () => { + for (const phase of PHASE_ORDER) { + expect(PHASE_EMPTY_LABEL[phase]).toBeTruthy(); + } + }); + + it("reads as a sentence for backlog, which is why the map is explicit", () => { + // The one entry that is NOT `"Nothing " + label.toLowerCase()`. If this + // ever gets refactored into a template, this is the case that breaks. + expect(PHASE_EMPTY_LABEL.backlog).toBe("Nothing in the backlog"); + expect(PHASE_EMPTY_LABEL.in_progress).toBe("Nothing in progress"); + expect(PHASE_EMPTY_LABEL.in_review).toBe("Nothing in review"); + expect(PHASE_EMPTY_LABEL.done).toBe("Nothing done"); + }); + + it("uses no em dash, like every other user-visible string", () => { + for (const phase of PHASE_ORDER) { + expect(PHASE_EMPTY_LABEL[phase]).not.toContain("—"); + expect(PHASE_LABEL[phase]).not.toContain("—"); + } + }); +}); + +describe("taskAgeLabel", () => { + const NOW = new Date("2026-09-16T12:00:00.000Z").getTime(); + + function isoMinutesAgo(minutes: number): string { + return new Date(NOW - minutes * 60_000).toISOString(); + } + + function isoDaysAgo(days: number): string { + return new Date(NOW - days * 86_400_000).toISOString(); + } + + it("undefined has no age", () => { + expect(taskAgeLabel(undefined, NOW)).toBeNull(); + }); + + it("null has no age", () => { + expect(taskAgeLabel(null, NOW)).toBeNull(); + }); + + it("an unparseable string has no age", () => { + expect(taskAgeLabel("not-a-timestamp", NOW)).toBeNull(); + }); + + it("5 minutes ago is suppressed as noise", () => { + expect(taskAgeLabel(isoMinutesAgo(5), NOW)).toBeNull(); + }); + + it("23 hours ago is still suppressed", () => { + expect(taskAgeLabel(isoMinutesAgo(23 * 60), NOW)).toBeNull(); + }); + + it("25 hours ago reads Yesterday", () => { + expect(taskAgeLabel(isoMinutesAgo(25 * 60), NOW)).toBe("Yesterday"); + }); + + it("3 days ago reads N days ago", () => { + expect(taskAgeLabel(isoDaysAgo(3), NOW)).toBe("3 days ago"); + }); + + it("22 days ago reads 3 weeks ago", () => { + expect(taskAgeLabel(isoDaysAgo(22), NOW)).toBe("3 weeks ago"); + }); + + it("40 days ago falls back to relativeDayLabel's month + year, not a hardcoded string", () => { + const iso = isoDaysAgo(40); + expect(taskAgeLabel(iso, NOW)).toBe(relativeDayLabel(iso, NOW)); + }); +}); diff --git a/src/lib/taskPhase.ts b/src/lib/taskPhase.ts new file mode 100644 index 00000000..f204b69d --- /dev/null +++ b/src/lib/taskPhase.ts @@ -0,0 +1,115 @@ +// A task's phase is DERIVED at render, from the task record plus the PR +// store, and never stored. That is the whole point: a value computed from +// signals already in memory cannot disagree with the PR chip or the sidebar +// badge, and it cannot go stale, because there is nothing to go stale. PR +// #292 originally shipped a hand-set status field (a pill you set by hand, +// persisted on the task record) and the maintainer rejected it before +// merge, exactly because a hand-maintained signal sitting beside live ones +// drifts: merge a PR without updating the dot and the row shows a merged +// chip next to a stale "In review" with nothing in the system reconciling +// them. See docs/ideas/task-status.md, "What was tried, and why it was +// rejected". This file is the "same question, opposite mechanism" second +// attempt: nothing here is typed by a person. +// +// The decisions baked into the precedence table below: +// +// - Archived beats merged. A shelved task is finished regardless of what its +// PR did, so `archived` is checked before the PR at all. +// - A draft PR is In progress, not In review. A draft says explicitly that +// it is not ready to be looked at. +// - A closed, unmerged PR falls back to In progress rather than Backlog. +// The branch has real work on it, closing usually means "try a different +// approach", and Backlog would claim nothing has happened here. +// - `changes_requested` stays In review. The PR chip already says so, and a +// phase that oscillates with every review round is noise, not signal. +// - A failing check does not move the phase. CI is a property of the work, +// not a stage of it, and the PR chip already turns red. +// - A shell spawn counts as progress because `task_record_spawn` fires for +// every spawn: `spawn_count > 0` really means "something has run here", +// not "something is still running". +// - Main-checkout tasks are never looked up by the PR poller at all +// (`pollableTasks` in src/store/pr.ts skips `is_main_checkout`), so a +// main-checkout task only ever leaves In progress by being archived. +// - A record written before `last_opened_at` existed shows no age rather +// than a guessed one (see `taskAgeLabel`). +// +// In practice Backlog is rare. Every GUI create path activates the new task, +// activation spawns its default tab, and that spawn is recorded, so a task +// is In progress within a second of existing. Only a CLI create that never +// opens the task stays in Backlog. + +import type { PrStatus, Task } from "./types"; +import { relativeDayLabel, daysSince } from "./relativeDay"; + +export type TaskPhase = "backlog" | "in_progress" | "in_review" | "done"; + +/** Display order for the dashboard's filter row. */ +export const PHASE_ORDER: readonly TaskPhase[] = ["in_progress", "in_review", "done", "backlog"]; + +export const PHASE_LABEL: Record = { + backlog: "Backlog", + in_progress: "In progress", + in_review: "In review", + done: "Done", +}; + +/** What the dashboard says when a filter matches nothing. An explicit map, + * not `"Nothing " + PHASE_LABEL[p].toLowerCase()`: that reads fine for three + * of the four and produces "Nothing backlog" for the fourth, and a sentence + * assembled from a label is a sentence nobody proofreads. */ +export const PHASE_EMPTY_LABEL: Record = { + backlog: "Nothing in the backlog", + in_progress: "Nothing in progress", + in_review: "Nothing in review", + done: "Nothing done", +}; + +/** + * First match wins. `pr` is the live snapshot for this task from + * `usePr.getState().byTask[id]?.lookup?.pr ?? null`. A lookup that failed + * (`cli-missing`, `no-remote`, `error`, ...) also has `pr === null`, and + * that is deliberate: "we do not know if there is a PR" falls through to + * the agent signals below rather than forcing Backlog, so a task on a + * machine with no `gh`/`glab` still phases correctly from `spawn_count`. + */ +export function taskPhase(task: Task, pr: PrStatus | null | undefined): TaskPhase { + if (task.archived || pr?.state === "merged") return "done"; + if (pr?.state === "open") return "in_review"; + if ( + pr?.state === "draft" || + pr?.state === "closed" || + (task.spawn_count ?? 0) > 0 || + task.has_resumable_history + ) { + return "in_progress"; + } + return "backlog"; +} + +/** Phase totals over a list of tasks, keyed by every {@link TaskPhase} so a + * caller never has to guard a missing key. `prOf` looks up the live PR for + * one task id, same contract as {@link taskPhase}'s second argument. */ +export function phaseCounts( + tasks: readonly Task[], + prOf: (taskId: string) => PrStatus | null | undefined, +): Record { + const counts: Record = { backlog: 0, in_progress: 0, in_review: 0, done: 0 }; + for (const task of tasks) { + counts[taskPhase(task, prOf(task.id))]++; + } + return counts; +} + +/** + * The recency half of the answer: phase alone does not say which In + * progress task you are actually on. Returns `null` when there is nothing + * to show: no timestamp, an unparseable one, or one so recent ("Today") + * that showing it would just be noise on a row stamped minutes ago. + */ +export function taskAgeLabel(lastOpenedAt: string | null | undefined, now: number = Date.now()): string | null { + if (!lastOpenedAt) return null; + const t = new Date(lastOpenedAt).getTime(); + if (Number.isNaN(t)) return null; + if (daysSince(lastOpenedAt, now) < 1) return null; + return relativeDayLabel(lastOpenedAt, now); +} diff --git a/src/store/app.ts b/src/store/app.ts index 3df785ad..68cc37bb 100644 --- a/src/store/app.ts +++ b/src/store/app.ts @@ -161,11 +161,14 @@ export interface AppState { * only way back into a task from the home screen without hunting the * sidebar for it. * - * localStorage rather than a persisted `last_opened_at` on the Task - * record, for the same reason collapsedGroups lives here: it is a - * per-machine UI convenience, and a disk write on every task click to - * store it would be the wrong trade. Pruned in `loadAll` alongside the - * group maps, so an archived or deleted task leaves no ghost. */ + * localStorage rather than the Task record, for the same reason + * collapsedGroups lives here: it is a per-machine UI convenience, an + * ORDERED list of the last eight visits at finer than a minute. The + * record's `last_opened_at` (stamped below, at most once a minute) is + * the coarse durable answer to a different question, "how long since I + * was in here", and cannot reproduce this order. Pruned in `loadAll` + * alongside the group maps, so an archived or deleted task leaves no + * ghost. */ recentTasks: string[]; /** Editable agent registry from settings.json. Loaded by `loadAll` so * `spawnArgsForCli` can consult `agent.command + args + capabilities` diff --git a/src/store/ui.test.ts b/src/store/ui.test.ts index f0795f3a..cb670bb1 100644 --- a/src/store/ui.test.ts +++ b/src/store/ui.test.ts @@ -103,3 +103,41 @@ describe("confirm modal", () => { await expect(p).resolves.toBe(true); }); }); + +// The dashboard's phase filter. Session-only by design (no localStorage), so +// the only things worth pinning are the default, the round trip, and the bail +// that keeps a re-click of the selected pill from copying the store. +describe("dashboard phase filter", () => { + beforeEach(() => { + useUI.setState({ dashboardPhase: null }); + }); + + it("defaults to All", () => { + expect(useUI.getState().dashboardPhase).toBeNull(); + }); + + it("round-trips a phase and back to All", () => { + useUI.getState().setDashboardPhase("in_review"); + expect(useUI.getState().dashboardPhase).toBe("in_review"); + useUI.getState().setDashboardPhase(null); + expect(useUI.getState().dashboardPhase).toBeNull(); + }); + + it("notifies ONCE for a phase set twice (docs/performance.md bear trap 8)", () => { + let notifications = 0; + const unsub = useUI.subscribe(() => { notifications++; }); + useUI.getState().setDashboardPhase("done"); + useUI.getState().setDashboardPhase("done"); + useUI.getState().setDashboardPhase("done"); + unsub(); + expect(notifications).toBe(1); + }); + + it("does not notify when clearing a filter that is already All", () => { + let notifications = 0; + const unsub = useUI.subscribe(() => { notifications++; }); + useUI.getState().setDashboardPhase(null); + unsub(); + expect(notifications).toBe(0); + }); +}); diff --git a/src/store/ui.ts b/src/store/ui.ts index 703240a4..6279b9b9 100644 --- a/src/store/ui.ts +++ b/src/store/ui.ts @@ -3,6 +3,7 @@ import { create } from "zustand"; import type { Prompt } from "@/store/prompts"; +import type { TaskPhase } from "@/lib/taskPhase"; export interface ConfirmCheckbox { label: string; @@ -268,6 +269,11 @@ interface UIState { * Settings overlay, so it can't refresh itself). RightPanel folds this * into its local reload token. */ fileTreeNonce: number; + /** Dashboard phase filter; `null` is "All". Session-only ON PURPOSE: the + * dashboard unmounts the moment a task is opened, so component state would + * reset on every visit anyway, and a filter is not worth a localStorage + * key (or the migration and pruning that come with one). */ + dashboardPhase: TaskPhase | null; // actions openNewProject: () => void; @@ -338,6 +344,8 @@ interface UIState { closeFindInFiles: () => void; setBusy: (msg: string | null) => void; reloadFileTree: () => void; + /** Pick the dashboard's phase filter, or `null` for All. */ + setDashboardPhase: (phase: TaskPhase | null) => void; /** Open the global confirm modal. Returns a Promise that resolves * to true (user confirmed) or false (cancelled / dismissed). Drop-in * replacement for `window.confirm()` with our own chrome + theming. */ @@ -461,6 +469,7 @@ export const useUI = create(set => ({ renameRequest: null, busyMessage: null, fileTreeNonce: 0, + dashboardPhase: null, confirm: null, terminalDrop: null, scratchClose: null, @@ -541,6 +550,10 @@ export const useUI = create(set => ({ })), setBusy: (msg) => set({ busyMessage: msg }), reloadFileTree: () => set(s => ({ fileTreeNonce: s.fileTreeNonce + 1 })), + // Bails on an unchanged value like `setWindowFocused` does: re-picking the + // pill that is already selected must not copy the store and wake every + // subscriber (docs/performance.md bear trap 8). + setDashboardPhase: (phase) => set(s => (s.dashboardPhase === phase ? s : { dashboardPhase: phase })), askConfirm: (req: any) => // Defer mounting the confirm dialog by a macrotask. When a Radix // ContextMenu / Dropdown item's onSelect calls askConfirm, the menu is From dbd6b67bb063b5a5032eaaba06b6cd0923a8cef1 Mon Sep 17 00:00:00 2001 From: Vinodkumar Naidu <7994336+nvkvin@users.noreply.github.com> Date: Mon, 14 Sep 2026 11:52:11 +0530 Subject: [PATCH 03/10] feat(task): started_at, base_sha, and a read-only git phase state Three record-level facts the derived phase needs and could not get. `started_at`: the first prompt a human submitted into any terminal of the task, stamped once by `task_mark_started` (sync, single-record, write-once). Creation spawns the agent, so a spawn is not evidence anyone has given the task work; this is. A one-time backfill (`data_migration_version` 1, its own ladder so it cannot re-trigger the workspace migration) stamps every existing record that had spawned from `last_opened_at` or `created`, so a fleet does not read Todo on upgrade. `base_sha`: the commit the branch was cut from, frozen right before `git branch --no-track` at the two create sites and at restore when the branch has to be re-cut; None for a reused branch, an imported worktree and a main checkout, where recording the base's current sha would name a commit the branch was never cut from. `task_git_phase_state`: where the branch stands against its base. Own commits, dirty (untracked counts), ahead of the remote branch (None when there is none), and merged_into_base, biased toward false because a wrong Done tells the user to archive live work. Two tiers: with own commits, every branch-side commit patch-equivalent on the base (rebase merges) or the squashed tree replayed on the merge base equivalent to one (squash merges); with none, the tip an ancestor of the base, moved off a known creation commit, and a reflog `commit` entry that itself landed on the base. That last condition is what separates a fast-forward merge from a fresh branch an agent pulled up to a newer base, or a branch whose work was reset away: identical in the DAG, measured on git 2.50. The squash probe pins author and committer so the dangling object dedupes instead of accumulating. Async and read-only: an async writer would race the sync setters (gotchas.md). `load_task_by_id` replaces the duplicated profile sweep in the three single-record commands and re-tags `profile` in one place. Refs #292 --- docs/data-model.md | 2 +- docs/gotchas.md | 9 + docs/ipc.md | 2 +- docs/tech-debt.md | 72 +++ src-tauri/src/lib.rs | 1308 ++++++++++++++++++++++++++++++++++++++++-- 5 files changed, 1346 insertions(+), 47 deletions(-) diff --git a/docs/data-model.md b/docs/data-model.md index ba31286d..33457eee 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. `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`. 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`). 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/gotchas.md b/docs/gotchas.md index 91d12b59..3d254566 100644 --- a/docs/gotchas.md +++ b/docs/gotchas.md @@ -800,3 +800,12 @@ that every other writer of that record also takes. The existing async writers 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` follows the rule the hard way and the easy way at once: it +fires on the user's FIRST prompt, 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`, and it is write-once, so there is only ever one write to +lose. `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/ipc.md b/docs/ipc.md index b1f1eb56..346593d6 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). `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"). +- **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` stamps `started_at` the first time the user submits a prompt into any of the task's terminals and returns what is on disk; same single-record read and same SYNC discipline as `task_touch`, and WRITE-ONCE on top of it, so a task that already carries a stamp gets the existing one back with no file write at all (the frontend bails too, but its copy is per-window state and a second window would otherwise re-submit). `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/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/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 983e20b5..8ae7673f 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -451,6 +451,43 @@ pub struct Task { /// not interchangeable. #[serde(default)] pub last_opened_at: Option, + /// RFC3339 UTC stamp of the FIRST time the user submitted a prompt into + /// any terminal of this task (the frontend's `markStarted`, fired from the + /// same four input paths that arm the work-state detector). Written once by + /// the app, never typed by a person, and never cleared. + /// + /// It is what separates "created, agent idling at its prompt" (Todo) from + /// "someone gave it work" (In progress). Spawning alone deliberately does + /// not count, because every GUI create spawns, so `spawn_count` cannot draw + /// that line on its own. + /// + /// `None` on records written before the field existed AND never worked + /// since. Records that predate it are backfilled once at startup (see + /// `backfill_started_at`), so an existing fleet does not all read Todo on + /// upgrade. + #[serde(default)] + pub started_at: Option, + /// The commit this task's branch was cut from, recorded at creation. + /// + /// Written ONLY where the base is actually known, which is every site that + /// cuts the task's own branch with `git branch --no-track ` + /// and rev-parses the resolved base immediately before doing so: + /// `task_create_sync`, `task_create_multi_sync`'s host repo, and + /// `restore_task_branch` when an archive-with-delete removed the branch and + /// restore has to cut a new one. That last case REWRITES the value, because + /// the new branch really does start somewhere else. + /// + /// `None` everywhere else, on purpose: reusing an existing branch, + /// `task_import_worktree` and `task_open_repo` all adopt a branch that + /// predates the task, and recording the base's CURRENT sha for one of those + /// 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 as the "was this branch ever really + /// worked on" anchor and falls back to the branch reflog's creation entry + /// when it is `None`. See that command for what an expired reflog costs. + #[serde(default)] + pub base_sha: Option, /// True when this task points at the project's main repo checkout /// (no git worktree created). Used by the "open repo directly" feature: /// archive skips `git worktree remove`, and the UI shows a distinct icon. @@ -1679,6 +1716,35 @@ fn load_tasks_in(id: &ProfileId) -> Vec { out } +/// ONE task by id, read straight out of whichever profile holds its file. +/// +/// The sweep is a `stat` per profile, which is nothing next to what the +/// alternative costs. [`load_tasks_all`] re-parses every record of every +/// profile, which is fine a handful of times per session and wrong on any path +/// the user walks repeatedly: `task_touch` fires on every activation, +/// `task_mark_started` on the first prompt of a task, and +/// `task_git_phase_state` runs once per visible task on every dashboard poll, +/// where the whole-fleet load would be N record parses per task per pass to +/// answer a question about one of them. +/// +/// The `profile` re-tag is the load-bearing line, exactly as in +/// [`load_tasks_in`]. `Task.profile` is `serde(skip)`, so a record parsed +/// straight from a file carries the DEFAULT profile rather than the one it came +/// from, and a `save_task` after that would write a second copy of a non-root +/// profile's task into the root tree. +fn load_task_by_id(id: &str) -> Option { + // Sweep the profiles for the one file, the way `delete_task_file` does: + // the caller has only an id. + for pid in profiles_registry().ids() { + let Ok(dir) = tasks_dir_in(&pid) else { continue }; + let Ok(s) = fs::read_to_string(dir.join(format!("{id}.json"))) else { continue }; + let Ok(mut w) = serde_json::from_str::(&s) else { continue }; + w.profile = pid.clone(); + return Some(w); + } + None +} + // ── Port blocks (GH #196) ── // Each live task owns a consecutive block starting at its base port: // 1 ($TERMIC_PORT) + composition members + extra named ports @@ -2146,6 +2212,97 @@ fn migrate_cli_enabled_default() { } } +/// Stamp the `data_migration_version` ladder at `v`, never downwards. Written +/// LAST by each step, so an interrupted or partly-failed run leaves the guard +/// down and simply retries next launch. +fn stamp_data_migration_version(v: u32) { + let mut s = load_settings_inner(); + if s.data_migration_version >= v { + return; + } + s.data_migration_version = v; + let _ = save_settings_inner(&s); +} + +/// The backfill rule for one record, split out so "running it twice changes +/// nothing" is a real assertion about the RULE rather than about the version +/// guard trivially bailing. Returns whether `w` changed and so needs saving. +/// +/// Every task on disk before `started_at` shipped predates the field, so +/// without this the user's whole fleet reads Todo on upgrade. A task an agent +/// has actually run in is not Todo, and the two persisted facts that say so +/// are `spawn_count` and `has_resumable_history`. +/// +/// The stamp is `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 (a hand-written or truncated file) is left +/// alone rather than stamped with an empty string, which nothing downstream +/// could parse. +fn backfill_started_at(w: &mut Task) -> bool { + if w.started_at.is_some() { + return false; + } + if w.spawn_count == 0 && !w.has_resumable_history { + return false; + } + let stamp = w + .last_opened_at + .clone() + .filter(|s| !s.trim().is_empty()) + .or_else(|| Some(w.created.clone()).filter(|s| !s.trim().is_empty())); + match stamp { + Some(s) => { + w.started_at = Some(s); + true + } + None => false, + } +} + +/// One-time backfill of `Task.started_at` across EVERY profile's tasks dir. +/// +/// Idempotent twice over: the rule itself is a no-op on a record that already +/// carries a stamp, and the ladder guard skips the sweep entirely once it has +/// committed. Best-effort like its siblings, and the version is stamped LAST +/// and only when every write landed, so a profile whose disk was full retries +/// on the next launch instead of being silently skipped forever. +/// +/// Ordered AFTER `migrate_workspaces_to_tasks`, which is what puts the records +/// in `tasks/` where `load_tasks_in` looks for them. +fn migrate_started_at_backfill() { + if load_settings_inner().data_migration_version >= STARTED_AT_BACKFILL_VERSION { + return; + } + let mut all_saved = true; + let mut changed = 0usize; + for pid in profiles_registry().ids() { + // `load_tasks_in` re-tags each record with the profile it came from, + // so `save_task` writes it back where it started rather than into the + // root tree. + for mut w in load_tasks_in(&pid) { + if !backfill_started_at(&mut w) { + continue; + } + match save_task(&w) { + Ok(()) => changed += 1, + Err(e) => { + log_migration(&format!("started_at backfill: task {} failed: {e}", w.id)); + all_saved = false; + } + } + } + } + if all_saved { + if changed > 0 { + log_migration(&format!("started_at backfill: stamped {changed} task(s)")); + } + stamp_data_migration_version(STARTED_AT_BACKFILL_VERSION); + } else { + log_migration("started_at backfill: at least one write failed; retrying next launch"); + } +} + /// Pure half of the migration, so the rule can be tested without a settings /// file (`TERMIC_DATA_DIR` is process-global and would race parallel tests). /// Returns whether `s` changed and therefore needs writing. @@ -2344,6 +2501,15 @@ fn migrate_workspaces_to_tasks() { /// Raw stdout, for callers that read blobs (`git show HEAD:some.png`) where /// a lossy UTF-8 decode would destroy the bytes. fn git_bytes(args: &[&str], cwd: &Path) -> Result> { + git_bytes_env(args, cwd, &[]) +} + +/// `git_bytes` plus extra environment for the child. Only one caller needs it +/// (the squash probe in `git_phase_state`, which pins author/committer identity +/// and dates so the object it writes is content-identical on every run), and it +/// lives here rather than at that call site so the login-shell env below has +/// exactly one implementation. +fn git_bytes_env(args: &[&str], cwd: &Path, extra: &[(&str, &str)]) -> Result> { let mut cmd = Command::new("git"); cmd.args(args).current_dir(cwd); // Run with the user's login-shell environment, same as the PTY (see @@ -2357,6 +2523,11 @@ fn git_bytes(args: &[&str], cwd: &Path) -> Result> { for (k, v) in inject { cmd.env(k, v); } + // After the login-shell env, so a pin always wins over whatever the user's + // rc exported. + for (k, v) in extra { + cmd.env(k, v); + } let out = cmd.output().with_context(|| format!("git {:?}", args))?; if !out.status.success() { return Err(anyhow!("git {:?} failed: {}", args, String::from_utf8_lossy(&out.stderr))); @@ -5745,6 +5916,8 @@ fn task_open_repo( split_layout: None, archived_at: None, last_opened_at: None, + started_at: None, + base_sha: None, pr_url: None, pr_number: None, pr_provider: None, @@ -6006,6 +6179,8 @@ fn task_import_worktree( split_layout: None, archived_at: None, last_opened_at: None, + started_at: None, + base_sha: None, pr_url: None, pr_number: None, pr_provider: None, @@ -6159,6 +6334,11 @@ fn task_create_sync(app: AppHandle, args: CreateTaskArgs) -> Result = None; let wt_arg = wt_path.to_str().unwrap(); let add_args: Vec<&str> = if has_git_crypt { // Skip checkout; we'll run it manually after symlinking the @@ -6204,6 +6384,12 @@ fn task_create_sync(app: AppHandle, args: CreateTaskArgs) -> Result Ok(()), Err(e) => Err(e), @@ -6406,6 +6592,8 @@ fn task_create_sync(app: AppHandle, args: CreateTaskArgs) -> Result Result = None; if host.non_git { // Non-git host (issue #4): there's no worktree to add. Make the // wrapper dir ourselves, then symlink the host's shared knowledge @@ -6608,6 +6801,11 @@ fn task_create_multi_sync(app: AppHandle, args: CreateMultiArgs) -> Result { emit_create_progress(&app, &task_id, format!("Adding host worktree at {}…", wrapper.display())); @@ -6899,6 +7097,8 @@ fn task_create_multi_sync(app: AppHandle, args: CreateMultiArgs) -> Result) -> bool { /// Fires on EVERY activation (every sidebar click, every Cmd-number switch), /// and two things follow from that. /// -/// It reads exactly ONE record. The siblings above call `load_tasks_all()`, -/// re-parsing every task file of every profile, which is fine a handful of -/// times per session and wrong on a path the user walks dozens of times an -/// hour. +/// It reads exactly ONE record, via [`load_task_by_id`]. The siblings above +/// call `load_tasks_all()`, re-parsing every task file of every profile, which +/// is fine a handful of times per session and wrong on a path the user walks +/// dozens of times an hour. /// /// It is SYNC on purpose, like those siblings. Every per-task setter in this /// file is an unlocked read-modify-write of the whole record, and they get @@ -8800,25 +9000,44 @@ fn task_touch(id: String) -> Result { } fn task_touch_sync(id: String) -> Result { - // Sweep the profiles for the one file, the way `delete_task_file` does: - // the caller has only an id, and a `stat` per profile is nothing next to - // parsing every record in all of them. - for pid in profiles_registry().ids() { - let Ok(dir) = tasks_dir_in(&pid) else { continue }; - let Ok(s) = fs::read_to_string(dir.join(format!("{id}.json"))) else { continue }; - let Ok(mut w) = serde_json::from_str::(&s) else { continue }; - // `profile` is `serde(skip)`, so a record parsed straight from a file - // carries the DEFAULT profile, not the one it came from. Without this - // line `save_task` would write a second copy of a non-root profile's - // task into the root tree. - w.profile = pid.clone(); - if !touch_task_record(&mut w, chrono::Utc::now()) { - return Ok(w.last_opened_at.clone().unwrap_or_default()); - } - save_task(&w).map_err(|e| e.to_string())?; + let mut w = load_task_by_id(&id).ok_or("no such task")?; + if !touch_task_record(&mut w, chrono::Utc::now()) { return Ok(w.last_opened_at.clone().unwrap_or_default()); } - Err("no such task".into()) + save_task(&w).map_err(|e| e.to_string())?; + Ok(w.last_opened_at.clone().unwrap_or_default()) +} + +/// Stamp `started_at` the first time work is submitted into this task, and +/// return the stamp now on disk. +/// +/// WRITE-ONCE. A task that already carries a stamp gets its existing one back +/// untouched, with no file write at all: the field records when work STARTED, +/// so a later prompt must not move it. The frontend bails on its own copy too, +/// and the bail lives here as well because that copy is per-window state and a +/// second window (or a relaunch) would otherwise re-submit. +/// +/// Same shape as [`task_touch_sync`] and for the same reasons: it reads ONE +/// record via [`load_task_by_id`] rather than `load_tasks_all()`, and +/// it is SYNC, so it serializes on the main thread against every other +/// unlocked read-modify-write of the same file. It fires on the user's first +/// prompt, which is exactly when the pane is spawning and `task_record_spawn` +/// and `task_set_tabs` are firing for the same task. See docs/gotchas.md, +/// "Task record setters serialize on the main thread". +#[tauri::command] +fn task_mark_started(id: String) -> Result { + task_mark_started_sync(id) +} + +fn task_mark_started_sync(id: String) -> Result { + let mut w = load_task_by_id(&id).ok_or("no such task")?; + if let Some(existing) = w.started_at { + return Ok(existing); + } + let stamp = chrono::Utc::now().to_rfc3339(); + w.started_at = Some(stamp.clone()); + save_task(&w).map_err(|e| e.to_string())?; + Ok(stamp) } /// Set the persisted `has_resumable_history` flag for a task. @@ -9123,6 +9342,42 @@ async fn task_restore(app: AppHandle, id: String) -> Result { .map_err(|e| e.to_string())? } +/// Make sure a restored task's own branch exists again, and report the commit +/// it was cut from when it had to be re-created. +/// +/// `Ok(None)` means LEAVE `Task.base_sha` alone: the branch survived archive +/// (only `delete_branch` archives remove it), nothing was cut, and the stored +/// creation point is still true of it. +/// +/// `Ok(Some(sha))` means the branch was gone and has just been re-cut from +/// `sha`, frozen before the cut the way the two create sites freeze theirs. The +/// record has to move with it: the old `base_sha` names a commit this branch no +/// longer has any relationship to, and `task_git_phase_state` would measure +/// against a base that was never its own. +/// +/// The base resolves through the tolerant `resolve_base_ref`, matching what +/// restore has always done here: this is a STORED base, and a local-only repo +/// pinned to `origin/main` must still restore. +fn restore_task_branch( + repo: &Path, + branch: &str, + base_branch: &str, +) -> std::result::Result, String> { + if git(&["rev-parse", "--verify", branch], repo).is_ok() { + return Ok(None); + } + // Branch was deleted at archive time: recreate from base (resolved to a ref + // that exists; local-only repos have no origin/main). + let base_ref = resolve_base_ref(repo, base_branch); + let sha = git(&["rev-parse", &format!("{base_ref}^{{commit}}")], repo) + .ok() + .map(|s| s.trim().to_string()) + .filter(|s| !s.is_empty()); + git(&["branch", "--no-track", branch, &base_ref], repo) + .map_err(|e| format!("recreate branch '{branch}' from '{base_ref}': {e}"))?; + Ok(sha) +} + fn task_restore_sync(app: AppHandle, id: String) -> Result { let mut list = load_tasks_all(); let idx = list.iter().position(|w| w.id == id).ok_or("task not found")?; @@ -9169,6 +9424,17 @@ fn task_restore_sync(app: AppHandle, id: String) -> Result { let wt_path = PathBuf::from(&list[idx].path); let repo = PathBuf::from(&proj.root_path); + // Set only when restore has to RE-CUT the task's own branch, because + // archive deleted it. That branch really is cut from the base's tip right + // now, so the recorded creation point has to move with it: the stored + // `base_sha` names a commit the branch no longer has any relationship to, + // and leaving it would make `task_git_phase_state` measure against a base + // that was never this branch's. Applied near the end rather than here, + // because the record is re-read off disk just before it is saved (see + // there). Restoring a task whose branch still exists leaves `base_sha` + // exactly as it was: nothing was cut, so nothing changed. + let mut recut_base_sha: Option = None; + if list[idx].composition.is_empty() { // ── Single-repo task ────────────────────────────────────────── if !proj.non_git { @@ -9224,18 +9490,8 @@ fn task_restore_sync(app: AppHandle, id: String) -> Result { add_args.push(wt_arg); add_args.push(&branch); - let branch_exists = git(&["rev-parse", "--verify", &branch], &repo).is_ok(); - if branch_exists { - git(&add_args, &repo).map_err(|e| e.to_string())?; - } else { - // Branch was deleted at archive time — recreate from base - // (resolved to a ref that exists; local-only repos have no - // origin/main). - let base_ref = resolve_base_ref(&repo, &base_branch); - git(&["branch", "--no-track", &branch, &base_ref], &repo) - .map_err(|e| format!("recreate branch '{branch}' from '{base_ref}': {e}"))?; - git(&add_args, &repo).map_err(|e| e.to_string())?; - } + recut_base_sha = restore_task_branch(&repo, &branch, &base_branch)?; + git(&add_args, &repo).map_err(|e| e.to_string())?; // git-crypt: bridge the key dir into the new worktree's gitdir. if has_git_crypt { @@ -9288,17 +9544,12 @@ fn task_restore_sync(app: AppHandle, id: String) -> Result { } else { let host_branch = list[idx].branch.clone(); let host_base = list[idx].base_branch.clone(); - let branch_exists = git(&["rev-parse", "--verify", &host_branch], &repo).is_ok(); - if branch_exists { - git(&["worktree", "add", wt_path.to_str().unwrap(), &host_branch], &repo) - .map_err(|e| format!("host worktree add: {e}"))?; - } else { - let host_base_ref = resolve_base_ref(&repo, &host_base); - git(&["branch", "--no-track", &host_branch, &host_base_ref], &repo) - .map_err(|e| format!("recreate host branch: {e}"))?; - git(&["worktree", "add", wt_path.to_str().unwrap(), &host_branch], &repo) - .map_err(|e| format!("host worktree add: {e}"))?; - } + // The host branch IS the task's branch, so the same re-cut rule + // applies. Members are composition and keep no `base_sha` of their + // own, so their own recreation below leaves the record alone. + recut_base_sha = restore_task_branch(&repo, &host_branch, &host_base)?; + git(&["worktree", "add", wt_path.to_str().unwrap(), &host_branch], &repo) + .map_err(|e| format!("host worktree add: {e}"))?; } // Copy the host project's files_to_copy globs back in, same as the @@ -9367,6 +9618,12 @@ fn task_restore_sync(app: AppHandle, id: String) -> Result { } rehome_ports_if_stolen(&mut list[idx], &snapshot, current_port_range()); list[idx].archived = false; + // AFTER the fresh-record adopt above, which would otherwise overwrite it + // with the stale value straight off disk. `None` means no branch was + // re-cut, and then the existing `base_sha` is still the right answer. + if let Some(sha) = recut_base_sha { + list[idx].base_sha = Some(sha); + } save_task(&list[idx]).map_err(|e| e.to_string())?; drop(port_guard); let task = list[idx].clone(); @@ -10098,6 +10355,350 @@ async fn task_git_status(id: String) -> Result { .await .map_err(|e| e.to_string())? } +/// Where a task's branch stands against the base it was cut from. Every field +/// is derived from git on demand; none of it is persisted, so it cannot go +/// stale or disagree with the repo. +#[derive(Clone, Debug, Serialize, Default, PartialEq, Eq)] +pub struct TaskGitState { + /// Commits on the branch side that the base cannot reach + /// (`rev-list --count B..T`). Deliberately 0 after a merge of any kind: + /// it feeds the "is there work to review" rule, never the Done rule. + pub own_commits: u32, + /// Anything staged, unstaged or untracked in the HOST worktree. + /// Composition members are OUT OF SCOPE: a multi-repo task's members are + /// separate repos under the wrapper and answering for them would mean one + /// `git status` per member per poll. + /// + /// Untracked counts as dirty on purpose. A new file nobody has added is + /// still local work that exists on this machine and nowhere else, which is + /// the question this field is asked. The cost is that a multi-repo task + /// with a GIT host reads dirty from creation: `ensure_multirepo_gitignore` + /// unconditionally writes termic's managed block for the member dirs into + /// the wrapper, which is that host's worktree, so the wrapper always has an + /// untracked or modified `.gitignore` in it. + pub dirty: bool, + /// Commits not on the remote branch. `None` when there is no remote branch + /// at all, which is NOT the same fact as zero: a branch that has never been + /// pushed has nothing to be ahead of. (See CLAUDE.md on collapsing `null` + /// and a real answer.) + pub ahead: Option, + /// Whether this branch's work is already in the base. BIASED TOWARD FALSE: + /// a missed Done costs the user nothing, a wrong Done tells them to archive + /// live work. See `git_phase_state` for the two tiers. + pub merged_into_base: bool, + /// Whether the creation commit S was recoverable (from `Task.base_sha`, or + /// from the branch reflog). False means the second merge tier can never + /// fire for this task. + pub base_known: bool, +} + +/// Identity and timestamps pinned on the throwaway `commit-tree` object the +/// squash check writes. Git hashes the commit's whole header, so an unpinned +/// author date would make a NEW dangling object on every single poll and the +/// user's repo would accumulate them until `gc`. Pinned, the object is +/// content-identical every time and simply dedupes into the one that is +/// already there. +const PHASE_PROBE_ENV: [(&str, &str); 6] = [ + ("GIT_AUTHOR_NAME", "termic"), + ("GIT_AUTHOR_EMAIL", "termic@localhost"), + ("GIT_AUTHOR_DATE", "@0 +0000"), + ("GIT_COMMITTER_NAME", "termic"), + ("GIT_COMMITTER_EMAIL", "termic@localhost"), + ("GIT_COMMITTER_DATE", "@0 +0000"), +]; + +/// `rev-list --count `, 0 on any failure. +fn rev_count(repo: &Path, range: &str) -> u32 { + git(&["rev-list", "--count", range], repo) + .ok() + .and_then(|s| s.trim().parse().ok()) + .unwrap_or(0) +} + +/// True iff every branch-side commit in `range` is patch-equivalent to +/// something already on the base side, and there is at least one to judge. +/// +/// `--no-merges` matters: agents routinely merge the base back INTO their +/// branch, and a merge commit has no patch-id, so it can never be marked +/// equivalent and would sink an otherwise merged branch. Dropping merges from +/// the question is safe because their content arrives with the commits they +/// merge. +/// +/// An empty range answers FALSE, not true: "nothing to compare" is not +/// evidence of a merge, and treating it as one is exactly the wrong-Done this +/// whole function is biased against. +fn all_patch_equivalent(repo: &Path, range: &str) -> bool { + let Ok(out) = git( + &["log", "--cherry-mark", "--right-only", "--no-merges", "--format=%m", range], + repo, + ) else { + return false; + }; + let mut judged = 0usize; + for line in out.lines() { + let l = line.trim(); + if l.is_empty() { + continue; + } + judged += 1; + // "=" is patch-equivalent; ">" is a branch-side commit with no twin. + if l != "=" { + return false; + } + } + judged > 0 +} + +/// The commit the branch was created at: `Task.base_sha` when the task +/// recorded one, else the branch reflog's creation entry. +/// +/// The LAST line of `git reflog show ` is the oldest entry, which for a +/// branch cut with `git branch --no-track ` is +/// `branch: Created from ` and whose commit IS the base. Once that reflog +/// expires (git's default `gc.reflogExpire` is 90 days) there is nothing left +/// to recover it from, and S is simply unknown from then on. +/// +/// `refs/heads/` fully qualified, so a tag or a file of the same name +/// cannot answer instead. +fn branch_creation_sha(repo: &Path, branch: &str, base_sha: Option<&str>) -> Option { + if let Some(s) = base_sha { + let s = s.trim(); + if !s.is_empty() { + return Some(s.to_string()); + } + } + let out = git(&["reflog", "show", "--format=%H", &format!("refs/heads/{branch}")], repo).ok()?; + out.lines() + .rev() + .map(|l| l.trim()) + .find(|l| !l.is_empty()) + .map(|s| s.to_string()) +} + +/// True iff somebody committed ON this branch AND that commit is now reachable +/// from the base. Both halves are load-bearing, and this is the discriminator +/// the DAG cannot supply. +/// +/// The reflog half: a fresh branch an agent fast-forwarded or rebased onto a +/// newer base has a tip that is an ancestor of the base and differs from where +/// it started, which is structurally identical to a branch that was ff-merged +/// INTO the base. The difference is only visible in how the ref got there. +/// `pull:`, `merge : Fast-forward` and `rebase (finish):` are not commits; +/// `commit:`, `commit (amend):`, `commit (merge):` and `commit (initial):` are. +/// +/// The reachability half is the one the brief for this feature did not ask for, +/// and it is required. MEASURED (git 2.50.1): a branch that committed C1 and +/// then ran `git reset --hard main` onto a base that had moved satisfies every +/// other condition. Its tip is an ancestor of the base, it moved off S, and its +/// reflog holds `commit: C1`. Reporting that merged would tell the user to +/// archive a task whose work was thrown away and never reached the base, which +/// is precisely the expensive failure this whole function is biased against. +/// Asking whether the committed sha actually landed rules it out: `C1` is not +/// an ancestor of the base, while an ff-merged branch's commit IS the base tip +/// and a merge-committed branch's is an ancestor of the merge. +/// +/// Newest-first with a small cap, because the newest `commit` entry decides +/// every realistic shape and an unbounded scan would be one subprocess per +/// commit on a long-lived branch. Scanning a few past it is still right: a +/// branch that committed, landed, committed again and then reset back to the +/// landed commit has its answer one entry down. +fn branch_commit_landed_on(repo: &Path, branch: &str, base: &str) -> bool { + /// How many `commit` reflog entries to probe, newest first. + const MAX_PROBES: usize = 10; + let Ok(out) = git( + &["reflog", "show", "--format=%H %gs", &format!("refs/heads/{branch}")], + repo, + ) else { + return false; + }; + let mut probed = 0usize; + for line in out.lines() { + let Some((sha, msg)) = line.trim().split_once(' ') else { continue }; + if !msg.trim_start().starts_with("commit") { + continue; + } + let sha = sha.trim(); + if sha.is_empty() { + continue; + } + if git(&["merge-base", "--is-ancestor", sha, base], repo).is_ok() { + return true; + } + probed += 1; + if probed >= MAX_PROBES { + break; + } + } + false +} + +/// Derive [`TaskGitState`] for one branch in one worktree. Pure in the sense +/// that matters: it takes the four inputs it needs and touches no task record, +/// so every DAG case below is testable on a temp repo. +/// +/// `B` is the base tip, `T` the BRANCH tip (`refs/heads/`, not HEAD, +/// which can be detached), `S` the commit the branch was cut from. +/// +/// B resolves through `try_resolve_base_ref`, NOT the tolerant +/// `resolve_base_ref` its callers usually take. The tolerant one falls back to +/// `HEAD`, and in a task worktree HEAD is the branch itself: `own_commits` +/// would be 0 and `merge-base --is-ancestor T B` trivially true, so a base +/// branch deleted after a merge would make every live task report merged. When +/// the base does not resolve, the answer is "not merged" and the caller learns +/// nothing it can act on, which is the right way round. +/// +/// `merged_into_base` has two tiers, and both are biased toward false: +/// +/// - Tier 1 (there IS own work): every branch-side commit is patch-equivalent +/// on the base. Covers rebase-and-merge. The squash variant replays the +/// branch's whole tree as one commit on the merge base and asks the same +/// question of that, which is what a squash-merge produced. +/// - Tier 2 (`own_commits == 0`, so a fast-forward or a merge commit swallowed +/// it): the tip is an ancestor of the base AND S is known AND the tip moved +/// off S AND the branch reflog proves work was committed here and that the +/// commit LANDED (see `branch_commit_landed_on`, which is what keeps a +/// `reset --hard` onto a moved base from reading as merged). Missing S or +/// missing reflog evidence means not merged, full stop. +fn git_phase_state( + repo: &Path, + branch: &str, + base_branch: &str, + base_sha: Option<&str>, +) -> std::result::Result { + // T: the branch tip. `refs/heads/` qualified and `^{commit}` peeled, so a + // detached HEAD, a same-named tag and a non-commit ref are all ruled out. + let t = git( + &["rev-parse", "--verify", "--quiet", &format!("refs/heads/{branch}^{{commit}}")], + repo, + ) + .map(|s| s.trim().to_string()) + .map_err(|_| format!("branch '{branch}' does not resolve in {}", repo.display()))?; + if t.is_empty() { + return Err(format!("branch '{branch}' does not resolve in {}", repo.display())); + } + + let dirty = git(&["status", "--porcelain", "--untracked-files=normal"], repo) + .map(|s| !s.trim().is_empty()) + .unwrap_or(false); + + let s = branch_creation_sha(repo, branch, base_sha); + let base_known = s.is_some(); + + let base = try_resolve_base_ref(repo, base_branch); + let own_commits = base + .as_deref() + .map(|b| rev_count(repo, &format!("{b}..{t}"))) + .unwrap_or(0); + + // ahead: the branch's own upstream first (which covers a remote that is not + // called origin), then the conventional origin/. Neither present + // means there is no remote branch to be ahead of, which is `None`. + let remote = git( + &["rev-parse", "--abbrev-ref", "--symbolic-full-name", &format!("{branch}@{{upstream}}")], + repo, + ) + .ok() + .map(|s| s.trim().to_string()) + .filter(|s| !s.is_empty()) + .or_else(|| { + let candidate = format!("refs/remotes/origin/{branch}"); + git(&["rev-parse", "--verify", "--quiet", &format!("{candidate}^{{commit}}")], repo) + .ok() + .map(|_| candidate) + }); + let ahead = remote.map(|r| rev_count(repo, &format!("{r}..{t}"))); + + let merged_into_base = match base.as_deref() { + None => false, + Some(b) if own_commits >= 1 => { + all_patch_equivalent(repo, &format!("{b}...{t}")) + || squash_equivalent(repo, b, &t) + } + Some(b) => { + git(&["merge-base", "--is-ancestor", &t, b], repo).is_ok() + && s.as_deref().map(|s| s != t).unwrap_or(false) + && branch_commit_landed_on(repo, branch, b) + } + }; + + Ok(TaskGitState { own_commits, dirty, ahead, merged_into_base, base_known }) +} + +/// The squash half of tier 1: rebuild what a squash-merge of this branch would +/// have produced (its whole tree as one commit on the merge base) and ask +/// whether THAT is patch-equivalent to something already on the base. +/// +/// Guarded on the squash being non-empty (`T^{tree}` differs from the merge +/// base's tree). Without that guard a branch whose tree matches its merge base +/// squashes to an empty patch, which is trivially "equivalent" to anything and +/// would report every such branch merged. +fn squash_equivalent(repo: &Path, base: &str, t: &str) -> bool { + let Ok(mb) = git(&["merge-base", base, t], repo) else { return false }; + let mb = mb.trim().to_string(); + if mb.is_empty() { + return false; + } + let tree_of = |rev: &str| { + git(&["rev-parse", &format!("{rev}^{{tree}}")], repo).ok().map(|s| s.trim().to_string()) + }; + let (Some(t_tree), Some(mb_tree)) = (tree_of(t), tree_of(&mb)) else { return false }; + if t_tree.is_empty() || t_tree == mb_tree { + return false; + } + // Dangling object, pinned so it is byte-identical on every poll. See + // PHASE_PROBE_ENV. + let Ok(tmp) = git_bytes_env( + &["commit-tree", &t_tree, "-p", &mb, "-m", "termic squash probe"], + repo, + &PHASE_PROBE_ENV, + ) else { + return false; + }; + let tmp = String::from_utf8_lossy(&tmp).trim().to_string(); + if tmp.is_empty() { + return false; + } + all_patch_equivalent(repo, &format!("{base}...{tmp}")) +} + +/// The record-level half of [`task_git_phase_state`]: the two refusals, then +/// the git work. Split out so both are testable without a Tauri runtime. +fn task_phase_state_for(w: &Task) -> std::result::Result { + if w.archived { + return Err("task is archived".to_string()); + } + if w.is_main_checkout { + return Err("task runs in the main checkout and has no branch of its own".to_string()); + } + git_phase_state(Path::new(&w.path), &w.branch, &w.base_branch, w.base_sha.as_deref()) +} + +/// Where a task's branch stands against its base: commits of its own, local +/// dirt, unpushed commits, and whether the work already landed. +/// +/// ASYNC + `spawn_blocking` because it is IO-heavy (several git invocations, +/// one of which writes an object), and READ-ONLY on the task record for exactly +/// that reason: an async command that wrote the record would race the sync +/// setters firing for the same task, which is how `task_touch` lost a stamp. +/// It must never call `save_task`. See docs/gotchas.md, "Task record setters +/// serialize on the main thread". +/// +/// Errors for an archived task (its worktree is gone) and for a main-checkout +/// task (it runs on the project's live checkout and has no branch of its own, +/// so every question here is about somebody else's work). +#[tauri::command] +async fn task_git_phase_state(id: String) -> std::result::Result { + tauri::async_runtime::spawn_blocking(move || { + // One record, not the whole fleet: this runs once per visible task on + // every dashboard poll, and `load_tasks_all()` here would be N record + // parses per task per pass. See `load_task_by_id`. + let w = load_task_by_id(&id).ok_or("no such task")?; + task_phase_state_for(&w) + }) + .await + .map_err(|e| e.to_string())? +} + /// Result of a branch switch: which branch we're on, whether local work was /// stashed to get there, and whether re-applying that stash hit conflicts /// (conflict markers are left in the tree and the stash is retained). @@ -18682,6 +19283,21 @@ pub struct Settings { /// the workspaces->tasks migration has committed. #[serde(default)] pub schema_version: u32, + /// Ladder for one-time backfills of TASK RECORD FIELDS, counted + /// independently of `schema_version` above. 0 (default, absent in files + /// written before the first step) means no backfill has run; each step + /// gates on `< ITS_OWN_CONST` and stamps its own number last. + /// + /// A second counter rather than a bump of `schema_version`: that one gates + /// the workspaces->tasks migration on `>= TASKS_SCHEMA_VERSION`, so raising + /// it would re-run that whole migration (backup, stage, rename) on every + /// v1 profile on the next launch. Same reasoning as `cli_default_migrated`, + /// with a counter instead of a bool because backfills accumulate and a bool + /// per step does not tell you which ones a profile has seen. + /// + /// Steps so far: 1 = `STARTED_AT_BACKFILL_VERSION`. + #[serde(default)] + pub data_migration_version: u32, /// When on (the default), a best-effort `git fetch` of the base ref runs /// before a new task's branch is cut, so it starts from the latest remote /// commit instead of a stale local `origin/*` (GH #79). `None` (absent in @@ -18839,6 +19455,13 @@ fn tray_enabled() -> bool { /// on `settings.schema_version < NEW_VALUE`. const TASKS_SCHEMA_VERSION: u32 = 1; +/// Step 1 of the `data_migration_version` ladder: backfill `Task.started_at` +/// for records written before the field existed. See +/// `migrate_started_at_backfill`. Do NOT fold this into +/// `TASKS_SCHEMA_VERSION`: that constant gates the workspaces->tasks +/// migration, which would re-run on every v1 profile if it moved. +const STARTED_AT_BACKFILL_VERSION: u32 = 1; + #[derive(Clone, Debug, Serialize, Deserialize)] pub struct Agent { pub id: String, // stable key referenced by Task.cli @@ -22142,6 +22765,12 @@ pub fn run() { // layout. Best-effort + gated by settings.schema_version, so it's a // cheap no-op on every launch after the first. migrate_workspaces_to_tasks(); + // One-time backfill of Task.started_at for records written before + // the field existed. AFTER the rename migration, which is what puts + // those 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. + migrate_started_at_backfill(); // One-time flip of cli_enabled for profiles that predate the CLI // graduating (0.26.0). Ordered after the task migration and before // the window so the control socket's first request already reads @@ -22303,7 +22932,7 @@ pub fn run() { repo_config_load, repo_config_load_at, repo_config_save, repo_config_scaffold, repo_config_add_allowed_host, repo_config_add_allowed_path, task_reorder, - task_restore, task_delete, task_run_script, task_run_script_stream, task_ensure_extra_ports, task_stop_script, task_record_spawn, task_set_has_history, task_touch, task_set_agent_session_id, + task_restore, task_delete, task_run_script, task_run_script_stream, task_ensure_extra_ports, task_stop_script, task_record_spawn, task_set_has_history, task_touch, task_mark_started, task_git_phase_state, task_set_agent_session_id, task_set_tabs, task_set_tab_session_id, task_set_split_layout, task_set_right_tabs, task_set_right_tab_session_id, @@ -29800,6 +30429,39 @@ filename f.rs }); } + // The single-record read the three hot per-task commands share + // (`task_touch`, `task_mark_started`, `task_git_phase_state`), instead of + // `load_tasks_all()` re-parsing every record of every profile to answer a + // question about one of them. + // + // The re-tag is the whole reason it is a helper rather than a `read_to_string` + // at each site: `Task.profile` is `serde(skip)`, so a record parsed straight + // from a file reads as the DEFAULT profile, and the next `save_task` would + // drop a second copy of a non-root profile's task into the root tree. This + // asserts it on the READ side; the write-back is covered by the touch and + // mark-started profile tests. + #[test] + fn loading_one_task_by_id_tags_it_with_the_profile_it_came_from() { + with_scratch_data_dir(|data| { + crate::profiles::save_registry(data, &two_profile_registry()).unwrap(); + crate::save_task(&a_task("t3", ProfileId::Slug("home".into()))).unwrap(); + + let w = crate::load_task_by_id("t3").expect("the task is there"); + assert_eq!(w.id, "t3"); + assert_eq!( + w.profile, + ProfileId::Slug("home".into()), + "an untagged read would be Root here, and the next save would \ + write a second copy of this task into the root tree", + ); + // It finds a ROOT-profile record too, not just a named one. + crate::save_task(&a_task("t4", ProfileId::Root)).unwrap(); + assert_eq!(crate::load_task_by_id("t4").unwrap().profile, ProfileId::Root); + + assert!(crate::load_task_by_id("nope").is_none()); + }); + } + #[test] fn touching_a_task_that_does_not_exist_is_an_error() { with_scratch_data_dir(|_data| { @@ -29807,6 +30469,562 @@ filename f.rs }); } + // ── started_at / task_mark_started ────────────────────────────── + // + // `started_at` is WRITE-ONCE: it records when work began, so a later + // prompt must not move it. The bail lives in the command as well as the + // frontend, because the frontend's copy is per-window state and a second + // window would otherwise re-submit. + + // Both new fields have to survive a record written before either existed, + // and `None` has to survive the trip: for `started_at` it is what makes a + // task read Todo, and for `base_sha` it is what stops the merge check + // trusting a creation point nobody recorded. + #[test] + fn a_task_written_before_started_at_and_base_sha_existed_reads_as_none() { + let mut value = serde_json::to_value(Task::default()).unwrap(); + assert!(value.get("started_at").is_some(), "started_at stopped serializing"); + assert!(value.get("base_sha").is_some(), "base_sha stopped serializing"); + let obj = value.as_object_mut().unwrap(); + obj.remove("started_at"); + obj.remove("base_sha"); + + let back: Task = serde_json::from_value(value).unwrap(); + assert_eq!(back.started_at, None); + assert_eq!(back.base_sha, None); + } + + #[test] + fn marking_started_stamps_once_and_never_moves_it() { + with_scratch_data_dir(|_data| { + crate::save_task(&a_task("t1", ProfileId::Root)).unwrap(); + + let first = crate::task_mark_started_sync("t1".into()).expect("the task is there"); + assert!( + chrono::DateTime::parse_from_rfc3339(&first).is_ok(), + "not RFC3339: {first}" + ); + assert_eq!( + crate::load_tasks_in(&ProfileId::Root)[0].started_at.as_deref(), + Some(first.as_str()), + ); + + // Second prompt, any time later: the SAME stamp comes back and the + // record is untouched. Compare the file's mtime as well as the + // value, because returning the right string while rewriting the + // file would still be a write on the user's first-prompt path. + let file = crate::tasks_dir_in(&ProfileId::Root).unwrap().join("t1.json"); + let before = fs::metadata(&file).unwrap().modified().unwrap(); + let bytes = fs::read_to_string(&file).unwrap(); + assert_eq!(crate::task_mark_started_sync("t1".into()).unwrap(), first); + assert_eq!(fs::metadata(&file).unwrap().modified().unwrap(), before); + // Byte equality as well as mtime: mtime is sub-second on APFS but + // would false-pass on a filesystem with 1s granularity. + assert_eq!(fs::read_to_string(&file).unwrap(), bytes); + assert_eq!( + crate::load_tasks_in(&ProfileId::Root)[0].started_at.as_deref(), + Some(first.as_str()), + ); + }); + } + + #[test] + fn marking_started_writes_back_to_the_profile_the_task_came_from() { + // Same trap as `task_touch`: the command reads ONE file, so the record + // it parses carries the DEFAULT profile (`profile` is `serde(skip)`). + // Forget the re-tag and the save lands in the root and the task exists + // twice. + with_scratch_data_dir(|data| { + crate::profiles::save_registry(data, &two_profile_registry()).unwrap(); + crate::save_task(&a_task("t2", ProfileId::Slug("home".into()))).unwrap(); + + let stamp = crate::task_mark_started_sync("t2".into()).expect("the task is there"); + assert!(!data.join("tasks/t2.json").exists(), "the mark moved t2 into the root"); + + let home = crate::load_tasks_in(&ProfileId::Slug("home".into())); + assert_eq!(home.len(), 1); + assert_eq!(home[0].started_at.as_deref(), Some(stamp.as_str())); + }); + } + + #[test] + fn marking_a_task_that_does_not_exist_is_an_error() { + with_scratch_data_dir(|_data| { + assert_eq!(crate::task_mark_started_sync("nope".into()), Err("no such task".into())); + }); + } + + // ── started_at backfill (data_migration_version 1) ────────────── + // + // Every record on disk predates the field, so without this the whole fleet + // reads Todo on upgrade. The rule is tested directly rather than through + // the version guard, or "running it twice changes nothing" would only + // prove that the guard bailed. + + #[test] + fn the_backfill_stamps_a_task_an_agent_has_run_in() { + let mut w = Task { + spawn_count: 3, + last_opened_at: Some("2026-02-01T09:00:00Z".into()), + created: "2026-01-01T00:00:00Z".into(), + ..Default::default() + }; + assert!(backfill_started_at(&mut w)); + assert_eq!(w.started_at.as_deref(), Some("2026-02-01T09:00:00Z")); + + // Idempotent: a second pass is a no-op and leaves the value alone. + assert!(!backfill_started_at(&mut w)); + assert_eq!(w.started_at.as_deref(), Some("2026-02-01T09:00:00Z")); + } + + #[test] + fn the_backfill_falls_back_to_created_when_the_task_was_never_opened() { + // `has_resumable_history` alone is enough: a session survived past the + // settle threshold, so somebody gave this task work. + let mut w = Task { + has_resumable_history: true, + created: "2026-01-01T00:00:00Z".into(), + ..Default::default() + }; + assert!(backfill_started_at(&mut w)); + assert_eq!(w.started_at.as_deref(), Some("2026-01-01T00:00:00Z")); + } + + #[test] + fn the_backfill_leaves_a_task_nothing_ever_ran_in_alone() { + let mut w = Task { created: "2026-01-01T00:00:00Z".into(), ..Default::default() }; + assert!(!backfill_started_at(&mut w)); + assert_eq!(w.started_at, None); + + // ...and it never overwrites a stamp that is already there, whatever + // the counters say. + let mut already = Task { + spawn_count: 9, + started_at: Some("2026-03-03T03:03:03Z".into()), + last_opened_at: Some("2026-05-05T05:05:05Z".into()), + created: "2026-01-01T00:00:00Z".into(), + ..Default::default() + }; + assert!(!backfill_started_at(&mut already)); + assert_eq!(already.started_at.as_deref(), Some("2026-03-03T03:03:03Z")); + } + + #[test] + fn the_backfill_sweeps_every_profile_and_stamps_the_version_last() { + with_scratch_data_dir(|data| { + crate::profiles::save_registry(data, &two_profile_registry()).unwrap(); + + let mut worked = a_task("ran", ProfileId::Slug("home".into())); + worked.spawn_count = 1; + worked.last_opened_at = Some("2026-02-01T09:00:00Z".into()); + crate::save_task(&worked).unwrap(); + // Root profile, never run in: must stay None. + crate::save_task(&a_task("idle", ProfileId::Root)).unwrap(); + + crate::migrate_started_at_backfill(); + + let home = crate::load_tasks_in(&ProfileId::Slug("home".into())); + assert_eq!(home[0].started_at.as_deref(), Some("2026-02-01T09:00:00Z")); + let root = crate::load_tasks_in(&ProfileId::Root); + assert_eq!(root[0].started_at, None, "nothing ran here, so nothing to stamp"); + + // The version is the guard, and it is written LAST. + assert_eq!( + crate::load_settings_inner().data_migration_version, + crate::STARTED_AT_BACKFILL_VERSION, + ); + // It is NOT the workspaces->tasks counter: bumping that one would + // re-run the rename migration on every v1 profile. + assert_eq!(crate::load_settings_inner().schema_version, 0); + + // A second launch changes nothing, including for a record that has + // since become eligible (the guard is committed, and the rule is a + // no-op on anything already stamped). + let mut later = a_task("idle", ProfileId::Root); + later.spawn_count = 4; + crate::save_task(&later).unwrap(); + crate::migrate_started_at_backfill(); + assert_eq!(crate::load_tasks_in(&ProfileId::Root)[0].started_at, None); + }); + } + + // ── task_git_phase_state: the DAG ─────────────────────────────── + // + // `merged_into_base` is biased toward FALSE on purpose: a missed Done + // costs the user nothing, a wrong Done tells them to archive live work. + // The discriminating case has its own test below, and the rest of this + // block exists so the bias cannot be quietly traded away for coverage. + + /// A main checkout with `main` at one commit, plus a task worktree on + /// `branch` cut the way termic cuts one: `git branch --no-track main` + /// and then `worktree add`. The two-step cut is what writes the + /// `branch: Created from main` reflog entry that `base_sha: None` falls + /// back to, so a fixture using `worktree add -b` would not be the same + /// shape. Returns (main tempdir, worktree tempdir, main path, worktree). + fn phase_fixture(branch: &str) -> (tempfile::TempDir, tempfile::TempDir, PathBuf, PathBuf) { + let main_dir = tempdir().unwrap(); + let wt_dir = tempdir().unwrap(); + let main = main_dir.path().to_path_buf(); + git_init_with_commit(&main); + git_set_identity(&main); + git_run(&main, &["branch", "--no-track", branch, "main"]); + let wt = wt_dir.path().join("wt"); + git_run(&main, &["worktree", "add", wt.to_str().unwrap(), branch]); + (main_dir, wt_dir, main, wt) + } + + /// Replay `branch`'s own commits onto `main` and fast-forward `main` to + /// them, i.e. what a forge's "Rebase and merge" does. `branch` itself is + /// left exactly where it was, which is the state the app then has to read. + fn rebase_and_merge(main: &Path, branch: &str) { + git_run(main, &["checkout", "-q", "-b", "replay", branch]); + git_run(main, &["rebase", "-q", "main"]); + git_run(main, &["checkout", "-q", "main"]); + git_run(main, &["merge", "-q", "--ff-only", "replay"]); + git_run(main, &["branch", "-q", "-D", "replay"]); + } + + /// What a forge's "Squash and merge" does: one commit on `main` carrying + /// the whole branch diff, with `branch` left untouched. + fn squash_and_merge(main: &Path, branch: &str) { + git_run(main, &["merge", "--squash", branch]); + git_run(main, &["commit", "-q", "-m", &format!("squash {branch}")]); + } + + // Restore is the THIRD place a task's branch gets cut, after the two + // create sites. An archive with `delete_branch` removes the branch, so + // restore cuts a new one from wherever the base is NOW, and the recorded + // creation point has to move with it: the stored `base_sha` names a commit + // the new branch was never cut from, and `task_git_phase_state` would + // measure against a base that was never its own. An archive that kept the + // branch cuts nothing, and then the original value is still true. + #[test] + fn restore_re_records_the_base_only_when_it_re_cuts_the_branch() { + let (_m, _w, main, _wt) = phase_fixture("alice-restore"); + let original = git_rev(&main, "main"); + + // The branch survived archive: nothing is cut, so nothing is recorded + // and the record keeps whatever it had. + assert_eq!(restore_task_branch(&main, "alice-restore", "main"), Ok(None)); + + // Now the archive-with-delete case. Drop the worktree and the branch, + // then move the base on, so a re-cut lands somewhere the original + // `base_sha` does not name. + git_run(&main, &["worktree", "remove", "--force", _wt.to_str().unwrap()]); + git_run(&main, &["branch", "-D", "alice-restore"]); + git_commit_file(&main, "m2.txt", "base moved\n", "M2"); + let moved = git_rev(&main, "main"); + assert_ne!(moved, original, "the base must have moved, or this proves nothing"); + + let recorded = restore_task_branch(&main, "alice-restore", "main") + .expect("the branch is gone, so it gets re-cut") + .expect("a re-cut must report the commit it cut from"); + assert_eq!(recorded, moved, "it records where the branch ACTUALLY starts now"); + assert_eq!(git_rev(&main, "refs/heads/alice-restore"), moved); + + // ...and restoring again, with the branch now back, records nothing. + assert_eq!(restore_task_branch(&main, "alice-restore", "main"), Ok(None)); + } + + #[test] + fn a_fresh_branch_is_not_merged() { + let (_m, _w, _main, wt) = phase_fixture("alice-fresh"); + let st = git_phase_state(&wt, "alice-fresh", "main", None).unwrap(); + assert_eq!(st.own_commits, 0); + assert!(!st.merged_into_base, "nothing has happened here yet"); + assert!(st.base_known, "the creation entry is right there in the reflog"); + assert!(!st.dirty); + assert_eq!(st.ahead, None, "no remote branch is not the same fact as zero"); + } + + // THE DISCRIMINATOR. A fresh branch an agent fast-forwarded onto a base + // that moved is structurally identical to an ff-merged branch: its tip is + // an ancestor of the base and differs from where it started. The DAG + // cannot tell them apart at all, so the reflog has to: this one was never + // committed on. + #[test] + fn a_fresh_branch_pulled_up_to_a_moved_base_is_not_merged() { + let (_m, _w, main, wt) = phase_fixture("alice-pull"); + git_commit_file(&main, "m2.txt", "base moved\n", "M2"); + git_run(&wt, &["merge", "-q", "--ff-only", "main"]); + + let st = git_phase_state(&wt, "alice-pull", "main", None).unwrap(); + assert_eq!(st.own_commits, 0); + assert!(st.base_known); + assert!( + !st.merged_into_base, + "a branch that only ever pulled the base in has nothing to have merged" + ); + + // The same answer when the base sha came off the record rather than + // the reflog: it is the reflog SUBJECTS that rule this out, and a + // recorded base must not smuggle it past them. + let s = git_rev(&main, "main~1"); + let with_record = git_phase_state(&wt, "alice-pull", "main", Some(&s)).unwrap(); + assert!(!with_record.merged_into_base); + } + + // The OTHER wrong-Done, and the one the two-tier rule as originally + // specified walked straight into. An agent that committed and then threw + // the work away with `reset --hard` onto a base that had moved satisfies + // every DAG condition of tier 2: tip an ancestor of the base, moved off S, + // and a `commit:` line in its reflog. MEASURED on this fixture before the + // fix, every one of those was true and the answer came back merged, which + // would have told the user to archive a task whose work never landed. + // Asking whether the committed sha is REACHABLE from the base is what + // separates them: here it is not. + #[test] + fn a_branch_that_reset_its_work_away_is_not_merged() { + let (_m, _w, main, wt) = phase_fixture("alice-reset"); + git_commit_file(&wt, "c1.txt", "one\n", "C1"); + let discarded = git_rev(&wt, "HEAD"); + git_commit_file(&main, "m2.txt", "base moved\n", "M2"); + git_run(&wt, &["reset", "-q", "--hard", "main"]); + + // The preconditions really are all met, or this proves nothing. + let st = git_phase_state(&wt, "alice-reset", "main", None).unwrap(); + assert_eq!(st.own_commits, 0); + assert!(st.base_known); + assert!(git(&["merge-base", "--is-ancestor", "refs/heads/alice-reset", "main"], &wt).is_ok()); + assert!( + git(&["merge-base", "--is-ancestor", &discarded, "main"], &wt).is_err(), + "the committed work must be absent from the base, or this is not the case", + ); + + assert!(!st.merged_into_base, "work that was reset away never reached the base"); + } + + #[test] + fn one_commit_fast_forwarded_into_the_base_is_merged() { + let (_m, _w, main, wt) = phase_fixture("alice-ff"); + git_commit_file(&wt, "c1.txt", "one\n", "C1"); + git_run(&main, &["merge", "-q", "--ff-only", "alice-ff"]); + + let st = git_phase_state(&wt, "alice-ff", "main", None).unwrap(); + assert_eq!(st.own_commits, 0, "a fast-forward leaves nothing on the branch side"); + assert!(st.base_known); + assert!(st.merged_into_base, "tier 2: ancestor, moved off S, and committed on"); + } + + #[test] + fn a_merge_commit_into_the_base_is_merged() { + let (_m, _w, main, wt) = phase_fixture("alice-merge"); + git_commit_file(&wt, "c1.txt", "one\n", "C1"); + // The base moves, so the merge cannot fast-forward. + git_commit_file(&main, "m2.txt", "base moved\n", "M2"); + git_run(&main, &["merge", "-q", "--no-ff", "-m", "merge alice-merge", "alice-merge"]); + + let st = git_phase_state(&wt, "alice-merge", "main", None).unwrap(); + assert_eq!(st.own_commits, 0); + assert!(st.merged_into_base); + } + + #[test] + fn rebase_and_merge_is_merged() { + let (_m, _w, main, wt) = phase_fixture("alice-rebase"); + git_commit_file(&wt, "c1.txt", "one\n", "C1"); + git_commit_file(&wt, "c2.txt", "two\n", "C2"); + git_commit_file(&main, "m2.txt", "base moved\n", "M2"); + rebase_and_merge(&main, "alice-rebase"); + + let st = git_phase_state(&wt, "alice-rebase", "main", None).unwrap(); + assert_eq!(st.own_commits, 2, "the originals are still only on the branch side"); + assert!(st.merged_into_base, "tier 1: every branch-side commit has a twin on base"); + } + + #[test] + fn a_squash_merge_of_two_commits_is_merged() { + let (_m, _w, main, wt) = phase_fixture("alice-squash"); + git_commit_file(&wt, "c1.txt", "one\n", "C1"); + git_commit_file(&wt, "c2.txt", "two\n", "C2"); + squash_and_merge(&main, "alice-squash"); + + let st = git_phase_state(&wt, "alice-squash", "main", None).unwrap(); + assert_eq!(st.own_commits, 2); + assert!( + st.merged_into_base, + "no individual commit has a twin, so only the squash tier can see this" + ); + } + + #[test] + fn a_branch_that_merged_the_base_in_and_was_then_squash_merged_is_merged() { + let (_m, _w, main, wt) = phase_fixture("alice-back"); + git_commit_file(&wt, "c1.txt", "one\n", "C1"); + git_commit_file(&main, "m2.txt", "base moved\n", "M2"); + // Agents do this constantly: pull the base back into the branch. + git_run(&wt, &["merge", "-q", "--no-ff", "-m", "merge main into alice-back", "main"]); + git_commit_file(&wt, "c2.txt", "two\n", "C2"); + squash_and_merge(&main, "alice-back"); + + let st = git_phase_state(&wt, "alice-back", "main", None).unwrap(); + assert!(st.own_commits >= 1); + assert!(st.merged_into_base); + } + + // The case `--no-merges` exists for, isolated. CONTROL RUN (git 2.50.1, + // this exact graph): with `--no-merges` the cherry-mark output is the one + // line `= C1`; drop the flag and it becomes `> merge main into alice-both` + // plus `= C1`, so tier 1 sees a non-equivalent commit and reports NOT + // merged. A merge commit has no patch-id, so it can never be marked + // equivalent, and dropping merges from the question is safe because their + // content arrives with the commits they merge. + #[test] + fn a_branch_that_merged_the_base_in_and_was_then_rebase_merged_is_merged() { + let (_m, _w, main, wt) = phase_fixture("alice-both"); + git_commit_file(&wt, "c1.txt", "one\n", "C1"); + git_commit_file(&main, "m2.txt", "base moved\n", "M2"); + git_run(&wt, &["merge", "-q", "--no-ff", "-m", "merge main into alice-both", "main"]); + rebase_and_merge(&main, "alice-both"); + + let st = git_phase_state(&wt, "alice-both", "main", None).unwrap(); + assert_eq!(st.own_commits, 2, "the branch's own commit plus the merge commit"); + assert!(st.merged_into_base, "the merge commit must not sink tier 1"); + } + + // Losing the reflog costs tier 2 outright, and a recorded `base_sha` does + // not buy it back: S is only one of the three things tier 2 needs, and the + // proof that anything was committed here is the other one. + #[test] + fn an_expired_reflog_costs_the_fast_forward_tier_even_with_a_recorded_base() { + let (_m, _w, main, wt) = phase_fixture("alice-noreflog"); + let s = git_rev(&main, "main"); + git_commit_file(&wt, "c1.txt", "one\n", "C1"); + git_run(&main, &["merge", "-q", "--ff-only", "alice-noreflog"]); + // What `gc` does once gc.reflogExpire (90 days by default) passes. + // Branch reflogs live in the COMMON git dir, shared by every worktree. + fs::remove_file(main.join(".git/logs/refs/heads/alice-noreflog")).unwrap(); + + let gone = git_phase_state(&wt, "alice-noreflog", "main", None).unwrap(); + assert!(!gone.base_known, "nothing is left to recover S from"); + assert!(!gone.merged_into_base); + + let recorded = git_phase_state(&wt, "alice-noreflog", "main", Some(&s)).unwrap(); + assert!(recorded.base_known, "the record still knows where the branch was cut"); + assert!( + !recorded.merged_into_base, + "S alone is not evidence that work was committed on this branch" + ); + } + + #[test] + fn an_untracked_file_counts_as_dirty() { + let (_m, _w, _main, wt) = phase_fixture("alice-dirty"); + assert!(!git_phase_state(&wt, "alice-dirty", "main", None).unwrap().dirty); + + // Untracked on purpose: it is local work that exists on this machine + // and nowhere else, which is exactly what the field is asked. + fs::write(wt.join("scratch.txt"), "not added\n").unwrap(); + assert!(git_phase_state(&wt, "alice-dirty", "main", None).unwrap().dirty); + + // ...and so is a tracked file edited but not staged. + fs::remove_file(wt.join("scratch.txt")).unwrap(); + fs::write(wt.join("base.txt"), "edited\n").unwrap(); + assert!(git_phase_state(&wt, "alice-dirty", "main", None).unwrap().dirty); + } + + #[test] + fn ahead_counts_unpushed_commits_and_is_none_without_a_remote_branch() { + let origin_dir = tempdir().unwrap(); + git_run(origin_dir.path(), &["init", "-q", "--bare", "-b", "main"]); + let (_m, _w, main, wt) = phase_fixture("alice-ahead"); + git_run(&main, &["remote", "add", "origin", origin_dir.path().to_str().unwrap()]); + git_run(&main, &["push", "-q", "origin", "main"]); + + // A remote exists, but this branch has never been pushed: there is + // nothing to be ahead OF, which is None rather than a count. + assert_eq!(git_phase_state(&wt, "alice-ahead", "main", None).unwrap().ahead, None); + + git_run(&wt, &["push", "-q", "-u", "origin", "alice-ahead"]); + assert_eq!(git_phase_state(&wt, "alice-ahead", "main", None).unwrap().ahead, Some(0)); + + git_commit_file(&wt, "c1.txt", "one\n", "C1"); + assert_eq!(git_phase_state(&wt, "alice-ahead", "main", None).unwrap().ahead, Some(1)); + + git_run(&wt, &["push", "-q"]); + assert_eq!(git_phase_state(&wt, "alice-ahead", "main", None).unwrap().ahead, Some(0)); + } + + // A base that resolves to nothing must not fall through to HEAD. In a task + // worktree HEAD *is* the branch, so `own_commits` would be 0 and + // `is-ancestor T B` trivially true, and a base branch deleted after a + // merge would report every live task merged. + #[test] + fn an_unresolvable_base_reports_not_merged_rather_than_measuring_against_head() { + let (_m, _w, _main, wt) = phase_fixture("alice-nobase"); + git_commit_file(&wt, "c1.txt", "one\n", "C1"); + + let st = git_phase_state(&wt, "alice-nobase", "gone/deleted-base", None).unwrap(); + assert_eq!(st.own_commits, 0); + assert!(!st.merged_into_base); + } + + // The squash tier writes a commit object into the USER'S repo on every + // single poll, so it had better be the same object every time. Git hashes + // the whole commit header, so an unpinned author/committer date makes a new + // dangling object per second and the repo accumulates them until `gc`. + // + // CONTROL RUN (git 2.50.1): two pinned `commit-tree` calls on one tree + // returned the same sha, an unpinned call on the same tree returned a + // different one, and `git fsck --dangling` counted 2 objects after three + // calls rather than 3. + #[test] + fn the_squash_probe_writes_the_same_object_every_time() { + let dir = tempdir().unwrap(); + let repo = dir.path(); + git_init_with_commit(repo); + git_set_identity(repo); + + let probe = |env: &[(&str, &str)]| { + String::from_utf8_lossy( + &git_bytes_env(&["commit-tree", "HEAD^{tree}", "-m", "probe"], repo, env).unwrap(), + ) + .trim() + .to_string() + }; + let first = probe(&PHASE_PROBE_ENV); + let second = probe(&PHASE_PROBE_ENV); + assert_eq!(first, second, "the pinned probe must dedupe, not accumulate"); + assert_ne!( + first, + probe(&[]), + "the control: without the pins the object is a different one, which \ + is what would pile up in the user's repo" + ); + } + + #[test] + fn a_branch_that_does_not_exist_is_an_error() { + let (_m, _w, _main, wt) = phase_fixture("alice-exists"); + assert!(git_phase_state(&wt, "alice-nope", "main", None).is_err()); + } + + #[test] + fn phase_state_refuses_an_archived_or_main_checkout_task() { + let (_m, _w, main, wt) = phase_fixture("alice-guard"); + let base = Task { + branch: "alice-guard".into(), + base_branch: "main".into(), + path: wt.to_string_lossy().into_owned(), + ..Default::default() + }; + // The control: this shape answers fine before either flag is set. + assert!(task_phase_state_for(&base).is_ok()); + + let archived = Task { archived: true, ..base.clone() }; + assert!(task_phase_state_for(&archived).is_err()); + + // A main-checkout task runs on the project's live checkout, so every + // question here would be about somebody else's work. + let live = Task { + is_main_checkout: true, + path: main.to_string_lossy().into_owned(), + branch: "main".into(), + ..base.clone() + }; + assert!(task_phase_state_for(&live).is_err()); + } + // ── Extra named ports (GH #196) ───────────────────────────────── #[test] From d0ae3b8eee68a1d17db27b7452c4569c2f51ae70 Mon Sep 17 00:00:00 2001 From: Vinodkumar Naidu <7994336+nvkvin@users.noreply.github.com> Date: Thu, 17 Sep 2026 12:24:04 +0530 Subject: [PATCH 04/10] feat(task): start on the first prompt, and read git for the review half Two holes in the derived phase, both of which made it say the wrong thing about a real task. Backlog was unreachable, so it is now Todo and it is the state every task starts in. Creating a task spawns its agent, so a spawn was never evidence anybody had given it work: the agent is sitting at its prompt. `markStarted` stamps `started_at` at the first prompt a human submits, from the nine places one can be sent (the terminal's own submit and resend, the seed prompt, both CLI send paths, the prompt runner's two entries and the review-comment sender). A task created with a prompt is therefore In progress from birth and one created empty stays Todo until somebody types, which is the distinction the phase existed to draw. In review needed a second route in. An open PR is one, but a repo with no forge, or a branch nobody has opened a PR on yet, had none: the work was committed and pushed and the phase still read In progress. `taskGit` polls `task_git_phase_state` and the rule is own commits, clean tree, nothing unpushed, and no PR at all. Draft and closed PRs outrank it on purpose, because both are a person saying how ready the work is and a clean branch underneath does not overrule that. `base_known` is deliberately not a condition: own commits are counted against the base branch, so gating on the creation commit would deny In review to every imported worktree and reused branch. The poller is scoped to the dashboard's mount, which is the only place the phase is drawn and is only mounted while no task is open, so it never shells out to git behind a working agent. Six tasks a pass, a 30s floor per task, an epoch counter so a pass that outlives the page drops its writes, and `sameGitState` so an unchanged answer never reaches the store (bear trap 8). Archived, main-checkout and open/merged-PR tasks are skipped entirely: the first two have no branch worth comparing and the last two already know their phase. e2e covers the two new claims end to end: a task stays Todo after its agent has spawned and moves on the first prompt, and a branch driven through commit, push and a real fast-forward merge reads In review then Done with no PR anywhere. The fixture's refs are captured before the block and restored after, and the teardown asserts only what this block owns, since `reset --hard` leaves another spec's untracked file in place and a blanket check would blame us for it. Refs #292 Refs #298 --- docs/e2e-coverage.md | 4 +- docs/performance.md | 2 + docs/ui.md | 111 +++++- e2e/helpers.ts | 12 + e2e/specs/projects.e2e.ts | 463 ++++++++++++++++++---- src/components/task/TerminalPane.tsx | 10 + src/components/views/Dashboard.tsx | 103 +++-- src/lib/cliAgentState.test.ts | 2 + src/lib/cliRpc.ts | 5 + src/lib/dirTabs.test.ts | 2 + src/lib/ipc.ts | 13 + src/lib/runPrompt.ts | 5 + src/lib/seedPrompt.ts | 6 + src/lib/sendComments.ts | 3 + src/lib/taskPhase.test.ts | 262 +++++++++--- src/lib/taskPhase.ts | 146 ++++--- src/lib/trayAttention.test.ts | 2 + src/lib/types.ts | 48 +++ src/main.tsx | 12 +- src/store/agentHooksSync.test.ts | 2 + src/store/app.test.ts | 106 ++++- src/store/app.ts | 43 +- src/store/cliPrompts.integration.test.ts | 2 + src/store/cliSend.integration.test.ts | 2 + src/store/cliTab.integration.test.ts | 2 + src/store/cliTabClose.integration.test.ts | 2 + src/store/pr.test.ts | 2 + src/store/race.integration.test.ts | 2 + src/store/resume.integration.test.ts | 2 + src/store/scratchpad.integration.test.ts | 2 + src/store/selectorFanout.test.ts | 2 + src/store/taskGit.test.ts | 362 +++++++++++++++++ src/store/taskGit.ts | 210 ++++++++++ 33 files changed, 1699 insertions(+), 253 deletions(-) create mode 100644 src/store/taskGit.test.ts create mode 100644 src/store/taskGit.ts diff --git a/docs/e2e-coverage.md b/docs/e2e-coverage.md index cc5dca44..e0c9ed1b 100644 --- a/docs/e2e-coverage.md +++ b/docs/e2e-coverage.md @@ -183,8 +183,8 @@ 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 | A task created but never opened reads `backlog` on its row and is counted by the Backlog pill; opening it once (the pane mounts, fakeagent spawns, `recordSpawn` folds the count back) moves the row to `in_progress` and drops the pill count by one, or removes the pill when that was the last one; a seeded PR then drives the row 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`); 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 | `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. 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`, which is the one assertion in these two rows with no DOM to read instead | `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. All four pills are always rendered, in the order all / todo / in_progress / in_review / done, 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` | | ✅ 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/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/ui.md b/docs/ui.md index bb55393d..895c9cf9 100644 --- a/docs/ui.md +++ b/docs/ui.md @@ -760,46 +760,113 @@ 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`, and nothing a person types. There -is no status field to set and none to go stale. Four values, first match wins: -**Done** when the task is archived or its PR is merged, **In review** when the -PR is open, **In progress** when the PR is draft or closed-unmerged or the task -has ever spawned (`spawn_count > 0`) or has resumable history, **Backlog** -otherwise. +record, plus the live PR snapshot in `usePr`, plus the live git state in +`useTaskGit`, and nothing a person types. There is no status field to set and +none to go stale. Four values, first match wins: + +| Phase | When | +|---|---| +| **Done** | `archived`, or the PR is merged, or `merged_into_base` | +| **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 | + +**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 Backlog, because the +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 lookup has `pr === null` like "no PR" does and -therefore falls through to the record, so a machine with no `gh`/`glab` still -phases correctly; a shell spawn counts as progress, because `task_record_spawn` -fires for every spawn; and a main-checkout task is never polled at all -(`pollableTasks` skips `is_main_checkout`), so it only leaves In progress by -being archived. Backlog is rare in practice: every GUI create path activates -the new task and activation spawns its default tab, so a task is In progress -within a second of existing. +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`. + +**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. + +**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. +

+ +