You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
|`src/utils/output.ts`| ✅ Done | Added `MessageDisplayFields` interface (timestamp is `number` — raw ms), `formatMessagesOutput()`, `toMessageJson()`. Imports `isJsonData`/`formatMessageData` from `json-formatter.ts`. |
15
+
|`src/commands/channels/subscribe.ts`| ✅ Done | Uses `formatMessagesOutput([msgFields])` + `toMessageJson(msgFields)`. Passes `message.timestamp` as raw number (no `formatMessageTimestamp` conversion). Added `serial` field. Removed `connectionId`/`encoding` from user-facing JSON. Sequence numbers added separately to JSON. |
16
+
|`src/commands/channels/history.ts`| ✅ Done | Uses `formatMessagesOutput(displayMessages)` + `displayMessages.map((msg) => toMessageJson(msg))`. Passes `message.timestamp` as raw number. Plain array JSON output. Added `channel`/`serial`/`indexPrefix`. Preserved `limitWarning`. Kept current error handling. |
17
+
|`.claude/CLAUDE.md`| ✅ Done | Added "Message display" conventions subsection with raw ms timestamp convention. Updated "History output" bullet to note `channels history` exception. |
18
+
|`test/unit/commands/channels/subscribe.test.ts`| ✅ No changes needed | Tests already expected the new format (survived merge). |
19
+
|`test/unit/commands/channels/history.test.ts`| ✅ No changes needed | Tests already expected the new format (survived merge). |
20
+
21
+
### Lint fixes applied
22
+
-`output.ts`: Fixed prettier formatting for ternary expression, combined consecutive `Array#push()` calls per `unicorn/no-array-push-push`, fixed prettier formatting for function parameter
23
+
-`history.ts`: Wrapped `toMessageJson` in arrow function per `unicorn/no-array-callback-reference`
24
+
25
+
### Deep cross-check findings (plan vs codebase vs CLAUDE.md)
26
+
27
+
These are additional issues found during a thorough review that the original plan doesn't fully address:
> Use `[index] timestamp` ordering: `` `${chalk.dim(`[${index + 1}]`)} ${formatTimestamp(timestamp)}` ``. Consistent across all history commands (channels, logs, connection-lifecycle, push).
33
+
34
+
The plan changes `channels history` to put `[index]` on its own line and `Timestamp:` as a labeled field — **intentionally diverging** from the other history commands (`logs history`, `logs push history`, `logs connection-lifecycle history`) which still use the old `[index] [timestamp]` single-line format.
35
+
36
+
**Action needed**: The CLAUDE.md update (Step 5) must clarify that `channels history` now uses the new `formatMessagesOutput` convention while other history commands retain the old `[index] [timestamp]` pattern. The existing "History output" bullet should be updated to note this exception.
37
+
38
+
#### 2. `formatTimestamp()` returns `[timestamp]` with brackets — plan uses bare timestamp
39
+
40
+
The existing `formatTimestamp()` in `src/utils/output.ts` returns `chalk.dim(`[${ts}]`)` — dim text **with square brackets**. The plan's target format shows `Timestamp: 2026-03-06T05:13:09.160Z` (no brackets, no dim on the value).
41
+
42
+
**Action needed**: `formatMessagesOutput()` must NOT use `formatTimestamp()` for the timestamp value. It should display the raw ISO string directly. The plan's pseudocode (line 285) is correct: `${chalk.dim("Timestamp:")} ${timestamp}`. Step 3 correctly says to remove the `formatTimestamp` import from subscribe.
43
+
44
+
#### 3. `history.ts` error handling — `handleCommandError` would break error test
45
+
46
+
The current `history.ts` (lines 127-134) uses a manual try/catch with `this.jsonError()` + `this.error()`. Per CLAUDE.md convention, it should use `this.handleCommandError()`. However, the current code prefixes the error with `"Error retrieving channel history: "`, and the test at line 233 of `history.test.ts` asserts `error?.message` contains `"Error retrieving channel history"`.
47
+
48
+
If we switch to `this.handleCommandError()`, it would pass the raw error message (e.g., `"API error"`) without the prefix, **breaking the test**.
49
+
50
+
**Decision**: Keep the current error handling pattern for now (don't switch to `handleCommandError`). This is a minor inconsistency with CLAUDE.md convention but avoids an unnecessary test change. The error handling improvement can be done in a separate PR if desired.
51
+
52
+
#### 4. `history.ts``limitWarning` — must be preserved
53
+
54
+
The current code shows a limit warning at the end (`limitWarning(messages.length, flags.limit, "messages")`). The plan's Step 4 doesn't explicitly mention keeping it.
55
+
56
+
**Action needed**: Step 4 must preserve the `limitWarning` call after `formatMessagesOutput`. Add it after the `this.log(formatMessagesOutput(...))` call.
57
+
58
+
#### 5. `formatMessagesOutput` needs `isJsonData` for data display
59
+
60
+
The plan says data should be inline for simple values and on a new line for JSON objects/arrays. The existing `formatMessageData()` from `src/utils/json-formatter.ts` handles colorized formatting but doesn't distinguish inline vs block. The `isJsonData()` function from the same file can be used to decide.
61
+
62
+
**Action needed**: `formatMessagesOutput` should use `isJsonData(data)` to decide:
Import both `isJsonData` and `formatMessageData` from `src/utils/json-formatter.ts` into `src/utils/output.ts`.
67
+
68
+
#### 6. Test coverage gaps (non-blocking)
69
+
70
+
-**Subscribe test**: No assertion for `"Serial:"` — acceptable since mock doesn't include `serial` and the field is optional (omitted when undefined).
71
+
-**Subscribe JSON test**: Only checks `error` is undefined and `stdout` is defined — doesn't verify JSON structure. Could be improved but not blocking.
72
+
-**History test**: Mock data doesn't include `serial` — acceptable for same reason.
73
+
74
+
#### 7. E2E compatibility — verified safe
75
+
76
+
-`channel-subscribe-e2e.test.ts` uses `readySignal = "Subscribed to channel"` — the subscribe command still outputs `success("Subscribed to channel: ...")` which contains this string. ✅ Safe.
77
+
-`channel-occupancy-e2e.test.ts` also uses `readySignal: "Subscribed to channel"` — same reasoning. ✅ Safe.
78
+
- Data check uses `output.includes("Subscribe E2E Test")` — the message data will still appear in the new format. ✅ Safe.
79
+
80
+
#### 8. `serial` field confirmed in Ably SDK types
81
+
82
+
The Ably SDK `Message` interface has `serial?: string` (optional). For realtime `InboundMessage`, `serial: string` (required). Both subscribe (realtime) and history (REST) messages will have this field available.
83
+
84
+
#### 9. `logCliEvent` in subscribe — keep full message details
85
+
86
+
The current subscribe code (line 212-218) passes a `messageEvent` object to `logCliEvent` that includes `connectionId`, `encoding`, and `sequence`. This is internal verbose logging, not user-facing output. The `logCliEvent` call should continue to pass the full message details for debugging purposes. Only the user-facing output (human-readable and `--json`) should use the new `formatMessagesOutput`/`toMessageJson` helpers.
87
+
88
+
**Action needed**: When updating subscribe.ts, keep the `logCliEvent` call with its current `messageEvent` object (or update it to include `serial` too). The `logCliEvent` data is separate from the user-facing output.
89
+
90
+
#### 10. `sequence` field in subscribe JSON output
91
+
92
+
The current code adds `sequence: this.sequenceCounter` to the JSON output when `--sequence-numbers` is used. The plan's `toMessageJson` doesn't include `sequence`. Since `sequence` is subscribe-specific (not part of the shared `MessageDisplayFields`), it should be added to the JSON output separately in subscribe.ts after calling `toMessageJson`:
93
+
94
+
```typescript
95
+
const jsonMsg =toMessageJson(msgFields);
96
+
if (flags["sequence-numbers"]) {
97
+
jsonMsg.sequence=this.sequenceCounter;
98
+
}
99
+
this.log(this.formatJsonOutput(jsonMsg, flags));
100
+
```
101
+
102
+
**Action needed**: Step 3 should note that `sequence` is added to JSON output separately, not via `toMessageJson`.
103
+
104
+
### Implementation order (updated)
105
+
106
+
To fix all 4 test failures, implement these steps:
107
+
108
+
1.**Step 1**: Add `MessageDisplayFields`, `formatMessagesOutput()`, `toMessageJson()` to `src/utils/output.ts`
109
+
- Import `isJsonData`, `formatMessageData` from `json-formatter.ts`
110
+
- Do NOT use `formatTimestamp()` — display raw ISO string
111
+
2.**Step 2**: Update `src/commands/channels/subscribe.ts` to use the new helpers
112
+
- Remove `formatTimestamp` import (no longer needed)
113
+
- Add `serial` from `message.serial`
114
+
3.**Step 3**: Update `src/commands/channels/history.ts` to use the new helpers
115
+
- Preserve `limitWarning` after `formatMessagesOutput`
116
+
- Keep current error handling (don't switch to `handleCommandError` — would break error test)
117
+
4.**Step 5** (recommended): Update `.claude/CLAUDE.md` with message display conventions
118
+
- Note the divergence from the old `[index] timestamp` pattern for `channels history`
119
+
120
+
Step 6 (tests) is already done — the tests are correct and just need the source to match.
121
+
122
+
---
123
+
3
124
## Original Request
4
125
5
126
Currently when running `bin/run.js channels subscribe test`, the output is:
@@ -255,9 +376,13 @@ Changes from current:
255
376
256
377
### 1. Add `formatMessagesOutput` helper to `src/utils/output.ts`
257
378
258
-
Create a single shared function that accepts an array of messages:
379
+
Create a single shared function that accepts an array of messages.
380
+
381
+
**Important**: Import `isJsonData` and `formatMessageData` from `src/utils/json-formatter.ts`. Do NOT use `formatTimestamp()` (which wraps in `[brackets]`) — display the raw ISO string directly.
- **History output**: Use `[index] timestamp` ordering: ```${chalk.dim(`[${index+1}]`)} ${formatTimestamp(timestamp)}```. Consistent across log history commands (logs, connection-lifecycle, push). Exception: `channelshistory` uses `formatMessagesOutput()` with `indexPrefix` for richer field display.
2.`src/commands/channels/subscribe.ts` — Use`formatMessagesOutput([msg])`+`toMessageJson(msg)`, add`serial`, remove`connectionId`/`encoding` from user-facing JSON, add `sequence` to JSON separately when `--sequence-numbers`, keep `logCliEvent` with full details, remove `formatTimestamp`/`formatMessageData`imports (keep`chalk`if`sequencePrefix`usesit).
0 commit comments