feat(coding-agent): senpi --neo launch handoff (neo-go-tui task 16)#115
feat(coding-agent): senpi --neo launch handoff (neo-go-tui task 16)#115code-yeongyu wants to merge 5 commits into
Conversation
Add --neo / --neo-isolated (+ hidden --neo-bin) to the CLI: resolve the per-platform Go TUI binary (SENPI_NEO_BIN -> --neo-bin -> require.resolve of @code-yeongyu/senpi-neo-tui-<platform>-<arch>), spawn it with inherited stdio, forward SIGINT/SIGTERM/SIGWINCH, and propagate exit code/signal. Dispatch sits after the version/export fast-paths and first-time setup, before any AgentSessionRuntime construction or extension loading, so the launcher stays a thin shim. Piped stdin falls back to classic print mode; --help stays classic. The platform/arch mapping table lives in cli/neo/platform.ts as the single source of truth shared with the packaging pipeline. senpi-qa cli-smoke: unknown-option probe moves from --neo (now real) to -zzz, and --help now asserts the --neo entry. Plan: .omo/plans/neo-go-tui.md
There was a problem hiding this comment.
4 issues found across 11 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/coding-agent/src/cli/neo/resolve-binary.ts">
<violation number="1" location="packages/coding-agent/src/cli/neo/resolve-binary.ts:68">
P0: `requireNeo(requirePath)` calls `require()` (module load) on a Go binary path, not `require.resolve()`. The binary is not a JS/JSON/Node addon module, so `require()` will always throw, making the whole package-resolution fallback chain fail unconditionally and produce a false "not installed" error.</violation>
</file>
<file name="packages/coding-agent/src/main.ts">
<violation number="1" location="packages/coding-agent/src/main.ts:620">
P3: The neo dispatch in main.ts awaits runNeoLauncher without a try-catch. While resolveNeoBinary returns clean error messages, the spawn call inside runNeoLauncher can still fail if the resolved binary isn't executable (EACCES) or if SENPI_NEO_BIN/--neo-bin points to a non-existent path (ENOENT). In those cases superviseNeoChild rejects the promise, producing an unhandled rejection with a stack trace — inconsistent with the design intent of stack-free actionable errors. Wrap the await in a try-catch to catch spawn failures and exit cleanly with code 1.</violation>
</file>
<file name="packages/coding-agent/src/cli/neo/launch.ts">
<violation number="1" location="packages/coding-agent/src/cli/neo/launch.ts:17">
P2: SIGWINCH cannot be sent via `child.kill()` on Windows; `process.kill()` and `child.kill()` only support SIGINT, SIGTERM, and SIGKILL on Windows. Calling `child.kill('SIGWINCH')` on Windows will throw (or be silently ignored), and the forward will fail. Registering the SIGWINCH listener on `process` is fine (Node delivers it on console resize), but forwarding it via kill to the child on Windows is unsafe.</violation>
</file>
<file name="packages/coding-agent/src/cli/neo/build-argv.ts">
<violation number="1" location="packages/coding-agent/src/cli/neo/build-argv.ts:19">
P2: Extension/custom CLI flags can be silently lost in `--neo` mode, so neo sessions may start with different behavior than the same command on the classic path. The launcher rebuilds argv from parsed fields but omits `parsed.unknownFlags`, which is the channel used to carry extension-registered flags. Including those unknown flags in the forwarded argv would keep neo handoff behavior aligned with existing extension flag semantics.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| const requirePath = neoBinaryRequirePath(target); | ||
| for (const requireNeo of requires) { | ||
| try { | ||
| return { ok: true, path: requireNeo(requirePath), source: "package" }; |
There was a problem hiding this comment.
P0: requireNeo(requirePath) calls require() (module load) on a Go binary path, not require.resolve(). The binary is not a JS/JSON/Node addon module, so require() will always throw, making the whole package-resolution fallback chain fail unconditionally and produce a false "not installed" error.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/coding-agent/src/cli/neo/resolve-binary.ts, line 68:
<comment>`requireNeo(requirePath)` calls `require()` (module load) on a Go binary path, not `require.resolve()`. The binary is not a JS/JSON/Node addon module, so `require()` will always throw, making the whole package-resolution fallback chain fail unconditionally and produce a false "not installed" error.</comment>
<file context>
@@ -0,0 +1,92 @@
+ const requirePath = neoBinaryRequirePath(target);
+ for (const requireNeo of requires) {
+ try {
+ return { ok: true, path: requireNeo(requirePath), source: "package" };
+ } catch {
+ // Try the next resolution root.
</file context>
| return { ok: true, path: requireNeo(requirePath), source: "package" }; | |
| return { ok: true, path: requireNeo.resolve(requirePath), source: "package" }; |
| import { resolveNeoBinary } from "./resolve-binary.ts"; | ||
|
|
||
| /** Signals forwarded from the launcher to the neo child while it runs. */ | ||
| const FORWARDED_SIGNALS = ["SIGINT", "SIGTERM", "SIGWINCH"] as const; |
There was a problem hiding this comment.
P2: SIGWINCH cannot be sent via child.kill() on Windows; process.kill() and child.kill() only support SIGINT, SIGTERM, and SIGKILL on Windows. Calling child.kill('SIGWINCH') on Windows will throw (or be silently ignored), and the forward will fail. Registering the SIGWINCH listener on process is fine (Node delivers it on console resize), but forwarding it via kill to the child on Windows is unsafe.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/coding-agent/src/cli/neo/launch.ts, line 17:
<comment>SIGWINCH cannot be sent via `child.kill()` on Windows; `process.kill()` and `child.kill()` only support SIGINT, SIGTERM, and SIGKILL on Windows. Calling `child.kill('SIGWINCH')` on Windows will throw (or be silently ignored), and the forward will fail. Registering the SIGWINCH listener on `process` is fine (Node delivers it on console resize), but forwarding it via kill to the child on Windows is unsafe.</comment>
<file context>
@@ -0,0 +1,84 @@
+import { resolveNeoBinary } from "./resolve-binary.ts";
+
+/** Signals forwarded from the launcher to the neo child while it runs. */
+const FORWARDED_SIGNALS = ["SIGINT", "SIGTERM", "SIGWINCH"] as const;
+type ForwardedSignal = (typeof FORWARDED_SIGNALS)[number];
+
</file context>
| readonly isolated: boolean; | ||
| } | ||
|
|
||
| export function buildNeoArgv(parsed: Args, options: BuildNeoArgvOptions): string[] { |
There was a problem hiding this comment.
P2: Extension/custom CLI flags can be silently lost in --neo mode, so neo sessions may start with different behavior than the same command on the classic path. The launcher rebuilds argv from parsed fields but omits parsed.unknownFlags, which is the channel used to carry extension-registered flags. Including those unknown flags in the forwarded argv would keep neo handoff behavior aligned with existing extension flag semantics.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/coding-agent/src/cli/neo/build-argv.ts, line 19:
<comment>Extension/custom CLI flags can be silently lost in `--neo` mode, so neo sessions may start with different behavior than the same command on the classic path. The launcher rebuilds argv from parsed fields but omits `parsed.unknownFlags`, which is the channel used to carry extension-registered flags. Including those unknown flags in the forwarded argv would keep neo handoff behavior aligned with existing extension flag semantics.</comment>
<file context>
@@ -0,0 +1,97 @@
+ readonly isolated: boolean;
+}
+
+export function buildNeoArgv(parsed: Args, options: BuildNeoArgvOptions): string[] {
+ const argv: string[] = [];
+
</file context>
| // - Piped stdin resolves appMode to "print", so a TTY-less --neo falls through | ||
| // to the classic print-mode path here instead of launching the TUI. | ||
| if (parsed.neo && !parsed.help && appMode === "interactive") { | ||
| const exitCode = await runNeoLauncher(parsed); |
There was a problem hiding this comment.
P3: The neo dispatch in main.ts awaits runNeoLauncher without a try-catch. While resolveNeoBinary returns clean error messages, the spawn call inside runNeoLauncher can still fail if the resolved binary isn't executable (EACCES) or if SENPI_NEO_BIN/--neo-bin points to a non-existent path (ENOENT). In those cases superviseNeoChild rejects the promise, producing an unhandled rejection with a stack trace — inconsistent with the design intent of stack-free actionable errors. Wrap the await in a try-catch to catch spawn failures and exit cleanly with code 1.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/coding-agent/src/main.ts, line 620:
<comment>The neo dispatch in main.ts awaits runNeoLauncher without a try-catch. While resolveNeoBinary returns clean error messages, the spawn call inside runNeoLauncher can still fail if the resolved binary isn't executable (EACCES) or if SENPI_NEO_BIN/--neo-bin points to a non-existent path (ENOENT). In those cases superviseNeoChild rejects the promise, producing an unhandled rejection with a stack trace — inconsistent with the design intent of stack-free actionable errors. Wrap the await in a try-catch to catch spawn failures and exit cleanly with code 1.</comment>
<file context>
@@ -607,6 +608,19 @@ export async function main(args: string[], options?: MainOptions) {
+ // - Piped stdin resolves appMode to "print", so a TTY-less --neo falls through
+ // to the classic print-mode path here instead of launching the TUI.
+ if (parsed.neo && !parsed.help && appMode === "interactive") {
+ const exitCode = await runNeoLauncher(parsed);
+ process.exit(exitCode);
+ }
</file context>
The pin (added in 44ff816 when the previous Rust neo-tui was removed) asserts --neo must not exist: help omits it, parseArgs errors on it, and main.ts never branches on parsed.neo. The neo-go-tui plan deliberately reintroduces --neo as the Go TUI launch flag, superseding the removal decision this test pinned. The new neo-args-parse/neo-argv/ neo-launch-dispatch/neo-resolve-binary suites pin the reintroduced behavior positively (65 tests), including the classic-path byte-identical guarantees the old pin cared about. Plan: .omo/plans/neo-go-tui.md
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 34569191 | Triggered | Generic High Entropy Secret | 5bd8364 | packages/neo/internal/store/auth_test.go | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
Summary
Task 16 of
.omo/plans/neo-go-tui.md:senpi --neonow hands the terminal off to the Go-native neo TUI binary. The launcher resolves the per-platform binary, spawns it with inherited stdio, forwards signals, and propagates the exit code — without constructing an AgentSessionRuntime or loading any extension in the launcher process. Without--neo, behavior is byte-identical (only--helpgains the--neo/--neo-isolatedentry).Changes
cli/args.ts:--neo,--neo-isolated, hidden--neo-bin; help entry. (--neowas previously a reserved explicit "Unknown option".)cli/neo/platform.ts(new): single-source platform/arch → package-name mapping shared with the task-18 packaging pipeline (win32→windows, x64/arm64 npm spellings).cli/neo/resolve-binary.ts(new): SENPI_NEO_BIN →--neo-bin→require.resolveover two roots (clipboard-native precedent); actionable errors, no stacks.cli/neo/build-argv.ts(new): full runtime-relevant flag passthrough; positional messages/@file args forwarded raw.cli/neo/launch.ts(new): spawn stdio-inherit, SIGINT/SIGTERM/SIGWINCH forwarding, exit-code + terminating-signal propagation.main.ts: early dispatch after first-time setup, before runtime-cwd/session-manager code; guards keep--helpand piped-stdin (print-mode) classic.cli-smoke.mjs: unknown-option probe--neo→-zzz; new--help lists --neoassertion.QA / Evidence
Evidence:
.omo/evidence/task-16-neo-go-tui.md(+ tmux captures) in the work worktree; verified independently by the orchestrator.senpi --neo→senpi-neo dev (github.com/code-yeongyu/senpi/packages/neo), exit 0.@code-yeongyu/senpi-neo-tui-darwin-arm64, exit 1, no stack trace.--neo→ classic print-mode fallback (stub not launched).--helpdiff vs pre-change baseline: ONLY the--neo/--neo-isolatedlines added.npm run checkexit 0; cli-smoke self-test 6/6; real~/.senpi/agent/auth.jsonunchanged.Risks
--neoruns — mitigated by the guard set (help/print-mode carve-outs) and the spy test asserting zero runtime construction/extension loading; non-neo paths are covered by the untouched 72-test args regression suite and the byte-identical help diff.--neoentries; it is documented in the--neohelp text and will be indocs/neo.md(task 22).Secret safety
No secrets/tokens in code, tests, or evidence; QA ran in isolated sandboxes with the real auth file hash asserted unchanged.
Summary by cubic
Adds
--neoand--neo-isolatedto launch the Go-native neo TUI, handing off the terminal to a per-platform binary with signal and exit-code propagation. Classic behavior is unchanged;--helplists the neo flags and piped stdin stays print mode.New Features
--neo,--neo-isolated(hidden dev override--neo-bin).SENPI_NEO_BIN→--neo-bin→require.resolveof@code-yeongyu/senpi-neo-tui-<platform>-<arch>(darwin/linux/windows on x64/arm64); actionable error when missing.--helpand non-tty stdin stay classic.@fileargs raw;--neo-isolatedmaps to--isolatedfor the Go binary.--helplists--neoand switches unknown-option probe to-zzz.Migration
npm i -g @code-yeongyu/senpi-neo-tui-<platform>-<arch>) or setSENPI_NEO_BIN.Written for commit 2635ff5. Summary will update on new commits.