Skip to content

feat(cli): add interactive sorokeep init setup wizard - #561

Closed
Primex-Tech wants to merge 908 commits into
TegoLabs:mainfrom
Primex-Tech:feat/370-init-wizard
Closed

Primex-Tech wants to merge 908 commits into
TegoLabs:mainfrom
Primex-Tech:feat/370-init-wizard

Conversation

@Primex-Tech

@Primex-Tech Primex-Tech commented Jul 29, 2026 •

Copy link
Copy Markdown
Contributor

closes #370 feat(cli): add an interactive 'sorokeep init' setup wizard

What does this PR do?

Why?

Does this touch secret-key handling or transaction submission?

  • Yes - see notes above
  • No

Checklist

  • Tests pass (npm test)
  • Type check passes (npx tsc --noEmit)
  • Lint passes (npm run lint)
  • Tests cover the new functionality (TDD preferred - see CONTRIBUTING.md)
  • No unnecessary dependencies added
  • Commit messages follow conventional format
  • No console.log in core logic
  • ADR added if this is a significant design decision (see docs/adr)
  • E2E sandbox tested, if this touches RPC or daemon behavior (see docs/e2e-sandbox.md)

dee-john and others added 30 commits July 1, 2026 10:46
AbdulmalikAlayande and others added 21 commits July 31, 2026 03:30
Extracts CLI construction into createProgram() (src/cli/program.ts) so
scripts/generate-man.ts can introspect the Command tree without running
the CLI, generates man/sorokeep.1, and wires build:man into npm run
build. Per TegoLabs#384.

Trimmed from the original PR before merging:
- A full set of unrelated ".claude/agents/kfc/*", ".claude/settings/
  kfc-settings.json", and ".claude/system-prompts/spec-workflow-
  starter.md" files - generic AI-agent workflow scaffolding with no
  connection to sorokeep or this issue.
- getContractsByTag() and related changes to src/commands/status.ts /
  src/db/repositories.ts - that's issue TegoLabs#379's scope (--tag filtering),
  a different, unrelated feature.

The PR's branch was also quite stale (predated the quiet-hours,
contract_groups, rpc/client.ts any-type, and CI audit-scope fixes
already on main), which produced a large raw diff and one real merge
conflict in src/index.ts (this PR's createProgram() refactor vs. the
already-merged --extension-jitter-ms option). Resolved by moving the
jitter option registration into createProgram() so both are preserved.
…s#548)

Relocates the misplaced test to tests/alerts/ (outside vitest.config.ts's
include glob otherwise) and rewrites it against the current SlackChannel
class / sendPagerDutyAlert function API - the old copy called a removed
sendSlackAlert function.
…ted)

Adds src/alerts/matrix.ts (real Matrix Client-Server API, token auth,
room-scoped delivery) and its tests, per TegoLabs#310.

The PR never registered the channel in builtins.ts, so `--type matrix`
was unreachable from the CLI despite the sender being fully implemented
and tested. Completed the registration (targetOption: "channel", since
the sender's target is a Matrix room ID) plus the matching builtins.test.ts
coverage, following the exact pattern used by every other lazily-imported
channel (discord/telegram/opsgenie). Verified `alerts add --type matrix`
end-to-end at the CLI.
… fixed + completed)

Adds src/alerts/teams.ts (Adaptive Card payload, severity coloring per
event type) and its tests, per TegoLabs#311.

Two things fixed before merging:
- CodeRabbit correctly flagged validateWebhookUrl()'s hostname check:
  `hostname.includes("webhook.office.com")` accepts any hostname that
  merely contains that substring, e.g. an attacker-controlled
  "x.webhook.office.com.evil.com" would pass, silently sending real
  alert content (contract IDs, TTL data) to an attacker-controlled
  server. Changed to an exact/suffix match on the real hostname, plus
  an explicit https-only check. Added regression tests for both.
- The PR never registered the channel in builtins.ts, so `--type teams`
  was unreachable from the CLI. Completed the registration
  (targetOption: "url", matching webhook/discord's pattern) and the
  matching builtins.test.ts coverage.
…#585, fixed + completed)

Adds src/alerts/email.ts (nodemailer SMTP transport, env/config token
resolution, password redaction on error) and its tests, per TegoLabs#312.

Fixed before merging:
- Bumped nodemailer ^7.0.7 -> ^9.0.3 (major version, verified tsc still
  compiles clean and all tests pass unchanged): the pinned 7.x range had
  six high-severity advisories, including SMTP/CRLF command injection
  and an SSRF via the raw-message option. This is a production
  dependency, so `npm audit --omit=dev` would have failed on it.
- The channel was registered in builtins.ts, but src/commands/alerts.ts
  still had a special-cased `--type email` branch printing "Email
  alerting is not yet implemented" *before* the registry was ever
  consulted - making the new channel completely unreachable from the
  CLI. Removed the special case.
- Updated the one existing test that asserted the old "not implemented"
  behavior, and added a real success-path test (registers a config with
  --channel <email>).
- Fixed a lint error (preserve-caught-error) the right way: the error
  handler already redacts the SMTP password from the thrown message,
  but attaching the raw caught error as `cause` would have smuggled the
  unredacted password back in via the cause chain. Redact the caught
  error's own message in place before using it as cause, so the
  guarantee holds through the whole chain.
…s#597, trimmed to scope)

Adds src/alerts/googlechat.ts and registers it in builtins.ts, per TegoLabs#313.
Registration and the sender itself were already correct.

Trimmed from the original PR before merging: a bundled Grafana/Prometheus
observability stack (devops/grafana/*, devops/prometheus/*, docker-compose.
observability.yml, docs/observability.md) - unrelated to this issue, shared
branch lineage with several other open PRs (TegoLabs#594, TegoLabs#595, TegoLabs#596) that also
carry the identical bundle. Left tests/docker/docker-compose.test.ts
untouched by reverting to main's version.

Updated tests/alerts/builtins.test.ts for the 10th channel (the PR's
branch predated matrix/teams/email, so its own copy of this file didn't
know about them).

Note for a follow-up: src/alerts/discord.ts has the same hostname-
validation weakness fixed in TegoLabs#557 (`hostname.includes("discord")`,
even looser than Teams's check) - pre-existing, not part of this PR,
flagging separately.
…egoLabs#318)

PR TegoLabs#568 referenced ChannelDefinition.maxRetries in dispatcher.ts but
never added the field to the interface, so the branch failed to
compile (TS2339). It also shipped a new dispatcher test that called
registerAlertChannel("webhook", { maxRetries: 2 }) — a two-argument
overload that doesn't exist on the real one-argument, throw-on-duplicate
registerAlertChannel API, so the test could never have run against
this codebase.

- Add optional maxRetries/retryBackoffMs to ChannelDefinition
  (registry.ts); dispatcher.ts's lookup already existed from the
  merge and needed no further changes.
- Give telegram a maxRetries: 3 default, matching the issue's own
  rationale (Telegram's Bot API rate limits are stricter than a
  generic webhook's) — every other channel keeps the global
  MAX_RETRY_COUNT default, preserving existing behavior exactly.
- Rewrite the broken test to swap in a real ChannelDefinition
  override via the actual registry API, and restore the registry to
  normal built-in state afterward so it doesn't leak into other
  tests. Added a companion test asserting webhook has no override by
  default.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…egoLabs#555)

Issue TegoLabs#319's scope explicitly excludes the orphaned src/alerts/*.test.ts
files ("reconciling those is phase-8's job, not this issue's") — they
aren't in vitest.config.ts's include glob (tests/**/*.test.ts), so any
tests added there never actually run. Keeping the docs update and the
contract suite applied to webhook/slack, whose real test files live
under tests/alerts/. Applying the contract suite to pagerduty/discord/
telegram is deferred to their dedicated orphan-reconciliation issues
(TegoLabs#354-TegoLabs#356).
- core/init: reject non-integer or non-positive target TTL / threshold values (Number.isSafeInteger) and reject threshold >= target TTL before any DB writes or network calls, matching the guard command constraints
- commands/init: stop treating --target-ttl 0 / --threshold 0 as unset (falsy), and only prompt for guard enablement when neither flag is provided
- index.ts: restore LF line endings to match the repository convention

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 12

🤖 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/init.ts`:
- Around line 63-74: Extract the duplicated initialization result flow from the
non-interactive and interactive branches into a shared helper that accepts the
resolved answers, invokes runInitWizard, handles unsuccessful results with the
existing error and exit behavior, saves the config, and prints the success
message. Replace both branch-specific copies with calls to this helper,
preserving their existing answer values and variable mappings.
- Around line 45-49: Update all failure guards in runInitWizard, including the
nonInteractive validation and the additional process.exit(1) sites, to
explicitly return immediately after process.exit(1). Ensure stubbed exits cannot
continue into saveConfig or success output while preserving the existing error
messages and exit behavior.
- Line 22: Remove the "testnet" default value from the .option call for
--network to allow the fallback chain to work correctly. Update the network
assignment to use the fallback pattern: for non-interactive mode, use network ??
existingConfig.network (or configuredNetwork), and for interactive mode, use
network || (await rl.question with the prompt) || existingConfig.network. This
preserves saved network configuration and makes the interactive prompt reachable
when no --network flag is provided.
- Around line 91-95: Update the guard configuration flow around enableGuard,
resolvedTargetTtlLedgers, and resolvedThresholdLedgers so TTL and threshold
prompts execute only when enableGuard is true; when disabled, retain or assign
appropriate non-prompt defaults while preserving the existing enabled behavior
and policy output.
- Around line 27-28: The `--guard-enabled` and `--guard-disabled` options are
mutually exclusive but currently both can be passed, with line 41 resolving them
post-hoc. Chain `.conflicts("guardDisabled")` to the `--guard-enabled` option
declaration and `.conflicts("guardEnabled")` to the `--guard-disabled` option
declaration so the CLI rejects contradictory input at parse time. Import Option
from commander if not already present to ensure the conflicts method is
available.

In `@src/core/init.ts`:
- Around line 64-65: Separate the alert threshold from the guard threshold in
the initialization flow: add a dedicated alert-threshold input with its own
default, validate it with isPositiveInteger like the other ledger settings, and
use it when writing alert_configs.threshold_ledgers. Keep thresholdLedgers
sourced only from answers.guardThresholdLedgers for the guard policy.
- Around line 98-103: The init flow around insertAlertConfig must be idempotent:
prevent repeated runs from creating duplicate alert_configs for the same
contract_id, channel_type, and channel_target. Update insertAlertConfig in the
repository, or perform an equivalent existence check before insertion, while
preserving updates to the existing contract and extension policy.
- Around line 82-110: Wrap the post-watch database writes in runInitWizard,
specifically insertAlertConfig and upsertExtensionPolicy, within a single
db.transaction(...) callback so both commit or roll back together. Keep
watchContract unchanged and preserve the existing write arguments and failure
handling.
- Around line 1-7: Remove the top-level registerBuiltinChannels() call from
src/core/init.ts and invoke it explicitly inside runInitWizard before any
getAlertChannel usage, or from the CLI bootstrap in src/index.ts. Preserve
channel availability while eliminating registration as an import-time side
effect.

In `@tests/commands/init.test.ts`:
- Around line 17-20: Update the init test’s saveConfig mock and assertions to
verify the expected saveConfig call contract without asserting permissions
applied by the mock. Add a focused test for the real saveConfig implementation
in config.ts that writes the file and verifies its mode is 0o600.
- Around line 33-62: Update the non-interactive error branches in
registerInitCommand to return immediately after process.exit, preventing
saveConfig from running. Then add tests covering --yes without --contract,
--channel-type, or --channel-target, and runInitWizard returning success: false
with an error; assert each reports the error, does not call saveConfig, and
preserves the existing successful flow.

In `@tests/core/init.test.ts`:
- Around line 99-132: Add tests in the init wizard suite for the unsupported
channel and watch-failure branches in runInitWizard: verify an unsupported
channelType returns the “Unsupported alert channel type” error without creating
persisted state, and mock watchContract to return success: false, then verify
the failure propagates and neither alert configuration nor extension policy is
written.
🪄 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: 4d23377b-e751-4b9d-93e2-1f82e89e7f77

📥 Commits

Reviewing files that changed from the base of the PR and between a9983ee and e5ca7c8.

📒 Files selected for processing (5)
  • src/commands/init.ts
  • src/core/init.ts
  • src/index.ts
  • tests/commands/init.test.ts
  • tests/core/init.test.ts
📜 Review details
🧰 Additional context used
🪛 ast-grep (0.45.0)
tests/commands/init.test.ts

[warning] 17-17: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.writeFileSync(configPath, JSON.stringify(config))
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').

(detect-non-literal-fs-filename-typescript)

🔇 Additional comments (5)
src/index.ts (1)

22-22: LGTM!

Also applies to: 54-54

src/core/init.ts (1)

28-80: LGTM!

tests/core/init.test.ts (1)

14-42: LGTM!

Also applies to: 44-97

src/commands/init.ts (1)

9-14: LGTM!

Also applies to: 77-90

tests/commands/init.test.ts (1)

26-31: LGTM!

Comment thread src/commands/init.ts Outdated
Comment thread src/commands/init.ts Outdated
Comment thread src/commands/init.ts
Comment thread src/commands/init.ts Outdated
Comment thread src/commands/init.ts Outdated
Comment thread src/core/init.ts
Comment thread src/core/init.ts Outdated
Comment thread tests/commands/init.test.ts
Comment thread tests/commands/init.test.ts Outdated
Comment thread tests/core/init.test.ts
Integrate the 48-commit upstream main advance (f5e41ed). Upstream moved all
command registration out of src/index.ts into src/cli/program.ts, so register
the new init command there instead. No other files overlap with upstream.

- src/index.ts: keep upstream's createProgram-based entry point
- src/cli/program.ts: register init command alongside the existing commands
- commands/init: remove the --network "testnet" default so the saved network
  and the interactive network prompt are actually used
- commands/init: add --alert-threshold so the alert trigger is independent
  from the guard threshold; mark --guard-enabled/--guard-disabled as mutually
  exclusive via Option.conflicts
- commands/init: return after every process.exit(1), share the result/save
  handling between the interactive and non-interactive paths, and skip the
  guard value prompts when the guard is disabled
- commands/init: register built-in channels from the action instead of at
  module import time
- core/init: register built-in channels inside runInitWizard instead of at
  module import time; validate the alert threshold as a positive integer;
  wrap the alert-config and policy writes in a single transaction; make
  repeated init runs idempotent by replacing the contract's alert configs
- tests: assert the real saveConfig 0o600 permissions, the saved-network
  fallback, the --yes missing-flag failure, and the mutually exclusive guard
  flags; cover the unsupported-channel, watch-failure, idempotent re-run, and
  independent-alert-threshold branches
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(cli): add an interactive 'sorokeep init' setup wizard