Skip to content

fix!: a reserved task name no longer takes down the CLI - #45

Merged
refsz merged 1 commit into
mainfrom
fix/reserved-names-do-not-kill-the-cli
Aug 19, 2026
Merged

fix!: a reserved task name no longer takes down the CLI#45
refsz merged 1 commit into
mainfrom
fix/reserved-names-do-not-kill-the-cli

Conversation

@refsz

@refsz refsz commented Aug 19, 2026

Copy link
Copy Markdown
Owner

The second accepted finding from external review of 0.2.0. Reproduced against the binary before changing anything:

$ sputnik list      Error: Task name 'init' is reserved by a built-in command
$ sputnik deploy    Error: Task name 'init' is reserved by a built-in command

Discovery threw, and discovery runs while the container is built — so a single name collision reached every invocation. No task ran, not even list, and the message named neither the file nor a way out. With several task directories configured, the class name alone leaves you guessing which file to edit.

Structural names: skip the task, keep the CLI

run, list, help, completion, context:switch, context:list stay reserved — losing list would leave no way to reach anything else. A collision now skips that one task:

Skipped task 'list' in /project/sputnik/ListTask.php: the name is reserved by a
built-in command - rename the task or give it a group prefix

Verified on the binary: list works, and an unrelated deploy task in the same directory still runs with exit 0.

A colliding alias drops just the alias. Removing an otherwise valid task over a secondary name would help nobody.

init becomes shadowable

The reviewer's argument holds: a one-off scaffold should not outrank a daily project command. So a project task named init wins, and the built-in says it is gone:

Task 'init' in /project/sputnik/InitTask.php shadows the built-in init command,
which is no longer reachable

This needed no ordering change — Symfony's addCommand() replaces by name, and task commands already register after the core ones.

The part that decided the design

Discovery results are baked into the compiled container via withPreloadedData, so discovery only runs when that cache is cold. A warning collected in the discovery pass alone would appear once and then disappear while the task stayed silently skipped — worse than no warning, because the user would remember seeing it and assume it was resolved.

So warnings travel through withPreloadedData into the container, and Kernel::createApplication() writes them to the shared OutputChannel from #32. Verified against the binary that the warning is still there on the second run, with a warm cache.

Tests

The reserved-name path had no test at all before this — the behaviour that broke a whole CLI was unverified. Now:

  • discovery: a structural collision skips only that task, an unrelated one survives
  • discovery: the warning contains the name, the file, and the word "rename"
  • discovery: a project task may take init, and the shadowing is reported
  • E2E against the real binary: list exits 0 with the warning while deploy still runs; and a project init runs instead of the scaffold (asserting the built-in's own output is absent)

The old E2E test asserted the exact behaviour being removed — that list fails when a task is named init. Rewritten into the two cases above rather than deleted.

Breaking

A task named init used to be an error and now silently wins over the built-in scaffold, with a warning. Reserved-name collisions no longer abort — a task that used to break the CLI loudly now goes missing with a warning, which is the trade this makes deliberately.

vendor/bin/phpunit                        760 tests, 1301 assertions, OK
vendor/bin/phpstan analyse                [OK] No errors
vendor/bin/php-cs-fixer fix --dry-run     0 of 156 files
vendor/bin/rector --dry-run               [OK]
mkdocs build --strict                     clean

🤖 Generated with Claude Code

https://claude.ai/code/session_018CTvnzcNYmFgm2HQcm821A

A project task named after a built-in threw during discovery, and discovery runs
while the container is built - so the exception reached every invocation:

    $ sputnik list      Error: Task name 'init' is reserved by a built-in command
    $ sputnik deploy    Error: Task name 'init' is reserved by a built-in command

One name collision and nothing ran, not even `list`, and the message named
neither the file nor a way out.

Two changes:

The names that carry the CLI - run, list, help, completion, context:switch,
context:list - stay reserved, but a collision now skips that one task and keeps
going. The warning names the file, because with several task directories the
class alone leaves you guessing:

    Skipped task 'list' in /project/sputnik/ListTask.php: the name is reserved by
    a built-in command - rename the task or give it a group prefix

An alias that collides drops just the alias; taking the whole task away over a
secondary name would help nobody.

`init` becomes shadowable. Scaffolding a project happens once, a project command
named init may well be a daily one, so the project wins and the built-in says it
is gone. Symfony's addCommand already replaces by name and tasks register after
the core commands, so this needed no ordering change.

Warnings travel through withPreloadedData into the compiled container. Discovery
only runs when that cache is cold, so a warning that lived in the discovery pass
alone would appear once and then vanish while the task stayed skipped - verified
that it survives a warm cache.

BREAKING: a task named `init` used to be an error and now silently wins over the
built-in scaffold, with a warning. Reserved-name collisions no longer abort.
@refsz
refsz merged commit 838bc3b into main Aug 19, 2026
10 checks passed
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