Skip to content

Address review feedback: launcher safety, exact render output, cli test errors, examples in --help - #60

Merged
jaykv merged 12 commits into
mainfrom
chore/review-feedback
Oct 12, 2026
Merged

jaykv merged 12 commits into
mainfrom
chore/review-feedback

Conversation

@jaykv

@jaykv jaykv commented Oct 12, 2026 •

Copy link
Copy Markdown
Owner

Stack 9 of 9. Base: fix/known-issues. Merge in order; each PR shows only its own changes.

Summary

Addresses the Sourcery review comments left on #52, #53, #54 and #56. Each comment was checked against the top of the stack first: four were already fixed by later PRs, one was already in place and gained a check, and the rest are fixed here, each with a regression test written first. Replies on the original threads point here.

What changed

Loaded CLIs, launchers and removal (#54 comments)

  • A name with path separators or .. is rejected by rm, update, info, build and docs. Before, cli rm ../outside deleted files one level above cliffy's directories.
  • A launcher counts as cliffy's only if it is a regular file, not a link, whose text equals the launcher for that CLI name. Anything else stops load and rm with a message. Before, any file containing from cliffy.clis. was overwritten.
  • Launchers and modules are written to a temp file and renamed; new launchers are created exclusively; links are never followed.
  • Removal goes launcher, module, metadata. If the launcher is not cliffy's, nothing is removed, the CLI stays listed and runnable, and the command exits 1.

Concurrency (#54 comments)

  • Every run and test run imports from its own temporary directory; the process-wide TEMP_FILES list is gone.
  • cli dev reloads run one at a time and the last change wins. A second save within one second is no longer dropped.

Output and errors (#52 comments on promises in SOUL.md)

  • cli render x.yaml > x.py is byte for byte the generated module: no line folding, and the status line goes to stderr. Same for cli init --render.
  • cli test reports a command that raises as command `x` raised Error: message with the run-block line, instead of a traceback through generated code. --traceback shows the full one.
  • Generated CLIs show the manifest's examples in an Examples section of the root --help, for both backends.
  • SOUL.md principle 1 now says which inputs a CLI depends on (the manifest, its includes and blueprint, the environment variables it reads, the cliffy version) instead of promising identical output unconditionally.

Packaging, release, examples, tests (#53, #54, #56 comments)

  • The PyPI upload uses --check-url, so a half-finished upload can be completed by re-running the job.
  • A test and a release-build step check that cliffy/py.typed ships in the wheel and sdist (the file was already there).
  • A requires entry starting with - is a validation error and can no longer reach pip as an option; requirements follow -- in the pip call.
  • The MCP example server answers invalid JSON-RPC messages with an error instead of exiting.
  • The builtin-name codegen test now invokes the print command; the symlink test skips only where symlinks cannot be created.

Behaviour changes a user would notice

  • cli render: no wrapping; the "Rendered" line is on stderr.
  • cli test: one-line error reports instead of tracebacks; new --traceback flag; a shell step whose program is missing ends the run with exit 1.
  • cli rm / rm-all exit 1 and remove nothing when a launcher was edited or is not cliffy's. cli load refuses to replace an edited launcher.
  • Root --help of a generated CLI gains an Examples section when the manifest has examples. Output is unchanged for manifests without them.
  • cli dev: a second save within a second triggers a reload.

Tests that are not pure additions

  • test_delete_temp_files and the TEMP_FILES assertions were removed together with the global and function they tested; tests of the per-run temporary directories replace them.
  • test_unload_keeps_foreign_executable now expects unload to refuse.
  • One test may now skip: the symlink test, only when creating a symlink raises.

How verified

Python 3.11 and 3.15: ruff format --check, ruff check, mypy cliffy --strict, mypy tests, pytest -n auto (843 passed, 1 skipped: the opt-in native build), the same with GITHUB_ACTIONS=true, make docs-check, mkdocs build --strict.

Left open

  • Examples appear in the root --help only. Commands have no examples field in the manifest; adding one is a schema decision.
  • Replacing or deleting cliffy's own existing launcher is still check-then-rename. A file another process puts at that exact path in between is replaced. Links are never followed, so nothing outside the bin directory is touched.
  • A requires entry that is not a valid requirement but does not look like an option is still rejected by pip rather than by the manifest.
  • With Typer, emoji codes such as :rocket: in an example render as emoji.

🤖 Generated with Claude Code

Summary by Sourcery

Harden CLI lifecycle and development workflows while making generated output, help examples, testing diagnostics, packaging, and example-server behavior more reliable.

New Features:

  • Show manifest examples in the generated CLI root help for both supported backends.
  • Add a traceback opt-in for CLI tests while presenting command and assertion failures in concise manifest-oriented messages.

Bug Fixes:

  • Prevent unsafe CLI names, foreign or modified launchers, symbolic links, and partial removals from accessing or deleting unintended files.
  • Preserve concurrent development reload correctness and isolate temporary generated-code imports across runs.
  • Ensure rendered output is exact generated source and status messages are kept off stdout.
  • Return JSON-RPC errors for invalid MCP example messages instead of terminating the server.
  • Reject option-like dependency entries before they can affect pip installation.

Enhancements:

  • Make launcher and generated-file deployment atomic and exclusive, and require exact launcher ownership for replacement or removal.
  • Clarify the reproducibility principle to account for manifest inputs, environment variables, and cliffy version.

CI:

  • Verify that the type-hints marker is included in release wheels and source distributions.

Deployment:

  • Allow interrupted PyPI uploads to be safely completed by rerunning the publish job.

Documentation:

  • Update guides, references, changelog, and known issues for launcher safety, exact rendering, test diagnostics, examples in help, reload behavior, and dependency validation.

Tests:

  • Add regression coverage for launcher ownership and safety, path validation, atomic writes, isolated imports, reload ordering, exact rendering, manifest-oriented test errors, examples in help, MCP protocol errors, and packaging metadata.

jaykv and others added 12 commits October 12, 2026 16:44
The print_command case passed --version and never ran the generated command.
It now runs the command and asserts its output; --version stays as its own case.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- A name given to rm, update, info, build and docs is validated before it is
  used in a path: ../x no longer selects metadata, modules or launchers outside
  of cliffy's directories. homer and Loader refuse such names themselves.
- A launcher is only cliffy's when it is a regular file, not a link, whose text
  is the launcher of exactly that CLI. A file that merely contains
  'from cliffy.clis.' is no longer overwritten or deleted.
- Launchers and modules are written to a temporary file and renamed into place.
  A new launcher is created exclusively; links are never followed.
- cli rm removes the launcher, the module and then the metadata record. When
  the launcher is not cliffy's, nothing is removed and the CLI stays listed
  and runnable. cli rm-all exits with 1 when a CLI could not be removed.
- The symlink test skips where symbolic links cannot be created.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
run_cli and Tester appended their file to a process-wide TEMP_FILES list and
then deleted everything on it, so a run that finished removed the file a run
on another thread was about to import. The generated code is now imported
from a TemporaryDirectory that belongs to the call. TEMP_FILES and
delete_temp_files are gone; their tests are replaced by tests of
import_generated_code.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Every change event started a reload thread of its own. A reload that had read
the older manifest could finish last and load the older CLI over the newer one.
Reloads now hold one lock and read the manifest only once it is their turn; a
reload that waits behind one for a newer change is skipped. The CLI of
--run-cli runs outside the lock.

An event within a second of the last reload is still treated as a duplicate
report of the same save, but only when the watched files are unchanged: a
second save within that second used to be dropped.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
cli render and cli init --render printed through rich with overflow="fold".
Lines longer than the terminal, or than 80 columns when redirected, were
broken, so cli render x.yaml > x.py was not the module that runs. Redirected
output is now the generated text character for character; on a terminal the
text is highlighted and left to the terminal to wrap. The 'Rendered ...'
line goes to stderr.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- A command that raises is reported as the command (or callback) it is in the
  manifest, with the exception and the line of its run block, instead of
  '<Result RuntimeError(...)>'. cli test --traceback adds the full traceback.
- An assertion that raises something other than AssertionError, or is not
  valid Python, is reported with its line. Both printed a traceback through
  cliffy/cli.py and cliffy/tester.py.
- A manifest whose generated code does not import, and a shell step whose
  program cannot be started, end the run with an error message instead of a
  traceback.

The generated CLI records which scripts each function was generated from
(CLI.blocks), which is what maps a line of generated code to a manifest line.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
SOUL principle 7 promises examples in the help, but `examples` was only read
by cli docs. Both backends now put an Examples section into the epilog of the
CLI: one paragraph per example, `$ command  # description`, after an epilog
set in cli_options. The text is a string literal in the generated code like
every other manifest string, and for Typer brackets are escaped so that rich
markup does not drop `[home]` from a command line. A manifest without
examples generates the same code as before.

examples/hello.yaml and examples/click_hello.yaml carry examples and a test
for them.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A message that is valid JSON but not an object (an array, a string, a number)
raised AttributeError in _handle_message and ended the server. It is answered
with -32600 Invalid Request, as is a batch array (MCP has no batches since
2025-06-18) and an object without a method. params or arguments that are not
objects get -32602. A response object is ignored. The server keeps serving.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
cli build does not check requires against the installed packages, so an entry
such as --index-url=... or -r file reached pip's argument list as an option
and changed where it installs from. A manifest or include with such an entry
is rejected when it is read, build_cli rejects it for a loaded CLI, and the
requirements follow -- in the pip call. Every subprocess call in cliffy/ is
an argument list without a shell.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
uv publish --check-url skips files that are on PyPI already with the same
content, so a release whose upload stopped half way can be finished by
re-running the job. The build job and a test check that the wheel and the
sdist carry cliffy/py.typed and the Typing :: Typed classifier, which they do.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A manifest can read environment variables and include files, and its header
names the cliffy version, so 'the same manifest produces the same CLI on every
machine, every time' was not true as written. The promise is now: the same
manifest with the same includes, blueprint and environment variables, read by
the same cliffy, gives the same CLI, and cliffy adds no input of its own.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Changelog and known issues for what changed on this branch; the guides for
cli test output and --traceback, examples in --help, cli render to a file,
launcher ownership and removal. The examples field description, the JSON
schema and the generated reference pages follow.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@sourcery-ai

sourcery-ai Bot commented Oct 12, 2026

Copy link
Copy Markdown

Sorry @jaykv, you've used your own review budget of 250,000 diff characters for the last 7 days.

You can request another review in 6 days and 20 hours by commenting @sourcery-ai review. Upgrade to get a review now.

@coderabbitai

coderabbitai Bot commented Oct 12, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 802a57fd-e817-4802-841c-a6202e0d09a6

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@sourcery-ai

sourcery-ai Bot commented Oct 12, 2026

Copy link
Copy Markdown

Reviewer's Guide

This PR hardens CLI filesystem operations and concurrent execution, makes rendering and test failures deterministic and actionable, adds manifest examples to generated root help, blocks pip-option injection, improves release and MCP-example behavior, and updates the associated tests and documentation.

Sequence diagram for safe CLI launcher loading and removal

sequenceDiagram
    participant User
    participant CLI
    participant Loader
    participant Filesystem
    User->>CLI: load or rm cli_name
    CLI->>Loader: validate_cli_name(cli_name)
    Loader->>Filesystem: script_state(script_path, cli_name)
    alt foreign launcher
        Filesystem-->>Loader: foreign
        Loader-->>CLI: LoaderError
        CLI-->>User: refuse and leave CLI loaded
    else owned or missing launcher
        alt load
            CLI->>Loader: load_from_cli(cli)
            Loader->>Filesystem: write_atomically
        else remove
            CLI->>Loader: unload_cli(cli_name)
            Loader->>Filesystem: unlink launcher
            Loader->>Filesystem: unlink module
            CLI->>Filesystem: remove_metadata(cli_name)
        end
        CLI-->>User: success
    end
Loading

Sequence diagram for manifest-based CLI test error reporting

sequenceDiagram
    participant User
    participant CLI
    participant Tester
    participant GeneratedCLI
    User->>CLI: test manifest
    CLI->>Tester: Tester(manifest)
    Tester->>Tester: import_generated_code
    CLI->>Tester: invoke_test(command, assert_script)
    Tester->>GeneratedCLI: invoke command
    alt command raises
        GeneratedCLI-->>Tester: Result with exception and exc_info
        CLI->>Tester: describe_exception(command, result)
        Tester-->>CLI: manifest command and run-block location
        CLI-->>User: one-line actionable error
        opt show_traceback
            CLI-->>User: full traceback
        end
    else command succeeds
        GeneratedCLI-->>Tester: Result
        Tester->>Tester: execute assertion
        CLI-->>User: test result
    end
Loading

Flow diagram for manifest examples in generated CLI help

flowchart LR
    Manifest[Manifest examples] --> Commander[Commander examples_epilog]
    Commander --> Options[cli_options_with_examples]
    Options --> Click[Click root help]
    Options --> Typer[Typer root help]
    Click --> Help[Examples section]
    Typer --> Help
Loading

File-Level Changes

Change Details Files
Harden loaded-CLI name and launcher handling against path traversal, foreign files, symlinks, partial writes, and incomplete removal.
  • Validate CLI names before filesystem lookup or mutation.
  • Identify owned launchers by exact regular-file content and avoid following links.
  • Write launchers and generated modules atomically, create new launchers exclusively, and roll back removal on failure.
  • Remove launcher, module, and metadata only as an all-or-nothing operation; preserve loaded CLIs when launchers are foreign or edited.
cliffy/helper.py
cliffy/loader.py
cliffy/homer.py
cliffy/cli.py
tests/test_helper.py
tests/test_homer.py
tests/test_loader.py
Make generated-code execution isolated and improve development reload correctness under concurrency.
  • Replace the process-wide temporary-file registry with per-run temporary directories.
  • Serialize reloads, skip superseded generations, and detect distinct saves during the debounce interval.
  • Add regression coverage for concurrent runs and reload races.
cliffy/helper.py
cliffy/builder.py
cliffy/tester.py
cliffy/reloader.py
tests/test_builder.py
tests/test_reloader.py
tests/test_tester.py
Provide exact render output and actionable, manifest-oriented CLI test diagnostics.
  • Send only generated source to stdout for render commands and status messages to stderr without line folding.
  • Add --traceback while defaulting to concise command, assertion, import, and shell-start errors.
  • Track generated function blocks to map runtime failures back to manifest scripts and assertion lines.
cliffy/cli.py
cliffy/commander.py
cliffy/tester.py
cliffy/rich.py
tests/test_cli.py
tests/test_tester.py
Expose manifest examples in generated root help for both Click and Typer backends.
  • Build an examples epilog while preserving existing CLI epilogs and example text.
  • Escape Typer rich-markup syntax so copied examples render literally.
  • Add backend-specific coverage for ordering, formatting, safety, and command-help exclusion.
cliffy/commander.py
cliffy/commanders/click.py
cliffy/commanders/typer.py
cliffy/manifest.py
examples/hello.yaml
examples/click_hello.yaml
tests/test_codegen.py
Prevent dependency-option injection and improve packaging and example-server robustness.
  • Reject requires entries beginning with '-' during manifest and loaded-CLI builds and terminate pip options before requirements.
  • Verify py.typed in release artifacts and make PyPI publishing retries idempotent with --check-url.
  • Return JSON-RPC errors for malformed messages and invalid parameters instead of terminating the MCP server.
  • Update the builtin-name test to exercise the print command and conditionally skip unsupported symlink tests.
cliffy/builder.py
cliffy/helper.py
cliffy/manifest.py
.github/workflows/publish.yaml
examples/blueprints/mcp_tools.py
tests/test_builder.py
tests/test_blueprint.py
tests/test_packaging.py
tests/test_codegen.py
tests/test_loader.py
Align project documentation, release notes, and generated examples with the new behavior.
  • Document launcher ownership, reload ordering, exact render output, test diagnostics, help examples, requirement validation, and known limitations.
  • Update SOUL.md to qualify reproducibility by explicit inputs and cliffy version.
  • Regenerate example artifacts and reference metadata.
SOUL.md
docs/guides/dev-loop.md
docs/guides/generating-docs.md
docs/guides/manifest-basics.md
docs/guides/testing.md
docs/project/changelog.md
docs/project/known-issues.md
docs/quickstart.md
docs/reference/cli.md
docs/reference/examples.md
docs/reference/manifest.md
scripts/docs.py
examples/generated/clickhello.py
examples/generated/hello.py
examples/cliffy_schema.json

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@jaykv
jaykv changed the base branch from fix/known-issues to main October 12, 2026 21:23
@jaykv
jaykv merged commit cf5ca6f into main Oct 12, 2026
8 checks passed
@jaykv
jaykv deleted the chore/review-feedback branch October 12, 2026 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant