Skip to content

ralph: phase automation — self-reporting status.json + idle nudger + stage UX polish - #216

Merged
agent-era merged 21 commits into
mainfrom
ralph-phase-automation
Apr 21, 2026
Merged

ralph: phase automation — self-reporting status.json + idle nudger + stage UX polish#216
agent-era merged 21 commits into
mainfrom
ralph-phase-automation

Conversation

@agent-era

Copy link
Copy Markdown
Owner

Summary

Adds a ralph-style "is the agent stuck?" nudger for tracker items, plus the on-disk contract that powers it, plus a round of UX polish on the stages/kanban flow that made the new signals more useful.

The agent self-reports a live status. Each tracker item gets a tracker/items/<slug>/status.json with three fields that matter: stage, state: 'working' | 'waiting_for_input' | 'waiting_for_approval', and brief_description (≤ 200 chars). The stage guide tells the agent exactly when to flip state; the kanban reads it to render useful glyphs; ralph reads it to decide whether the agent is stuck or legitimately waiting.

Ralph samples every 60s and nudges when the agent is running but idle (ai_status === 'idle'idleThresholdMs, default 3 min), hasn't advanced stages, and hasn't self-reported a non-working state. Caps at maxNudgesPerStage (default 3) to avoid spam; resets on any stage change or fresh waiting flag. All guards covered by unit tests.

Kanban + main-view surfaces the state. Waiting_for_input cards get a yellow ! with the agent's brief_description; waiting_for_approval cards get a green ✓ Ready — <brief> banner and a dedicated "press [m] to approve and advance" hint when highlighted. The existing m key now does the approve+advance in one keystroke (resets state to working, clears brief, advances stage).

Stage UX cleanup bundled in since the new signals exposed friction:

  • Gate labels name the next step. Was "Gate on advance / Auto-advance / Require approval"; now per-stage "After requirements / Move on / Ask me first", "After implementation / …", "After cleanup and submit / …".
  • "Cleanup" → "Cleanup and submit" in all user-facing strings (kanban column, stage tab, action label, stage-guide title). The on-disk stage key (cleanup) and filename (cleanup.md) are unchanged so saved configs keep working.
  • Dropped the 1..5 numbering from stage filenames — stages/2-discovery.md etc. became stages/discovery.md, # Stage 2: Discovery headings became # Discovery.
  • Removed the vestigial backlog-body generator (it's been merged into discovery for display for a while).
  • Discovery stage body compressed ~180 → ~80 words; requirements knobs cut from 4+1 to 2+1 and body compressed ~400 → ~90 words. Check-in cadence is driven by the project-global inputMode (Style tab) instead of per-stage approval flags.
  • Initial prompt now passes through to codex/gemini via native CLI args (was silently dropped — only claude was wired up).
  • Ralph nudges any running agent, not just attached sessions (attachment ≠ agent-running).

Back-compat. normaliseItemState reads legacy is_waiting_for_user / awaiting_advance_approval booleans and maps them to the new enum, so existing status.json files on disk keep working.

Test plan

  • npx tsc --noEmit — clean
  • npx jest — 672 passing (new: status.json helpers, tri-state parse + legacy back-compat, ralph guard invariants including waiting_for_approval suppression, nudge text polite-check-in assertions, stage-guide generation per setting, moveItem resets state on advance)
  • Manually verify on a live project: ralph nudges only when the agent is running+idle+working; waiting states suppress nudges; [m] approves on a waiting_for_approval card and the next stage starts with a clean brief_description
  • Open the Stages screen: discovery/requirements/implement/cleanup-and-submit tabs all show the new labels; Style tab has inputMode; Ralph tab copy reads correctly
  • Check kanban with fresh status.json in each of the three states — ⟳/!/✓ glyphs and green "Ready — …" banner render as expected

🤖 Generated with Claude Code

promptium-ai and others added 18 commits April 20, 2026 21:50
Foundation for ralph-phase-automation. Agents write a per-item
status.json with {stage, is_waiting_for_user, brief_description,
timestamp} at every meaningful transition, letting ralph distinguish
"idle because stuck" from "idle because waiting for me" in every
input mode, not just ask_questions.

- ItemStatus interface + ITEM_STATUS_STALE_MS constant (24h).
- TrackerService.getItemStatusPath / getItemStatus / writeItemStatus
  / isItemStatusStale with loose schema validation and 120-char
  brief_description clamping.
- Unit tests cover round-trip, missing file, malformed JSON,
  missing required fields, truncation, and the staleness rule.

Also lands the discovery notes and requirements for this item so
reviewers can see the design context.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Each stage guide now appends a common "Agent status protocol"
section telling the agent to keep tracker/items/<slug>/status.json
in sync at every transition (stage start, pause, resume, advance).
The appended block renders stage-specific instructions for three
new settings, exposed via the existing TrackerStagesScreen:

- input_mode (ask_questions | inline | batch | doc_review): picks
  how the agent asks for input during the stage. Non-ask_questions
  modes tell the agent to flip is_waiting_for_user in status.json.
- gate_on_advance (none | review_and_advance | wait_for_approval):
  how the stage transition is gated. Defaults: discovery=none,
  requirements=wait_for_approval, implement=review_and_advance,
  cleanup=wait_for_approval.
- submit (auto | approve) on cleanup only: PR creation gate.

TrackerService gains getItemStage (prefers status.json, falls back
to index.json buckets, defaults to backlog) and listItemsByStage.
moveItem now mirrors the canonical stage into status.json so ralph
and the kanban stay coherent. Existing body generator is kept
private as defaultStageFileBody so only the public wrapper appends
the protocol.

Covered by 26 new unit tests (status.json round-trip, staleness,
stage derivation, moveItem mirroring, protocol rendering for every
mode × gate × stage combination).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The full ralph runtime:

- src/cores/RalphCore.ts samples (ai_status, stage, status.json) per
  active worktree session. Fires stage-aware nudges via TmuxService
  when the agent is idle for >= idleThresholdMs AND the stage hasn't
  changed AND status.json isn't flagged is_waiting_for_user fresh
  AND nudges_this_stage < cap AND ralph is enabled for the project.
  Logs every nudge to logs/ralph.log.

- src/cores/RalphCore.ts also exposes loadRalphConfig/saveRalphConfig
  for the per-project tracker/ralph.json settings.

- TrackerStagesScreen gains a "Ralph" tab with three dropdowns:
  enabled (on/off), idle threshold (1/3/10/30 min), max nudges per
  stage (1/3/5/10). Changes persist immediately.

- WorktreeContext runs RalphCore alongside WorktreeCore and decorates
  each worktree with a .ralph field containing is_waiting_for_user,
  brief_description, nudge count, cap state. WorktreeRow renders a
  compact suffix in the project/feature cell:
  "⏸ <brief>" when waiting, "n:X/Y" when nudged, "!" when capped.

- 18 new unit tests cover every safety invariant (no nudge on
  waiting/working/fresh-flag/disabled/missing-session), detection
  (fires on sustained idle + unchanged stage), counter increments,
  cap behaviour, resets on stage change and on fresh waiting flip,
  stale-flag ignore, nudge text content, and log-entry format.

Full suite: 664 tests pass, typecheck clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a comprehensive end-to-end ralph test (tests/unit/ralph.test.ts)
covering the full agent lifecycle from acceptance criteria §38:
waiting → clear → nudge → cap → advance → reset. Verifies the
sequence of nudge firing, suppression while waiting, cap enforcement,
and counter/cap reset on stage advancement.

Also writes implementation.md with what was built, key design
decisions (status.json vs MCP, canonical stage choice, agent-owned
flag lifecycle), and notes for the cleanup stage.

Full suite: 665 tests pass, typecheck clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
input_mode is a personal preference for how the agent delivers
questions (ask_questions tool vs inline chat vs batched vs doc
review) — not something that should change per stage. Moved out
of COMMON_STAGE_OPTIONS (so stage tabs now only carry
gate_on_advance) and into WorkStyle.inputMode, surfaced as a new
row on the Style tab.

- New InputModeStyle type + inputMode field on WorkStyle with
  default 'ask_questions'. loadWorkStyle already splats defaults,
  so existing work-style.json files pick up the new field on read.
- defaultStageFileContent and renderStageProtocol take an optional
  WorkStyle third argument and read inputMode from it.
- TrackerStagesScreen: preview + cycleStageOption pass the live
  workStyle. Cycling the new inputMode row on the Style tab
  regenerates all four stage guide files on disk so the protocol
  tail reflects the change immediately.
- generateWorkStyleFileContent renders an "Input mode" row in the
  working-style.md output and drops the now-redundant "use the
  ask_questions tool" footer.
- RalphCore.buildNudgeText now gets inputMode from WorkStyle, not
  from stage settings.
- Tests updated to pass WorkStyle where they used to set
  input_mode in stage settings.

Full suite: 665 tests pass, typecheck clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Writing a stage-review is a natural thing to do whenever the stage
did real work — not a separate gate mode. Merge review_and_advance
into auto_advance and drop it as a user-visible option.

gate_on_advance now has two values: auto_advance (default for
discovery + implement) or require_approval (default for
requirements + cleanup). The auto_advance instruction tells the
agent to append a short "## Stage review" section to the stage's
output file whenever the stage produced meaningful findings /
decisions / changes, and to skip it for trivial no-op stages.
Review authorship is implicit, not a separate toggle.

Legacy gate values ('none', 'review_and_advance', 'wait_for_approval')
in saved settings are mapped on read so existing projects don't
break. 'none' and 'review_and_advance' collapse to auto_advance;
'wait_for_approval' maps to require_approval. Covered by a test.

Full suite: 666 tests pass, typecheck clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The reminder now reads like a colleague checking in, not a system
notification. The agent sees no "ralph" or bracketed system label
at all — we want it read as conversation so the agent engages with
it the same way it would with a human message.

The rewritten text also explicitly reminds the agent of two things
every time:

- Update status.json with the current stage and either
  is_waiting_for_user: true + brief_description (if blocked on the
  user) or false + a short working-on note (if still going).
- Flipping is_waiting_for_user: true stops these check-ins from
  firing — so the agent knows the escape hatch.

New tests lock in: no "ralph" token in agent-facing text, the
reminder mentions is_waiting_for_user and brief_description, and
the message doesn't open with a system-style bracket prefix.

Full ralph suite: 22 tests pass, typecheck clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
TrackerBoardScreen now reads each item's status.json per card and
treats a fresh is_waiting_for_user: true as equivalent to the tmux
'waiting' state for rendering — same yellow glyph, same bold
treatment. The agent's brief_description replaces the generic
"waiting for you" label when present, so the kanban answers "what
is this one waiting on?" at a glance.

When not waiting, if the agent reported a brief_description (e.g.,
"drafting section 2", "writing RalphCore"), we show that instead
of "running" / "session idle" so idle agents with known activity
aren't flattened to the same label.

Stale status.json records (> 24h, per TrackerService's rule) are
ignored, matching ralph's own suppression semantics — so a crashed
agent whose flag never got cleared doesn't permanently mark the
card as "waiting".

Full suite: 669 tests pass, typecheck clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
A single truncated line was too tight for the agent's brief_description —
things like "drafting section 2 of requirements.md" got clipped mid-word.
Each card now wraps the secondary text onto up to 2 lines, breaking on
spaces when possible and appending '…' when content overflows.

ROWS_PER_ITEM goes from 3 to 4 so the scroll math reserves a fixed slot
for the second line. Short descriptions leave the second line blank —
trading a bit of whitespace for predictable layout and no overlap when
multiple items render in the same column at different lengths.

Full suite: 669 tests pass, typecheck clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two related improvements driven by the user ask:

1. "Stage done, waiting for approval to advance" is now a distinct
   status.json state, not just generic is_waiting_for_user:

   - New optional field awaiting_advance_approval: boolean on
     ItemStatus. When true, the agent's work for the stage is done
     and they're specifically waiting on the user's sign-off to
     advance (relevant only under gate_on_advance = require_approval).
   - Ralph suppresses nudges on this flag the same way it suppresses
     on is_waiting_for_user. Covered by a new unit test.
   - TrackerBoardScreen renders this state in green with a ✓ glyph
     and a bold "READY TO ADVANCE — <brief>" secondary line, so the
     human spots items ready for sign-off at a glance and never
     confuses them with stuck-mid-stage waits (yellow "!").
   - Stage protocol instructs the agent to set both
     is_waiting_for_user AND awaiting_advance_approval when the
     require_approval gate kicks in at stage end, and flip both
     back to false on the advance.

2. brief_description should be about substance, not the stage:

   - Stage column already tells the user what stage an item is in,
     so "in requirements" / "doing discovery" is dead weight. The
     protocol now explicitly calls out what brief_description is
     for (the concrete work or blocker) with good/bad examples,
     and the nudge check-in echoes the same framing.

Full suite: 673 tests pass, typecheck clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Discovery had 4 knobs (skip / depth / web_search / questions) + the
common gate = 5 rows, giving 243 combinations most of which made no
sense. The real axes were "how hard do I research?" and "how do I
close out?". Dropped the other three into baked-in behaviours.

Settings (2 rows):
- effort: skim | standard | deep — research scope only (code + web).
  Not about interrupting the user.
- report: just_advance | confirm_if_notable | always_confirm — how
  discovery closes out. Supersedes the common gate_on_advance for
  this stage because `confirm_if_notable` (the middle value) is
  what the two-value gate can't express.

Baked-in (not tunable):
- Trivial items (typo / rename / doc tweak): write one-line notes.md
  and advance regardless of effort.
- Clarifying questions are rare and narrow: only when the problem
  statement itself is too vague to interpret. "Should it be X or Y?"
  is requirements-work, not discovery.

Also compressed the stage body. Research/report steps are short
mode-specific sentences; output-fields list uses minimal labels
(Problem / Findings / Recommendation) instead of long prose. The
generated discovery guide goes from ~650 words to ~180 — lower
context cost per agent re-read, same high-entropy content.

Legacy settings (skip/depth/web_search/questions) are ignored on
read — existing projects just fall back to defaults rather than
breaking. Stale tests updated to the new keys.

Full suite: 674 tests pass, typecheck clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Claude has been piping the initial prompt in through `claude "prompt"` for
a while, but the else-branch for codex/gemini just ran the resume command
and dropped the prompt on the floor. Wire each tool's native flag into a
claude-style `resume || fresh` fallback so the prompt lands regardless of
whether there's a prior session to resume.

- codex: `codex resume --last ... "prompt" || codex ... "prompt"` — with
  --last filling the [SESSION_ID] slot, the trailing positional maps to
  [PROMPT].
- gemini: `gemini --resume latest ... -i "prompt" || gemini ... -i
  "prompt"` — -i/--prompt-interactive runs the prompt then stays
  interactive.

Plain launches (no initial prompt) are unchanged to keep the
auto-resume test's expected command intact.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous guard required \`wt.session?.attached === true\`, which
conflates "user is viewing the tmux pane" with "agent is running". A
backgrounded tmux session still runs the agent, tmux capture-pane works
regardless of attachment, and send-keys delivers into detached sessions
fine. The authoritative "agent is running and ready" signal is
\`ai_status === 'idle'\`, which getAIStatus already returns as
'not_running' whenever no tool process is detected — so plain shells and
dead sessions can't slip through.

Flipped the corresponding test: a backgrounded session with
\`ai_status: 'idle'\` now gets nudged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Dropped the duplicated header/skip-rules block, inlined the output
fields as a single sentence, and tightened the research + report steps.
The body goes from ~180 words to ~80 while keeping every test assertion
(effort-specific keywords, Options in deep only, Trivial/Clarifying
rules, report-mode phrasing) intact.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Requirements had 4 per-stage settings (style × 3, detail × 3, approval
× 3, user_stories × 3) plus the common gate_on_advance — too many
dials, and the overlap with the project-global inputMode meant users
tuned the same thing in two places.

Reduce to 2 knobs:
- style: interview | draft_first (dropped freeform — fuzzy, rarely
  useful; check-in cadence comes from inputMode anyway).
- detail: minimal | standard | thorough (unchanged — legitimate scope
  dial).

Dropped as settings:
- approval (per_section | end_only | none) — already covered by the
  project-global inputMode on the Style tab. `doc_review` ≈ end-only,
  `ask_questions` ≈ per-section check-ins, etc.
- user_stories (skip | include | lead) — niche formatting preference;
  belongs in the spec body when the item is user-facing, not as a flag.

Body compressed from ~400 words to ~90 (same treatment as discovery):
dropped the Steps/Output/Advancing section headers, inlined the output
sections as a single ordered list, and let the protocol tail handle
stage advancement via status.json (so the index.json reference here
became redundant — two tests that asserted it now cover implement +
cleanup only).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…approve

Polishes a handful of UX rough edges the stages/kanban flow accumulated:

**Gate labels clarify what "advance" actually does.** Replaced the common
"Gate on advance / Auto-advance / Require approval" with per-stage labels
that name the next step: "After requirements / Move on · Ask me first",
"After implementation / …", "After cleanup and submit / …". Options stay
terse (labels carry the stage context) so the settings table isn't
redundant.

**Cleanup stage is now "Cleanup and submit"** in all user-facing strings
(kanban column, stage tab, `actionLabel`, stage-guide title). The stage
key on disk (`cleanup`) and file name (`cleanup.md`) are unchanged so
saved configs keep working.

**Dropped the 1..5 stage-file numbering.** `stages/2-discovery.md` etc.
become `stages/discovery.md`, `0-overview.md` becomes `overview.md`,
and the `# Stage 2: Discovery` headings become `# Discovery`. The
numbers were duplicating STAGE_ORDER and making rename/reorder painful.
Removed STAGE_FILE_NUMBERS and the parallel stageFileNumber() helper.

**`backlog` stage no longer gets a generated guide file.** It's been
merged into discovery for display for a while; the body generator was
pure tech debt. Narrowed `defaultStageFileContent` / `defaultStageFileBody`
/ `getStageFilePath` to `Exclude<TrackerStage, 'archive' | 'backlog'>`
so the compiler blocks accidental calls.

**`ItemStatus` collapses two overlapping booleans into one tri-state.**
`is_waiting_for_user` + `awaiting_advance_approval` → `state: 'working' |
'waiting_for_input' | 'waiting_for_approval'`. The state is what the
kanban cares about (green ✓ READY TO ADVANCE for approval, yellow ! for
input); ralph treats both waiting states as nudge-suppressors. Parser
still accepts the legacy boolean schema so on-disk files keep reading.

**One-key approve from the kanban.** A `waiting_for_approval` card shows
a "· [m] approve" hint when selected; the existing `m` key (advance to
next stage) already does the right thing now that `moveItem` resets
`state` to `'working'` and clears `brief_description`.

Tests: 672 passing (was 675 — removed old backlog-body tests, added
3 new ones covering the tri-state enum, legacy-boolean back-compat,
and waiting_for_approval nudge suppression).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Follow-up from the simplify review on the tri-state migration, plus
a few UX tweaks from reviewing the board in-person:

**WorktreeInfo.ralph.state** now carries the tri-state end-to-end
(was re-collapsed to a boolean), so WorktreeRow in the main view
can render waiting_for_input (⏸) vs waiting_for_approval (✓) distinctly
instead of showing the same chip for both.

**Extracted `isItemWaiting` / `isItemReadyToAdvance`** helpers on
TrackerService — collapses three copies of the same "fresh + non-working
state" check (RalphCore, WorktreeContext, TrackerBoardScreen).

**Fixed stale `is_waiting_for_user` prose** in three live user-facing
spots that still told agents to write the old boolean schema:
- INPUT_MODE_LABELS (renders into `working-style.md`)
- Ralph tab help text
- `isItemStatusStale` + `nudgesThisStage` doc comments

**Deleted the dead `COLUMN_TITLES` alias** — with "Cleanup" already
spelled "Cleanup and submit" in STAGE_LABELS, the override was a
pure pass-through. Board column now reads STAGE_LABELS directly.

**Prominent approve hint on the kanban.** The trailing `· [m] approve`
suffix was hard to spot — moved it to a dedicated `press [m] to approve
and advance` line under the selected ready-to-advance card. Shortened
the green banner from `READY TO ADVANCE — …` to `Ready — …`.

**`brief_description` limit 120 → 200 chars.** The focused card now
shows up to 4 wrapped lines (non-focused cards stay at 2) so the full
description fits without truncation. Updated the protocol schema doc
and the corresponding test.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Apr 21, 2026

Copy link
Copy Markdown

Code Review — PR #216: Ralph phase automation

Overview

This PR introduces three interconnected features:

  1. RalphCore — a 60s sampling loop that nudges idle agents by sending text into their tmux session when they appear stuck.
  2. Agent self-reporting status.json — a new ItemStatus schema for agents to declare their live state (working / waiting_for_input / waiting_for_approval), which ralph reads to suppress false-positive nudges.
  3. Stage + UX polish — cleaner stage labels, numbered filename prefix removal, kanban glyph trifecta (⟳ / ! / ✓), and new inputMode project-global setting.

Overall the architecture is solid and follows project conventions well. RalphCore correctly implements CoreBase<T>, back-compat for the legacy boolean schema is well handled, and the unit tests are thorough. A few things are worth addressing before merge.


Bugs / Correctness

1. getItemStatus doesn't validate stage against the enum whitelist (src/services/TrackerService.ts)

return {
  stage: parsed.stage as Exclude<TrackerStage, 'archive'>,  // ← unsafe cast
  ...
};

Only typeof parsed.stage !== 'string' is checked, but parsed.stage is never validated to be one of the known stage values. A status.json written with stage: "../../evil" would be returned without error and could reach getStageFilePath, which now uses path.join(..., ${stage}.md). Add a whitelist check:

const VALID_STAGES = new Set(['backlog','discovery','requirements','implement','cleanup']);
if (!VALID_STAGES.has(parsed.stage as string)) return null;

2. Silent breakage for existing installations — stage file rename (src/services/TrackerService.ts)

getStageFilePath now returns ${stage}.md instead of ${n}-${stage}.md. Any project that already has the numbered files on disk (2-discovery.md etc.) will silently see empty stage guides because readStageFile returns '' when the file doesn't exist. There's no migration. Consider a fallback read:

getStageFilePath(projectPath, stage) {
  const newPath = path.join(dir, `${stage}.md`);
  const oldPath = path.join(dir, `${this.STAGE_FILE_NUMBERS[stage]}-${stage}.md`);
  return fs.existsSync(newPath) ? newPath : oldPath; // migrate lazily on write
}

Or at minimum document in the PR that ensureStageFiles (or a migration helper) must be run on upgrade.

3. Unreachable-looking but reachable branch in sampleOnce (src/cores/RalphCore.ts, line ~315)

} else if (!this.state.worktrees[key]) {
  this.state.worktrees[key] = cur;
  changed = true;
}

This branch fires for new worktrees where getProjectPath returns '' (causing sampleWorktree to return prev unchanged). The path is reachable but the intent isn't obvious — it silently registers blank state for worktrees ralph can't locate. A comment clarifying this is needed; otherwise the next reader will almost certainly delete it as dead code.


Performance

4. discoverProjects() + loadRalphConfig() called per-render in worktreesWithRalph (src/contexts/WorktreeContext.tsx)

const worktreesWithRalph = React.useMemo(() => {
  const projects = gitRef.current.discoverProjects();   // ← fs scan
  ...
  return state.worktrees.map(wt => {
    ...
    const cfg = loadRalphConfig(projectPath);           // ← fs.readFileSync per worktree
    ...
  });
}, [state.worktrees, ralphState]);

discoverProjects() scans the filesystem on every state.worktrees or ralphState change — WorktreeCore refreshes every few seconds. loadRalphConfig adds a readFileSync per unique project per refresh. Consider deduplicating within the memo (one loadRalphConfig call per unique projectPath, not per worktree) and possibly caching discoverProjects output in a ref that refreshes on a slower interval.

5. getItemStatus called per item per kanban render (src/screens/TrackerBoardScreen.tsx, line ~595)

const itemStatus = service.getItemStatus(projectPath, item.slug);

This is a synchronous readFileSync inside the per-item render loop. With 20+ items visible it's N file reads per frame. This was fine for the old boolean-only signal, but now it's the primary gate for three rendering paths. Moving this read into a pre-render memo or a periodic refresh would help.


Minor Issues

6. buildNudgeText maps both implement and cleanup to implementation.md (src/cores/RalphCore.ts, line ~237)

The cleanup stage probably has a more appropriate output description ("PR description" or "final review"), but it currently says implementation.md. Low priority, but worth aligning with the stage guide.

7. wrapToLines edge case: single-word text wider than width

let breakAt = remaining.lastIndexOf(' ', width);
if (breakAt <= 0) breakAt = width;

When breakAt === 0 (space is at index 0), the code falls through to breakAt = width which hard-cuts. If remaining is a long word with a leading space that's within width, it gets incorrectly cut. The condition should be breakAt < 1 or breakAt <= 0 is fine — but remaining.slice(0, 0) would push an empty string. A test for a single-word string wider than the column would catch this.

8. Missing stage validation in getItemStage

getItemStage prefers status.json even when the timestamp is stale. This is inconsistent with isItemWaiting (which ignores stale records). A crashed agent with a 48-hour-old status.json at requirements would show as being in requirements forever in the kanban stage derivation, while nudges are correctly suppressed. This inconsistency is probably intentional (stage doesn't auto-revert) but worth a comment confirming it.


Test Coverage

The ralph.test.ts and tracker status.json suites are thorough — the full state-machine flow test (§ waiting → clear → nudge → cap → advance → reset) is particularly good. A few gaps:

  • wrapToLines has no tests; the edge case noted above (Add execute session feature for running programs in worktrees #7) could silently truncate agent descriptions in production.
  • worktreesWithRalph decoration in WorktreeContext is untested; specifically that loadRalphConfig fallback and the legacy is_waiting_for_user mapping reach the rendered chip correctly.
  • launchAISessionWithFallback for the else branch (unknown tool) has no test coverage.

Summary

Architecture ✅ Follows CoreBase pattern, clean separation
Back-compat ✅ Legacy boolean schema handled, normaliseItemState is clean
Test coverage ✅ Strong for new core; gaps in context decoration and wrap util
Stage file migration ⚠️ Silent breakage risk for existing installations
Input validation ⚠️ stage field not whitelist-validated before use in file paths
Performance ⚠️ Repeated I/O in render-path memos
Code clarity ⚠️ One confusing branch in sampleOnce needs a comment

The nudger logic itself is well-reasoned and the guard conditions (disabled, working, waiting, capped, archive, no session) are all correctly tested. The stage validation and migration issues are the most important to resolve before this lands.

🤖 Generated with Claude Code

- Whitelist `stage` against STAGE_ORDER in `getItemStatus` before it flows
  to `getStageFilePath` — a crafted status.json with `stage: "../evil"`
  could have reached path.join otherwise.
- Dedupe `loadRalphConfig` in `worktreesWithRalph`: was readFileSync per
  worktree per refresh, now one read per unique project per refresh.
- Clarify the "register blank state" branch in `RalphCore.sampleOnce` with
  a why-comment so it doesn't read as dead code.
- `buildNudgeText` had cleanup mapped to `implementation.md` as its
  "output" — but cleanup polishes the impl in place and drafts the PR,
  there's no single output file. Say "the cleanup checklist (tests, review,
  PR prep)" instead.
- Test: verify getItemStatus rejects stages outside the enum (including
  `../../evil`, `archive`, unknown, empty).

Not addressed (out of scope / pre-existing):
- `getItemStatus` readFileSync in kanban render loop — pre-existing hot
  path, separate project to cache tracker status at the item level.
- Stage-file rename back-compat (old `2-discovery.md` files sit orphaned).
  `ensureStageFiles` regenerates the new-named file on first access, which
  is the robust path; an explicit migration isn't worth the complexity.
- `wrapToLines` single-wide-word edge case — pre-existing utility.
- `getItemStage` ignoring staleness for stage derivation — intentional
  (stage doesn't auto-revert on crash; only the waiting signal does).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Apr 21, 2026

Copy link
Copy Markdown

Code Review — PR #216: ralph phase automation

Overview

This PR ships three interlocking pieces: (1) an agent-owned status.json self-report file so the harness can distinguish "idle because stuck" from "idle because waiting on the user", (2) a RalphCore sampling loop that sends stage-aware nudges when an agent stalls, and (3) a round of stage UX polish (label cleanup, gate-on-advance dropdowns, input-mode tab). The overall design is sound and well-motivated.


Code Quality and Architecture

Positives

  • RalphCore correctly implements the CoreBase<T> contract and is fully decoupled from React, matching the project's layer separation.
  • The renderStageProtocol / defaultStageFileBody split is clean — adding a new setting in the future needs one edit in the tail generator, not five case blocks.
  • Security is handled well: getItemStatus validates the stage field against STAGE_ORDER before using it in file paths (path-traversal guard), brief_description is clamped to 200 chars, and shellQuote is applied to user-provided initial prompts in launchAISessionWithFallback.
  • Legacy back-compat in normaliseItemState correctly maps the old boolean schema to the new enum.
  • The nudge text test asserting text.toLowerCase().not.toContain('ralph') is a nice UX invariant.

Potential Bugs

1. Dead guard in sampleWorktree (unreachable code)

The second stage-change check near the end of sampleWorktree is unreachable:

if (prev.lastStage !== null && prev.lastStage !== stage) return next;

If the stage changed, stageChanged is true, which sets idleSince = null. The earlier guard if (idleSince === null || nowMs - idleSince < config.idleThresholdMs) return next always fires first. This should be removed to avoid misleading future readers.

2. ralphRef initialization outside useRef initial value (WorktreeContext.tsx)

const ralphRef = React.useRef<RalphCore | null>(null);
if (!ralphRef.current) {
  ralphRef.current = new RalphCore({...});   // allocates services on every render
}

New service instances (TrackerService, TmuxService, GitService) are allocated on every render and then discarded when the guard short-circuits. The established pattern (see WorktreeCore init) is to set the initial value in the useRef call itself:

const ralphRef = React.useRef(new RalphCore({
  tracker: new TrackerService(),
  tmux: new TmuxService(),
  ...
}));

3. ROWS_PER_ITEM vs selected-card line count mismatch (TrackerBoardScreen)

The comment says "up to 4 rows: slug + 2 secondary lines + marginBottom" and ROWS_PER_ITEM = 4. But for a selected card, maxLines = isSelected ? 4 : SECONDARY_MAX_LINES — that is 4 secondary lines plus the slug row plus the "[m] approve" hint row — potentially 6 visual rows. A selected card with a long brief_description can overflow its reserved slot and push items below it out of frame.


Performance Considerations

4. Synchronous file reads in useMemo (hot path)

worktreesWithRalph runs on every state.worktrees or ralphState change (every 2s AI status poll):

const projects = gitRef.current.discoverProjects();                          // filesystem scan
const status = trackerRef.current.getItemStatus(projectPath, wt.feature);   // readFileSync per worktree

With 10 worktrees this is 10+ synchronous I/O calls every 2 seconds, all on the render thread. Consider moving these reads into RalphCore.sampleOnce (which already runs on its own 60s interval) and exposing the results as part of RalphState, so the memo only does an in-memory lookup.

5. loadRalphConfig called per-worktree in sampleWorktree

Multiple worktrees in the same project re-read tracker/ralph.json on each sampling pass. The worktreesWithRalph memo already dedupes this read per project; sampleOnce does not. Easy fix: build a project-keyed config map at the top of sampleOnce, same pattern the memo uses.


Breaking Change — Stage File Renaming

The rename from stages/2-discovery.md to stages/discovery.md is a silent breaking change for any project that already has numbered stage files on disk. getStageFilePath now returns the unnumbered path; old numbered files will never be found, and the next ensureStageFiles call will silently regenerate them from defaults — overwriting any user edits. A one-time migration helper, or at least a startup check and warning, would protect existing users.


Test Coverage

Strengths

  • Safety invariants are thoroughly covered: disabled ralph, all ai_status values, both waiting states, staleness, cap, reset-on-stage-change.
  • The protocol-rendering tests (per gate x mode x stage) are a solid regression net.
  • The full fake-agent end-to-end flow test in ralph.test.ts is readable and covers the complete lifecycle.

Gaps

  • No UI-level test for the ralph chip (acknowledged in the PR). The WorktreeRow suffix logic (ralphSuffix) and the worktreesWithRalph decoration are untested at the component level. A mock-rendered E2E test seeding a worktree with a waiting_for_approval status and asserting the suffix renders would close this.
  • wrapToLines has no unit tests. It has edge cases (word longer than width, empty string, exactly-full line) that are quick to cover and would protect against regressions.
  • CapturingTmuxService uses as any cast. If RalphCore accepted an interface for its tmux dependency, tests could avoid the cast entirely.

Minor / Style

  • Comments in the WorktreeContext.tsx ralphRef block are quite long (~6 lines). AGENTS.md asks for no comments unless the "why" is non-obvious; these explain what rather than why.
  • The ALL_TABS ternary checking tab === 'ralph' appears in three render locations; a small tabLabel(tab) helper would deduplicate it.

Summary

The core design (self-reported status.json + sampling loop + stage-aware nudges) is solid and well-tested. Main items to address before merging:

  1. Bug: remove the unreachable stage-change guard near the bottom of sampleWorktree.
  2. Correctness: fix the ralphRef initialization pattern (move construction into useRef initial value).
  3. Performance: move getItemStatus / discoverProjects reads out of the render-path useMemo and into RalphCore's state.
  4. Breaking change: add a migration path for existing numbered stage files, or at least a warning.
  5. Overflow: audit the selected-card row budget (ROWS_PER_ITEM = 4 vs actual max line count when the approval hint line is visible).

Test coverage is strong; closing the UI chip test and the wrapToLines unit tests would make this essentially complete.

- Remove the unreachable stage-change guard near the bottom of
  `sampleWorktree` — a stage change already zeroes `idleSince`, which the
  earlier `idleSince` guard catches, so the second check was dead code.
- Dedupe `loadRalphConfig` in `sampleOnce`: N worktrees in the same project
  were causing N readFileSync calls per 60s tick. Cache per project.

Skipped (not actually bugs / out of scope):
- `ralphRef` init — guarded lazy init is correct and cheaper than
  `useRef(new X())` (the latter also evaluates once per render, just
  discards the result).
- Selected-card row overflow vs `ROWS_PER_ITEM = 4`: overflow happens for
  one card and is acceptable UX for the "show the full 200-char
  description" feature. Cards below shift by at most a row.
- Sync `getItemStatus` / `discoverProjects` in the kanban render memo —
  pre-existing, belongs in its own project to cache tracker state at
  the item level.
- Stage file renaming "migration": `ensureStageFiles` only writes when
  the new path is missing, so user edits to orphaned numbered files
  survive on disk (they're just unreferenced). No data loss.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Apr 21, 2026

Copy link
Copy Markdown

Code Review — PR #216: ralph phase automation

Overview

This is a substantial, well-structured PR. The on-disk status.json contract is a clean design: the agent self-reports state, ralph reads it to decide whether to nudge, and the kanban renders it. The separation between RalphCore (business logic), TrackerService (I/O helpers), and the React layer is solid and follows project conventions.


Bugs / Correctness Issues

listItemsByStage comment contradicts implementation

The method says "Also surface slugs that have a status.json but aren't in the index yet", but the loop only iterates knownSlugs — which were derived from the index. Slugs with a status.json but absent from index.json are never surfaced. Either the comment is wrong (remove it) or the loop should also scan the tracker/items/ directory.

// current — only overrides index-known slugs; never adds new ones
for (const slug of knownSlugs) {
  const status = this.getItemStatus(projectPath, slug);
  if (status) out.set(slug, status.stage);
}

getItemStage trusts status.json.stage unconditionally

getItemStage returns status.stage as the canonical value with no freshness check — the 24 h staleness guard only applies in isItemWaiting, not here. A crashed agent that wrote the wrong stage and then died would permanently shadow index.json. Consider either applying the same staleness check here, or using status.json only for the state field and index.json as the authoritative source for stage placement.


Performance Concerns

Sync filesystem I/O inside useMemo

worktreesWithRalph in WorktreeContext.tsx calls gitRef.current.discoverProjects() (filesystem walk) and trackerRef.current.getItemStatus() (per-item readFileSync) on every state change from either state.worktrees or ralphState. For a project with many tracker items this could stall the render loop.

The config-dedup pattern (ralphConfigByProject) shows the cost is on your radar — the same treatment should apply to discoverProjects(). Ideally, project discovery moves into WorktreeCore state and is passed down rather than re-discovered in the render path.


Code Quality / Clarity

ROWS_PER_ITEM = 4 reserves empty rows for unselected items

Increasing the slot size means all cards reserve 4 rows even when their secondary text fits in one. The focused-card expansion (maxLines = isSelected ? 4 : SECONDARY_MAX_LINES) limits line output correctly, but the layout slot is still 4 rows for every item. Worth validating against a column with 8+ items to ensure visible item count doesn't regress.

Stage file renaming has no automatic migration

Dropping the numeric prefix (2-discovery.mddiscovery.md) is a clean change, but ensureStageFiles / ensureTracker aren't called on app start — only when creating a new tracker. Existing projects will silently fall back to defaults when the old-named files are present. A one-time migration helper or upgrade note would help users with customised stage files.


Security

The path-traversal guard in getItemStatus (whitelisting stage against STAGE_ORDER before using it in file paths) is explicit and correct. The brief_description truncation at 200 chars in writeItemStatus is also good.


Test Coverage

The 464-line ralph.test.ts is thorough: idle threshold, stage resets, waiting-state suppression, cap semantics, nudge text assertions. Two gaps worth addressing:

  1. listItemsByStage — no test for the "slug has status.json but is absent from index" path (which is currently broken per the bug above).
  2. worktreesWithRalph decoration — the memo logic in WorktreeContext has no test. An integration test that writes a status.json and verifies the ralph field on the returned worktree would catch future regressions.

Summary

The core design is sound and the tri-state nudge protocol is a genuine improvement. The two correctness issues (listItemsByStage bug and getItemStage unconditional stage trust) are the highest-priority fixes before merge. The sync I/O in useMemo is worth a follow-up if it doesn't block this PR.

- `listItemsByStage`: the "also surface slugs absent from the index"
  clause in the comment was never actually implemented — the loop only
  iterates index-known slugs. Rewrote the comment to describe what the
  code does; callers needing orphan-detection can scan the items dir.
- `getItemStage`: added a comment explaining why the staleness guard
  (used elsewhere for ralph suppression) deliberately doesn't apply
  here. The last stage the agent wrote is still the correct stage even
  if the process later crashed — nothing auto-reverts, so no staleness
  check is appropriate.

Skipping the sync-I/O-in-useMemo concern (pre-existing, separate
project) and the ROWS_PER_ITEM=4 interaction with the 4-line selected
card (acceptable UX trade-off for showing the full 200-char
brief_description).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Apr 21, 2026

Copy link
Copy Markdown

Code Review — PR #216: Ralph phase automation + status.json + stage UX polish

Overview

This is a substantial, well-scoped PR that adds three related features: (1) an on-disk agent self-reporting contract (status.json), (2) the Ralph idle-nudger engine (RalphCore), and (3) a round of kanban/stage UX polish that surfaces the new signals. The PR description is excellent — it accurately describes all changes and the back-compat strategy.


✅ Strengths

  • Architecture is clean: RalphCore correctly follows the CoreBase<T> pattern (subscribe/get/start/stop), mirrors WorktreeCore, and keeps business logic out of React.
  • Testability is well-considered: now and logger injection points make the core trivially testable without fake timers.
  • Back-compat is explicit: normaliseItemState mapping from legacy booleans is a nice touch and reduces migration risk.
  • Config deduplication: Both sampleOnce and worktreesWithRalph cache per-project loadRalphConfig calls to avoid N × file reads per refresh cycle.
  • Nudge text is thoughtful: Written in second-person, conversational, explains the status.json contract inline — the agent is likely to act on it correctly.
  • Stage UX changes are additive and back-compat: On-disk stage key (cleanup) unchanged; only display strings updated.

🔴 Issues

1. discoverProjects() called inside useMemo on every render — O(disk) hot path

In WorktreeContext.tsx, worktreesWithRalph calls gitRef.current.discoverProjects() on every state.worktrees or ralphState change. discoverProjects is a filesystem scan. This runs at the 2 s AI-status refresh rate. It should be memoized separately (e.g. cached in WorktreeCore or fetched once with a longer TTL).

2. RalphCore instantiated outside useRef in WorktreeProvider

const ralphRef = React.useRef<RalphCore | null>(null);
if (!ralphRef.current) {
  ralphRef.current = new RalphCore({ ... });
}

The if block outside hooks is technically safe here (refs are stable) but the getWorktrees closure captures coreRef.current correctly via the function reference. However, if WorktreeProvider ever remounts (test teardown, StrictMode double-invoke), ralph.start() will fire before the previous instance's ralph.stop() because the useEffect for stop runs after the new render. Consider using useEffect for initialization or at minimum documenting the strict-mode caveat.

3. wrapToLines is a new utility but lives inline in TrackerBoardScreen.tsx

Word-wrapping logic is genuinely reusable (e.g. anywhere a brief_description is displayed). It should live in src/shared/utils/ or at minimum be extracted to a sibling file rather than embedded in the screen component. As written it's also untested — a few edge cases (zero-width input, single very long word) deserve unit tests.

4. ROWS_PER_ITEM = 4 is a breaking change for existing scroll math

Increasing the constant from 3 → 4 means all existing kanban columns render shorter (one fewer item visible). The isSelected ? 4 : SECONDARY_MAX_LINES expansion for focused cards means the selected card uses 4 rows while unselected use 3 (2 secondary + 1 margin). The constant should either be kept at 3 for unselected items and 4 only for the selected slot, or the scroll math should be per-item rather than fixed. As-is, all columns show one fewer item at all times to reserve a row that's only used on the focused card.

5. loadRalphConfig in worktreesWithRalph reads from disk synchronously on the React render path

loadRalphConfig does fs.readFileSync. Even with the per-project dedup cache, this fires on every useMemo recompute triggered by the 2 s AI-status refresh. If the tracker/ directory is on a network mount or slow SSD, this will cause visible jank. The ralph config rarely changes; it should be loaded once at startup (or on file-watch event) and stored in RalphCore's state, not re-read in the render path.


🟡 Minor / Style

  • WorktreeContext.tsx comment block — the multi-line comment before worktreesWithRalph is informative but longer than the project convention (single-line "why" comments only). Trim to the key insight.
  • ralphSuffix IIFE in WorktreeRow.tsx — readable, but could be a helper function buildRalphSuffix(ralph) for easier unit-testing of the display logic (the ! for capped, n:X/Y for nudge count, etc.).
  • gateOptionFor in TrackerStagesScreen.tsx — the inline label ternary is clear but the function is called three times with hard-coded string literals; consider making stage the parameter type 'requirements' | 'implement' | 'cleanup' explicit (it already is — good).
  • sampleWorktree signature — the loadCfg parameter has a default of loadRalphConfig but the caller always passes the memoized loadCfg closure. The default is dead code and slightly misleading; remove it.
  • shallowEqual at module level — not exported, which is correct, but it's a general-purpose helper that could go in shared/utils. Low priority.

🔒 Security / Safety

No security concerns — this feature writes to local project files only and delivers text to tmux panes that the user already owns. The nudge text is constructed from trusted project config (not user-facing strings from external sources), so there is no injection risk.


🧪 Test Coverage

The PR description mentions 672 passing tests with new coverage for:

  • status.json helpers
  • tri-state parse + legacy back-compat
  • Ralph guard invariants (including waiting_for_approval suppression)
  • Nudge text polite-check assertions
  • Stage-guide generation per setting
  • moveItem state reset on advance

This is solid. Gaps to consider:

  • wrapToLines edge cases (zero width, single long token, exactly-fits input)
  • worktreesWithRalph decoration logic (does it correctly fall back when projectPath is missing?)
  • ROWS_PER_ITEM = 4 scroll math regression (does the column scroll position stay correct when items wrap to 2 secondary lines?)

Summary

The core design is sound and the PR is well-tested for its stated scope. The main issues to address before merge are the disk I/O on the React render path (issues #1 and #5) and the ROWS_PER_ITEM scroll math regression (issue #4). The others are minor polish items.

@agent-era
agent-era merged commit ddb93b5 into main Apr 21, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants