[yudongyouqing]

Summary

• A route condition reaches the judge where a prompt reaches its node, but the executor filled parameters only in promptTemplate and validation checked references only in promptTemplate and inputs: the deal is above ${params.threshold} reached the judge as written, hinging the decision on a placeholder no reader can evaluate, and an undeclared ${params.ghost} in a condition passed validation -- item 2 of DAG conditional branches: seven follow-ups left out of #1001 #1008.
• Conditions are filled on the same terms as prompts now, at the same fill site, secrets withheld included (_fill_text does the withholding); and an undeclared reference in a route is refused at validation with the route named: node 'a': params.ghost in route 'go' names no declared param.
• Composed-graph nodes were never filled for promptTemplate either and are unchanged by this PR; the fill stays where it was, now covering routes beside it.

Type

• Fix

Verification

• Relevant tests pass locally: uv run pytest tests/test_playbook_validate.py tests/test_playbook_executor.py tests/test_playbook_store.py tests/test_unified_playbook.py tests/test_cli_playbook_commands.py tests/test_agent_loop_playbook_entry.py -q -> 288 passed
• New tests failed first: the fill test dispatched 'the deal is above ${params.floor}' verbatim on the base and the validation test accepted an undeclared reference; both assert the filled and refused outcomes on this head
• Relevant lint / type checks pass locally: ruff check and ruff format --check on all four files, make lint-types -> clean
• User-facing docs or screenshots are updated when needed: no user-facing copy changes; the judge now reads a condition it can evaluate

Risk

• Security impact considered: filling uses the same _fill_text that withholds secrets from prompts, so a secret param in a condition is withheld, not exposed
• Backward compatibility considered: a playbook whose conditions name only declared params loads and runs identically; one that named undeclared references was already broken at run time and now says so at load
• Rollback path is clear: plain revert restores the unfilled, unchecked condition

Related Issues

Item 2 of #1008

[gloryfromca]

No blockers; this can merge as far as I am concerned.

The route-condition references now follow the same declaration, filling, and secret-withholding path as node prompt references, while unchanged routes retain their prior wire shape. I covered the full diff, repository rules, the validation and dispatch callers through the DAG judge, the feature's originating history, backward compatibility, architecture boundaries, and whether tests were weakened; I found no issue worth raising.

Verification: PYTHONUTF8=1 uv run --frozen --python 3.12 --extra dev --dev pytest tests/test_playbook_executor.py tests/test_playbook_validate.py -x (134 passed); Ruff check and format checks passed for all changed files; source-language, commit-message, and git diff --check checks passed. A separate all-extras setup attempt could not build datrie because this host has no GCC, before tests began.

[0xKT]

Not a blocker (review of 0435fe95, main 43159933): the route fill and the route declare-check do what the commit says, and a secret referenced from a route is withheld today, because the fill goes through _fill_text. Two small things for a follow-up:

Severity of this one finding, not a verdict on the pull request. Any blocking findings from this pass are review threads on the changed files; GitHub renders those collapsed, as a file name with no text.

1. Nothing pins that a route withholds secrets. If the route fill at raven/playbook/executor.py:549 is switched to the plain params.fill_param_refs(condition, values) -- which does substitute secrets (fill_param_refs("tok=${params.TOKEN}", {"TOKEN": "s3cr3t"}) gives tok=s3cr3t) -- tests/test_playbook_executor.py and tests/test_playbook_validate.py still pass (134 passed). The route text goes into the judge's request, so a later refactor could send a secret to a model with every test green. test_a_secret_never_reaches_a_dispatched_work_order covers only promptTemplate; adding a secret param to the route in test_a_route_condition_is_filled_before_it_reaches_the_judge and asserting its value is absent from the dispatched nodes would close it.
2. Two texts are now behind the code. _fill_text's docstring (executor.py:60) still says the paths through it are "three"; with the route fill there are more. And docs-site/docs/playbooks.md still describes parameter references as belonging in promptTemplate or prompts only.

[yudongyouqing]

Pinned: the fill test now carries a secret param in the route and asserts the withheld marker and the absence of the value, and I ran your mutant to prove the pin -- filling routes with the plain secret-substituting fill_param_refs fails exactly on the leaked token. _fill_text's docstring counts its paths again, and the playbooks doc names route conditions where parameter references belong. 275 across the playbook suites; the branch is rebased onto current main.

[gloryfromca]

No blockers; this can merge as far as I am concerned.

I reviewed the new regression-test and documentation delta, the full branch diff after its rebase, the surrounding secret-fill and judge-dispatch callers, relevant history, repository rules, backward compatibility, architecture boundaries, and whether the tests were weakened. The new assertion pins both the withheld marker and absence of the supplied secret from dispatched nodes, while the refreshed texts accurately describe route conditions.

Verification: PYTHONUTF8=1 uv run --frozen --python 3.12 --extra dev --dev pytest tests/test_playbook_*.py -x (472 passed). Ruff check and format, source-language, commit-message, large-file, and git diff --check checks also passed.

[LivXue]

No new Windows compatibility regression found in the inspected changes.

Reviewed head 7333c47f9f911d8aac7988eca65c51f4d78a11ed, base/merge base 4315993300e538ec954c79565cbc8585006c8053: complete five-file diff, route declaration/fill/secret-withholding logic, parameter substitution, dispatch/judge callers, documentation, tests and existing discussions.

Native Windows results (Python 3.13.12):

• The executor/validator run stopped at 81 passed, 1 failed: tests/test_playbook_validate.py::test_path_confinement_waits_for_dispatch_roots. All 79 executor cases passed, including the new route fill and secret-withholding regression case.
• The failing path-confinement case also fails at the exact base. The unchanged raven/agent/subagent/prompt_paths.py resolves a drive-qualified path as relative using POSIX path rules, then rejects the resulting combined path. This affects absolute Windows DAG file references, predates this PR, and is covered separately by #1066; it is not re-filed as a route-parameter regression.
• Rerunning the validator with only that confirmed baseline failure deselected: 54 passed, 1 deselected. Thus the two inspected test files have 133 distinct passing cases and the one inherited failure.

Code reading: the new route substitution reuses the existing callable regex replacement, which does not reinterpret backslashes in supplied text; no new filesystem, shell, process or dependency behavior is introduced. Tests use a fake DAG dispatcher, so they do not verify a live judge/model or remote execution. No full repository suite was run. This is a Windows-scoped assessment, not a merge verdict.

Exact verification commands

Runs used detached head/base checkouts, the existing frozen dev environment, source imports pinned to each checkout, and isolated profile/Raven/OAuth state.

# native; exit 1; cwd C:\Users\livxu\AppData\Local\Temp\raven-windows-review\93f210e71847404cb53894de7ac36f62\pr1058\head
uv run --frozen --no-sync pytest tests/test_playbook_executor.py tests/test_playbook_validate.py -n 0 -x --tb=short --basetemp C:\Users\livxu\AppData\Local\Temp\raven-windows-review\93f210e71847404cb53894de7ac36f62\checks\1058-native\pytest --junitxml C:\Users\livxu\AppData\Local\Temp\raven-windows-review\93f210e71847404cb53894de7ac36f62\checks\1058-native\junit.xml

# base-reference; exit 1; cwd C:\Users\livxu\AppData\Local\Temp\raven-windows-review\93f210e71847404cb53894de7ac36f62\base431599
uv run --frozen --no-sync pytest tests/test_playbook_validate.py::test_path_confinement_waits_for_dispatch_roots -n 0 -x --tb=short --basetemp C:\Users\livxu\AppData\Local\Temp\raven-windows-review\93f210e71847404cb53894de7ac36f62\checks\1058-base-reference\pytest --junitxml C:\Users\livxu\AppData\Local\Temp\raven-windows-review\93f210e71847404cb53894de7ac36f62\checks\1058-base-reference\junit.xml

# native-rest; exit 0; cwd C:\Users\livxu\AppData\Local\Temp\raven-windows-review\93f210e71847404cb53894de7ac36f62\pr1058\head
uv run --frozen --no-sync pytest tests/test_playbook_validate.py --deselect=tests/test_playbook_validate.py::test_path_confinement_waits_for_dispatch_roots -n 0 -x --tb=short --basetemp C:\Users\livxu\AppData\Local\Temp\raven-windows-review\93f210e71847404cb53894de7ac36f62\checks\1058-native-rest\pytest --junitxml C:\Users\livxu\AppData\Local\Temp\raven-windows-review\93f210e71847404cb53894de7ac36f62\checks\1058-native-rest\junit.xml

[ypflll]

Approving. The defect is real and the fix is the narrow one.

A route condition reaching the judge as the deal is above ${params.threshold} hinged a decision on text no reader can evaluate, and filling it through the same _fill_text as a prompt is what makes the secret withholding come for free rather than needing its own proof. Keeping the fill at its existing site, and saying plainly that composed-graph nodes were never filled either and stay that way here, is the right scope.

Nonblocking -- lock the secret withholding with a test of its own.

The Risk note says a secret param in a condition is withheld rather than exposed, and that is true today by construction: the condition goes through _fill_text. But nothing fails if that stops being true. A later change that fills a route by any other route -- a direct format, a new helper, an inlined substitution -- leaves the existing tests green while a secret reaches the judge, which is the one reader this PR exists to give readable text to.

The test is close to free: the route version of whichever prompt-side secret test already exists, asserting the withheld marker lands in the condition and the secret itself does not.

Minor, take it or leave it: **({"routes": {...}} if n.routes else {}) builds a conditional dict to avoid setting a key that already has a default. {name: ... for name, condition in (n.routes or {}).items()} is the same result flat -- and is the form validate_graph_nodes uses two files over, so the two would read alike.