Standardise output for missing or optional fields

[umair-ably]

targeting #236... tests on CI will run once this targets main (once the other PR is in)

Intent

The changes standardize how missing or optional fields are displayed across the CLI: instead of showing inconsistent placeholders like "None", "N/A", or "Unknown", we omit the field entirely from both human and JSON output when it has no value. As part of this, AccountConfig fields that were never actually optional are now typed as required, eliminating their fallback strings at the source.

Summary

• Remove fallback placeholder strings ("Unknown", "None", "N/A") from ~20 commands — fields with no value are now omitted instead
• Make accountId, accountName, and userEmail required in AccountConfig interface (they are always populated during ably login)
• Update storeAccount() signature to require accountInfo with required fields, matching actual usage
• Conditionally spread clientId in JSON output (issue-ably-token, issue-jwt-token) so the key is absent rather than null
• Update tests to match: assert fields are absent rather than checking for placeholder text, pass required fields to storeAccount()

Test plan

• pnpm test:unit passes
• pnpm exec eslint . clean
• Verified human-readable output omits fields when values are missing
• Verified JSON output excludes keys rather than emitting null/"Unknown"

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
cli-web-cli	Ready	Preview, Comment	Apr 1, 2026 10:13am

[claude-code-ably-assistant]

Walkthrough

This PR standardises how missing or optional fields are displayed across the CLI: instead of showing placeholder strings like "Unknown", "None", or "N/A", fields with no value are now omitted entirely from both human-readable and JSON output. As part of this cleanup, accountId, accountName, and userEmail in AccountConfig are promoted from optional to required, matching how they are always populated during ably login, which eliminates the need for fallback strings at the source.

Changes

Area	Files	Summary
Commands – Accounts	accounts/current.ts, accounts/list.ts, accounts/switch.ts	Remove || "Unknown" / || "Unknown ID" fallbacks; omit fields when values are absent
Commands – Apps/Auth/Keys	apps/current.ts, auth/keys/current.ts	Conditionally render account ID in parentheses only when present
Commands – Auth tokens	auth/issue-ably-token.ts, auth/issue-jwt-token.ts	Omit clientId from JSON output (conditional spread) and human output (guard with if) instead of emitting null/"None"
Commands – Channels/Push	channels/presence/enter.ts, channels/annotations/subscribe.ts, push/channels/list.ts	Remove || "N/A" / || "unknown" / || "[Unknown timestamp]" fallbacks
Commands – Rooms	rooms/messages/history.ts, rooms/messages/subscribe.ts, rooms/messages/reactions/subscribe.ts, rooms/presence/enter.ts, rooms/reactions/subscribe.ts	Remove || "Unknown" / || "N/A" fallbacks for clientId, author, reaction fields
Commands – Spaces	spaces/members/subscribe.ts, spaces/subscribe.ts	Remove || "Unknown" fallbacks for clientId and connectionId
Services	config-manager.ts, interactive-helper.ts	Promote accountId, accountName, userEmail to required in AccountConfig; update storeAccount() signature to match; remove || "Unknown" from interactive account picker
Tests – Helpers	test/helpers/mock-config-manager.ts	Update storeAccount() to match new required signature; add sensible test defaults
Tests – Unit	test/unit/commands/auth/issue-ably-token.test.ts, issue-jwt-token.test.ts, services/config-manager.test.ts	Assert fields are absent rather than checking for placeholder text; pass required fields to storeAccount()

Review Notes

• Behavioral change: Commands that previously displayed "Unknown" / "None" / "N/A" for missing values will now silently omit those lines/fields. Consumers parsing human-readable output that relied on those placeholder strings will see a difference.
• Breaking API change: storeAccount() signature changed — alias and accountInfo are now required (no longer optional). Any callers outside this repo (unlikely for an internal service, but worth noting) would need updating.
• Type tightening: AccountConfig.accountId, accountName, and userEmail are now non-optional. This removes a category of null-safety handling but assumes the login flow always populates these fields — worth verifying that all login paths set all three before merging.
• rooms/presence/enter.ts non-null assertion: this.chatClient!.clientId! uses ! to assert non-null after removing the ?? "unknown" fallback. Reviewers should confirm the Chat SDK guarantees clientId is set by the time this line executes.
• No new dependencies.
• Test coverage looks complete — all changed output paths have corresponding test assertions updated.

[claude-code-ably-assistant]

Review Summary

File	Status	Issues
test/e2e/auth/basic-auth.test.ts	1 issue	Missing required userEmail in 3 calls
src/commands/accounts/current.ts	1 issue	Inconsistent null fallback in JSON path
src/services/interactive-helper.ts	Minor	Old configs show undefined in prompt
All other changed files	OK	—

test/e2e/auth/basic-auth.test.ts

Bug: 3 storeAccount calls missing the now-required userEmail field

The ConfigManager interface and TomlConfigManager implementation were changed to require userEmail in accountInfo, but these e2e tests were not updated:

• Line ~226: storeAccount("token", "platform-test", { accountId: ..., accountName: ... })
• Line ~242: storeAccount("token", "lineending-test", { accountId: ..., accountName: ... })
• Line ~270: storeAccount("isolated-token", "isolated-account", { accountId: ..., accountName: ... })

These are TypeScript errors under the project's strict config. They aren't caught by pnpm test:unit because vitest uses esbuild to transpile (no type checking), but tsc -p tsconfig.test.json --noEmit would fail. Same fix pattern as the unit tests in this PR — just add userEmail: "..." to each call.

src/commands/accounts/current.ts:127

Inconsistency: user: { email: currentAccount.userEmail || null } in cached JSON path

In the error/catch path, JSON output still uses the || null fallback, meaning old configs emit "email": null instead of omitting the key. Inconsistent with the stated goal of omitting fields with no value. Fix: conditionally spread the user key.

src/services/interactive-helper.ts:63

Minor: old configs will show undefined in the account selection prompt

AccountConfig.userEmail is now typed as required, but existing user configs stored before this change won't have it. The interactive account picker would display "(undefined, undefined)" for those users instead of "(Unknown, Unknown)". Not a crash, but noticeably confusing UX for users who have not re-logged-in. Not caught by CI since the interactive prompt path is not exercised in tests.

Overall: The approach is sound. The e2e test TypeScript gap should be fixed before merge; the other two are smaller but worth addressing.

[umair-ably]

@claude your test issue you flagged was valid - fixed. The others weren't... userEmail has been stored since the initial implementation so I've removed the deadend fallback instead

[claude-code-ably-assistant]

Review: Standardise output for missing or optional fields

Clean, well-scoped PR. The intent is correct and the approach is consistent. A few things worth considering.

src/commands/accounts/current.ts — conditional guard removed (backward compat)

Before, the cached-account block in the catch path was guarded so it was hidden when both fields were absent:

if (currentAccount.accountName || currentAccount.accountId) {
  this.log(`${formatLabel("Account (cached)")} ...`)
}

After: unconditional. The TOML config is loaded with parse(content) as unknown as AblyConfig, so TypeScript won't catch missing fields at runtime. A user with a manually-edited or pre-login config could see "undefined (undefined)" instead of nothing.

The login command has always stored these fields (login.ts:182), so this is low risk in practice. Just worth noting as an explicit tradeoff — if desired, a runtime guard on load would eliminate the exposure.

accounts/current.ts catch block — JSON schema change

user.email was previously explicitly null when missing; it will now be absent from the JSON envelope entirely. Consistent with the PR's goal, but callers checking for "email": null would be silently broken.

push/channels/list.ts — final fallback removed

- const id = sub.deviceId || sub.clientId || "unknown";
+ const id = sub.deviceId || sub.clientId;

Both fields are likely optional in the SDK types. If both are absent, output would show "client: undefined". Very low real-world risk but the original fallback cost nothing.

Everything else: LGTM

The SDK types for PresenceMessage.clientId, SpaceMember.clientId/connectionId, and Chat Message.clientId are non-optional strings (TypeScript build would fail otherwise), so removing the || "fallback" guards is correct. The AccountConfig typing changes and storeAccount signature updates match actual usage. Test updates are accurate and complete.

[sacOO7]

This should re-use formatClientId

[umair-ably]

not the intention of this PR... but have addressed in the last commit 9683748

[sacOO7]

Found few inconsistencies

• Log timestamp fallback: In logs/push/history.ts, logs/connection-lifecycle/history.ts, and logs/history.ts, [Unknown timestamp] still appears in 3 log history commands. The SDK allows Message.timestamp to be optional, so the guard is fine, but the placeholder text doesn’t match PR standards. Decision: either replace with empty string or document as an intentional exception.

• Channel filter in integrations: In integrations/list.ts:54, channelFilter || null outputs null in JSON. To avoid this, use a conditional spread so the field is omitted when not present.

• Null handling across multiple files: In 15+ places (spaces-output.ts, presence/*.ts, cursors/set.ts, rules/*.ts), ?? null emits "field": null in JSON. Decide whether to convert all to conditional spread, document exceptions for meaningful nulls (like presence data).

[sacOO7]

Lgtm