diff --git a/.context/TASKS.md b/.context/TASKS.md index be89c3d1..574497f5 100644 --- a/.context/TASKS.md +++ b/.context/TASKS.md @@ -173,9 +173,16 @@ These have priority because other knowledge ingestion projects depend on them. feat/pad-undo-snapshot #commit:b9ce72e8 #added:2026-05-27-145715 -- [ ] `ctx system`: emit a VERBATIM RELAY on unknown subcommand (replace today's +- [x] `ctx system`: emit a VERBATIM RELAY on unknown subcommand (replace today's silent help-dump + exit 0). Scope: - `ctx system` ONLY. + `ctx system` ONLY. #completed:2026-05-28 #branch:feat/system-unknown-relay + Shipped: `ctx system ` now emits a verbatim NudgeBox (via the write + layer) naming the verb + version-skew hint, best-effort fires the event-log + + webhook relay (gated on a session ID read TTY-safely from stdin), suppresses + cobra's help dump, and exits non-zero. Bare `ctx system` and valid subcommands + unchanged. Handler in internal/cli/system/core/unknown (RunE on system.Cmd() + only; parent.Cmd untouched). Verified end-to-end against a real build (box + + EXIT=1). Spec: specs/system-unknown-subcommand-relay.md. - Problem: `ctx system ` prints the full Long help and exits 0 ( cobra `legacyArgs` only raises "unknown command" for the ROOT command, never a non-root group). In a @@ -201,6 +208,29 @@ These have priority because other knowledge ingestion projects depend on them. its own spec before implementation. #priority:medium #session:96765858 #branch:feat/pad-undo-snapshot #commit: b9ce72e8 #added:2026-05-27-130130 + - DONE 2026-05-28 (branch feat/system-unknown-relay, session 0066d49b). + Spec: specs/system-unknown-subcommand-relay.md. + Approach used: add a RunE on system.Cmd() only (legacyArgs lets the + leftover args reach the group's RunE for non-root); on unknown verb emit a + message.NudgeBox to stdout, set SilenceUsage (else cobra re-dumps the help + we're killing), exit non-zero. system is Hidden so RootCmd PersistentPreRunE + early-returns — no context/git preconditions. + Decisions settled with user: (1) DO fire the event-log + webhook relay leg + (nudge.Relay), gated on a real session ID read best-effort from stdin via + session.ReadID (TTY-safe, timeout-guarded → IDUnknown means skip the leg); + (2) scoped to ctx system only, parent.Cmd untouched. + Follow-up surfaced: ctx hook (and any parent.Cmd group) has the same latent + exit-0-on-unknown behavior — not wired into hooks.json so out of scope here; + capture as its own task if it ever gets hook-wired. + +- [ ] Generalize the unknown-subcommand guard beyond `ctx system` (deferred from + the #5 work above). `ctx hook` and any future `parent.Cmd` group still print + help + exit 0 on an unknown subcommand — the same latent pollution #5 fixed for + `ctx system`. Low priority while no other group is wired into hooks.json; the + build-time wiring guard (specs/hooks-wiring-guard.md) only checks `ctx system` + + `ctx agent` today. If a `ctx hook ` ever gets hook-wired, either extend + the guard's coverage or fold a reusable opt-in into `parent.Cmd` (an optional + unknown-subcommand handler groups opt into). #priority:low #added:2026-05-28 ## Important diff --git a/internal/assets/commands/text/errors.yaml b/internal/assets/commands/text/errors.yaml index 9db41f00..88d77dca 100644 --- a/internal/assets/commands/text/errors.yaml +++ b/internal/assets/commands/text/errors.yaml @@ -68,6 +68,8 @@ err.schema.drift: short: 'schema drift detected' err.cli.no-tool-specified: short: 'no tool specified: use --tool or set the tool field in .ctxrc' +err.cli.unknown-subcommand: + short: 'unknown ctx system subcommand "%s"' err.config.golden-not-found: short: "no .claude/settings.golden.json found - run 'ctx permission snapshot' first" err.config.invalid-tool: diff --git a/internal/assets/commands/text/hooks.yaml b/internal/assets/commands/text/hooks.yaml index 5d8c0b2b..4a4b7c81 100644 --- a/internal/assets/commands/text/hooks.yaml +++ b/internal/assets/commands/text/hooks.yaml @@ -564,3 +564,14 @@ specs-nudge.nudge-message: short: plan-to-specs nudge emitted version-drift.relay-message: short: versions out of sync +system-unknown.relay-prefix: + short: 'IMPORTANT: Relay this notice to the user VERBATIM before answering their question.' +system-unknown.box-title: + short: Unknown System Subcommand +system-unknown.body: + short: |- + ctx system: unknown subcommand "%s". + + A Claude Code hook (hooks.json) is calling a ctx command this binary no longer ships — a version skew between the installed plugin and the on-PATH ctx binary. Align the plugin and binary to the same release, or fix the hook command. +system-unknown.relay-message: + short: 'ctx system: unknown subcommand "%s" (likely plugin/binary version skew)' diff --git a/internal/cli/system/core/unknown/doc.go b/internal/cli/system/core/unknown/doc.go new file mode 100644 index 00000000..1a29ac94 --- /dev/null +++ b/internal/cli/system/core/unknown/doc.go @@ -0,0 +1,35 @@ +// / ctx: https://ctx.ist +// ,'`./ do you remember? +// `.,'\ +// \ Copyright 2026-present Context contributors. +// SPDX-License-Identifier: Apache-2.0 + +// Package unknown holds the RunE the `ctx system` group installs so +// that an unrecognised subcommand fails loud and legible instead of +// dumping help at exit 0. +// +// # Why this exists +// +// `ctx system` is a grouping command. Cobra raises an +// "unknown command" error only for the *root*; for a non-root group +// an unmatched subcommand falls through to the group's own Run/RunE, +// and a group with neither prints help and returns nil — exit 0. In +// a Claude Code UserPromptSubmit hook, exit 0 reads as "hook +// success", so the ~51-line help blob is injected into the agent's +// context every prompt. That is exactly how a stale `hooks.json` +// wiring `ctx system check-anchor-drift` (a command the binary later +// deleted) polluted sessions silently. +// +// A non-zero exit alone does not fix it: the harness swallows a +// failed hook's exit code, so the signal has to travel on hook +// stdout. [Handler] therefore emits a verbatim-relay box naming the +// unknown verb and hinting at the likely cause (plugin/binary +// version skew), best-effort records the event (event log + webhook) +// when a session is present on stdin, suppresses cobra's help dump, +// and returns a non-nil error. +// +// Scope is `ctx system` only — the single group wired into +// hooks.json. The shared [parent.Cmd] is untouched; other groups +// keep cobra's default behavior. See +// specs/system-unknown-subcommand-relay.md. +package unknown diff --git a/internal/cli/system/core/unknown/handle.go b/internal/cli/system/core/unknown/handle.go new file mode 100644 index 00000000..829c2ad2 --- /dev/null +++ b/internal/cli/system/core/unknown/handle.go @@ -0,0 +1,82 @@ +// / ctx: https://ctx.ist +// ,'`./ do you remember? +// `.,'\ +// \ Copyright 2026-present Context contributors. +// SPDX-License-Identifier: Apache-2.0 + +package unknown + +import ( + "fmt" + "os" + + "github.com/spf13/cobra" + + "github.com/ActiveMemory/ctx/internal/assets/read/desc" + "github.com/ActiveMemory/ctx/internal/cli/system/core/message" + "github.com/ActiveMemory/ctx/internal/cli/system/core/nudge" + coreSession "github.com/ActiveMemory/ctx/internal/cli/system/core/session" + "github.com/ActiveMemory/ctx/internal/config/embed/text" + "github.com/ActiveMemory/ctx/internal/config/hook" + cfgSession "github.com/ActiveMemory/ctx/internal/config/session" + "github.com/ActiveMemory/ctx/internal/config/warn" + "github.com/ActiveMemory/ctx/internal/entity" + errCli "github.com/ActiveMemory/ctx/internal/err/cli" + logWarn "github.com/ActiveMemory/ctx/internal/log/warn" + writeSetup "github.com/ActiveMemory/ctx/internal/write/setup" +) + +// relay is the event-log + webhook relay leg, indirected through a +// package variable so tests can observe the call without a live +// webhook or an initialized context. Production binds it to +// [nudge.Relay]. +var relay = nudge.Relay + +// handle is [Handler] with the stdin source injected for testability. +// Bare invocation prints help; an unknown verb emits the verbatim +// relay box (through the write layer), best-effort records the relay +// event when a session is present, suppresses cobra's help dump, and +// returns the unknown-subcommand error. +// +// Parameters: +// - cmd: the system command (for output and SilenceUsage) +// - args: leftover args; non-empty means an unknown subcommand +// - stdin: hook-input source; ReadID is TTY-safe and timeout-guarded +// +// Returns: +// - error: nil for a bare `ctx system` (help printed); otherwise the +// unknown-subcommand error from [errCli.UnknownSubcommand]. +func handle(cmd *cobra.Command, args []string, stdin *os.File) error { + if len(args) == 0 { + // Bare `ctx system`: preserve help + exit 0. + return cmd.Help() + } + verb := args[0] + + prefix := desc.Text(text.DescKeySystemUnknownRelayPrefix) + title := desc.Text(text.DescKeySystemUnknownBoxTitle) + body := fmt.Sprintf(desc.Text(text.DescKeySystemUnknownBody), verb) + writeSetup.Nudge(cmd, message.NudgeBox(prefix, title, body)) + + // Best-effort relay leg: only when a hook supplied a session on + // stdin. ReadID is TTY-safe and timeout-guarded, so a manual typo + // at a terminal returns IDUnknown without blocking. A relay + // failure is logged, not returned: the stdout box already reached + // the agent, and the user's real problem is the unknown verb. + if sid := coreSession.ReadID(stdin); sid != cfgSession.IDUnknown { + msg := fmt.Sprintf( + desc.Text(text.DescKeySystemUnknownRelayMessage), verb, + ) + ref := entity.NewTemplateRef( + hook.System, hook.VariantUnknownSubcommand, nil, + ) + if relayErr := relay(msg, sid, ref); relayErr != nil { + logWarn.Warn(warn.RelayUnknownSubcommand, relayErr) + } + } + + // Suppress cobra's help dump on error — that dump is the very + // pollution this handler exists to kill. + cmd.SilenceUsage = true + return errCli.UnknownSubcommand(verb) +} diff --git a/internal/cli/system/core/unknown/testmain_test.go b/internal/cli/system/core/unknown/testmain_test.go new file mode 100644 index 00000000..43f06ed2 --- /dev/null +++ b/internal/cli/system/core/unknown/testmain_test.go @@ -0,0 +1,21 @@ +// / ctx: https://ctx.ist +// ,'`./ do you remember? +// `.,'\ +// \ Copyright 2026-present Context contributors. +// SPDX-License-Identifier: Apache-2.0 + +package unknown + +import ( + "os" + "testing" + + "github.com/ActiveMemory/ctx/internal/assets/read/lookup" +) + +// TestMain loads the embedded description maps before running tests; +// desc.Text returns empty strings until lookup.Init has run. +func TestMain(m *testing.M) { + lookup.Init() + os.Exit(m.Run()) +} diff --git a/internal/cli/system/core/unknown/unknown.go b/internal/cli/system/core/unknown/unknown.go new file mode 100644 index 00000000..8d6c4e7e --- /dev/null +++ b/internal/cli/system/core/unknown/unknown.go @@ -0,0 +1,29 @@ +// / ctx: https://ctx.ist +// ,'`./ do you remember? +// `.,'\ +// \ Copyright 2026-present Context contributors. +// SPDX-License-Identifier: Apache-2.0 + +package unknown + +import ( + "os" + + "github.com/spf13/cobra" +) + +// Handler is the RunE installed on the `ctx system` group. It is +// reached only when cobra finds no matching subcommand (or for a +// bare `ctx system`); a valid subcommand runs its own RunE and never +// reaches here. It delegates to [handle] with the real stdin. +// +// Parameters: +// - cmd: the system command (for output and SilenceUsage) +// - args: leftover args; non-empty means an unknown subcommand +// +// Returns: +// - error: nil for bare `ctx system` (help printed); otherwise the +// unknown-subcommand error after emitting the relay box. +func Handler(cmd *cobra.Command, args []string) error { + return handle(cmd, args, os.Stdin) +} diff --git a/internal/cli/system/core/unknown/unknown_test.go b/internal/cli/system/core/unknown/unknown_test.go new file mode 100644 index 00000000..21a62e51 --- /dev/null +++ b/internal/cli/system/core/unknown/unknown_test.go @@ -0,0 +1,161 @@ +// / ctx: https://ctx.ist +// ,'`./ do you remember? +// `.,'\ +// \ Copyright 2026-present Context contributors. +// SPDX-License-Identifier: Apache-2.0 + +package unknown + +import ( + "bytes" + "errors" + "io" + "os" + "strings" + "testing" + + "github.com/spf13/cobra" + + "github.com/ActiveMemory/ctx/internal/config/hook" + "github.com/ActiveMemory/ctx/internal/entity" +) + +// stubRelay swaps the package relay seam for the duration of a test +// and returns a restore func. +func stubRelay( + fn func(string, string, *entity.TemplateRef) error, +) func() { + prev := relay + relay = fn + return func() { relay = prev } +} + +// tempStdin writes content to a temp file and rewinds it, yielding an +// *os.File usable as injected stdin. Empty content models a stream +// with no hook JSON (session resolves to IDUnknown). +func tempStdin(t *testing.T, content string) *os.File { + t.Helper() + f, err := os.CreateTemp(t.TempDir(), "stdin-*.json") + if err != nil { + t.Fatalf("create temp stdin: %v", err) + } + t.Cleanup(func() { _ = f.Close() }) + if content != "" { + if _, err := f.WriteString(content); err != nil { + t.Fatalf("write temp stdin: %v", err) + } + } + if _, err := f.Seek(0, io.SeekStart); err != nil { + t.Fatalf("rewind temp stdin: %v", err) + } + return f +} + +func TestHandleUnknownEmitsBoxSilencesUsageAndErrors(t *testing.T) { + cmd := &cobra.Command{Use: "system"} + var out bytes.Buffer + cmd.SetOut(&out) + + called := false + defer stubRelay(func(string, string, *entity.TemplateRef) error { + called = true + return nil + })() + + // Empty stdin → no session → relay leg must be skipped. + err := handle(cmd, []string{"check-anchor-drift"}, tempStdin(t, "")) + if err == nil { + t.Fatal("want non-nil error for an unknown subcommand") + } + if !strings.Contains(err.Error(), "check-anchor-drift") { + t.Errorf("error should name the verb; got %v", err) + } + + got := out.String() + for _, want := range []string{ + "check-anchor-drift", "version skew", "VERBATIM", + } { + if !strings.Contains(got, want) { + t.Errorf("relay box missing %q\n---\n%s", want, got) + } + } + if !cmd.SilenceUsage { + t.Error("want SilenceUsage=true so cobra does not re-dump help") + } + if called { + t.Error("relay must be skipped when stdin carries no session") + } +} + +func TestHandleUnknownFiresRelayWithSession(t *testing.T) { + cmd := &cobra.Command{Use: "system"} + cmd.SetOut(&bytes.Buffer{}) + + var gotMsg, gotSID string + var gotRef *entity.TemplateRef + defer stubRelay( + func(msg, sid string, ref *entity.TemplateRef) error { + gotMsg, gotSID, gotRef = msg, sid, ref + return nil + }, + )() + + err := handle( + cmd, []string{"check-anchor-drift"}, + tempStdin(t, `{"session_id":"sess-9"}`), + ) + if err == nil { + t.Fatal("want non-nil error") + } + if gotSID != "sess-9" { + t.Errorf("relay session = %q, want sess-9", gotSID) + } + if !strings.Contains(gotMsg, "check-anchor-drift") { + t.Errorf("relay msg %q should name the verb", gotMsg) + } + if gotRef == nil || + gotRef.Variant != hook.VariantUnknownSubcommand || + gotRef.Hook != hook.System { + t.Errorf("relay ref = %+v, want hook=%q variant=%q", + gotRef, hook.System, hook.VariantUnknownSubcommand) + } +} + +func TestHandleRelayFailureDoesNotMaskError(t *testing.T) { + cmd := &cobra.Command{Use: "system"} + cmd.SetOut(&bytes.Buffer{}) + defer stubRelay(func(string, string, *entity.TemplateRef) error { + return errors.New("relay boom") + })() + + err := handle(cmd, []string{"x"}, tempStdin(t, `{"session_id":"s"}`)) + if err == nil || !strings.Contains(err.Error(), "x") { + t.Fatalf("want unknown-subcommand error naming x, got %v", err) + } +} + +func TestHandleBareReturnsNilNoRelay(t *testing.T) { + cmd := &cobra.Command{Use: "system"} + cmd.SetOut(&bytes.Buffer{}) + cmd.SetErr(&bytes.Buffer{}) + + called := false + defer stubRelay(func(string, string, *entity.TemplateRef) error { + called = true + return nil + })() + + // Bare `ctx system` (no leftover args) prints help and exits 0; + // the help-output itself is covered by the system_test integration + // test against the real command. Here: no error, no relay, no + // SilenceUsage flip. + if err := handle(cmd, nil, tempStdin(t, `{"session_id":"s"}`)); err != nil { + t.Fatalf("bare `ctx system`: want nil error, got %v", err) + } + if called { + t.Error("bare `ctx system` must not fire the relay") + } + if cmd.SilenceUsage { + t.Error("bare `ctx system` must not set SilenceUsage") + } +} diff --git a/internal/cli/system/system.go b/internal/cli/system/system.go index 8389d3c7..b2534687 100644 --- a/internal/cli/system/system.go +++ b/internal/cli/system/system.go @@ -36,6 +36,7 @@ import ( "github.com/ActiveMemory/ctx/internal/cli/system/cmd/resume" sessEvent "github.com/ActiveMemory/ctx/internal/cli/system/cmd/sessionevent" "github.com/ActiveMemory/ctx/internal/cli/system/cmd/specsnudge" + "github.com/ActiveMemory/ctx/internal/cli/system/core/unknown" "github.com/ActiveMemory/ctx/internal/config/embed/cmd" ) @@ -54,7 +55,7 @@ import ( // Returns: // - *cobra.Command: Parent command with hook plumbing subcommands func Cmd() *cobra.Command { - return parent.Cmd(cmd.DescKeySystem, cmd.UseSystem, + c := parent.Cmd(cmd.DescKeySystem, cmd.UseSystem, sysBootstrap.Cmd(), blocknonpathctx.Cmd(), checkceremony.Cmd(), @@ -82,4 +83,11 @@ func Cmd() *cobra.Command { sessEvent.Cmd(), specsnudge.Cmd(), ) + // An unknown `ctx system ` must fail loud — emit a verbatim + // relay and exit non-zero — instead of dumping help at exit 0, + // which a UserPromptSubmit hook reads as success and injects every + // prompt. Scoped here only; the shared parent.Cmd stays untouched. + // See specs/system-unknown-subcommand-relay.md. + c.RunE = unknown.Handler + return c } diff --git a/internal/cli/system/system_test.go b/internal/cli/system/system_test.go new file mode 100644 index 00000000..ce82e695 --- /dev/null +++ b/internal/cli/system/system_test.go @@ -0,0 +1,91 @@ +// / ctx: https://ctx.ist +// ,'`./ do you remember? +// `.,'\ +// \ Copyright 2026-present Context contributors. +// SPDX-License-Identifier: Apache-2.0 + +package system_test + +import ( + "bytes" + "strings" + "testing" + + "github.com/spf13/cobra" + + "github.com/ActiveMemory/ctx/internal/cli/parent" + "github.com/ActiveMemory/ctx/internal/cli/system" + embedCmd "github.com/ActiveMemory/ctx/internal/config/embed/cmd" +) + +// newRoot wraps a command in a minimal parent so the wrapped group +// reports HasParent()==true — the condition under which cobra lets an +// unmatched subcommand reach the group's RunE (rather than raising +// "unknown command" as it does only for the root). +func newRoot(child *cobra.Command) (*cobra.Command, *bytes.Buffer) { + root := &cobra.Command{Use: "ctx"} + var out bytes.Buffer + root.SetOut(&out) + root.SetErr(&out) + root.AddCommand(child) + return root, &out +} + +func TestSystemUnknownSubcommandFailsLoud(t *testing.T) { + root, out := newRoot(system.Cmd()) + root.SetArgs([]string{"system", "no-such-verb"}) + + if err := root.Execute(); err == nil { + t.Fatal("want non-nil error for an unknown `ctx system` subcommand") + } + got := out.String() + if !strings.Contains(got, "no-such-verb") { + t.Errorf("want a relay box naming the verb; got:\n%s", got) + } + // The whole point: no ~51-line Long-help dump on the unknown path. + if strings.Contains(got, "Available Commands:") { + t.Errorf("unknown subcommand must not dump help; got:\n%s", got) + } +} + +func TestSystemBareStillPrintsHelp(t *testing.T) { + root, out := newRoot(system.Cmd()) + root.SetArgs([]string{"system"}) + + if err := root.Execute(); err != nil { + t.Fatalf("bare `ctx system`: want nil error (help), got %v", err) + } + if out.Len() == 0 { + t.Error("bare `ctx system` should print help") + } +} + +// TestParentCmdScopeUnchanged documents that the fix is scoped to the +// system group only: the shared parent.Cmd still produces a group +// with no RunE, so other groups keep cobra's default (help + exit 0) +// on an unknown subcommand. If someone moves the fix into parent.Cmd, +// this fails. +func TestParentCmdScopeUnchanged(t *testing.T) { + if system.Cmd().RunE == nil { + t.Fatal("system.Cmd() must set a RunE (the unknown-subcommand fix)") + } + + grp := parent.Cmd(embedCmd.DescKeySystem, "grouplike", + &cobra.Command{ + Use: "real", + RunE: func(*cobra.Command, []string) error { return nil }, + }, + ) + if grp.RunE != nil { + t.Error("parent.Cmd must not set RunE; fix is scoped to system") + } + + root, _ := newRoot(grp) + root.SetArgs([]string{"grouplike", "bogus"}) + if err := root.Execute(); err != nil { + t.Errorf( + "a parent.Cmd group should exit 0 (help) on an unknown "+ + "subcommand, got %v", err, + ) + } +} diff --git a/internal/cli/system/testmain_test.go b/internal/cli/system/testmain_test.go new file mode 100644 index 00000000..62475fee --- /dev/null +++ b/internal/cli/system/testmain_test.go @@ -0,0 +1,22 @@ +// / ctx: https://ctx.ist +// ,'`./ do you remember? +// `.,'\ +// \ Copyright 2026-present Context contributors. +// SPDX-License-Identifier: Apache-2.0 + +package system_test + +import ( + "os" + "testing" + + "github.com/ActiveMemory/ctx/internal/assets/read/lookup" +) + +// TestMain loads the embedded description maps so the system command +// tree renders real Short/Long text (desc lookups return empty until +// lookup.Init has run). +func TestMain(m *testing.M) { + lookup.Init() + os.Exit(m.Run()) +} diff --git a/internal/config/embed/text/err_cli.go b/internal/config/embed/text/err_cli.go index ea7e9181..e1d01266 100644 --- a/internal/config/embed/text/err_cli.go +++ b/internal/config/embed/text/err_cli.go @@ -11,4 +11,8 @@ const ( // DescKeyErrCliNoToolSpecified is the text key for err cli no tool specified // messages. DescKeyErrCliNoToolSpecified = "err.cli.no-tool-specified" + // DescKeyErrCliUnknownSubcommand is the text key for the terse + // error returned when `ctx system` receives an unrecognised + // subcommand (the rich guidance goes in the stdout relay box). + DescKeyErrCliUnknownSubcommand = "err.cli.unknown-subcommand" ) diff --git a/internal/config/embed/text/system_unknown.go b/internal/config/embed/text/system_unknown.go new file mode 100644 index 00000000..68abd224 --- /dev/null +++ b/internal/config/embed/text/system_unknown.go @@ -0,0 +1,25 @@ +// / ctx: https://ctx.ist +// ,'`./ do you remember? +// `.,'\ +// \ Copyright 2026-present Context contributors. +// SPDX-License-Identifier: Apache-2.0 + +package text + +// DescKeys for the `ctx system` unknown-subcommand verbatim relay. +const ( + // DescKeySystemUnknownRelayPrefix is the text key for the + // "relay this verbatim" prefix above the unknown-subcommand box. + DescKeySystemUnknownRelayPrefix = "system-unknown.relay-prefix" + // DescKeySystemUnknownBoxTitle is the text key for the + // unknown-subcommand box title. + DescKeySystemUnknownBoxTitle = "system-unknown.box-title" + // DescKeySystemUnknownBody is the text key for the + // unknown-subcommand box body. It is a format string taking the + // unrecognised subcommand name. + DescKeySystemUnknownBody = "system-unknown.body" + // DescKeySystemUnknownRelayMessage is the text key for the + // human-readable relay-event description recorded in the event + // log / webhook. Format string taking the subcommand name. + DescKeySystemUnknownRelayMessage = "system-unknown.relay-message" +) diff --git a/internal/config/hook/hook.go b/internal/config/hook/hook.go index d80be801..969798d8 100644 --- a/internal/config/hook/hook.go +++ b/internal/config/hook/hook.go @@ -49,6 +49,10 @@ const ( SpecsNudge = "specs-nudge" // VersionDrift is the hook name for version drift nudges. VersionDrift = "version-drift" + // System is the relay-event hook label for the `ctx system` + // group itself (used by the unknown-subcommand relay, which is + // not a check-* hook but still records a relay event). + System = "system" ) // Supported integration tool names for ctx setup command. diff --git a/internal/config/hook/variant.go b/internal/config/hook/variant.go index 84f45137..40e4f2be 100644 --- a/internal/config/hook/variant.go +++ b/internal/config/hook/variant.go @@ -51,4 +51,7 @@ const ( VariantStale = "stale" // VariantWindow selects the context window variant. VariantWindow = "window" + // VariantUnknownSubcommand tags the relay event emitted when + // `ctx system` is handed a subcommand it does not recognise. + VariantUnknownSubcommand = "unknown-subcommand" ) diff --git a/internal/config/warn/warn.go b/internal/config/warn/warn.go index 52bf705a..7a68c91a 100644 --- a/internal/config/warn/warn.go +++ b/internal/config/warn/warn.go @@ -44,6 +44,12 @@ const ( // CopilotClose is the format for Copilot CLI file close failures. CopilotClose = "copilot-cli: close %s: %v" + // RelayUnknownSubcommand is the format for a best-effort relay + // failure when `ctx system` reports an unknown subcommand. The + // stdout box already reached the agent; this only logs that the + // event-log/webhook leg could not be recorded. + RelayUnknownSubcommand = "relay unknown-subcommand event: %v" + // JSONEncode is the JSON-safe error for encoding failures. JSONEncode = `{"error": "json encode: %v"}` diff --git a/internal/err/cli/cli.go b/internal/err/cli/cli.go index 25f41d3c..3c82834c 100644 --- a/internal/err/cli/cli.go +++ b/internal/err/cli/cli.go @@ -111,3 +111,20 @@ func FlagPlaceholder(name, value string) error { name, value, ) } + +// UnknownSubcommand returns the terse error for an unrecognised +// `ctx system` subcommand. The rich, user-facing guidance (likely +// plugin/binary version skew) is emitted separately as a stdout +// relay box; this is the stderr companion that names the verb so +// the cause is legible in logs without the help dump. +// +// Parameters: +// - verb: the unrecognised subcommand name +// +// Returns: +// - error: "unknown ctx system subcommand \"\"" +func UnknownSubcommand(verb string) error { + return fmt.Errorf( + desc.Text(text.DescKeyErrCliUnknownSubcommand), verb, + ) +} diff --git a/specs/system-unknown-subcommand-relay.md b/specs/system-unknown-subcommand-relay.md new file mode 100644 index 00000000..ea7cc2cf --- /dev/null +++ b/specs/system-unknown-subcommand-relay.md @@ -0,0 +1,278 @@ +# `ctx system` Unknown-Subcommand Verbatim Relay + +## Problem + +`ctx system ` prints the `system` group's ~51-line Long +help and exits **0**. Cobra's default `legacyArgs` raises an +"unknown command" error only for the *root* command +(`!cmd.HasParent()`); for a non-root grouping command like +`ctx system`, leftover args fall through and — because the group +has no `Run`/`RunE` — cobra calls `Help()` and returns `nil`. + +In a Claude Code `UserPromptSubmit` hook, exit 0 is read by the +harness as "hook success", so the entire help blob is injected +into the agent's context **on every prompt**. This is exactly the +failure mode that the version-skew bug rode in on: a stale +`hooks.json` wired `ctx system check-anchor-drift` after the +binary deleted that command, and instead of a visible error the +agent silently ate 51 lines of help each turn (see +`specs/experiments/acdl-session-start.md` §Root Cause, Bug #2). + +A non-zero exit code **alone** does not fix this: in a +`UserPromptSubmit` hook a non-zero exit with help suppressed is +swallowed by the harness — the user/agent sees nothing. The signal +has to travel on the channel the harness actually surfaces: hook +**stdout**. So the fix must *emit a message* (not just fail) when +`ctx system` is handed a verb it does not recognise. + +The companion build-time guard already shipped +(`specs/hooks-wiring-guard.md`, +`TestShippedHooksResolveToRegisteredCommands`): it stops *our own* +package from shipping a hooks.json that names a missing verb. This +spec covers the **runtime** half — making a skew that reaches a +user's machine (old plugin, new binary, or vice versa) fail +*loud and legible* instead of silent. + +## Approach + +Give the `ctx system` command — and only that command — a `RunE` +that intercepts the unknown-subcommand case: + +- **Bare `ctx system`** (no args): print help, exit 0. Unchanged + behavior. +- **`ctx system ` where `` is a real subcommand**: + cobra descends into it and runs *its* `RunE`; `system`'s own + `RunE` is never reached. Unchanged behavior. +- **`ctx system `**: cobra finds no matching subcommand, + so the leftover args reach `system`'s `RunE`. The handler emits a + verbatim-relay box to stdout naming the unknown verb and hinting + at the likely cause (plugin/binary version skew), sets + `SilenceUsage`, and returns a non-nil error → exit non-zero. When + a real session ID is present on stdin (i.e. the command was run + by a hook), it *also* records a relay event — event-log append + then webhook — via the same `nudge.Relay` path the `check-*` + hooks use, so the skew is captured for out-of-band/autonomous + alerting, not just the live session. + +The behavior attaches to `system.Cmd()` after it is built by the +shared `parent.Cmd(...)`. The shared `parent.Cmd` is **not** +changed: every other group (`ctx hook`, …) keeps cobra's current +default. Scope is `ctx system` only, because that is the only group +wired into `hooks.json` and therefore the only one whose skew +pollutes a session. + +Why `RunE` and not `Args: cobra.NoArgs`: `NoArgs` would surface +cobra's *generic* "unknown command" error on stderr and still emit +nothing to stdout — the harness would swallow it just like a bare +non-zero exit. We need a controlled message on stdout, which a +`RunE` gives us. + +`ctx system` is registered `Hidden` (group.go `hiddenCmds`), so +`RootCmd`'s `PersistentPreRunE` early-returns for it +(`if cmd.Hidden { return nil }`) — adding a `RunE` does **not** +subject `ctx system` to the context-dir / init / git +preconditions. Verify this holds during implementation. + +## Behavior + +### Happy Path + +1. A stale `hooks.json` (or a typo) runs + `ctx system check-anchor-drift`. +2. Cobra finds no `check-anchor-drift` subcommand; the arg falls + through to `system`'s `RunE` as `args = ["check-anchor-drift"]`. +3. The handler renders a verbatim-relay box to **stdout**: + - names the unknown subcommand, + - states the likely cause (a hook referencing a command this + binary no longer ships — version skew between the installed + plugin's `hooks.json` and the on-PATH `ctx` binary), + - gives the recovery (align plugin and binary to the same + release). +4. Best-effort: the handler reads the session ID from stdin + (`session.ReadID`, which is TTY-safe and timeout-guarded). If a + real session ID is present, it calls `nudge.Relay` to append a + relay event to the local event log and fire the relay webhook + (log-first: webhook only on a successful append). +5. The handler sets `cmd.SilenceUsage = true` and returns a + non-nil error. +6. The harness surfaces the stdout box to the user/agent; the + non-zero exit makes the failure debuggable instead of disguised + as success; the event-log/webhook record gives out-of-band + visibility. No 51-line help dump. + +### Edge Cases + +| Case | Expected behavior | +|------|-------------------| +| Bare `ctx system` (no args) | Print help, exit 0. Unchanged. | +| Valid hidden subcommand (`ctx system bootstrap`) | Routes to the subcommand; `system`'s `RunE` not invoked. Unchanged. | +| Valid subcommand + bad flag (`ctx system heartbeat --nope`) | Cobra's flag parsing on the *subcommand* errors as today; `system`'s `RunE` not involved. | +| Multiple leftover args (`ctx system foo bar`) | Treated as unknown; relay names the first token (`foo`). Exit non-zero. | +| Unknown verb that is a prefix of a real one (`ctx system check`) | No exact match → unknown; relay names `check`. (Cobra prefix-matching is not enabled.) | +| `ctx system --help` | Cobra's help flag short-circuits before `RunE`; prints help, exit 0. Unchanged. | +| Manual `ctx system typo` at a terminal (stdin is a TTY) | `session.ReadID` detects the char device and returns `IDUnknown` immediately — **no hang**. Stdout box + exit non-zero; relay leg (event log + webhook) is skipped (no session context to attribute it to). | +| Hook stdin present but blank/garbage session JSON | `ReadID` returns `IDUnknown` within the 2s timeout; relay leg skipped; stdout box + exit non-zero still happen. | +| Hook runs the dead command under a non-`UserPromptSubmit` event | Stdout may not be injected for every event, but the non-zero exit + stderr error + event-log/webhook relay still beat a silent exit-0 help dump; no regression. | + +### Validation Rules + +The "unknown" determination is cobra's, not ours: if `system`'s +`RunE` runs with `len(args) > 0`, cobra has already failed to match +a subcommand. The handler treats any non-empty `args` as an unknown +subcommand and `args[0]` as its name. No additional parsing. + +### Error Handling + +| Error condition | User-facing message (stdout verbatim box) | Recovery | +|-----------------|-------------------------------------------|----------| +| `ctx system ` | Verbatim-relay box: `ctx system: unknown subcommand "". A Claude Code hook is likely calling a ctx command this binary no longer ships (version skew between the installed plugin and the on-PATH ctx binary). Align the plugin and binary to the same release.` | Update plugin/binary to matching versions; or fix the hook command. | + +The returned error (stderr, via `main.go`'s `writeErr`; cobra's +own printing stays silenced by `RootCmd`'s `SilenceErrors`) is a +terse companion to the stdout box — it carries the unknown verb so +a human reading logs sees the cause without the help dump. + +## Interface + +### CLI + +No new command or flag. Changes the behavior of the existing +(hidden) `ctx system` grouping command when invoked with an +unrecognised subcommand. + +``` +ctx system # was: help + exit 0 ; now: verbatim relay + exit 1 +ctx system # unchanged: help, exit 0 +ctx system bootstrap # unchanged: runs bootstrap +``` + +## Implementation + +### Files to Create/Modify + +| File | Change | +|------|--------| +| `internal/cli/system/system.go` | After `parent.Cmd(...)`, set `c.RunE` to the unknown-subcommand handler. Keep grouping construction via `parent.Cmd`. | +| `internal/cli/system/core/.../unknown.go` (new, e.g. `core/unknown/`) | Handler: render the verbatim box, set `SilenceUsage`, return the error. Keeps `system.go` thin. | +| `internal/err/` | New `UnknownSubcommand(verb string) error` constructor. Locate the existing system/CLI error file rather than creating a package. | +| `internal/assets/commands/text/*.yaml` + `internal/config/embed/text` | New `DescKey` for the relay box title and body text (no hardcoded user-facing strings, per convention). | +| `internal/cli/system/*_test.go` | Tests per Testing section. | + +### Key Functions + +```go +// in internal/cli/system/system.go (sketch) +c := parent.Cmd(cmd.DescKeySystem, cmd.UseSystem, /* subs... */) +c.RunE = unknown.Handler // name TBD +return c + +// in the new handler (sketch) +func Handler(cmd *cobra.Command, args []string) error { + if len(args) == 0 { + return cmd.Help() // bare `ctx system`: unchanged + } + verb := args[0] + box := message.NudgeBox(relayPrefix, title, bodyFor(verb)) + fmt.Fprintln(cmd.OutOrStdout(), box) + + // Best-effort relay leg: only when a hook supplied a session. + // ReadID is TTY-safe and timeout-guarded, so a manual typo at a + // terminal returns IDUnknown without blocking. + if sid := session.ReadID(os.Stdin); sid != cfgSession.IDUnknown { + ref := entity.NewTemplateRef(hook.System, variantUnknownSub, nil) + if relayErr := nudge.Relay(relayMsgFor(verb), sid, ref); relayErr != nil { + logWarn.Warn(warn.RelayUnknownSubcommand, relayErr) // log, don't mask + } + } + + cmd.SilenceUsage = true // do NOT re-dump help on error + return errSystem.UnknownSubcommand(verb) +} +``` + +Note: a relay-leg failure is logged via the existing warn path and +does **not** change the returned error — the user's actual problem +is the unknown subcommand, and that is what the exit reflects. The +hook name/variant for the `TemplateRef` (`hook.System` / +`variantUnknownSub`) may need new constants; reuse existing ones if +a suitable pair already exists. + +### Helpers to Reuse + +- `message.NudgeBox(relayPrefix, title, content)` — the same + verbatim-box framing the `check-*` hooks use + (`internal/cli/system/core/message`). +- `session.ReadID(os.Stdin)` — TTY-safe, timeout-guarded session-ID + read from hook stdin; returns `cfgSession.IDUnknown` when absent + (`internal/cli/system/core/session`). +- `nudge.Relay(msg, sessionID, ref)` — log-first event-log append + + relay webhook (`internal/cli/system/core/nudge`). +- `entity.NewTemplateRef(hook, variant, vars)` — relay event ref for + filtering/aggregation. +- `desc.Text(...)` — load box title/body from assets, not literals. +- `parent.Cmd` — still builds the grouping command; only the `RunE` + is added on top. + +## Configuration + +None. No `.ctxrc` keys, env vars, or settings. + +## Testing + +- **Unit (handler):** call the handler with `args=["bogus"]`; + assert (a) stdout contains the verbatim box, the token `bogus`, + and the skew hint; (b) it returns a non-nil error; + (c) `cmd.SilenceUsage` is true after the call. +- **Unit (bare):** handler with `args=[]` prints help and returns + nil. +- **Cobra-level:** build `system.Cmd()`, execute with + `["definitely-not-a-cmd"]`, capture output; assert non-nil error + and the box on stdout, and assert **no** Long-help body is + printed. Execute with `[]` (bare) and assert help + nil error. +- **Routing:** assert a known subcommand name resolves to its own + command (e.g. `system.Cmd().Find([]string{"bootstrap"})` returns + `bootstrap`), proving the handler is bypassed for valid verbs. +- **Relay leg — fired:** with a stdin carrying a real session ID, + assert `nudge.Relay` records an event (event-log append observed + in a temp state dir) for the unknown verb. Assert a relay failure + is logged but the handler still returns the `UnknownSubcommand` + error (relay failure does not mask it). +- **Relay leg — skipped:** with a TTY/blank stdin (`ReadID` → + `IDUnknown`), assert no relay event is recorded and the call does + not block; stdout box + error still happen. +- **Scope:** assert `parent.Cmd` is unchanged — another group built + from `parent.Cmd` (e.g. `ctx hook`) still exits 0 on an unknown + subcommand. This documents the intentional scoping and will flag + if someone later moves the fix into the shared helper. + +## Non-Goals + +- **Not** changing the shared `parent.Cmd`. Other groups keep + cobra's default unknown-subcommand behavior. +- **Not** fixing `ctx hook` or any other group. They are not wired + into `hooks.json`, so their skew does not pollute a session. If + that changes, revisit. +- **Not** the build-time wiring guard — already shipped + (`specs/hooks-wiring-guard.md`). This is its runtime complement. +- **Not** removing the leftover `check-anchor-drift` prose mention + in `commands.yaml` (acdl follow-up #3 — a separate explicit + decision). +- **Not** generalizing the unknown-subcommand handler into the + shared `parent.Cmd`. `ctx hook` (and any future group) has the + same latent exit-0-on-unknown behavior, but it is not wired into + `hooks.json`, so its skew does not pollute a session. Captured as + a follow-up task, not built here. + +## Resolved Decisions + +Both decisions below were settled with the user at spec time +(session 0066d49b): + +1. **Relay leg is in scope.** The handler fires the event-log + + webhook relay (`nudge.Relay`), gated on a real session ID read + best-effort from stdin. When no session is present (manual TTY + typo, blank JSON), it emits the stdout box and exits non-zero + without a relay record. +2. **Scoped to `ctx system` only.** `parent.Cmd` is untouched; + other groups keep cobra's default. Generalization is recorded as + a follow-up task (see Non-Goals).