Pluggable agent harnesses: claude behind interface, codex/gemini ready

[rr0hit]

Summary

• Introduces internal/harness package — a 14-method interface abstracting the agent CLI driver (Claude Code today; Codex/Gemini ready). Existing claude-specific code moves into internal/harness/claude/ behind claude.New().
• Adds tasks.harness column for per-task adapter pinning. NULL = claude (back-compat for pre-column DBs). Set on first flow do / flow do --here from the ambient harness; --force is required to switch.
• Adds tasks.session_cwd column — fixes a pre-existing transcript-not-found bug for --here-bound sessions where claude was started in a directory other than task.work_dir (commonly via --force binds). Session identity is now keyed on (cwd, session_id) to match claude's on-disk layout.
• Ambient harness detection via env-var probe (CLAUDE_CODE_SESSION_ID, future CODEX_THREAD_ID, GEMINI_SESSION_ID) — flow do <new-task> from inside a codex shell would pick codex when that adapter lands; falls back to claude.
• Schema changes are additive + nullable; no backfill; legacy rows behave exactly as before via NULL-fallback paths.

What this unlocks

Codex and Gemini adapters can drop in as one line each in allHarnesses(). Their per-CLI specifics (binary name, headless flag spelling, transcript location/format, hook config path) live entirely inside their own sub-packages. Flow's caller code never branches on harness name — it dispatches via the interface or via harnessForTask(task) / harnessForSpawn(task) / ambientHarness() helpers in internal/app/harness.go.

Design choices worth flagging

• Pre-allocation strategy is per-harness. Claude's NewSessionID() mints a v4 UUID locally (passed via --session-id). Codex/Gemini will probe their CLI (codex exec / gemini -p) to mint and capture a session id from stdout before opening the user-facing tab. The interface contract is uniform — every impl returns a real id, callers have one spawn path.
• No env-var injection. Flow never sets FLOW_TASK or any other env var on spawned harness processes. The harness's own session-id env var is the only correlator, and flow only reads it. Avoids polluting nested-session environments and keeps --here's discovery path symmetric with first-spawn binding.
• Harness owns transcripts end-to-end. RenderTranscript(workDir, sessionID, compact, cutoff, w) lives on the interface; each harness handles both path resolution AND format decoding (claude jsonl, codex event log, gemini single-object json) and writes normalized text. The ~270 lines of claude jsonl decoders move into internal/harness/claude/transcript.go.
• Cross-harness binding under --force. flow do --here --force on a task pinned to a different harness switches the pinning alongside the session_id rebind, with a warning that the prior transcript is orphaned from flow's view.

Commits (chronological)

1. Extract abstraction; claude impl behind interface
2. Close claude hardcoding leaks in spawner backends + app helpers (closing 7 leaks identified by an audit pass)
3. Tighten interface — rename PrepareSpawn → NewSessionID, HeadlessRun → SkipPermissionsRun, drop HookEnvForSpawn, replace TranscriptPath → RenderTranscript
4. Per-task harness column + ambient detection
5. session_cwd column — fix (cwd, session_id) identity for --here binds

Test plan

• Schema migration is additive (verified by reviewing the ALTER statements; runs on first flow invocation after upgrade)
• flow do <new-task> writes both harness and session_cwd columns; verify with sqlite3 ~/.flow/flow.db "SELECT harness, session_cwd FROM tasks WHERE slug='<slug>'"
• flow do --here <task> from a directory ≠ work_dir writes session_cwd = os.Getwd(); subsequent flow transcript <task> finds the file
• flow do --here --force <task> on a task pinned to a different harness emits the warning and switches the pinning
• Pre-existing tasks (NULL harness / session_cwd) continue to render transcripts via the work_dir + claude fallbacks
• flow done <task> still drives the headless close-out sweep against the task's pinned harness
• make test passes locally (verified uncached on the branch)

🤖 Generated with Claude Code

[anshulsao]

Solid extraction overall — the interface seams are well-chosen, the schema migration is clean (additive + nullable, no backfill), and make test is green on the branch. The session_cwd fix is a real correctness win that the PR body undersells: it actually makes flow do --here + flow transcript work for the --force and "started claude elsewhere" cases, which I've hit personally.

Five non-blocking-ish items below, ordered by impact. Two are [should-fix] mostly because they'd bite the moment a second harness lands; the rest are nits.

Backend flow diagrams

flow do <task> — harness pick on bootstrap

BEFORE                                  AFTER
──────                                  ─────
cmdDo(args) → findTask → task           cmdDo(args) → findTask → task
        │                                       │
        ▼                                       ▼
(claude assumed everywhere)             [+ h = harnessForSpawn(task)]
        │                                  1. task.harness pinned → use it
        ▼                                  2. ambient env-var set → use it
liveClaudeSessions()                       3. else → claude
        │                                       │
        ▼                                       ▼
spawner.FocusSession(sid)               h.LiveSessionIDs() → map[id]count
        │                               spawner.FocusSession(sid, h.Binary())
        ▼                                       │
newUUID()                                       ▼
        │                               h.NewSessionID()
        ▼                                       │
UPDATE session_id, …                            ▼
        │                               UPDATE session_id,
        ▼                                      [+ session_cwd  = task.work_dir],
"claude --session-id <id> …" prompt            [+ harness      = h.Name()]
        │                                       │
        ▼                                       ▼
spawner.SpawnTab(...)                   command = h.LaunchCmd(sid, prompt, opts)
                                        spawner.SpawnTab(...)

flow do --here <task> — new mismatch gate

BEFORE                                  AFTER
──────                                  ─────
sid = $CLAUDE_CODE_SESSION_ID           [+ h = ambientHarness()]
v4 UUID regex check                        - probes every h.SessionIDEnvVar()
        │                                  - nil if none / multiple set
        ▼                                       │
findTask → task                                 ▼
        │                               sid = os.Getenv(h.SessionIDEnvVar())
        ▼                               [+ h.ValidateSessionID(sid)]
"session bound elsewhere?" check                │
        │                                       ▼
        ▼                               findTask → task
UPDATE session_id, status                       │
                                                ▼
                                        [+ if task.harness ≠ h.Name() and pinned:
                                             require --force; warn that prior
                                             transcript history orphans]
                                                │
                                                ▼
                                        "session bound elsewhere?" check
                                                │
                                                ▼
                                        UPDATE session_id,
                                              [+ session_cwd = os.Getwd()],
                                              [+ harness     = h.Name()],
                                              status

flow transcript <task> — data-flow rewire

BEFORE                                  AFTER
──────                                  ─────
sessionJSONLPath(task)                  [+ h = harnessForTask(task)]
  = ~/.claude/projects/                          │
    EncodeCwdForClaude(task.work_dir)/           ▼
    <session_id>.jsonl                  cwd = task.session_cwd
        │                                     ?? task.work_dir   ← legacy fallback
        ▼                                       │
renderTranscript(path, compact, cutoff)         ▼
  - jsonl decoder lives in              h.RenderTranscript(cwd, sid, compact,
    internal/app/transcript.go            cutoff, os.Stdout)
                                          - claude impl owns path resolution
                                            AND format decode; jsonl decoder
                                            moves to harness/claude/transcript.go
                                          - decoder targets io.Writer (was stdout)
                                          - error path: enhanced hint pointing
                                            users at `flow do --here` for legacy
                                            session_cwd=NULL rows

Schema delta — tasks row

BEFORE                                  AFTER
──────                                  ─────
tasks columns:                          tasks columns:
  ...                                     ...
  session_id           TEXT               session_id           TEXT
  session_started      TEXT               session_started      TEXT
  session_last_resumed TEXT               session_last_resumed TEXT
                                        [+ session_cwd        TEXT  ]
                                        [+ harness            TEXT  ]
                                          ALTER ADD COLUMN, nullable,
                                          no backfill. NULL = back-compat
                                          read paths (work_dir / claude).

Package layout

BEFORE                                  AFTER
──────                                  ─────
internal/app/                           internal/app/
  bootstrap.go  newUUID, EncodeCwd        harness.go   (dispatcher: 6 helpers)
  sessions.go   liveClaudeSessions        transcript.go (slim; delegates to h)
  transcript.go ~270L jsonl decoder
                                        internal/harness/      (new)
                                          harness.go    (Name, LaunchOpts,
                                                         14-method Harness iface)
                                          claude/       (new)
                                            claude.go
                                            transcript.go (jsonl decoder relocated)
                                            *_test.go

Vibecode dial

Vibecode dial:
                       ●
   tight ──────────────┴────────── vibecoded
                       ↑
       (mostly tight refactor; modest helper sprawl + a couple of
        unowned coercion paths nudge it one notch right)

Receipts:

Files changed:          36   (all in scope — harness extraction touches
                              every spawn backend by design)
Lines added (code):     ~1500
Lines added (test):     ~656 (harness_test.go 251 + claude tests 405)
Lines added (comments): heavy — design-rationale doc blocks throughout
New abstractions:       1   (Harness interface, 14 methods)
New helper funcs:       6   (harnessForTask, harnessForSpawn, ambientHarness,
                             defaultHarness, harnessByName, liveSessionsForTasks)
Adjacent refactors:     0
Stale-comment edits:    0   (signature-driven; necessary)
Drive-by changes:       0

What kept it from full-tight: 6 dispatch helpers in internal/app/harness.go where 2–3 would suffice; duplicated encodeCwd / EncodeCwd; one --here comment that asserts more invariant than the code actually upholds; harnessByName's silent unknown-value coercion has a write-back path that can erase non-NULL non-claude values on --fresh bootstrap.

[rr0hit]

Thanks for the thorough pass @anshulsao — all 7 items addressed in 835a308. Per-comment replies above; net of the changes:

[should-fix]

• do.go duplicate-process warning + live-session error now format with h.Binary() — won't say "claude processes" when a codex/gemini duplicate tab triggers it.
• Bootstrap UPDATE now preserves a pre-existing harness pin via a CASE WHEN harness IS NULL OR harness = '' THEN ? ELSE harness END clause; the "set once on first bind" doc invariant is now upheld in code. flow do --here --force remains the explicit lane to switch harnesses. New regression test pre-pins to "codex" and verifies cmdDo doesn't silently overwrite.
• RenderTranscript first param renamed workDir → cwd in interface + impl. Error message at file-not-found drops the misleading session_cwd=%q claim.

[nit]

• --here cwd-invariant comment hedged for the chained-cd edge case.
• Dropped the duplicate encodeCwd (kept EncodeCwd).
• Test renamed to TestHarnessForSpawn_AllPathsLandOnClaudeToday with doc comment acknowledging precedence is uncoverable today.
• Stray blank line in skill.go imports trimmed.

Deferred: tightening harnessByName to return (Harness, error) for unknown non-empty values (Anshul's option (a) on item 2) — meaningful only once the registry grows past claude. The COALESCE write-guard removes the actual hazard for now and a comment in harnessByName flags the read-side coercion for whoever wires codex.

Ready for another look when you have a minute.

[anshulsao]

All 7 asks addressed. CI rollup green (vet/test/build), go test ./... green locally. Approving.

Verification per comment:

#	Tag	Ask	Fix
1	should-fix	do.go:177 hardcoded "claude processes"	both stderr templates now interpolate h.Binary() (live-duplicate warning + running-elsewhere error)
2	should-fix	harnessByName silent coercion	harnessByName → (harness, error); empty/NULL → claude+nil for back-compat, unknown non-empty → nil+error listing registered names. Plus a belt-and-braces CASE WHEN harness IS NULL OR harness = '' THEN ? ELSE harness END on the bootstrap UPDATE so even a same-name pin doesn't get re-written. TestCmdDoRefusesUnsupportedHarnessPin pins the future-pin → refuse-and-preserve contract
3	should-fix	workDir param lies	interface + impl renamed workDir→cwd; error message switched to cwd=%q so it no longer claims a session_cwd source the caller may not have used
4	nit	--here cwd-invariant comment	rewritten to acknowledge the chained-cd blind spot explicitly
5	nit	duplicate encodeCwd/EncodeCwd	unexported version dropped; doc on the exported form picked up the merge
6	nit	test name oversells	renamed TestHarnessForSpawn_AllPathsLandOnClaudeToday; docstring now spells out what is and isn't testable today
7	nit	stray blank line in skill.go imports	removed

Two notes I like beyond the asks:

• The COALESCE-style guard on the bootstrap UPDATE is a tighter answer than what I suggested. I asked for one of (a) error path or (b) preserve-pin; you did both, which means even if a future code path resolved an unknown pin to claude, the column wouldn't get overwritten silently anymore. Belt-and-braces is the right call for a "set once on first bind" invariant.
• done.go's warn-and-skip on harnessForTask error keeps the status flip intact when the close-out sweep can't run. That's the right tradeoff — the flip is the user-facing contract; the sweep is best-effort plumbing.

Vibecode dial — re-review

Vibecode dial:
                ●
   tight ───────┴────────────── vibecoded
                ↑
       (tightened from near-tight to tight after fixes —
        no scope creep, fixes match asks, one extra test
        per concern, no drive-bys)

Receipts on the fix delta (bdfe9c8..386511a):

Files changed:          11   (all directly traceable to one of the asks)
Lines added (code):     ~150
Lines added (test):     ~70  (one regression test per should-fix:
                              future-pin refusal; existing tests
                              updated to thread the new error return)
Lines added (comments): heavy — but all of it is doc on the new
                        error semantics (where unknown values
                        error vs. where they're tolerated)
New abstractions:       0
New helper funcs:       1   (registeredHarnessNames — used in the
                              error message; reasonable)
Adjacent refactors:     0
Stale-comment edits:    1   (the encodeCwd→EncodeCwd doc merge)
Drive-by changes:       0

What kept it from full-tight: a single new helper (registeredHarnessNames) was added to format the error message. Could have been inlined into the fmt.Errorf, but it's small and named so I'd leave it. Otherwise nothing to nit — fixes are surgical, comments are load-bearing, no whilst-we're-here cleanups landed.

[rr0hit]

New commit eda3a3c lands the GH #59 fix on this branch. Worth a second look since it reshapes a chunk of what bdfe9c8 introduced.

The shift in approach: bdfe9c8 introduced session_cwd to record where each session was actually started. That was a symptom-side fix — it let flow transcript find the jsonl, but flow do resume still consulted task.work_dir alone (the open half of #59). This commit reverts the session_cwd column entirely (it was unreleased, no migration baggage) and instead enforces the invariant *any task with session_id IS NOT NULL has work_dir == cwd of session at creation*. Resume always spawns at work_dir`; the invariant guarantees the jsonl is there.

Enforcement is per-harness: new Harness.ValidateSession(workDir, sessionID) error method. Claude's impl stats ~/.claude/projects/<encode(workDir)>/<sid>.jsonl. Gemini will share that shape; codex (sid-only sessions, prompts the user about cwd at resume) will no-op. This is the right home for the policy because each harness's transcript layout decides whether (cwd, sid) is a meaningful key.

Why stat, not os.Getwd(): comparing the subprocess's cwd to work_dir is fooled by cd <work_dir> && flow do --here task from inside a claude Bash invocation — the chained cd only affects flow's subprocess, not where the actual session jsonl was written. The stat catches that.

Gates:

• flow do --here refuses on validation failure. --force does NOT override.
• flow update task --work-dir refuses if a session is bound AND the new path doesn't satisfy ValidateSession. This is the supported "fix a broken row" path — point work_dir at where the jsonl actually lives.

flow run playbook --here: the inserted run-task now takes work_dir = os.Getwd() rather than the playbook's default, since the run is by definition about the binding session's cwd.

Skill update: §4.16 documents the new safety property and adds a cwd-mismatch sub-recipe (Open in new tab / Point work_dir at this session's directory). Anti-patterns explicitly warn against the cd-cheat and against --force as a bypass.

Existing rows that already violate the invariant (your audit found ≥6) error cleanly on next use; the flow update task --work-dir path is how they get fixed. No backfill subcommand — would have hardcoded claude paths, premature with the multi-harness future.

A cleanup commit follows to drop the orphan session_cwd column from any DB that ran bdfe9c8-era binaries during development.