From 4cd7525945fffa2d86f07ccd82b4e18d1a22f7e7 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Mon, 19 Oct 2026 23:05:55 -0400 Subject: [PATCH 1/4] fix(plugin): classify known release startup diagnostics --- ...6-10-20-workflow-assistant-smoke-design.md | 54 +++++++++++++++ .../2026-10-20-workflow-assistant-smoke.md | 49 ++++++++++++++ ...-20-workflow-assistant-smoke.md.scope-lock | 1 + example/workflow-agent-plugin/README.md | 21 ++++-- example/workflow-agent-plugin/plugin.json | 2 +- example/workflow-agent-plugin/tests/main.go | 43 +++++++++--- .../workflow-agent-plugin/tests/smoke_test.go | 66 ++++++++++++++++++- 7 files changed, 216 insertions(+), 20 deletions(-) create mode 100644 docs/plans/2026-10-20-workflow-assistant-smoke-design.md create mode 100644 docs/plans/2026-10-20-workflow-assistant-smoke.md create mode 100644 docs/plans/2026-10-20-workflow-assistant-smoke.md.scope-lock diff --git a/docs/plans/2026-10-20-workflow-assistant-smoke-design.md b/docs/plans/2026-10-20-workflow-assistant-smoke-design.md new file mode 100644 index 00000000..3cbe7ed9 --- /dev/null +++ b/docs/plans/2026-10-20-workflow-assistant-smoke-design.md @@ -0,0 +1,54 @@ +# Workflow Assistant release smoke compatibility + +## Evidence and goal + +The official v0.87.0 Darwin arm64 asset matches its published checksum and +GitHub digest, embeds clean source `4b2c11cc` and MCP SDK v1.3.0, and passes the +five-version wire matrix. The existing package smoke completes its fixture +checks but rejects stderr: Sonic 1.15.2 prints its exact public Go 1.27 +`encoding/json` fallback warning at startup. The helper currently allows only +the exact legacy plugin-deprecation notice. + +Recognize and report that known diagnostic without weakening unknown-stderr, +release-version, input-privacy or acceptance boundaries. This is a separate +package-maintenance follow-up; the completed two-PR/six-task MCP plan is unchanged. + +## Chosen behavior + +- Classify complete stderr lines by exact equality with the existing deprecation + notice or the observed Sonic fallback notice. Reject every other line, + including prefix/suffix mutations, injected extra lines and blank lines. +- Return symbolic diagnostic status; never include raw stderr in smoke reports. + Preserve existing empty/deprecation-only statuses and distinguish Sonic-only + and combined known diagnostics. +- Keep the strict release-version guard and mismatch handling unchanged. + Source pseudo-versions and snapshots remain unknown. +- Publish package patch 0.1.1 with the same three skills and direct MCP manifest. + Document verified release pairings and the nonfatal diagnostic explicitly. +- Retain runtime, desktop, binary-health and input-equivalence unknowns. + +## Rejected alternatives + +Blanket stderr suppression can hide sensitive or unexpected output. Relabeling a +source binary would fabricate release evidence. Upgrading Sonic changes core +runtime behavior and is unnecessary for this bounded maintainer-client fix. +The published engine tag, release assets, workflows and permissions are unchanged. + +## Verification + +Use RED/GREEN tests for exact known lines and adversarial variants. Run standalone +race/vet checks, then the actual unchanged v0.86.2 and official v0.87.0 assets. +Check wrong-version/source-version failures, safe reports and preserved unknowns. +Record the smoke client's source separately from the official binary's source. +Independent spec/quality review, public-data audit and ordinary draft-PR CI follow. + +## Design backport — 2026-10-20 + +The genuine-child negative test disproved the assumption that existing cleanup +already rejected nonzero exit: `cmd.Wait()` errors were discarded. A child that +emitted the known warning and exited 2 returned no cleanup error (observed RED). +Retain its wait error and report bounded `check_failed` after reaping; protocol +and fixture failures keep their earlier precedence. Forced termination also +fails while cleanup still reaps the child and reader. Independent review found +this necessary to enforce the chosen successful-shutdown boundary within Task 1's +same helper/test files. The one-PR/one-task scope manifest and lock are unchanged. diff --git a/docs/plans/2026-10-20-workflow-assistant-smoke.md b/docs/plans/2026-10-20-workflow-assistant-smoke.md new file mode 100644 index 00000000..9eb1cf7b --- /dev/null +++ b/docs/plans/2026-10-20-workflow-assistant-smoke.md @@ -0,0 +1,49 @@ +# Workflow Assistant release smoke plan + +**Goal:** Classify the verified non-sensitive Sonic startup diagnostic while +preserving strict smoke-client guards and truthful release evidence. + +**Design:** `2026-10-20-workflow-assistant-smoke-design.md`. + +## Scope Manifest + +**PR Count:** 1 +**Tasks:** 1 +**Estimated Lines of Change:** ~150 + +**Out of scope:** +- Engine/SDK/dependency, transport, skill, workflow, permission or deployment changes. +- Retagging, restarting or republishing the v0.87.0 engine release. +- Blanket stderr suppression, source-version relabeling or expanded acceptance claims. +- Changes to completed MCP plans, unrelated locks or installed plugins/sessions. + +**PR Grouping:** + +| PR # | Title | Tasks | Branch | +|------|-------|-------|--------| +| 1 | Handle known release startup diagnostic in Workflow Assistant smoke | Task 1 | fix/workflow-assistant-release-smoke | + +**Status:** Locked + +### Task 1: Correct the bounded smoke diagnostic classification + +Files: package `tests/main.go`, `tests/smoke_test.go`, `plugin.json`, `README.md`, +and this design/plan. Add no engine or workflow changes. + +1. Extract the existing exact-deprecation-only stderr classification without + changing behavior; confirm existing tests pass. Add adversarial tests and + observe the Sonic-positive case fail before support is added. +2. Add exact Sonic classification and explicit symbolic statuses. Reject unknown, + suffixed/prefixed, injected and malformed lines without echoing their content. +3. Bump package/client metadata to 0.1.1. Document both actually tested released + CLIs, diagnostic handling and unchanged host/runtime limits. Attribute the + existing Codex/Autodev host probe to historical package 0.1.0 evidence; + package 0.1.1 smoke does not establish fresh host activation. +4. Run standalone race/vet checks; run real released v0.86.2 and v0.87.0 smokes; + verify strict wrong-release/source-pseudo-version failures. Preserve the + existing actual v0.87.0 five-version wire evidence; runtime source is unchanged. +5. Complete independent spec/quality review, privacy/scope audit, clean package + export and ordinary draft-PR CI. Keep the original 0.1.0 export immutable. + +Rollback: revert the package-only follow-up. No installed or engine release state +is changed by this task. diff --git a/docs/plans/2026-10-20-workflow-assistant-smoke.md.scope-lock b/docs/plans/2026-10-20-workflow-assistant-smoke.md.scope-lock new file mode 100644 index 00000000..36c1d624 --- /dev/null +++ b/docs/plans/2026-10-20-workflow-assistant-smoke.md.scope-lock @@ -0,0 +1 @@ +5c9b8b5c0cbc4ffb64e61943b5a330020434f955d642b6b6d6cf3b87cefa85de diff --git a/example/workflow-agent-plugin/README.md b/example/workflow-agent-plugin/README.md index 777f450c..332747b4 100644 --- a/example/workflow-agent-plugin/README.md +++ b/example/workflow-agent-plugin/README.md @@ -14,11 +14,12 @@ with explicit coverage limits. No service, dashboard or orchestration platform. ## Prerequisites and local setup Supply a trusted `wfctl` executable yourself. This package does not bundle, -download or update it. The tested release is -[v0.86.2](https://github.com/GoCodeAlone/workflow/releases/tag/v0.86.2). +download or update it. Package 0.1.1's maintainer smoke was tested with the Darwin +arm64 assets from [v0.86.2](https://github.com/GoCodeAlone/workflow/releases/tag/v0.86.2) +and [v0.87.0](https://github.com/GoCodeAlone/workflow/releases/tag/v0.87.0). Choose the asset for your OS/architecture and verify its checksum against that release's `checksums.txt` before using it. Follow the -[public CLI setup guide](https://github.com/GoCodeAlone/workflow/blob/v0.86.2/docs/mcp.md). +[public CLI setup guide](https://github.com/GoCodeAlone/workflow/blob/v0.87.0/docs/mcp.md). `wfctl version` should report the selected version. Other releases are untested; the skills discover tools dynamically and retain unknown version claims. @@ -48,7 +49,7 @@ plugins; do not assume live inventory changes reload the server's type registry. | Surface | Package intent and verification status | |---|---| -| Local Codex supporting the portable format | Codex CLI 0.163.0 loaded this package and public Autodev 6.6.3 together in a disposable offline home: all three skills registered and the host discovered 33 tools/5 resources from wfctl v0.86.2. Model-driven selection and complete prompt behavior **not yet tested**. | +| Local Codex supporting the portable format | Historical package **0.1.0** probe: Codex CLI 0.163.0 loaded it and public Autodev 6.6.3 together in a disposable offline home; all three skills registered and the host discovered 33 tools/5 resources from wfctl v0.86.2. Package 0.1.1 has separate maintainer smoke evidence, with unchanged skills/MCP manifest; fresh host activation, model-driven selection and complete prompt behavior **not yet tested**. | | ChatGPT desktop with local execution/plugin support | Same intended package, subject to account/workspace features. Complete desktop activation **not yet tested**. | | Other Agent Plugins clients | Requires skills + stdio + both placeholders; no host certification claimed. | | ChatGPT browser, remote/cloud executor | Cannot directly run this local executable. No remote endpoint, tunnel or hosted bridge included. | @@ -69,7 +70,7 @@ SHA256 if available, the supplied source commit/path, observed server version, tools/schemas used and separate check outcomes. Redaction/partial status and equivalence to original input are distinct. No aggregate deploy-ready badge. -The real v0.86.2 interface differs from portions of its reference documentation: +The tested interfaces differ from portions of the v0.86.2 reference documentation: - `get_module_schema` takes `module_type`; `get_step_schema` takes `step_type`. - `compat_check` takes `yaml_content`. It checks the running module/step registry; @@ -118,7 +119,7 @@ validator and ECMA-regex adapter. From this directory, run: ```sh cd tests GOWORK=off go test -race ./... -GOWORK=off go run . -wfctl /path/to/reviewed/wfctl -expect-version v0.86.2 +GOWORK=off go run . -wfctl /path/to/reviewed/wfctl -expect-version v0.87.0 ``` The smoke client uses only bundled public fixtures, launches the actual manifest command @@ -126,6 +127,14 @@ with temporary plugin data/minimal environment, and emits safe summaries and digests. It tests positive/negative configs and known upstream false positives. It does not accept user YAML or read organization repositories/secrets. Missing executable/version/tools or protocol failures return a bounded failure category. +Released-version matching remains strict; snapshots and source pseudo-versions +are rejected. The client recognizes only the exact legacy plugin-deprecation +notice and the exact Sonic Go 1.27 `encoding/json` fallback warning observed in +v0.87.0. It reports symbolic stderr statuses (`empty`, `known_deprecation_only`, +`known_sonic_fallback_only`, or `known_deprecation_and_sonic_fallback`) after +fixture checks and successful child shutdown. Changed, extra or malformed +diagnostics fail as `unexpected_stderr`; raw stderr is never included. Package +0.1.0's smoke rejected that new warning; 0.1.1 adds this bounded classification. Fixture files use `.yaml.txt` because they include intentional failures and must stay outside the repository's recursive runnable `.yaml` example discovery. Unit-test doubles cover failures; only the real release run is MCP integration diff --git a/example/workflow-agent-plugin/plugin.json b/example/workflow-agent-plugin/plugin.json index 894fdadf..93680361 100644 --- a/example/workflow-agent-plugin/plugin.json +++ b/example/workflow-agent-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://agent-plugins.org/schemas/1.0.0/plugin.schema.json", "name": "workflow-assistant", - "version": "0.1.0", + "version": "0.1.1", "description": "Configure and diagnose public Workflow apps with wfctl MCP and produce bounded readiness evidence.", "author": {"name": "GoCodeAlone", "url": "https://github.com/GoCodeAlone"}, "homepage": "https://github.com/GoCodeAlone/workflow/tree/main/example/workflow-agent-plugin", diff --git a/example/workflow-agent-plugin/tests/main.go b/example/workflow-agent-plugin/tests/main.go index 94104e1b..c6bb0000 100644 --- a/example/workflow-agent-plugin/tests/main.go +++ b/example/workflow-agent-plugin/tests/main.go @@ -38,6 +38,7 @@ var reasons = map[string]bool{ var requiredTools = []string{"list_module_types", "list_step_types", "list_trigger_types", "list_workflow_types", "get_module_schema", "get_config_skeleton", "validate_config", "inspect_config", "compat_check", "template_validate_config", "list_plugins"} const deprecation = "[DEPRECATED] requires.plugins[].version and requires.plugins[].source in app.yaml are deprecated. Run 'wfctl migrate plugins' to migrate to wfctl.yaml + .wfctl-lock.yaml." +const sonicFallback = "WARNING: sonic/ast only supports (go1.17~1.26 and amd64 CPU) or (go1.20~1.26 and arm64 CPU), but your environment is not suitable and will fallback to encoding/json" func digest(data []byte) string { sum := sha256.Sum256(data); return hex.EncodeToString(sum[:]) } func fileDigest(path string) (string, error) { @@ -343,7 +344,8 @@ func (c *client) close() ([]byte, error) { close(c.stopping) _ = c.stdin.Close() // EOF first; broken stdin must not prevent reaping. waited := make(chan struct{}) - go func() { _ = c.cmd.Wait(); close(waited) }() + var waitErr error + go func() { waitErr = c.cmd.Wait(); close(waited) }() wait := func() bool { timer := time.NewTimer(3 * time.Second) defer timer.Stop() @@ -363,6 +365,9 @@ func (c *client) close() ([]byte, error) { } } } + if c.closeErr == nil && waitErr != nil { + c.closeErr = failure("check_failed") + } _ = c.stdout.Close() timer := time.NewTimer(time.Second) defer timer.Stop() @@ -467,7 +472,7 @@ func runSmoke(ctx context.Context, opts options) (map[string]any, error) { } defer c.close() s := session{client: c} - initialized := s.request("initialize", map[string]any{"protocolVersion": "2024-11-05", "capabilities": map[string]any{}, "clientInfo": map[string]any{"name": "workflow-package-smoke", "version": "0.1.0"}}) + initialized := s.request("initialize", map[string]any{"protocolVersion": "2024-11-05", "capabilities": map[string]any{}, "clientInfo": map[string]any{"name": "workflow-package-smoke", "version": "0.1.1"}}) info := object(initialized["serverInfo"]) if s.err == nil { s.err = checkVersion(info, opts.expected) @@ -534,10 +539,9 @@ func runSmoke(ctx context.Context, opts options) (map[string]any, error) { if closeErr != nil { return nil, closeErr } - for _, line := range strings.Split(strings.TrimSuffix(string(stderr), "\n"), "\n") { - if len(stderr) != 0 && line != deprecation { - return nil, failure("unexpected_stderr") - } + stderrStatus, err := classifyStderr(stderr) + if err != nil { + return nil, err } binaryHash, err := fileDigest(binary) if err != nil { @@ -551,10 +555,6 @@ func runSmoke(ctx context.Context, opts options) (map[string]any, error) { for name, data := range fixtures { fixtureHashes[name] = digest(data) } - stderrStatus := "empty" - if len(stderr) != 0 { - stderrStatus = "known_deprecation_only" - } return map[string]any{ "status": "passed", "scope": "bundled_public_fixtures_only", "captured_at": time.Now().UTC().Format(time.RFC3339Nano), "server": map[string]any{"name": info["name"], "version": info["version"]}, "release_source": "https://github.com/GoCodeAlone/workflow/tree/" + opts.expected, "binary_sha256": binaryHash, "tool_count": len(tools), "tool_inventory_sha256": digest(inventory), "fixture_sha256": fixtureHashes, @@ -563,6 +563,29 @@ func runSmoke(ctx context.Context, opts options) (map[string]any, error) { "runtime_acceptance": "not_checked", "desktop_activation": "not_checked", "plugin_binary_health": "not_checked", "original_input_equivalence": "not_checked", }, nil } +func classifyStderr(stderr []byte) (string, error) { + if len(stderr) == 0 { + return "empty", nil + } + var hasDeprecation, hasSonic bool + for _, line := range strings.Split(strings.TrimSuffix(string(stderr), "\n"), "\n") { + switch line { + case deprecation: + hasDeprecation = true + case sonicFallback: + hasSonic = true + default: + return "", failure("unexpected_stderr") + } + } + if hasSonic { + if hasDeprecation { + return "known_deprecation_and_sonic_fallback", nil + } + return "known_sonic_fallback_only", nil + } + return "known_deprecation_only", nil +} func mainWith(ctx context.Context, args []string, out io.Writer, run func(context.Context, options) (map[string]any, error)) int { flags := flag.NewFlagSet("workflow-package-smoke", flag.ContinueOnError) flags.SetOutput(io.Discard) diff --git a/example/workflow-agent-plugin/tests/smoke_test.go b/example/workflow-agent-plugin/tests/smoke_test.go index d2727d67..99a74bb0 100644 --- a/example/workflow-agent-plugin/tests/smoke_test.go +++ b/example/workflow-agent-plugin/tests/smoke_test.go @@ -25,7 +25,7 @@ func wantFailure(t *testing.T, err error, want string) { } func TestUnknownAndMismatchedVersionsFail(t *testing.T) { - for _, value := range []any{nil, "", "dev", "v0.85.4", "private-value"} { + for _, value := range []any{nil, "", "dev", "v0.85.4", "v0.87.0-rc.1", "v0.87.0-snapshot", "v0.86.3-0.20261020020202-4b2c11cc2d49", "private-value"} { if err := checkVersion(map[string]any{"name": "wfctl-mcp", "version": value}, "v0.86.2"); err == nil { t.Errorf("accepted version %#v", value) } @@ -35,6 +35,49 @@ func TestUnknownAndMismatchedVersionsFail(t *testing.T) { } } +func TestStderrClassificationIsExactAndBounded(t *testing.T) { + const sonicLine = "WARNING: sonic/ast only supports (go1.17~1.26 and amd64 CPU) or (go1.20~1.26 and arm64 CPU), but your environment is not suitable and will fallback to encoding/json" + for _, tc := range []struct { + name, input, want string + }{ + {"empty", "", "empty"}, + {"legacy", deprecation + "\n", "known_deprecation_only"}, + {"legacy without newline", deprecation, "known_deprecation_only"}, + {"repeated legacy", deprecation + "\n" + deprecation + "\n", "known_deprecation_only"}, + {"sonic", sonicLine + "\n", "known_sonic_fallback_only"}, + {"sonic without newline", sonicLine, "known_sonic_fallback_only"}, + {"combined", sonicLine + "\n" + deprecation + "\n", "known_deprecation_and_sonic_fallback"}, + {"combined reversed", deprecation + "\n" + sonicLine + "\n", "known_deprecation_and_sonic_fallback"}, + } { + t.Run(tc.name, func(t *testing.T) { + got, err := classifyStderr([]byte(tc.input)) + if err != nil || got != tc.want { + t.Fatalf("classification = %q, %v; want %q", got, err, tc.want) + } + }) + } + for _, input := range []string{ + "SYNTHETIC_STDERR_CANARY", "prefix " + sonicLine, sonicLine + " suffix", + strings.Replace(sonicLine, "1.26", "1.27", 1), sonicLine + "\r\n", + "\n", "\n" + sonicLine, sonicLine + "\n\n", deprecation + "\n\n", + sonicLine + "\nSYNTHETIC_STDERR_CANARY\n", deprecation + "\nSYNTHETIC_STDERR_CANARY\n", + sonicLine + string([]byte{0xff}), sonicLine + "\x00", " " + deprecation, + } { + var report bytes.Buffer + status := mainWith(t.Context(), nil, &report, func(context.Context, options) (map[string]any, error) { + got, err := classifyStderr([]byte(input)) + if got != "" { + t.Fatal("rejected stderr produced a success classification") + } + wantFailure(t, err, "unexpected_stderr") + return nil, err + }) + if status != 1 || strings.Contains(report.String(), "CANARY") || strings.Contains(report.String(), sonicLine) { + t.Fatal("stderr leaked or failure not reported") + } + } +} + func TestWrongServerFails(t *testing.T) { wantFailure(t, checkVersion(map[string]any{"name": "other", "version": "v0.86.2"}, "v0.86.2"), "wrong_server") } @@ -139,6 +182,16 @@ func TestClientShutdownSendsEOFBeforeSignals(t *testing.T) { } } +func TestKnownWarningDoesNotMaskChildExitFailure(t *testing.T) { + c := helperClient(t, "known-warning-exit") + stderr, err := c.close() + wantFailure(t, err, "check_failed") + got, classificationErr := classifyStderr(stderr) + if classificationErr != nil || got != "known_sonic_fallback_only" { + t.Fatal("helper did not emit the known diagnostic") + } +} + func TestRealChildCanaryOutputIsSuppressed(t *testing.T) { executable, err := os.Executable() if err != nil { @@ -191,7 +244,8 @@ func TestBrokenStdinDoesNotInterruptCleanup(t *testing.T) { t.Fatal("closed child stdin accepted write") } stderr, err := c.close() - if err != nil || len(stderr) != 0 { + wantFailure(t, err, "check_failed") + if len(stderr) != 0 { t.Fatalf("close = %q, %v", stderr, err) } if c.cmd.ProcessState == nil { @@ -269,7 +323,8 @@ func TestContextCancellationReapsSilentChildAndReader(t *testing.T) { t.Fatal("context cancellation did not stop silent child reader") } stderr, err := c.close() - if err != nil || len(stderr) != 0 { + wantFailure(t, err, "check_failed") + if len(stderr) != 0 { t.Fatalf("close = %q, %v", stderr, err) } if c.cmd.ProcessState == nil || c.cmd.ProcessState.Success() { @@ -335,8 +390,13 @@ func TestHelperProcess(t *testing.T) { fmt.Fprintln(os.Stdout, `{"ready":true}`) _, _ = io.Copy(io.Discard, os.Stdin) case "canary": + fmt.Fprintln(os.Stderr, sonicFallback) fmt.Fprintln(os.Stderr, "SYNTHETIC_PROCESS_CANARY") fmt.Fprintln(os.Stdout, "SYNTHETIC_PROCESS_CANARY") + case "known-warning-exit": + fmt.Fprintln(os.Stderr, sonicFallback) + _, _ = io.Copy(io.Discard, os.Stdin) + os.Exit(2) case "launch-marker": if err := os.WriteFile(os.Getenv("SMOKE_LAUNCH_MARKER"), []byte("launched"), 0600); err != nil { os.Exit(2) From 52379799070ca75b9fd7b756ee63e4e8a86a6356 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Mon, 19 Oct 2026 23:08:15 -0400 Subject: [PATCH 2/4] fix(plugin): reject forced smoke shutdown even after zero exit --- ...6-10-20-workflow-assistant-smoke-design.md | 4 +++ example/workflow-agent-plugin/tests/main.go | 1 + .../workflow-agent-plugin/tests/smoke_test.go | 25 +++++++++++++++++++ 3 files changed, 30 insertions(+) diff --git a/docs/plans/2026-10-20-workflow-assistant-smoke-design.md b/docs/plans/2026-10-20-workflow-assistant-smoke-design.md index 3cbe7ed9..f29a1d22 100644 --- a/docs/plans/2026-10-20-workflow-assistant-smoke-design.md +++ b/docs/plans/2026-10-20-workflow-assistant-smoke-design.md @@ -52,3 +52,7 @@ and fixture failures keep their earlier precedence. Forced termination also fails while cleanup still reaps the child and reader. Independent review found this necessary to enforce the chosen successful-shutdown boundary within Task 1's same helper/test files. The one-PR/one-task scope manifest and lock are unchanged. +Spec review also identified a child that handles forced SIGTERM with exit 0; +its genuine-child regression failed before correction. Record required signal +escalation as `check_failed` regardless of final exit status, while retaining +bounded cleanup and timeout precedence. diff --git a/example/workflow-agent-plugin/tests/main.go b/example/workflow-agent-plugin/tests/main.go index c6bb0000..975de397 100644 --- a/example/workflow-agent-plugin/tests/main.go +++ b/example/workflow-agent-plugin/tests/main.go @@ -357,6 +357,7 @@ func (c *client) close() ([]byte, error) { } } if !wait() { + c.closeErr = failure("check_failed") _ = c.cmd.Process.Signal(syscall.SIGTERM) if !wait() { _ = c.cmd.Process.Kill() diff --git a/example/workflow-agent-plugin/tests/smoke_test.go b/example/workflow-agent-plugin/tests/smoke_test.go index 99a74bb0..6276a7bb 100644 --- a/example/workflow-agent-plugin/tests/smoke_test.go +++ b/example/workflow-agent-plugin/tests/smoke_test.go @@ -11,8 +11,10 @@ import ( "fmt" "io" "os" + "os/signal" "path/filepath" "strings" + "syscall" "testing" "time" ) @@ -192,6 +194,23 @@ func TestKnownWarningDoesNotMaskChildExitFailure(t *testing.T) { } } +func TestEscalatedExitZeroDoesNotPassShutdown(t *testing.T) { + c := helperClient(t, "term-zero") + select { + case ready := <-c.messages: + if object(ready)["ready"] != true { + t.Fatal("signal helper not ready") + } + case <-time.After(3 * time.Second): + t.Fatal("signal helper readiness timeout") + } + _, err := c.close() + wantFailure(t, err, "check_failed") + if c.cmd.ProcessState == nil || !c.cmd.ProcessState.Success() { + t.Fatal("helper did not handle SIGTERM with exit 0") + } +} + func TestRealChildCanaryOutputIsSuppressed(t *testing.T) { executable, err := os.Executable() if err != nil { @@ -397,6 +416,12 @@ func TestHelperProcess(t *testing.T) { fmt.Fprintln(os.Stderr, sonicFallback) _, _ = io.Copy(io.Discard, os.Stdin) os.Exit(2) + case "term-zero": + terminated := make(chan os.Signal, 1) + signal.Notify(terminated, syscall.SIGTERM) + fmt.Fprintln(os.Stderr, sonicFallback) + fmt.Fprintln(os.Stdout, `{"ready":true}`) + <-terminated case "launch-marker": if err := os.WriteFile(os.Getenv("SMOKE_LAUNCH_MARKER"), []byte("launched"), 0600); err != nil { os.Exit(2) From a7f8106997436aef0875ec7330a3f9ed0288ef26 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Mon, 19 Oct 2026 23:09:34 -0400 Subject: [PATCH 3/4] test(plugin): skip Unix signal handling on Windows --- example/workflow-agent-plugin/tests/smoke_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/example/workflow-agent-plugin/tests/smoke_test.go b/example/workflow-agent-plugin/tests/smoke_test.go index 6276a7bb..2f61a30e 100644 --- a/example/workflow-agent-plugin/tests/smoke_test.go +++ b/example/workflow-agent-plugin/tests/smoke_test.go @@ -13,6 +13,7 @@ import ( "os" "os/signal" "path/filepath" + "runtime" "strings" "syscall" "testing" @@ -195,6 +196,9 @@ func TestKnownWarningDoesNotMaskChildExitFailure(t *testing.T) { } func TestEscalatedExitZeroDoesNotPassShutdown(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Windows does not support catching SIGTERM") + } c := helperClient(t, "term-zero") select { case ready := <-c.messages: From 0e1ecb9d764fa28c6219c1f530583eacf3ddc401 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Mon, 19 Oct 2026 23:14:32 -0400 Subject: [PATCH 4/4] fix(plugin): reject truncated smoke stderr captures --- ...6-10-20-workflow-assistant-smoke-design.md | 6 ++++ example/workflow-agent-plugin/README.md | 2 ++ example/workflow-agent-plugin/tests/main.go | 5 ++- .../workflow-agent-plugin/tests/smoke_test.go | 35 +++++++++++++++++++ 4 files changed, 47 insertions(+), 1 deletion(-) diff --git a/docs/plans/2026-10-20-workflow-assistant-smoke-design.md b/docs/plans/2026-10-20-workflow-assistant-smoke-design.md index f29a1d22..29780a46 100644 --- a/docs/plans/2026-10-20-workflow-assistant-smoke-design.md +++ b/docs/plans/2026-10-20-workflow-assistant-smoke-design.md @@ -56,3 +56,9 @@ Spec review also identified a child that handles forced SIGTERM with exit 0; its genuine-child regression failed before correction. Record required signal escalation as `check_failed` regardless of final exit status, while retaining bounded cleanup and timeout precedence. +Quality review disproved another existing assumption: the bounded stderr read +did not reject overflow, so an exact-known prefix of 1 MiB + 1 bytes hid an +unknown suffix. A genuine-child canary test reproduced RED. Reject captured +stderr above 1 MiB as bounded `unexpected_stderr` before classification, retaining +earlier cleanup failures. This enforces Task 1's injected-output rejection in the +same files; the manifest and lock remain unchanged. diff --git a/example/workflow-agent-plugin/README.md b/example/workflow-agent-plugin/README.md index 332747b4..31612ce4 100644 --- a/example/workflow-agent-plugin/README.md +++ b/example/workflow-agent-plugin/README.md @@ -135,6 +135,8 @@ v0.87.0. It reports symbolic stderr statuses (`empty`, `known_deprecation_only`, fixture checks and successful child shutdown. Changed, extra or malformed diagnostics fail as `unexpected_stderr`; raw stderr is never included. Package 0.1.0's smoke rejected that new warning; 0.1.1 adds this bounded classification. +Stderr above the client's 1 MiB capture limit also fails, so truncation cannot +hide an unknown suffix behind recognized notices. Fixture files use `.yaml.txt` because they include intentional failures and must stay outside the repository's recursive runnable `.yaml` example discovery. Unit-test doubles cover failures; only the real release run is MCP integration diff --git a/example/workflow-agent-plugin/tests/main.go b/example/workflow-agent-plugin/tests/main.go index 975de397..3bb09ebc 100644 --- a/example/workflow-agent-plugin/tests/main.go +++ b/example/workflow-agent-plugin/tests/main.go @@ -39,6 +39,7 @@ var requiredTools = []string{"list_module_types", "list_step_types", "list_trigg const deprecation = "[DEPRECATED] requires.plugins[].version and requires.plugins[].source in app.yaml are deprecated. Run 'wfctl migrate plugins' to migrate to wfctl.yaml + .wfctl-lock.yaml." const sonicFallback = "WARNING: sonic/ast only supports (go1.17~1.26 and amd64 CPU) or (go1.20~1.26 and arm64 CPU), but your environment is not suitable and will fallback to encoding/json" +const maxStderrBytes = 1024 * 1024 func digest(data []byte) string { sum := sha256.Sum256(data); return hex.EncodeToString(sum[:]) } func fileDigest(path string) (string, error) { @@ -378,9 +379,11 @@ func (c *client) close() ([]byte, error) { c.closeErr = failure("timeout") } if _, err := c.stderr.Seek(0, io.SeekStart); err == nil { - c.closedStderr, err = io.ReadAll(io.LimitReader(c.stderr, 1024*1024+1)) + c.closedStderr, err = io.ReadAll(io.LimitReader(c.stderr, maxStderrBytes+1)) if err != nil { c.closeErr = failure("internal_error") + } else if len(c.closedStderr) > maxStderrBytes && c.closeErr == nil { + c.closeErr = failure("unexpected_stderr") } } else { c.closeErr = failure("internal_error") diff --git a/example/workflow-agent-plugin/tests/smoke_test.go b/example/workflow-agent-plugin/tests/smoke_test.go index 2f61a30e..8898b1b4 100644 --- a/example/workflow-agent-plugin/tests/smoke_test.go +++ b/example/workflow-agent-plugin/tests/smoke_test.go @@ -215,6 +215,33 @@ func TestEscalatedExitZeroDoesNotPassShutdown(t *testing.T) { } } +func TestOversizedStderrIsRejectedWithoutEcho(t *testing.T) { + c := helperClient(t, "stderr-overflow") + select { + case ready := <-c.messages: + if object(ready)["ready"] != true { + t.Fatal("overflow helper not ready") + } + case <-time.After(3 * time.Second): + t.Fatal("overflow helper readiness timeout") + } + var out bytes.Buffer + status := mainWith(t.Context(), nil, &out, func(context.Context, options) (map[string]any, error) { + _, err := c.close() + return nil, err + }) + var report map[string]any + if status != 1 || json.Unmarshal(out.Bytes(), &report) != nil || report["reason"] != "unexpected_stderr" { + t.Fatal("oversized stderr did not fail with a bounded category") + } + if strings.Contains(out.String(), "CANARY") || strings.Contains(out.String(), "WARNING") { + t.Fatal("raw stderr leaked") + } + if c.cmd.ProcessState == nil || !c.cmd.ProcessState.Success() { + t.Fatal("overflow helper did not exit cleanly") + } +} + func TestRealChildCanaryOutputIsSuppressed(t *testing.T) { executable, err := os.Executable() if err != nil { @@ -426,6 +453,14 @@ func TestHelperProcess(t *testing.T) { fmt.Fprintln(os.Stderr, sonicFallback) fmt.Fprintln(os.Stdout, `{"ready":true}`) <-terminated + case "stderr-overflow": + knownPrefix := strings.TrimSuffix(strings.Repeat(deprecation+"\n", 28)+strings.Repeat(sonicFallback+"\n", 6326), "\n") + if len(knownPrefix) != 1024*1024+1 { + os.Exit(2) + } + fmt.Fprint(os.Stderr, knownPrefix+"\nSYNTHETIC_OVERFLOW_CANARY") + fmt.Fprintln(os.Stdout, `{"ready":true}`) + _, _ = io.Copy(io.Discard, os.Stdin) case "launch-marker": if err := os.WriteFile(os.Getenv("SMOKE_LAUNCH_MARKER"), []byte("launched"), 0600); err != nil { os.Exit(2)