Skip to content

chore: address readiness-audit warnings#62

Merged
naorsabag merged 2 commits into
masterfrom
chore/readiness-audit-fixes
May 4, 2026
Merged

chore: address readiness-audit warnings#62
naorsabag merged 2 commits into
masterfrom
chore/readiness-audit-fixes

Conversation

@naorsabag
Copy link
Copy Markdown
Owner

@naorsabag naorsabag commented May 3, 2026

Summary

Local-only fixes from READINESS-AUDIT.md — every item that doesn't require publishing, force-pushes, or external services. Audit blockers (B1–B9) and external warnings (W1, W6, W7, W11, W12, W13, W14) are out of scope for this PR.

  • W5 openhop init is now idempotent. On a converged machine (every detected client already has the skill), both live and --dry-run modes exit 0. Only failed results or zero detected clients exit non-zero.
  • W10 Server defaults to 127.0.0.1; HOST=0.0.0.0 is set in docker-compose.yml so the in-container port mapping still works. Casual npm run dev is no longer LAN-exposed.
  • W3 README lists examples/self-loops.yaml under ## Examples.
  • W4 (partial) README adds the local-first/no-telemetry line, token-light claim, and platform-badge grid (Claude Code / Cursor / Windsurf / Cline / Continue). Deferred items: npm version badge (needs publish), Discord CTA (no Discord yet).
  • W8 AGENTS.md updated — CI lint/formatter ARE enforced; the stale patch.test.ts known-failure note is removed.
  • W9 Autolink <http://localhost:8788> wrapped in backticks (link-checker safe).

Test plan

  • npm test --workspaces --if-present — cli 83/83, server 19/19, shared 93/93, web 22/22
  • npm run lint — 0 errors (12 pre-existing warnings)
  • npm run format:check — clean
  • Manual: openhop init twice on a fresh machine → second run exits 0 with all clients marked "already installed"
  • Manual: npm run dev outside docker → server visible on 127.0.0.1:8787, not on LAN IP
  • Manual: docker compose up → server reachable through host port mapping

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added support indicators for more AI agents and a new example workflow (self-loops.yaml).
    • README highlights: local-first, no-telemetry, and token-light messaging.
  • Bug Fixes

    • Docker service configured so container ports are reachable from outside Docker.
    • CLI init command refined to return clearer exit codes for not-found and conflict cases.
  • Documentation

    • CI/lint/format checks noted and shared test suite status updated.

Local-only fixes from READINESS-AUDIT.md — no publishing, no force-pushes,
no remote-state changes. Test suites and lint remain clean.

- W5  cli: openhop init exits 0 on a converged machine in both live and
        --dry-run modes; fail only on `failed` results or zero detected
        clients
- W10 server: default bind to 127.0.0.1; HOST=0.0.0.0 in docker-compose
        so the in-container port mapping still works
- W3  README: list examples/self-loops.yaml under ## Examples
- W4  README (partial): local-first/no-telemetry line, token-light claim,
        platform-badge grid (Claude Code/Cursor/Windsurf/Cline/Continue);
        deferred npm-version badge (needs publish) and Discord CTA
- W8  AGENTS.md: CI lint/formatter ARE enforced; remove stale
        patch.test.ts known-failure note
- W9  README: wrap http://localhost:8788 autolink in backticks
- W17 (in openhop-launch repo) — separate change, not in this commit

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 3, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: fb8d2414-6d70-48b1-a231-82fbd5f5dc54

📥 Commits

Reviewing files that changed from the base of the PR and between 9d3f8d9 and 76a01fd.

📒 Files selected for processing (2)
  • packages/cli/src/help-json.ts
  • packages/cli/src/init.ts

Walkthrough

Updates docs (AGENTS.md, README.md), adds a new example, adjusts Docker service env to set HOST=0.0.0.0, changes server startup to respect HOST (default 127.0.0.1), and refines CLI init exit codes and help metadata to add NOT_FOUND and CONFLICT handling.

Changes

Host Binding Configuration

Layer / File(s) Summary
Docker Environment
docker-compose.yml
openhop service environment now sets HOST=0.0.0.0 so the server in-container can bind all interfaces for port mappings.
Server Implementation
packages/server/src/index.ts
Server uses process.env.HOST ?? '127.0.0.1', passes host to app.listen, and includes the bound host in the startup log.

CLI Installation Result Classification

Layer / File(s) Summary
Exit Codes
packages/cli/src/init.ts
Introduces EXIT_NOT_FOUND = 4 and EXIT_CONFLICT = 5.
Result Handling Logic
packages/cli/src/init.ts
Exit policy: if any result status === 'failed'EXIT_CONFLICT; if no detected clients (detectedCount === 0) → EXIT_NOT_FOUND; otherwise EXIT_OK.
Help Metadata
packages/cli/src/help-json.ts
COMMAND_META.init.exitCodes expanded to include NOT_FOUND and CONFLICT alongside existing codes.

Documentation & Examples

Layer / File(s) Summary
CI / Tests Guidance
AGENTS.md
Notes CI enforces npm run lint and npm run format:check; updates Tests section to state shared test suite fully passes and the singular-op-name caveat was updated to plural ops.
Project Overview / Examples
README.md
Adds "Works with" badges for additional agents, reorganizes Features bullets (CLI + HTTP API + web UI, local-first/no-telemetry, token-light), and adds self-loops.yaml to examples.
Manifest
package.json
Referenced in diffs for help/exit-code metadata changes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'chore: address readiness-audit warnings' is directly related to the PR's main objective of implementing fixes from READINESS-AUDIT.md, and accurately summarizes the work across multiple files and domains.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/readiness-audit-fixes

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


Review rate limit: 9/10 reviews remaining, refill in 6 minutes.

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/cli/src/init.ts`:
- Around line 334-337: The code currently calls process.exit(EXIT_GENERIC) for
two policy paths; replace these with documented CLI exit constants instead of
EXIT_GENERIC: when failed.length > 0 (the installation actually failed) use the
conflict exit constant (EXIT_CONFLICT), and when !anyOutcome (no
installs/would-installs/already-installed — essentially “nothing to do”/not
found) use the not-found exit constant (EXIT_NOT_FOUND); update references near
the failed, installed, wouldInstall, alreadyInstalled checks and remove
EXIT_GENERIC usage so only documented exit codes (EXIT_CONFLICT and
EXIT_NOT_FOUND) are returned.
- Around line 335-337: The exit logic treats advisory-only detections as failure
because the boolean anyOutcome omits the advisory array; update the anyOutcome
expression to include advisory (e.g., const anyOutcome = installed.length > 0 ||
wouldInstall.length > 0 || alreadyInstalled.length > 0 || advisory.length > 0)
so that a run with only advisory clients counts as a successful detection and
does not trigger process.exit(EXIT_GENERIC); keep the existing process.exit call
for the no-outcome case.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: aaeb0984-d27e-4f7c-bb64-6181e06da96a

📥 Commits

Reviewing files that changed from the base of the PR and between 3a21b1b and 9d3f8d9.

📒 Files selected for processing (5)
  • AGENTS.md
  • README.md
  • docker-compose.yml
  • packages/cli/src/init.ts
  • packages/server/src/index.ts

Comment thread packages/cli/src/init.ts Outdated
Comment thread packages/cli/src/init.ts Outdated
Address CodeRabbit review on PR #62.

- W5 followup: failed install → CONFLICT (5), no-detected-clients →
  NOT_FOUND (4). EXIT_GENERIC (1) is outside the machine-API set
  documented in .coderabbit.yaml `packages/cli/**` instructions
  (0 success, 2 usage, 3 validation, 4 not-found, 5 conflict, 6 network,
  7 auth).
- Switch the no-detected check to a detection-count formulation so
  advisory-only detections (e.g. Continue.dev only) are correctly
  treated as success rather than exit-non-zero.
- Update help-json COMMAND_META.init to declare NOT_FOUND and CONFLICT
  alongside SUCCESS, GENERIC, USAGE — keeping declared codes in sync
  with what the command can actually emit.

Pre-existing EXIT_GENERIC on the sourceMissing path is left untouched
(out of scope; not introduced by this PR).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@naorsabag naorsabag merged commit 2225743 into master May 4, 2026
7 checks passed
@naorsabag naorsabag deleted the chore/readiness-audit-fixes branch May 4, 2026 05:13
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.

1 participant