Release/v0.10.0–add JSONL command channel #10
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds a JSONL command-envelope system and CLI to alfred-live; introduces encoding/decoding/validation/execution helpers, types, example, and tests. Both packages are bumped to v0.10.0 and alfred-live publishes a new Changes
Sequence DiagramsequenceDiagram
participant CLI as alfredctl CLI
participant Envelope as Envelope Handler
participant Router as CommandRouter
participant Registry as ConfigRegistry
participant Result as Result Builder
CLI->>Envelope: parse args → build CommandEnvelope
Envelope->>Envelope: validateCommandEnvelope(envelope)
CLI->>Envelope: encodeCommandEnvelope(envelope)
Envelope-->>CLI: JSONL string (stdout)
Note over CLI,Result: execution via executeCommandLine
CLI->>Envelope: decodeCommandEnvelope(line)
Envelope->>Router: executeCommandEnvelope(router, envelope)
Router->>Registry: perform read/write/list
Registry-->>Router: return data or error
Router->>Result: buildResultEnvelope(id, result)
Result->>Envelope: encodeResultEnvelope(resultEnvelope)
Envelope-->>CLI: ResultEnvelope JSONL string
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@alfred-live/bin/alfredctl.js`:
- Around line 109-114: The error details printed when
encodeCommandEnvelope(envelope) fails may include sensitive fields like auth;
redact any auth/token fields before writing encoded.error.details to stderr.
Update the error printing logic that checks encoded.ok to sanitize
encoded.error.details (e.g., remove or replace auth, token, password keys) prior
to JSON.stringify, referencing encodeCommandEnvelope, encoded, and
encoded.error.details so the stderr output never contains the envelope.auth
contents.
In `@alfred-live/src/command-envelope.js`:
- Around line 205-210: validateOkResultEnvelope currently only ensures 'error'
is absent for ok results but allows envelopes with ok: true and missing data;
update validateOkResultEnvelope to also verify that envelope hasOwnProperty
'data' and that envelope.data is not undefined (or null if you prefer) and
return a ValidationError like 'data is required when ok is true.' when missing;
locate the validateOkResultEnvelope function and add this additional check
alongside the existing error-check.
🧹 Nitpick comments (1)
alfred-live/examples/control-plane/jsonl-channel.js (1)
11-20: Add explicit error handling forregistry.register().
register()returns a Result object that indicates success or failure. The example currently ignores this return value and may fail later if registration fails (e.g., invalid path or duplicate registration). Since the example already handles errors fromexecuteCommandLine()(line 31–32), add the same check here for consistency and robustness:Suggested addition
-registry.register('retry/count', retryCount, { +const result = registry.register('retry/count', retryCount, { parse: (value) => { const parsed = Number(value); if (!Number.isFinite(parsed)) { throw new Error('retry/count must be a number'); } return parsed; }, format: (value) => value.toString(), }); +if (!result.ok) { + throw new Error(result.error.message); +}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@scripts/hooks/pre-push`:
- Around line 7-13: The script currently sets BASE_REF to "@{u}" then falls back
to "origin/main" and runs git diff into CHANGED_FILES which can fail if
origin/main doesn't exist; modify the logic around BASE_REF (the BASE_REF
variable and CHANGED_FILES assignment in the pre-push hook) to verify the chosen
ref exists before using it (e.g., use git rev-parse --verify or git show-ref)
and if it does not exist fall back to a safe ref such as HEAD or an empty tree,
then compute CHANGED_FILES from that verified ref so git diff won't error under
set -e; update conditionals that set BASE_REF to perform this existence check.
In `@scripts/release/preflight.mjs`:
- Around line 125-130: The current spawnSync call storing result in
installResult can leave error details hidden when pnpm is missing
(installResult.status may be null and installResult.error populated); update the
post-spawn handling around spawnSync('pnpm', ...) to check installResult.error
and log a clear message (including installResult.error.message or the error
object) before calling process.exit(installResult.status ?? 1), ensuring both
non-zero status and spawn errors are surfaced; refer to the installResult
variable, the spawnSync invocation, and the process.exit call when making this
change.
|
@coderabbitai Do you approve? Or do you have more changes you'd like to see? |
|
✅ Well-structured release:
✅ Quality practices:
✅ API design:
The testing (lint + tests) has passed, and the scope is appropriate for a minor version bump. Nice work on this release! ✅ Actions performedComments resolved. |
1 participant
Summary
alfredctlCLI plus JSONL command example/testsPackages Changed
Testing
Summary by CodeRabbit
New Features
Documentation
Tests
Chores