Repository navigation
refactor: keep the L2 system prompt in one file instead of three inline copies - #161
Merged
Merged
Conversation
…ne copies prompts/L2_SYSTEM.txt holds the text the claude adapter, the omp adapter and run.sh's built-in claude fallback each carried inline (byte-identical, 657 characters). bin/l2-prompt.sh reads it and returns 2 with a message when it cannot. Both adapters' l2-argv exit 2 and print no argv without it, and run.sh's fallback builds no argv and logs why, because an engine started without the prompt runs L2 with no word on where the report goes. install.sh links the file and the loader. tests/l2-prompt.sh (25 assertions): red first (8 failures), then green. Covers the text, the loader's missing and empty cases, a symlinked lib, both adapters with and without the file in a throwaway tree, and that no script keeps an inline copy.
…claude engine included Review of ba742eb: with prompts/L2_SYSTEM.txt absent and the claude adapter current, l2-argv exits 2, the built-in fallback also finds no prompt, and every L2 attempt fails after L1 has been paid for (the claude engine is exempt from the argv probe). run.sh now checks the prompt for every engine next to the AUTODREAM_L2_ENGINE validation, so the night stops with a FATAL naming the file. tests/run-all.sh: setup_env, setup_env_altroot and the two install-like fixtures now put L2_SYSTEM.txt in the sandbox's autodream dir, as install.sh does, because a run from a copied bin/ finds the prompt only there. New test_a_missing_l2_prompt_stops_the_run_before_l1 runs a copied install with no prompt anywhere and checks the exit code, the FATAL, that no L1 round starts and that no report is written; it fails with the check mutated away.
…ader branch, and fix the fallback's log line Review nits on ba742eb. -s accepted a prompt file of blank lines, which would start an engine with an empty instruction; the loader now needs a non-space character. The relative-readlink branch had no test (install.sh writes absolute links, but a hand-made link may not); added. run.sh's fallback logged 'the claude adapter has no l2-argv here', which is only half the story when the adapter exited 2; it now says it printed no L2 command. Both new loader tests fail with their rule mutated away (27 assertions).
Conflicts with the report-contract lib (the two suite lists in tests/run-all.sh and tests.yml, and the CHANGELOG date section) resolved by keeping both. tests/report-format.sh 133, tests/l2-prompt.sh 27, adapter-contract 141, install-skill 34, review-skip 23 and pending-triage 22 pass on the merged tree.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The L2 aggregator's system prompt (657 characters) was inline in three places: the claude adapter's
l2-argv, the omp adapter'sl2-argv, andrun.sh's built-in claude fallback. It now lives inprompts/L2_SYSTEM.txt, read throughbin/l2-prompt.sh. The text states the stdout grammar thatbin/report-format.shparses (the report,AUTODREAM_REPORT_END, the optional pin block, the summary), so a copy that drifted from the parser would show up only as an L2 run whose output the runner cannot split.All three old copies were byte-identical (checked with
cmp, including the em dash, andrun.sh's\$turned back into$).install.shlinks the prompt and the loader.A missing prompt now stops things instead of starting an engine with no instructions: both adapters'
l2-argvexit 2 and print no argv, andrun.shrefuses to start before L1 for every engine, claude included.Sibling of #160 (the report contract as one lib); the two are independent.
Status
tests/l2-prompt.shcb3e3a0adds two). Covers the text, the loader's missing, empty and symlinked cases, both adapters in a throwaway tree with and without the file, and that no script keeps an inline copy.test_a_missing_l2_prompt_stops_the_run_before_l1intests/run-all.shRefusing to start, and an L1 round starts)tests/run-all.shon a clean export of7e707a4(the head beforecb3e3a0, which only touches the loader's whitespace check, one log line andtests/l2-prompt.sh, whose suite was re-run)mainin this sandbox (adapter-claudeneedsps, tworeview-cmuxinteractive assertions, theprune-worktreeswatchdog timing). Run withTZ=America/New_York.ba742ebbash -c.The review found one real gap, fixed in
7e707a4: with the prompt file absent and the claude adapter current, the claude engine was exempt from the startup probe, so a run paid for L1 and then failed all three L2 attempts. The prompt is now checked for every engine before L1. The sandbox installs intests/run-all.sh(four helpers) now carryL2_SYSTEM.txtasinstall.shdoes, because a run from a copiedbin/finds the prompt only there. My first version of the new test was vacuous (it passed with the check removed because the mock only writes captures when a capture variable is set); the version here asserts on the run log.Review nits, fixed in
cb3e3a0-s); the relative-symlink branch of the loader has a test; the fallback's log line no longer claims the adapter "has no l2-argv". Each new loader test fails with its rule mutated away;tests/l2-prompt.shis now 27 assertions.Open follow-ups
CHANGELOG.md: refactor: read the report's marker and sentinel in one lib instead of copies in six scripts #160 adds a section under the same date, so whichever merges second needs a trivial rebase.Test plan
bash tests/l2-prompt.shtests/run-all.shon a clean export of the head,TZ=America/New_Yorkgit pullin the install checkout, then./install.shso the install linksL2_SYSTEM.txtandl2-prompt.sh(a stale install still finds them through the checkout)