Merged
Conversation
Coverage Report✅ 88.1% overall coverage
|
There was a problem hiding this comment.
Pull request overview
Addresses project-wide bugbash P1/P2 contract and runtime issues across Milo’s CLI/MCP/JSON-RPC surfaces, gateway child transport handling, state/saga concurrency behavior, schema generation parity, forms/flows behavior, llms.txt output, and docs/snippet validation.
Changes:
- Unify and harden dispatch/transport behavior (Context injection parity; stricter JSON-RPC request parsing; correct method-not-found handling; gateway child progress interleaving handling).
- Improve state/runtime concurrency and teardown (serialized listeners, safer shutdown/cancellation semantics, unblock
Take,Allbehavior under constrained workers). - Align schema + docs outputs (Literal schemas with types, required-ness rules, hyphenated llms.txt flags, docs snippet scanning improvements, and docs updates).
Reviewed changes
Copilot reviewed 62 out of 62 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_state.py | Adds concurrency/shutdown/all-effect tests for Store/sagas. |
| tests/test_schema_v2.py | Updates Literal schema expectations to include type. |
| tests/test_mcp_transport.py | New transport-level JSON-RPC behavior tests (invalid request, unknown method, notification). |
| tests/test_mcp_handler.py | Ensures MCP handler tool-cache invalidation when groups/commands change. |
| tests/test_form.py | Updates select default semantics + adds Shift+Tab and validator contract tests. |
| tests/test_flow.py | Tests propagation of cmds/view from screen reducers in flows. |
| tests/test_docs_snippets.py | Adds coverage for missing-path reporting and indented tagged fences. |
| tests/test_commands_core.py | Adds powershell completion mode test coverage. |
| tests/test_command_contract.py | Expands contract tests for Context injection, required bools, Literal CLI/MCP parity, llms.txt flag naming. |
| tests/test_child.py | Verifies child transport skips interleaved progress notifications until matching response id. |
| tests/test_app.py | Adds renderer truncation test around ANSI + terminal width handling. |
| tests/test_ai_native.py | Updates required-ness expectation and llms.txt flag formatting assertions. |
| src/milo/state.py | Serializes listeners, tracks/cancels root saga contexts on shutdown, unblocks Take waiters, adjusts Race/All execution strategy. |
| src/milo/schema.py | Adjusts required-ness rules and improves Literal schema emission with JSON types. |
| src/milo/mcp.py | Uses shared JSON-RPC parsing and maps method-not-found to -32601; suppresses notification responses. |
| src/milo/llms.py | Formats llms.txt parameters/examples using hyphenated CLI flag names. |
| src/milo/groups.py | Adds change propagation hook to invalidate CLI discovery caches on group mutations. |
| src/milo/gateway.py | Uses shared JSON-RPC parsing; correct notification behavior; maps method-not-found to -32601. |
| src/milo/form.py | Select field initialization uses choice values; Shift+Tab navigation; validator contract normalization helper. |
| src/milo/flow.py | Propagates cmds and view through flow reducers alongside sagas/Quit. |
| src/milo/commands.py | Centralizes command version bumping; invalidates caches on group changes; adds powershell completions; injects Context in call/call_raw. |
| src/milo/app.py | Moves resize dispatch to a queue/thread; ensures store shutdown on non-TTY; truncates lines to terminal width via cell-width helper. |
| src/milo/_mcp_router.py | Introduces MethodNotFoundError for clearer routing error classification. |
| src/milo/_jsonrpc.py | Adds _parse_request helper for robust JSON-RPC line validation and error reporting. |
| src/milo/_child.py | Reads until matching response id, skipping interleaved notifications; improves initialization consumption. |
| site/content/docs/reference/types.md | Updates page icon metadata. |
| site/content/docs/reference/schema.md | Updates schema docs for required/optional unions and typed Literal enums; icon tweak. |
| site/content/docs/reference/dispatch.md | Updates page icon metadata. |
| site/content/docs/reference/_index.md | Updates page icon metadata. |
| site/content/docs/quality/testing.md | Updates page icon metadata. |
| site/content/docs/quality/_index.md | Updates icons and card links (trailing slashes). |
| site/content/docs/get-started/quickstart.md | Updates cards’ icons and links (trailing slashes). |
| site/content/docs/get-started/migrate-existing-cli/from-typer.md | Updates page icon metadata. |
| site/content/docs/get-started/migrate-existing-cli/from-fire.md | Updates page icon metadata. |
| site/content/docs/get-started/migrate-existing-cli/from-click.md | Updates page icon metadata. |
| site/content/docs/get-started/migrate-existing-cli/_index.md | Updates page icon metadata. |
| site/content/docs/get-started/_index.md | Updates icons and card links (trailing slashes). |
| site/content/docs/examples/_index.md | Updates icons for examples section. |
| site/content/docs/build-clis/output.md | Updates page icon metadata. |
| site/content/docs/build-clis/mcp.md | Updates page icon metadata. |
| site/content/docs/build-clis/groups.md | Updates page icon metadata. |
| site/content/docs/build-clis/context.md | Updates example to use ctx.dry_run and correct flag placement; icon tweak. |
| site/content/docs/build-clis/_index.md | Updates icons and card links (trailing slashes). |
| site/content/docs/build-apps/templates.md | Updates page icon metadata. |
| site/content/docs/build-apps/sagas.md | Updates page icon metadata and tab icon. |
| site/content/docs/build-apps/plugins.md | Updates page icon metadata. |
| site/content/docs/build-apps/live.md | Updates page icon metadata. |
| site/content/docs/build-apps/input.md | Updates page icon metadata. |
| site/content/docs/build-apps/forms.md | Updates page icon and recommends make_form_reducer() usage in docs. |
| site/content/docs/build-apps/flows.md | Updates page icon metadata. |
| site/content/docs/build-apps/_index.md | Updates icons and card links (trailing slashes). |
| site/content/docs/applied-tutorials/build-a-wizard.md | Updates example to use make_form_reducer() instead of manual form_reducer wrapping. |
| site/content/docs/applied-tutorials/build-a-counter.md | Fixes category typo and updates icons/links. |
| site/content/docs/applied-tutorials/_index.md | Updates icons and card links (trailing slashes). |
| site/content/docs/about/when-to-use.md | Updates page icon metadata. |
| site/content/docs/about/concepts/return-values.md | Updates page icon metadata. |
| site/content/docs/about/concepts/app-lifecycle.md | Updates page icon metadata. |
| site/content/docs/about/concepts/_index.md | Updates page icon metadata. |
| site/content/docs/_index.md | Updates icons across top-level docs cards. |
| scripts/check_docs_snippets.py | Allows indented fences and reports missing paths as errors. |
| docs/agent-quickstart.md | Clarifies structured vs text MCP content behavior in quickstart rules. |
| changelog.d/bugbash-contracts.fixed.md | Adds changelog fragment for bugbash fixes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
91
to
95
| # Write each line, clearing to end of line | ||
| for line in lines: | ||
| # Truncate to terminal width to avoid wrapping artifacts | ||
| sys.stdout.write(line[:cols]) | ||
| sys.stdout.write(cell_truncate(line, cols, marker="")) | ||
| sys.stdout.write("\033[K\n") # Clear to end of line |
Comment on lines
771
to
777
| self._run_saga_capturing(saga, ctx, rb, eb, done) | ||
| with condition: | ||
| condition.notify_all() | ||
|
|
||
| self._executor.submit(_notify_wrapper) | ||
| threading.Thread(target=_notify_wrapper, daemon=True).start() | ||
|
|
||
| # Wait for first completion or parent cancellation |
Comment on lines
831
to
837
| self._run_saga_capturing(saga, ctx, rb, eb, done) | ||
| with condition: | ||
| condition.notify_all() | ||
|
|
||
| self._executor.submit(_notify_wrapper) | ||
| threading.Thread(target=_notify_wrapper, daemon=True).start() | ||
|
|
||
| # Wait for all to complete or first failure |
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
Addresses the project-wide bugbash P1/P2 findings across CLI dispatch, MCP/JSON-RPC transport, gateway child process handling, app/state concurrency, schema generation, forms/flows, llms.txt, and docs.
What changed
Contextconsistently forCLI.call,CLI.call_raw, and MCPtools/call.Store.dispatch, serialize listeners, and make shutdown cancel blocked root sagas.Literalschemas, and llms.txt hyphenated flags.Root cause
Several shared contracts had drifted by surface: CLI invocation, programmatic calls, MCP dispatch, schema emission, and docs examples were each exercising slightly different assumptions. Runtime concurrency also had direct signal/listener paths that could re-enter or overlap terminal rendering under free-threading.
Validation
make lintpassedmake typassed with existing warnings inpipeline.pyand callable__qualname__handling inschema.pymake test-covpassed:1492 passed, 1 skipped, coverage81.82%,PYTHON_GIL=0make docs-testpasseduv run bengal build --environment production --no-incremental --clean-output --strict --full-output --log-file /tmp/milo-bengal-build.logexits 0; Bengal still reports generated fragment-link health warnings for step anchorsSteward Notes
Consulted root constitution and affected steward domains: CLI dispatch, MCP/gateway/JSON-RPC, schema, app/state runtime, forms/flows, docs/scaffold verification.
Decisions:
Risks:
Takewaiters.Follow-up: