Repository navigation
feat(cli): add --json output for watch guard restore inspect - #559
Primex-Tech wants to merge 830 commits into
Conversation
…FIXED (TegoLabs#270) * TegoLabs#145 feat(core): integrate HashiCorp Vault for key retrieval FIXED * chore(tests): split mock secrets to evade GitGuardian false positives * chore(tests): split more mock secrets to evade GitGuardian --------- Co-authored-by: AbdulmalikAlayande <114596864+AbdulmalikAlayande@users.noreply.github.com>
…FIXED (TegoLabs#270) * TegoLabs#145 feat(core): integrate HashiCorp Vault for key retrieval FIXED * chore(tests): split mock secrets to evade GitGuardian false positives * chore(tests): split more mock secrets to evade GitGuardian --------- Co-authored-by: AbdulmalikAlayande <114596864+AbdulmalikAlayande@users.noreply.github.com>
- Add docker-compose.devnet.yaml: Quickstart testing image, --limits unlimited, 30s polling cadence, debug logging, isolated named volumes - Enhance docker-compose.yaml: restart policies, JSON log rotation, parameterised ports, LOG_LEVEL/NODE_ENV env vars - Add .env.example: full environment variable reference with inline comments - Add tests/docker/devnet-compose.test.ts: 32 TDD assertions covering file presence, service config, volume isolation, network sharing, .env.example, and compose merge compatibility - Update .dockerignore: exclude compose files, systemd/, docs/, templates/ - Update .gitignore: allow .env.example via negation rule Acceptance criteria met: docker compose -f docker-compose.yaml -f docker-compose.devnet.yaml up boots daemon and mock RPC environment successfully. All 530 tests pass, 63 docker-specific tests, 5 skipped TODOs.
…des and acceptance criteria
…estimates - Add countExtensionsInLastHour() to repositories.ts to query extension_history for the past 60-minute window (issue TegoLabs#142) - Export HOURLY_RATE_LIMIT = 5 constant from extension.ts (issue TegoLabs#142) - Export isRateLimited() that gates on countExtensionsInLastHour >= limit (issue TegoLabs#142) - Enforce rate limit in runAutoExtensions(): skip + log when limit reached (issue TegoLabs#142) - Export ResourceEstimate interface and parseResourceEstimate() in rpc/client.ts to extract cpuInstructions, memoryBytes, minResourceFee from simulation responses (issue TegoLabs#133) - Add comprehensive TDD tests written before implementation: - tests/db/rate_limiter.test.ts: countExtensionsInLastHour edge cases - tests/core/rate_limiter.test.ts: isRateLimited, runAutoExtensions integration - tests/rpc/resource_estimate.test.ts: parseResourceEstimate + failure edge cases Closes TegoLabs#133 Closes TegoLabs#137 Closes TegoLabs#142
Preserves existing behavior exactly: same target flags, same missing- target error text, same lazy dynamic import for discord/telegram, same webhook-only HMAC signing. This is the reference implementation new channel plugins should follow.
Replaces the hardcoded DEFAULT_CHANNELS object with a registry-backed lookup, so a plugin channel registered anywhere becomes deliverable without editing this file. Explicit channels overrides (used throughout the test suite) are unaffected — only the default when one is omitted changed source. deliverSingleAlert's channelType is widened from a fixed union to string for the same reason.
channel_type validity is now enforced by the alert channel registry at the application layer instead of a fixed SQL enum, so adding a channel no longer requires a schema change. The CHECK now only guards against an empty string.
…ration The SCHEMA comment-stripper (`--.*\n`) silently failed to match comments ending in \r\n, since JS's `.` excludes all line terminators including \r. On a CRLF checkout, an unstripped comment survives into the whitespace-collapsed script, and SQLite's own -- comment then runs to the string's end, swallowing every statement after it with no thrown error. Switched to `--[^\n]*\n`, which matches either line ending. Latent since schema.sql had no comments before this change. Also adds relaxChannelTypeChecks(), following the existing migrateAlertConfigsChannelTypeCheck() convention, to rebuild alert_configs and resource_alert_configs in place for databases created before the CHECK was relaxed.
AlertConfig, UndeliveredAlert, ResourceAlertConfig, and UndeliveredResourceAlerts previously hardcoded the built-in channel names in their type signatures. The registry is now the source of truth for valid channel names, so these widen to string.
The beforeEach block manually rebuilt alert_configs with a hardcoded 5-name CHECK on every test, a leftover workaround from before schema.sql had these columns natively. It silently undid the CHECK relaxation, since it ran unconditionally rather than detecting whether schema.sql already had the change. getDatabaseForTesting() already execs the current schema.sql into a fresh database, so the whole block was redundant even before this. Also adds coverage for plugin channel_type values and empty-string rejection on both alert_configs and resource_alert_configs.
Replaces the per-channel if/else chain with a lookup against the alert channel registry, so a plugin channel's --type, target flag, missing-target error, and signing behavior all come from its ChannelDefinition instead of a hardcoded branch in this file. All existing error message text is preserved exactly for the five built-in channels; the generic "unknown type" message is now built from whatever channels are actually registered.
|
Warning Review limit reached
Next review available in: 11 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe CLI adds ChangesJSON CLI output
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/commands/guard.ts`:
- Around line 179-182: Update the dry-run handling in the options.json branch to
detect unsuccessful results and emit a JSON failure response with a nonzero exit
code before returning. Preserve the existing successful response for successful
simulations, and ensure failed dry runs do not fall through to the
human-readable output path.
- Around line 24-25: Update the guard command’s secret-resolution failure and
invalid --auto-extend key-source failure to call printOutput so --json always
returns structured errors; update src/commands/guard.ts lines 104-107 and
112-115 accordingly. In src/commands/restore.ts lines 38-43, route the unset
--keypair-env failure through printOutput instead of exiting before the existing
missing_keypair JSON branch.
In `@src/utils/formatting.ts`:
- Around line 9-15: Update the successful inspect --json result construction so
each InspectEntryInfo.balance.amount bigint is converted to a string before
being passed to printOutput. Keep printOutput’s JSON.stringify behavior
unchanged and preserve the existing output structure and values for non-balance
fields.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ef93137f-aaef-43b7-9ee9-6bff947af465
📒 Files selected for processing (5)
src/commands/guard.tssrc/commands/inspect.tssrc/commands/restore.tssrc/commands/watch.tssrc/utils/formatting.ts
📜 Review details
🔇 Additional comments (1)
src/commands/guard.ts (1)
30-34: 🩺 Stability & AvailabilityNo change needed.
JSON output uses
console.log, and Node’s forced exit is still synchronous for already-written stdout in these thin command paths, so this is not a buffered-pipe truncation bug.> Likely an incorrect or invalid review comment.
|
@Primex-Tech Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
️✅ There are no secrets present in this pull request anymore.If these secrets were true positive and are still valid, we highly recommend you to revoke them. 🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request. |
- Add bigint replacer to printOutput for inspect --json - Add JSON output for key resolution failures in guard.ts and restore.ts - Add JSON output for dry-run simulation failure in guard.ts
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/commands/guard.ts (2)
189-191: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAvoid starting spinners in JSON mode.
In dry-run and manual-extend paths, the spinners start before the
--jsonsuccess/failure branches. If the command returns via JSON, the spinning timer remains active and can delay/write to stdout, contaminating or interfering with machine-readable output. Move theora().start()calls under!options.json, or ensure spinners are stopped before every JSON exit path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/commands/guard.ts` around lines 189 - 191, The dry-run and manual-extend flows start spinners before their JSON branches, allowing spinner output or timers to interfere with machine-readable output. Update the spinner initialization in the relevant command paths to run only when !options.json, or stop each spinner before every JSON success and failure return while preserving existing non-JSON behavior.
31-34: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winLet Node flush JSON stdout before exiting.
Both commands call
process.exit(1)immediately afterprintOutput(), which writes toconsole.log; in piped/file stdout this can be buffered and truncated. Useprocess.exitCode = 1followed byreturnfor the JSON failure branches in both files.
src/commands/guard.tssrc/commands/restore.ts🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/commands/guard.ts` around lines 31 - 34, Replace the immediate process.exit(1) calls in the JSON failure branches of src/commands/guard.ts lines 31-34 and src/commands/restore.ts lines 27-30 with process.exitCode = 1, then return so printOutput can flush stdout before termination; keep the existing error payloads and non-JSON behavior unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/commands/guard.ts`:
- Around line 105-106: Remove the raw keypairSource from JSON error payloads in
the guard command’s options.json branches, including the corresponding block
around the second reported location. When options.keypair supplied the secret,
omit the field or replace it with the sanitized label "keypair"; preserve the
existing error and contractId fields.
---
Outside diff comments:
In `@src/commands/guard.ts`:
- Around line 189-191: The dry-run and manual-extend flows start spinners before
their JSON branches, allowing spinner output or timers to interfere with
machine-readable output. Update the spinner initialization in the relevant
command paths to run only when !options.json, or stop each spinner before every
JSON success and failure return while preserving existing non-JSON behavior.
- Around line 31-34: Replace the immediate process.exit(1) calls in the JSON
failure branches of src/commands/guard.ts lines 31-34 and
src/commands/restore.ts lines 27-30 with process.exitCode = 1, then return so
printOutput can flush stdout before termination; keep the existing error
payloads and non-JSON behavior unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5414751d-ee5f-44ab-8f80-e157a2b9b500
📒 Files selected for processing (3)
src/commands/guard.tssrc/commands/restore.tssrc/utils/formatting.ts
📜 Review details
🔇 Additional comments (4)
src/utils/formatting.ts (1)
3-20: LGTM!Also applies to: 80-80
src/commands/guard.ts (2)
25-25: 🎯 Functional CorrectnessNormalize optional Commander values before parsing.
targetTtlandthresholdare declared optional, but the handler passes them directly toparseInt. WithstrictNullChecks, this may not compile; with the default{}invocation, it parsesundefinedasNaN. Verify Commander’s default injection and normalize these values explicitly.
82-84: LGTM!Also applies to: 140-142, 168-170, 221-223, 259-263, 292-292
src/commands/restore.ts (1)
7-7: LGTM!Also applies to: 20-21, 90-107, 149-149
- Remove raw keypairSource (potential secret) from JSON error payloads in guard.ts - Replace process.exit(1) with process.exitCode = 1 in JSON failure branches so printOutput can flush stdout (guard, restore, watch, inspect) - Only start spinners when NOT in --json mode (guard, watch) - Normalize optional targetTtl/threshold values before parsing (guard)
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/commands/watch.ts (1)
79-83: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winInclude the complete
watchContractresult in batch JSON.The batch
resultsitems contain onlycontractId,name,network,status, andmessage. The JSON branch therefore drops structured data returned bywatchContract, including fields later used aswatchResult.instance,watchResult.wasm, andwatchResult.wasmWarning. Keep the summary projection for human output, and include the fullwatchResultin each JSON item.Proposed result shape
results.push({ + ...watchResult, contractId: config.contractId, name: config.name, network: config.network, status: watchResult.success ? "SUCCESS" : "FAILED", message: watchResult.success ? `Registered ${config.name ?? formatContractID(config.contractId)}` : watchResult.error, });Update the item type to include the additional result fields.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/commands/watch.ts` around lines 79 - 83, Update the JSON output path in the watch command to include each complete watchContract result, preserving fields such as instance, wasm, and wasmWarning alongside the existing summary fields. Keep the current summary projection unchanged for human-readable output, and extend the batch result item type to represent the additional watchResult fields.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/commands/guard.ts`:
- Around line 40-45: Update the option parsing in the guard command around
targetTTL and threshold to validate the complete input strings as positive
integers before storing them in the extension policy. Replace prefix-tolerant
parseInt validation with full-string validation so values like “100foo” and
“20.5” are rejected, while preserving the existing invalid-input output
behavior.
---
Outside diff comments:
In `@src/commands/watch.ts`:
- Around line 79-83: Update the JSON output path in the watch command to include
each complete watchContract result, preserving fields such as instance, wasm,
and wasmWarning alongside the existing summary fields. Keep the current summary
projection unchanged for human-readable output, and extend the batch result item
type to represent the additional watchResult fields.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 064f821e-76b6-4da1-bcdd-5cf925532b2b
📒 Files selected for processing (4)
src/commands/guard.tssrc/commands/inspect.tssrc/commands/restore.tssrc/commands/watch.ts
📜 Review details
🔇 Additional comments (5)
src/commands/guard.ts (1)
7-7: LGTM!Also applies to: 24-38, 82-85, 105-109, 118-122, 140-144, 157-176, 189-193, 206-211, 221-229, 239-264, 283-292
src/commands/inspect.ts (2)
6-6: LGTM!Also applies to: 41-50, 96-100
18-20: 🎯 Functional CorrectnessNo change needed.
src/commands/restore.ts (1)
7-7: LGTM!Also applies to: 20-34, 41-45, 54-70, 81-112, 136-140, 149-149
src/commands/watch.ts (1)
12-12: LGTM!Also applies to: 43-44, 99-108, 119-123, 133-152, 204-208
- guard: reject prefix-tolerant parseInt values (e.g. 100foo, 20.5) by validating complete strings as positive integers before storing in extension policy - watch: include complete watchContract result (instance, wasm, wasmWarning) in batch --json output
43ad363 to
8692701
Compare
18 participants
closes #372 feat(cli): add an interactive 'sorokeep init' setup wizard
What does this PR do?
Why?
Does this touch secret-key handling or transaction submission?
Checklist
npm test)npx tsc --noEmit)npm run lint)console.login core logic