mirror of
https://github.com/vercel/workflow.git
synced 2026-09-14 19:59:43 +08:00
Karthik Kalyan
f9073d0739
* Add attribute inspection to the CLI and probe the cancel window once
`wf inspect attributes` lists the distinct attribute keys on a project's
runs with their run counts and first/last seen times, and
`wf inspect runs --attribute key=value` filters by them. Between them
they turn attributes from something you can only write into something
you can discover and query. Both are analytics-only — storage has no
cross-run attribute index — so the listing says so rather than falling
back, and the filter warns and is ignored the way --since/--until
already do.
The flag is parsed and bounded in lib/inspect so the error names
--attribute rather than the parameter it becomes, and so it is testable
next to the other inspect flag helpers. It splits on the first `=` only,
since a value may contain one, and keeps an empty value, which matches
runs whose attribute was set to the empty string.
`wf cancel` also probed the plan's listing window inside its per-status
fan-out, so a four-status cancel issued four identical probes. The
window is a property of the plan rather than of a status, so the probe
is hoisted above the fan-out: eight requests become five. The harness
only ever modelled the storage path, so that probe logic had no
coverage; the new test fails with two probes before the change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Do not depend on an unreleased world export for the flag cap
The --attribute cap was imported from @workflow/world, where the
constant is added by a different branch, so on main it resolved to
undefined and `values.length > undefined` was always false: the flag
accepted any number of pairs and the test for it never threw.
Declare the cap in the CLI instead. The World and the backend enforce
the same bound independently, and this copy exists only so the error can
name the flag the user typed rather than the parameter it becomes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Degrade sleeps to the event log and bound the inspect flags
`wf inspect sleeps` was the only list path that could not degrade: it
branched on analytics being present and either returned or exited, so on
any backend providing analytics the storage branch below it was
unreachable and an analytics failure ended the command. It now warns and
falls through, like the run, step, and event listings. An argument the
World rejected is not retried — the same argument fails either path, so
falling back would trade a precise message for a slower failure.
handleApiError also only recognised errors carrying an HTTP status.
A client-side argument rejection has none, because no request was made,
so it fell past every branch and was rethrown as an unhandled error. It
is now reported as given: the message already names the method, the
parameter, and what it received.
--limit and --runId are checked before any backend setup so a mistyped
value names the flag and costs no round trip. The limit bound is
deliberately looser than the per-endpoint caps, which differ by resource
and stay with the World; this one catches a typo'd digit or a negative.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Scope --attribute to inspect and document the inspect flags
--attribute was added to the shared cliFlags, which cancel, health,
start, and web all spread — so `workflow health --attribute k=v` parsed
and was silently ignored. It belongs with the other inspect-only
filters in the command's own flags, next to --runId and --since.
The configuration reference documented every shared flag but none of
the inspect-only ones, so --runId, --stepId, --hookId, --since/--until,
--withData and --decrypt had no entries at all. They now do, in an
Inspect filtering section, alongside --attribute. --status and
--workflowName were documented under bulk cancel only; both also filter
inspect listings, which is now noted where they are.
--limit's entry described a default with no bound and is now rejected
outside 1 to 1000, so it says so, and points out that individual
listings cap lower.
The attributes guide claimed filtering was available "through the
Analytics API", which is no longer the whole story: the CLI can now
discover keys and filter by them, so that section splits into a CLI half
and an API half.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Stop dropping inspect flags silently
Three flags the caller typed were being discarded without saying so —
the same failure the World argument guards were added to remove,
reintroduced one layer up.
--attribute and --since/--until warned that the backend has no
analytics read path, but that condition is also false when --withData
asks for payloads, which only storage carries. Blaming the backend for
the caller's own flag sends them looking in the wrong place, so the
warning now names whichever applies.
inspect attributes dropped --sort entirely, explained only by a code
comment. It is forwarded now, and still left unset when absent so the
backend's alphabetical key order stands rather than the `desc` the
time-ordered listings impose.
A repeated --attribute key silently kept the last value, and a test
asserted that as if it were intended. Matching is per-key, so resolving
it means discarding a filter the caller typed: it is rejected instead.
The shared --limit entry also stated the 1-to-1000 bound that only
inspect enforces, which is wrong for cancel's own 1-to-500. The bound
moves to an inspect entry and the shared one points at both.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Reject --attribute on listings that cannot use it
Only the runs listing filters by attributes, but the flag was parsed for
every inspect resource: `inspect steps --attribute tenant=acme` returned
a normal, unfiltered step list with no warning, as did events, hooks,
attributes, and `inspect run <id>`, which already names one run. That is
the silent drop the preceding commit set out to remove, missed one layer
up in the command itself.
Validated alongside the other flag bounds, before any backend setup, so
a flag on the wrong subcommand costs no round trip.
Covered at the command level as well as in the unit, since the defect
was not in the validator but in nothing calling it: the tests drive
`Inspect.run` with a mocked setup module and assert the backend is never
reached. Five of them fail without this change.
Reported in review; verified against a real project rather than found
by the suite, which is why the command-level coverage goes in with it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Resolve the test's oclif root without a URL pathname
`new URL('../..', import.meta.url).pathname` yields `/D:/a/...` on
Windows — a leading slash before the drive letter — so `Config.load`
could not find package.json and every command-level test failed there
while passing on Linux. `fileURLToPath` handles both.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Address review on the attribute flag
Attribute keys naming an Object.prototype member were rejected as
duplicates before anything was stored, because the duplicate check used
`in`, which walks the prototype. `--attribute toString=v` failed on
first sight, and `__proto__=v` would have set the prototype rather than
stored a value had it got that far. The map is null-prototype now and
the check uses Object.hasOwn.
--url and --web return before the filter is parsed, and neither
forwards it, so `inspect runs --attribute k=v --url` opened an
unfiltered view and a malformed pair skipped validation entirely. Both
are rejected: the dashboard takes no attribute filter.
--sort carried an oclif default of desc, so the "forward only when
asked" check in the attribute listing was always true and overrode the
backend's alphabetical key order. Every time-ordered listing already
falls back to desc itself, so the parser-level default is gone and the
flag now means what it says.
The docs claimed --since and --until must be supplied together, but the
CLI resolves the pair before the World sees it: --since alone is valid
and --until defaults to now. Only --until alone is rejected.
The vercel[bot] comment about ANALYTICS_MAX_ATTRIBUTE_FILTERS not being
exported was already addressed in 1811e4f0e, before #3943 landed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Address review: hoist the attribute parse, cap the limit, merge main
The INVALID_ARGUMENT handling was dead because this branch was cut from
main before #3943 landed: nothing in the tree threw that code and
WorkflowWorldError had no `field`, so both new arms were unreachable and
the field assertion in output.test.ts described an API that did not
exist here. Merging main makes all of it live, and makes the two
comments claiming the World enforces these bounds true — world-vercel
asserts them now.
parseAttributeFilters ran inside toInspectOptions, after setupCliWorld,
so a malformed pair paid for auth and a project lookup before failing.
It is parsed in the same block as the other bounds now, and
toInspectOptions receives the result. The gap was untested because the
only malformed-pair case paired it with --url, which returns before the
parse either way; there are now command-level cases for a missing
separator, an empty key, too many pairs, and a duplicate key.
--limit allowed up to 1000, but the cross-run listings cap at 100 and so
does the storage step listing a run-scoped read falls back to, so
101-1000 produced an opaque backend 400 — and on steps it depended on
whether analytics had rows for that run. Capped at 100, the smallest any
reachable listing accepts. The docs claim that listings "report the
limit they accept" was false and is gone.
--attribute with --withData warned and returned every row, which is the
failure the scope guard exists to prevent and is knowable at validation
time. It is a hard error now.
listSleeps degraded on any failure, including a plan-window 402 whose
message tells the caller to upgrade. Access, plan, and invalid-argument
failures are reported; only availability failures degrade. The comment
claiming the sibling listings degrade was wrong — none of them do — and
now says why sleeps is the exception.
Also: --sort/--since/--until/--workflowName help text and the options
type no longer say "runs only"; examples and the unknown-resource text
list attributes; listAttributes and listRuns' filter forwarding have
coverage, including both warning strings; the unreachable 'web' case is
out of the scope test; the ineffective biome suppression is gone; and
the cancel arithmetic is two statuses, so one probe is saved, not three.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(cli): warn on flags inspect attributes cannot apply; bound cancel --limit
`inspect attributes` accepted --status, --runId, --stepId, --hookId and
--withData, dropped all five, and printed the full key table. --status is
the likely one: filtering runs by attribute and status together is
documented, so reaching for it on the key listing is natural and the answer
looks narrowed. Warn per flag, as the sibling listings do.
`cancel --limit` advertised 1-500. Both read paths cap at 100 — the
analytics runs listing rejects more locally, the storage listing it falls
back to caps server-side — so 101-500 always failed, and cancel's catch
handled only the plan gate and rethrew the rest with nothing printed.
Bound it to 100 and route the catch through the shared reporter.
Collapse the three actionable-error checks listSleeps had inlined into
`reportActionableApiError`, shared with `handleApiError` and cancel, so the
set cannot drift between the three callers. Fold inspect's bounds chain and
--attribute parse into `validateInspectFlags` (run() 45 -> 39).
Correct two comments: the MAX_LIMIT rationale (cross-run listings now
reject locally rather than returning an opaque 400), and listSleeps'
--interactive note, which described a partial-table reprint that cannot
happen — pages after the first are fetched inside the keypress listener,
whose rejection never reaches that catch.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(cli): forward --hookId to the listings
`--hookId` was declared as a flag, declared on `InspectCLIOptions`, and read
by `listEvents` (`correlationIdFilter = opts.hookId || opts.stepId`), but
`toInspectOptions` never copied it across. So `inspect events --hookId`
parsed, cleared every check, sent no correlationId, and returned the run's
whole event list.
Pre-existing on main, but this PR both documents the flag and adds a
`listAttributes` warning that depends on it, so the branch was unreachable
from the CLI and its unit test only passed by calling `listAttributes`
directly.
That is the gap: a listing's own tests pass options in, so they cannot see a
drop in the projection. `inspect-flag-forwarding.test.ts` goes through
`Inspect.run` instead, and pins the whole mapping key by key. Three of its
four cases fail without the one-line fix.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>