docs: rewrite AGENTS.md with verified repo-specific guidance#20
Conversation
Replaced the previous generic Technical Context / Key Constraints structure with high-signal facts an agent would otherwise miss: - Exact developer commands (bun install/test/build) and the absence of any linter/formatter/typecheck scripts - Full architecture listing including session.ts and version.ts - Interceptor recursion guard (inEmit), Bun AsyncLocalStorage caveat, gRPC-only trace constraint, and silent-inactive behavior - Complete service.name resolution chain and config file placeholders - Test location, runner, and coverage scope - CI publish workflow trigger and steps - Design spec references including 011-align-bat-otel
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 40 minutes and 54 seconds. ⌛ 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. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces AGENTS.md, a comprehensive documentation file detailing the project's architecture, tech stack, and configuration. Feedback highlights two inaccuracies in the documentation: first, the claim that Bun's AsyncLocalStorage is broken is outdated and using a global variable for session tracking introduces race conditions; second, the plugin does not go inactive 'silently' but logs an informational message when the endpoint is missing.
|
|
||
| - **Cannot modify OpenCode source** — integration via npm plugin `event` hook only | ||
| - **Monkey-patch `process.stderr.write`** — interceptor has a recursion guard (`inEmit` flag); if emit callback writes to stderr it is silently skipped | ||
| - **Session tracking uses a module-level Map, not AsyncLocalStorage** — Bun's AsyncLocalStorage is broken; `session.ts` tracks active session via a plain variable |
There was a problem hiding this comment.
The documentation justifies using a module-level variable for session tracking by claiming Bun's AsyncLocalStorage is broken. However, AsyncLocalStorage is stable in modern Bun versions, and using a global variable for activeSessionId creates a race condition in concurrent environments. If multiple sessions are active or interleaved (common in agentic frameworks), logs from one session will be incorrectly tagged with another session's traceId. This architectural flaw should be corrected in the implementation rather than documented as a constraint.
There was a problem hiding this comment.
Fixed in 2319d5c. Removed the claim that Bun AsyncLocalStorage is "broken" — the doc now describes the actual design choice (module-level Map + plain activeSessionId variable) and notes the concrete consequence: concurrent session interleaving will mis-tag logs. Whether to migrate to AsyncLocalStorage is a code decision for a separate PR.
| - **Monkey-patch `process.stderr.write`** — interceptor has a recursion guard (`inEmit` flag); if emit callback writes to stderr it is silently skipped | ||
| - **Session tracking uses a module-level Map, not AsyncLocalStorage** — Bun's AsyncLocalStorage is broken; `session.ts` tracks active session via a plain variable | ||
| - **Trace exporter only supports gRPC** — if a non-gRPC traces protocol is configured, trace export is disabled with a warning | ||
| - **Plugin goes inactive silently** if `OTEL_EXPORTER_OTLP_LOGS_ENDPOINT` is not set (no interceptor installed) |
There was a problem hiding this comment.
The statement that the plugin goes inactive "silently" is inaccurate. According to src/index.ts (line 59), the plugin logs an informational message ("No OTEL logs endpoint configured — plugin inactive") via the OpenCode client when the endpoint is missing. It would be more accurate to state that it logs a status message and remains inactive.
There was a problem hiding this comment.
Fixed in 2319d5c. Changed "silently inactive" to: "logs a status message via the app logger but does not install the interceptor" — verified against src/index.ts:59.
…ior wording - Remove claim that Bun AsyncLocalStorage is 'broken'; describe the actual design choice (module-level Map + plain variable) and note the concurrent session interleaving consequence - Replace 'silently inactive' with accurate description: logs a status message via app logger but does not install the interceptor
1 participant
Summary
Rewrites
AGENTS.mdfrom a generic technical context dump into a compact, high-signal instruction file that helps future agent sessions avoid mistakes and ramp up quickly.Changes
buncommands plus explicit note that no linter/formatter/typecheck existspackage.json(no config file),noEmit: truein tsconfigsession.tsandversion.tsmodules with one-line role descriptionsinEmitrecursion guard, Bun AsyncLocalStorage caveat, gRPC-only trace limitation, silent-inactive behaviorservice.nameresolution chain, config file${ENV_VAR}placeholder supportv*tags) and workflow steps011-align-bat-otelreference andCLAUDE.mdownership boundary notepackage.json), env var table (duplicates README), generic framingTest Plan
bun testandbun run buildare unaffected