feat(ci): add release workflow and clean up plugin packaging#1
Merged
digitalfiz merged 7 commits intomainfrom Mar 11, 2026
Merged
feat(ci): add release workflow and clean up plugin packaging#1digitalfiz merged 7 commits intomainfrom
digitalfiz merged 7 commits intomainfrom
Conversation
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a release GitHub Actions workflow; publishes TypeScript sources from Changes
Sequence DiagramsequenceDiagram
participant App as Application
participant LoadConfig as loadConfig()
participant Env as Environment
participant OTEL as OTEL SDK
participant Exporter as OTLP Exporter
App->>LoadConfig: call loadConfig()
LoadConfig->>Env: read OPENCODE_OTLP_HEADERS
LoadConfig->>Env: read OPENCODE_RESOURCE_ATTRIBUTES
LoadConfig->>Env: set OTEL_EXPORTER_OTLP_HEADERS (if present)
LoadConfig->>Env: set OTEL_RESOURCE_ATTRIBUTES (if present)
LoadConfig-->>App: return PluginConfig (otlpHeaders, resourceAttributes)
App->>OTEL: initialize SDK (reads OTEL_* env)
OTEL->>Exporter: create exporter (no custom headers)
Exporter-->>OTEL: exporter ready
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
- Add .github/workflows/release.yml — publishes to npm on v* tag push - Remove root index.ts re-export; point package.json main/module at src/index.ts - Add files field to package.json to limit published content to src/ - Add version 0.0.1 to package.json - Rename OPENCODE_OTLP_HEADERS and OPENCODE_RESOURCE_ATTRIBUTES from bare OTEL_* names; loadConfig copies them to the standard OTEL_* vars before SDK init - Remove parseHeaders from otel.ts (SDK now reads OTEL_EXPORTER_OTLP_HEADERS natively) - Update tests and docs to reflect all changes
digitalfiz
force-pushed
the
feat/release-workflow-and-cleanup
branch
from
03147f8 to
8f40e61
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.github/workflows/release.yml (1)
1-37: Consider adding tag-version validation and explicit permissions.The workflow looks solid overall. A few recommendations:
Tag-version mismatch risk: The workflow publishes whatever version is in
package.json, regardless of the tag. If someone pushesv1.2.3butpackage.jsonsays0.0.1, npm will publish0.0.1.Explicit permissions: Adding a
permissionsblock follows the principle of least privilege.🔧 Suggested improvements
on: push: tags: - "v*" +permissions: + contents: read + jobs: release: name: Publish to npm runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 + - name: Validate tag matches package version + run: | + TAG_VERSION="${GITHUB_REF#refs/tags/v}" + PKG_VERSION=$(jq -r .version package.json) + if [ "$TAG_VERSION" != "$PKG_VERSION" ]; then + echo "Tag version ($TAG_VERSION) does not match package.json version ($PKG_VERSION)" + exit 1 + fi + - uses: actions/setup-node@v4🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yml around lines 1 - 37, Add validation to ensure the pushed git tag matches package.json version before running the release job and add an explicit permissions block to limit token scopes; specifically, in the release job (job name "release") add an early step that reads the tag from github.ref and compares it to the version field from package.json (fail the workflow if mismatch) prior to the "Publish to npm" step (which runs npm publish), and add a top-level permissions block granting only the minimal permissions needed (e.g., contents: read and packages: write) and ensure NODE_AUTH_TOKEN continues to be provided via secrets for the npm publish step.tests/config.test.ts (1)
86-102: LGTM! Good test coverage for the new propagation behavior.The tests correctly validate the
OPENCODE_*→OTEL_*environment variable propagation.Consider adding a symmetric test for
OTEL_RESOURCE_ATTRIBUTESstaying undefined whenOPENCODE_RESOURCE_ATTRIBUTESis unset, similar to the test at lines 98-102:Optional test addition
test("does not set OTEL_EXPORTER_OTLP_HEADERS when OPENCODE_OTLP_HEADERS is unset", () => { delete process.env["OPENCODE_OTLP_HEADERS"] loadConfig() expect(process.env["OTEL_EXPORTER_OTLP_HEADERS"]).toBeUndefined() }) + + test("does not set OTEL_RESOURCE_ATTRIBUTES when OPENCODE_RESOURCE_ATTRIBUTES is unset", () => { + delete process.env["OPENCODE_RESOURCE_ATTRIBUTES"] + loadConfig() + expect(process.env["OTEL_RESOURCE_ATTRIBUTES"]).toBeUndefined() + })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/config.test.ts` around lines 86 - 102, Add a symmetric unit test that verifies OTEL_RESOURCE_ATTRIBUTES remains unset when OPENCODE_RESOURCE_ATTRIBUTES is not present: in tests/config.test.ts add a test similar to "does not set OTEL_EXPORTER_OTLP_HEADERS when OPENCODE_OTLP_HEADERS is unset" but for OPENCODE_RESOURCE_ATTRIBUTES/OTEL_RESOURCE_ATTRIBUTES, calling loadConfig() and asserting process.env["OTEL_RESOURCE_ATTRIBUTES"] is undefined; reference the existing test names and the loadConfig() helper to match style and placement.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/release.yml:
- Around line 1-37: Add validation to ensure the pushed git tag matches
package.json version before running the release job and add an explicit
permissions block to limit token scopes; specifically, in the release job (job
name "release") add an early step that reads the tag from github.ref and
compares it to the version field from package.json (fail the workflow if
mismatch) prior to the "Publish to npm" step (which runs npm publish), and add a
top-level permissions block granting only the minimal permissions needed (e.g.,
contents: read and packages: write) and ensure NODE_AUTH_TOKEN continues to be
provided via secrets for the npm publish step.
In `@tests/config.test.ts`:
- Around line 86-102: Add a symmetric unit test that verifies
OTEL_RESOURCE_ATTRIBUTES remains unset when OPENCODE_RESOURCE_ATTRIBUTES is not
present: in tests/config.test.ts add a test similar to "does not set
OTEL_EXPORTER_OTLP_HEADERS when OPENCODE_OTLP_HEADERS is unset" but for
OPENCODE_RESOURCE_ATTRIBUTES/OTEL_RESOURCE_ATTRIBUTES, calling loadConfig() and
asserting process.env["OTEL_RESOURCE_ATTRIBUTES"] is undefined; reference the
existing test names and the loadConfig() helper to match style and placement.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: db6ada2f-b35b-4bf5-a18c-0c1898a44b46
📒 Files selected for processing (10)
.github/workflows/release.ymlAGENTS.mdCONTRIBUTING.mdREADME.mdindex.tspackage.jsonsrc/config.tssrc/otel.tstests/config.test.tstests/otel.test.ts
💤 Files with no reviewable changes (1)
- index.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/config.test.ts`:
- Around line 86-102: Add regression tests in tests/config.test.ts using the
existing loadConfig helper to cover preexisting OTEL_* env behavior: (1) set
process.env["OTEL_EXPORTER_OTLP_HEADERS"] and
process.env["OTEL_RESOURCE_ATTRIBUTES"] to known values, ensure OPENCODE_* vars
are deleted, call loadConfig(), and assert those OTEL_* values remain unchanged;
(2) additionally verify that when OPENCODE_OTLP_HEADERS or
OPENCODE_RESOURCE_ATTRIBUTES are set, loadConfig() overwrites
OTEL_EXPORTER_OTLP_HEADERS and OTEL_RESOURCE_ATTRIBUTES respectively; use the
same test names (or new ones) near the current tests so they exercise
loadConfig() and avoid leaking env between tests by cleaning up/deleting the env
vars after each assertion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 807e273b-0499-4ecc-b7c4-607374a2de7e
📒 Files selected for processing (9)
.github/workflows/release.ymlAGENTS.mdCONTRIBUTING.mdREADME.mdpackage.jsonsrc/config.tssrc/otel.tstests/config.test.tstests/otel.test.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- .github/workflows/release.yml
- CONTRIBUTING.md
- package.json
- src/otel.ts
- README.md
- src/config.ts
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/probe.ts (1)
31-33:⚠️ Potential issue | 🟡 MinorMissing
socket.destroy()in error handler causes potential resource leak.The timeout and success handlers both call
socket.destroy(), but the error handler does not. While the socket may be in a failed state, explicitly destroying it ensures proper cleanup and prevents the timeout handler from firing afterward.🔧 Proposed fix
socket.on("error", (err) => { + socket.destroy() resolve({ ok: false, ms: Date.now() - start, error: err.message }) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/probe.ts` around lines 31 - 33, In the socket error handler inside probe.ts (the socket.on("error", (err) => { ... }) block), call socket.destroy() before resolving to ensure the socket is cleaned up and prevent the timeout handler from firing later; update the handler to destroy the socket then resolve({ ok: false, ms: Date.now() - start, error: err.message }) (ensure the same resolution behavior as the timeout and success handlers).src/util.ts (1)
16-21:⚠️ Potential issue | 🟠 MajorDo not evict when updating an existing key.
At capacity, this deletes the oldest entry even when
keyis already present. That drops an unrelated pending item and can permanently shrink the map belowMAX_PENDING. Only evict on true inserts.Proposed fix
export function setBoundedMap<K, V>(map: Map<K, V>, key: K, value: V) { - if (map.size >= MAX_PENDING) { + if (!map.has(key) && map.size >= MAX_PENDING) { const [firstKey] = map.keys() if (firstKey !== undefined) map.delete(firstKey) } map.set(key, value) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/util.ts` around lines 16 - 21, The setBoundedMap function currently evicts the oldest entry when map.size >= MAX_PENDING even if the provided key already exists; change logic in setBoundedMap to first check whether map.has(key) and only perform eviction when the key is not present and the map is at capacity (i.e., if (!map.has(key) && map.size >= MAX_PENDING) { evict oldest }), then call map.set(key, value) as before; reference the setBoundedMap function, the map, key, and MAX_PENDING symbols when applying this change.src/otel.ts (1)
24-34:⚠️ Potential issue | 🟠 MajorParse
OTEL_RESOURCE_ATTRIBUTESwith the spec’s escaping rules.This
split(",")/indexOf("=")parser breaks valid attribute strings that contain escaped commas or equals, so some user-supplied resource values will be exported incorrectly. Please reuse an OpenTelemetry parser here or make the parser escape-aware.OpenTelemetry specification OTEL_RESOURCE_ATTRIBUTES escaping commas equals environment variable format JavaScript🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/otel.ts` around lines 24 - 34, The current ad-hoc parser for process.env["OTEL_RESOURCE_ATTRIBUTES"] (the raw variable) incorrectly splits on every comma/equals and loses escaped characters; replace this with the OpenTelemetry-provided parser (for example reuse parseResourceAttributes from `@opentelemetry/`* resource/core utilities) to produce attrs, or implement an escape-aware parser that honors the OTEL_RESOURCE_ATTRIBUTES escaping rules (backslash-escaped commas and equals, trimming, and empty-key handling) so attrs is populated per the spec instead of using raw.split(",") and indexOf("=").
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/config.ts`:
- Around line 14-19: The parseEnvInt function currently uses parseInt which
accepts partial numeric strings (e.g. "1.5" or "5000ms"); update parseEnvInt to
only accept strict positive integer strings by validating the raw env value
(e.g. using a /^\d+$/ check or ensuring Number.isInteger(Number(raw)) and the
raw matches the integer representation) before converting; if the raw value
fails that strict integer test return fallback, otherwise return the parsed
positive integer (keep the existing positive > 0 requirement) so OTLP interval
env vars cannot be set with partial numeric strings.
---
Outside diff comments:
In `@src/otel.ts`:
- Around line 24-34: The current ad-hoc parser for
process.env["OTEL_RESOURCE_ATTRIBUTES"] (the raw variable) incorrectly splits on
every comma/equals and loses escaped characters; replace this with the
OpenTelemetry-provided parser (for example reuse parseResourceAttributes from
`@opentelemetry/`* resource/core utilities) to produce attrs, or implement an
escape-aware parser that honors the OTEL_RESOURCE_ATTRIBUTES escaping rules
(backslash-escaped commas and equals, trimming, and empty-key handling) so attrs
is populated per the spec instead of using raw.split(",") and indexOf("=").
In `@src/probe.ts`:
- Around line 31-33: In the socket error handler inside probe.ts (the
socket.on("error", (err) => { ... }) block), call socket.destroy() before
resolving to ensure the socket is cleaned up and prevent the timeout handler
from firing later; update the handler to destroy the socket then resolve({ ok:
false, ms: Date.now() - start, error: err.message }) (ensure the same resolution
behavior as the timeout and success handlers).
In `@src/util.ts`:
- Around line 16-21: The setBoundedMap function currently evicts the oldest
entry when map.size >= MAX_PENDING even if the provided key already exists;
change logic in setBoundedMap to first check whether map.has(key) and only
perform eviction when the key is not present and the map is at capacity (i.e.,
if (!map.has(key) && map.size >= MAX_PENDING) { evict oldest }), then call
map.set(key, value) as before; reference the setBoundedMap function, the map,
key, and MAX_PENDING symbols when applying this change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dfb86fb7-773d-44b4-96ca-a2cc6276717a
📒 Files selected for processing (11)
src/config.tssrc/handlers/activity.tssrc/handlers/message.tssrc/handlers/permission.tssrc/handlers/session.tssrc/index.tssrc/otel.tssrc/probe.tssrc/types.tssrc/util.tstests/config.test.ts
✅ Files skipped from review due to trivial changes (4)
- src/index.ts
- src/handlers/session.ts
- src/handlers/message.ts
- src/handlers/permission.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
README.md (1)
74-75: Consider adding usage examples for the new environment variables.The descriptions for
OPENCODE_OTLP_HEADERSandOPENCODE_RESOURCE_ATTRIBUTESare clear, but users would benefit from concrete examples showing the comma-separatedkey=valueformat. For instance:export OPENCODE_OTLP_HEADERS="x-api-key=your-token,x-tenant-id=tenant-123" export OPENCODE_RESOURCE_ATTRIBUTES="service.version=1.0.0,deployment.environment=production"Additionally, since
OPENCODE_OTLP_HEADERSmay contain sensitive auth tokens, consider adding a note about keeping these values secure and not committing them to version control.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 74 - 75, Add concrete usage examples for OPENCODE_OTLP_HEADERS and OPENCODE_RESOURCE_ATTRIBUTES showing comma-separated key=value pairs (e.g., two examples demonstrating header auth tokens and resource attributes like service.version and deployment.environment), and append a short security note advising that OPENCODE_OTLP_HEADERS may contain sensitive tokens and should not be committed to version control or exposed in logs/CI. Update the README table row descriptions for OPENCODE_OTLP_HEADERS and OPENCODE_RESOURCE_ATTRIBUTES to include a one-line “Example:” pointer and the security caution for OPENCODE_OTLP_HEADERS.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@README.md`:
- Around line 74-75: Add concrete usage examples for OPENCODE_OTLP_HEADERS and
OPENCODE_RESOURCE_ATTRIBUTES showing comma-separated key=value pairs (e.g., two
examples demonstrating header auth tokens and resource attributes like
service.version and deployment.environment), and append a short security note
advising that OPENCODE_OTLP_HEADERS may contain sensitive tokens and should not
be committed to version control or exposed in logs/CI. Update the README table
row descriptions for OPENCODE_OTLP_HEADERS and OPENCODE_RESOURCE_ATTRIBUTES to
include a one-line “Example:” pointer and the security caution for
OPENCODE_OTLP_HEADERS.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
README.md (1)
98-110: Consider adding authentication headers to vendor examples.The Datadog and Honeycomb examples only set the endpoint, but most managed OTLP collectors require authentication headers. Users may be confused when their exports fail silently.
📝 Suggested documentation improvement
### Datadog example ```bash export OPENCODE_ENABLE_TELEMETRY=1 export OPENCODE_OTLP_ENDPOINT=https://api.datadoghq.com +export OPENCODE_OTLP_HEADERS="DD-API-KEY=your-api-key"Honeycomb example
export OPENCODE_ENABLE_TELEMETRY=1 export OPENCODE_OTLP_ENDPOINT=https://api.honeycomb.io +export OPENCODE_OTLP_HEADERS="x-honeycomb-team=your-api-key"</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@README.mdaround lines 98 - 110, Update the Datadog and Honeycomb telemetry
examples to include the environment variable for auth headers by adding
OPENCODE_OTLP_HEADERS alongside OPENCODE_ENABLE_TELEMETRY and
OPENCODE_OTLP_ENDPOINT; specifically add
OPENCODE_OTLP_HEADERS="DD-API-KEY=your-api-key" for the Datadog example and
OPENCODE_OTLP_HEADERS="x-honeycomb-team=your-api-key" for the Honeycomb example
so consumers know how to supply required authentication when using the OTLP
exporter.</details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In@README.md:
- Around line 98-110: Update the Datadog and Honeycomb telemetry examples to
include the environment variable for auth headers by adding
OPENCODE_OTLP_HEADERS alongside OPENCODE_ENABLE_TELEMETRY and
OPENCODE_OTLP_ENDPOINT; specifically add
OPENCODE_OTLP_HEADERS="DD-API-KEY=your-api-key" for the Datadog example and
OPENCODE_OTLP_HEADERS="x-honeycomb-team=your-api-key" for the Honeycomb example
so consumers know how to supply required authentication when using the OTLP
exporter.</details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: Organization UI **Review profile**: CHILL **Plan**: Pro **Run ID**: `220ee0bc-6f2a-4da7-aabb-9c2f28be33b3` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 1165019af5dc2b447198bf889e9201907452e3fa and 65441b58ea48b6e6528ff9684191b7319e59d33b. </details> <details> <summary>📒 Files selected for processing (3)</summary> * `README.md` * `src/config.ts` * `tests/config.test.ts` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Documentation
Chores
Breaking Change