Improve Codex branding and prompt fetch resilience#30
Conversation
|
Warning Rate limit exceeded@riatzukiza has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 7 minutes and 11 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (6)
📒 Files selected for processing (9)
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughThe PR renames the plugin identifier from "openai-codex-plugin" to "openhax/codex" throughout the codebase. It modifies the logger to suppress logs uniformly when logging is disabled, removing a prior test-environment exception. It introduces multi-URL fetch with per-URL fallback logic for OpenCode codex prompts, supporting conditional requests and aggregated error handling. Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant Logger
participant Console
rect rgb(240, 248, 255)
Note over Logger: OLD BEHAVIOR
App->>Logger: logToConsole(msg, shouldLog=false)
alt IS_TEST_ENV = true
Logger-->>App: (returns early, logs suppressed)
else IS_TEST_ENV = false
Logger->>Console: write(msg)
end
end
rect rgb(240, 245, 240)
Note over Logger: NEW BEHAVIOR
App->>Logger: logToConsole(msg, shouldLog=false)
alt shouldLog = false
Logger-->>App: (returns early, logs always suppressed)
else shouldLog = true
Logger->>Console: write(msg)
end
end
sequenceDiagram
participant Caller
participant OpenCodeFetch
participant GitHubURL1
participant GitHubURL2
participant Cache
Caller->>OpenCodeFetch: fetch codex content
rect rgb(240, 248, 255)
Note over OpenCodeFetch: Iterate URLs
OpenCodeFetch->>GitHubURL1: GET with If-None-Match (etag from cache)
alt 200 OK
GitHubURL1-->>OpenCodeFetch: new content
OpenCodeFetch->>Cache: write content + sourceUrl + timestamps
OpenCodeFetch-->>Caller: return content
else 304 Not Modified
OpenCodeFetch->>Cache: fetch cached content
OpenCodeFetch-->>Caller: return cached content
else fetch fails or non-OK
OpenCodeFetch->>GitHubURL2: retry with next URL
alt 200 OK
GitHubURL2-->>OpenCodeFetch: new content
OpenCodeFetch->>Cache: write content + sourceUrl
OpenCodeFetch-->>Caller: return content
else all URLs exhausted
OpenCodeFetch->>Cache: try fallback to any cached content
alt cache exists
OpenCodeFetch-->>Caller: return cached content (log error)
else no cache
OpenCodeFetch-->>Caller: throw aggregated error
end
end
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Areas requiring extra attention:
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (6)
docs/configuration.mdis excluded by none and included by nonedocs/development/ARCHITECTURE.mdis excluded by none and included by nonedocs/development/TESTING.mdis excluded by none and included by nonespec/handle-missing-codex-prompt-warming.mdis excluded by none and included by nonespec/opencode-prompt-cache-404.mdis excluded by none and included by nonespec/plugin-name-rename.mdis excluded by none and included by none
📒 Files selected for processing (9)
lib/constants.ts(1 hunks)lib/logger.ts(1 hunks)lib/prompts/opencode-codex.ts(2 hunks)lib/request/response-handler.ts(1 hunks)test/auth.test.ts(7 hunks)test/constants.test.ts(1 hunks)test/logger.test.ts(3 hunks)test/prompts-codex.test.ts(2 hunks)test/prompts-opencode-codex.test.ts(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
test/constants.test.ts (1)
lib/constants.ts (1)
PLUGIN_NAME(7-7)
test/prompts-opencode-codex.test.ts (2)
lib/cache/session-cache.ts (1)
openCodePromptCache(77-77)lib/prompts/opencode-codex.ts (2)
getOpenCodeCodexPrompt(34-132)getCachedPromptPrefix(139-149)
lib/prompts/opencode-codex.ts (2)
lib/cache/session-cache.ts (1)
openCodePromptCache(77-77)lib/logger.ts (1)
logError(100-102)
🔇 Additional comments (8)
lib/constants.ts (1)
7-7: Plugin identity rename looks correct
PLUGIN_NAMEnow matches the newopenhax/codexbranding and stays consistent with logger usage and tests; no further changes needed here.test/auth.test.ts (1)
202-315: Auth logging expectations align with new prefixThe updated
console.errorexpectations correctly reflect the[openhax/codex]prefix and preserve the existing payload formats and failure semantics for auth flows. No issues from the test side.test/logger.test.ts (1)
102-180: Logger console prefix tests match runtime behaviorThe updated expectations for
logWarnand overflow/persist warnings correctly reflect the[openhax/codex]prefix and the logger’s console formatting. The tests still exercise the key behaviors (no info mirroring, warn on persistence/queue issues) without introducing new coupling.test/constants.test.ts (1)
18-21: Constants test correctly tracks renamed plugin identityThe expectation for
PLUGIN_NAMEnow matches the implementation ("openhax/codex"), keeping the branding and tests in sync.test/prompts-codex.test.ts (1)
137-250: Codex prompt fetch tests remain accurate after prefix changeThe failure and fallback expectations now use the
[openhax/codex]tag but still assert the same behaviors: logging HTTP failures, falling back to cached or bundled instructions, and leaving shapes of messages and arguments unchanged. Looks good.lib/logger.ts (1)
138-159: Uniform console logging guard looks correctRemoving the test-environment escape hatch so that
!shouldLogalways returns early makes debug/info suppression consistent across environments, while still allowing warn/error through. This matches the updated tests and shouldn’t affect production behavior.test/prompts-opencode-codex.test.ts (1)
1-374: OpenCode Codex prompt tests comprehensively cover the new caching and fallback logicThis suite does a solid job of exercising the main and edge paths: session vs file cache, TTL handling, 304 conditional requests, network/non-200 failures, multi-URL (dev → main) fallback with
sourceUrlpersistence, cache directory creation, missing/malformed metadata, and cached-prefix reads. The mocked paths and expectations line up with thegetOpenCodeCodexPrompt/getCachedPromptPrefiximplementation, and I don’t see behavioral holes here.lib/prompts/opencode-codex.ts (1)
15-25: No blockers on URL ordering andsourceUrltracking; dev→main fallback looks correct.Using
OPENCODE_CODEX_URLSto prefer thedevbranch and fall back tomain, plus persistingsourceUrlinOpenCodeCacheMeta, cleanly separates which ETag belongs to which URL. The(!cachedMeta.sourceUrl || cachedMeta.sourceUrl === url)guard keeps conditional requests aligned with the stored origin while remaining backward‑compatible with legacy metadata that lackedsourceUrl. This all looks consistent with the rest of the function.
71bfa3d to
a98014d
Compare
Summary
openhax/codexand align log prefixesTesting