Skip to content

feat(napi): in-process ai-hist capture (no CLI shell-out) - #39

Merged
khaliqgant merged 2 commits into
mainfrom
feat/ai-hist-napi
Jul 7, 2026
Merged

feat(napi): in-process ai-hist capture (no CLI shell-out)#39
khaliqgant merged 2 commits into
mainfrom
feat/ai-hist-napi

Conversation

@khaliqgant

@khaliqgant khaliqgant commented Jul 7, 2026

Copy link
Copy Markdown
Member

Enables the Agent Relay runtime to sync + push history in-process — no subprocess, no CLI invocation — per the requirement not to shell out to the ai-hist CLI.

What

  • ai-hist → lib + thin bin. Splits the crate so its logic is callable as a library. main.rs becomes a 3-line wrapper over ai_hist_cli::run(). Adds pub fn sync_and_push() — the in-process equivalent of ai-hist sync && ai-hist push — with sync progress gated behind a SYNC_QUIET flag so an embedding host's stdout isn't spammed. No CLI behavior change.
  • crates/ai-hist-napi. A napi-rs cdylib exposing syncAndPush() to Node, running the blocking work on a worker thread so the event loop isn't blocked. Published as ai-hist-native with per-platform .node packages — same distribution model as @agent-relay/broker-*, except loaded in-process, not spawned.
  • CI + docs. publish-napi.yml builds the 4 targets (darwin arm64/x64, linux musl x64/arm64) and publishes via napi's tooling; docs/reflex-zero-setup.md covers the flow and the one-time publish/wire-up steps.

Verified locally

  • cargo build --workspace ✅, cargo test -p ai-hist-cli --lib ✅ (30 tests)
  • napi build produces a loadable addon; Node loads index.js and syncAndPush is a callable async function returning { sent, accepted, authenticated }.

Companion + follow-up

  • Relay calls require('ai-hist-native').syncAndPush() from its Reflex capture loop (relay#1233).
  • To ship: register OIDC trusted publishers for ai-hist-native + the 4 platform packages, run publish-napi.yml (dry-run first — validate the musl cross-compiles), then add ai-hist-native as an optional dependency of agent-relay.

Note: this work was previously stacked on the (now-merged) #38 branch; this PR re-bases it cleanly onto main.

🤖 Generated with Claude Code

Review in cubic

Lets a host (the Agent Relay runtime) sync + push history in-process, with no
subprocess and no CLI invocation.

- Split the `ai-hist` crate into a lib + a thin bin. `main.rs` is a 3-line
  wrapper over `ai_hist_cli::run()`; adds `pub fn sync_and_push()` (the
  in-process equivalent of `ai-hist sync && ai-hist push`) with sync progress
  gated behind a quiet flag so an embedding host's stdout isn't spammed. No CLI
  behavior change; 30 lib tests pass.
- `crates/ai-hist-napi`: a napi-rs cdylib exposing `syncAndPush()` to Node,
  running the blocking work on a worker thread. Published as `ai-hist-native`
  (per-platform `.node` packages, same distribution model as
  @agent-relay/broker-*, but loaded in-process rather than spawned). Verified:
  `napi build` produces a loadable addon whose `syncAndPush()` returns
  `{ sent, accepted, authenticated }`.
- `.github/workflows/publish-napi.yml` builds the four targets and publishes via
  napi's tooling; `docs/reflex-zero-setup.md` documents the flow + the one-time
  publish/wire-up steps.

The companion relay change calls `require('ai-hist-native').syncAndPush()` from
the Reflex capture loop.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@cursor

cursor Bot commented Jul 7, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@coderabbitai

coderabbitai Bot commented Jul 7, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@khaliqgant, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 47 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 953de964-b4da-4353-9c66-16d981a2bd08

📥 Commits

Reviewing files that changed from the base of the PR and between c5883d3 and 0dfab9e.

⛔ Files ignored due to path filters (2)
  • Cargo.lock is excluded by !**/*.lock
  • crates/ai-hist-napi/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (13)
  • .github/workflows/publish-napi.yml
  • Cargo.toml
  • crates/ai-hist-napi/.gitignore
  • crates/ai-hist-napi/Cargo.toml
  • crates/ai-hist-napi/build.rs
  • crates/ai-hist-napi/index.d.ts
  • crates/ai-hist-napi/index.js
  • crates/ai-hist-napi/package.json
  • crates/ai-hist-napi/src/lib.rs
  • crates/ai-hist/Cargo.toml
  • crates/ai-hist/src/lib.rs
  • crates/ai-hist/src/main.rs
  • docs/reflex-zero-setup.md
📝 Walkthrough

Walkthrough

Adds a new ai-hist-napi Rust crate exposing a syncAndPush() N-API binding that wraps ai-hist-cli's sync/push logic, generated npm package artifacts (typings, loader, package.json), workspace wiring, a GitHub Actions workflow to build and publish per-platform native addons, and documentation for the reflex zero-setup flow.

Changes

ai-hist-napi Addon and Publishing

Layer / File(s) Summary
ai-hist library target and napi crate implementation
crates/ai-hist/Cargo.toml, crates/ai-hist-napi/Cargo.toml, crates/ai-hist-napi/build.rs, Cargo.toml, crates/ai-hist-napi/.gitignore, crates/ai-hist-napi/src/lib.rs
Exposes ai-hist-cli as a library, adds the ai-hist-napi crate with napi-build setup, and implements sync_and_push() returning SyncPushResult (sent/accepted/authenticated) via a blocking worker thread; wires the new crate into the workspace.
Generated npm package, typings, and native loader
crates/ai-hist-napi/index.d.ts, crates/ai-hist-napi/index.js, crates/ai-hist-napi/package.json
Adds NAPI-RS generated SyncPushResult typings and syncAndPush() declaration, a per-platform native binding loader (isMusl detection, OS/arch dispatch, fallback modules), and package metadata for the npm publish.
CI build and publish workflow
.github/workflows/publish-napi.yml
Adds a workflow building the addon across macOS and Linux musl targets, uploading .node artifacts, then assembling and publishing npm packages with dry-run or provenance modes.
Reflex zero-setup documentation
docs/reflex-zero-setup.md
Documents the in-process capture flow, component breakdown, publishing steps, and operational notes for the napi addon.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant NodeApp
  participant IndexJs as index.js
  participant NapiLib as ai-hist-napi lib.rs
  participant AiHistCli as ai_hist_cli::sync_and_push
  participant RelayCloud as relayhistory-cloud

  NodeApp->>IndexJs: require native binding
  IndexJs->>NapiLib: syncAndPush()
  NapiLib->>AiHistCli: spawn_blocking(sync_and_push)
  AiHistCli->>RelayCloud: POST /v1/ingest
  RelayCloud-->>AiHistCli: sent/accepted counts
  AiHistCli-->>NapiLib: sync result
  NapiLib-->>IndexJs: SyncPushResult
  IndexJs-->>NodeApp: Promise<SyncPushResult>
Loading
sequenceDiagram
  participant GitHubActions
  participant BuildJob as build job
  participant PublishJob as publish job
  participant Npm as npm registry

  GitHubActions->>BuildJob: run matrix build (macOS, Linux musl)
  BuildJob->>BuildJob: napi build --zig (aarch64-musl)
  BuildJob-->>GitHubActions: upload .node artifacts
  GitHubActions->>PublishJob: trigger after build
  PublishJob->>PublishJob: napi create-npm-dirs / napi artifacts
  alt dry_run true
    PublishJob->>PublishJob: napi prepublish --dry-run
  else dry_run false
    PublishJob->>Npm: napi prepublish --provenance
  end
Loading

Possibly related PRs

Poem

A rabbit hops through Rust and node,
Compiling addons, cross-arch owned,
With musl and zig it builds so neat,
Then pushes history, sync complete,
Hop, publish, provenance signed — a job well done! 🐇📦

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: adding in-process ai-hist capture via a N-API addon instead of shelling out.
Description check ✅ Passed The description matches the changeset and explains the new in-process sync/push flow, N-API module, CI, and docs.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/ai-hist-napi

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.

@gemini-code-assist

Copy link
Copy Markdown

Warning

Gemini encountered an error creating the review. You can try again by commenting /gemini review.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 138b40982e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

if (localFileExisted) {
nativeBinding = require('./ai-hist-native.linux-x64-gnu.node')
} else {
nativeBinding = require('ai-hist-native-linux-x64-gnu')

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Publish the GNU Linux native packages

On standard glibc Linux (for example Ubuntu x64), this branch runs because isMusl() is false and tries to load ai-hist-native-linux-x64-gnu, but the new package/workflow only builds the musl triples (x86_64-unknown-linux-musl and aarch64-unknown-linux-musl). That means require('ai-hist-native') will throw on the most common Linux runtime instead of loading the published musl artifact; either publish the *-gnu packages too or make the loader/package targets match the Linux artifacts you ship.

Useful? React with 👍 / 👎.

@agent-relay-code

Copy link
Copy Markdown
Contributor

ℹ️ pr-reviewer: review only — no file changes were applied to the PR (nothing to commit after review). The notes below are advisory and were not pushed.

Only the pre-existing runtime state file is modified (not part of this PR, not touched by me). No stray edits from my review. Clean.

Review: PR #39 — feat(napi): in-process ai-hist capture (no CLI shell-out)

Summary

This PR adds a napi (Node native addon) crate ai-hist-napi (published as npm ai-hist-native) exposing syncAndPush(), refactors crates/ai-hist/src/main.rs into a reusable library (ai_hist_cli) with a thin binary wrapper, adds a new sync_and_push() library entry point, and wires up a publish-napi.yml CI workflow plus docs/reflex-zero-setup.md.

I traced the refactor and the new library API across callers, types, and config, and ran the canonical CI Rust steps end-to-end.

Verification

  • cargo test --workspacePASS (30 + 27 tests, 0 failures). The new ai-hist-napi cdylib compiles cleanly in-tree.
  • cargo build -q -p ai-hist-cliPASS (the exact command CI runs).
  • The main.rslib.rs move is faithful: old main.rs (5027 lines) moved to lib.rs (5100 lines); main.rs is now a 7-line wrapper calling ai_hist_cli::run(). mod cloud/mod learn already existed and resolve correctly.
  • sync_and_push() mirrors the CLI Push handler; the cloud::push(...) call signature and PushReport field types (sent: usize, accepted: u64) match the casts in both the lib fn and the napi wrapper.
  • Cargo package name (ai-hist-cli) and lib name (ai_hist_cli) are unchanged, so CI's -p ai-hist-cli reference still resolves.
  • npm name ai-hist-native vs crate dir ai-hist-napi is intentional and consistently documented; package-lock.json is in sync with package.json (needed for npm ci).
  • Python/TypeScript CI steps operate only on files this PR doesn't touch.

No mechanical fixes were required — the new/changed files are clean (no lint, typo, or import-order issues) and I made no edits. Working tree left unchanged (only a pre-existing untracked runtime state.json mod, unrelated to this PR).

Addressed comments

  • No bot or human review comments were present in the provided context (context.json has no comments; no comment files in .workforce/). Nothing to address.

Advisory Notes

  • Fail-open check (verified safe, no change): sync_and_push() returns authenticated: false as a documented no-op when there's no stored auth, whereas the CLI Push errors out. This is not a fail-closed→fail-open regression: it explicitly reports authenticated: false rather than fabricating success, and is the intended contract for background callers per the doc comments. Left unchanged.
  • Version skew (release-management, out of scope for a code fix): ai-hist-cli crate is 0.1.0, the repo just tagged ai-hist@0.4.1 (commit c5883d3), while the napi crate and ai-hist-native npm package are 0.4.0. docs/reflex-zero-setup.md says to keep the addon version "in step with the ai-hist SDK." This doesn't break the build but the maintainer should reconcile versions before the first real publish. No code changed.
  • CI cross-compile caveat (already flagged in-PR): publish-napi.yml is workflow_dispatch-only (won't run on this PR) and the diff itself notes musl cross-compiles may need per-runner tuning; validate via dry_run before the first real publish. Informational only.

The Rust build/test that this PR affects passes locally. Remaining items (npm publish dry-run, version reconciliation) require human judgment and post-merge OIDC/publish setup that can't be exercised here, and I cannot confirm the status of the other CI jobs (Python/TS/e2e) or GitHub mergeability from this sandbox — so I am not declaring the PR final.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 4

♻️ Duplicate comments (1)
crates/ai-hist-napi/Cargo.toml (1)

12-14: 🗄️ Data Integrity & Integration | 🔴 Critical | 🏗️ Heavy lift

Confirm the ai-hist-cli dependency alias resolves to the actual package name.

Same concern as the ai-hist/Cargo.toml review: ai-hist-cli = { path = "../ai-hist" } (Line 13) has no package field, so Cargo will require the target manifest's [package] name to literally be ai-hist-cli. This is consistent with ai_hist_cli::sync_and_push() used in src/lib.rs, but should be confirmed against the actual [package] section of crates/ai-hist/Cargo.toml, which isn't part of this diff.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/ai-hist-napi/Cargo.toml` around lines 12 - 14, Confirm that the
ai-hist-cli dependency in the ai-hist-napi Cargo.toml actually matches the
package name defined in the target manifest for ai-hist; since the dependency
uses ai-hist-cli = { path = "../ai-hist" } without a package field, Cargo will
only resolve it if the [package] name in crates/ai-hist/Cargo.toml is exactly
ai-hist-cli. Check the ai_hist_cli reference in src/lib.rs and either keep the
alias as-is if the package name matches, or add an explicit package mapping if
the package name differs.
🧹 Nitpick comments (2)
.github/workflows/publish-napi.yml (2)

84-110: 🔒 Security & Privacy | 🔵 Trivial

Consider gating the publish job behind a protected environment.

For a job that publishes 5 npm packages via OIDC, adding environment: name: npm-publish with required reviewers gives a human approval gate before token exchange — mirroring the pattern recommended for other trusted-publishing setups (e.g. PyPI's environment: + required reviewers) and reducing blast radius if the workflow file itself is ever compromised.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/publish-napi.yml around lines 84 - 110, The publish job in
publish-napi.yml should be gated behind a protected environment so npm OIDC
publishing cannot proceed without approval. Update the publish job definition to
use a dedicated environment such as npm-publish, and ensure the workflow relies
on that environment’s required reviewers before the Publish step in the publish
job runs.

46-46: 🔒 Security & Privacy | 🔵 Trivial | ⚖️ Poor tradeoff

Consider pinning third-party actions to commit SHAs.

This workflow has real publish power (npm OIDC trusted publishing + provenance across 5 packages), which makes it an attractive target — per npm's trusted-publishing guidance, a compromised action inside a publish workflow can be used to ship a malicious package while still appearing "trusted." dtolnay/rust-toolchain@stable and Swatinem/rust-cache@v2 float; pinning to SHAs (with Dependabot updates) reduces this supply-chain exposure.

Also applies to: 53-53, 57-57, 78-78, 90-90, 96-96

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/publish-napi.yml at line 46, The publish workflow uses
floating third-party action tags, which should be pinned to immutable commit
SHAs to reduce supply-chain risk. Update the action references in this workflow
(including actions/checkout, dtolnay/rust-toolchain, and Swatinem/rust-cache) to
specific commit SHAs, and keep Dependabot configured to manage future SHA
updates so the publish path stays secure.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/publish-napi.yml:
- Line 46: The checkout step in the publish-napi workflow leaves unnecessary
GitHub credentials on disk. Update the actions/checkout usage in both affected
jobs to set persist-credentials to false, so the GITHUB_TOKEN is not retained
after checkout; use the checkout step itself as the unique place to apply this
change.
- Around line 101-110: The Publish step is interpolating the workflow_dispatch
input directly inside the shell script, which is unsafe and flagged by zizmor.
Update the Publish job to read the dry_run value from the safer inputs context
via an env var, then use that env var in the shell condition instead of
embedding github.event.inputs.dry_run directly in the run block.
- Around line 21-23: The workflow-level permissions are too broad because
`id-token: write` is granted to every job, including the `build` job that never
publishes. Move the OIDC permission off the top-level `permissions` block and
scope `id-token: write` only to the publish job, keeping `contents: read`
wherever needed and preserving the existing `build` matrix behavior.
- Around line 68-76: The Build addon job in the publish-napi workflow calls npx
napi build with --zig for the aarch64-unknown-linux-musl target, but Zig is
never installed or added to PATH first. Add a Zig setup step before this matrix
branch runs, and ensure the build step in the Build addon job can find zig when
matrix.target is aarch64-unknown-linux-musl.

---

Duplicate comments:
In `@crates/ai-hist-napi/Cargo.toml`:
- Around line 12-14: Confirm that the ai-hist-cli dependency in the ai-hist-napi
Cargo.toml actually matches the package name defined in the target manifest for
ai-hist; since the dependency uses ai-hist-cli = { path = "../ai-hist" } without
a package field, Cargo will only resolve it if the [package] name in
crates/ai-hist/Cargo.toml is exactly ai-hist-cli. Check the ai_hist_cli
reference in src/lib.rs and either keep the alias as-is if the package name
matches, or add an explicit package mapping if the package name differs.

---

Nitpick comments:
In @.github/workflows/publish-napi.yml:
- Around line 84-110: The publish job in publish-napi.yml should be gated behind
a protected environment so npm OIDC publishing cannot proceed without approval.
Update the publish job definition to use a dedicated environment such as
npm-publish, and ensure the workflow relies on that environment’s required
reviewers before the Publish step in the publish job runs.
- Line 46: The publish workflow uses floating third-party action tags, which
should be pinned to immutable commit SHAs to reduce supply-chain risk. Update
the action references in this workflow (including actions/checkout,
dtolnay/rust-toolchain, and Swatinem/rust-cache) to specific commit SHAs, and
keep Dependabot configured to manage future SHA updates so the publish path
stays secure.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: bf590f8e-0405-4874-b1da-71720ffd0142

📥 Commits

Reviewing files that changed from the base of the PR and between c5883d3 and 138b409.

⛔ Files ignored due to path filters (2)
  • Cargo.lock is excluded by !**/*.lock
  • crates/ai-hist-napi/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (13)
  • .github/workflows/publish-napi.yml
  • Cargo.toml
  • crates/ai-hist-napi/.gitignore
  • crates/ai-hist-napi/Cargo.toml
  • crates/ai-hist-napi/build.rs
  • crates/ai-hist-napi/index.d.ts
  • crates/ai-hist-napi/index.js
  • crates/ai-hist-napi/package.json
  • crates/ai-hist-napi/src/lib.rs
  • crates/ai-hist/Cargo.toml
  • crates/ai-hist/src/lib.rs
  • crates/ai-hist/src/main.rs
  • docs/reflex-zero-setup.md

Comment thread .github/workflows/publish-napi.yml Outdated
Comment on lines +21 to +23
permissions:
contents: read
id-token: write

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Scope id-token: write to the publish job only.

The build job never publishes and doesn't need OIDC tokens; granting id-token: write at the workflow level gives every job (including the multi-platform matrix build) unnecessary token-minting capability, which flags as excessive-permissions.

🔒️ Proposed fix
 permissions:
   contents: read
-  id-token: write

 defaults:
   run:
     working-directory: crates/ai-hist-napi

 jobs:
   build:
     name: Build ${{ matrix.target }}
+    permissions:
+      contents: read
     runs-on: ${{ matrix.os }}
@@
   publish:
     name: Publish to npm
     needs: build
+    permissions:
+      contents: read
+      id-token: write
     runs-on: ubuntu-latest
📝 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
permissions:
contents: read
id-token: write
permissions:
contents: read
defaults:
run:
working-directory: crates/ai-hist-napi
jobs:
build:
name: Build ${{ matrix.target }}
permissions:
contents: read
runs-on: ${{ matrix.os }}
publish:
name: Publish to npm
needs: build
permissions:
contents: read
id-token: write
runs-on: ubuntu-latest
🧰 Tools
🪛 zizmor (1.26.1)

[error] 23-23: overly broad permissions (excessive-permissions): id-token: write is overly broad at the workflow level

(excessive-permissions)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/publish-napi.yml around lines 21 - 23, The workflow-level
permissions are too broad because `id-token: write` is granted to every job,
including the `build` job that never publishes. Move the OIDC permission off the
top-level `permissions` block and scope `id-token: write` only to the publish
job, keeping `contents: read` wherever needed and preserving the existing
`build` matrix behavior.

Source: Linters/SAST tools

- os: ubuntu-latest
target: aarch64-unknown-linux-musl
steps:
- uses: actions/checkout@v4

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

Set persist-credentials: false on checkout.

Neither job pushes to the repo, so the default persisted GITHUB_TOKEN credential left on disk by actions/checkout is unnecessary attack surface (flagged by zizmor as artipacked) — especially risky in the build job where npm ci executes third-party install scripts.

🔒️ Proposed fix
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v4
+        with:
+          persist-credentials: false

Also applies to: 89-89

🧰 Tools
🪛 zizmor (1.26.1)

[warning] 46-46: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false

(artipacked)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/publish-napi.yml at line 46, The checkout step in the
publish-napi workflow leaves unnecessary GitHub credentials on disk. Update the
actions/checkout usage in both affected jobs to set persist-credentials to
false, so the GITHUB_TOKEN is not retained after checkout; use the checkout step
itself as the unique place to apply this change.

Source: Linters/SAST tools

Comment on lines +68 to +76
- name: Build addon
run: |
set -euo pipefail
# zig simplifies the aarch64 musl cross-compile.
if [ "${{ matrix.target }}" = "aarch64-unknown-linux-musl" ]; then
npx napi build --platform --release --target "${{ matrix.target }}" --zig
else
npx napi build --platform --release --target "${{ matrix.target }}"
fi

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

Does napi-rs napi build --zig flag automatically download the zig toolchain?

💡 Result:

No, the napi-rs CLI does not automatically download or install the Zig toolchain [1]. When you use flags like --cross-compile or -x (which utilize cargo-zigbuild), or when you previously used the --zig flag, the napi-rs CLI expects the zig binary to already be installed and available in your system's PATH [1]. If the tool is missing, the build will fail with an error such as zig: not found [1]. To use cross-compilation features provided by napi-rs, you must ensure that the Zig compiler is manually installed in your environment [1]. In CI/CD environments (such as GitHub Actions), this is typically handled by adding a step to install Zig before running the build command, for example by using actions like goto-bus-stop/setup-zig [1].

Citations:


Install Zig before the aarch64-musl build. napi build --zig still expects zig on PATH, so this branch will fail unless a Zig setup step runs first.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/publish-napi.yml around lines 68 - 76, The Build addon job
in the publish-napi workflow calls npx napi build with --zig for the
aarch64-unknown-linux-musl target, but Zig is never installed or added to PATH
first. Add a Zig setup step before this matrix branch runs, and ensure the build
step in the Build addon job can find zig when matrix.target is
aarch64-unknown-linux-musl.

Comment on lines +101 to +110
- name: Publish
run: |
set -euo pipefail
if [ "${{ github.event.inputs.dry_run }}" = "true" ]; then
echo "Dry run — skipping publish. Prepared packages:"
npx napi prepublish -t npm --dry-run || true
else
# Publishes the per-platform packages then the main package (OIDC provenance).
npx napi prepublish -t npm --provenance
fi

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Avoid interpolating the workflow_dispatch input directly into shell.

${{ github.event.inputs.dry_run }} is always a string (not a validated boolean) when read via github.event.inputs — GitHub only coerces the type for the inputs.* context. Direct expression interpolation into a run: block is a template-injection vector flagged by zizmor; assign it to an env var first (and prefer inputs.dry_run over github.event.inputs.dry_run).

🛡️ Proposed fix
+    env:
+      DRY_RUN: ${{ inputs.dry_run }}
     run: |
       set -euo pipefail
-      if [ "${{ github.event.inputs.dry_run }}" = "true" ]; then
+      if [ "$DRY_RUN" = "true" ]; then
         echo "Dry run — skipping publish. Prepared packages:"
         npx napi prepublish -t npm --dry-run || true
       else
         # Publishes the per-platform packages then the main package (OIDC provenance).
         npx napi prepublish -t npm --provenance
       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
- name: Publish
run: |
set -euo pipefail
if [ "${{ github.event.inputs.dry_run }}" = "true" ]; then
echo "Dry run — skipping publish. Prepared packages:"
npx napi prepublish -t npm --dry-run || true
else
# Publishes the per-platform packages then the main package (OIDC provenance).
npx napi prepublish -t npm --provenance
fi
- name: Publish
env:
DRY_RUN: ${{ inputs.dry_run }}
run: |
set -euo pipefail
if [ "$DRY_RUN" = "true" ]; then
echo "Dry run — skipping publish. Prepared packages:"
npx napi prepublish -t npm --dry-run || true
else
# Publishes the per-platform packages then the main package (OIDC provenance).
npx napi prepublish -t npm --provenance
fi
🧰 Tools
🪛 zizmor (1.26.1)

[error] 104-104: code injection via template expansion (template-injection): may expand into attacker-controllable code

(template-injection)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/publish-napi.yml around lines 101 - 110, The Publish step
is interpolating the workflow_dispatch input directly inside the shell script,
which is unsafe and flagged by zizmor. Update the Publish job to read the
dry_run value from the safer inputs context via an env var, then use that env
var in the shell condition instead of embedding github.event.inputs.dry_run
directly in the run block.

Source: Linters/SAST tools

- P1 (Codex): the napi loader resolves `ai-hist-native-linux-<arch>-gnu` on
  standard glibc Linux (Ubuntu etc.), but only the musl triples were built, so
  `require('ai-hist-native')` threw on the most common Linux runtime. Now build
  BOTH gnu and musl for x64/arm64 (6 targets total); the loader picks the right
  one per libc.
- Workflow hardening (CodeRabbit): scope `id-token: write` to the publish job
  only (build no longer mints OIDC tokens); `persist-credentials: false` on both
  checkouts; install zig for the cross-compiled Linux targets (napi
  --cross-compile needs it); read `dry_run` via an env var + `inputs.` instead
  of interpolating `github.event.inputs.*` into the shell.
- docs: note the gnu+musl split.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@cursor

cursor Bot commented Jul 7, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@khaliqgant

Copy link
Copy Markdown
Member Author

Addressed the review feedback (latest commit).

P1 (Codex) — glibc Linux was unsupported. The napi loader resolves ai-hist-native-linux-<arch>-gnu on standard glibc Linux (Ubuntu/Debian — the common case), but only the musl triples were built, so require('ai-hist-native') would throw there. Now the package + workflow build both gnu and musl for x64/arm64 (6 targets); the loader picks the right one per libc.

Workflow hardening (CodeRabbit):

  • id-token: write scoped to the publish job only — the build matrix no longer mints OIDC tokens.
  • persist-credentials: false on both checkouts.
  • Install zig for the cross-compiled Linux targets (napi --cross-compile needs it — it doesn't auto-download it).
  • dry_run read via an env: var + inputs.dry_run instead of interpolating github.event.inputs.* into the run: shell (template-injection hardening).

Verified locally: workflow YAML + package.json valid, napi build still produces a loadable addon. As noted in the workflow/docs, the musl/gnu cross-compiles should be validated with a dry_run before the first real publish (CI I can't exercise from here).

@agent-relay-code

Copy link
Copy Markdown
Contributor

ℹ️ pr-reviewer: review only — no file changes were applied to the PR (nothing to commit after review). The notes below are advisory and were not pushed.

Review: PR #39 — feat(napi): in-process ai-hist capture (no CLI shell-out)

Summary

This PR extracts the ai-hist CLI logic from main.rs into a library (lib.rs), makes main.rs a thin ai_hist_cli::run() wrapper, and adds a new ai-hist-napi crate exposing syncAndPush() to Node via napi-rs so the Agent Relay runtime can capture history in-process instead of shelling out to the CLI. It also adds a napi publish workflow and setup docs.

I traced the real change by diffing the base main.rs against the new lib.rs (the raw PR diff was truncated at 180 KB, so main.rs and docs/reflex-zero-setup.md weren't in it — I read them from the checkout). The library extraction is a faithful move; the only behavioral additions are:

  • SYNC_QUIET atomic + sync_note! macro that gates sync-progress println!s ([claude] +N rows, etc.). All converted lines are genuinely progress output; no user-facing results were silenced.
  • sync_and_push() / SyncPushOutcome, mirroring the CLI Push handler (crates/ai-hist/src/lib.rs:805) exactly: same MachineIdentity, load_cursor, and cloud::push(..., 500, &HashSet::new()).

Type-checked the casts: PushReport.sent: usizeu64u32 (napi) and PushReport.accepted: u64 are consistent (crates/ai-hist/src/cloud.rs:171).

Verification (CI-equivalent, run end to end)

  • cargo test --workspacepass (30 CLI + 27 core tests; ai-hist-napi crate compiles clean under its #![deny(clippy::all)]; no warnings).
  • cargo build -p ai-hist-cli --bin ai-histpass (binary name unchanged).
  • Python wrapper + dispatch tests (test_ai_hist.py, test_cli_dispatch.py) — 187 passed, confirming the main.rslib.rs refactor is behavior-preserving.

No mechanical issues found (napi index.js/index.d.ts are auto-generated and correct; Cargo/workspace/lockfile wiring is consistent). No fixes applied — nothing to auto-edit, and no semantic/safety changes were warranted.

Safety review

  • The missing-auth path in sync_and_push() returns authenticated: false, sent: 0, accepted: 0 rather than erroring. This is not a fail-open regression — it reports the no-op truthfully to background callers and is documented; it does not fabricate a success/acked state. No guard defaults, lifecycle, dispatch, or in-flight code touched.

Addressed comments

  • No bot or reviewer comments were present in .workforce/context.json (no review/comment payload supplied), so there were none to reconcile against the current checkout.

Advisory Notes

  • .github/workflows/publish-napi.yml header warns that napi cross-compilation (esp. musl via zig) "can need tuning per runner image; validate a dry run before the first real publish." The workflow is workflow_dispatch-only and not part of PR CI, so it can't be validated here. Flagging as advisory: run the dry_run: true path once before the first real publish. No code change — out of scope for automated verification and requires a live runner.

The PR is mechanically clean, behavior-preserving, and passes the full CI-equivalent build/test suite locally. Remaining gating (PR mergeability and the actual GitHub CI check runs) is not observable from this sandbox, so I'm not asserting those are green.

@khaliqgant
khaliqgant merged commit 169d485 into main Jul 7, 2026
3 checks passed
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