Maintainability pass: clip consolidation, workflow steps/tools split, chat-session decomposition + 0048 draft #123
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "glen/refactor-maintainability"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Maintenance refactor from the architecture review — four mechanical commits, a fresh-eyes review, and a draft design doc. Zero behavior change: the review child verified every moved function byte-identical and probed the tool key set across all gate combinations empirically; full suite 777 passed throughout.
The commits
lib/clip.ts— seven hand-rolled text-clip helpers consolidated into four shared shapes (clipEnds/clipHead/clipTail/clipLines). gh/forgejo/web_fetch markers unified onto the informative… [N chars clipped]convention (3 test assertions follow). Three justified locals stay: wake's pointer-awareclipText, harness-events' unknown→JSON value clip, child-status's null-passthrough.workflows/chat/steps/— the 646-line steps.ts (42"use step"functions) split by domain into 7 files + index re-export; importers keepfrom "./steps". Every function keeps its directive and lazy imports (paths../../→../../../).workflows/chat/tools/— the 540-line tools.ts split into per-domain factories;buildChatToolssignature and gating unchanged; key sets verified identical.components/chat-cards/— chat-session.tsx 1,584→725 lines: approval cards, tool dispatch, andChatMessageViewextracted verbatim. Adding a tool card now touches a 135-line dispatch file, not a 1,600-line session.docs/0048-machines-retirement.md(DRAFT — needs your approval before any implementation). Maps the machine-era read residue exhaustively and frames the real decision: A) keep/r/rendering, delete only the dead "is it live?" semantics (~150 lines, zero risk) vs B) backfill run transcripts and delete the run view entirely (~zero residue, needs a migration). The review caught my first draft's wrong premise (run rows carry no transcript per 0012) — corrected.Review findings addressed: the doc's residue list completed (redirect, hrefFor, glyph, sandboxes lease arm, reaper's second read, previews' read, the project-delete write), nonexistent identifiers fixed, and three code nits (web.ts marker, unused
ToolContext, alias placement).The 0048 implementation itself is deliberately NOT in this PR — it follows separately once you pick A or B.
workflows/chat/steps/{turn,sandbox,web,projects,children,plan,hosts}.ts, each function keeping its "use step" directive and lazy Node imports (paths rewritten ../../ -> ../../../); steps/index.ts re-exports the union so the four importers (turn body, tools, plan-tools, model) keep their './steps' paths unchanged. persistTurn's private textOf helper moves with it; the floating maybeTitleStep doc comment is reunited with its function. No behavior change — the full suite passes unchanged.workflows/chat/tools/{core,children,web,hosts,projects,sandbox}.ts each build one group via a small factory (closing over conversationId and the same hasSandbox/hasProject/hosts gates); tools/index.ts keeps buildChatTools with its exact signature and doc, spreading the groups. shared.ts holds the ToolContext type and the shared commandDescription zod schema. Tool key set verified identical to the single-file original; hostTools keeps the hasProject gate (a test pins it).components/chat-cards/{approvals,dispatch,message-view}.tsx hold the approval-gated cards (create_pr evidence, ask_chad, accept_plan, exec, project gates, the PR card), the agent-tool dispatch switch + generic fallback, and ChatMessageView with its chips/copy/images helpers — moved verbatim, imported back, ChatMessageView re-exported so chat.tsx / agent-panel.tsx are untouched. Adding a tool card now means touching dispatch.tsx (135 lines), not a 1,600-line session file.