feat(telemetry): record sanitised detail for unrecognised commands

[MattieTK]

Unknown-command telemetry only reported command: "unknown command" with empty args, so we could not see what was attempted. That bucket also mixed unrelated failures: the most common case is a real leaf given an extra positional (cf zones get <id>, cf dns records list example.com), because yargs' strict-commands check runs at leaf level too. Several failures were not reported at all: multiple unknown words at the root, root-level typos with a "Did you mean" suggestion, and flag-only invocations such as cf --bogus.

.fail now captures where resolution stopped while yargs still holds that level's context, and the parse-error reporter adds:

• resolutionFailure: unknown-subcommand | surplus-positional | suggested-command | missing-command
• knownCommand and knownCommandIsLeaf
• unknownTokens (at most three) and unknownTokenCount
• suggestedCommand
• unknownFlagCount, with known cf flag names in argsUsed

A token is sent only when it is a word from cf's command catalogue and sits where a command name was expected. Everything else, including any surplus positional after a complete command, becomes a shape label (<hex>, <uuid>, <domain>, <path>, <email>, <word> …), so IDs and resource names are not sent. Flag values are never sent.

Existing command values are kept for dashboard continuity: in-group typos and plural "Unknown commands:" failures still report the group or leaf name, now with the extra fields. Near-miss typos of cli telemetry are resolved before the exclusion check, so they stay unreported.

The catalogue is loaded lazily on this error path only, and not at all when telemetry is disabled. telemetry.md is updated.

Example (cf zones get 023e…353 --zone-id abc):

{"command":"unknown command","resolutionFailure":"surplus-positional","knownCommand":"zones get","knownCommandIsLeaf":true,"unknownTokens":["<hex>"],"unknownTokenCount":1,"unknownFlagCount":0,"argsUsed":["zone-id"]}

Expect a small rise in unknown command volume from the root-level cases that were previously dropped.

Validation:

• pnpm check
• pnpm test src/__tests__/telemetry-errors.test.ts src/__tests__/lib/telemetry/command-resolution.test.ts
• Built bin/cf run with DEBUG=1 and no Sparrow key against adversarial argv (IDs, domains, paths, emails, --flag=value, tokens after --)

[pkg-pr-new]

npm i https://pkg.pr.new/cf@342

commit: b12c6ff

[github-actions]

cf --help startup

	mean	stddev	min	max
PR	168.1 ms	3.0 ms	164.1 ms	177.3 ms
base	170.5 ms	2.6 ms	164.6 ms	174.5 ms

Δ (PR vs base): ↓ -2.4 ms (-1.4%)

Measured with hyperfine on ubuntu-latest (3 warmup runs, 20 timed runs). CI numbers carry ±a few ms of runner jitter; treat small deltas as noise.

[clanky-the-clanker]

💬 Approved with comments — The unknown-command telemetry is well scoped: it sends only cf catalogue words or shape labels, never flag values, and it is skipped when telemetry is disabled. The only finding is a WARNING with no runtime impact: the docs say unrecognised flags are counted, but unknown short flags are not.

Findings: 1 warning

_{The decision is advisory: this comment never approves, blocks or requests changes on the pull request.}

🔧 Fix with your agent

Paste the following into your coding agent to investigate and fix these issues.

Fix the following code review issues found in pull request #342.

Before making changes, review each issue and present a brief summary table:
- For each finding, state whether you agree, disagree, or need clarification
- If you disagree (e.g., the fix would require disproportionate changes
  for minimal ROI, or the finding is factually incorrect), explain why
- If you need to clarify something before deciding, ask those questions
- Then share your plan for which issues to tackle and in what order

## WARNING (1)

### [Documentation] `telemetry.md` and the changeset say unrecognised flags are counted, but `passedFlags()` skips unknown short flags (e.g. `-x`), so `unknownFlagCount` misses them. Either count them, which is safe because only a count is sent, or say the docs cover long flags only.
- **File:** packages/cli/telemetry.md:30

---

📝 Documentation: ⚠️ 1 warning

The telemetry docs say all unrecognised flags are counted, but unknown short flags are not.

Recommendation: 📝 Minor Comments

• [WARNING] telemetry.md and the changeset say unrecognised flags are counted, but passedFlags() skips unknown short flags (e.g. -x), so unknownFlagCount misses them. Either count them, which is safe because only a count is sent, or say the docs cover long flags only. (packages/cli/telemetry.md:30)

View full review

packages/cli/telemetry.md and .changeset/unknown-command-telemetry.md say that flags which are not cf options are counted. In practice, unknownFlagCount is computed from passedFlags() (packages/cli/src/lib/telemetry/command-resolution.ts, in sanitizeCommandResolution). That function skips short flags it does not recognise, such as -x, and stops reading a short-flag cluster at the first unknown character (packages/cli/src/lib/telemetry/sanitization.ts, in passedFlags). So cf --bogus adds to the count but cf -x does not.

The docs over-report, so nothing extra is sent. The difference still matters in two places: the user-facing telemetry disclosure, and anyone reading dashboards built on unknownFlagCount. Two possible fixes:

• Count unrecognised short-flag characters in this path. Only a count is sent, so this leaks nothing.
• Narrow the docs to say that unrecognised long flags are counted.

---

✅ All other checks passed

🚀 Release: ✅ LGTM

Risk: Low. The minor changeset matches a feat change to cf's telemetry behaviour, and its imperative title follows the repository's changeset convention. No comparison against previous tags was done, because tag history is not available here.

---

🔒 Security: ✅ LGTM

The new telemetry fields only carry values cf already knows:

• Unknown tokens: A token is sent only if it is a word in cf's command catalogue (packages/cli/src/lib/telemetry/command-resolution.ts). When a complete leaf command is followed by an extra positional, the allowed set is empty, so even a value that looks like a cf word becomes a shape label. Anything else becomes a label such as <hex>, <uuid> or <domain>.
• Flags: Only flag names are read, never values, and parsing stops at --. Names are sent only when they are cf options; unrecognised names are counted but not sent.
• Suggested command: This comes from yargs' "Did you mean" suggestion and is also checked against a character pattern before it is sent.
• Disabled telemetry: When telemetry is disabled, the reporter returns early, before the catalogue is loaded.
• Opt-out commands: Near-miss typos of cli telemetry are resolved to the intended command before the exclusion check, so they stay unreported.

---

📋 Code Quality: ✅ LGTM

I found no defects in the changed code. This comes from reading the code; the tests were not run.

• Resolution capture: The .fail capture in packages/cli/src/index.ts is used only when resolutionFailure.error === err. A stale capture therefore cannot attach to an unrelated error.
• command values: Existing values are kept. Singular "Unknown command:" failures and root-level failures report unknown command. Plural and suggestion failures below the root keep the resolved group or leaf name.
• Telemetry exclusion: The exclusion is checked against both the resolved path and the intended (suggested) command, so cf clii telemetry disable stays unreported. A test covers this.
• Catalogue loading: The vocabulary loads lazily, is cached, and falls back to shape labels when it cannot load. The extra source-tree anchor in the loader is needed because the module lives in lib/telemetry.

Test coverage is adequate for the new behaviour.

---

⚡ Performance: ✅ LGTM

The new work runs only on the parse-error path. It is skipped when telemetry is disabled, because dispatcher.enabled is checked before the catalogue loads. The catalogue is loaded lazily and cached. Token and flag processing is bounded: at most three tokens and one linear pass over argv. Normal command startup is unaffected.

---

_{Reviewed commit: b12c6ff2}

[ask-bonk]

When resolution stops at a group, every remaining positional is passed through the global command-word allowlist. For example, cf workers deploy zones records both deploy and zones; after the unknown deploy, zones can be a positional/resource value for the intended command rather than a command slot. This contradicts the updated policy that positional values are never collected. Only the first unresolved token is known to be in a command position; force later tokens to shape labels.

Suggested change

	// Surplus positionals at a leaf are almost always resource values, so a
	// value that happens to be a cf word (`test`, `cache`) is still labelled.
	const commandWords =
	failure.kind === "surplus-positional"
	? new Set<string>()
	: vocabulary.commands;
	const suggestedCommand =
	failure.kind === "suggested-command" ? failure.suggestedCommand : undefined;
	return {
	resolutionFailure: failure.kind,
	knownCommand: failure.knownCommand.join(" "),
	knownCommandIsLeaf: failure.knownCommandIsLeaf,
	unknownTokens: failure.unknownTokens
	.slice(0, MAX_UNKNOWN_TOKENS)
	.map((token) => sanitizeCommandToken(token, commandWords)),
	unknownTokens: failure.unknownTokens
	.slice(0, MAX_UNKNOWN_TOKENS)
	.map((token, index) =>
	sanitizeCommandToken(
	token,
	index === 0 ? commandWords : new Set<string>()
	)
	),

[ask-bonk]

passedFlags() defaults to only the global aliases, but this new path never supplies aliases from the resolved command. Valid command-specific short options are therefore neither named nor counted. For instance, cf smart-shield health-checks delete id extra -f reaches this surplus-positional path, although the generated command registers -f for force; this event reports argsUsed: [] and unknownFlagCount: 0. Build a short-alias map from the command metadata (plus the global aliases) while loading the vocabulary and pass it here.

Suggested change

	for (const flag of passedFlags(rawArgv)) {
	for (const flag of passedFlags(rawArgv, vocabulary.shortFlagAliases)) {

[ask-bonk]

🤖 I've done a thorough review of your PR.

Adds sanitized telemetry context for command-resolution failures.

Posted 2 actionable inline findings.

github run