feat: add server-owned day item store and CLI - #391
Conversation
|
Auto-merge decision: Repository class: |
|
Blast-radius review of the original PR head found two runtime-boundary defects: Proof on this head: |
Review verdict: BLOCKFour reviewers at head
Two reviewers converged independently on the store's concurrency model. Every finding below was re-verified against the branch by hand. P1 — the board is loaded once and never reloaded
Two named servers on one host therefore diverge permanently: items added through server B are invisible to server A until A restarts, and the gap only widens. Lost updates are the narrower case — the same item has to be mutated from both servers, which is uncommon since ids are time-based ( P1 — automatic
|
|
Auto-merge decision: Repository class: |
|
Blast-radius review: The day store reloads and locks the disk-backed mutation base; disabled work indexes cannot derive completion from stale snapshots; active day links feed direct merged-PR and ticket observations without entering render paths. Completed and dismissed items are excluded from producer polling. Proof: |
|
Blast-radius review: PROVEN — no P0/P1/P2 findings. The delta only isolates |
|
Blast-radius review: PROVEN — no P0/P1/P2 findings. The delta only makes the pomodoro deadline test helper explicitly disable the newly default-enabled work index. Production code is unchanged, the runtime test module is green, and |
|
Blast-radius review: PROVEN — no P0/P1/P2 findings. The delta only makes shared scheduler test helpers explicitly disable the newly default-enabled work index. Dedicated work-index coverage remains explicit; production paths are unchanged. Root app tests pass 206/206, five headless deadline tests pass, and |
Link-derived day completion needs the work index, but turning the index on by default makes every server shell out to gh and Linear on a refresh interval for all users. Keep the documented opt-in default and say in the config reference that automatic completion follows the index. refs #391 Claude-Session: https://claude.ai/code/session_01FYVGxU6SxcYSvJJCNKYbZJ
The cli harness set XDG_CONFIG_HOME and XDG_RUNTIME_DIR but left XDG_STATE_HOME alone, so spawned servers resolved host-global state to the developer's own directory. The day board store makes that load-bearing: parallel tests would share one real board and its lock file. refs #391 Claude-Session: https://claude.ai/code/session_01FYVGxU6SxcYSvJJCNKYbZJ
GitHub reports canonical pull request urls while a stored link may keep a trailing slash. The work index trims one for lookup but completion compared the stored value byte for byte, so a slashed link never closed and could not be removed. Also correct the generated config, which still advertised the work index as enabled by default. refs #391 Claude-Session: https://claude.ai/code/session_01FYVGxU6SxcYSvJJCNKYbZJ
GitHub accepts burntsushi/ripgrep and reports BurntSushi/ripgrep, so a link stored as typed never matched the canonical url and the item stayed todo with no way to remove the link. Compare urls case-insensitively next to the existing slash trimming, share one helper with the work index, and cover the timeout and failure fallbacks that used raw equality. refs #391 Claude-Session: https://claude.ai/code/session_01FYVGxU6SxcYSvJJCNKYbZJ
Review verdict at
|
| Axis | Verdict | New findings |
|---|---|---|
| standards-only | PASS | none |
| spec | BLOCK → fixed | trailing-slash links, generated config default |
| blast-radius | BLOCK → fixed | case-rewritten links |
| architecture | BLOCK | 2 open, below |
Fixed since the first verdict
- Trailing-slash PR links never closed and could not be removed. Now compared with slashes trimmed.
- The generated config at
src/main.rsstill advertisedwork_index.enabled = trueafter the default was restored tofalse. - GitHub rewrites owner and repo case (
burntsushi/ripgrep→BurntSushi/ripgrep), so a link stored as typed never matched. Comparison is now case-insensitive, shared with the work index, and covers the timeout and failure fallbacks that used raw equality.
Open, pending a decision
P1 — cross-server pane binding collision. DayBinding stores host plus pane_id, but NEXT_WORKSPACE_ID (src/workspace.rs:129) is a per-process counter starting at 1, so two named servers on one host both own w1:p1. An item bound in one session resolves against whichever server serves day.list, so it can report another pane's column. Either a server identifier joins the binding here, or the sync PR fixes it while revisiting binding identity across machines.
P2 — unbounded work-index refresh inputs. board.links() excludes only persisted done_at and dismissed items, so items completed by link derivation stay in the producer input set forever, each costing a sequential provider read. Either link-derived completion settles into done_at, or the sync PR bounds the set.
Deferred to later phases
- AC11's
<id>.conflict-<host>.mdfiles fail the current filename/front-matter id equality check (src/day.rs:384). - AC15 is a view criterion; this diff has no UI.
- Day mutations need render invalidation and indexed pane/link lookup before the UI PR.
Note on verification: just check-parallel is unreliable on this host under load — base and head each failed it on different tests, all passing in isolation. CI is the authority for this PR.
|
Auto-merge decision: Repository class: |
Pane ids restart at 1 in every server process, so two named sessions on one host both own w1:p1 and an item bound in one could report the other's pane and column. Bindings now carry the session that created them, and a binding from another server reads as unbound rather than resolving to a stranger's pane. Bindings written before this field stay host-scoped so existing item files keep loading. refs #391 Claude-Session: https://claude.ai/code/session_01FYVGxU6SxcYSvJJCNKYbZJ
An item completed from its links carried no done_at, so it stayed in the work index input set forever and cost a provider read on every refresh. Record the completion once when it is first observed, so the item drops out of the set. refs #391 Claude-Session: https://claude.ai/code/session_01FYVGxU6SxcYSvJJCNKYbZJ
Settling cloned the item from the list snapshot and rewrote the whole file, so a concurrent dismiss, done, note, link, or bind from another server could be overwritten. Take the store lock and re-read instead, and only record the completion if the item is still neither done nor dismissed. refs #391 Claude-Session: https://claude.ai/code/session_01FYVGxU6SxcYSvJJCNKYbZJ
Only a named session produced a server id, so every unnamed server shared "default" and two of them could still resolve each other's pane ids. Derive the id from the resolved socket path instead, and hash it: the raw path is a home directory, and item files sync between machines. refs #391 Claude-Session: https://claude.ai/code/session_01FYVGxU6SxcYSvJJCNKYbZJ
A link completes an item through the work index, which looks pull requests up by owner, repo and number and tickets up by identifier. A pasted browser location such as .../pull/9/files, or an identifier with padding around it, was stored happily and then matched nothing: the call reported success while the item's automatic completion was quietly broken, with no way to take the link back. Store the canonical form, and refuse a value the lookup could never resolve. refs #391 Claude-Session: https://claude.ai/code/session_01FYVGxU6SxcYSvJJCNKYbZJ
Showing an item as done from an old snapshot is harmless because derivation runs again and corrects it. Writing done_at down is not: the item leaves the work index input set, so nothing looks at its links again and a ticket reopened afterwards stays done forever. A snapshot restored from disk at startup, or one a failed provider call left behind, was enough to make that permanent. Require a reading this server took within one refresh interval, and keep the completion time a retried day.done already recorded. refs #391 Claude-Session: https://claude.ai/code/session_01FYVGxU6SxcYSvJJCNKYbZJ
HERDR_SOCKET_PATH=herdr.sock names a different socket in every working directory, but the bare string was hashed as-is: the parent of a bare file name is empty, canonicalization failed, and the fallback kept the relative spelling. Two servers started in different directories under one session therefore shared an identity and could still resolve each other's pane ids. refs #391 Claude-Session: https://claude.ai/code/session_01FYVGxU6SxcYSvJJCNKYbZJ
day.link accepted any single token as a ticket, but the work index resolves identifiers through normalize_ticket_id, which recognizes the configured teams only. Anything else was accepted, stored, and then dropped in silence, and the rejection message invented an example the resolver would itself discard. Reject a pull request numbered zero for the same reason. refs #391 Claude-Session: https://claude.ai/code/session_01FYVGxU6SxcYSvJJCNKYbZJ
A snapshot stamps observed_at at refresh time while serving provider-cached reads up to ten minutes old, so freshness never described the age of the evidence itself. Settling a reopened ticket that way wrote a permanent wrong answer, because a settled item leaves the refresh input set. Only merged pull requests settle now. A merge is terminal, so evidence of one cannot become wrong however it was cached. An item with any linked ticket derives its column on every list and is never written down. Availability is read for GitHub alone, so an unrelated Linear or Missive outage no longer holds a verified item open. refs #391 Claude-Session: https://claude.ai/code/session_01FYVGxU6SxcYSvJJCNKYbZJ
encode_error takes impl Into<String>, so the formatted message did not need borrowing. Clippy rejects it and CI failed on all three platforms. refs #391 Claude-Session: https://claude.ai/code/session_01FYVGxU6SxcYSvJJCNKYbZJ
The session name rode along in the identity to keep it readable, but it is not stable. Restarting one HERDR_SOCKET_PATH with HERDR_SESSION unset renamed the same server from work-<hash> to sock-<hash>, so every binding it had written silently read as unbound. The socket path already identifies a server on its own, so nothing else belongs in the value. refs #391 Claude-Session: https://claude.ai/code/session_01FYVGxU6SxcYSvJJCNKYbZJ
Neither path alone identifies the server a pane id belongs to. The socket path routes a client, but HERDR_SESSION moves the data directory that persist::io restores workspaces and panes from, so one socket path can serve two unrelated sets of panes across restarts. The data directory alone has the mirror problem, since HERDR_SOCKET_PATH can point two servers sharing it at different sockets. Hashing the pair keeps one server stable across its own restart and separates any two servers whose pane ids mean different things. refs #391 Claude-Session: https://claude.ai/code/session_01FYVGxU6SxcYSvJJCNKYbZJ
Item files and their directory were created under the ambient umask, which is usually 022. A day item carries the title, note, tickets, and pull request links a person wrote down, so all of it was readable by any other account that could traverse the state directory. refs #391 Claude-Session: https://claude.ai/code/session_01FYVGxU6SxcYSvJJCNKYbZJ
HERDR_PANE_ID names a pane of the server the process started under. --session points the request at a different server, where the same public id belongs to an unrelated pane, so binding silently recorded a stranger's pane. Resolve the pane through the server actually being addressed instead. refs #391 Claude-Session: https://claude.ai/code/session_01FYVGxU6SxcYSvJJCNKYbZJ
Naming the server that issued a pane id needs an identity that survives a restart together with the panes it describes. Five review rounds established that no id derived from ambient facts does: paths name a location rather than a pane-set incarnation, so deleting and recreating a session reuses them while renumbering panes from 1, and a session name renames the server whenever it is unset. Persisting such an identity reaches restore and handoff, and the sync work has to revisit binding identity across machines anyway. Drop the field rather than ship a scheme that is wrong in a different way each round. A binding is host-scoped again, with the collision two named servers on one host can hit recorded where the binding is defined. refs #391 Claude-Session: https://claude.ai/code/session_01FYVGxU6SxcYSvJJCNKYbZJ
GitHub answers a request for a renamed or transferred repository with the new url, so a pull request link stored before the rename stops matching and its item never completes automatically. Recognizing it needs the producer to remember which request produced each work item, which reaches every construction of one. The item stays open and can be completed by hand, so name the limit where the matching happens rather than widen this change. refs #391 Claude-Session: https://claude.ai/code/session_01FYVGxU6SxcYSvJJCNKYbZJ
A requested create mode is only an upper bound, so the ambient umask subtracted from it: at the usual 022 an item, its directory, and the store lock were published to every account that could reach the state directory, and at a permissive setting they came out unreadable to their own owner and the store could not be read or mutated again. Set each one exactly after creating it, restricting every directory component as it appears. refs #391 Claude-Session: https://claude.ai/code/session_01FYVGxU6SxcYSvJJCNKYbZJ
Discarding the inherited pane on any --session left `herdr day bind <id>` binding whichever pane happened to be focused, because the server falls back to focus when no caller is named. Ask whether the request addresses the server this process was started under instead, which naming your own session does. refs #391 Claude-Session: https://claude.ai/code/session_01FYVGxU6SxcYSvJJCNKYbZJ
`herdr day bind <id>` printed a debug-formatted io error carrying the whole server response when no pane could be resolved. Say what to do instead, the way every other argument problem in the command is reported. refs #391 Claude-Session: https://claude.ai/code/session_01FYVGxU6SxcYSvJJCNKYbZJ
Comparing session names could not see a pane reached over HERDR_SOCKET_PATH, which has no name, so `--session default` from one routed the request to the default server while still handing it the inherited pane id and binding a colliding pane there. Compare the resolved socket instead, which is what a session selection actually moves. refs #391 Claude-Session: https://claude.ai/code/session_01FYVGxU6SxcYSvJJCNKYbZJ
A directory was created at 0777 minus the umask and only then restricted, so under a permissive umask it was briefly reachable by any account that could traverse to it. Ask mkdir for 0700 so the window is never wider than the result. A chmod that fails is no longer discarded either: it is what restores the bits the umask took, so swallowing it left an item nothing could open behind a write that reported success. refs #391 Claude-Session: https://claude.ai/code/session_01FYVGxU6SxcYSvJJCNKYbZJ
A stopped server made `herdr day bind <id>` exit 2 with a plain message instead of the ordinary server error and exit 1. Only a server that answered without a pane is an argument problem. Linking one pull request twice in different owner case now stores it once, since GitHub rewrites that case and both spellings would each cost a provider read. refs #391 Claude-Session: https://claude.ai/code/session_01FYVGxU6SxcYSvJJCNKYbZJ
`DirBuilder::mode` is Unix only, so the Windows build saw a `mut` binding nothing ever mutated and rejected it. Construct the builder inside the branch that configures it. refs #391 Claude-Session: https://claude.ai/code/session_01FYVGxU6SxcYSvJJCNKYbZJ
One live server is reachable through a symlinked parent under two spellings, and comparing the text called one of them somewhere else: `day bind` then dropped the caller's own pane and bound whichever was focused. Resolve the directory holding each socket before comparing. refs #391 Claude-Session: https://claude.ai/code/session_01FYVGxU6SxcYSvJJCNKYbZJ
Deduplication ran per item, so two items linking one pull request in different owner case each bought a provider read on every refresh. Fold the case across the whole input set, sorting on the folded key so the spellings being compared actually land next to each other. refs #391 Claude-Session: https://claude.ai/code/session_01FYVGxU6SxcYSvJJCNKYbZJ
Owner-only permissions are a Unix mode, so on Windows a store under a shared directory inherits that directory's ACLs. Creating a directory is also not synced into its own parent, matching the session snapshot's existing contract rather than closing it for one store. refs #391 Claude-Session: https://claude.ai/code/session_01FYVGxU6SxcYSvJJCNKYbZJ
The session tests compile on every target, so a bare `std::os::unix::fs::symlink` broke the Windows build. Creating one there needs a privilege a plain user does not have, so gate the case rather than port it. refs #391 Claude-Session: https://claude.ai/code/session_01FYVGxU6SxcYSvJJCNKYbZJ
A detached server stops polling the work index after six idle intervals, which also swallowed the refresh `day link` asks for. A CLI-only user could link a merged pull request and read `todo` from every later `day list` until a TUI attached. Mark a refresh someone asked for and let it through: it has a reader by definition. refs #391 Claude-Session: https://claude.ai/code/session_01FYVGxU6SxcYSvJJCNKYbZJ
Two spellings of one socket compared unequal, so `day bind` dropped the caller's own pane and bound whichever was focused. refs #391 Claude-Session: https://claude.ai/code/session_01FYVGxU6SxcYSvJJCNKYbZJ
A link's own refresh request was spent the first time one ran, so an item linked to a pull request that was still open went back to being unwatched and its later merge was never seen. `day list` is where a CLI-only user reads the board, so let it renew the request while anything is still waiting. The refresh interval still decides how often the providers are asked. refs #391 Claude-Session: https://claude.ai/code/session_01FYVGxU6SxcYSvJJCNKYbZJ
…dence Renewal keyed on the item not reading `done`, which is wrong for a ticket: a ticket can reopen, so its completion is never written down and the column is only ever derived. A detached server stopped looking the moment the ticket first read done and showed that answer forever. Key on the written completion instead, which is the one answer that cannot change. refs #391 Claude-Session: https://claude.ai/code/session_01FYVGxU6SxcYSvJJCNKYbZJ
1 participant
In plain words
Adds the server-owned day-item store and the
herdr dayAPI/CLI. It deliberately does not add git sync or any TUI day-board view.Acceptance criteria
items/<id>.mdfile per item, JSON-shaped YAML front matter and title body; every mutation atomically replaces only that file.todo,working,blocked, anddoneare derived on API reads from completion, exact linked-work state, and the local live binding. No column is persisted.PaneActivityclock at request time;[day_board].stale_afterdefaults to 600 seconds.AppStatetest retains an item whose pane disappeared, returnstodoplus a notice, and passes invariants before and after derivation.AppState; seven neutralday.*JSON methods expose all behavior without adding a private TUI-socket path.herdr day add|list|bind|link|note|done|dismisstargets the local server and bind resolves the caller pane inside an agent pane.herdr day list --jsonreturns each flattened item withcolumn,stale, and any missing-pane notice.Verification
just lint— green on current base; fmt and clippy all targets passed.cargo test day:: --no-fail-fast— 14 passed, 0 failed.column: todo,stale: false, and the store contained exactly one item file. No installed binary or live server was changed.just check-parallel— lint passed; Rust reached 232 passing tests, then stopped on existingagent_start_stops_retrying_when_the_pane_shell_stays_busybecause the ub1 cleanup watchdog receivedPermission denied. The scoped server/API/CLI suite and all-target clippy are green; CI is the full native-runner gate.Decisions the spec did not settle
day-board/items/; the later sync slice can replace the root without changing item shape or API methods.doneandcompleted; canceled work remains visible until explicitly completed.todowith a notice; remote-host liveness remains out of scope until sync.https://claude.ai/code/session_01FYVGxU6SxcYSvJJCNKYbZJ