From 35ae0c3273b6bf09af8fa0e7d874b3fa117e4b4c Mon Sep 17 00:00:00 2001 From: Jack Lau Date: Thu, 15 Oct 2026 01:01:17 -0400 Subject: [PATCH] fix(todo): never hang `todo context` on an unclosed stdin; accept the body as arguments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `todo context ` read its body from stdin and only refused a tty. Called inline by an agent with the text as an argument, stdin is the tool harness's socket, which never closes: io.ReadAll blocked forever and the text was dropped. Observed 2026-10-15 as a devbrain process stuck for ~4h on a unix socket stdin, task file never written. Trailing arguments after the id are now the body; with none, stdin must be a pipe or file — a tty or a socket is refused up front with the existing hint. The skills' heredoc form is unchanged. Co-Authored-By: Claude Fable 5.1 --- internal/todo/context_test.go | 62 +++++++++++++++++++++++++++++++++++ internal/todo/todo.go | 21 ++++++++---- 2 files changed, 76 insertions(+), 7 deletions(-) create mode 100644 internal/todo/context_test.go diff --git a/internal/todo/context_test.go b/internal/todo/context_test.go new file mode 100644 index 0000000..4edfadd --- /dev/null +++ b/internal/todo/context_test.go @@ -0,0 +1,62 @@ +package todo + +import ( + "bytes" + "os" + "path/filepath" + "strings" + "syscall" + "testing" + "time" +) + +func writeOpenTask(t *testing.T, dir, id string) { + t.Helper() + body := "---\nid: " + id + "\nstatus: open\n---\n\n# " + id + "\n" + if err := os.WriteFile(filepath.Join(dir, id+".md"), []byte(body), 0o644); err != nil { + t.Fatal(err) + } +} + +// The body may be passed as trailing arguments instead of on stdin — the way an +// agent invokes it inline — and must be stored, not silently dropped. +func TestContextBodyFromArgs(t *testing.T) { + dir := t.TempDir() + writeOpenTask(t, dir, "0001-task") + var out bytes.Buffer + c := &cli{dir: dir, project: "p", stdout: &out, stderr: &out, stdin: strings.NewReader("")} + if code := c.context([]string{"0001-task", "first finding;", "second finding"}); code != 0 { + t.Fatalf("context with body args = %d, want 0: %s", code, out.String()) + } + b, _ := os.ReadFile(filepath.Join(dir, "0001-task.md")) + if !strings.Contains(string(b), "## Context (synthesized ") || !strings.Contains(string(b), "first finding; second finding") { + t.Errorf("body args were not stored:\n%s", b) + } +} + +// A socket on stdin — a tool harness that never closes it — must be refused the +// way a tty is, not read until the end of time. +func TestContextRefusesSocketStdin(t *testing.T) { + dir := t.TempDir() + writeOpenTask(t, dir, "0002-task") + fds, err := syscall.Socketpair(syscall.AF_UNIX, syscall.SOCK_STREAM, 0) + if err != nil { + t.Skip("socketpair unavailable:", err) + } + rd := os.NewFile(uintptr(fds[0]), "stdin-socket") + peer := os.NewFile(uintptr(fds[1]), "stdin-socket-peer") + defer rd.Close() + defer peer.Close() // stays open for the whole test: nothing ever ends stdin + var out bytes.Buffer + c := &cli{dir: dir, project: "p", stdout: &out, stderr: &out, stdin: rd} + done := make(chan int, 1) + go func() { done <- c.context([]string{"0002-task"}) }() + select { + case code := <-done: + if code != 1 || !strings.Contains(out.String(), "stdin") { + t.Errorf("socket stdin: code=%d stderr=%q, want 1 and a stdin hint", code, out.String()) + } + case <-time.After(2 * time.Second): + t.Fatal("context blocked on an unclosed socket stdin instead of refusing it") + } +} diff --git a/internal/todo/todo.go b/internal/todo/todo.go index f30ed80..e9e90af 100644 --- a/internal/todo/todo.go +++ b/internal/todo/todo.go @@ -56,7 +56,7 @@ Tasks are created by /distill and worked by /continue — this CLI is the substr todo self-heal [status...] close open/taken tasks whose recorded PR has merged (zombie sweep) todo release taken/review/held -> open (un-claim / un-hold) todo reopen [reason] FORCE done -> open (work verified absent; regenerate) - todo context attach a synthesized "## Context" body section (reads stdin) + todo context attach a synthesized "## Context" body section (body on stdin or as trailing args) ` // cli holds one invocation's state: identity, streams, and the once-per-process @@ -974,15 +974,22 @@ func (c *cli) context(args []string) int { if !ok { return c.die("no such todo: " + id) } - if f, isFile := c.stdin.(*os.File); isFile { - if fi, err := f.Stat(); err == nil && fi.Mode()&os.ModeCharDevice != 0 { - return c.die("context reads the body on stdin (pipe or heredoc it)") // don't hang on a tty + // The body is either the trailing arguments (an agent calling inline) or, + // when there are none, stdin — which must be something that ends. A tty, or + // a socket a tool harness keeps open for the life of the process, would + // block here forever, so both are refused up front. + ctx := strings.TrimSpace(strings.Join(args[1:], " ")) + if ctx == "" { + if f, isFile := c.stdin.(*os.File); isFile { + if fi, err := f.Stat(); err == nil && fi.Mode()&(os.ModeCharDevice|os.ModeSocket) != 0 { + return c.die("context reads the body on stdin (pipe or heredoc it) or as trailing arguments") + } } + data, _ := io.ReadAll(c.stdin) + ctx = strings.TrimRight(string(data), "\n") // $(cat) } - data, _ := io.ReadAll(c.stdin) - ctx := strings.TrimRight(string(data), "\n") // $(cat) if ctx == "" { - return c.die("context needs the body on stdin") + return c.die("context needs the body on stdin or as trailing arguments") } // task minus any prior block; trailing blank lines stripped like $(awk …) var kept []string