consolidate: collapse onenv-api into the onenv CLI (0.7.0) - #1
Closed
doublej wants to merge 3 commits into
Closed
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ 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. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (43)
📝 WalkthroughWalkthroughThe PR consolidates onenv from a two-component system (HTTP API + CLI manager) into a single CLI binary, removing the ChangesConsolidate to single CLI with macOS mutation confirmation
Sequence DiagramsequenceDiagram
participant User
participant CLI as onenv CLI
participant ConfirmMutation as confirmMutation()
participant osascript as osascript
User->>CLI: onenv set namespace key value
CLI->>ConfirmMutation: confirmMutation(action, namespace, details)
ConfirmMutation->>ConfirmMutation: check ONENV_CONFIRM_MUTATIONS env
alt ONENV_CONFIRM_MUTATIONS=1 and macOS
ConfirmMutation->>osascript: execute AppleScript dialog
osascript->>User: show approval prompt with timeout
alt user approves before timeout
User-->>osascript: allow
osascript-->>ConfirmMutation: stdout=allowed
ConfirmMutation-->>CLI: resolve
else user denies or timeout
User-->>osascript: deny/no response
osascript-->>ConfirmMutation: stdout=denied/timeout
ConfirmMutation-->>CLI: reject CliError(PERMISSION_DENIED)
CLI-->>User: error
end
else ONENV_CONFIRM_MUTATIONS not set or non-macOS
alt non-macOS
ConfirmMutation-->>CLI: reject CliError(PERMISSION_UNSUPPORTED)
else disabled (env not set or ≠ "1")
ConfirmMutation-->>CLI: resolve (no-op)
end
end
alt confirmed or disabled
CLI->>CLI: perform mutation (set value in 1Password)
CLI-->>User: success
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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
🧹 Nitpick comments (1)
onenv-manager/src/lib/permission.test.ts (1)
29-35: ⚡ Quick winAssert the exact permission contract here.
On darwin this test exits without asserting anything, and elsewhere it only checks
CliError, so a regression fromPERMISSION_UNSUPPORTEDto any other CLI failure would still pass. Please pin the specific error code/message here and add a mocked macOS allow/deny test so the new branch is covered on any CI host.🤖 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 `@onenv-manager/src/lib/permission.test.ts` around lines 29 - 35, The test currently only checks for any CliError and short-circuits on darwin; update it to assert the exact permission contract by expecting the thrown CliError to have the specific code/message 'PERMISSION_UNSUPPORTED' (e.g., check error.code or error.message) when process.platform !== 'darwin'. Additionally add two darwin-specific tests that force process.platform to 'darwin' (or mock the platform check) and mock the macOS permission prompt/flow used by confirmMutation to simulate an allow and a deny: assert that the allow branch resolves (or returns the expected success) and the deny branch rejects with the exact permission error (or specific deny code/message). Reference confirmMutation and CliError so the tests locate and assert the precise error contract.
🤖 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 `@docs/guides/service-account-setup.md`:
- Around line 18-20: The fenced code block containing the environment variable
example (OP_SERVICE_ACCOUNT_TOKEN=ops_eyJ...) is missing a language identifier;
update that markdown code fence to include a shell language tag (e.g., ```bash
or ```shell) so the block reads as a bash/shell snippet and satisfies markdown
linting and syntax highlighting requirements.
- Around line 26-28: The fenced code block containing the
OP_SERVICE_ACCOUNT_TOKEN example is missing a language identifier; update the
triple-backtick fence for that block to include a shell/bash identifier (e.g.,
change ``` to ```bash) so the snippet is highlighted correctly and passes
markdown linting—locate the fenced block that shows
OP_SERVICE_ACCOUNT_TOKEN=op://Personal/<item-id>/credential and add the language
token after the opening backticks.
---
Nitpick comments:
In `@onenv-manager/src/lib/permission.test.ts`:
- Around line 29-35: The test currently only checks for any CliError and
short-circuits on darwin; update it to assert the exact permission contract by
expecting the thrown CliError to have the specific code/message
'PERMISSION_UNSUPPORTED' (e.g., check error.code or error.message) when
process.platform !== 'darwin'. Additionally add two darwin-specific tests that
force process.platform to 'darwin' (or mock the platform check) and mock the
macOS permission prompt/flow used by confirmMutation to simulate an allow and a
deny: assert that the allow branch resolves (or returns the expected success)
and the deny branch rejects with the exact permission error (or specific deny
code/message). Reference confirmMutation and CliError so the tests locate and
assert the precise error contract.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: df2f0f59-bf7a-421b-ab7d-00ba15e1f9b8
⛔ Files ignored due to path filters (1)
onenv-api/bun.lockis excluded by!**/*.lock
📒 Files selected for processing (41)
.atlas.github/workflows/ci.ymlAGENTS.mdCHANGELOG.mdCLAUDE.mdINSTALL.mdREADME.mddocs/CLAUDE-onenv.mddocs/examples/onenv-api.plistdocs/guides/agent-api.mddocs/guides/ci-and-deploys.mddocs/guides/running-commands.mddocs/guides/service-account-setup.mddocs/src/routes/+page.svelteinstall.tsonenv-api/.env.exampleonenv-api/README.mdonenv-api/biome.jsononenv-api/package.jsononenv-api/src/index.tsonenv-api/src/lib/config.test.tsonenv-api/src/lib/config.tsonenv-api/src/lib/logging.tsonenv-api/src/lib/manager-service.tsonenv-api/src/lib/onenv-client.test.tsonenv-api/src/lib/onenv-client.tsonenv-api/src/lib/op-token.tsonenv-api/src/lib/permission.tsonenv-api/src/lib/rate-limit.tsonenv-api/src/lib/state-store.tsonenv-api/src/server.tsonenv-api/tsconfig.jsononenv-manager/package.jsononenv-manager/src/cli.tsonenv-manager/src/commands/prime-data-api.tsonenv-manager/src/commands/prime-data.tsonenv-manager/src/commands/prime-md.tsonenv-manager/src/commands/prime-xml.tsonenv-manager/src/commands/prime.tsonenv-manager/src/lib/permission.test.tsonenv-manager/src/lib/permission.ts
💤 Files with no reviewable changes (23)
- onenv-api/src/lib/config.test.ts
- docs/guides/agent-api.md
- docs/examples/onenv-api.plist
- onenv-api/src/lib/logging.ts
- onenv-api/tsconfig.json
- onenv-api/package.json
- docs/guides/running-commands.md
- onenv-api/src/lib/op-token.ts
- onenv-api/.env.example
- onenv-api/biome.json
- onenv-api/src/lib/state-store.ts
- onenv-api/src/lib/onenv-client.test.ts
- onenv-api/src/index.ts
- onenv-api/src/lib/config.ts
- onenv-api/src/lib/permission.ts
- .github/workflows/ci.yml
- onenv-api/src/lib/rate-limit.ts
- onenv-api/src/lib/manager-service.ts
- onenv-api/src/server.ts
- onenv-api/src/lib/onenv-client.ts
- docs/guides/ci-and-deploys.md
- onenv-api/README.md
- onenv-manager/src/commands/prime-data-api.ts
Comment on lines
18
to
20
| ``` | ||
| OP_SERVICE_ACCOUNT_TOKEN=ops_eyJ... | ||
| ``` |
There was a problem hiding this comment.
Add language identifier to fenced code block.
The code block should specify bash or shell as the language identifier for proper syntax highlighting and to comply with markdown linting rules.
📝 Proposed fix
-```
+```bash
OP_SERVICE_ACCOUNT_TOKEN=ops_eyJ...</details>
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.22.1)</summary>
[warning] 18-18: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @docs/guides/service-account-setup.md around lines 18 - 20, The fenced code
block containing the environment variable example
(OP_SERVICE_ACCOUNT_TOKEN=ops_eyJ...) is missing a language identifier; update
that markdown code fence to include a shell language tag (e.g., ```bash or
linting and syntax highlighting requirements.
When ONENV_CONFIRM_MUTATIONS=1 is set, every mutating CLI path — set, edit, unset, import — calls confirmMutation() before touching 1Password. On macOS this pops an osascript Allow/Deny dialog naming the action, namespace, and key(s). Deny → throws PERMISSION_DENIED. Non-macOS with the env set → throws PERMISSION_UNSUPPORTED so the guard fails loud rather than silently no-op'ing. Default off. Off-path zero cost: confirmMutation returns immediately when the env var isn't '1'. Ported from the soon-to-be-deleted onenv-api withPermission() wrapper, with the wiring point moved from HTTP handlers into the CLI command files so a single guard covers all entry points.
Audit turned up zero callers of the api (no running process, no launchctl entries, no references anywhere in ~/Documents/development or ~/.config). The CLI is the agent surface, and the permission brokering the api uniquely offered is now an opt-in CLI mode (ONENV_CONFIRM_MUTATIONS, previous commit). Deletes onenv-api/ entirely. Strips api references from README, INSTALL, CLAUDE.md, AGENTS.md, docs/CLAUDE-onenv.md, the docs landing page and guides, the CI workflow, and .atlas. Removes the docs/examples/onenv-api.plist LaunchAgent template and the docs/guides/agent-api.md guide. install.ts simplifies to single- package setup (no more api token prompt). Primer drops the <api> section and adds <permission>: prime-data.ts swaps the ApiSection field for PermissionSection, prime-md.ts and prime-xml.ts renderers updated, prime-data-api.ts removed. The strict-export injection warning added in 0.6.0 is preserved. Net delete heavy.
Adds a 0.7.0 entry covering the ONENV_CONFIRM_MUTATIONS gate and the onenv-api removal. Notes the removal as breaking for anyone who was running the api — switch to invoking the CLI directly with the env var set if you want mutation prompts.
doublej
force-pushed
the
worktree-consolidate
branch
from
May 21, 2026 22:55
5243999 to
ba88c10
Compare
doublej
added a commit
that referenced
this pull request
May 22, 2026
Deletes the Express HTTP server outright and ports its valuable bit (the macOS AppleScript permission dialog) into the CLI as an opt-in ONENV_CONFIRM_MUTATIONS=1 guard for set / edit / unset / import. Off by default; deny / timeout raises PERMISSION_DENIED; non-macOS errors with PERMISSION_UNSUPPORTED. Picks up the non-doc changes from PR #1 (worktree-consolidate). The docs on main were already api-free from earlier today, so this just lands the code half. - delete onenv-api/ (whole subtree) - delete onenv-manager/src/commands/prime-data-api.ts - add onenv-manager/src/lib/permission.{ts,test.ts} - wire confirmMutation into set / edit / unset / import - drop the <api> section from the primer (xml, md, json); add a <permission> section - install.ts: drop the API_TOKEN prompt, the api install/build/start steps, and the agent-api outro - CI: drop the api job - .atlas: refresh description Keeps the 90-day-expires fix from main (worktree-consolidate predated it). 89 tests pass (was 86 + 3 new permission tests).
Owner
Author
|
Superseded — collapse landed on main as 83479b0 (api removal + permission gate) and d28f2ae (source moved to repo root); both shipped in v0.7.0 with extra polish (mermaid README, audit-doc scrub, release harness). See https://github.com/doublej/onenv/releases/tag/v0.7.0. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
onenv-apiExpress server outright — a consumer audit (no running processes, no launchctl/LaunchAgent entries, no references across~/Documents/developmentor~/.config) turned up zero callers.ONENV_CONFIRM_MUTATIONS=1guard forset/edit/unset/import. Off by default; deny / timeout raisesPERMISSION_DENIED; non-macOS errors withPERMISSION_UNSUPPORTED.<api>section and add a<permission>section (XML, Markdown, JSON renderers).README.md,INSTALL.md,CLAUDE.md,AGENTS.md,docs/CLAUDE-onenv.md, the docs landing page, the docs guides, the CI workflow, and the project.atlas.Net: +245 / −2309 across 42 files.
just checkclean, 48/48 tests pass (was 45 + 3 new for the permission gate).Test plan
just check— biome,tsc --noEmit, vitest (48 passing), loc-checkbun build --target node install.ts— compilesnode dist/cli.js --version→0.7.0onenv primein JSON / XML / Markdown formats — noonenv-api,x-onenv-token,/v1/,AGENT_API_TOKEN, or:4317ONENV_CONFIRM_MUTATIONS=1 onenv set _test_ TMP_KEY— Allow → key set; Deny →PERMISSION_DENIEDonenv unset _test_ TMP_KEYcleanupbun run install.ts— confirm the wizard no longer prompts forAGENT_API_TOKENand that the linkedonenvbinary still worksSummary by CodeRabbit
New Features
ONENV_CONFIRM_MUTATIONSenvironment variableRemovals
Documentation