Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@

`hand` is a CLI that manages a fleet of coding agents from a fleet home (`data/`, `state/`, this file).
This checkout is the tool's own source, not a fleet home itself - there is no `state/hand.db` here, so `internal/home.IsHome` reports false.
`internal/agentsmd`'s `generatedBody` constant is the authoritative template `hand init` writes and `hand update` refreshes into every real fleet home's `AGENTS.md`; see SPECS.md's "AGENTS.md (target)" section for the full design, and `hand --help` for the command reference.
`internal/agentsmd`'s `generatedBody` constant is the authoritative template `hand init` writes and `hand update` refreshes into every real fleet home's `AGENTS.md`; see SPECS.md's "hand doctor" section for the checks that hold it in place, and `hand --help` for the command reference.

## Rules

- SPECS.md carries only the contract a caller can depend on; the reasoning behind a clause lives in a dated record under `docs/adr/`, reached from that section's `Why:` line. `docs/adr/README.md` owns when a record is written and the rule that a landed one is never rewritten to match a later change.
- Comments obey two rules `make lint` enforces through `tools/commentlint`: a comment may not open with the identifier it documents, and a comment block may not exceed three lines. CONTRIBUTING.md's "Comments" section owns the bar for writing one at all, the exemptions, and the reasoning.
- Command output goes through `internal/axi` as TOON and every failure through `cmd/root.go`'s error document; `hand watch`'s event stream is the one exception, and SPECS.md's "Output shape" section owns the contract.
- Harness/herdr syntax, exit-code enforcement, watch's stdout/errOut split, and first-run prompt handling are commented at point of use (`internal/herdr`, `internal/harness`, `cmd/root.go`, `cmd/precondition.go`, `internal/watcher`, `cmd/teardown.go`, `cmd/prdetect.go`, `cmd/merge.go`, `cmd/launch.go`); SPECS.md's "Exit codes" and each command's spec section own the authoritative tables.
Expand Down
4 changes: 3 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,9 @@ Exempt from both rules: the package doc comment, directives (`//go:build`, `//go

Rule 2 will occasionally be wrong, because a genuinely subtle invariant sometimes needs a fourth line.
That is accepted: a rule that is right most of the time and mechanically enforced binds harder than one that is right always and enforced never.
Prose that outgrows three lines belongs in SPECS.md, which is where it is read.
Prose that outgrows three lines belongs in one of two files, and which one depends on what it is.
A rule a caller can depend on or be wrong about goes in SPECS.md.
The reasoning for why a rule is shaped that way goes in a record under docs/adr/, whose README.md owns when one is worth writing and how SPECS.md points at it.

`go run ./tools/commentlint .` runs the check alone and prints one `file:line:column` per violation.

Expand Down
12 changes: 12 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,18 @@ Optional:
- [no-mistakes](https://github.com/yes2games/no-mistakes) - validation pipeline for projects in `no-mistakes` mode
- [qmd](https://github.com/tobi/qmd) - semantic search over historical task data, beyond `hand search`'s keyword matching

`hand` never installs or configures qmd, and every command works without it.
To point it at a fleet home's corpus by hand:

```sh
qmd collection add data/ --name secondhand
qmd context add qmd://secondhand "Task briefs, scout reports, decisions, and backlog history"
qmd embed

qmd search "login auth decision" --json
qmd vsearch "how did we handle the deploy failure" -c secondhand
```

## Installation

From source:
Expand Down
1,127 changes: 442 additions & 685 deletions SPECS.md

Large diffs are not rendered by default.

55 changes: 55 additions & 0 deletions docs/adr/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
# Architecture decision records

`SPECS.md` is the contract: what a caller can depend on, and what a caller can be wrong about.
This directory holds the reasoning that produced it.

Every file here answers one question `SPECS.md` deliberately does not: why is the contract shaped this way, and what was rejected to get there.
A worker who wants to change a contract clause reads the ADR behind it first, because the alternative it names is usually the change they were about to make.

## When to write one

The bar is that a future worker might undo the decision by accident.

A decision nobody could reasonably disagree with does not get an ADR.
A decision whose obvious-looking simplification is the bug it exists to prevent does.

If the reasoning is only "this is what the contract says", it is contract and belongs in `SPECS.md`.
If the reasoning is only "this is how the code happens to be written", it belongs in a code comment or nowhere.

## Format

Each record is one file, `docs/adr/<slug>.md`, with this shape:

```markdown
# <One-line statement of the decision, in the present tense>

- Date: <YYYY-MM-DD>
- Status: accepted | superseded by <slug>.md
- Issues: <fully qualified issue references, none, or none single>
- PRs: <fully qualified PR references, none, or none single>

## Context
## Decision
## Rejected alternatives
## Consequences
```

`none single` is not `none`.
It says the decision accreted across more work than one reference carries, so naming one would credit that one wrongly; `none` says no issue or PR exists.

There is no template to install, no numbering scheme, and no index file that has to be kept correct.
The filename is the slug alone: it is an identifier a `SPECS.md` clause links to and a reader recognizes, and a date prefix on it would be metadata the linker has to remember and the reader has to ignore.
The date lives inside the file, where it belongs to the record rather than to the link.

## An ADR is never edited to match a later change

A record states what was decided on its date, and stays that way even after the decision is reversed.
Reversing one means writing a new record that states the new decision and naming the old one in its context; the old record's `Status` becomes `superseded by <new-slug>.md`, and nothing else in it changes.

Correcting a typo or a broken link is fine.
Rewriting the reasoning is not, because a record that tracks the current design is just the current design written twice, and it loses the only thing it was for.

## How `SPECS.md` points here

A `SPECS.md` section whose shape is not self-evident carries one `Why:` line at its end naming the records behind it.
One line per section, never a link per clause: the contract stays readable as a contract, and the reasoning is one hop away rather than woven back through it.
48 changes: 48 additions & 0 deletions docs/adr/a-fork-projects-upstream-is-declared-never-inferred.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
# A fork project's upstream is declared by a command of its own, never inferred

- Date: 2026-08-05
- Status: accepted
- Issues: atqamz/secondhand#134
- PRs: atqamz/secondhand#135, atqamz/secondhand#142

## Context

`hand` pushes a worker's branch to the repo it cloned.
A fork contribution's PR does not live there: it lives on the repo the work is offered to, which is the repo `hand pr` and gate-opened-PR detection have to look at.

Both compared against the clone's `origin` remote alone, so a genuine upstream PR was refused as belonging to a foreign repo, and teardown's landed-work check read landed work as unlanded.

Widening either comparison is easy and is the whole risk here.
The guard's only job is refusing a PR that belongs to somebody else's repo, so whatever tells `hand` about an upstream decides how narrow the guard stays.

## Decision

A project carries an optional `upstream` slug, and only an operator's declaration puts one there: `hand project upstream <name> <repo>`, cleared by passing an empty repo.
A project that declares nothing is guarded exactly as it was before this existed.

It is a command of its own rather than a flag on `hand project add`, because a fork project is normally already registered by the time the first upstream contribution comes up, and `hand project add` clones - it cannot be re-run against a project that already exists.

What the declared slug then does to PR matching - searched alongside the project's own repo, head refs restricted to the project's repo, every comparison case-folded - is in `an-unrecorded-pr-is-recovered-by-head-ref.md`.

## Rejected alternatives

**`hand project add --upstream <repo>`, with no separate command.**
It looks like the smaller surface and is the change a future worker is most likely to make.
It serves only a project registered after somebody already knew an upstream contribution was coming, and the recovery for every other project is `hand project remove` plus `hand project add`, which re-clones a working repo to record one string.

**Infer the upstream from GitHub's fork parent.**
It removes the command and makes what the guard accepts depend on what GitHub answers at that moment rather than on what an operator declared.
A fork of a fork, a renamed parent, or an unreachable API each move the guard without anybody deciding to.

**Accept any PR whose repo looks related to the project's own.**
There is no resemblance test that admits an upstream and refuses a stranger's repo of the same name.
The narrow version of this is the head-repo filter in `an-unrecorded-pr-is-recovered-by-head-ref.md`, which works precisely because it asks about a branch `hand` pushed rather than about a repo name.

## Consequences

The slug is projected into `data/projects.md`, whose fields are whitespace-separated, so one containing whitespace is refused at declaration time rather than read back truncated later.

An operator who forgets the declaration sees a refusal that names it - the declared upstream, or that none is declared - because "wrong upstream" and "no upstream" are different mistakes with different fixes.

Nothing reconciles the declaration with reality afterwards.
A project whose upstream is stale carries a wrong slug until somebody re-declares it, which is the cost of the guard depending on a statement rather than on a lookup.
70 changes: 70 additions & 0 deletions docs/adr/a-silent-report-channel-is-parked-on-three-bounds.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
# A silent report channel is its own trigger, bounded by three tiers

- Date: 2026-08-04
- Status: accepted
- Issues: atqamz/secondhand#127
- PRs: none single

## Context

`stale` watches herdr transitions.
A pane that registers no transition at all gives it nothing to fire on, and a worker can sit healthy and quiet indefinitely without one.
That is the shape of a wedged worker: the pane is alive, herdr has nothing new to say, and the report channel stopped growing.

So the channel's own silence has to be a trigger of its own.
The question is what bound to measure it against, since the same duration is alarming for one worker and expected for another.

A worker that reported `paused: waiting on the nightly build` has already explained its quiet.
A worker that reported `done` and still holds a pane is finished and unhurried.
A worker that reported `working: refactoring the parser` and then went quiet is the case the trigger exists for.

## Decision

`parked` fires on the report channel not growing for longer than its bound, independently of `stale`.
The bound is chosen by the last classified report line, and there are three tiers with a config key each:

- `paused`: the long bound, `config/parked-paused-bound`, default 3600s.
- `done` and `failed`: their own longer bound, `config/parked-done-bound`, default 5400s.
- everything else, including `working`, `blocked`, `needs-decision` and no report at all: the short bound, `config/parked-other-bound`, default 1200s.

`done` and `failed` are bounded rather than exempt.
What actually severs a task from steering is the status file being torn down, not the worker's own last word about being finished, so a finished worker still attached to a pane is silence like any other.

The trigger is edge-triggered like every other one: it fires once per silence episode and refires only once the report file grows past the mtime it fired for.
That instant is persisted as `parked_fired_for` rather than re-derived; see `the-watcher-persists-what-it-announces.md`.

The event carries the last report line and its age, and nothing else.
A parked worker and a crashed one are indistinguishable from the status file alone, so the process check is left to the caller.

## Rejected alternatives

**Extend `stale` to cover it, rather than adding a trigger.**
`stale` is defined over herdr transitions, and the case here is the absence of one.
Folding the two together means one threshold answering two questions, and the answer is wrong for whichever question it was not tuned for.

**One bound for every state.**
Tuned short it wakes an operator about a `paused` worker that said what it was waiting for, and about a `done` worker whose pane is simply still open.
Tuned long it is silent for the twenty minutes that matter on a `working` worker.

**Two tiers, reusing the `paused` bound for `done` and `failed`.**
It is the tempting simplification, and it makes the most expected silence in the fleet - a finished worker nobody has torn down yet - share a threshold with a worker that is actively waiting on something.
Three keys cost one config file each and let the expected case be quieter than the explained one.

**Exempt `done` and `failed` entirely.**
A `done` report is a claim, not a fact, and a worker that wrongly believes it finished is exactly the worker whose silence needs surfacing.
See `the-report-channel-is-the-only-outcome-signal.md`.

**Derive the latch from the file's current mtime on each tick instead of persisting it.**
A done task's report file never grows again, so every restart re-fires against the same frozen instant and evicts real history from the capped `state/events.log` (atqamz/secondhand#127).

**Have the event report whether the worker's process is alive.**
The status file cannot tell a parked worker from a crashed one, and a check that guesses would make the event's own claim the unreliable part.
`hand status <id>` and the pane itself answer it on demand.

## Consequences

Three config keys exist that an operator can set independently, and a fleet that wants one bound sets all three to the same value.

A worker whose report channel is growing is never parked, however long it has been busy, which is deliberate: activity is the signal, not progress.

Adding a report state means deciding its tier, and the short bound is the safe default for anything unexplained.
73 changes: 73 additions & 0 deletions docs/adr/a-steer-that-never-lands-leaves-a-durable-trace.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
# A busy composer is waited out, and a steer that never lands leaves a durable trace

- Date: 2026-08-04
- Status: accepted
- Issues: atqamz/secondhand#102
- PRs: none single

## Context

`hand send` types a message into a running worker's pane.
The pane is busy whenever the agent is mid-response, which is most of the time a supervisor has something to say.

The first implementation treated a busy composer as an error and returned immediately.
Every caller then wrote the same retry loop in shell, and the loops were all slightly different.
Worse, two of them racing the same pane lost a steer outright: both saw the composer free, both typed, and one message ended up interleaved into the other.

There is also a failure that looks like success.
The text goes into the composer and the submit keystroke fails, so the message is sitting in the pane unsent, and the process that put it there exits.
Nothing anywhere records that a steer was attempted.

## Decision

A busy composer is the normal arrival state, not an error.
`hand send` waits for it, bounded by `--wait` (default `config/send-wait`, else `2m`), because an unbounded wait is a hang and a zero wait is the shell loop coming back.

A per-task `send:<id>` lock serializes senders, so a second `hand send` waits behind the first instead of racing it on the same pane.
`hand watch`'s usage-limit resume takes that lock without waiting, since it has a whole tick to try again and must not stall behind a long steer.

Whenever the message does not demonstrably land - the wait elapses, the text fails to send, or the submit keystroke fails after the text went in - the message and a timestamp are written to the task row.
A steer that never arrived is a thing the operator has to know about, and the process that attempted it is gone.
The three cases are recorded identically because they are the same fact about the world: a steer with no evidence it landed.

They exit differently, though.
The elapsed wait is exit 6 and its own code, because it is transient and a caller can retry with a longer `--wait`.
The two delivery failures are ordinary exit 1.

The trace is cleared by any later send that reaches the pane, whatever message it carried, since a delivered steer moots an abandoned one.
Failing to clear it warns and still succeeds: the message is already in the pane, and failing there invites a retry that double-sends.

The row lock for the trace is separate from the send lock and short-lived.
A `hand send` waiting out two minutes must not block a `hand status` read or a watcher tick on the same task.

## Rejected alternatives

**Keep failing fast on a busy composer.**
It moves the retry into every caller, and unsynchronized retries against one pane were what lost a steer in the first place.

**Wait indefinitely.**
An invocation that never returns is indistinguishable from a hung one, and there is no upper bound on an agent turn.

**Hold the task-row lock for the whole wait, so the trace write needs no second lock.**
A two-minute wait would then block every reader of that task, including the watcher.
The lock is only needed for the write.

**Keep the undelivered message in memory and print it on failure.**
Printing puts it in a transcript nobody re-reads.
The operator finds out about a lost steer from `hand status`, so the trace has to be in the store.

**Give each failure mode its own exit code.**
Only the retryable one changes what a caller does next.
A distinct code for "the submit keystroke failed" is a code nobody branches on.

**Clear the trace only when the same message is re-sent successfully.**
It keeps a stale trace alive after the operator has said something better, and comparing message text to decide whether a concern is resolved is a guess about intent.

## Consequences

`hand send` is slow by default, and that is the intended trade: the caller's alternative was a loop that was slower and raced.

The undelivered trace is a second place a message lives, so it has to be cleared on the success path.
Forgetting that leaves a permanent warning on a healthy task, which is why the clear failure warns instead of failing.

`--file` exists for the same reason the wait does: a multi-paragraph steer through shell quoting is a correctness problem the caller should not have to solve.
Loading