Skip to content

refactor(cli): extract the shared resource output policy - #6597

Closed
SaxonF wants to merge 1 commit into
notebooks/02-api-deepobject-queryfrom
notebooks/03-cli-resource-output
Closed

SaxonF wants to merge 1 commit into
notebooks/02-api-deepobject-queryfrom
notebooks/03-cli-resource-output

Conversation

@SaxonF

@SaxonF SaxonF commented Sep 14, 2026

Copy link
Copy Markdown

Pure refactor, no behavior change. Prepares the ground for the notebooks command family, which needs exactly the same -o/--output policy that compute already had.

What changed

compute.output.ts had grown a generic output policy for structured resources that have no command-specific encoding:

  • which -o values answer with a payload (an allowlist, so a format the global flag learns later does not silently start emitting TOML),
  • how that payload is encoded (json / yaml / toml through the generic encoders),
  • an up-front refusal of -o env, whose flat KEY=value shape cannot hold a structured payload.

None of that is compute-specific, so it moves to command-internal/resource-output.ts as resourceOutput(EnvNotSupportedError), parameterized by the command family's own env-not-supported error so each family keeps its own error tag and actionability. compute.output.ts now builds its three exports from it and keeps only computeRenderFormat and its retry-suggestion helper.

Reviewer notes

The extraction dropped the long explanatory comments that lived on the compute versions. Worth deciding as part of review whether the allowlist rationale in particular should be carried onto the shared helper, since it now governs two command families rather than one.

@SaxonF
SaxonF added this pull request to stack #6600 September 14, 2026 11:20
@SaxonF
SaxonF force-pushed the notebooks/02-api-deepobject-query branch from a1c9cf8 to 3ee8ed9 Compare September 14, 2026 23:50
`compute.output.ts` grew a generic `-o`/`--output` policy for structured
resources that have no command-specific encoding: which formats answer
with a payload, how the payload is encoded, and an up-front refusal of
`-o env`. None of it is compute-specific.

Move it to `command-internal/resource-output.ts` as `resourceOutput`,
parameterized by the command family's env-not-supported error, and have
`compute.output.ts` build its three exports from it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@SaxonF
SaxonF force-pushed the notebooks/03-cli-resource-output branch from 47077c2 to b89c657 Compare September 15, 2026 00:12
@SaxonF

SaxonF commented Sep 15, 2026

Copy link
Copy Markdown
Author

Closing: dropped from the notebooks stack to keep that work isolated.

Upstream #6554 replaced experimental/workers with experimental/compute while this stack was in flight, so this extraction would now have to edit a command family that already ships, purely to make room for notebooks. That is more review surface and more risk than the feature itself warrants.

Instead, notebooks.output.ts in #6598 carries its own copy of the policy, with a comment recording that it duplicates compute.output.ts and that the two are expected to agree. compute is left exactly as it is on develop.

Worth doing as a standalone refactor once a third command family needs the same policy — at that point it can be reviewed on its own rather than inside a feature PR.

@SaxonF SaxonF closed this Sep 15, 2026
@SaxonF
SaxonF removed this pull request from stack #6600 September 15, 2026 00: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