Skip to content
Closed
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
78 changes: 78 additions & 0 deletions plans/20260809-0355-open-issues-sequencing/plan.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
# Plan index — all open issues, 2026-08-09

Six open issues. One plan file each; this file is the sequencing decision across
them. Measured against `origin/main` = `e41a1a6a4`.

| # | Title | Plan | Status |
|---|---|---|---|
| [#109](https://github.com/Nuncio-hq/crew/issues/109) | e2e shard 4 is a dead lane | [plan](../20260809-0400-e2e-shard4-revival/plan.md) | not started — **do first** |
| [#110](https://github.com/Nuncio-hq/crew/issues/110) | Agent question card never renders | [plan](../20260809-0405-channel-question-card/plan.md) | root cause proven, fix pending |
| [#111](https://github.com/Nuncio-hq/crew/issues/111) | Ratchet vs upstream-owned files | [plan](../20260809-0410-file-size-ratchet-upstream-files/plan.md) | deferred behind #109 |
| [#105](https://github.com/Nuncio-hq/crew/issues/105) | Agent attention & recovery | [plan](../20260809-0425-agent-attention-recovery/plan.md) | **in flight — PR #108** |
| [#104](https://github.com/Nuncio-hq/crew/issues/104) | Hermes first-class operations | [plan](../20260809-0420-hermes-first-class-operations/plan.md) | not started |
| [#102](https://github.com/Nuncio-hq/crew/issues/102) | Channel-first missions | [plan](../20260809-0415-channel-first-missions/plan.md) | not started |

## The one decision that orders everything

**Restore the e2e safety net before building more product on top of it.**

`Desktop Smoke E2E (4)` has been `cancelled` at the 30-minute timeout on **6 of 6
consecutive `main` runs** since `b57d26def` (#95). At `e41a1a6a4` it was killed
after **test 106 of 250** — 144 tests never ran. Shard 1 has hard-failed since
`25263120e` (#96).

The three not-started feature issues (#102, #104, #105) all ship runtime behaviour
whose acceptance criteria are explicitly about **replay, reconnect, and restart**.
None of that is provable by unit tests or typecheck. e2e is the only net that has
ever caught the *code-alive-but-unwired* class in this fork — it caught the v0.5.5
sync's only real regression when no gate job did.

Building four features while 58% of a shard reports nothing is the expensive order.

## Recommended sequence

```
1. #109 revive shard 4 ← unblocks honest verification for everything below
2. #110 question card ← independent, small, user-facing bug; root cause already proven
3. #105 finish + merge PR #108 ← already gate-green; owner is mid-work
4. #111 decide the ratchet rule ← cheap decision, prevents next-sync pain
5. #104 Hermes ops, phases 01 → 02 → 04 ← phase 03 waits for #108
6. #102 channel-first missions ← consumes #105's projections
```

## Known couplings — the things that will bite if ignored

- **#110 ↔ #105.** PR #108 modifies `desktop/src/app/useLiveHomeFeedActions.ts`,
the exact file whose app-wide user-input subscription causes #110. Whichever
lands second must re-verify `channels.spec.ts:500` and re-run the bisect.
- **#104 Phase 03 ↔ #105 Slice 2.** Both define the `46040 → 46041/46042`
clarification path. Do not build both; #108 already implements it.
- **#102 Phase 2 ↔ #105.** #102 consumes `needs_input` / `failed` / retry
projections that #105 defines. #105 first, or they get defined twice.
- **#111 ↔ #102.** #102 attaches to `MessageThreadPanel.tsx`, which sits at
999/1000 lines. Decide the ratchet rule before adding a large Crew delta there.

## Standing verification rules for all six

These are repo-specific and have each been learned from a real failure:

- **Mutation check.** Revert the fix line; the suite must go red. A test that
exercises a new helper without proving the production path calls it proves
nothing.
- **Attribute e2e by test name**, never by shard colour. "failed" = lost all 3
attempts; "flaky" = passed on retry. Shard composition shifts whenever spec
files are added, so shard identity is not stable.
- **Never inherit green.** A lane with zero completed runs at the current head is
UNKNOWN, not green.
- **Scope negative claims** to where you actually looked.
- **`git merge-base --is-ancestor` lies here** — the repo squash-merges. Use
`git diff origin/main..<branch>` instead, and `git diff $(git merge-base origin/main <tip>) <tip>`
when the PR is BEHIND.

## Note on these plan files

#102, #104, and #105 already carry complete phase plans inside their issue bodies.
These plan files deliberately **do not copy** that content — duplicated specs drift
apart and the copy silently becomes wrong. Each file covers only what the issue
does not: current status, cross-issue coupling, Crew-specific risk, and sequencing.
The issue remains the spec.
102 changes: 102 additions & 0 deletions plans/20260809-0400-e2e-shard4-revival/plan.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
# Plan — #109 Revive e2e shard 4

Spec: [#109](https://github.com/Nuncio-hq/crew/issues/109)
Status: not started. **Highest priority of the six open issues.**

## Outcome

`Desktop Smoke E2E (4)` completes within its job timeout on `main` and reports a
real pass/fail verdict for all 250 of its tests.

## Why this is first

The lane has been `cancelled` at 30 minutes on **6 of 6 consecutive `main` runs**
since `b57d26def` (#95). At `e41a1a6a4` it was killed after **test 106 of 250** —
**144 tests never executed at all**.

Every other open issue (#102, #104, #105, #111) ships UI into the surfaces this
lane covers. Advisory or not, this lane is the only one that has ever caught a
*code-alive-but-unwired* regression in this fork. Building four features on top
of a dead net is the expensive order.

## Root cause shape (hypothesis, not yet proven)

19 upstream `project-*` tests each fail on a **uniform 31.4–31.6s** locator
timeout, ×3 attempts ≈ 95s each. That arithmetic alone exceeds the job budget.

Four independent spec files failing at an identical duration on their first
assertion points at **one shared entry point** — a testid, route, or project-view
surface that #95 renamed, moved, or removed — not 19 independent breaks.

All timing-out specs are **upstream-owned** (present at `desktop-v0.5.7`).
`project-outcomes.spec.ts`, the only Crew-owned project spec, is **not** failing.
That asymmetry is itself evidence: Crew's own spec was updated with #95, upstream's
were not.

## Phases

### Phase 0 — Prove the shared cause (do not skip)

- [ ] Sibling worktree at `origin/main`; `pnpm build:e2e`.
- [ ] Run `project-inbox.spec.ts:9` alone — smallest failing spec (146 lines).
- [ ] Capture the exact locator that times out.
- [ ] Diff #95's changes against the component that locator targets.
- [ ] Confirm the same locator is the first failing assertion in
`project-commit-detail`, `project-issue-comments`, and `project-pr-review`.

**Gate:** either one shared cause is named with evidence, or the work is
re-planned as N independent breaks. Do not start fixing before this.

### Phase 1 — Decision fork (needs Oscar)

Two branches, materially different cost:

- **#95 broke the surface accidentally** → fix product code. One fix likely
revives the whole lane. Proceed without asking.
- **#95 intentionally redesigned the surface** → choose:
- adapt ~19 upstream spec files → permanent fork delta, conflicts on **every**
upstream sync; or
- skip them + file a Crew-native replacement issue → the #65 precedent, honest
about the coverage loss on the surface Crew changed most.

This is a product call. Post the evidence and a recommendation; do not pick
silently.

### Phase 2 — Land and verify

- [ ] Fix or skip per Phase 1.
- [ ] Shard 4 **completes** (not `cancelled`) at the PR head.
- [ ] Freeze the branch tip long enough for one full 4-shard run — pushes cancel
in-flight runs, which is how this lane reached merge with zero completions
during the v0.5.5 sync.

### Phase 3 — Stop it going dark silently

- [ ] The two fast failures in shard 4 (`overscroll-boundary.spec.ts:34`,
`profile-active-turn.spec.ts:107`) are unrelated to the timeout — triage
separately, do not bundle.
- [ ] Decide whether `Desktop Smoke E2E` should join `gate.needs`. It caught the
v0.5.5 sync's only real regression while no gate job did. Separate PR:
this touches CI config.

## Acceptance criteria

- Shard 4 conclusion is `success` or `failure` — never `cancelled` — on `main`.
- A verdict exists for all 250 shard-4 tests.
- **Raising the job timeout is not an acceptable fix on its own.** The timeout is
a symptom of retry arithmetic; a longer budget still leaves ~19 red tests with
zero coverage and hides the regression for longer.

## Risks

- Shard composition shifts whenever spec files are added, so "shard 4" is not a
stable set. Attribute by **test name** from the per-shard report, never by
shard colour.
- `project-pr-review.spec.ts` is 1383 lines of upstream spec. Adapting it is a
large permanent fork delta — cost it honestly before choosing that branch.

## Evidence

Runs `31263949909`, `31256878409`, `31253637576`, `31188797720`, `31170460252`,
`31156789903`, `31153733457`, `31143687790`, `31068758927`.
Last all-green e2e on `main`: `9f5fe05a1` (2026-08-06 03:33Z).
120 changes: 120 additions & 0 deletions plans/20260809-0405-channel-question-card/plan.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
# Plan — #110 Agent question card never renders in the channel

Spec: [#110](https://github.com/Nuncio-hq/crew/issues/110)
Status: **root cause proven locally**, fix not yet written.
Owner: CTO session (in flight)

## Outcome

An agent elicitation (`KIND_AGENT_USER_INPUT_REQUESTED`, 46040) published into a
channel renders its answer card in that channel, and `channels.spec.ts:500`
passes without any change to the test's assertions.

## Root cause (proven, not inferred)

Bisected locally in an isolated sibling worktree, single test, build verified
green each round:

| tree | result |
|---|---|
| `b57d26def` (#95) | **1 passed** |
| `25263120e` (#96) | **1 failed** — 4/4 runs, deterministic |
| `25263120e` with `desktop/src/app/AppShell.tsx` reverted to `b57d26def` | **1 passed** — 3/3 runs |

The failing change is 14 lines in `AppShell.tsx`, which now passes every channel
id into `useLiveHomeFeedActions`:

```ts
const liveHomeChannelIds = React.useMemo(
() => (channelsQuery.data ?? []).map((c) => c.id),
[channelsQuery.data],
);
useLiveHomeFeedActions(identityQuery.data?.pubkey, refetchHomeFeedFromLiveSignal, liveHomeChannelIds);
```

`useLiveHomeFeedActions` then opens an **app-wide** live subscription per channel
using `buildChannelUserInputFilter(channelId, 50, since)` — the same filter
`useChannelUserInput` uses for the channel card.

The failure mechanism is a **readiness-signal collision in the mock bridge**:

- `e2eBridge.ts:10436` `__BUZZ_E2E_HAS_MOCK_LIVE_SUBSCRIPTION__` returns
`hasMockLiveSubscription(channel.id, kind)` — a plain boolean, true if *any*
subscription matches `(channelId, kind)`.
- The test clicks into `general`, then waits on that predicate before emitting.
- Since #96 the predicate is already satisfied by the **AppShell** subscription,
which exists on every route as soon as channels load.
- So the test emits before `ChannelPane` → `useChannelUserInput` has subscribed.
The mock delivers live events only to subscribers present at emit time, the
channel hook misses it, and `hasCards` stays false. Card never renders.

Confirmed by instrumentation: the emitted event carries only `[["h", channelId]]`
(no `e` tag), and no page errors or exceptions occur. The
`if (!conversationId) return;` early-return in `useChannelUserInput.onEvent`
noted in the issue is **not** the cause — the id falls back to `event.id`, which
is valid, so the guard never trips.

## Blast radius — what this does and does not prove

**Test-visible today.** In production `useChannelUserInput.load()` subscribes and
*then* awaits `fetchEvents(filter)` for history, in that order, so a real relay
would return the request in history and the card would still appear. The mock
bridge does not replay runtime-emitted events into `fetchEvents`, which is why
only e2e sees it.

**Not proven safe.** Two real concerns remain and must not be waved away:

1. Every channel now carries a duplicate app-wide live subscription for the same
filter. That is a deliberate Mission Inbox design choice in #96, not a defect
— but it is unmeasured at realistic channel counts.
2. The mock bridge's live-only delivery hides a genuine ordering assumption. Any
future code path that relies on the live event alone (no history refetch)
would break in production the same way it breaks here.

## Options for the fix

Ranked. **Do not "fix" this by relaxing the assertions in `channels.spec.ts:500`**
— the user-facing property (card renders, is answerable) is correct and must stay
asserted verbatim.

1. **Make the mock bridge store emitted live events so `fetchEvents` replays them.**
Raises fidelity to a real relay (a late subscriber's history fetch returns the
event) and kills this whole race class for every spec, not just this one.
Largest blast radius on shared test infra — must be run against the full smoke
suite, not just this spec.
2. **Give the readiness helper a subscriber-count option** and have the test wait
for the channel-level subscription specifically. Smallest change, but couples
the test to a subscriber count that changes whenever the product adds a
listener — it will rot.
3. **Emit after an explicit ChannelPane readiness marker.** Needs a new marker;
mount is not the same as subscription-established, so this risks re-introducing
the same race in a quieter form.

Recommendation: **(1)**, with the full smoke suite as the gate. Fall back to (2)
only if (1) destabilises other specs.

## Acceptance criteria

- `channels.spec.ts:500` passes, assertions unchanged.
- **Mutation check (required):** revert the fix line(s), re-run, and the test must
go red again. A pass without this proves nothing.
- Full `pnpm test:e2e:smoke` shard 1 completes with no new failures.
- If option (1) is taken, state in the PR which other specs changed behaviour and why.

## Verification commands

```bash
cd desktop
pnpm build:e2e
pnpm exec playwright test --project=smoke tests/e2e/channels.spec.ts \
--grep "channel question card accepts an answer" --retries=0
```

Build in a sibling worktree, never in the shared harness worktree — a concurrent
`pnpm build` silently corrupts a live Playwright run.

## Open questions

- Does option (1) change the meaning of any spec that currently *relies* on
emitted events being invisible to a later `fetchEvents`? Must be checked before
committing to it.
80 changes: 80 additions & 0 deletions plans/20260809-0410-file-size-ratchet-upstream-files/plan.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
# Plan — #111 File-size ratchet vs upstream-owned files

Spec: [#111](https://github.com/Nuncio-hq/crew/issues/111)
Status: not started. **Deliberately deferred behind #109** — see below.

## Outcome

The next upstream sync does not fail `Desktop Fast` on a file whose growth Crew
does not control, and the rule that decides this is written down rather than
re-litigated mid-merge.

## Current state (verified at `origin/main` = `e41a1a6a4`)

```
999 desktop/src/features/messages/ui/MessageThreadPanel.tsx ← 1 line of headroom
986 desktop/src/features/channels/ui/ChannelPane.tsx
972 desktop/src/features/messages/ui/MessageRow.tsx
```

## Why it is deferred, not forgotten

The fix for this issue is an **extraction** — and extraction in exactly this file
is the change class that produced the code-alive-but-unwired regressions during
the v0.5.5 and v0.5.7 syncs. Both were caught by e2e or human review, not by unit
tests or typecheck:

- the v0.5.7 merge silently dropped Crew's existing `message-thread-panel-head.tsx`
extraction, orphaning a live 103-line Crew file;
- the recovery briefly produced two components with the same name and different
prop APIs.

e2e shard 4 — the net for that class — is currently dead (#109). Doing UI surgery
that depends on the net before restoring the net is the wrong order. **Restore
#109 first.**

## The real problem is a rule conflict, not a line count

D-022 says: extract Crew's additions, and **do not restructure upstream's own code
just to pass the guard.** For this file the excess is upstream's — it was 987
lines at `desktop-v0.5.7` and 1043 after the merge. Both halves of D-022 cannot be
satisfied at once. That conflict, not the single line of headroom, is what needs a
decision.

## Options

1. **Pre-emptive extraction of Crew's remaining delta** in this file — done now,
with attention, instead of mid-sync under time pressure.
2. **Treat the ratchet as a true ratchet for upstream-heavy files** — "must not
grow" against a recorded baseline, the way `discovery.rs` (1494/1495) is
already handled — instead of a hard 1000. Requires a change to
`check-file-sizes-core.mjs` plus a `DECISIONS.md` entry.
3. Do nothing; absorb the breakage during the next sync.

Recommendation: **(2)**. A fixed limit on a file whose growth Crew does not
control turns the guard into a tax on syncing, and the guard's purpose is to stop
*Crew* from making files worse. (1) is worth doing as well if the file keeps
growing, but it does not solve the recurrence.

## Constraints

- **Never** raise `MAX_LINES` in `check-file-sizes.mjs` as a blanket escape hatch.
- **Never** add a per-file override to slip under the guard.
- A source-scanning contract test goes blind when code moves to another file. After
any extract: repoint the test, keep every `>= 0` guard, and re-read the invariant
by hand — the guard is off in the interval.
- Option (2) edits CI policy → separate PR, and outside my standing merge authority.
Needs Oscar's explicit approval.

## Acceptance criteria

- Chosen option recorded in `docs/crew/DECISIONS.md` with its reasoning.
- If (2): `check-file-sizes-core.mjs` change is covered by a contract test that
fails when a baseline is raised rather than held.
- If (1): the mutation check applies — the extracted code must be proven still
*called*, not merely still present.

## Dependencies

Blocked on **#109** (e2e shard 4) for option (1). Option (2) is independent and
could land first, since it is a CI-policy change with no runtime behaviour.
Loading