Skip to content

feat(oauth): v3.0 Part B/2 structure + Anthropic Max/Pro OAuth support (#83–#85)#87

Merged
Steffen025 merged 9 commits intomainfrom
release/oauth-anthropic-max-v2
Mar 22, 2026
Merged

feat(oauth): v3.0 Part B/2 structure + Anthropic Max/Pro OAuth support (#83–#85)#87
Steffen025 merged 9 commits intomainfrom
release/oauth-anthropic-max-v2

Conversation

@Steffen025
Copy link
Owner

@Steffen025 Steffen025 commented Mar 21, 2026

Summary

3 commits from dev:

What this adds

Structure Cleanup (#83)

  • Move orphan skills into Utilities/ category
  • Rename voice-server/VoiceServer/
  • Add missing PAI root docs
  • Remove obsolete planning docs

Anthropic Max/Pro OAuth Support (#84 + #85)

Users with Anthropic Max ($100/mo) or Pro ($20/mo) can use OpenCode without API keys.

  • Installer preset anthropic-max — one-time setup, extracts OAuth token from macOS Keychain
  • API-fix plugin anthropic-max-bridge.js — 3 OAuth fixes (system prompt format, beta header, Bearer token)
  • Auto-refresh plugin anthropic-token-bridge.js — checks every 5 messages, refreshes from Keychain
  • Critical bug fix: claudeAiOauth nested Keychain structure was not parsed correctly
  • Contrib package contrib/anthropic-max-bridge/ — standalone for external users
  • Manual fallback PAI-Install/anthropic-max-refresh.sh

Testing

  • Verified against live Anthropic API (claude-sonnet-4-6) ✅
  • Auto-refresh working without Safari popup ✅
  • CI secret scanner passes ✅

Summary by CodeRabbit

  • New Features

    • macOS Voice Server: install/start/stop/restart/status/uninstall tooling, menu‑bar helper, health/test notifications, pronunciation overrides, and voice profiles; voice selection now resolves dynamically and includes agent title.
    • Anthropic Max/Pro OAuth support: new provider preset, installer integration, bridge plugins, and token refresh tooling.
    • Skill tooling: skill index generator, search CLI, and structure validator.
    • Pipelines: new primitive and guidance.
  • Documentation

    • Plugin-system docs, Anthropic Max guides/technical refs, pipelines and architecture updates.

…ver/, PAI root docs, voice ID fix (#83)

* refactor: move orphan skills into Utilities/ category

Moved 6 flat-level skills into Utilities/ for PAI 4.0.3 parity:
- AudioEditor → Utilities/AudioEditor (was missing from Utilities)
- CodeReview → Utilities/CodeReview (OpenCode-specific)
- OpenCodeSystem → Utilities/OpenCodeSystem (OpenCode-specific)
- Sales → Utilities/Sales (OpenCode-specific)
- System → Utilities/System (OpenCode-specific)
- WriteStory → Utilities/WriteStory (OpenCode-specific)

OpenCode-specific skills preserved in Utilities/ category.

* feat: add missing PAI/ root docs from 4.0.3

Added 3 files missing from PAI/ root:
- doc-dependencies.json (documentation dependency graph)
- PIPELINES.md (pipeline system documentation)
- THEHOOKSYSTEM.md (hooks/plugin system documentation)

All .claude/ references replaced with .opencode/.

* refactor: rename voice-server/ to VoiceServer/ + add missing files

Renamed: voice-server/ → VoiceServer/ (CamelCase, matches PAI 4.0.3)

Added missing files from PAI 4.0.3:
- install.sh, start.sh, stop.sh, restart.sh, status.sh, uninstall.sh
- voices.json, pronunciations.json
- menubar/ directory

All .claude/ references replaced with .opencode/ in copied files.

* fix: restore missing files + remove legacy content from skills/PAI/ deletion

Restored from git history (accidentally deleted with skills/PAI/):
- PAI/THEPLUGINSYSTEM.md (OpenCode replacement for THEHOOKSYSTEM.md)
- PAI/Tools/GenerateSkillIndex.ts + ValidateSkillStructure.ts + SkillSearch.ts
- PAI/PIPELINES.md, PAI/doc-dependencies.json
- PAISECURITYSYSTEM/HOOKS.md

Removed legacy files NOT in 4.0.3 upstream:
- PAI/BACKUPS.md, BROWSERAUTOMATION.md, CONSTITUTION.md, RESPONSEFORMAT.md,
  SCRAPINGREFERENCE.md, TERMINALTABS.md (all pre-v3.0 artifacts)
- PAI/UPDATES/ directory (not in 4.0.3)
- PAI/Workflows/ (11 legacy workflows not in 4.0.3 root)
- PAI/USER/ reset to 4.0.3 template (README.md placeholders only,
  removed 47 personal template files that don't belong in public repo)

Fixed references: THEHOOKSYSTEM → THEPLUGINSYSTEM in SKILL.md and DOCUMENTATIONINDEX.md

Regenerated skill-index.json (52 skills, 7 categories)

PAI/ root now matches 4.0.3 exactly, with only 2 justified deviations:
- THEPLUGINSYSTEM.md (replaces THEHOOKSYSTEM.md per ADR-001)
- MINIMAL_BOOTSTRAP.md (OpenCode lazy loading, created in v3.0 WP-C)

* chore: remove INSTALLER-REFACTOR-PLAN.md (internal planning doc)

* fix: replace all hardcoded ElevenLabs voice IDs with dynamic routing

VoiceServer sendNotification() now uses title (agent name) instead of
voiceId for voice resolution. This enables dynamic provider switching
via TTS_PROVIDER env var (google/elevenlabs/macos).

Changed: voice_id:'fTtv3eikoepIosk8dTZ5' → voice_id:'default' + title:'AgentName'

Files affected:
- VoiceServer/server.ts (line 425: voiceId → safeTitle)
- PAI/SKILL.md (7 phase curls)
- PAI/Algorithm/v3.7.0.md (2 voice curls)
- agents/Algorithm.md (frontmatter + 3 references)
- PAI/Tools/algorithm.ts (VOICE_ID constant)

Zero hardcoded ElevenLabs IDs remain in the codebase.

* fix: address CodeRabbit review findings on PR #83

Fixes verified against actual code:

Tools path resolution:
- SkillSearch.ts: INDEX_FILE pointed to PAI/Skills/ instead of skills/
- ValidateSkillStructure.ts: SKILLS_DIR went 3 levels up instead of 2
- Both now match GenerateSkillIndex.ts (.opencode/skills/)

Symlink cycle prevention:
- GenerateSkillIndex.ts: findSkillFiles now tracks visited canonical paths
- ValidateSkillStructure.ts: scanDirectory now populates visitedPaths Set

Error handling:
- GenerateSkillIndex.ts + SkillSearch.ts: main().catch now exits non-zero

Documentation accuracy:
- doc-dependencies.json: add missing THEPLUGINSYSTEM.md entry
- THEHOOKSYSTEM.md: PAI_DIR example $HOME/.claude → $HOME/.opencode
- THEPLUGINSYSTEM.md: library count 8 → 9 (matches table)
- pronunciations.json: stale skills/PAI/USER/ → PAI/USER/ path

Stats accuracy:
- ValidateSkillStructure.ts: category SKILL.md files no longer counted as flat skills
- pai-voice.5s.sh: PAI_DIR default $HOME/.claude → $HOME/.opencode

Skipped (verified not applicable):
- PIPELINES.md paths: upstream semantics, both paths correct in context
- VoiceServer shell script hardening: upstream 4.0.3 files, out of scope
- PAISECURITYSYSTEM/HOOKS.md: doc is factually accurate for hook architecture
- THEHOOKSYSTEM.md voiceId examples: settings.json config docs, not hardcoded IDs

* fix: delete THEHOOKSYSTEM.md and HOOKS.md, complete hook→plugin migration (ADR-001)

- Delete .opencode/PAI/THEHOOKSYSTEM.md (superseded by THEPLUGINSYSTEM.md)
- Delete .opencode/PAISECURITYSYSTEM/HOOKS.md (superseded by PLUGINS.md)
- Update doc-dependencies.json: remove THEHOOKSYSTEM.md entry, rename
  hook-system section to plugin-system referencing THEPLUGINSYSTEM.md
- Update PAISECURITYSYSTEM/ARCHITECTURE.md: all hook→plugin terminology,
  execution flow updated for plugin model (throw Error, not exit codes)
- Update PAISECURITYSYSTEM/README.md: hook→plugin in prose and file tree
- Fix GenerateSkillIndex.ts division by zero when totalSkills == 0
…ion (#84)

* feat(installer): add anthropic-max preset for Max/Pro OAuth subscription

Adds 'anthropic-max' as a built-in installer preset so users with an
existing Anthropic Max or Pro subscription can use PAI-OpenCode without
paying extra for API keys.

## What's new

### Provider preset
- provider-models.ts: add 'anthropic-max' to ProviderName union, PROVIDER_MODELS,
  and PROVIDER_LABELS (label: 'Anthropic Max/Pro (OAuth)')
- Model tiers: haiku-4-5 / sonnet-4-6 / opus-4-6 — same models as 'anthropic'
  but routed through OAuth instead of an API key

### Installer engine (steps-fresh.ts)
- installAnthropicMaxBridge(): new helper function that:
    1. Copies .opencode/plugins/anthropic-max-bridge.js to the local plugins dir
    2. Extracts the OAuth token from macOS Keychain (service: Claude Code-credentials)
    3. Parses claudeAiOauth.{accessToken,refreshToken,expiresAt}
    4. Writes token to ~/.local/share/opencode/auth.json under the 'anthropic' key
    5. Returns success + hours remaining, non-throwing (install continues on failure)
- stepInstallPAI(): call installAnthropicMaxBridge() when provider === 'anthropic-max'
- runFreshInstall(): skip API key prompt for anthropic-max, show Claude CLI check message

### CLI (quick-install.ts)
- --preset anthropic-max works without --api-key (all other presets still require it)
- Inline instructions printed when anthropic-max selected
- Updated help text and examples

### New files
- .opencode/plugins/anthropic-max-bridge.js: 80-line minimal plugin (3 API fixes only)
    Fix 1: system prompt array-of-objects format (prevents HTTP 400)
    Fix 2: anthropic-beta: oauth-2025-04-20 header (prevents HTTP 401)
    Fix 3: Authorization: Bearer <token> instead of x-api-key
- PAI-Install/anthropic-max-refresh.sh: one-command token refresh after expiry
- docs/providers/anthropic-max.md: user-facing setup guide, troubleshooting, tech details

## Usage

Interactive:
  bash install.sh  →  choose 'Anthropic Max/Pro (OAuth)'

Headless:
  bun PAI-Install/cli/quick-install.ts --preset anthropic-max --name 'User'

Token refresh (every ~8-12 hours):
  bash PAI-Install/anthropic-max-refresh.sh

## Notes
- macOS only (requires Keychain access)
- Requires Claude Code CLI installed and authenticated
- Using OAuth tokens in third-party tools may violate Anthropic ToS
- Non-fatal: if Keychain extraction fails, install continues with a warning

* fix(anthropic-max): address CodeRabbit findings + add standalone contrib package

CodeRabbit fixes (PR #84):
- anthropic-max-refresh.sh: use quoted heredoc ('PYEOF') and export tokens
  as env vars to prevent shell injection via token values
- anthropic-max-refresh.sh: add validation for REFRESH_TOKEN (non-empty) and
  EXPIRES_AT (numeric, >0) before writing auth.json
- steps-fresh.ts: fix progress regression — onProgress(92) called after 95%
  steps; corrected to 96
- steps-fresh.ts: replace brittle import.meta.url.replace('file://', '') with
  fileURLToPath(new URL(import.meta.url)) from node:url
- anthropic-max-bridge.js: add explicit comment explaining the authorize block
  returns { type: 'failed' } intentionally (tokens come from auth.json, not
  from an OAuth redirect flow)
- docs/providers/anthropic-max.md: add YAML frontmatter (title, tags,
  published, type, summary) and convert tip block to Obsidian callout syntax

Goal 2 — standalone extractable package:
- contrib/anthropic-max-bridge/ — self-contained, no Bun/PAI required
  - install.sh (bash only, injection-safe heredoc, validates all tokens)
  - refresh-token.sh (same safety fixes as PAI-Install version)
  - plugins/anthropic-max-bridge.js (identical to .opencode/plugins/)
  - README.md (quick-start focused, team-sharing notes)
  - TECHNICAL.md (curl proof, token structure, what we ruled out)
- docs/providers/anthropic-max.md: Option B now points to contrib/ in this
  repo instead of the external jeremy-opencode repository

* fix(anthropic-max): address second-round CodeRabbit findings

- anthropic-max-bridge.js: guard experimental.chat.system.transform against
  output.system being undefined/null/non-array before calling .map(); normalise
  to [] if missing, wrap bare string/object into array, then apply existing
  string->{type,text} conversion
- contrib/anthropic-max-bridge/install.sh: chmod auth.json to 0o600
  (owner read/write only) immediately after writing via os.chmod + stat module;
  mirrors the chmodSync(authFile, 0o600) already present in steps-fresh.ts
- steps-fresh.ts: add zero-case message when hoursRemaining === 0 ('Token
  installed — expired or valid for less than 1 hour. Run
  anthropic-max-refresh.sh now.'); Math.max(0, ...) clamp was already present

* fix(anthropic-max): check Bun.spawn exit code for Keychain lookup

Previously proc.exited was awaited but its return value discarded, so a
non-zero exit from the 'security' command (e.g. item not found) was only
caught via an empty-stdout check. Stdout and stderr are now read concurrently
with proc.exited via Promise.all to avoid pipe deadlock; the exit code is
inspected before accepting keychainJson, and stderr is surfaced in the error
message when present.

* nitpick(steps-fresh): add warning when refresh token is missing

When extracting OAuth tokens from Keychain, warn users if the refresh
token is undefined. This alerts them that they'll need to re-authenticate
with 'claude' when the access token expires.
* feat(oauth): add runtime auto-refresh token bridge

- Add anthropic-token-bridge.js/.ts: checks token every 5 messages,
  auto-refreshes from macOS Keychain when <2h remaining
- Add lib/token-utils.ts + lib/refresh-manager.ts: token lifecycle logic
  (3 strategies: Keychain → claude setup-token → exchange API)
- Add info()/warn()/error() wrappers to file-logger.ts (needed by token bridge)
- Add anthropic-token-bridge.js to contrib/anthropic-max-bridge/plugins/
- Update contrib/install.sh: copies both plugins, updates token refresh note

* docs(contrib): update README + TECHNICAL for auto-refresh token bridge

- README: Token Expiry → auto-refresh primary, manual fallback secondary
- README: File Reference → add anthropic-token-bridge.js entry
- README: install.sh step → mention both plugins copied
- README: token flow diagram → show both plugins
- README: HTTP 401 troubleshooting → mention auto-refresh failed
- TECHNICAL: Token lifetime → replace manual-only with auto-refresh details

* fix(oauth): address CI findings in token bridge

- token-utils.ts: os.homedir() instead of process.env.HOME (avoids unexpanded ~)
- refresh-manager.ts: remove dead promisify(spawn), add execCommand timeout (15s),
  add pragma: allowlist secret on Keychain extractions, mask token in logs
- anthropic-token-bridge.ts/.js: read role from output.message instead of input
- Regenerate compiled .js from fixed TS sources (both plugin + contrib copy)

* fix(oauth): parse claudeAiOauth nested structure from Keychain

The macOS Keychain stores credentials as { claudeAiOauth: { accessToken, refreshToken } }
but the plugin was looking for flat structure. This caused auto-refresh to fail with
hasAccess: false, hasRefresh: false errors, falling back to browser OAuth flow.

Now correctly extracts from nested structure with fallback for legacy formats.
@coderabbitai
Copy link

coderabbitai bot commented Mar 21, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Replaces hard-coded ElevenLabs voice IDs with "default" and adds title: "Jeremy" in notify payloads; introduces macOS VoiceServer lifecycle scripts, menubar, voices/pronunciations, and voice config changes; adds Anthropic Max/Pro OAuth bridge (plugins, token refresh libs, installers); adds Pipelines docs and new skill tooling; reorganizes skill index and removes some PRD docs.

Changes

Cohort / File(s) Summary
Voice server & macOS lifecycle
.opencode/VoiceServer/install.sh, .opencode/VoiceServer/start.sh, .opencode/VoiceServer/stop.sh, .opencode/VoiceServer/restart.sh, .opencode/VoiceServer/status.sh, .opencode/VoiceServer/uninstall.sh, .opencode/VoiceServer/menubar/install-menubar.sh, .opencode/VoiceServer/menubar/pai-voice.5s.sh, .opencode/VoiceServer/voices.json, .opencode/VoiceServer/pronunciations.json, .opencode/VoiceServer/server.ts
Add macOS LaunchAgent installer/entrypoint/management scripts, menubar integration, voice profile and pronunciation configs; server.ts now uses voiceConfig.voice_name/safeTitle for agentName resolution.
Agent payloads & docs
.opencode/PAI/Algorithm/v3.7.0.md, .opencode/agents/Algorithm.md, .opencode/PAI/SKILL.md, .opencode/PAI/Tools/algorithm.ts
Replace specific ElevenLabs voice_id (fTtv3eikoepIosk8dTZ5) with "default" and add title: "Jeremy" in documentation examples and runtime notify calls.
Anthropic OAuth bridge & contrib
.opencode/plugins/anthropic-max-bridge.js, .opencode/plugins/anthropic-token-bridge.js, .opencode/plugins/anthropic-token-bridge.ts, contrib/anthropic-max-bridge/*
Introduce Anthropic Max/Pro OAuth bridge plugins and token-bridge handlers: header/auth injection, system normalization, auth provider shim; add contrib installer, README, TECHNICAL doc, and helper scripts for macOS Keychain-based flow.
Token management libs
.opencode/plugins/lib/file-logger.ts, .opencode/plugins/lib/token-utils.ts, .opencode/plugins/lib/refresh-manager.ts
Add file-logger wrappers and token utilities: masked token handling, TokenStatus interface, auth.json read/write, Keychain extraction, exec helper, and refresh orchestration with concurrency/cooldown control.
Installer / provider changes
PAI-Install/cli/quick-install.ts, PAI-Install/engine/provider-models.ts, PAI-Install/engine/steps-fresh.ts, PAI-Install/anthropic-max-refresh.sh
Add anthropic-max provider preset and installer path: CLI help/validation, provider models/labels, fresh-install bridge step copying plugins and seeding ~/.local/share/opencode/auth.json from Keychain, plus a refresh helper script.
Skill tooling & index
.opencode/PAI/Tools/GenerateSkillIndex.ts, .opencode/PAI/Tools/SkillSearch.ts, .opencode/PAI/Tools/ValidateSkillStructure.ts, .opencode/skills/skill-index.json
Add skill index generator, CLI search tool, structure validator (symlink/cycle/dedupe checks), and update skill-index (reclassify many skills under Utilities, remove pai and voiceserver).
PAI docs & plugin system
.opencode/PAI/PIPELINES.md, .opencode/PAI/THEPLUGINSYSTEM.md, .opencode/PAI/DOCUMENTATIONINDEX.md, .opencode/PAISECURITYSYSTEM/ARCHITECTURE.md, .opencode/PAISECURITYSYSTEM/README.md
Add Pipelines spec, introduce Plugin System docs replacing Hook references, and update security docs to plugin model (semantics and configuration references updated).
Doc dependencies & removed PRDs
.opencode/PAI/doc-dependencies.json, docs/providers/anthropic-max.md, docs/architecture/INSTALLER-REFACTOR-PLAN.md, .prd/*
Add doc-dependencies JSON and provider docs; delete legacy PRD and installer-refactor plan documents.
Misc installers & helpers
contrib/anthropic-max-bridge/*, PAI-Install/*, contrib/anthropic-max-bridge/refresh-token.sh
Add contrib installer scripts, token refresh helpers, README/TECHNICAL docs and other installer helper scripts supporting Anthropic Max integration.
Small edits / index updates
.opencode/PAI/DOCUMENTATIONINDEX.md, .opencode/PAI/SKILL.md, .opencode/PAI/Algorithm/v3.7.0.md
Update documentation index to point to plugin system docs, adjust example/curl payloads, and update skill doc references.

Sequence Diagram(s)

sequenceDiagram
    participant Client as OpenCode Client
    participant TokenBridge as Anthropic Token Bridge
    participant RefreshMgr as Refresh Manager / Token Utils
    participant Keychain as macOS Keychain
    participant AuthFile as ~/.local/share/opencode/auth.json
    participant Anthropic as Anthropic OAuth API

    Client->>TokenBridge: experimental.chat.system.transform / chat.message
    TokenBridge->>RefreshMgr: checkAnthropicToken()
    RefreshMgr->>AuthFile: read auth.json
    AuthFile-->>RefreshMgr: return token data
    RefreshMgr->>RefreshMgr: evaluate expiresSoon / timeRemaining
    alt token missing/expired or expiresSoon
        TokenBridge->>RefreshMgr: refreshAnthropicToken()
        RefreshMgr->>RefreshMgr: isRefreshInProgress() / cooldown check
        alt allowed to refresh
            RefreshMgr->>Keychain: security find-generic-password (Claude Code)
            alt Keychain returns oauth JSON
                Keychain-->>RefreshMgr: oauth credentials
                RefreshMgr->>AuthFile: updateAnthropicTokens(access,refresh,expires)
            else Keychain fails
                RefreshMgr->>Anthropic: generate & exchange setup token
                Anthropic-->>RefreshMgr: access/refresh/expires_in
                RefreshMgr->>AuthFile: updateAnthropicTokens(...)
            end
            RefreshMgr-->>TokenBridge: refresh result (success)
        else
            RefreshMgr-->>TokenBridge: refresh skipped (in-progress/cooldown)
        end
    else token valid
        RefreshMgr-->>TokenBridge: token OK
    end
    TokenBridge-->>Client: continue handling request
Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related PRs

Poem

🐰 I hopped through scripts and docs tonight,
I swapped a voice id for a gentle "default",
I fetched tokens from Keychain until they shone bright,
I stitched plugins, installers, and docs into a vault,
Hopping on — PAI hums, organized and light.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 14.12% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title clearly summarizes the main changes: OAuth support for Anthropic Max/Pro and v3.0 structure updates, with specific PR references.

✏️ 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 release/oauth-anthropic-max-v2

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.

❤️ Share

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

Copy link

@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: 15

Note

Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.

🟡 Minor comments (6)
.opencode/PAI/doc-dependencies.json-2-2 (1)

2-2: ⚠️ Potential issue | 🟡 Minor

Invalid JSON Schema reference format.

The $schema field should reference a valid JSON Schema URI or file path (e.g., "./doc-dependencies-schema.json" or "https://example.com/schemas/doc-dependencies-schema.json"), not just a string identifier. This prevents automatic schema validation by standard JSON Schema tools.

📝 Suggested fix
-  "$schema": "doc-dependencies-schema",
+  "$schema": "./doc-dependencies-schema.json",
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.opencode/PAI/doc-dependencies.json at line 2, The $schema value is an
invalid identifier string; update the "$schema" field in
.opencode/PAI/doc-dependencies.json to point to a valid JSON Schema URI or local
path (for example change "$schema": "doc-dependencies-schema" to "$schema":
"./doc-dependencies-schema.json" or a full HTTPS schema URL) so standard JSON
Schema validators can resolve it; ensure the referenced file or URL actually
exists and matches the expected schema name.
.opencode/VoiceServer/uninstall.sh-9-14 (1)

9-14: ⚠️ Potential issue | 🟡 Minor

Remove unused RED color variable.

The RED variable is defined but never used in the script. Either remove it or use it for error messages (e.g., line 48 removal failure).

🧹 Proposed fix
 # Colors
-RED='\033[0;31m'
 GREEN='\033[0;32m'
 YELLOW='\033[1;33m'
 BLUE='\033[0;34m'
 NC='\033[0m'
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.opencode/VoiceServer/uninstall.sh around lines 9 - 14, The RED color
variable is defined but unused; either delete the RED line or apply it to error
output — update the script to use RED (e.g., wrap error messages like the
removal-failure echo) and reset with NC, or remove the RED assignment entirely
to avoid dead code; reference the RED and NC variables when making the change.
.opencode/VoiceServer/menubar/pai-voice.5s.sh-18-27 (1)

18-27: ⚠️ Potential issue | 🟡 Minor

API key extraction may truncate values containing = characters.

If the ELEVENLABS_API_KEY value contains an = character, cut -d'=' -f2 will only return the portion before the first = in the value. This is unlikely for API keys but worth a defensive fix.

🛡️ Proposed fix
     if [ -f ~/.env ] && grep -q "ELEVENLABS_API_KEY=" ~/.env 2>/dev/null; then
-        API_KEY=$(grep "ELEVENLABS_API_KEY=" ~/.env | cut -d'=' -f2)
+        API_KEY=$(grep "ELEVENLABS_API_KEY=" ~/.env | cut -d'=' -f2-)
         if [ "$API_KEY" != "your_api_key_here" ] && [ -n "$API_KEY" ]; then
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.opencode/VoiceServer/menubar/pai-voice.5s.sh around lines 18 - 27, The API
key extraction using cut -d'=' -f2 can truncate keys containing '='; update the
assignment for API_KEY to preserve the entire value (e.g., use cut -d'=' -f2- or
a sed expression that strips only the key name) and also trim surrounding
quotes/whitespace before comparing; adjust the check that uses API_KEY (the if
block that echoes "Voice: ElevenLabs AI" vs "Voice: macOS Say") to use the new
extraction so full keys are evaluated correctly.
.opencode/PAI/Tools/SkillSearch.ts-167-175 (1)

167-175: ⚠️ Potential issue | 🟡 Minor

Exit non-zero on invalid --tier input.

Line 173 reports bad CLI usage, but Line 175 returns successfully. That makes malformed invocations look successful to shell scripts and CI.

💡 Proposed fix
     if (tier === 'always' || tier === 'deferred') {
       listSkills(index, tier);
     } else {
       console.error('Invalid tier. Use: always or deferred');
+      process.exit(1);
     }
     return;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.opencode/PAI/Tools/SkillSearch.ts around lines 167 - 175, The CLI currently
logs "Invalid tier" but returns success; modify the handling around
args.indexOf('--tier') so that after validating the tier you still call
listSkills(index, tier) for 'always' or 'deferred', and when the tier is invalid
(the else branch) call process.exit(1) (or throw an error) instead of returning;
update the block that references args, tierIndex, and listSkills to exit with a
non-zero status on invalid input so CI and shell scripts see failure.
contrib/anthropic-max-bridge/TECHNICAL.md-27-39 (1)

27-39: ⚠️ Potential issue | 🟡 Minor

Specify languages on the short fenced blocks.

The two fences here are still missing a language tag, so markdownlint will keep failing with MD040. text or http would be enough.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@contrib/anthropic-max-bridge/TECHNICAL.md` around lines 27 - 39, The fenced
code blocks shown (the one containing "anthropic-beta: oauth-2025-04-20" and the
block showing the header examples with "x-api-key"/"Authorization: Bearer") lack
language tags and trigger markdownlint MD040; update both triple-fenced blocks
to include a language (e.g., text or http) so they read ```text or ```http
respectively, preserving the existing contents and formatting in TECHNICAL.md.
contrib/anthropic-max-bridge/plugins/anthropic-token-bridge.js-49-56 (1)

49-56: ⚠️ Potential issue | 🟡 Minor

writeAuthFile doesn't preserve restrictive permissions on update.

The install scripts set chmod 0600 on auth.json, but when writeAuthFile updates the file via fs.writeFileSync, it may not preserve those permissions (depending on OS/umask). Consider explicitly setting permissions after write.

🛡️ Add chmod after write
+import { chmodSync } from "node:fs";
+
 function writeAuthFile(authData) {
   try {
     fs.writeFileSync(AUTH_FILE, JSON.stringify(authData, null, 2) + "\n");
+    fs.chmodSync(AUTH_FILE, 0o600);
     return true;
   } catch (err) {
     error("Failed to write auth.json", { error: String(err) });
     return false;
   }
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@contrib/anthropic-max-bridge/plugins/anthropic-token-bridge.js` around lines
49 - 56, The writeAuthFile function currently overwrites AUTH_FILE without
guaranteeing restrictive permissions; after fs.writeFileSync in writeAuthFile,
explicitly set file permissions to 0o600 (use fs.chmodSync or async equivalent)
to preserve the install-intended mode, and if chmod fails catch and log the
error similarly to the existing catch block (referencing writeAuthFile and
AUTH_FILE) so failures are surfaced; ensure the function still returns true on
success and false on any write/chmod error.
🧹 Nitpick comments (22)
.opencode/PAI/doc-dependencies.json (3)

154-159: Clarify pattern format for regex_match check.

The patterns field contains string values like "A_", "P_", and "arbol-[apf]-", but it's unclear whether these are treated as literal prefixes or regular expressions by the validation engine. The bracket notation in "arbol-[apf]-" suggests regex syntax, but storing them as plain strings creates ambiguity. Consider:

  • Documenting whether these are regex patterns or literal prefixes
  • Using explicit regex delimiters or escaping if they are regexes
  • Or using a separate field to distinguish between literal and regex patterns
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.opencode/PAI/doc-dependencies.json around lines 154 - 159, The patterns
block is ambiguous about whether values are literal prefixes or regexes; update
the schema/JSON and consumers so pattern intent is explicit: either convert
values to explicit regex strings (e.g., add a "regex:" prefix or use a separate
boolean) or escape literal prefixes and rename keys to indicate type (e.g.,
action_prefix_literal vs worker_pattern_regex). Specifically update the
"patterns" object and keys action_prefix, pipeline_prefix, flow_prefix, and
worker_pattern to use a clear convention (regex delimiter/flag or separate
literal/regex fields) and adjust any regex_match validation code to interpret
the new convention accordingly.

135-139: Consider path brittleness for readme_mappings.

The relative paths (e.g., "../ACTIONS/README.md") will break if this configuration file or the target directories are relocated. If the directory structure is stable, this is acceptable; otherwise, consider using absolute paths from a known root or a path resolution mechanism.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.opencode/PAI/doc-dependencies.json around lines 135 - 139, The
readme_mappings entries (the readme_mappings object with keys like "ACTIONS.md",
"PIPELINES.md", "FLOWS.md") use brittle relative paths (e.g.
"../ACTIONS/README.md"); update the config to produce robust paths by resolving
them from a known project root instead of hardcoded relative segments—either
switch the values to absolute paths derived from a configurable root (env var or
repo root constant) or store only target directory names and have the loader
compute paths at runtime with a path-resolution utility (so code that reads
readme_mappings constructs final paths rather than relying on fragile "../"
strings).

6-133: Consider adding circular dependency detection.

While the current tiered structure (tier 1 → tier 2) naturally prevents circular dependencies, the configuration format doesn't enforce this constraint. As the documentation grows, consider adding a validation rule to detect circular dependencies in the upstream/dependents relationships to prevent documentation generation or validation issues.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.opencode/PAI/doc-dependencies.json around lines 6 - 133, The
authoritative_docs dependency graph can form cycles because upstream/dependents
relationships are not validated; add a validation function (e.g.,
validateDocDependencies) that builds a directed graph from authoritative_docs
using the "upstream" and "dependents" edges and runs cycle detection (Kahn's
algorithm or DFS with recursion stack) to fail validation with clear messages
naming the cycle members; wire this check into the existing doc/CI validation
routine so parsing of .opencode/PAI/doc-dependencies.json calls
validateDocDependencies and errors out if any cycle is detected, referencing the
"authoritative_docs", "upstream", and "dependents" keys in the error output.
.opencode/VoiceServer/restart.sh (1)

14-23: Consider checking exit codes from stop/start scripts.

The success message on line 23 prints unconditionally, even if stop.sh or start.sh fails. This could be misleading to users.

🛠️ Suggested improvement with error handling
 # Stop the server
-"$SCRIPT_DIR/stop.sh"
+"$SCRIPT_DIR/stop.sh" || echo -e "${YELLOW}Warning: stop.sh had issues${NC}"

 # Wait a moment
 sleep 2

 # Start the server
-"$SCRIPT_DIR/start.sh"
+"$SCRIPT_DIR/start.sh"
+if [ $? -ne 0 ]; then
+  echo -e "\033[0;31mError: Failed to start Voice Server${NC}"
+  exit 1
+fi

 echo -e "${GREEN}OK Voice server restarted${NC}"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.opencode/VoiceServer/restart.sh around lines 14 - 23, The restart script
prints a success message unconditionally; check the exit codes of
"$SCRIPT_DIR/stop.sh" and "$SCRIPT_DIR/start.sh" (use the shell's $? or
conditional execution) and abort with a non‑zero exit status if either fails;
only print "OK Voice server restarted" when both commands succeed, and log an
error message including which script failed (stop.sh or start.sh) when a failure
occurs so callers can detect and act on the error.
.opencode/VoiceServer/uninstall.sh (1)

54-59: Consider graceful termination before force-killing.

Using kill -9 (SIGKILL) immediately doesn't allow the process to clean up. Consider trying SIGTERM first with a brief wait.

♻️ Proposed improvement
 # Kill any remaining processes
 if lsof -i :8888 > /dev/null 2>&1; then
     echo -e "${YELLOW}> Cleaning up port 8888...${NC}"
-    lsof -ti :8888 | xargs kill -9 2>/dev/null
+    lsof -ti :8888 | xargs kill 2>/dev/null
+    sleep 1
+    # Force kill if still running
+    if lsof -i :8888 > /dev/null 2>&1; then
+        lsof -ti :8888 | xargs kill -9 2>/dev/null
+    fi
     echo -e "${GREEN}OK Port 8888 cleared${NC}"
 fi
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.opencode/VoiceServer/uninstall.sh around lines 54 - 59, Replace the hard
kill in uninstall.sh that pipes lsof -ti :8888 to "kill -9" with a graceful
termination sequence: first send SIGTERM to the PIDs returned by lsof -ti :8888
(use the same lsof call), wait a short interval (e.g., loop with checks or sleep
for a few seconds) to allow processes to exit, and only if any PIDs remain send
SIGKILL; keep the same logging lines around the cleanup so the messages with
${YELLOW}, ${GREEN}, and ${NC} remain consistent.
.opencode/VoiceServer/status.sh (2)

63-68: API key parsing may truncate values containing = characters.

Using cut -d'=' -f2 only extracts the first field after =. If the API key contains = characters (common in base64-encoded tokens), the value will be truncated.

🔧 More robust parsing
-    API_KEY=$(grep "ELEVENLABS_API_KEY=" "$ENV_FILE" | cut -d'=' -f2)
+    API_KEY=$(grep "ELEVENLABS_API_KEY=" "$ENV_FILE" | cut -d'=' -f2-)

The -f2- syntax extracts field 2 through the end, preserving any = in the value.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.opencode/VoiceServer/status.sh around lines 63 - 68, The API key and voice
ID extraction in status.sh uses cut -d'=' -f2 which will truncate values
containing '='; update the parsing for both API_KEY and VOICE_ID (the grep + cut
lines that set API_KEY and VOICE_ID) to use cut -d'=' -f2- so the value includes
everything after the first '=' (preserving '=' characters in base64 or similar
tokens) and keep the surrounding logic (the checks for non-empty and comparison
against placeholder) unchanged.

6-6: Unused variable PLIST_PATH.

This variable is defined but never used in the script. Either remove it or use it (e.g., to check if the plist file exists).

🧹 Suggested fix
 SERVICE_NAME="com.pai.voice-server"
-PLIST_PATH="$HOME/Library/LaunchAgents/${SERVICE_NAME}.plist"
 LOG_PATH="$HOME/Library/Logs/pai-voice-server.log"

Or alternatively, use it to check plist existence:

+# Check if plist exists
+if [ -f "$PLIST_PATH" ]; then
+    echo -e "  Plist file: ${GREEN}exists${NC}"
+else
+    echo -e "  Plist file: ${YELLOW}not found${NC}"
+fi
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.opencode/VoiceServer/status.sh at line 6, The PLIST_PATH variable (defined
as PLIST_PATH="$HOME/Library/LaunchAgents/${SERVICE_NAME}.plist") is declared
but never used; either remove the PLIST_PATH declaration or use it where plist
operations occur (e.g., replace any hardcoded plist path checks with PLIST_PATH
and add a test -f "$PLIST_PATH" to verify existence before load/unload/install
steps). Update references that currently build the path from SERVICE_NAME to use
PLIST_PATH so the variable is actually used consistently.
.opencode/VoiceServer/install.sh (2)

55-55: Same API key parsing limitation as in status.sh.

Consider using cut -d'=' -f2- to handle API keys containing = characters.

🔧 Suggested fix
-    API_KEY=$(grep "ELEVENLABS_API_KEY=" "$ENV_FILE" | cut -d'=' -f2)
+    API_KEY=$(grep "ELEVENLABS_API_KEY=" "$ENV_FILE" | cut -d'=' -f2-)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.opencode/VoiceServer/install.sh at line 55, The API_KEY extraction in
install.sh currently uses cut -d'=' -f2 which breaks when the key contains '=';
update the assignment for the API_KEY variable (the line that uses grep
"ELEVENLABS_API_KEY=" and cut) to use cut -d'=' -f2- so it preserves any '='
characters in the value (or alternatively use shell parameter expansion after
reading the line) so API keys containing '=' are parsed correctly.

136-155: Health check could use retry logic instead of fixed sleep.

The fixed sleep 2 may be insufficient if the server takes longer to start (e.g., first-time Bun compilation). A retry loop would be more robust.

♻️ Suggested retry loop
-# Wait for server to start
-sleep 2
-
-# Test the server
-echo -e "${YELLOW}> Testing voice server...${NC}"
-if curl -s -f -X GET http://localhost:8888/health > /dev/null 2>&1; then
+# Wait for server to start with retry
+echo -e "${YELLOW}> Waiting for voice server to start...${NC}"
+MAX_RETRIES=10
+RETRY_COUNT=0
+while ! curl -s -f -X GET http://localhost:8888/health > /dev/null 2>&1; do
+    RETRY_COUNT=$((RETRY_COUNT + 1))
+    if [ $RETRY_COUNT -ge $MAX_RETRIES ]; then
+        echo -e "${RED}X Voice server did not start after ${MAX_RETRIES} attempts${NC}"
+        echo "  Check logs at: $LOG_PATH"
+        echo "  Try running manually: bun run $SCRIPT_DIR/server.ts"
+        exit 1
+    fi
+    sleep 1
+done
+
+echo -e "${GREEN}OK Voice server is running${NC}"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.opencode/VoiceServer/install.sh around lines 136 - 155, Replace the fixed
sleep with a retry loop that polls the health endpoint (GET
http://localhost:8888/health) using curl -s -f, retrying up to a MAX_RETRIES
(e.g., 10) with a short sleep/backoff between attempts (SLEEP_SEC), and only
proceed to the POST notify (http://localhost:8888/notify) after a successful
health check; if retries are exhausted, keep the existing error branch that
prints LOG_PATH and the suggestion to run bun run $SCRIPT_DIR/server.ts and exit
1. Ensure you update the block around the health check and the code that calls
server.ts so function/filename references (server.ts, LOG_PATH, SCRIPT_DIR)
remain correct.
.opencode/plugins/anthropic-token-bridge.ts (1)

8-10: Keep the init hook free of synchronous disk I/O.

fileLog() ultimately uses synchronous file writes, and this factory runs during plugin initialization. Defer that trace or make it best-effort outside the init path. As per coding guidelines, "Avoid blocking operations in lifecycle plugin init hooks."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.opencode/plugins/anthropic-token-bridge.ts around lines 8 - 10, The
synchronous fileLog call in the AnthropicTokenBridge factory blocks plugin init;
remove the direct call to fileLog() from inside the AnthropicTokenBridge()
function and defer the trace (e.g., call fileLog from an async lifecycle method
or schedule it off the init path using setImmediate/process.nextTick or a
non-blocking async logger). Locate the call to fileLog("AnthropicTokenBridge
plugin loaded", "info") inside the AnthropicTokenBridge function and either move
it into the plugin's async init/start method or replace it with a non-blocking
alternative so the factory remains free of synchronous disk I/O.
PAI-Install/cli/quick-install.ts (1)

89-89: Avoid CLI examples that embed live API keys.

These snippets put secrets into shell history and process listings. Prefer an environment-variable example or a prompt-based flow instead. As per coding guidelines, "Use environment variables for sensitive configuration (e.g., process.env.API_KEY) instead of hardcoding values."

Also applies to: 102-102, 105-107

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@PAI-Install/cli/quick-install.ts` at line 89, Replace the CLI examples that
show a literal --api-key <key> with guidance and code paths that use an
environment variable or prompt instead: update the help/usage text in
quick-install.ts (the --api-key flag handling and displayed examples) to
instruct users to set an env var (e.g., read from process.env.API_KEY) or to
invoke an interactive prompt when the flag is omitted; remove any inline example
values in the usage output and change example lines at the locations referenced
(around --api-key and the examples at lines ~102 and ~105-107) so they
demonstrate exporting an environment variable or running the CLI without
embedding secrets.
docs/providers/anthropic-max.md (1)

98-112: Add the overview diagrams required for docs/**.

This section explains a multi-step auth flow but only in prose. Please add an ASCII quick overview and a Mermaid diagram in a collapsible section around the token/refresh path. As per coding guidelines, "ASCII diagrams for quick overview" and "Mermaid diagrams in collapsible sections for detail."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/providers/anthropic-max.md` around lines 98 - 112, Update the "How it
works (technical)" section (the header and table shown) by adding an ASCII quick
overview and a collapsible Mermaid diagram that illustrate the token/refresh
path described as "macOS Keychain → auth.json → plugin → Anthropic API"; place
the ASCII diagram immediately after the "Token path" line for quick reference
and add a collapsible section (details/summary or equivalent used in docs)
containing a Mermaid flowchart that shows each step (Keychain, auth.json,
plugin, request to Anthropic API and any refresh step) and label arrows with
token/refresh actions; ensure the diagrams follow the project's docs guidelines
(ASCII for quick overview, Mermaid in collapsible for detail) and keep wording
consistent with the existing "How it works (technical)" section.
contrib/anthropic-max-bridge/README.md (2)

105-114: Add language specifier to fenced code block.

The file structure diagram should have a language specifier for better rendering and linting compliance.

📝 Add language identifier
-```
+```text
 contrib/anthropic-max-bridge/
 ├── README.md              ← You are here
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@contrib/anthropic-max-bridge/README.md` around lines 105 - 114, The fenced
code block showing the file tree in contrib/anthropic-max-bridge/README.md needs
a language specifier for linting and rendering; change the opening fence from
``` to ```text (or another appropriate identifier) so the block containing the
directory diagram is annotated, ensuring the README's file structure diagram is
properly highlighted and compliant.

120-136: Add language specifier to flow diagram code block.

📝 Add language identifier
-```
+```text
 Claude Code CLI
     └─ authenticates with Anthropic
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@contrib/anthropic-max-bridge/README.md` around lines 120 - 136, The fenced
flow-diagram code block lacks a language specifier; update the triple-backtick
opening fence for the diagram (the block containing "Claude Code CLI ...
Anthropic API accepts → response streams back") to include a language
identifier—use ```text—so the block becomes ```text and ensures correct
rendering and syntax highlighting where applicable.
.opencode/plugins/anthropic-max-bridge.js (1)

116-137: Inconsistent indentation in authorize function.

The authorize async function body has incorrect indentation relative to the object structure.

🔧 Fix indentation
 			{
 				// Primary method — token comes from auth.json (see README)
 				label: "Claude Pro/Max (OAuth)",
 				type: "oauth",
-		authorize: async () => {
-				// INTENTIONAL: This OAuth authorize flow is deliberately disabled.
-				// Tokens are NOT obtained through a browser redirect here.
-				// Instead, they are extracted from the macOS Keychain (where
-				// Claude Code CLI stores them) and written to auth.json by the
-				// PAI-OpenCode installer or the refresh script.
-				// The plugin only reads from auth.json — it never triggers its own
-				// OAuth flow.
-				return { type: "failed" };
-			},
+				authorize: async () => {
+					// INTENTIONAL: This OAuth authorize flow is deliberately disabled.
+					// Tokens are NOT obtained through a browser redirect here.
+					// Instead, they are extracted from the macOS Keychain (where
+					// Claude Code CLI stores them) and written to auth.json by the
+					// PAI-OpenCode installer or the refresh script.
+					// The plugin only reads from auth.json — it never triggers its own
+					// OAuth flow.
+					return { type: "failed" };
+				},
 			},
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.opencode/plugins/anthropic-max-bridge.js around lines 116 - 137, The
authorize async function inside the methods array has misaligned indentation;
open the object with label "Claude Pro/Max (OAuth)" and re-indent the authorize:
async () { ... } block so its body and closing brace align with the surrounding
object properties (methods array entry) and ensure the trailing comma/brace
structure remains correct; specifically fix the authorize function inside the
plugin methods entry so its async function, return { type: "failed" }, and
closing braces line up with the surrounding object (the methods array item for
the OAuth method).
contrib/anthropic-max-bridge/plugins/anthropic-max-bridge.js (1)

105-127: Same indentation issue as the .opencode version.

The authorize function body (lines 110-119) has inconsistent indentation.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@contrib/anthropic-max-bridge/plugins/anthropic-max-bridge.js` around lines
105 - 127, Fix the inconsistent indentation in the authorize function inside the
methods array for the "Claude Pro/Max (OAuth)" entry: re-indent the authorize:
async () { ... } block so its body (comments and return { type: "failed" };) and
the closing brace line up with the other object properties (label/type) and
match surrounding object formatting; ensure the inner comments and return
statement are uniformly indented and that the closing braces for the authorize
function and the method object align correctly with the methods array entries.
contrib/anthropic-max-bridge/install.sh (1)

195-204: Same shell variable expansion pattern as refresh-token.sh.

Line 202 embeds $EXPIRES_AT and $NOW_MS directly in Python code. While these are validated earlier, consider using environment variables for consistency with the rest of the script.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@contrib/anthropic-max-bridge/install.sh` around lines 195 - 204, The Python
one-liners embed shell variables ($EXPIRES_AT, $NOW_MS) directly; change them to
read these values from environment variables instead for consistency with
refresh-token.sh: export or pass EXPIRES_AT and NOW_MS into the environment and
update the two python3 -c invocations (the NOW_MS assignment and the HOURS_LEFT
calculation) to read os.environ values (convert to int/float as needed) rather
than interpolating shell variables into the Python code; ensure the variables
NOW_MS, EXPIRES_AT and the HOURS_LEFT assignment logic continue to work and
preserve the same validations and messages.
contrib/anthropic-max-bridge/plugins/anthropic-token-bridge.js (1)

1-4: Comment at line 1 appears to be a build artifact.

The comment // lib/file-logger.ts suggests this file was bundled from TypeScript sources. This is fine for distribution, but consider adding a header comment indicating this is a compiled bundle.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@contrib/anthropic-max-bridge/plugins/anthropic-token-bridge.js` around lines
1 - 4, The top-of-file comment looks like an accidental build artifact; add a
clear header comment at the very top of anthropic-token-bridge.js indicating
this is a compiled/transpiled bundle (orig: lib/file-logger.ts), include a brief
note that it is generated code, optional build/timestamp and a "do not edit"
warning so maintainers know not to modify this file directly; update the header
near the existing imports/LOG_PATH/appendFileSync symbols to make the provenance
explicit.
contrib/anthropic-max-bridge/refresh-token.sh (1)

108-109: Shell variable expansion in Python command could be safer.

While EXPIRES_AT and NOW_MS are validated/generated earlier, embedding shell variables directly in Python code is fragile. If validation logic changes, this could become an injection vector.

🛡️ Use environment variables consistently
-NOW_MS=$(python3 -c "import time; print(int(time.time() * 1000))")
-HOURS_LEFT=$(python3 -c "print(round(($EXPIRES_AT - $NOW_MS) / 3600000, 1))")
+NOW_MS=$(python3 -c "import time; print(int(time.time() * 1000))")
+export PAI_EXPIRES_AT="$EXPIRES_AT"
+export PAI_NOW_MS="$NOW_MS"
+HOURS_LEFT=$(python3 -c "import os; print(round((int(os.environ['PAI_EXPIRES_AT']) - int(os.environ['PAI_NOW_MS'])) / 3600000, 1))")
+unset PAI_EXPIRES_AT PAI_NOW_MS
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@contrib/anthropic-max-bridge/refresh-token.sh` around lines 108 - 109, The
two lines using python -c with embedded shell vars (NOW_MS, EXPIRES_AT ->
HOURS_LEFT) are fragile and can open an injection vector; instead pass values
via environment or avoid embedding by invoking Python without shell expansion
(e.g., read EXPIRES_AT/NOW_MS from os.environ or from stdin) or compute
HOURS_LEFT purely in shell arithmetic; update the commands that set NOW_MS,
HOURS_LEFT to read EXPIRES_AT/NOW_MS from environment (os.environ['EXPIRES_AT']
/ os.environ['NOW_MS']) or perform the subtraction/division with shell
arithmetic to ensure no direct shell variable interpolation into a Python
string.
PAI-Install/engine/provider-models.ts (1)

22-35: Clarify model version difference between anthropic and anthropic-max providers.

The anthropic provider uses claude-sonnet-4-5 (standard tier), while anthropic-max uses claude-sonnet-4-6. Both model versions are available as of March 2026—4-6 is the latest recommended release, while 4-5 is legacy but still supported.

If this difference is intentional (e.g., anthropic-max's OAuth tier has access to newer models), add an explanatory comment. Otherwise, verify consistency between the two providers.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@PAI-Install/engine/provider-models.ts` around lines 22 - 35, The
PROVIDER_MODELS mapping shows a deliberate mismatch: 'anthropic' maps standard
-> "anthropic/claude-sonnet-4-5" while 'anthropic-max' maps standard ->
"anthropic/claude-sonnet-4-6"; update the file to either make the versions
consistent or document the reason for the difference. If the difference is
intentional (e.g., anthropic-max OAuth has access to 4-6), add a concise comment
above the PROVIDER_MODELS entry referencing the keys 'anthropic' and
'anthropic-max' and explaining why 'claude-sonnet-4-6' is used for
anthropic-max; otherwise, change the 'standard' value for one provider so both
use the same model string to maintain consistency.
.opencode/plugins/anthropic-token-bridge.js (2)

40-47: Consider async file operations for consistency (optional).

readAuthFile and writeAuthFile use synchronous fs.readFileSync/fs.writeFileSync. While acceptable for small config files and not in the init hook, using async variants (fs.promises.readFile/fs.promises.writeFile) would align better with the async plugin patterns used elsewhere in this file.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.opencode/plugins/anthropic-token-bridge.js around lines 40 - 47, Convert
readAuthFile and writeAuthFile to async functions that use fs.promises.readFile
and fs.promises.writeFile (or the promise-based fs methods) instead of
readFileSync/writeFileSync; update their signatures (e.g., async function
readAuthFile() / async function writeAuthFile(auth)) to return/await parsed JSON
or resolved void, handle errors with the same error(...) call (including the
caught error) and ensure any callers of readAuthFile/writeAuthFile (in this
file) are awaited or updated to handle Promises so the async flow remains
consistent with other plugin functions.

287-294: Hardcoded 8-hour expiry assumption for Keychain-extracted tokens.

When extracting tokens from Keychain, the code assumes an 8-hour expiry (28800 seconds) rather than reading the actual expiration from the credential data. If the Keychain credential includes expiry information (e.g., oauth.expiresAt or similar), consider extracting it to avoid premature or missed refreshes.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.opencode/plugins/anthropic-token-bridge.js around lines 287 - 294, The code
in extractFromKeychain usage hardcodes an 8-hour TTL (28800) when calling
updateAnthropicTokens; change this to compute the real TTL from the credential
returned by extractFromKeychain (e.g., look for fields like
keychainTokens.expiresAt, keychainTokens.oauth?.expiresAt, or
keychainTokens.expiry) and convert that timestamp to seconds remaining
(Math.max(0, Math.floor((expiresAt - Date.now())/1000))). Pass that computed TTL
to updateAnthropicTokens(accessToken, refreshToken, ttl) and fall back to a
sensible default only if no expiry is present or the computed TTL is
non-positive. Ensure you reference extractFromKeychain and updateAnthropicTokens
when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.opencode/PAI/doc-dependencies.json:
- Around line 122-125: The doc-dependencies entry for "FEEDSYSTEM.md" in
.opencode/PAI/doc-dependencies.json points to a non-existent file; either add a
new FEEDSYSTEM.md with the documented content to the repo or remove the
"FEEDSYSTEM.md" object from doc-dependencies.json (the JSON key "FEEDSYSTEM.md")
so the configuration no longer references a missing file—update whichever you
choose and commit the change.

In @.opencode/PAI/Tools/GenerateSkillIndex.ts:
- Around line 104-106: The parseFrontmatter function currently only matches LF
delimiters; update its regex in parseFrontmatter to accept CRLF too (use \r?\n
in the frontmatter boundary pattern) so it recognizes frontmatter on CRLF
checkouts the same way as ValidateSkillStructure; ensure the same regex is used
for both opening and closing --- delimiters in parseFrontmatter so valid skills
aren't skipped.
- Around line 288-300: The current loop in GenerateSkillIndex.ts silently warns
and skips when a duplicate skill name is found (skillFiles, parseSkillFile,
index.skills), which must instead fail generation; change the duplicate handling
inside the for (const filePath of skillFiles) loop so that when
index.skills[key] already exists you throw an exception or exit with a non-zero
status (including a clear error that contains both the duplicate skill.name and
both file paths: skill.path and index.skills[key].path) rather than calling
console.warn and continue, ensuring generation fails fast on duplicate names.

In @.opencode/PAI/Tools/SkillSearch.ts:
- Around line 47-58: The match against skill names in searchSkills is using the
raw key/skill.name so queries with spaces (e.g., "private investigator") won't
match camelCase or concatenated names; normalize both the query and the skill
identifiers before the includes check by creating a normalized skill name (apply
camelCase split like replace(/([a-z])([A-Z])/g, '$1 $2'), replace
non-alphanumerics with spaces, collapse whitespace, and toLowerCase()) for both
`key` and `skill.name`, then compare query.toLowerCase() against those
normalized forms in the existing name-match branch so spaced queries match
`PrivateInvestigator`/`OpenCodeSystem`.

In @.opencode/PAI/Tools/ValidateSkillStructure.ts:
- Around line 103-121: The validator currently skips running validateSkill for
category-root SKILL.md when hasChildSkills is true (see variables/categories,
flatSkills and call to validateSkill), causing category SKILL.md to be
unvalidated while GenerateSkillIndex.ts still indexes them; update this block to
also call validateSkill for category SKILL.md (or otherwise increment/record
them) even when hasChildSkills is true so category metadata files are validated
by validateSkill, or change GenerateSkillIndex.ts to stop indexing category
SKILL.md — ensure validateSkill(relativePath, skillMdPath, issues, skillNames)
is invoked for those entries so the validator and GenerateSkillIndex use the
same model.

In @.opencode/plugins/anthropic-token-bridge.js:
- Around line 49-57: writeAuthFile currently writes AUTH_FILE without
restrictive permissions; update it to ensure the created auth.json is only
user-readable/writable (mode 0o600). After writing (or by opening with the
appropriate mode), set file permissions to 0o600 (e.g., via fs.chmodSync or
using fs.openSync with mode) and handle any chmod errors similarly to existing
error handling so the function still returns false on failure; reference
writeAuthFile and AUTH_FILE when making this change.

In @.opencode/plugins/anthropic-token-bridge.ts:
- Around line 24-27: When status.exists is false the handlers currently bail
out; instead call the recovery path to repopulate auth.json from the keychain by
invoking refreshAnthropicToken() (and log the action with fileLog) rather than
returning. Update both places that check status.exists (the one referencing
fileLog("No Anthropic OAuth token configured", "debug") and the similar check
around lines 67-70) to attempt refreshAnthropicToken() and only return if that
refresh fails; keep existing logging but add a log entry before/after calling
refreshAnthropicToken() to indicate recovery was attempted.

In @.opencode/plugins/lib/refresh-manager.ts:
- Around line 57-60: The token restore routine currently discards the real
Keychain expiry; update refreshAnthropicToken (and any code paths at the other
occurrences around extractFromKeychain) so that when restoring tokens from
KeychainTokens you read and persist the actual claudeAiOauth.expiresAt value
instead of hard-coding an 8-hour TTL; specifically, propagate the expiresAt
timestamp parsed in extractFromKeychain() into refreshAnthropicToken() (or
accept an optional expiresAt parameter) and use that value when saving
tokens/backing to Keychain and when computing next-refresh scheduling (also fix
the similar logic at the other occurrences referenced in the comment so they all
use the stored expiresAt).
- Around line 141-180: The exchangeSetupToken function lacks a fetch timeout;
update exchangeSetupToken to use an AbortController (same pattern as
execCommand) by creating a controller, passing controller.signal to fetch, and
scheduling a setTimeout to call controller.abort() after ~10_000 ms; ensure you
clear the timeout on success/failure and let the existing catch path handle
abort errors so refreshAnthropicToken's finally runs and isRefreshing is
cleared. Reference: exchangeSetupToken (and mirror the timeout/abort pattern
used in execCommand) to add the controller, timeout handle, and signal wiring
for fetch.

In @.opencode/plugins/lib/token-utils.ts:
- Around line 65-120: checkAnthropicToken currently only checks anthropic.type
and then assumes anthropic.access and anthropic.expires are present/valid;
validate both before reporting a healthy OAuth token. Add explicit checks for
anthropic.access (non-empty string) and anthropic.expires (finite number / valid
timestamp) inside the same function (where anthropic.access, anthropic.expires,
REFRESH_THRESHOLD_MS, maskToken are used); if access is missing return valid:
false with reason like "missing_access" and maskedAccess null/empty, and if
expires is invalid treat as expired/invalid (log a warning without calling new
Date(expires).toISOString to avoid throwing) and return valid: false with reason
like "invalid_expires" or "token_expired" so the refresh path can recover.
Ensure all log calls guard against invalid expires by formatting dates only
after validation.

In `@contrib/anthropic-max-bridge/plugins/anthropic-max-bridge.js`:
- Around line 45-51: The transform experimental.chat.system.transform assumes
output.system is an array and calls .map(), which can throw if output.system is
undefined, null, or a string; update the function to defensively normalize
output.system first (e.g., if it's a string, wrap into an array; if falsy, set
to an empty array) and then map over it to convert string items into {type:
"text", text: s} objects so the transform safely handles non-array values.

In `@PAI-Install/anthropic-max-refresh.sh`:
- Around line 40-43: The script currently only checks that EXPIRES_AT is numeric
and positive but does not verify whether the token is already expired; before
rewriting auth.json you must compare EXPIRES_AT against the current epoch time
and abort if EXPIRES_AT <= now. Update both validation sites that reference
EXPIRES_AT to: ensure it matches ^[0-9]+$, then get the current timestamp (e.g.,
via date +%s), and call die with a clear message if EXPIRES_AT is less than or
equal to the current time so the script stops instead of writing a stale token
to auth.json.
- Around line 53-65: The auth file is rewritten by the Python heredoc but never
restores strict permissions; after updating data["anthropic"] using the
auth_file variable in the python3 <<'PYEOF' block, re-apply mode 0600 to that
file (e.g., call os.chmod(auth_file, 0o600) inside the same Python block or
invoke chmod 0600 "$PAI_AUTH_FILE" immediately after the heredoc) so the
rewritten auth.json keeps the same restrictive permission invariant.

In `@PAI-Install/cli/quick-install.ts`:
- Around line 186-190: The current branch that handles provider ===
"anthropic-max" only prints advisory text but must fail-fast on non-macOS;
modify the block that checks provider === "anthropic-max" (in quick-install.ts)
to assert process.platform === "darwin" and otherwise log a clear error and exit
(or throw) so Linux/Windows users cannot continue the install path that expects
macOS Keychain extraction; keep the existing informational console messages for
macOS.

In `@PAI-Install/engine/steps-fresh.ts`:
- Around line 223-241: The installer currently only copies
anthropic-max-bridge.js; update the fresh-install flow (the block using
repoRoot, pluginSrc, pluginDst, pluginsDir and onProgress) to also copy
anthropic-token-bridge.js: construct a second pluginSrc/tokenPluginSrc pointing
to ".opencode/plugins/anthropic-token-bridge.js" and tokenPluginDst in
pluginsDir, check existsSync for the token plugin (fail with a clear message
similar to the existing one), mkdirSync(pluginsDir, { recursive: true }) if
needed, and call copyFileSync for the token plugin just like for
anthropic-max-bridge.js so token auto-refresh support is installed.

---

Minor comments:
In @.opencode/PAI/doc-dependencies.json:
- Line 2: The $schema value is an invalid identifier string; update the
"$schema" field in .opencode/PAI/doc-dependencies.json to point to a valid JSON
Schema URI or local path (for example change "$schema":
"doc-dependencies-schema" to "$schema": "./doc-dependencies-schema.json" or a
full HTTPS schema URL) so standard JSON Schema validators can resolve it; ensure
the referenced file or URL actually exists and matches the expected schema name.

In @.opencode/PAI/Tools/SkillSearch.ts:
- Around line 167-175: The CLI currently logs "Invalid tier" but returns
success; modify the handling around args.indexOf('--tier') so that after
validating the tier you still call listSkills(index, tier) for 'always' or
'deferred', and when the tier is invalid (the else branch) call process.exit(1)
(or throw an error) instead of returning; update the block that references args,
tierIndex, and listSkills to exit with a non-zero status on invalid input so CI
and shell scripts see failure.

In @.opencode/VoiceServer/menubar/pai-voice.5s.sh:
- Around line 18-27: The API key extraction using cut -d'=' -f2 can truncate
keys containing '='; update the assignment for API_KEY to preserve the entire
value (e.g., use cut -d'=' -f2- or a sed expression that strips only the key
name) and also trim surrounding quotes/whitespace before comparing; adjust the
check that uses API_KEY (the if block that echoes "Voice: ElevenLabs AI" vs
"Voice: macOS Say") to use the new extraction so full keys are evaluated
correctly.

In @.opencode/VoiceServer/uninstall.sh:
- Around line 9-14: The RED color variable is defined but unused; either delete
the RED line or apply it to error output — update the script to use RED (e.g.,
wrap error messages like the removal-failure echo) and reset with NC, or remove
the RED assignment entirely to avoid dead code; reference the RED and NC
variables when making the change.

In `@contrib/anthropic-max-bridge/plugins/anthropic-token-bridge.js`:
- Around line 49-56: The writeAuthFile function currently overwrites AUTH_FILE
without guaranteeing restrictive permissions; after fs.writeFileSync in
writeAuthFile, explicitly set file permissions to 0o600 (use fs.chmodSync or
async equivalent) to preserve the install-intended mode, and if chmod fails
catch and log the error similarly to the existing catch block (referencing
writeAuthFile and AUTH_FILE) so failures are surfaced; ensure the function still
returns true on success and false on any write/chmod error.

In `@contrib/anthropic-max-bridge/TECHNICAL.md`:
- Around line 27-39: The fenced code blocks shown (the one containing
"anthropic-beta: oauth-2025-04-20" and the block showing the header examples
with "x-api-key"/"Authorization: Bearer") lack language tags and trigger
markdownlint MD040; update both triple-fenced blocks to include a language
(e.g., text or http) so they read ```text or ```http respectively, preserving
the existing contents and formatting in TECHNICAL.md.

---

Nitpick comments:
In @.opencode/PAI/doc-dependencies.json:
- Around line 154-159: The patterns block is ambiguous about whether values are
literal prefixes or regexes; update the schema/JSON and consumers so pattern
intent is explicit: either convert values to explicit regex strings (e.g., add a
"regex:" prefix or use a separate boolean) or escape literal prefixes and rename
keys to indicate type (e.g., action_prefix_literal vs worker_pattern_regex).
Specifically update the "patterns" object and keys action_prefix,
pipeline_prefix, flow_prefix, and worker_pattern to use a clear convention
(regex delimiter/flag or separate literal/regex fields) and adjust any
regex_match validation code to interpret the new convention accordingly.
- Around line 135-139: The readme_mappings entries (the readme_mappings object
with keys like "ACTIONS.md", "PIPELINES.md", "FLOWS.md") use brittle relative
paths (e.g. "../ACTIONS/README.md"); update the config to produce robust paths
by resolving them from a known project root instead of hardcoded relative
segments—either switch the values to absolute paths derived from a configurable
root (env var or repo root constant) or store only target directory names and
have the loader compute paths at runtime with a path-resolution utility (so code
that reads readme_mappings constructs final paths rather than relying on fragile
"../" strings).
- Around line 6-133: The authoritative_docs dependency graph can form cycles
because upstream/dependents relationships are not validated; add a validation
function (e.g., validateDocDependencies) that builds a directed graph from
authoritative_docs using the "upstream" and "dependents" edges and runs cycle
detection (Kahn's algorithm or DFS with recursion stack) to fail validation with
clear messages naming the cycle members; wire this check into the existing
doc/CI validation routine so parsing of .opencode/PAI/doc-dependencies.json
calls validateDocDependencies and errors out if any cycle is detected,
referencing the "authoritative_docs", "upstream", and "dependents" keys in the
error output.

In @.opencode/plugins/anthropic-max-bridge.js:
- Around line 116-137: The authorize async function inside the methods array has
misaligned indentation; open the object with label "Claude Pro/Max (OAuth)" and
re-indent the authorize: async () { ... } block so its body and closing brace
align with the surrounding object properties (methods array entry) and ensure
the trailing comma/brace structure remains correct; specifically fix the
authorize function inside the plugin methods entry so its async function, return
{ type: "failed" }, and closing braces line up with the surrounding object (the
methods array item for the OAuth method).

In @.opencode/plugins/anthropic-token-bridge.js:
- Around line 40-47: Convert readAuthFile and writeAuthFile to async functions
that use fs.promises.readFile and fs.promises.writeFile (or the promise-based fs
methods) instead of readFileSync/writeFileSync; update their signatures (e.g.,
async function readAuthFile() / async function writeAuthFile(auth)) to
return/await parsed JSON or resolved void, handle errors with the same
error(...) call (including the caught error) and ensure any callers of
readAuthFile/writeAuthFile (in this file) are awaited or updated to handle
Promises so the async flow remains consistent with other plugin functions.
- Around line 287-294: The code in extractFromKeychain usage hardcodes an 8-hour
TTL (28800) when calling updateAnthropicTokens; change this to compute the real
TTL from the credential returned by extractFromKeychain (e.g., look for fields
like keychainTokens.expiresAt, keychainTokens.oauth?.expiresAt, or
keychainTokens.expiry) and convert that timestamp to seconds remaining
(Math.max(0, Math.floor((expiresAt - Date.now())/1000))). Pass that computed TTL
to updateAnthropicTokens(accessToken, refreshToken, ttl) and fall back to a
sensible default only if no expiry is present or the computed TTL is
non-positive. Ensure you reference extractFromKeychain and updateAnthropicTokens
when making the change.

In @.opencode/plugins/anthropic-token-bridge.ts:
- Around line 8-10: The synchronous fileLog call in the AnthropicTokenBridge
factory blocks plugin init; remove the direct call to fileLog() from inside the
AnthropicTokenBridge() function and defer the trace (e.g., call fileLog from an
async lifecycle method or schedule it off the init path using
setImmediate/process.nextTick or a non-blocking async logger). Locate the call
to fileLog("AnthropicTokenBridge plugin loaded", "info") inside the
AnthropicTokenBridge function and either move it into the plugin's async
init/start method or replace it with a non-blocking alternative so the factory
remains free of synchronous disk I/O.

In @.opencode/VoiceServer/install.sh:
- Line 55: The API_KEY extraction in install.sh currently uses cut -d'=' -f2
which breaks when the key contains '='; update the assignment for the API_KEY
variable (the line that uses grep "ELEVENLABS_API_KEY=" and cut) to use cut
-d'=' -f2- so it preserves any '=' characters in the value (or alternatively use
shell parameter expansion after reading the line) so API keys containing '=' are
parsed correctly.
- Around line 136-155: Replace the fixed sleep with a retry loop that polls the
health endpoint (GET http://localhost:8888/health) using curl -s -f, retrying up
to a MAX_RETRIES (e.g., 10) with a short sleep/backoff between attempts
(SLEEP_SEC), and only proceed to the POST notify (http://localhost:8888/notify)
after a successful health check; if retries are exhausted, keep the existing
error branch that prints LOG_PATH and the suggestion to run bun run
$SCRIPT_DIR/server.ts and exit 1. Ensure you update the block around the health
check and the code that calls server.ts so function/filename references
(server.ts, LOG_PATH, SCRIPT_DIR) remain correct.

In @.opencode/VoiceServer/restart.sh:
- Around line 14-23: The restart script prints a success message
unconditionally; check the exit codes of "$SCRIPT_DIR/stop.sh" and
"$SCRIPT_DIR/start.sh" (use the shell's $? or conditional execution) and abort
with a non‑zero exit status if either fails; only print "OK Voice server
restarted" when both commands succeed, and log an error message including which
script failed (stop.sh or start.sh) when a failure occurs so callers can detect
and act on the error.

In @.opencode/VoiceServer/status.sh:
- Around line 63-68: The API key and voice ID extraction in status.sh uses cut
-d'=' -f2 which will truncate values containing '='; update the parsing for both
API_KEY and VOICE_ID (the grep + cut lines that set API_KEY and VOICE_ID) to use
cut -d'=' -f2- so the value includes everything after the first '=' (preserving
'=' characters in base64 or similar tokens) and keep the surrounding logic (the
checks for non-empty and comparison against placeholder) unchanged.
- Line 6: The PLIST_PATH variable (defined as
PLIST_PATH="$HOME/Library/LaunchAgents/${SERVICE_NAME}.plist") is declared but
never used; either remove the PLIST_PATH declaration or use it where plist
operations occur (e.g., replace any hardcoded plist path checks with PLIST_PATH
and add a test -f "$PLIST_PATH" to verify existence before load/unload/install
steps). Update references that currently build the path from SERVICE_NAME to use
PLIST_PATH so the variable is actually used consistently.

In @.opencode/VoiceServer/uninstall.sh:
- Around line 54-59: Replace the hard kill in uninstall.sh that pipes lsof -ti
:8888 to "kill -9" with a graceful termination sequence: first send SIGTERM to
the PIDs returned by lsof -ti :8888 (use the same lsof call), wait a short
interval (e.g., loop with checks or sleep for a few seconds) to allow processes
to exit, and only if any PIDs remain send SIGKILL; keep the same logging lines
around the cleanup so the messages with ${YELLOW}, ${GREEN}, and ${NC} remain
consistent.

In `@contrib/anthropic-max-bridge/install.sh`:
- Around line 195-204: The Python one-liners embed shell variables ($EXPIRES_AT,
$NOW_MS) directly; change them to read these values from environment variables
instead for consistency with refresh-token.sh: export or pass EXPIRES_AT and
NOW_MS into the environment and update the two python3 -c invocations (the
NOW_MS assignment and the HOURS_LEFT calculation) to read os.environ values
(convert to int/float as needed) rather than interpolating shell variables into
the Python code; ensure the variables NOW_MS, EXPIRES_AT and the HOURS_LEFT
assignment logic continue to work and preserve the same validations and
messages.

In `@contrib/anthropic-max-bridge/plugins/anthropic-max-bridge.js`:
- Around line 105-127: Fix the inconsistent indentation in the authorize
function inside the methods array for the "Claude Pro/Max (OAuth)" entry:
re-indent the authorize: async () { ... } block so its body (comments and return
{ type: "failed" };) and the closing brace line up with the other object
properties (label/type) and match surrounding object formatting; ensure the
inner comments and return statement are uniformly indented and that the closing
braces for the authorize function and the method object align correctly with the
methods array entries.

In `@contrib/anthropic-max-bridge/plugins/anthropic-token-bridge.js`:
- Around line 1-4: The top-of-file comment looks like an accidental build
artifact; add a clear header comment at the very top of
anthropic-token-bridge.js indicating this is a compiled/transpiled bundle (orig:
lib/file-logger.ts), include a brief note that it is generated code, optional
build/timestamp and a "do not edit" warning so maintainers know not to modify
this file directly; update the header near the existing
imports/LOG_PATH/appendFileSync symbols to make the provenance explicit.

In `@contrib/anthropic-max-bridge/README.md`:
- Around line 105-114: The fenced code block showing the file tree in
contrib/anthropic-max-bridge/README.md needs a language specifier for linting
and rendering; change the opening fence from ``` to ```text (or another
appropriate identifier) so the block containing the directory diagram is
annotated, ensuring the README's file structure diagram is properly highlighted
and compliant.
- Around line 120-136: The fenced flow-diagram code block lacks a language
specifier; update the triple-backtick opening fence for the diagram (the block
containing "Claude Code CLI ... Anthropic API accepts → response streams back")
to include a language identifier—use ```text—so the block becomes ```text and
ensures correct rendering and syntax highlighting where applicable.

In `@contrib/anthropic-max-bridge/refresh-token.sh`:
- Around line 108-109: The two lines using python -c with embedded shell vars
(NOW_MS, EXPIRES_AT -> HOURS_LEFT) are fragile and can open an injection vector;
instead pass values via environment or avoid embedding by invoking Python
without shell expansion (e.g., read EXPIRES_AT/NOW_MS from os.environ or from
stdin) or compute HOURS_LEFT purely in shell arithmetic; update the commands
that set NOW_MS, HOURS_LEFT to read EXPIRES_AT/NOW_MS from environment
(os.environ['EXPIRES_AT'] / os.environ['NOW_MS']) or perform the
subtraction/division with shell arithmetic to ensure no direct shell variable
interpolation into a Python string.

In `@docs/providers/anthropic-max.md`:
- Around line 98-112: Update the "How it works (technical)" section (the header
and table shown) by adding an ASCII quick overview and a collapsible Mermaid
diagram that illustrate the token/refresh path described as "macOS Keychain →
auth.json → plugin → Anthropic API"; place the ASCII diagram immediately after
the "Token path" line for quick reference and add a collapsible section
(details/summary or equivalent used in docs) containing a Mermaid flowchart that
shows each step (Keychain, auth.json, plugin, request to Anthropic API and any
refresh step) and label arrows with token/refresh actions; ensure the diagrams
follow the project's docs guidelines (ASCII for quick overview, Mermaid in
collapsible for detail) and keep wording consistent with the existing "How it
works (technical)" section.

In `@PAI-Install/cli/quick-install.ts`:
- Line 89: Replace the CLI examples that show a literal --api-key <key> with
guidance and code paths that use an environment variable or prompt instead:
update the help/usage text in quick-install.ts (the --api-key flag handling and
displayed examples) to instruct users to set an env var (e.g., read from
process.env.API_KEY) or to invoke an interactive prompt when the flag is
omitted; remove any inline example values in the usage output and change example
lines at the locations referenced (around --api-key and the examples at lines
~102 and ~105-107) so they demonstrate exporting an environment variable or
running the CLI without embedding secrets.

In `@PAI-Install/engine/provider-models.ts`:
- Around line 22-35: The PROVIDER_MODELS mapping shows a deliberate mismatch:
'anthropic' maps standard -> "anthropic/claude-sonnet-4-5" while 'anthropic-max'
maps standard -> "anthropic/claude-sonnet-4-6"; update the file to either make
the versions consistent or document the reason for the difference. If the
difference is intentional (e.g., anthropic-max OAuth has access to 4-6), add a
concise comment above the PROVIDER_MODELS entry referencing the keys 'anthropic'
and 'anthropic-max' and explaining why 'claude-sonnet-4-6' is used for
anthropic-max; otherwise, change the 'standard' value for one provider so both
use the same model string to maintain consistency.

Comment on lines +103 to +121
if (existsSync(skillMdPath)) {
// Found a skill
const relativePath = relative(SKILLS_DIR, fullPath);
const pathParts = relativePath.split(sep);

if (pathParts.length === 1) {
// Check if this is a category directory (has subdirs with SKILL.md)
const subEntries = await readdir(fullPath, { withFileTypes: true });
const hasChildSkills = subEntries.some(e =>
e.isDirectory() && existsSync(join(fullPath, e.name, 'SKILL.md'))
);
if (hasChildSkills) {
// Category metadata SKILL.md — register as category, don't count as skill
categories.add(entry.name);
} else {
// Flat skill: skills/SkillName/
flatSkills++;
await validateSkill(skillMdPath, relativePath, issues, skillNames);
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Do not skip validation for top-level category SKILL.md files.

When a directory has child skills, Lines 114-117 treat its SKILL.md as metadata and never run validateSkill(...). GenerateSkillIndex.ts still indexes those same files (Utilities/SKILL.md, Security/SKILL.md, etc.) as regular entries, so malformed category-root skills can pass validation and then break generation/search later. Either validate/count them here or stop generating them there, but the two tools need the same model.

💡 If category roots are meant to stay in the index, align the validator like this
               if (hasChildSkills) {
-                // Category metadata SKILL.md — register as category, don't count as skill
+                // Category root SKILL.md is also indexed as a flat skill
                 categories.add(entry.name);
+                flatSkills++;
+                await validateSkill(skillMdPath, relativePath, issues, skillNames);
               } else {
                 // Flat skill: skills/SkillName/
                 flatSkills++;
                 await validateSkill(skillMdPath, relativePath, issues, skillNames);
               }

As per coding guidelines, .opencode/skills/**: "Verify: SKILL.md follows PAI v3.0 format (USE WHEN triggers, MANDATORY/OPTIONAL sections)".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.opencode/PAI/Tools/ValidateSkillStructure.ts around lines 103 - 121, The
validator currently skips running validateSkill for category-root SKILL.md when
hasChildSkills is true (see variables/categories, flatSkills and call to
validateSkill), causing category SKILL.md to be unvalidated while
GenerateSkillIndex.ts still indexes them; update this block to also call
validateSkill for category SKILL.md (or otherwise increment/record them) even
when hasChildSkills is true so category metadata files are validated by
validateSkill, or change GenerateSkillIndex.ts to stop indexing category
SKILL.md — ensure validateSkill(relativePath, skillMdPath, issues, skillNames)
is invoked for those entries so the validator and GenerateSkillIndex use the
same model.

Security:
- auth.json now written with mode 0600 (token-utils, compiled JS, contrib)
- Validate access token and expires field before reporting healthy status
- Refresh scripts check for already-expired Keychain tokens before writing
- Refresh scripts restore chmod 600 after rewriting auth.json
- quick-install.ts fails fast on non-macOS for anthropic-max preset

Correctness:
- Use actual Keychain expiresAt instead of hardcoded 8h TTL
- Token bridge attempts refresh on no_anthropic_config (not just expiry)
- Installer copies anthropic-token-bridge.js alongside the bridge plugin
- contrib anthropic-max-bridge.js gets defensive output.system null-safety
- Remove non-existent FEEDSYSTEM.md from doc-dependencies.json
- GenerateSkillIndex frontmatter regex accepts CRLF line endings
- SkillSearch normalizes PascalCase names for natural language queries

Shell scripts:
- VoiceServer: sed replaces cut -d= -f2 to handle = in values
- VoiceServer: remove unused RED/PLIST_PATH variables
- restart.sh: check exit codes from stop.sh/start.sh

Docs:
- Add language tags to fenced code blocks in contrib README/TECHNICAL
Copy link

@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: 9

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
PAI-Install/engine/steps-fresh.ts (1)

413-420: ⚠️ Potential issue | 🟠 Major

Skip writing API key environment variable for anthropic-max provider.

When provider is anthropic-max, the code generates ANTHROPIC-MAX_API_KEY= in the .env file. The hyphen makes this an invalid shell environment variable name. Since anthropic-max uses OAuth tokens from the macOS Keychain and requires no API key, omit the provider environment variable line entirely for this provider instead of deriving a name from the raw provider ID.

Problematic code (lines 413–420)
const providerEnvVar = `${(state.collected.provider || "zen").toUpperCase()}_API_KEY`;
...
${providerEnvVar}=${state.collected.apiKey || ""}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@PAI-Install/engine/steps-fresh.ts` around lines 413 - 420, The provider env
var generation currently builds providerEnvVar from state.collected.provider
which yields an invalid name for "anthropic-max" (e.g., ANTHROPIC-MAX_API_KEY);
update the logic around providerEnvVar and envContent so that when
state.collected.provider === "anthropic-max" you do not include any provider API
key line in envContent (omit the `${providerEnvVar}=${...}` line entirely),
otherwise continue deriving the uppercase provider name (default "zen") and
append the `${providerEnvVar}=${state.collected.apiKey || ""}` line; use the
symbols state.collected.provider, providerEnvVar and envContent to locate and
change the code.
♻️ Duplicate comments (4)
.opencode/plugins/anthropic-token-bridge.js (1)

49-52: ⚠️ Potential issue | 🟠 Major

mode: 0o600 does not re-lock an existing auth.json.

fs.writeFileSync(..., { mode: 0o600 }) only applies when the file is created. If auth.json already exists with broader permissions, this write keeps them, so the OAuth tokens can remain overexposed. Call fs.chmodSync(AUTH_FILE, 0o600) after the write. The same helper is duplicated in contrib/anthropic-max-bridge/plugins/anthropic-token-bridge.js.

🔒 Suggested fix
 function writeAuthFile(authData) {
   try {
     fs.writeFileSync(AUTH_FILE, JSON.stringify(authData, null, 2) + "\n", { mode: 0o600 });
+    fs.chmodSync(AUTH_FILE, 0o600);
     return true;
   } catch (err) {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.opencode/plugins/anthropic-token-bridge.js around lines 49 - 52, The
writeAuthFile function currently uses fs.writeFileSync(AUTH_FILE, ..., { mode:
0o600 }) which only sets permissions on creation; to ensure existing auth.json
is re-locked, call fs.chmodSync(AUTH_FILE, 0o600) immediately after the write
and handle any errors appropriately; apply the same change to the duplicated
helper in contrib/anthropic-max-bridge/plugins/anthropic-token-bridge.js so both
writeAuthFile implementations explicitly enforce file mode after writing.
PAI-Install/engine/steps-fresh.ts (1)

243-247: ⚠️ Potential issue | 🟠 Major

Don't silently downgrade installs when the token bridge is missing.

This copy step is optional right now. If packaging misses anthropic-token-bridge.js, fresh installs still succeed but lose auto-refresh and only fail hours later when the OAuth token expires. Treat the missing file like the primary bridge and return a clear failure message here.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@PAI-Install/engine/steps-fresh.ts` around lines 243 - 247, The current
optional copy of the token bridge (tokenBridgeSrc -> join(pluginsDir,
"anthropic-token-bridge.js")) silently skips when existsSync(tokenBridgeSrc) is
false; change this to treat the missing file as a fatal error by checking
existsSync(tokenBridgeSrc) and throwing/logging a clear failure (including the
expected path and guidance) or exiting similarly to the primary bridge failure
instead of proceeding to copyFileSync; update the block around tokenBridgeSrc,
existsSync, and copyFileSync to fail fast with a descriptive message when the
file is absent.
.opencode/plugins/lib/refresh-manager.ts (2)

143-154: ⚠️ Potential issue | 🟠 Major

Abort the setup-token exchange if the network call hangs.

This fetch() has no deadline. If the request stalls, refreshAnthropicToken() never reaches finally, isRefreshing stays true, and every later refresh attempt is blocked until process restart. Add an AbortController timeout here, then regenerate the JS plugin copies that embed the same helper.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.opencode/plugins/lib/refresh-manager.ts around lines 143 - 154,
exchangeSetupToken() currently calls fetch without a deadline which can hang and
prevent refreshAnthropicToken() from reaching its finally block (leaving
isRefreshing true); wrap the fetch in an AbortController with a timeout (create
an AbortController, call setTimeout to abort after a sensible timeout like
5–10s, pass controller.signal to fetch, and clear the timeout once fetch
completes) so stalled requests are aborted and the finally path (and
isRefreshing reset) always runs; update the exchangeSetupToken function to use
this pattern and then regenerate the JS plugin copies that embed the same helper
so other runtimes receive the timeout behavior.

210-214: ⚠️ Potential issue | 🟠 Major

Don't write an expired Keychain token back with a fake future expiry.

When expiresAt exists but is already past, this fallback turns that stale Keychain token into a fresh 8-hour token in auth.json. Only use the default for the legacy no-expiry shape; if Keychain explicitly says the token is expired, fall through to setup-token generation instead.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.opencode/plugins/lib/refresh-manager.ts around lines 210 - 214, The current
logic for expiresInSeconds treats a past keychainTokens.expiresAt as if it were
missing and assigns the 8-hour default, which writes an invalid refreshed expiry
into auth.json; change the condition so the 8-hour default is only used when
keychainTokens.expiresAt is null/undefined (the legacy no-expiry shape), and if
keychainTokens.expiresAt exists but is <= Date.now() treat it as expired (do not
set expiresInSeconds to 28800) so the code falls through to the
setup-token/refresh flow instead; update the calculation around
keychainTokens.expiresAt and expiresInSeconds accordingly.
🧹 Nitpick comments (3)
.opencode/PAI/Tools/GenerateSkillIndex.ts (1)

20-41: Consider sharing or extending the interface definition with SkillSearch.ts.

SkillSearch.ts defines its own SkillEntry interface (lines 24-31) without the category and isHierarchical fields added here. While structurally compatible (extra fields are ignored at runtime), maintaining duplicate interface definitions increases drift risk.

Options:

  1. Extract a shared interface to a common types file that both scripts import.
  2. Document that this generator produces a superset of what SkillSearch expects.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.opencode/PAI/Tools/GenerateSkillIndex.ts around lines 20 - 41, SkillEntry
is duplicated between GenerateSkillIndex.ts and SkillSearch.ts which risks
drift; extract the canonical interface (and SkillIndex if useful) into a shared
types module (e.g., export an interface named SkillEntry and SkillIndex) and
update GenerateSkillIndex.ts and SkillSearch.ts to import those types instead of
redeclaring them, or alternatively export the more complete SkillEntry from
GenerateSkillIndex.ts and import it in SkillSearch.ts; ensure the shared
interface includes category and isHierarchical to keep both modules consistent.
.opencode/VoiceServer/uninstall.sh (1)

1-4: Consider enabling strict shell mode for safer cleanup flow.

Using set -euo pipefail would catch silent failures and unset-variable mistakes during uninstall.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.opencode/VoiceServer/uninstall.sh around lines 1 - 4, Add strict shell mode
at the top of the uninstall script to fail fast and catch unset variables: after
the shebang in uninstall.sh enable `set -euo pipefail` (and consider setting
IFS=$'\n\t' for safer word splitting) so the uninstall flow aborts on errors,
undefined variables, or pipeline failures; ensure any commands that are allowed
to fail are wrapped or explicitly handled so the script still performs necessary
cleanup.
.opencode/plugins/anthropic-token-bridge.js (1)

345-347: Keep plugin init free of synchronous disk I/O.

The factory does a blocking appendFileSync via fileLog() before returning the hook object. Defer this log to the first async hook, or make init a pure object return, so plugin startup stays non-blocking. As per coding guidelines, "Avoid blocking operations in lifecycle plugin init hooks".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.opencode/plugins/anthropic-token-bridge.js around lines 345 - 347, The
AnthropicTokenBridge function performs synchronous disk I/O by calling
fileLog("AnthropicTokenBridge plugin loaded", "info") during initialization;
remove that blocking appendFileSync call from the top-level of
AnthropicTokenBridge and either defer logging into the first async hook (e.g.,
inside the returned async hook function) or make AnthropicTokenBridge a pure,
non-blocking factory that returns the hook object immediately; locate the
fileLog usage and replace it with an async-safe alternative (call fileLog inside
an async lifecycle method or use an async logging API) so the init of
AnthropicTokenBridge remains non-blocking.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.opencode/PAI/Tools/GenerateSkillIndex.ts:
- Around line 43-50: The matching against ALWAYS_LOADED_SKILLS is currently
case-sensitive and will misclassify names like "core"; normalize for
case-insensitive matching by creating a lowercase lookup (e.g., derive a Set
from ALWAYS_LOADED_SKILLS.map(s => s.toLowerCase())) and then check membership
using frontmatter.name?.toLowerCase() (or equivalent null-safe lowercasing) when
classifying skills; update the classification logic that references
ALWAYS_LOADED_SKILLS to use the normalized set instead of exact string
comparison.

In @.opencode/VoiceServer/install.sh:
- Around line 52-68: The ElevenLabs check sets ELEVENLABS_CONFIGURED based on
ENV_FILE but the LaunchAgent that runs the service does not load ENV_FILE, so
process.env.ELEVENLABS_API_KEY remains unset at runtime; modify install.sh to
propagate the key into the service runtime by (a) when ENV_FILE contains
ELEVENLABS_API_KEY and it's not the placeholder, write that key into the
LaunchAgent environment (or into the service wrapper) used to install the
LaunchAgent so the agent can read ELEVENLABS_API_KEY at runtime, and (b) ensure
the same logic is applied for the other similar checks (lines referenced around
90-95 and 115-121) so they also export their variables to the LaunchAgent; use
the existing symbols ENV_FILE and ELEVENLABS_CONFIGURED to locate the check and
update the LaunchAgent installation step that registers the service.
- Around line 6-7: This script runs macOS-specific LaunchAgent operations but
lacks a platform guard; add an early check (before any launchctl/LaunchAgents
path usage) that verifies the OS is Darwin (e.g., via uname -s) and if not,
print a clear error like "This installer only supports macOS" and exit nonzero;
ensure this guard is placed near the top (just after set -e) so
functions/commands that reference launchctl or ~/Library/LaunchAgents are never
executed on non-macOS hosts.
- Around line 141-149: The /notify POST call can report success even on 4xx/5xx
because it omits curl's -f/--fail; update the curl invocation that posts to
/notify to fail on non-2xx (e.g., add -f or check HTTP status) and branch on its
exit status so the script only prints "OK Test notification sent" when the POST
actually succeeded—modify the POST block (the curl -X POST ... -d '{"message":
"Voice server installed successfully"}' invocation) to use --fail or capture the
HTTP code and handle non-success by printing an error and exiting non‑zero.

In @.opencode/VoiceServer/uninstall.sh:
- Around line 1-8: The script uninstall.sh currently assumes macOS; add an
explicit early platform guard at the top that checks the OS (e.g., via uname -s
or $OSTYPE) and exits with a clear error if not macOS, so the LaunchAgents logic
(SERVICE_NAME, PLIST_PATH, LOG_PATH) only runs on Darwin; ensure the guard
prints a helpful message and exits non-zero to prevent running macOS-specific
uninstall steps on other platforms.
- Around line 54-57: Current uninstall.sh unconditionally kills any PID
listening on port 8888; change it to only kill processes that are the
VoiceServer by querying lsof for PIDs plus the COMMAND and filtering before
kill. Specifically, replace the blind lsof -ti :8888 | xargs kill -9 flow with
logic that: (1) uses lsof to get PID and command/name for entries on :8888
(LISTEN), (2) checks the command/name contains the expected VoiceServer
identifier (e.g., "VoiceServer" or the actual binary/script name used to run the
service), and (3) only kills those matching PIDs; ensure you still handle
no-match cases and suppress errors when no PID to kill. This protects unrelated
processes while retaining the cleanup behavior in uninstall.sh.

In `@contrib/anthropic-max-bridge/README.md`:
- Around line 120-136: The fenced code block in
contrib/anthropic-max-bridge/README.md that contains the flow diagram starting
with "Claude Code CLI" should be marked with a language to satisfy MD040; change
the opening fence from ``` to ```text so the block is treated as plain text.
Locate the fenced block in the README (the one listing "Claude Code CLI",
"install.sh / refresh-token.sh", and "OpenCode") and update the fence to ```text
at the start and keep the closing ``` unchanged.

In `@PAI-Install/engine/steps-fresh.ts`:
- Around line 606-620: The interactive "anthropic-max" branch currently shows
instructions and calls requestInput("anthropic-max-confirm", ...) on all
platforms; gate this flow to macOS by checking the runtime platform (e.g.
process.platform or os.platform()) before emitting the message and calling
requestInput, and if not darwin immediately emit an error or throw to fail fast;
update the block surrounding provider === "anthropic-max" (the emit({...}) and
requestInput call) to perform the platform check and bail out early on
non-darwin so no anthropic-max config is written on unsupported OSes.
- Around line 290-308: The code currently defaults oauth.expiresAt to
Date.now()+8h which can accept stale/malformed keychain data; instead, require a
valid numeric, non-expired expiresAt from creds.claudeAiOauth. After parsing
keychainJson and verifying accessToken (the oauth variable), check that typeof
oauth.expiresAt === "number" and oauth.expiresAt > Date.now(); if not, return
the same failure shape (success:false, message:"Unexpected token format in
Keychain...") so we don't write a synthesized TTL into auth.json. If the check
passes, set expiresAt = oauth.expiresAt and keep accessToken/refreshToken
assignment as-is.

---

Outside diff comments:
In `@PAI-Install/engine/steps-fresh.ts`:
- Around line 413-420: The provider env var generation currently builds
providerEnvVar from state.collected.provider which yields an invalid name for
"anthropic-max" (e.g., ANTHROPIC-MAX_API_KEY); update the logic around
providerEnvVar and envContent so that when state.collected.provider ===
"anthropic-max" you do not include any provider API key line in envContent (omit
the `${providerEnvVar}=${...}` line entirely), otherwise continue deriving the
uppercase provider name (default "zen") and append the
`${providerEnvVar}=${state.collected.apiKey || ""}` line; use the symbols
state.collected.provider, providerEnvVar and envContent to locate and change the
code.

---

Duplicate comments:
In @.opencode/plugins/anthropic-token-bridge.js:
- Around line 49-52: The writeAuthFile function currently uses
fs.writeFileSync(AUTH_FILE, ..., { mode: 0o600 }) which only sets permissions on
creation; to ensure existing auth.json is re-locked, call
fs.chmodSync(AUTH_FILE, 0o600) immediately after the write and handle any errors
appropriately; apply the same change to the duplicated helper in
contrib/anthropic-max-bridge/plugins/anthropic-token-bridge.js so both
writeAuthFile implementations explicitly enforce file mode after writing.

In @.opencode/plugins/lib/refresh-manager.ts:
- Around line 143-154: exchangeSetupToken() currently calls fetch without a
deadline which can hang and prevent refreshAnthropicToken() from reaching its
finally block (leaving isRefreshing true); wrap the fetch in an AbortController
with a timeout (create an AbortController, call setTimeout to abort after a
sensible timeout like 5–10s, pass controller.signal to fetch, and clear the
timeout once fetch completes) so stalled requests are aborted and the finally
path (and isRefreshing reset) always runs; update the exchangeSetupToken
function to use this pattern and then regenerate the JS plugin copies that embed
the same helper so other runtimes receive the timeout behavior.
- Around line 210-214: The current logic for expiresInSeconds treats a past
keychainTokens.expiresAt as if it were missing and assigns the 8-hour default,
which writes an invalid refreshed expiry into auth.json; change the condition so
the 8-hour default is only used when keychainTokens.expiresAt is null/undefined
(the legacy no-expiry shape), and if keychainTokens.expiresAt exists but is <=
Date.now() treat it as expired (do not set expiresInSeconds to 28800) so the
code falls through to the setup-token/refresh flow instead; update the
calculation around keychainTokens.expiresAt and expiresInSeconds accordingly.

In `@PAI-Install/engine/steps-fresh.ts`:
- Around line 243-247: The current optional copy of the token bridge
(tokenBridgeSrc -> join(pluginsDir, "anthropic-token-bridge.js")) silently skips
when existsSync(tokenBridgeSrc) is false; change this to treat the missing file
as a fatal error by checking existsSync(tokenBridgeSrc) and throwing/logging a
clear failure (including the expected path and guidance) or exiting similarly to
the primary bridge failure instead of proceeding to copyFileSync; update the
block around tokenBridgeSrc, existsSync, and copyFileSync to fail fast with a
descriptive message when the file is absent.

---

Nitpick comments:
In @.opencode/PAI/Tools/GenerateSkillIndex.ts:
- Around line 20-41: SkillEntry is duplicated between GenerateSkillIndex.ts and
SkillSearch.ts which risks drift; extract the canonical interface (and
SkillIndex if useful) into a shared types module (e.g., export an interface
named SkillEntry and SkillIndex) and update GenerateSkillIndex.ts and
SkillSearch.ts to import those types instead of redeclaring them, or
alternatively export the more complete SkillEntry from GenerateSkillIndex.ts and
import it in SkillSearch.ts; ensure the shared interface includes category and
isHierarchical to keep both modules consistent.

In @.opencode/plugins/anthropic-token-bridge.js:
- Around line 345-347: The AnthropicTokenBridge function performs synchronous
disk I/O by calling fileLog("AnthropicTokenBridge plugin loaded", "info") during
initialization; remove that blocking appendFileSync call from the top-level of
AnthropicTokenBridge and either defer logging into the first async hook (e.g.,
inside the returned async hook function) or make AnthropicTokenBridge a pure,
non-blocking factory that returns the hook object immediately; locate the
fileLog usage and replace it with an async-safe alternative (call fileLog inside
an async lifecycle method or use an async logging API) so the init of
AnthropicTokenBridge remains non-blocking.

In @.opencode/VoiceServer/uninstall.sh:
- Around line 1-4: Add strict shell mode at the top of the uninstall script to
fail fast and catch unset variables: after the shebang in uninstall.sh enable
`set -euo pipefail` (and consider setting IFS=$'\n\t' for safer word splitting)
so the uninstall flow aborts on errors, undefined variables, or pipeline
failures; ensure any commands that are allowed to fail are wrapped or explicitly
handled so the script still performs necessary cleanup.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 42bbce00-a9e2-4281-b340-daff4fa2af01

📥 Commits

Reviewing files that changed from the base of the PR and between 6118bd0 and aef6e11.

📒 Files selected for processing (20)
  • .opencode/PAI/Tools/GenerateSkillIndex.ts
  • .opencode/PAI/Tools/SkillSearch.ts
  • .opencode/PAI/doc-dependencies.json
  • .opencode/VoiceServer/install.sh
  • .opencode/VoiceServer/menubar/pai-voice.5s.sh
  • .opencode/VoiceServer/restart.sh
  • .opencode/VoiceServer/status.sh
  • .opencode/VoiceServer/uninstall.sh
  • .opencode/plugins/anthropic-token-bridge.js
  • .opencode/plugins/anthropic-token-bridge.ts
  • .opencode/plugins/lib/refresh-manager.ts
  • .opencode/plugins/lib/token-utils.ts
  • PAI-Install/anthropic-max-refresh.sh
  • PAI-Install/cli/quick-install.ts
  • PAI-Install/engine/steps-fresh.ts
  • contrib/anthropic-max-bridge/README.md
  • contrib/anthropic-max-bridge/TECHNICAL.md
  • contrib/anthropic-max-bridge/plugins/anthropic-max-bridge.js
  • contrib/anthropic-max-bridge/plugins/anthropic-token-bridge.js
  • contrib/anthropic-max-bridge/refresh-token.sh
✅ Files skipped from review due to trivial changes (4)
  • .opencode/PAI/Tools/SkillSearch.ts
  • .opencode/PAI/doc-dependencies.json
  • PAI-Install/anthropic-max-refresh.sh
  • .opencode/plugins/lib/token-utils.ts
🚧 Files skipped from review as they are similar to previous changes (6)
  • .opencode/VoiceServer/restart.sh
  • PAI-Install/cli/quick-install.ts
  • .opencode/VoiceServer/menubar/pai-voice.5s.sh
  • .opencode/VoiceServer/status.sh
  • contrib/anthropic-max-bridge/plugins/anthropic-max-bridge.js
  • .opencode/plugins/anthropic-token-bridge.ts

Comment on lines +120 to +136
```
Claude Code CLI
└─ authenticates with Anthropic
└─ stores token in macOS Keychain
Service: "Claude Code-credentials"

install.sh / refresh-token.sh
└─ reads token from Keychain
└─ writes to ~/.local/share/opencode/auth.json

OpenCode
└─ reads auth.json on startup
└─ anthropic-max-bridge: 3 API fixes on every request
└─ anthropic-token-bridge: checks token every 5 messages,
auto-refreshes from Keychain if expiring
└─ Anthropic API accepts → response streams back
```
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Add a language to this fenced block.

Use text for the flow diagram so the docs pass MD040 and stay consistent with the other fenced examples.

🧰 Tools
🪛 markdownlint-cli2 (0.21.0)

[warning] 120-120: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@contrib/anthropic-max-bridge/README.md` around lines 120 - 136, The fenced
code block in contrib/anthropic-max-bridge/README.md that contains the flow
diagram starting with "Claude Code CLI" should be marked with a language to
satisfy MD040; change the opening fence from ``` to ```text so the block is
treated as plain text. Locate the fenced block in the README (the one listing
"Claude Code CLI", "install.sh / refresh-token.sh", and "OpenCode") and update
the fence to ```text at the start and keep the closing ``` unchanged.

…token expiry logic

Session-start improvements:
- GenerateSkillIndex.ts: case-insensitive ALWAYS_LOADED_SKILLS matching
- anthropic-token-bridge: enhanced session-start token validation
- refresh-manager: improved expiresAt handling with null/undefined/past checks

Platform guards:
- VoiceServer/install.sh: curl --fail for notify endpoint
- VoiceServer/uninstall.sh: Darwin platform guard + safe process kill
- steps-fresh.ts: macOS platform gate for anthropic-max + expiresAt validation

Token logic fixes:
- steps-fresh.ts: skip provider API key for anthropic-max in .env
- refresh-manager: AbortController 10s timeout for fetch
- contrib plugin: mirror all fixes
The 5-second setTimeout delay caused API calls to fail with expired tokens
before refresh completed. Now refreshes immediately and synchronously
when session starts with an expired/soon-expiring token.

- Changed from setTimeout(5000) to immediate await
- Added fallback async retry on failure
- Applied to: .ts source + compiled .js + contrib plugin
Root cause fix for stale token issue: the session-start hook previously
only checked auth.json's `expires` timestamp, which can be valid even
when Anthropic has invalidated the token (e.g. after re-authentication).

Now the hook always reads the Keychain first (source of truth) and writes
it to auth.json before any expiry logic runs. If Keychain and auth.json
tokens differ, a warning is logged and the fresh token is synced.

Fallback path (Keychain unavailable or expired) still triggers full
refreshAnthropicToken() as before.

Also exports extractFromKeychain() from refresh-manager.ts so it can
be used directly in the bridge hook.
Copy link

@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: 1

♻️ Duplicate comments (1)
.opencode/PAI/Tools/GenerateSkillIndex.ts (1)

299-304: ⚠️ Potential issue | 🟠 Major

Make duplicate skill names fatal during generation.

The duplicate handling still warns and skips rather than throwing. This allows stale copies to silently affect the generated index when both legacy and relocated versions exist.

💡 Proposed fix
-      // Check for duplicates - don't overwrite existing entries
+      // Check for duplicates - fail fast so stale copies cannot silently win
       if (index.skills[key]) {
-        console.warn(`⚠️  Duplicate skill name "${skill.name}" found at ${skill.path} (existing: ${index.skills[key].path})`);
-        // Skip adding duplicate
-        continue;
+        throw new Error(
+          `Duplicate skill name "${skill.name}" found at ${skill.path} (existing: ${index.skills[key].path})`
+        );
       }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.opencode/PAI/Tools/GenerateSkillIndex.ts around lines 299 - 304, Replace
the current duplicate handling in GenerateSkillIndex.ts (the block checking
index.skills[key]) so that instead of logging a warning and continuing, the
generator throws an Error to fail the build; use the same identifiers
(index.skills, key, skill.name, skill.path, index.skills[key].path) to construct
a clear error message like "Duplicate skill name '<skill.name>' found at
<skill.path> (existing: <index.skills[key].path>)" and throw that Error so
generation is fatal when duplicates are detected.
🧹 Nitpick comments (6)
.opencode/VoiceServer/uninstall.sh (2)

59-69: **Filtered kill approach is good, but lsof header row may cause spurious errors.**The filtering approach addresses the prior review feedback about killing unrelated processes. However, "lsof has a header line that contains PID and this text will be passed onto kill as well. Whilst it will not create an issue, a better overall command would have read lsof | grep "workspace" | grep -v "PID" | awk '{print $2}'... or some other way of filtering the first line altogether."

Line 63 processes lsof -i :8888 output through grep and awk, but lsof outputs a header row (COMMAND PID USER...). The grep filter for (VoiceServer|bun|server\.ts) should exclude the header in practice, so this is likely safe. However, if a future match pattern inadvertently matches the header, awk '{print $2}' would extract "PID" and attempt kill -0 PID, which would fail harmlessly.

The filtered kill approach correctly addresses the prior review concern about killing unrelated processes. The grep pattern (VoiceServer|bun|server\.ts) effectively excludes the header row. The kill -0 check before kill -9 adds an extra safety layer.

Minor note: Consider using lsof -ti :8888 (terse output, PIDs only) combined with process verification for a cleaner approach:

Alternative using terse output
 # Kill any remaining VoiceServer processes (check process name before killing)
 if lsof -i :8888 > /dev/null 2>&1; then
     echo -e "${YELLOW}> Cleaning up VoiceServer processes on port 8888...${NC}"
-    # Get PIDs with their commands, filter for VoiceServer/bun, then kill
-    lsof -i :8888 | grep -E '(VoiceServer|bun|server\.ts)' | awk '{print $2}' | while read -r pid; do
-        if [ -n "$pid" ] && kill -0 "$pid" 2>/dev/null; then
-            kill -9 "$pid" 2>/dev/null
-        fi
-    done
+    # Get PIDs and verify each is a VoiceServer process before killing
+    for pid in $(lsof -ti :8888 2>/dev/null); do
+        cmd=$(ps -p "$pid" -o command= 2>/dev/null || true)
+        if [[ "$cmd" == *"server.ts"* ]] || [[ "$cmd" == *"bun"* ]]; then
+            kill -9 "$pid" 2>/dev/null || true
+        fi
+    done
     echo -e "${GREEN}OK Port 8888 cleared${NC}"
 fi
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.opencode/VoiceServer/uninstall.sh around lines 59 - 69, The lsof parsing
can accidentally pass the header ("PID") into kill if future grep patterns match
it; update the uninstall.sh cleanup to use lsof's terse PID-only output (lsof
-ti :8888) and then verify each PID's command before killing (e.g., for each PID
returned by lsof -ti :8888, use ps -p <pid> -o comm= or readlink /proc/<pid>/exe
to check it matches VoiceServer, bun, or server.ts) and only then run kill -9
after the existing kill -0 check; reference the lsof -i :8888 invocation, the
grep/awk pipeline, and the kill -0 / kill -9 sequence when making the change.

88-89: Path in "completely remove" message may be incorrect if script is invoked via symlink.

$(dirname "${BASH_SOURCE[0]}") gives the directory of the script file, but if the script is invoked via a symlink, it resolves to the symlink's directory, not the actual script location.

Use resolved path for accuracy
+SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
+
 echo -e "${BLUE}Notes:${NC}"
-echo "  - Your server files are still in: $(dirname "${BASH_SOURCE[0]}")"
+echo "  - Your server files are still in: $SCRIPT_DIR"
 echo "  - Your ~/.env configuration is preserved"
 echo "  - To reinstall, run: ./install.sh"
 echo
 echo "To completely remove all files:"
-echo "  rm -rf $(dirname "${BASH_SOURCE[0]}")"
+echo "  rm -rf $SCRIPT_DIR"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.opencode/VoiceServer/uninstall.sh around lines 88 - 89, The message in
uninstall.sh that prints the script directory uses $(dirname
"${BASH_SOURCE[0]}") which can be wrong when the script is invoked via a
symlink; update the echo to compute the script's real directory by resolving the
path of "${BASH_SOURCE[0]}" (e.g., via readlink -f or realpath) and then taking
its dirname or absolute directory, so the printed path reflects the actual
script location rather than the symlink location; modify the echo that
references dirname "${BASH_SOURCE[0]}" accordingly in uninstall.sh.
.opencode/VoiceServer/install.sh (4)

144-145: Fixed 2-second sleep may be insufficient for slower machines.

A simple sleep 2 before health-checking could fail on slower systems or under load. Consider polling the health endpoint with a timeout loop.

Proposed polling approach
-# Wait for server to start
-sleep 2
+# Wait for server to start (poll up to 10 seconds)
+echo -e "${YELLOW}> Waiting for server to start...${NC}"
+for i in {1..10}; do
+    if curl -s -f http://localhost:8888/health > /dev/null 2>&1; then
+        break
+    fi
+    sleep 1
+done
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.opencode/VoiceServer/install.sh around lines 144 - 145, Replace the fixed
"sleep 2" in install.sh with a polling loop that repeatedly queries the server
health endpoint (e.g., using curl) until it returns a success status or a
configurable timeout is reached; implement a short interval sleep between
attempts (e.g., 1s), a total timeout (e.g., 60s), and exit with an error code if
the health check never succeeds so downstream steps fail fast instead of
assuming the server is ready.

96-101: Bun path is resolved at install time — verify it remains valid.

$(which bun) is evaluated when the plist is generated, embedding the absolute path. If Bun is later upgraded or relocated (e.g., via version manager), the LaunchAgent will fail to start. This is a common pattern but worth noting.

Consider using a wrapper approach for more resilience:

Alternative: Use shell wrapper to resolve bun at runtime
     <key>ProgramArguments</key>
     <array>
-        <string>$(which bun)</string>
-        <string>run</string>
-        <string>${SCRIPT_DIR}/server.ts</string>
+        <string>/bin/bash</string>
+        <string>-lc</string>
+        <string>exec "\$(command -v bun)" run "${SCRIPT_DIR}/server.ts"</string>
     </array>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.opencode/VoiceServer/install.sh around lines 96 - 101, The plist currently
embeds a build-time resolved Bun path using ProgramArguments with "$(which
bun)", which can break if Bun is moved; change the install to instead write a
small shell wrapper (e.g., create an executable script next to server.ts that
resolves Bun at runtime via "which bun" or PATH lookup and then exec "$BUN" run
"${SCRIPT_DIR}/server.ts") and update the ProgramArguments in the generated
plist to call that wrapper (use the wrapper path in place of "$(which bun)");
ensure the wrapper is made executable and that SCRIPT_DIR/server.ts is still
passed through.

58-74: Environment variable extraction uses basic parsing that may mishandle edge cases.

Line 61 uses sed 's/^[^=]*=//' which doesn't handle quoted values or inline comments. For example, ELEVENLABS_API_KEY="sk-abc" # my key would extract "sk-abc" # my key including quotes and comment.

This is unlikely to cause issues with typical .env files, but for robustness:

More robust extraction
-    API_KEY=$(grep "ELEVENLABS_API_KEY=" "$ENV_FILE" | sed 's/^[^=]*=//')
+    API_KEY=$(grep "^ELEVENLABS_API_KEY=" "$ENV_FILE" | head -1 | sed 's/^[^=]*=//' | sed 's/^["'"'"']//' | sed 's/["'"'"']$//' | sed 's/ *#.*//')
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.opencode/VoiceServer/install.sh around lines 58 - 74, The current
extraction of ELEVENLABS_API_KEY from ENV_FILE (where API_KEY is set using sed
's/^[^=]*=//') can retain quotes and inline comments; update the parsing logic
for the block that reads ENV_FILE and sets API_KEY/ELEVENLABS_CONFIGURED so it
strips surrounding quotes, removes any trailing inline comment, and trims
whitespace before checking the value. Locate the code that checks for
"ELEVENLABS_API_KEY=" in ENV_FILE and replace the simple sed extraction with a
more robust extraction (e.g., using grep/awk/perl-style regex to capture the
value, then trim quotes and comments) so the subsequent comparisons against
"your_api_key_here" and -n "$API_KEY" behave correctly and ELEVENLABS_CONFIGURED
is set accurately.

121-129: Conditionally include ELEVENLABS_API_KEY in plist only when configured.

When API_KEY is unset in the install script (lines 60-74), line 128 expands to an empty string, setting ELEVENLABS_API_KEY="" in the LaunchAgent environment. While server.ts will correctly load and overwrite this value from .env via direct assignment (line 24), setting an empty string in the plist is unnecessary and potentially confusing for debugging.

The proposed fix to conditionally include the entry only when a valid key exists is a reasonable improvement:

Proposed fix
     <key>EnvironmentVariables</key>
     <dict>
         <key>HOME</key>
         <string>${HOME}</string>
         <key>PATH</key>
         <string>/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:${HOME}/.bun/bin</string>
-        <key>ELEVENLABS_API_KEY</key>
-        <string>${API_KEY:-}</string>
+$(if [ -n "${API_KEY:-}" ]; then
+cat << APIKEY
+        <key>ELEVENLABS_API_KEY</key>
+        <string>${API_KEY}</string>
+APIKEY
+fi)
     </dict>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.opencode/VoiceServer/install.sh around lines 121 - 129, The plist currently
always writes an ELEVENLABS_API_KEY entry under the EnvironmentVariables dict
even when API_KEY is empty; update the install.sh logic that builds the plist so
the <key>ELEVENLABS_API_KEY</key> / <string>...</string> pair is only emitted
when API_KEY is non-empty (check [ -n "${API_KEY:-}" ] or similar) rather than
inserting an empty string, leaving EnvironmentVariables unchanged otherwise;
this change should be applied where the plist heredoc is constructed so
consumers (and debugging) no longer see ELEVENLABS_API_KEY="" while server.ts
still reads/overrides values from .env (server.ts line 24).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@contrib/anthropic-max-bridge/plugins/anthropic-token-bridge.js`:
- Around line 416-447: In the experimental.chat.system.transform handler, call
extractFromKeychain() first and, if it returns a token, sync that token into
auth.json (using the same sync/write helper the main plugin uses) before calling
checkAnthropicToken(); ensure this is awaited and wrap it in try/catch with
fileLog/fileLogError so Keychain-held freshest tokens replace stale auth.json
values prior to running checkAnthropicToken(), and keep the existing
refreshAnthropicToken()/isRefreshInProgress logic unchanged.

---

Duplicate comments:
In @.opencode/PAI/Tools/GenerateSkillIndex.ts:
- Around line 299-304: Replace the current duplicate handling in
GenerateSkillIndex.ts (the block checking index.skills[key]) so that instead of
logging a warning and continuing, the generator throws an Error to fail the
build; use the same identifiers (index.skills, key, skill.name, skill.path,
index.skills[key].path) to construct a clear error message like "Duplicate skill
name '<skill.name>' found at <skill.path> (existing: <index.skills[key].path>)"
and throw that Error so generation is fatal when duplicates are detected.

---

Nitpick comments:
In @.opencode/VoiceServer/install.sh:
- Around line 144-145: Replace the fixed "sleep 2" in install.sh with a polling
loop that repeatedly queries the server health endpoint (e.g., using curl) until
it returns a success status or a configurable timeout is reached; implement a
short interval sleep between attempts (e.g., 1s), a total timeout (e.g., 60s),
and exit with an error code if the health check never succeeds so downstream
steps fail fast instead of assuming the server is ready.
- Around line 96-101: The plist currently embeds a build-time resolved Bun path
using ProgramArguments with "$(which bun)", which can break if Bun is moved;
change the install to instead write a small shell wrapper (e.g., create an
executable script next to server.ts that resolves Bun at runtime via "which bun"
or PATH lookup and then exec "$BUN" run "${SCRIPT_DIR}/server.ts") and update
the ProgramArguments in the generated plist to call that wrapper (use the
wrapper path in place of "$(which bun)"); ensure the wrapper is made executable
and that SCRIPT_DIR/server.ts is still passed through.
- Around line 58-74: The current extraction of ELEVENLABS_API_KEY from ENV_FILE
(where API_KEY is set using sed 's/^[^=]*=//') can retain quotes and inline
comments; update the parsing logic for the block that reads ENV_FILE and sets
API_KEY/ELEVENLABS_CONFIGURED so it strips surrounding quotes, removes any
trailing inline comment, and trims whitespace before checking the value. Locate
the code that checks for "ELEVENLABS_API_KEY=" in ENV_FILE and replace the
simple sed extraction with a more robust extraction (e.g., using
grep/awk/perl-style regex to capture the value, then trim quotes and comments)
so the subsequent comparisons against "your_api_key_here" and -n "$API_KEY"
behave correctly and ELEVENLABS_CONFIGURED is set accurately.
- Around line 121-129: The plist currently always writes an ELEVENLABS_API_KEY
entry under the EnvironmentVariables dict even when API_KEY is empty; update the
install.sh logic that builds the plist so the <key>ELEVENLABS_API_KEY</key> /
<string>...</string> pair is only emitted when API_KEY is non-empty (check [ -n
"${API_KEY:-}" ] or similar) rather than inserting an empty string, leaving
EnvironmentVariables unchanged otherwise; this change should be applied where
the plist heredoc is constructed so consumers (and debugging) no longer see
ELEVENLABS_API_KEY="" while server.ts still reads/overrides values from .env
(server.ts line 24).

In @.opencode/VoiceServer/uninstall.sh:
- Around line 59-69: The lsof parsing can accidentally pass the header ("PID")
into kill if future grep patterns match it; update the uninstall.sh cleanup to
use lsof's terse PID-only output (lsof -ti :8888) and then verify each PID's
command before killing (e.g., for each PID returned by lsof -ti :8888, use ps -p
<pid> -o comm= or readlink /proc/<pid>/exe to check it matches VoiceServer, bun,
or server.ts) and only then run kill -9 after the existing kill -0 check;
reference the lsof -i :8888 invocation, the grep/awk pipeline, and the kill -0 /
kill -9 sequence when making the change.
- Around line 88-89: The message in uninstall.sh that prints the script
directory uses $(dirname "${BASH_SOURCE[0]}") which can be wrong when the script
is invoked via a symlink; update the echo to compute the script's real directory
by resolving the path of "${BASH_SOURCE[0]}" (e.g., via readlink -f or realpath)
and then taking its dirname or absolute directory, so the printed path reflects
the actual script location rather than the symlink location; modify the echo
that references dirname "${BASH_SOURCE[0]}" accordingly in uninstall.sh.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: a0fa2270-c50f-4364-a640-b2322a65781b

📥 Commits

Reviewing files that changed from the base of the PR and between aef6e11 and 7665d38.

📒 Files selected for processing (8)
  • .opencode/PAI/Tools/GenerateSkillIndex.ts
  • .opencode/VoiceServer/install.sh
  • .opencode/VoiceServer/uninstall.sh
  • .opencode/plugins/anthropic-token-bridge.js
  • .opencode/plugins/anthropic-token-bridge.ts
  • .opencode/plugins/lib/refresh-manager.ts
  • PAI-Install/engine/steps-fresh.ts
  • contrib/anthropic-max-bridge/plugins/anthropic-token-bridge.js

contrib/anthropic-max-bridge/plugins/anthropic-token-bridge.js
- Session-start hook now syncs Keychain→auth.json first (same pattern as
  main plugin); extractFromKeychain() result is awaited and wrapped in its
  own try/catch with fileLog/fileLogError; existing refreshAnthropicToken()
  / isRefreshInProgress() fallback path is unchanged

.opencode/PAI/Tools/GenerateSkillIndex.ts
- Duplicate skill name detection now throws an Error instead of warn+continue,
  making the build fail fast with a clear message

.opencode/VoiceServer/install.sh
- API_KEY extraction strips surrounding quotes, inline comments, and
  whitespace before comparing against sentinel values
- ProgramArguments now point to a runtime wrapper (run-server.sh) that
  resolves Bun via PATH at launch time instead of baking in /Users/steffen/.bun/bin/bun
- ELEVENLABS_API_KEY plist entry is only emitted when API_KEY is non-empty
- Fixed sleep 2 with a polling health-check loop (60s timeout, 1s interval)
  that exits with an error if the server never responds

.opencode/VoiceServer/uninstall.sh
- Port cleanup now uses lsof -ti for PID-only output (no header line risk)
  and verifies each PID's command via ps before killing
- Script directory echo now resolves symlinks via readlink -f / realpath
Copy link

@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: 3

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

Inline comments:
In @.opencode/VoiceServer/install.sh:
- Line 26: The ENV_FILE variable in the installer is set to "$HOME/.env" but the
server expects "$HOME/.opencode/.env" (or uses OPENCODE_DIR/PAI_DIR). Update the
ENV_FILE assignment (the ENV_FILE variable in install.sh) to point to the same
default as the server (e.g., "$HOME/.opencode/.env") or derive it from the same
environment variable (OPENCODE_DIR/PAI_DIR) so both installer and server
read/write the identical path; ensure any logic that checks for ElevenLabs keys
uses that ENV_FILE variable.
- Around line 62-66: The API_KEY extraction currently concatenates multiple
ELEVENLABS_API_KEY lines and naively strips any '#' inside values; update the
pipeline that sets API_KEY to (1) use grep -m1 to return only the first
ELEVENLABS_API_KEY match from ENV_FILE to avoid concatenation and (2) stop using
the broad comment-stripping sed that truncates values containing '#' — either
remove inline-comment stripping entirely (require users to quote values) or
replace it with a pattern that only removes trailing comments when the '#' is
outside quotes (i.e., use a comment-strip pattern that matches #[^"']*$). Apply
these changes where API_KEY is assigned so you still strip the key name, remove
surrounding quotes, and trim whitespace safely.

In `@contrib/anthropic-max-bridge/plugins/anthropic-token-bridge.js`:
- Around line 211-219: The code destructures only stdout and exitCode from
execCommand but later logs stderr incorrectly; update the call that runs
execCommand for the "security find-generic-password" command to destructure {
stdout, stderr, exitCode } (matching the earlier call pattern) and pass the real
stderr into the error(...) call (replace stderr: stdout with stderr) so the
error log contains the actual stderr from execCommand; reference the execCommand
invocation and the error(...) call around the "Claude Code-credentials" lookup.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: ea76048e-41ed-4fdc-9425-8be3ab66193a

📥 Commits

Reviewing files that changed from the base of the PR and between 7665d38 and 0585cea.

📒 Files selected for processing (4)
  • .opencode/PAI/Tools/GenerateSkillIndex.ts
  • .opencode/VoiceServer/install.sh
  • .opencode/VoiceServer/uninstall.sh
  • contrib/anthropic-max-bridge/plugins/anthropic-token-bridge.js
✅ Files skipped from review due to trivial changes (1)
  • .opencode/VoiceServer/uninstall.sh
🚧 Files skipped from review as they are similar to previous changes (1)
  • .opencode/PAI/Tools/GenerateSkillIndex.ts

.opencode/VoiceServer/install.sh
- ENV_FILE now matches the server's path resolution:
  ${OPENCODE_DIR:-${PAI_DIR:-$HOME/.opencode}}/.env (was $HOME/.env)
- API_KEY extraction uses grep -m1 to avoid concatenating multiple matching
  lines; inline-comment stripping now only removes trailing '#[^"']*$'
  (unquoted comments) so '#' inside a key value is preserved

.opencode/plugins/lib/refresh-manager.ts
.opencode/plugins/anthropic-token-bridge.js
contrib/anthropic-max-bridge/plugins/anthropic-token-bridge.js
- extractFromKeychain: destructure { stdout, stderr, exitCode } from
  execCommand and pass the real stderr into the error() call (was
  incorrectly passing stdout as stderr)
Copy link

@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: 6

🧹 Nitpick comments (2)
.opencode/plugins/anthropic-token-bridge.js (1)

480-483: Unhandled promise rejection in retry setTimeout.

The refreshAnthropicToken() call inside setTimeout returns a Promise that isn't handled. While the function has internal error handling, if it throws unexpectedly, this could result in an unhandled rejection warning.

🔧 Proposed fix to handle the promise
               } else {
                 fileLog("Session-start token refresh failed - will retry async", "error");
-                setTimeout(() => refreshAnthropicToken(), 3e4);
+                setTimeout(() => {
+                  refreshAnthropicToken().catch((err) => {
+                    fileLogError("Retry refresh failed", err);
+                  });
+                }, 3e4);
               }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.opencode/plugins/anthropic-token-bridge.js around lines 480 - 483, The
setTimeout callback calls refreshAnthropicToken() but ignores its returned
Promise, risking an unhandled rejection; update the retry to handle rejections
by invoking refreshAnthropicToken() and attaching a .catch handler (or use an
async IIFE with try/catch) inside the setTimeout callback so any thrown error is
logged via fileLog (e.g., "Session-start token refresh retry failed") and does
not produce an unhandled promise rejection; locate the existing setTimeout(...)
call where fileLog("Session-start token refresh failed - will retry async",
"error") is used and modify that callback to handle errors from
refreshAnthropicToken().
.opencode/plugins/lib/refresh-manager.ts (1)

235-244: Avoid external setup-token flow after a local write failure.

If valid Keychain tokens are found but updateAnthropicTokens(...) fails, the code falls through to setup-token generation/exchange. That adds avoidable external calls and does not resolve local persistence failure.

Proposed fix
 			if (expiresInSeconds > 0) {
 				const success = updateAnthropicTokens(
 					keychainTokens.accessToken,
 					keychainTokens.refreshToken,
 					expiresInSeconds,
 				);
 				if (success) {
 					info("Token refresh successful via Keychain");
 					return true;
 				}
+				error("Failed to persist Keychain tokens to auth.json");
+				return false;
 			}
 		}
 
 		// Strategy 2: Generate new setup token via Claude Code CLI
 		info("Keychain extraction failed, generating new setup token");

Also applies to: 247-253

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.opencode/plugins/lib/refresh-manager.ts around lines 235 - 244, When valid
Keychain tokens are found but updateAnthropicTokens(...) returns false,
short-circuit and stop the external setup-token flow instead of falling through
to generate/exchange a new token; specifically, in the branch that calls
updateAnthropicTokens(keychainTokens.accessToken, keychainTokens.refreshToken,
expiresInSeconds) (and the analogous block around the other call at 247-253),
detect the failure and return false (or otherwise abort the refresh) so we do
not trigger the external setup-token generation when a local persistence/update
failed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.opencode/plugins/lib/refresh-manager.ts:
- Around line 48-50: The child.close handler currently ignores the Node.js
signal parameter and treats a null exit code as success; update the callback
signature to include (exitCode: number | null, signal: string | null) and treat
any non-null signal as a failure by setting a non-zero exitCode and augmenting
stderr (or otherwise recording the signal) before calling settle({ stdout,
stderr, exitCode }); modify the handler where child.on("close", ...) is used
(and any callers like extractFromKeychain() that rely on settle) so
signal-terminated processes are not misclassified as successful.
- Around line 148-150: The timeout created with setTimeout using the
AbortController is only cleared on the success path; change the code so the
timeout variable is declared with let in the outer scope, wrap the fetch/await
logic in a try/finally, and call clearTimeout(timeout) in the finally block to
guarantee the timer is cleaned up even if fetch/AbortController throws;
reference the existing AbortController, timeout, clearTimeout and the fetch call
in refresh-manager.ts when making the change.

In @.opencode/VoiceServer/install.sh:
- Around line 85-91: The user-facing message incorrectly tells users to add the
ElevenLabs key to ~/.env while the script actually reads
${OPENCODE_DIR:-${PAI_DIR:-$HOME/.opencode}}/.env; update the echo text in the
ELEVENLABS_CONFIGURED conditional (the block referencing ELEVENLABS_CONFIGURED)
to instruct users to add their ELEVENLABS_API_KEY into the repository env file
the script reads (use the ${OPENCODE_DIR:-${PAI_DIR:-$HOME/.opencode}}/.env path
or an equivalent human-friendly description like "$OPENCODE_DIR/.env (defaults
to $HOME/.opencode/.env)") so the instructions match the path expression used
when sourcing the .env.
- Around line 147-154: The LaunchAgent plist written in install.sh does not
propagate OPENCODE_DIR or PAI_DIR, so at runtime server.ts can read a different
.env; update the plist EnvironmentVariables dict creation in install.sh to
include OPENCODE_DIR and PAI_DIR (if set) alongside HOME and PATH — e.g., when
composing the <dict> for EnvironmentVariables, add
<key>OPENCODE_DIR</key><string>${OPENCODE_DIR}</string> and
<key>PAI_DIR</key><string>${PAI_DIR}</string> (or omit/leave empty if unset),
ensuring these variables are injected into the same place where
${ELEVENLABS_PLIST_ENTRY} is included so the LaunchAgent uses the same config
paths the installer used.

In `@contrib/anthropic-max-bridge/plugins/anthropic-token-bridge.js`:
- Around line 324-345: The current logic treats any positive expiresInSeconds as
valid and rewrites auth.json even when the token is within the configured
refresh window; update the branch that computes expiresInSeconds (and the
matching block at the other similar spot) to check against REFRESH_THRESHOLD_MS
(or compare keychainTokens.expiresAt - Date.now() directly) so that tokens with
remaining lifetime <= REFRESH_THRESHOLD_MS are considered
expired/refresh-needed; only call
updateAnthropicTokens(keychainTokens.accessToken, keychainTokens.refreshToken,
expiresInSeconds) and return true when the remaining lifetime is strictly
greater than the refresh threshold.
- Around line 309-317: The cooldown timestamp is being set before the refresh
attempt, so failed refreshes still trigger the REFRESH_COOLDOWN_MS lock; change
the logic around lastRefreshAttempt and isRefreshing in the refresh flow (the
block using lastRefreshAttempt, isRefreshing and REFRESH_COOLDOWN_MS) so that
lastRefreshAttempt is only updated after a successful token refresh, and ensure
isRefreshing is reset on both success and failure (use a try/catch/finally or
equivalent around the refresh call). This prevents failed attempts from arming
the 5-minute cooldown while still preventing concurrent refreshes.

---

Nitpick comments:
In @.opencode/plugins/anthropic-token-bridge.js:
- Around line 480-483: The setTimeout callback calls refreshAnthropicToken() but
ignores its returned Promise, risking an unhandled rejection; update the retry
to handle rejections by invoking refreshAnthropicToken() and attaching a .catch
handler (or use an async IIFE with try/catch) inside the setTimeout callback so
any thrown error is logged via fileLog (e.g., "Session-start token refresh retry
failed") and does not produce an unhandled promise rejection; locate the
existing setTimeout(...) call where fileLog("Session-start token refresh failed
- will retry async", "error") is used and modify that callback to handle errors
from refreshAnthropicToken().

In @.opencode/plugins/lib/refresh-manager.ts:
- Around line 235-244: When valid Keychain tokens are found but
updateAnthropicTokens(...) returns false, short-circuit and stop the external
setup-token flow instead of falling through to generate/exchange a new token;
specifically, in the branch that calls
updateAnthropicTokens(keychainTokens.accessToken, keychainTokens.refreshToken,
expiresInSeconds) (and the analogous block around the other call at 247-253),
detect the failure and return false (or otherwise abort the refresh) so we do
not trigger the external setup-token generation when a local persistence/update
failed.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 37a533dc-1507-4fe0-b089-ac1ea4a0e544

📥 Commits

Reviewing files that changed from the base of the PR and between 0585cea and 023364a.

📒 Files selected for processing (4)
  • .opencode/VoiceServer/install.sh
  • .opencode/plugins/anthropic-token-bridge.js
  • .opencode/plugins/lib/refresh-manager.ts
  • contrib/anthropic-max-bridge/plugins/anthropic-token-bridge.js

Comment on lines +48 to +50
child.on("close", (exitCode: number | null) => {
settle({ stdout, stderr, exitCode: exitCode ?? 0 });
});
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's locate and examine the file
fd "refresh-manager.ts" --type f

Repository: Steffen025/pai-opencode

Length of output: 49


🌐 Web query:

Node.js child_process spawn close event code signal null documentation

💡 Result:

In Node.js child_process.spawn, the 'close' event is emitted when all stdio streams of the child process are closed. Its listener receives up to two arguments: code (number) and signal (string or null). The code parameter is the exit code if the child exited normally, otherwise null. The signal parameter is the string name of the signal that terminated the process (e.g., 'SIGTERM'), otherwise null. Exactly one of code or signal is non-null. When both code and signal are null in the 'close' event, it typically indicates the child process was killed abruptly (e.g., by SIGKILL from OOM killer) where Node.js could not retrieve the exit information, or there was an issue closing stdio streams. However, official docs show examples with signal non-null and code null for signal-terminated processes. This behavior matches the 'exit' event, which fires when the process ends (before stdio fully closes). The key difference: 'close' waits for stdio closure, useful when streams are piped/shared; 'exit' fires immediately on process termination. Example from official docs (v25.8.1): const grep = spawn('grep', ['ssh']); grep.on('close', (code, signal) => { console.log(child process terminated due to receipt of signal ${signal}); }); grep.kill('SIGHUP'); // Outputs: child process terminated due to receipt of signal SIGHUP (code=null) For normal exit (code=0, signal=null): ls.on('close', (code) => { console.log(child process exited with code ${code}); }); Use 'exit' if you only care about process termination; use 'close' when stdio handling matters.

Citations:


🏁 Script executed:

# Search for refresh-manager.ts in the repository
find . -name "refresh-manager.ts" -type f 2>/dev/null

Repository: Steffen025/pai-opencode

Length of output: 109


🏁 Script executed:

# If not found with find, try git ls-files
git ls-files | grep -i refresh

Repository: Steffen025/pai-opencode

Length of output: 190


🏁 Script executed:

# Examine the refresh-manager.ts file, particularly around lines 48-50
cat -n .opencode/plugins/lib/refresh-manager.ts | head -80 | tail -50

Repository: Steffen025/pai-opencode

Length of output: 1602


🏁 Script executed:

# Also check the execCommand function context (likely starting before line 48)
cat -n .opencode/plugins/lib/refresh-manager.ts | head -70 | tail -40

Repository: Steffen025/pai-opencode

Length of output: 1292


Handle signaled child exits as failures in execCommand.

The close event callback at line 48 only accepts exitCode: number | null and ignores the signal parameter that Node.js provides. When a process is terminated by a signal (e.g., SIGTERM), the code is null while signal contains the signal name. The current code's exitCode ?? 0 coalesces null to success, misclassifying signal terminations as successful command execution. This can mask actual failures in the security command invoked by extractFromKeychain().

Proposed fix
-		child.on("close", (exitCode: number | null) => {
-			settle({ stdout, stderr, exitCode: exitCode ?? 0 });
-		});
+		child.on("close", (exitCode: number | null, signal: NodeJS.Signals | null) => {
+			if (signal) {
+				settle({
+					stdout,
+					stderr: stderr || `terminated by signal: ${signal}`,
+					exitCode: 1,
+				});
+				return;
+			}
+			settle({ stdout, stderr, exitCode: exitCode ?? 1 });
+		});
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
child.on("close", (exitCode: number | null) => {
settle({ stdout, stderr, exitCode: exitCode ?? 0 });
});
child.on("close", (exitCode: number | null, signal: NodeJS.Signals | null) => {
if (signal) {
settle({
stdout,
stderr: stderr || `terminated by signal: ${signal}`,
exitCode: 1,
});
return;
}
settle({ stdout, stderr, exitCode: exitCode ?? 1 });
});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.opencode/plugins/lib/refresh-manager.ts around lines 48 - 50, The
child.close handler currently ignores the Node.js signal parameter and treats a
null exit code as success; update the callback signature to include (exitCode:
number | null, signal: string | null) and treat any non-null signal as a failure
by setting a non-zero exitCode and augmenting stderr (or otherwise recording the
signal) before calling settle({ stdout, stderr, exitCode }); modify the handler
where child.on("close", ...) is used (and any callers like extractFromKeychain()
that rely on settle) so signal-terminated processes are not misclassified as
successful.

Comment on lines +148 to +150
const controller = new AbortController();
const timeout = setTimeout(() => controller.abort(), 10000); // 10 second timeout

Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, let's verify the file exists and examine its structure
fd refresh-manager.ts

Repository: Steffen025/pai-opencode

Length of output: 49


🏁 Script executed:

# Read the file around lines 148-150 and 162
wc -l .opencode/plugins/lib/refresh-manager.ts

Repository: Steffen025/pai-opencode

Length of output: 111


🏁 Script executed:

# Read specific sections of the file
sed -n '140,170p' .opencode/plugins/lib/refresh-manager.ts

Repository: Steffen025/pai-opencode

Length of output: 970


🏁 Script executed:

# Get more context around the function to understand the full structure
sed -n '130,180p' .opencode/plugins/lib/refresh-manager.ts

Repository: Steffen025/pai-opencode

Length of output: 1490


🏁 Script executed:

# Get the complete function including catch and finally blocks
sed -n '143,180p' .opencode/plugins/lib/refresh-manager.ts

Repository: Steffen025/pai-opencode

Length of output: 1192


🏁 Script executed:

# Get more lines to see the complete function with catch/finally
sed -n '143,200p' .opencode/plugins/lib/refresh-manager.ts

Repository: Steffen025/pai-opencode

Length of output: 1651


🏁 Script executed:

# Verify the line 162 reference - it should be the clearTimeout line
sed -n '162p' .opencode/plugins/lib/refresh-manager.ts

Repository: Steffen025/pai-opencode

Length of output: 91


🏁 Script executed:

# Search for calls to exchangeSetupToken to see where it's used
rg -n "exchangeSetupToken" .opencode/plugins/lib/refresh-manager.ts

Repository: Steffen025/pai-opencode

Length of output: 219


🏁 Script executed:

# Check if this file is part of an init hook or exports functions used in init
rg -n "export\|init\|setup" .opencode/plugins/lib/refresh-manager.ts | head -20

Repository: Steffen025/pai-opencode

Length of output: 49


🏁 Script executed:

# Check the structure around line 256 where exchangeSetupToken is called
sed -n '240,270p' .opencode/plugins/lib/refresh-manager.ts

Repository: Steffen025/pai-opencode

Length of output: 903


🏁 Script executed:

# Check the file header and overall structure to understand if this is an init hook
head -50 .opencode/plugins/lib/refresh-manager.ts

Repository: Steffen025/pai-opencode

Length of output: 1404


Move clearTimeout(timeout) to a finally block to ensure cleanup on both success and error paths.

If the fetch call throws before resolving, the timeout remains pending for the full 10 seconds until it fires. The current code only clears it after successful completion. Update the timeout variable scope declaration and add a finally block to guarantee cleanup:

async function exchangeSetupToken(setupToken: string): Promise<OAuthTokens | null> {
+	let timeout: ReturnType<typeof setTimeout> | undefined;
	try {
		info("Exchanging setup token for OAuth credentials");
		
		// AbortController timeout to prevent hanging
		const controller = new AbortController();
-		const timeout = setTimeout(() => controller.abort(), 10000); // 10 second timeout
+		timeout = setTimeout(() => controller.abort(), 10000); // 10 second timeout
		
		const response = await fetch("https://api.anthropic.com/v1/oauth/setup_token/exchange", {
			method: "POST",
			headers: {
				"Content-Type": "application/json",
				Authorization: `Bearer ${setupToken}`,
				"anthropic-version": "2023-06-01",
			},
			body: JSON.stringify({ grant_type: "setup_token" }),
			signal: controller.signal,
		});
		
-		clearTimeout(timeout);
-
		if (!response.ok) {
			error("Token exchange failed", { status: response.status });
			return null;
		}
		return {
			accessToken: data.access_token,
			refreshToken: data.refresh_token,
			expiresIn: data.expires_in ?? 28800,
		};
	} catch (err) {
		error("Exception exchanging setup token", { error: String(err) });
		return null;
+	} finally {
+		if (timeout) clearTimeout(timeout);
	}
}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const controller = new AbortController();
const timeout = setTimeout(() => controller.abort(), 10000); // 10 second timeout
async function exchangeSetupToken(setupToken: string): Promise<OAuthTokens | null> {
let timeout: ReturnType<typeof setTimeout> | undefined;
try {
info("Exchanging setup token for OAuth credentials");
// AbortController timeout to prevent hanging
const controller = new AbortController();
timeout = setTimeout(() => controller.abort(), 10000); // 10 second timeout
const response = await fetch("https://api.anthropic.com/v1/oauth/setup_token/exchange", {
method: "POST",
headers: {
"Content-Type": "application/json",
Authorization: `Bearer ${setupToken}`,
"anthropic-version": "2023-06-01",
},
body: JSON.stringify({ grant_type: "setup_token" }),
signal: controller.signal,
});
if (!response.ok) {
error("Token exchange failed", { status: response.status });
return null;
}
const data = await response.json() as any;
return {
accessToken: data.access_token,
refreshToken: data.refresh_token,
expiresIn: data.expires_in ?? 28800,
};
} catch (err) {
error("Exception exchanging setup token", { error: String(err) });
return null;
} finally {
if (timeout) clearTimeout(timeout);
}
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.opencode/plugins/lib/refresh-manager.ts around lines 148 - 150, The timeout
created with setTimeout using the AbortController is only cleared on the success
path; change the code so the timeout variable is declared with let in the outer
scope, wrap the fetch/await logic in a try/finally, and call
clearTimeout(timeout) in the finally block to guarantee the timer is cleaned up
even if fetch/AbortController throws; reference the existing AbortController,
timeout, clearTimeout and the fetch call in refresh-manager.ts when making the
change.

Comment on lines +85 to +91
if [ "$ELEVENLABS_CONFIGURED" = false ]; then
echo
echo "To enable AI voices, add your ElevenLabs API key to ~/.env:"
echo " echo 'ELEVENLABS_API_KEY=your_api_key_here' >> ~/.env"
echo " Get a free key at: https://elevenlabs.io"
echo
fi
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

User guidance references wrong path.

Line 87-88 instructs users to add the API key to ~/.env, but the script actually reads from ${OPENCODE_DIR:-${PAI_DIR:-$HOME/.opencode}}/.env (line 27). Users following these instructions will add the key to the wrong file.

Proposed fix
 if [ "$ELEVENLABS_CONFIGURED" = false ]; then
     echo
-    echo "To enable AI voices, add your ElevenLabs API key to ~/.env:"
-    echo "  echo 'ELEVENLABS_API_KEY=your_api_key_here' >> ~/.env"
+    echo "To enable AI voices, add your ElevenLabs API key to $ENV_FILE:"
+    echo "  echo 'ELEVENLABS_API_KEY=your_api_key_here' >> \"$ENV_FILE\""
     echo "  Get a free key at: https://elevenlabs.io"
     echo
 fi
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if [ "$ELEVENLABS_CONFIGURED" = false ]; then
echo
echo "To enable AI voices, add your ElevenLabs API key to ~/.env:"
echo " echo 'ELEVENLABS_API_KEY=your_api_key_here' >> ~/.env"
echo " Get a free key at: https://elevenlabs.io"
echo
fi
if [ "$ELEVENLABS_CONFIGURED" = false ]; then
echo
echo "To enable AI voices, add your ElevenLabs API key to $ENV_FILE:"
echo " echo 'ELEVENLABS_API_KEY=your_api_key_here' >> \"$ENV_FILE\""
echo " Get a free key at: https://elevenlabs.io"
echo
fi
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.opencode/VoiceServer/install.sh around lines 85 - 91, The user-facing
message incorrectly tells users to add the ElevenLabs key to ~/.env while the
script actually reads ${OPENCODE_DIR:-${PAI_DIR:-$HOME/.opencode}}/.env; update
the echo text in the ELEVENLABS_CONFIGURED conditional (the block referencing
ELEVENLABS_CONFIGURED) to instruct users to add their ELEVENLABS_API_KEY into
the repository env file the script reads (use the
${OPENCODE_DIR:-${PAI_DIR:-$HOME/.opencode}}/.env path or an equivalent
human-friendly description like "$OPENCODE_DIR/.env (defaults to
$HOME/.opencode/.env)") so the instructions match the path expression used when
sourcing the .env.

Comment on lines +147 to +154
<key>EnvironmentVariables</key>
<dict>
<key>HOME</key>
<string>${HOME}</string>
<key>PATH</key>
<string>/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:${HOME}/.bun/bin</string>
${ELEVENLABS_PLIST_ENTRY}
</dict>
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Environment variables not propagated for custom config paths.

If the user has OPENCODE_DIR or PAI_DIR set when running install.sh, the installer reads the .env from that custom path. However, these variables aren't included in the LaunchAgent's EnvironmentVariables, so server.ts will default to ~/.opencode/.env at runtime—potentially a different file than what the installer checked.

Proposed fix: propagate custom config dir to the LaunchAgent
     <key>EnvironmentVariables</key>
     <dict>
         <key>HOME</key>
         <string>${HOME}</string>
         <key>PATH</key>
         <string>/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:${HOME}/.bun/bin</string>
+        <key>OPENCODE_DIR</key>
+        <string>${OPENCODE_DIR:-${PAI_DIR:-${HOME}/.opencode}}</string>
 ${ELEVENLABS_PLIST_ENTRY}
     </dict>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.opencode/VoiceServer/install.sh around lines 147 - 154, The LaunchAgent
plist written in install.sh does not propagate OPENCODE_DIR or PAI_DIR, so at
runtime server.ts can read a different .env; update the plist
EnvironmentVariables dict creation in install.sh to include OPENCODE_DIR and
PAI_DIR (if set) alongside HOME and PATH — e.g., when composing the <dict> for
EnvironmentVariables, add
<key>OPENCODE_DIR</key><string>${OPENCODE_DIR}</string> and
<key>PAI_DIR</key><string>${PAI_DIR}</string> (or omit/leave empty if unset),
ensuring these variables are injected into the same place where
${ELEVENLABS_PLIST_ENTRY} is included so the LaunchAgent uses the same config
paths the installer used.

Comment on lines +309 to +317
const timeSinceLastAttempt = Date.now() - lastRefreshAttempt;
if (timeSinceLastAttempt < REFRESH_COOLDOWN_MS) {
info("Refresh on cooldown", {
minutesRemaining: Math.ceil((REFRESH_COOLDOWN_MS - timeSinceLastAttempt) / 60000)
});
return false;
}
isRefreshing = true;
lastRefreshAttempt = Date.now();
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Failed refreshes currently arm the 5-minute cooldown.

lastRefreshAttempt is updated before the attempt runs, so a failed refresh still blocks subsequent attempts for 5 minutes. That makes the retry scheduled at Line 486 a no-op.

🔧 Suggested fix
   isRefreshing = true;
-  lastRefreshAttempt = Date.now();
   try {
     info("Starting token refresh process");
     const keychainTokens = await extractFromKeychain();
     if (keychainTokens) {
@@
       if (expiresInSeconds > 0) {
         const success2 = updateAnthropicTokens(keychainTokens.accessToken, keychainTokens.refreshToken, expiresInSeconds);
         if (success2) {
+          lastRefreshAttempt = Date.now();
           info("Token refresh successful via Keychain");
           return true;
         }
       }
@@
     const success = updateAnthropicTokens(oauthTokens.accessToken, oauthTokens.refreshToken, oauthTokens.expiresIn);
     if (success) {
+      lastRefreshAttempt = Date.now();
       info("Token refresh successful via setup token exchange");
       return true;
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@contrib/anthropic-max-bridge/plugins/anthropic-token-bridge.js` around lines
309 - 317, The cooldown timestamp is being set before the refresh attempt, so
failed refreshes still trigger the REFRESH_COOLDOWN_MS lock; change the logic
around lastRefreshAttempt and isRefreshing in the refresh flow (the block using
lastRefreshAttempt, isRefreshing and REFRESH_COOLDOWN_MS) so that
lastRefreshAttempt is only updated after a successful token refresh, and ensure
isRefreshing is reset on both success and failure (use a try/catch/finally or
equivalent around the refresh call). This prevents failed attempts from arming
the 5-minute cooldown while still preventing concurrent refreshes.

Comment on lines +324 to +345
// Calculate expiresInSeconds - only use Keychain expiry if it's valid and in the future
let expiresInSeconds;
if (keychainTokens.expiresAt === undefined || keychainTokens.expiresAt === null) {
// Legacy: no expiry in Keychain, use 8h default
expiresInSeconds = 28800;
} else if (keychainTokens.expiresAt > Date.now()) {
// Valid future expiry
expiresInSeconds = Math.round((keychainTokens.expiresAt - Date.now()) / 1000);
} else {
// Expired - don't use this token, fall through to setup-token refresh
info("Keychain token is expired, falling through to setup-token exchange");
expiresInSeconds = 0; // Will cause validation to fail, forcing setup-token path
}

// Only update if we have a valid expiry (not expired)
if (expiresInSeconds > 0) {
const success2 = updateAnthropicTokens(keychainTokens.accessToken, keychainTokens.refreshToken, expiresInSeconds);
if (success2) {
info("Token refresh successful via Keychain");
return true;
}
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Don’t short-circuit on Keychain tokens that are already inside the refresh window.

checkAnthropicToken() treats anything under REFRESH_THRESHOLD_MS as needing refresh, but both of these branches return success for any positive expiresInSeconds. That just copies a near-expiry token back into auth.json, and the session-start path exits before the fallback refresh runs.

🔧 Suggested fix
-      if (expiresInSeconds > 0) {
+      if (expiresInSeconds > 0) {
         const success2 = updateAnthropicTokens(keychainTokens.accessToken, keychainTokens.refreshToken, expiresInSeconds);
         if (success2) {
-          info("Token refresh successful via Keychain");
-          return true;
+          if (expiresInSeconds * 1000 > REFRESH_THRESHOLD_MS) {
+            info("Token refresh successful via Keychain");
+            return true;
+          }
+          info("Keychain token synced but is inside refresh threshold, continuing with setup-token exchange");
         }
       }
-            if (expiresInSeconds > 0) {
+            if (expiresInSeconds > 0) {
               const synced = updateAnthropicTokens(accessToken, refreshToken, expiresInSeconds);
               if (synced) {
                 const hoursRemaining = Math.floor(expiresInSeconds / 3600);
                 fileLog(`Keychain→auth.json sync successful, token valid for ${hoursRemaining}h`, "info");
-                return; // Done — fresh token written, no further refresh needed
+                if (expiresInSeconds * 1000 > REFRESH_THRESHOLD_MS) {
+                  return; // Done — fresh token written, no further refresh needed
+                }
+                fileLog("Keychain token is inside refresh threshold, continuing with fallback refresh", "warn");
               } else {
                 fileLog("Keychain→auth.json sync failed (write error), falling back to refresh", "error");
               }
             } else {

Also applies to: 451-456

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@contrib/anthropic-max-bridge/plugins/anthropic-token-bridge.js` around lines
324 - 345, The current logic treats any positive expiresInSeconds as valid and
rewrites auth.json even when the token is within the configured refresh window;
update the branch that computes expiresInSeconds (and the matching block at the
other similar spot) to check against REFRESH_THRESHOLD_MS (or compare
keychainTokens.expiresAt - Date.now() directly) so that tokens with remaining
lifetime <= REFRESH_THRESHOLD_MS are considered expired/refresh-needed; only
call updateAnthropicTokens(keychainTokens.accessToken,
keychainTokens.refreshToken, expiresInSeconds) and return true when the
remaining lifetime is strictly greater than the refresh threshold.

@Steffen025 Steffen025 merged commit 8fe6e35 into main Mar 22, 2026
9 checks passed
@Steffen025 Steffen025 deleted the release/oauth-anthropic-max-v2 branch March 22, 2026 02:16
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