fix(router): preserve literal multiline and Windows-path dispatch payloads#481
fix(router): preserve literal multiline and Windows-path dispatch payloads#481
Conversation
… contracts The shared router normalized every intercepted remainder through POSIX shell splitting. That kept natural-language prompts tidy, but it also silently changed payloads that are supposed to be forwarded verbatim, notably multiline inline seed content and Windows-style paths. This patch keeps the shared router narrow: multiline remainders are now recognized by the parser, and exact literal payloads are preserved only for cases where shell normalization is destructive. The existing natural-language normalization path stays intact for the common single- line prompt forms. Constraint: Recent router fixes favor small shared-layer corrections with focused regression coverage Rejected: Split multiline and Windows-path handling into separate PRs | both regressions come from the same remainder-preservation boundary Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep router dispatch exact for literal payloads; do not reintroduce shell normalization on newline-containing or Windows-literal inputs without contract tests Tested: pytest tests/unit/router/test_command_parser.py tests/unit/router/test_extract_first_argument.py tests/unit/router/test_valid_dispatch_normalization.py -q; pytest tests/unit/router -q; pytest tests/integration/test_codex_skill_smoke.py tests/integration/test_codex_skill_fallback.py -q; python -m compileall src/ouroboros/router tests/unit/router/test_command_parser.py tests/unit/router/test_extract_first_argument.py tests/unit/router/test_valid_dispatch_normalization.py Not-tested: Full repository test suite Related: Q00#479 Related: Q00#480
There was a problem hiding this comment.
Review — ouroboros-agent[bot]
Verdict: REQUEST_CHANGES
Reviewing commit
b5dcc1afor PR #481
Review record:
10f72bd8-6035-4e4a-9c1c-ab56f2d5dbed
Blocking Findings
| # | File:Line | Severity | Finding |
|### Recovery Notes
First recoverable review artifact generated from codex analysis log.
---|-----------|----------|---------|
| 1 | src/ouroboros/router/dispatch.py:328 | BLOCKING | The new multiline remainder support does not round-trip through the public ParsedOooCommand entrypoint. resolve_skill_dispatch(parsed, ...) and resolve_parsed_skill_dispatch(parsed, prompt=None, ...) reconstruct Resolved.prompt with f"{parsed.command_prefix} {parsed.remainder}", which inserts a space even when the original separator was a newline. For multiline inline seed content, callers that pass a parsed command now get a different prompt than the one they parsed, losing the exact payload layout that this PR is trying to preserve. |
Non-blocking Suggestions
| 1 | tests/unit/router/test_valid_dispatch_normalization.py:151 | tests | Add a regression test that resolves a multiline ParsedOooCommand directly, not just a raw prompt string. That is the public path currently missing coverage, and it would catch the prompt reconstruction issue above. |
Design Notes
The change is narrowly targeted and the parser/extractor split still makes sense, but the new multiline capability is only wired through the raw-prompt flow. The parsed-command API needs the same exact-text preservation guarantee to keep the router’s public contract consistent.
Reviewed by ouroboros-agent[bot] via Codex deep analysis
Summary
/ouroboros:<skill>dispatch by letting the shared parser carry newline-containing remaindersCloses #479.
Refs #480.
Problem
The shared router introduced in #459 normalizes intercepted remainders through a POSIX shell path:
parse_ooo_command()only matches a single-line remainder because the regex does not span newlinesextract_first_argument()always round-trips the remainder throughshlex.split()and rejoins itThat is fine for natural-language prompts, but it breaks literal payload contracts in two ways:
/ouroboros:runwith multiline inline YAML falls out of the deterministic dispatch path entirely, even thoughskills/run/SKILL.mddocumentsseed_file_or_contentand the MCP tool already supports inlineseed_contentBecause the router is now shared, both failures affect every runtime using deterministic skill dispatch.
What changed
src/ouroboros/router/command_parser.pyre.DOTALLsrc/ouroboros/router/dispatch.pyC:\...,\\server\share\...) for the same reasonTests
tests/unit/router/test_command_parser.py/ouroboros:runremainder stays attached to the commandtests/unit/router/test_extract_first_argument.pytests/unit/router/test_valid_dispatch_normalization.pyWhy this PR is intentionally narrow
Recent merged fixes in this repo tend to land best when they:
This PR follows that shape. It does not change skill routing policy, runtime-specific behavior, or MCP tool contracts. It only preserves router payloads that current docs and tool definitions already imply should survive dispatch unchanged.
Verification
PYTHONPATH=src python3 -m pytest tests/unit/router/test_command_parser.py tests/unit/router/test_extract_first_argument.py tests/unit/router/test_valid_dispatch_normalization.py -qPYTHONPATH=src python3 -m pytest tests/unit/router -qPYTHONPATH=src python3 -m pytest tests/integration/test_codex_skill_smoke.py tests/integration/test_codex_skill_fallback.py -qpython3 -m compileall src/ouroboros/router tests/unit/router/test_command_parser.py tests/unit/router/test_extract_first_argument.py tests/unit/router/test_valid_dispatch_normalization.pyNon-goals
ouroboros run <seed.yaml>CLI contract