-
Notifications
You must be signed in to change notification settings - Fork 516
feat(workers): bring the command family's output onto one shape #6389
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
johnstonmatt
wants to merge
3
commits into
FUNC-840/select-workers-new-name
from
FUNC-851/general-output-polish
+839
−65
Open
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
a74e3f9
feat(workers): bring the command family's output onto one shape
johnstonmatt 548a185
test(cli): guard every legacy boolean flag against a required default
johnstonmatt 1d7e06b
chore(workers): describe behaviour rather than its history in comments
johnstonmatt File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
71 changes: 71 additions & 0 deletions
71
apps/cli/src/legacy/cli/legacy-boolean-flag-defaults.unit.test.ts
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| import { describe, expect, it } from "vitest"; | ||
| import { Primitive, type Command } from "effect/unstable/cli"; | ||
| import { | ||
| legacyCommandInternals, | ||
| legacyFlattenSubcommands, | ||
| legacyUserGlobalFlagParams, | ||
| } from "../docs/legacy-docs-introspection.ts"; | ||
| import { legacyUnwrapParam } from "../shared/legacy-param-introspection.ts"; | ||
| import { legacyRoot } from "./root.ts"; | ||
|
|
||
| /** | ||
| * `Flag.boolean(name)` builds a bare `Single` param, and a bare `Single` is | ||
| * *required* — omitting it fails the whole command with a missing-flag error | ||
| * before the handler ever runs. Every boolean flag therefore has to be closed | ||
| * off with `Flag.withDefault(false)` or `Flag.optional`. | ||
| * | ||
| * Nothing else catches this: handler integration tests build their flags record | ||
| * directly, so they never touch the parser, and the required-ness is invisible | ||
| * to the type checker because a required boolean flag still infers as | ||
| * `boolean`. The flag only misbehaves when a real invocation omits it, which is | ||
| * precisely the invocation no handler test makes — so the guard walks the | ||
| * command tree instead of waiting for a command to be exercised end to end. | ||
| */ | ||
|
|
||
| /** | ||
| * The published getter for a primitive's kind — `Primitive.getTypeName`, whose | ||
| * own doc example pins `Primitive.boolean` to `"boolean"`. Reading | ||
| * `primitiveType._tag` instead would couple this guard to effect's runtime | ||
| * representation, which this repo forbids in tests as well as in source. | ||
| * | ||
| * Derived from `Primitive.boolean` rather than written as the literal | ||
| * `"boolean"`: were that name to change upstream, a hardcoded literal would | ||
| * match nothing and leave the guard silently passing every command, which is | ||
| * the one failure mode a regression test must not have. | ||
| */ | ||
| const BOOLEAN_TYPE_NAME = Primitive.getTypeName(Primitive.boolean); | ||
|
|
||
| function booleanFlagsRequiringAValue(command: Command.Command.Any): ReadonlyArray<string> { | ||
| const internals = legacyCommandInternals(command); | ||
| // All three parameter sets a command can be parsed with, not just its own: | ||
| // `Command.withSharedFlags` puts inherited flags on `contextConfig`, and the | ||
| // root's persistent flags arrive as `globalFlags`. A bare boolean introduced | ||
| // through either would break every command that inherits it while a guard | ||
| // reading only `config.flags` stayed green. | ||
| const params = [ | ||
| ...internals.config.flags, | ||
| ...internals.contextConfig.flags, | ||
| ...legacyUserGlobalFlagParams(command), | ||
| ]; | ||
|
|
||
| // Throws rather than skipping if effect's internal shape moves, so this | ||
| // cannot quietly degrade into a test that inspects nothing. | ||
| const own = params.flatMap((flag) => { | ||
| const unwrapped = legacyUnwrapParam(flag); | ||
| if (unwrapped === undefined) { | ||
| throw new Error(`Unrecognizable flag param on "${command.name}".`); | ||
| } | ||
| const { single, isOptional } = unwrapped; | ||
| return Primitive.getTypeName(single.primitiveType) === BOOLEAN_TYPE_NAME && !isOptional | ||
| ? [`${command.name} --${single.name}`] | ||
| : []; | ||
| }); | ||
|
|
||
| return [...own, ...legacyFlattenSubcommands(command).flatMap(booleanFlagsRequiringAValue)]; | ||
| } | ||
|
|
||
| describe("legacy boolean flag wiring", () => { | ||
| it("gives every boolean flag a default, so omitting it is not a parse error", () => { | ||
| expect(booleanFlagsRequiringAValue(legacyRoot)).toEqual([]); | ||
| }); | ||
| }); |
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
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
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
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When inspecting local traces or the NDJSON exporter for
supabase experimental workers list, this now recordslegacy.workers.list, even though the preceding command-move commit deliberately renamed it tolegacy.experimental.workers.listto match the new command route. The same stale rename affects all five worker handlers, making traces inconsistent with the invoked command and breaking filters keyed to the established span names; restore thelegacy.experimental.workers.*names.AGENTS.md reference: apps/cli/AGENTS.md:L338-L338
Useful? React with 👍 / 👎.