diff --git a/docs/backlog/custom-providers-names-nonexistent-signals.md b/docs/backlog/custom-providers-names-nonexistent-signals.md new file mode 100644 index 000000000..7876f90cd --- /dev/null +++ b/docs/backlog/custom-providers-names-nonexistent-signals.md @@ -0,0 +1,13 @@ +--- +worth: yes +where: docs/custom-providers.md +added: 2026-10-20 +--- +# custom providers doc uses signal markers that do not exist + +`docs/custom-providers.md` gives `<<>>` and `<<>>` as the signals a +wrapper's model must pass through, in the stream-format section and again in the requirements list. +Neither exists: the real markers are in `pkg/status/status.go` (`ALL_TASKS_DONE`, `TASK_FAILED`, +`REVIEW_DONE`, `CODEX_REVIEW_DONE`, ...). Anyone testing a wrapper by grepping its output for the +documented markers finds nothing, even though the wrapper works. Surfaced as a pre-existing finding in the +revmux review of the issue #466 prompt fix. diff --git a/docs/backlog/make-plan-prompt-lacks-bare-signal-rule.md b/docs/backlog/make-plan-prompt-lacks-bare-signal-rule.md new file mode 100644 index 000000000..dc960e802 --- /dev/null +++ b/docs/backlog/make-plan-prompt-lacks-bare-signal-rule.md @@ -0,0 +1,14 @@ +--- +worth: maybe +where: pkg/config/defaults/prompts/make_plan.txt +added: 2026-10-20 +--- +# make_plan prompt lacks the bare-name signal rule + +The fix for https://github.com/umputun/ralphex/issues/466 tells agents in `task.txt`, `review_first.txt`, +`review_second.txt`, `codex.txt` and `custom_eval.txt` to refer to a signal by its bare name and never quote +the full marker, because `detectSignal` (`pkg/executor/executor.go`) matches a marker anywhere in the +output. `make_plan.txt` was left out: no plan-mode failure has been reported. The exposure is the same for +`PLAN_READY`, so a sentence like "I will output <<>> once you accept" would end plan +creation early. `QUESTION` and `PLAN_DRAFT` are parsed with their `END` payload markers, so the wording +there needs its own thought rather than a copy of the one-line rule. diff --git a/pkg/config/defaults/prompts/codex.txt b/pkg/config/defaults/prompts/codex.txt index 3f1032a2b..f5770a688 100644 --- a/pkg/config/defaults/prompts/codex.txt +++ b/pkg/config/defaults/prompts/codex.txt @@ -35,6 +35,8 @@ Do NOT reject issues just because they existed before this branch - fix them any ## After Evaluation +SIGNALS: a signal marker takes effect wherever it appears in your output, even inside a sentence. Write the marker only when you are sending it, exactly as given and alone on its own line. To refer to a signal, including to say you are not sending one, write its bare name (CODEX_REVIEW_DONE), never the full marker. + **If there were actionable issues to fix:** - Fix them, run tests/linter to verify - ALL tests must pass, ALL linter issues resolved - Do NOT commit yet - more codex iterations may follow diff --git a/pkg/config/defaults/prompts/custom_eval.txt b/pkg/config/defaults/prompts/custom_eval.txt index 4f6546609..68d5ae1ae 100644 --- a/pkg/config/defaults/prompts/custom_eval.txt +++ b/pkg/config/defaults/prompts/custom_eval.txt @@ -35,6 +35,8 @@ Do NOT reject issues just because they existed before this branch - fix them any ## After Evaluation +SIGNALS: a signal marker takes effect wherever it appears in your output, even inside a sentence. Write the marker only when you are sending it, exactly as given and alone on its own line. To refer to a signal, including to say you are not sending one, write its bare name (CODEX_REVIEW_DONE), never the full marker. + **If there were actionable issues to fix:** - Fix them, run tests/linter to verify - ALL tests must pass, ALL linter issues resolved - Do NOT commit yet - more review iterations may follow diff --git a/pkg/config/defaults/prompts/review_first.txt b/pkg/config/defaults/prompts/review_first.txt index 8f494fad7..6d2f84421 100644 --- a/pkg/config/defaults/prompts/review_first.txt +++ b/pkg/config/defaults/prompts/review_first.txt @@ -73,7 +73,7 @@ SIGNAL LOGIC - READ CAREFULLY: IMPORTANT: Do not decide on a signal path until you have completed Steps 1-3 in full — all agents finished, all results collected, all findings verified and acted on. -IMPORTANT: A signal marker is irreversible. The moment you write one it takes effect, and you cannot retract it by explaining afterwards that you did not mean it. If you emit a marker it MUST be the final non-empty line of your output, with nothing after it. While you are still reasoning about which path applies, emit nothing. +IMPORTANT: A signal marker is irreversible. The moment you write one it takes effect, and you cannot retract it by explaining afterwards that you did not mean it. If you emit a marker it MUST be the final non-empty line of your output, with nothing after it. While you are still reasoning about which path applies, emit nothing. A marker takes effect wherever it appears, even inside a sentence: to refer to a signal, including to say you are not sending one, write its bare name (REVIEW_DONE, TASK_FAILED), never the full marker. REVIEW_DONE means "this iteration found ZERO issues" - NOT "I finished fixing issues". diff --git a/pkg/config/defaults/prompts/review_second.txt b/pkg/config/defaults/prompts/review_second.txt index 0a5c36ca0..fade2ec5d 100644 --- a/pkg/config/defaults/prompts/review_second.txt +++ b/pkg/config/defaults/prompts/review_second.txt @@ -55,7 +55,7 @@ SIGNAL LOGIC - READ CAREFULLY: IMPORTANT: Do not decide on a signal path until you have completed Steps 1-3 in full — all agents finished, all results collected, all findings verified and acted on. -IMPORTANT: A signal marker is irreversible. The moment you write one it takes effect, and you cannot retract it by explaining afterwards that you did not mean it. If you emit a marker it MUST be the final non-empty line of your output, with nothing after it. While you are still reasoning about which path applies, emit nothing. +IMPORTANT: A signal marker is irreversible. The moment you write one it takes effect, and you cannot retract it by explaining afterwards that you did not mean it. If you emit a marker it MUST be the final non-empty line of your output, with nothing after it. While you are still reasoning about which path applies, emit nothing. A marker takes effect wherever it appears, even inside a sentence: to refer to a signal, including to say you are not sending one, write its bare name (REVIEW_DONE, TASK_FAILED), never the full marker. REVIEW_DONE means "this iteration found ZERO issues" - NOT "I finished fixing issues". diff --git a/pkg/config/defaults/prompts/task.txt b/pkg/config/defaults/prompts/task.txt index beb4be0e3..2d12566ec 100644 --- a/pkg/config/defaults/prompts/task.txt +++ b/pkg/config/defaults/prompts/task.txt @@ -45,6 +45,8 @@ STEP 3 - COMPLETE (after validation passes): If any phase fails after reasonable fix attempts, output exactly: <<>> +SIGNALS: a signal marker takes effect wherever it appears in your output, even inside a sentence. Write the marker only when you are sending it, exactly as given and alone on its own line. To refer to a signal, including to say you are not sending one, write its bare name (ALL_TASKS_DONE, TASK_FAILED), never the full marker. + REMINDER: ONE section (Task/Iteration) per loop cycle. After commit, STOP and let the loop handle the next section. OUTPUT FORMAT: No markdown formatting (no **bold**, `code`, # headers). Plain text and - lists are fine. Do not echo phase names or step numbers - just do the work. diff --git a/pkg/web/dashboard_test.go b/pkg/web/dashboard_test.go index dec674b1a..7275a8068 100644 --- a/pkg/web/dashboard_test.go +++ b/pkg/web/dashboard_test.go @@ -230,7 +230,7 @@ func TestStartServerAsync_PortInUse(t *testing.T) { // first server should start errCh, err := startServerAsync(ctx, srv, 8999) require.NoError(t, err) - defer func() { cancel(); <-errCh }() + defer func() { cancel(); assert.NoError(t, <-errCh) }() // second server should fail _, err = startServerAsync(ctx, srv2, 8999) diff --git a/pkg/web/watcher_test.go b/pkg/web/watcher_test.go index 4f8f60c46..9e574a0ac 100644 --- a/pkg/web/watcher_test.go +++ b/pkg/web/watcher_test.go @@ -972,8 +972,8 @@ func TestWatcher_CloseAfterStart(t *testing.T) { // wait for watcher to exit select { - case <-done: - // watcher exited + case err := <-done: + require.NoError(t, err) case <-time.After(time.Second): t.Fatal("watcher did not stop after Close") }