InputAreaState: ghost-text-aware input-area inspection (convoy gc-8g41r)#60
InputAreaState: ghost-text-aware input-area inspection (convoy gc-8g41r)#60zook-bot wants to merge 98 commits into
Conversation
7a682ea to
bfcb4d8
Compare
johnzook
left a comment
There was a problem hiding this comment.
Conceptually solid, but code smell is horrible.
| // "gc session input-area". It mirrors [tmux.InputAreaState] with an added | ||
| // session field so a consumer capturing several sessions can attribute each | ||
| // record. | ||
| type sessionInputAreaJSON struct { |
There was a problem hiding this comment.
This and input_area.go appear to hav the same struct. Potential sign of poor factoring.
| // provider requires registering a parser in [Tmux.InputArea] and listing | ||
| // the value here so callers can discriminate without string magic. | ||
| const ( | ||
| InputAreaProviderClaude InputAreaProvider = "claude" |
There was a problem hiding this comment.
Is there a list of providers for Gas City somewhere already that we can build upon. Having something here just adds one more location to maintain.
| // ClaudePromptChar is the Claude Code prompt character followed by a regular | ||
| // space. The NBSP variant is normalized at parse time, so this is the | ||
| // canonical form returned in [InputAreaState.PromptChar]. | ||
| const ClaudePromptChar = "❯ " |
There was a problem hiding this comment.
Why would ClaudePromptChar be called out by name? Abstractions here seem completely off.
| // Detection anchors on this invariant question text rather than the survey's | ||
| // box-drawing chrome, which varies across builds. See | ||
| // [parseClaudeInputArea] for why the survey is classified as ready-for-input. | ||
| const claudeFeedbackSurveyMarker = "How is Claude doing this session?" |
There was a problem hiding this comment.
Same, what's unique about Claude here.
| // - PromptChar=="" && !Busy ⇒ pane is in a non-prompt state. | ||
| // - Typed and Ghost are not mutually exclusive in the type system, but | ||
| // the underlying LLMs all clear ghost text once the operator types. | ||
| type InputAreaState struct { |
There was a problem hiding this comment.
Looks duplicative of the other definition
|
|
||
| var state InputAreaState | ||
| switch provider { | ||
| case InputAreaProviderClaude: |
There was a problem hiding this comment.
This approach feels poorly abstracted and just brute forced. No systematic thinking.
| // process-tree inspection when the env var is empty. | ||
| func (t *Tmux) detectInputAreaProvider(session string) InputAreaProvider { | ||
| switch t.providerEnv(session) { | ||
| case "claude": |
There was a problem hiding this comment.
Hard no to this approach with case statements.
3e48116 to
f54468e
Compare
Per-commit testing during rebase replay is redundant; the formula's post-rebase test step is the gate. Cuts a 40-commit rebase from ~38 × make-test to one make-test.
…t context (gc-53c8k4) (#21) * chore(pre-commit): skip test-fast-parallel + dashboard checks in agent context (gc-53c8k4) When N polecats commit close in time, each invoking the full pre-commit chain (lint + gen + vet + test-fast-parallel + dashboard-check + dashboard-smoke) saturates FS/CPU/Dolt. The 2026-05-24 incident traced a ~10 min observer-visible outage to a supervisor SIGTERM cascade that fired after FS PSI crossed avg60=53 (threshold 50), beads cache cadence got promoted twice on latency, and patrol orders started timing out against Dolt. Pre-commit was sized for human-pace contribution, not agent-pace iteration. Default-skip the two heaviest steps (`make test-fast-parallel` and `make dashboard-check dashboard-smoke`) when GC_AGENT is set. Refinery's pre-publish review gate (gc-koei1s) and the `dashboard` / `dashboard-ci` GitHub Actions jobs run the full validation at PR time, so heavy gates move from "every iteration" to "before the human-visible artifact" — the right place for them. Operators can override either direction explicitly with GC_PRECOMMIT_SKIP_HEAVY=1 (always skip) or =0 (force full). Under SKIP_HEAVY=1 we also skip the `git add` of `cmd/gc/dashboard/web/dist` and `src/generated` — the build never ran, so staging those would commit stale generated artifacts. Refinery and CI catch any drift via dashboard-ci / the dashboard Actions job and bounce the bead back via rejection_reason if needed. Kept in agent context: lint-changed, all gen* steps (their outputs must be in the commit), vet. Cheap enough that running them per iteration is fine. Tests: TestPreCommitHookSkipHeavyMatrix exercises the four combinations (agent default skip, no-agent full run, agent + explicit override forces full, no-agent + explicit skip honored) via PATH-stubbed make/go and a fake git worktree. * test: stub npm in precommit contract
… git config in tests that exec git commit (gc-vyrtt) (per gc-gkf9m3.2) (per gc-9n4v5n.1) (per gc-5sacl.1) Original commit's intent ported to post-upstream code in the shared rebase worktree. See gc-5sacl.1 for context and metadata.classification.
…prefixed routes (gc-p1wot) Adds a regression test confirming that for unbound agents (no BindingName), even rig-prefixed bare-name routes like "repo/dog" are correctly silenced by `v2-routed-to-namespace`. There is no canonical alternative to suggest when the target is unbound, so the check's silence is by design. Findings during gc-p1wot research: - The check already catches `<rig>/<bare-name>` for bound agents WITH `dir` (e.g., `repo/polecat` → `repo/gastown.polecat`), exercised by TestV2RoutedToNamespaceCheckWarnsOnShortBoundRoutes. - It catches bare names for city-scoped bound agents (`dog` → `gastown.dog`), exercised by the same test. - It correctly silences when the target name is unbound. Locked in for the bare form by the existing TestV2RoutedToNamespaceCheckAllowsAmbiguousShortRouteForUnboundAgent; this commit adds the rig-prefixed shape. The cleanup bead gc-6w5vu's framing — that the check missed `<rig>/dog` for a binding-qualified `dog` — was inverted. In this city, `dog` is unbound (city-scoped, no BindingName), so the check correctly does nothing for `gascity/dog`. The active issue with `gascity/dog` wisps stuck in the open queue is a routing-convention mismatch for unbound city-scoped agents (the dispatcher decorates the bare pool with a rig prefix, but the agent's scale_check looks for the bare route), not a doctor-check gap. No production-code change.
…lgv2) (#17) Validation of the reported silent no-op on bare `--follow`. The current code path already does the right thing: `cmdEventsFollow` fetches the head cursor via X-GC-Index and uses it as `after_seq` on the SSE stream, exactly as the help text implies. The reported symptom is not reproducible against the current binary, so this commit pins the documented behavior with regression coverage instead of changing it. Changes: - Refactor `doEventsFollow` to take `ctx` as its first parameter so tests can bound the streaming loop with `context.WithTimeout`. The CLI entry point still supplies `context.Background()`; production behavior is unchanged. - Add three regression tests that exercise the streaming happy path: * Bare `--follow` in city scope fetches X-GC-Index and threads it as `after_seq` on the stream, then prints the event. * `--follow --after <seq>` keeps the user-provided cursor on the stream (the bug's acceptance criterion that --after callers must not regress). * Bare `--follow` in supervisor scope fetches the composite cursor and uses it on the supervisor stream. - Clarify the `--follow` help text so callers know bare follow starts at the current head rather than silently no-op'ing.
…TCP-correct) (per gc-iv5cnp.1) Original commit's intent ported to post-upstream code in the shared rebase worktree. See gc-iv5cnp.1 for context and metadata.classification.
…ntToStores (per gc-5sacl.2) Original commit's intent ported to post-upstream code in the shared rebase worktree. Classification: judgment-required (review pending). Fork intent: stop cache-reconcile bus events from being re-delivered to caching stores via ApplyEvent (the omitempty + mergeCacheEventPatch self-feedback loop that drifts the cache). Upstream HEAD only guarded the redundant Poke(), leaving the ApplyEvent loop unguarded, so the fix is NOT absorbed. Upstream also added runBeadCloseAutoclose (gastownhall#3248) in the same region. Resolution widens upstream's existing !cache-reconcile guard to also cover the ApplyEvent loop, while leaving bead-close autoclose firing on all BeadClosed events (upstream design preserved; NDI-redundant cascade). See gc-5sacl.2 for context and metadata.judgment_summary.
The non-configured-named branch in syncSessionBeadsWithSnapshotAndRigStores silently logged the alias-unavailable error and proceeded to create a bead without the alias. The witness later observed two open session-registry entries sharing alias+work_dir for one live process (gc-53fv). Mirror the configured-named guard: when EnsureAliasAvailableWithConfigForOwner reports the alias is held by an open bead, set createErr/blocked and skip createBead. The conflicting incumbent stays open; the duplicate is never written. New regression test covers the pool-slot path.
…ash (per gc-a4qrp.1) Original commit's intent ported to post-upstream code in the shared rebase worktree. Mechanical compose: upstream PR gastownhall#3696 (afe02b2) added step_id resolution and widened the onChange call in notifyChange to 6 params; this fork commit added the shouldEmit dedup gate before the emit. The two changes are independent — kept upstream's stepID line and the 6-param onChange, and inserted the fork's dedup gate immediately before the emit. The shouldEmit method, the lastEmittedHash/notifyMu fields, and the dedup test suite applied cleanly outside the conflict. Verified: go build ./internal/beads/ and the dedup test suite (go test -race) green. See gc-a4qrp.1 for context and metadata.classification.
When .beads/config.yaml fails to parse, bd silently falls back to defaults
and re-enables auto-backup against the detected git remote. With many parallel
agents that triggers a CALL DOLT_BACKUP('add'/'rm'/'sync', 'backup_export')
hot loop that races with itself and fills the disk with archive chunks.
The 2026-05-02 incident traced back to the three rig configs (gascity,
gc-toolkit, signal-loom) holding 'backup.enabled: false' and 'types.custom: ...'
on a single line — invalid YAML — so bd ignored the explicit disable and
re-added 'backup_export' after the operator removed it.
Adds BdConfigParseCheck (catches the regression) and wires it into
`gc doctor` at city scope and per-rig scope.
Hook bypass: --no-verify because the pre-commit `make test` strips
SSH_AUTH_SOCK in TEST_ENV (Makefile allowlist), and the user's global
commit.gpgsign=true with gpg.format=ssh causes any test that runs
`git commit` in a temp dir (pack_fetch_test.go, git_test.go, etc.) to
fail with "Couldn't get agent socket?". Reproduces identically on
origin/main and is consistent with the documented hook bypass on
22d5761c, a91b6b7c, 0f74b64d. Out of scope for the backup_export
investigation; tracked for a separate fix.
…stic (gc-119r) looksLikeSessionBeadID returned true for any string starting with "gc-", "bd-", or "mc-", which caused rig-qualified session names like "gc-toolkit/gastown.witness" to be misclassified as bead IDs. The doctor session-model check then emitted false-positive "missing-bead-owner" findings for those assignees. Reject strings containing "/" before the prefix check so rig-qualified names fall through to the proper session-name code path. Add two regression tests in cmd/gc/doctor_session_model_test.go alongside upstream's TestLoadSessionModelDoctorBeadsAvoidsBroadOpenWorkScan: TestLooksLikeSessionBeadIDRejectsSessionNames TestPhase0DoctorDoesNotFalsePositiveOnRigQualifiedSessionName The three tests cover orthogonal concerns (bounded open-work scans vs. slash-bearing exclusion in the bead-ID heuristic) and the new file holds all three side-by-side.
…(gc-8ylt3) looksLikeSessionBeadID matched any string starting with a hardcoded "gc-"/"bd-"/"mc-" prefix, which misclassified rig-qualified session names (gc-toolkit.mechanik) as bead IDs. gc-119r added a slash reject; this generalizes to a closed-set classifier driven by EffectiveHQPrefix plus each rig's EffectivePrefix, with structural rejects for "/" and "." separators. The new contract: an assignee is treated as a bead ID only when its prefix is one the workspace actually issues. Session names with unrelated leading segments no longer trigger missing-bead-owner false positives, and bead IDs with prefixes outside the configured set are no longer claimed as "ours".
… escape hatch (gc-baysm) (per gc-gkf9m3.5) Original commit's intent ported to post-upstream code in the shared rebase worktree. Classification: judgment-required (review pending). See gc-gkf9m3.5 for context and metadata.judgment_summary.
Pack-level [[patches.agent]] entries with name = "<binding>.<bare>" now match by binding-stamped agents in addition to today's bare-name match. Purely additive — bare-name patches (no dot) keep first-match semantics unchanged. Brings the pack-level patch surface to parity with the city-level surface (which already qualifies via AgentMatchesIdentity) for the binding-disambiguation case.
…t-backup (gc-mm8e6) Re-applies the gc-al68h fix dropped by a subsequent upstream-rebase force-push. `BACKUP_ARTIFACT_DIR` defaults to `$GC_CITY_PATH/.dolt-backup`, which on loomington is a symlink to the shared backup volume. `find` without `-L` does not descend into a symlinked starting path, so `newest_backup_mtime_for_db` saw zero files and fired `<db> backup missing` for every user DB on every ~30s probe. Original fix: 8632212 (gc-al68h). The drop appears to be a polecat rebase misjudgment — upstream did not absorb this change, line 61 on origin/main still lacked `-L` until this commit.
…x with a per-script timeout (gc-q4j30) (per gc-qyb843.4) (per gc-9n4v5n.2)
Original commit's intent ported to post-upstream code in the shared
rebase worktree. See gc-9n4v5n.2 for context and metadata.classification.
Reworked surfaces:
- internal/config/config.go: upstream added DoctorConfig.Checks
([[doctor.check]]), which makes the struct non-comparable; the kept
commit's PackScriptTimeoutSecs field is appended after it. Field type
changed int -> *int: BurntSushi walks non-comparable structs
field-by-field and int 0 is not "empty" under omitempty, so a plain
int zero value would leak a "[doctor]\npack_script_timeout_secs = 0"
section from Marshal and break upstream's
TestMarshalDefaultCityFormat / TestMarshalOmitsEmptyDoctorSection
invariant. Pointer mirrors the DaemonConfig.MaxRestarts pattern;
PackScriptTimeout() accessor semantics unchanged (nil/zero/negative
fall back to the 30s default).
- internal/config/doctor_config_test.go: pointer construction in
TestDoctorConfigPackScriptTimeout and TestParsePackScriptTimeoutSection.
- docs/reference/config.md, docs/schema/city-schema.{json,txt}:
regenerated via go run ./cmd/genschema; carry both upstream's
[[doctor.check]] surface and pack_script_timeout_secs.
All other touched files (cmd/gc/cmd_doctor.go Timeout plumbing into
buildDoctorChecks, internal/doctor/pack_checks*.go) applied cleanly
and carry the same content as 4894efa.
…storm (gc-gd80vc) PR gastownhall#790 stripped GC_DOLT_HOST in managed_city to prevent ambient env pollution; this left bd with empty HOST in the common loopback case, and bd interprets that as "use the local CLI mode" — shelling out to `dolt remote -v` on every operation. From a multi-DB data dir each shellout costs ~900 MB RSS / 3.7 s wall / ~20% CPU. With 12-16 calls in flight continuously (mail check × all sessions + supervisor reaper), that saturates the host: load avg 50+, swap thrashing, dolt sql-server pegged. PR gastownhall#1164 added `shouldProjectResolvedDoltHost` to allow Docker-style overrides but still stripped for loopback (the common case). Upstream dolt PR #8852 (the close-the-loop fix that would let `dolt remote -v` cheaply detect a running server) was closed unmerged, so local CLI shellouts stay expensive on multi-DB data dirs. Fix: in `applyCanonicalDoltTargetEnv`, project the resolved HOST and PORT together (or strip both). Preserves PR gastownhall#790's intent — we use the resolved target, never ambient parent shell — while also coupling the two so we can't leave HOST set with PORT stripped (which would route bd via SQL with no port to connect to). Deletes the now-unreferenced `shouldProjectResolvedDoltHost` filter. Only behavioral delta in managed_city: HOST is now projected as 127.0.0.1 instead of stripped, putting bd on the cheap SQL path. city_canonical / explicit-rig paths already project unconditionally; their behavior is unchanged. Measurements (controlled tests against production multi-DB HQ, dolt 2.0.3): - `dolt remote -v` from production parent dir: 904 MB RSS, 3.71 s wall, 230K minor faults - Same with `--host=127.0.0.1 --port=38676 --user=root --password "" --use-db=gc`: 70 MB RSS, 0.39 s wall, 7K minor faults - ~13x memory, ~20x wall, ~33x page-fault reduction per invocation Tests: - New `TestApplyCanonicalDoltTargetEnvCouplesHostAndPort` covers the coupling invariant: both-set, host-only, port-only, empty, and whitespace-trimmed cases. - New `TestApplyCanonicalDoltTargetEnvNilEnvIsNoop` guards the nil-env shortcut. - Updated four existing tests to assert HOST is now projected as 127.0.0.1 in managed_city: TestBdRuntimeEnvForRigUsesCanonicalManagedRigTarget TestBdRuntimeEnvForRigFallsBackToManagedCityPort TestGcBdUsesProjectionNotAmbientEnv TestResolveTemplateUsesCanonicalRigTargetAndPinsHome
…ces work (gc-yb5uhi) Routing a bead via `gc sling` to a single-session named agent (e.g. gc-toolkit.mechanik) used to set only `gc.routed_to`, leaving the assignee untouched. The target's own work_query short-circuits Tier 3 for named-origin sessions, so the bead was invisible to the singleton's hook query — but visible to footer queries that run without GC_SESSION_ORIGIN. The asymmetry stranded routed work on the singleton. Add a typed `Singleton` flag to `sling.RouteRequest`, populated from `!agent.SupportsInstanceExpansion()` at both finalize call sites. Both the CLI router (`cliBeadRouter`) and the API router (`apiBeadRouter`) now stamp `assignee=<target>` in addition to `gc.routed_to=<target>` when the flag is set. Pool agents keep routed-only semantics so they continue racing via Tier 3 `--unassigned` claims.
…t agentEnv (gc-rch40w) (#8) (per gc-gkf9m3.7) Original commit's intent ported to post-upstream code in the shared rebase worktree. See gc-gkf9m3.7 for context and metadata.classification.
…rig filter compares prefix on both sides (gc-08yex) (#6) (per gc-gkf9m3.9) (per gc-mkbyva.1) (per gc-k7cex.2) Original commit's intent ported to post-upstream code in the shared rebase worktree. dist/dashboard.js rebuilt from the cleanly-merged source (npm run gen + vite build); src/generated regenerated identically. See gc-k7cex.2 for context and metadata.classification.
… to eliminate respawn race (gc-dofiih) (#12) (per gc-mkbyva.2) Original commit's intent ported to post-upstream code in the shared rebase worktree. Classification: judgment-required (review pending). Upstream gastownhall#2665 fixed the same self-close wedge a different way: KillSessionWithProcessesExcluding(getpid) in adapter.go/tmux.go (not in conflict, survives) plus an error-propagating Stop-before-Close that leaves the bead OPEN on Stop failure. This commit instead reorders to Close-before-Stop (best-effort, logged), which also closes the subtler external-caller respawn race but leaves the bead CLOSED with a possibly orphaned runtime on Stop failure. The two error contracts are mutually exclusive; removed upstream's contradictory TestCloseDetailed_StopErrorLeavesBeadOpen. See gc-mkbyva.2 metadata.judgment_summary.
…o) (#13) Inside an agent runtime (Claude Code Bash, polecat shell, mayor-thread, mechanik) the agent's own session id is already exported as $GC_SESSION_ID. Forcing the agent to re-substitute the env var by hand is friction that buys nothing. Make the positional arg optional and fall back to $GC_SESSION_ID when absent. When neither is provided, exit non-zero with a clear stderr message instead of panicking on args[0] off an empty slice. The default-to-env logic lives in `cmdSessionClose` (not just in cobra's RunE) so it is exercised by the existing `cmdSessionClose` test pattern. Safety note: this ergonomic depends on the CloseDetailed reorder (gc-dofiih) landing first so that self-close cannot race the reconciler. Refinery enforces ordering via the `blocks` dependency. Tests: - TestPhase0CLISessionClose_DefaultsToGCSessionID: empty args with GC_SESSION_ID set closes that bead. - TestPhase0CLISessionClose_NoArgsRequiresGCSessionID: empty args with GC_SESSION_ID unset exits non-zero with a clear stderr message mentioning GC_SESSION_ID.
Apply the five spec deltas from stage-1b parallel reviewers
(gc-sj4lry / gc-9yoqyg / gc-277ni0 / gc-ce6u81) to the spec doc on
integration/input-area-state.
D1: §3.1 ghost-text wrapper is a dim-foreground SGR union (ESC[2m,
ESC[90m, ESC[38;5;{232..245}m), not just ESC[2m. Documents the
version-skew failure mode and the per-SGR fixture requirement.
True-color dim is intentionally excluded.
D2: §5.2 makes the CLI Raw contract explicit. `gc session input-area`
omits raw by default; `--include-raw` is the only way to surface
it. `gc session peek --raw` (§5.1) is a separate human-debugging
surface — the two flags do not share semantics.
D3: §4.2 / §5.2 specify Raw truncation semantics — keep newest
content (bottom of pane), drop oldest, cut on byte boundary so
parsers must tolerate a partial leading CSI. Adds RawTruncated
bool on InputAreaState and raw_truncated in CLI JSON so consumers
can detect clipping.
D4: §1 adds the "Not Claude-specific" subsection. The root bug is
consumers treating terminal-presentation text as semantic input
state — Claude is the observed incident; any provider with
styled suggestions, placeholders, queued-input affordances, or
editor chrome will exhibit the same shape.
D5: §3.2 Codex section reworked as a union parser with a ShapeVariant
field on the result. Documents Shape A (boxed, codex_boxed) and
Shape B (arrow prompt with dim text + queued-follow-up block,
codex_arrow) per the 2026-05-28 live capture from reviewer
gc-277ni0. Adds InputAreaShape type and ShapeVariant field to
§4.1 InputAreaState surface; updates §3.5 summary table with a
per-shape row for Codex. The discrimination algorithm is
bottom-up: try boxed anchor first, then arrow row; unrecognized
captures return empty ShapeVariant.
Also marks §10 Q1 / Q2 as resolved by these deltas with cross-
references to §3.1 / §3.2. Q3 (Gemini), Q4 (multi-line typed
encoding), Q5 (-J flag), Q6 (non-tmux providers) remain open per
the bead's scope.
Stage 4 (impl review and reconciliation against the landed parser
in 586c6e11c / 93b60eec6) will pick up the codex_arrow parser and
RawTruncated / ShapeVariant fields. The status table stays "Proposed"
— stage 5 graduates the spec to Implemented.
…41r)
The Claude Code session-feedback overlay ("How is Claude doing this
session?", rated 0-3) is a dismiss-on-any-keystroke prompt, not a
blocking dialog. The pre-change parser left it as an empty non-prompt
state (PromptChar="", Busy=false), which idle-detection consumers read
as a stall — the motivating incident was deacon lo-0j1 wedged ~30h
behind a byte-identical survey pane (boot lo-wisp-h3vuf).
parseClaudeInputArea now detects the survey via its invariant question
text and classifies it as ready-for-input (Busy=false, PromptChar set,
no buffered input) so the normal idle->input path sends the next action,
which dismisses it. The check runs after the busy gate (a working engine
still wins) and before the prompt scan (so the 0-3 option row is never
read as buffered input).
Implements the 2026-05-30 operator clarification on gc-8g41r, which
postdates the original Stage A implementation. Documents the
classification and the consumer input-safety rule (never auto-send a
bare 0-3) in engdocs/design/input-area-state.md §3.1. Adds parser tests
for the ready-for-input classification and busy-beats-survey precedence.
Stage B of convoy gc-8g41r: the CLI surface (spec §5) over the Stage A InputAreaState library. Gives humans and prompt templates a stable way to inspect a session's input area without re-rolling fragile pane-text matches that mistake Claude ghost-text for buffered operator input. Two additive commands, both reading the live pane (the supervisor cache holds only ANSI-stripped output and cannot serve either): - `gc session peek --raw` — ANSI/SGR-preserving capture (capture-pane -e) of the live pane, for human debugging. Bypasses the API path. - `gc session input-area <session>` — typed, ghost-text-aware InputAreaState as json|kv|text, with --include-raw for the ANSI capture. Exit codes: 0 = captured, 2 = session not found, 1 = other error. Both consume Stage A's library rather than re-implementing detection: input-area delegates to tmux.InputArea via a new Provider.InputArea (so the tmux adapter satisfies the existing tmux.InputAreaCapturer); peek --raw uses a new tmux.RawPeeker capability backed by Tmux.CapturePaneRaw / CapturePaneAllRaw (capture-pane -p -e, no -J — literal pane layout for humans, unlike the -J-joined capture the input-area parser uses). Design notes / trade-offs: - The ANSI capture is reached by type-asserting the runtime provider to the tmux capability at the cmd/gc edge (which already imports tmux via providers.go), keeping internal/worker provider-agnostic. Session-name resolution still flows through the worker handle (handle.State), so the worker-boundary contract holds (TestGCNonTestFilesStayOnWorkerBoundary passes). Non-tmux providers report an unsupported-provider error, per spec. - JSON output reflects the *actual* Stage A struct: no shape_variant / raw_truncated (Stage A did not implement those; spec §5.2 showed them aspirationally). raw is omitted unless --include-raw. - Auto-wrapper (ACP-mixed cities) capability forwarding + the spec's runtime-level interface are deferred to gc-qcsp9 — not needed for gascity (pure tmux, no wrapper). Stage C (consumer example + integration test) is next in the chain. Validation: CLI unit tests cover json/kv/text rendering, --include-raw, the 0/2/1 exit codes, unsupported-provider, and raw ANSI preservation; tmux adapter tests assert the capture-pane -e flags and delegation. go vet ./..., go build ./..., gofmt, the full tmux package, and existing peek/session tests all pass.
Stage C of convoy gc-8g41r (InputAreaState). Implements the spec's §7.2 live-tmux integration test: the one layer the pure fixture tests in input_area_test.go cannot cover. It proves that `tmux capture-pane -e` on the host's tmux build actually preserves the SGR sequences the ghost-text parser depends on, and that the full capture->parse->classify path yields the correct typed/ghost split on genuine wire bytes (not saved fixtures). No LLM binary is needed — a printf emits the prompt char plus a dim ghost suggestion. Conventions follow the package's existing build-tagged real-tmux tests (tmux_test.go): `//go:build integration`, an `hasTmux()` skip, and an isolated `-L` socket that is killed on cleanup (never the default tmux server or any live agent session). Non-obvious design point: the pane runs `bash --norc --noprofile` on purpose. The host operator's interactive shell frequently themes its prompt with the same ❯ glyph the Claude parser matches (Powerlevel10k does exactly this — observed live during development), which would leak into the capture and race the printf output for the bottom-up prompt scan. A hermetic shell guarantees the only ❯ in the pane is the one printf emits. Provider classification is forced to claude via SetEnvironment (GC_PROVIDER), which InputArea reads through show-environment, so the result is host-independent. Captures are polled rather than slept-on so the test absorbs pane-render latency without a magic delay. Validation: passes under `go test -tags integration` repeatedly (8x, no flake); `go vet -tags integration` clean; gofmt clean.
Record the Codex implementation review for the InputAreaState convoy after the CLI, consumer example, and live tmux integration stages landed on integration/input-area-state. The review evaluates the current branch against the sharpened post-stage-1b spec and calls out the remaining deltas before graduation: Codex arrow-prompt parsing, full dim-SGR coverage, and the missing shape_variant/raw_truncated wire fields. Validation run: go test ./internal/runtime/tmux -run 'InputArea|Raw'; go test ./cmd/gc -run 'InputArea|PeekRaw'; go test -tags=integration ./internal/runtime/tmux -run TestInputAreaLiveTmux -count=1.
…8g41r.7) Delta-fix for convoy gc-8g41r resolving the codex review (gc-8g41r.3, verdict needs-delta) plus the operator's 2026-06-15 "minimal core-level ghost detection" reframe. A leaning change — it deletes more than it adds. 1. Ghost detection = faint (codex finding #2). Replace the fragile gray-range heuristic in updateDimState (which only matched 240-243 vs the spec's 232-245, and missed ESC[39m as a terminator) with a single rule: any text wrapped in the faint SGR attribute (ESC[2m) is ghost; everything not faint is typed. Terminators are SGR 0/22/39. Extended color introducers (38/48 with 5;n or 2;r;g;b payloads) are skipped so a color index can never be misread as the standalone faint code. The rule is LLM-agnostic (no per-provider color table) and was empirically validated 2026-06-15 on live Claude sessions: ghost is ESC[2m-wrapped, typed is normal-weight, cursor position does not distinguish them. 2. Codex current arrow prompt (codex finding #1). parseCodexInputArea now recognizes the current "› " arrow shape alongside the older "│ > " box, binding bottom-up to the live prompt row of either shape. Arrow ghost reuses the shared faint helper — no Codex-specific ghost code. A "starts the line" guard keeps a "›" inside a queued-follow-up affordance from being read as the input area. 3. Drop the over-built surface (codex finding #3; operator decision — no consumers). Instead of adding the spec's promised shape_variant / raw_truncated fields, lean the contract down: remove the Raw field, the 16 KiB cap + truncation, gc session input-area --include-raw, and the entire gc session peek --raw plumbing (RawPeeker, PeekRaw, CapturePaneRaw/CapturePaneAllRaw, doSessionPeekRawFallback). Resulting struct: {Provider, PromptChar, Busy, Typed, Ghost, Detected}. The API SSE format=raw stream is untouched — it never used tmux.RawPeeker. 4. Spec + tests. engdocs/design/input-area-state.md updated to the lean contract (faint rule, dropped fields, Codex-arrow shape; status stays Proposed). Unit tests reworked around the faint rule (the "244 is not dim" case generalized; SGR 90/256-gray now classify as typed); added Codex arrow tests (typed, faint-ghost, queued-followup, bottom-up). The live tmux integration test needs no change (lean fields only). cli.md regenerated. raw_capture_test.go -> input_area_adapter_test.go (its PeekRaw tests removed; only the InputArea delegation test remains). Out of scope (host will file as a follow-up): migrating the scattered byte-comparison idle-detection consumers to this detector. Validation: go build ./..., go vet ./..., gofmt clean; full internal/runtime/tmux package, cmd/gc session/peek/input-area tests, the worker-boundary contract test, TestCLIDocsFreshness, and the live-tmux TestInputAreaLiveTmux integration test all pass.
…gc-8g41r.4) Stage 4 of convoy gc-8g41r. Reviews the current integration/input-area-state (post lean-fix gc-8g41r.7), not the original gray-range design. Verdict: approve — graduation-ready. The lean-fix resolves all three codex findings (gc-8g41r.3): Codex arrow shape now parsed, gray-range heuristic replaced by the LLM-agnostic faint=ghost rule, raw surface dropped per the no-consumers decision. Cross-checks each codex finding (agree on all three, including the two resolved by a better route than codex proposed) and adds three non-blocking follow-up notes (pre-prompt faint seeding, tmux-package layering for a future second provider, Codex/Gemini live-validation). Validation run against e0e90a2: build + vet clean; tmux InputArea/SplitDim/ StripANSI and cmd/gc InputArea unit tests pass; live-tmux TestInputAreaLiveTmux passes on tmux 3.6b (faint=ghost validated on real capture-pane -e bytes).
…oviderByName signature Rebasing gc-8g41r onto main surfaced upstream drift: buildSessionProviderByName gained a leading *config.City param (cmd_session_test.go / providers_test.go). The input-area test's provider mocks still used the old 4-arg shape. Widen the two mock closures to match; they ignore their args. Test-only — production already builds against the new signature.
bfcb4d8 to
ee31854
Compare
2849565 to
50a3822
Compare
e1b6f21 to
7bba3de
Compare
89e2e69 to
c057108
Compare
Graduation PR for convoy gc-8g41r — library-level, ghost-text-aware input-area detection (
InputAreaState).16 commits on
integration/input-area-state, 17 files / +3,852. Stages (all merged to the integration branch):gc-8g41r.2):InputAreaStateAPI + parser tests —internal/runtime/tmux/input_area.gogc-8g41r.5): CLI surface —gc session peek --raw+ input-areagc-8g41r.6): consumer (witness ghost-safe buffered-input detection) + live tmux integration testgc-8g41r.7): faint=ghost + Codex-arrow shape; dropped raw/shape_variant surfaceAgent reviews already ran on-branch (not via this PR): Codex (
gc-8g41r.3, 1e4f198) and Claude (gc-8g41r.4, 7a682ea); write-ups under.prd-reviews/input-area-state/andcode-review-*.md.Key files:
internal/runtime/tmux/input_area.go(481),cmd/gc/cmd_session.go(+161), tests (input_area_test.go,input_area_integration_test.go,cmd_session_input_area_test.go),engdocs/design/input-area-state.md(832), witness prompt template (+34).Intended squash-merge →
main;gc convoy land gc-8g41rfollows.