feat: bind native entitlement to broker identity#641
Conversation
📝 WalkthroughPriority Level: P4/NIT (optional)No actionable P0–P3 issues identified. The changes consistently enforce device/repository-bound entitlements, bounded Activation Key handling, fail-closed validation, and production resolver wiring. Focused validation coverage is appropriate; broad validation remains delegated to CI. WalkthroughThe PR binds native license activation and private-repository token issuance to a Keychain-backed broker device identity and exact repository. Activation keys remain bounded to stdin and broker request handling, while production entitlement resolution validates license, device, and repository state. ChangesNative activation and identity binding
Broker activation-key transport
Production entitlement resolution
Rollout and boundary documentation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant DesktopActivationLicenseClient
participant NeonDiffCLI
participant LicenseAPI
participant GitHubBrokerClient
participant GitHubBrokerService
participant LicenseStore
DesktopActivationLicenseClient->>NeonDiffCLI: activate with machineId and repo
DesktopActivationLicenseClient->>NeonDiffCLI: send activation key via stdin
NeonDiffCLI->>LicenseAPI: submit licenseKey, machineId, and repo
GitHubBrokerClient->>GitHubBrokerService: request token with activationKey and repositories
GitHubBrokerService->>LicenseStore: validate device, license, and repository entitlement
LicenseStore-->>GitHubBrokerService: entitlement result
GitHubBrokerService-->>GitHubBrokerClient: minted token or rejection
Possibly related issues
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
coderabbitai
Bot
added
the
license
evaOS review status: completedPR: #641 - feat: bind native entitlement to broker identity evaOS review completed for this PR head. Automation note: agents should wait for this comment to reach PR URL: #641 Review URL: #641 (review) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@services/license-api/src/github-broker/license-entitlement.ts`:
- Around line 26-31: Update the expiry validation in the license entitlement
resolver around the existing license status and expiresAt predicate to fail
closed: when expiresAt is present but Date.parse returns a non-finite value,
treat the license as expired/denied. Preserve the current behavior for
explicitly expired licenses and valid timestamps, including valid future dates
remaining active.
In `@services/license-api/test/github-broker-production-entitlement.test.ts`:
- Around line 126-162: Extend the HTTPS token issuance test around startBroker
and the existing activate call with a second authorized private repository, then
request that repository using a key activated for PRIVATE_REPO. Assert the
response is denied and verify fake.createInstallationAccessToken was never
called, while preserving the existing successful issuance assertion for
PRIVATE_REPO.
In `@src/cli.ts`:
- Around line 367-369: Update the machineId construction in the CLI argument
handling to check whether args["license-machine-id"] is !== undefined rather
than relying on truthiness, ensuring empty supplied values reach
parseLicenseMachineIdArg and are rejected. Preserve omission only when the
option is absent, and add a focused CLI test covering an empty
--license-machine-id value.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 1d85135e-8b8d-4d66-848f-c58f61c9d390
📒 Files selected for processing (21)
README.mdapps/neondiff-desktop/Sources/NeonDiffDesktopAppCore/Models/NeonDiffDesktopModel.swiftapps/neondiff-desktop/Sources/NeonDiffDesktopAppCore/Services/DesktopActivationLicenseClient.swiftapps/neondiff-desktop/Sources/NeonDiffDesktopCore/Services/ActivationLicenseClient.swiftapps/neondiff-desktop/Sources/NeonDiffDesktopCore/Services/GitHubBrokerClient.swiftapps/neondiff-desktop/Tests/NeonDiffDesktopAppCoreTests/DesktopActivationLicenseClientTests.swiftapps/neondiff-desktop/Tests/NeonDiffDesktopCoreTests/ActivationLicenseClientTests.swiftapps/neondiff-desktop/Tests/NeonDiffDesktopCoreTests/GitHubBrokerClientTests.swiftdocs/SETUP.mddocs/github-app-setup.mddocs/license-boundary.mdservices/license-api/src/github-broker/index.tsservices/license-api/src/github-broker/license-entitlement.tsservices/license-api/src/github-broker/service.tsservices/license-api/src/server.tsservices/license-api/test/github-broker-entitlement.test.tsservices/license-api/test/github-broker-production-entitlement.test.tsservices/license-api/test/github-broker-support.tssrc/cli.tssrc/license.tstests/license.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Swift desktop smoke
- GitHub Check: Build, test, and package
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use focused local tests or smoke checks by default; treat npm test and npm run build as broad GitHub CI checks unless reproducing CI, a focused harness is unavailable, or the user explicitly requests broad validation.
Files:
services/license-api/src/github-broker/index.tsservices/license-api/src/github-broker/license-entitlement.tstests/license.test.tsservices/license-api/test/github-broker-entitlement.test.tsservices/license-api/test/github-broker-support.tsservices/license-api/src/server.tsservices/license-api/test/github-broker-production-entitlement.test.tssrc/cli.tsservices/license-api/src/github-broker/service.tssrc/license.ts
docs/SETUP.md
📄 CodeRabbit inference engine (AGENTS.md)
Read and maintain docs/SETUP.md before changing installation, GitHub App, provider, daemon, or first-run behavior; keep first-run detail there.
Files:
docs/SETUP.md
{README.md,docs/SETUP.md,docs/github-app-setup.md}
📄 CodeRabbit inference engine (AGENTS.md)
For setup or GitHub App setup changes, update README.md, docs/SETUP.md, and docs/github-app-setup.md before claiming the first-run path is fixed.
Files:
docs/SETUP.mddocs/github-app-setup.mdREADME.md
README.md
📄 CodeRabbit inference engine (AGENTS.md)
Keep README.md as the public landing page containing product value, installation, setup, first dry-run review, safety boundaries, and links; do not put long release ledgers, raw local command dumps, un contextualized internal paths, or active sprint churn there.
Files:
README.md
🧠 Learnings (1)
📚 Learning: 2026-07-13T18:34:30.267Z
Learnt from: 100yenadmin
Repo: electricsheephq/evaos-code-review-bot-neondiff PR: 531
File: apps/neondiff-desktop/Sources/NeonDiffDesktop/App/NeonDiffDesktopApp.swift:204-227
Timestamp: 2026-07-13T18:34:30.267Z
Learning: In the apps/neondiff-desktop Swift codebase, avoid flagging the SwiftLint `required_deinit` issue for types that do not actually own resources needing explicit cleanup. Only require an explicit `deinit` when the class/struct owns resources that must be released (e.g., strong references that need breaking, observers/timers/event subscriptions, file handles, unmanaged/unsafe resources). For cases like AppDelegate in NeonDiffDesktopApp.swift that don’t own such cleanup requirements, do not report missing `deinit` just to satisfy the rule, especially since the repo does not currently enforce it via an owned SwiftLint config/merge gate.
Applied to files:
apps/neondiff-desktop/Tests/NeonDiffDesktopAppCoreTests/DesktopActivationLicenseClientTests.swiftapps/neondiff-desktop/Tests/NeonDiffDesktopCoreTests/GitHubBrokerClientTests.swiftapps/neondiff-desktop/Sources/NeonDiffDesktopAppCore/Services/DesktopActivationLicenseClient.swiftapps/neondiff-desktop/Sources/NeonDiffDesktopAppCore/Models/NeonDiffDesktopModel.swiftapps/neondiff-desktop/Tests/NeonDiffDesktopCoreTests/ActivationLicenseClientTests.swiftapps/neondiff-desktop/Sources/NeonDiffDesktopCore/Services/GitHubBrokerClient.swiftapps/neondiff-desktop/Sources/NeonDiffDesktopCore/Services/ActivationLicenseClient.swift
🪛 ast-grep (0.44.1)
src/license.ts
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawnSync } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawnSync } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawnSync } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawnSync } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawnSync } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🪛 LanguageTool
docs/github-app-setup.md
[uncategorized] ~62-~62: The official name of this software platform is spelled with a capital “H”.
Context: ...uth) during > installation" and set the /github/connect/callback URL; that > registrat...
(GITHUB)
[uncategorized] ~69-~69: The official name of this software platform is spelled with a capital “H”.
Context: ...ory to the license API. A later private /github/token request carries the Keychain-own...
(GITHUB)
🔇 Additional comments (19)
README.md (1)
141-147: LGTM!docs/SETUP.md (1)
208-217: LGTM!docs/github-app-setup.md (1)
59-76: LGTM!docs/license-boundary.md (1)
84-91: LGTM!apps/neondiff-desktop/Sources/NeonDiffDesktopAppCore/Models/NeonDiffDesktopModel.swift (1)
1141-1162: LGTM!apps/neondiff-desktop/Sources/NeonDiffDesktopAppCore/Services/DesktopActivationLicenseClient.swift (1)
13-29: LGTM!Also applies to: 50-51
apps/neondiff-desktop/Sources/NeonDiffDesktopCore/Services/ActivationLicenseClient.swift (1)
10-14: LGTM!Also applies to: 40-49, 137-152, 174-175
src/cli.ts (1)
3315-3315: LGTM!Also applies to: 3690-3699
src/license.ts (1)
124-125: LGTM!Also applies to: 190-190, 216-216, 504-504, 521-521
apps/neondiff-desktop/Tests/NeonDiffDesktopAppCoreTests/DesktopActivationLicenseClientTests.swift (1)
7-35: LGTM!apps/neondiff-desktop/Tests/NeonDiffDesktopCoreTests/ActivationLicenseClientTests.swift (1)
34-42: LGTM!Also applies to: 88-91
tests/license.test.ts (1)
451-486: LGTM!apps/neondiff-desktop/Sources/NeonDiffDesktopCore/Services/GitHubBrokerClient.swift (1)
529-549: LGTM!apps/neondiff-desktop/Tests/NeonDiffDesktopCoreTests/GitHubBrokerClientTests.swift (1)
198-199: LGTM!Also applies to: 223-223
services/license-api/src/github-broker/service.ts (1)
43-53: LGTM!Also applies to: 360-360, 402-403, 487-503, 549-585
services/license-api/test/github-broker-entitlement.test.ts (1)
88-92: LGTM!Also applies to: 102-105, 127-131
services/license-api/src/github-broker/index.ts (1)
28-28: LGTM!services/license-api/src/server.ts (1)
6-6: LGTM!Also applies to: 20-20, 43-48
services/license-api/test/github-broker-support.ts (1)
195-202: LGTM!
![evaos-code-review-bot[bot]](https://avatars.githubusercontent.com/in/4184532?s=60&v=4){kind=small}
There was a problem hiding this comment.
Walkthrough
PR: #641 - feat: bind native entitlement to broker identity
Head: 3cc4127eceab12e36676e0e9a77c4ebe18387ba0 into main. Review event: COMMENT.
Provider: GLM/Z.ai through ZCode (zcode-glm, zcode, model GLM-5.2).
Estimated review effort: 2/5 (~34 min)
Changed Files
| File | Status | Churn | Purpose | Risk |
|---|---|---|---|---|
README.md |
modified | +7/-0 | Documentation | Low |
docs/SETUP.md |
modified | +10/-0 | Documentation | Moderate: validated P2 finding |
docs/github-app-setup.md |
modified | +15/-3 | Documentation | Low |
docs/license-boundary.md |
modified | +8/-0 | Documentation | Low |
src/cli.ts |
modified | +15/-0 | Runtime code | Moderate: validated P3 finding |
src/license.ts |
modified | +6/-1 | Runtime code | Moderate: runtime path |
tests/license.test.ts |
modified | +37/-0 | Test coverage | Moderate: validated P2 finding |
Review Signal
Validated inline findings: 3 (P0: 0, P1: 0, P2: 2, P3: 1).
Dropped findings before posting: 2. High-severity findings: 0.
Risk Taxonomy
- Data loss: 1
- Proof gap: 1
- Security boundary: 1
Validation and Proof
1 required validation/proof recommendation(s) selected from changed files.
- required: TypeScript/web build or CI proof - Runtime TypeScript/web files or package/config files changed. Proof: npm run build; typecheck; focused Vitest; green GitHub check.
Proof status: sufficient - PR metadata mentions acceptable proof for each required validation recommendation.
Profile validation hints: Current-head, App-authored, no-secret, no-duplicate, and no-repo-mutation invariants outrank style feedback.
Profile proof expectations: Require focused tests, TypeScript build, App-backed doctor, release-status proof, and launchd heartbeat before live promotion.
Related Context
Related issues/PRs: #612, #613, #610, #630, #517.
Suggested labels: docs, tests.
Suggested reviewers: none from current metadata.
Review Settings Preview
- Profile: assertive
- Enabled sections: Review summary (inline_review); Walkthrough (inline_review); Changed-files table (walkthrough); Effort estimate (walkthrough); Related issues/PRs (walkthrough); Suggested labels (suggestion_only); Review status comment (sticky_status)
- Path instructions:
src/github.ts- Treat GitHub App identity or token-mode regressions as high risk. - Path instructions:
src/state.ts- Check duplicate suppression, leases, processed heads, and migration safety. - Path instructions:
src/worker.ts- Check stale-head guards, repo mutation protection, and review posting policy. - Path instructions:
src/zcode.ts- Check read-only ZCode policy, prompt constraints, and output parsing. - Label suggestions: bot, regression-hardening, release
- Reviewer suggestions: none
- Suggestion behavior: suggestions only; labels and reviewers are not auto-applied.
- Roadmap-only settings: auto-apply labels; auto-request reviewers; required status checks
Pre-merge checklist
- Inline comments target current RIGHT-side diff lines.
- No secret-like content survived into posted inline comments.
- REQUEST_CHANGES is only used when eligible P0/P1 findings survive validation.
- Required behavior proof is present or not applicable.
- Labels and reviewers are suggestions only; the bot did not auto-apply them.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3cc4127ece
ℹ️ 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".
evaOS review status: completedPR: #641 - feat: bind native entitlement to broker identity evaOS review completed for this PR head. Automation note: agents should wait for this comment to reach PR URL: #641 Review URL: #641 (review) |
There was a problem hiding this comment.
Walkthrough
PR: #641 - feat: bind native entitlement to broker identity
Head: 6ca5c8056f200c182550846fee315a5b26ed3d23 into main. Review event: COMMENT.
Provider: GLM/Z.ai through ZCode (zcode-glm, zcode, model GLM-5.2).
Estimated review effort: 3/5 (~44 min)
Changed Files
| File | Status | Churn | Purpose | Risk |
|---|---|---|---|---|
README.md |
modified | +7/-0 | Documentation | Low |
docs/SETUP.md |
modified | +10/-0 | Documentation | Low |
docs/github-app-setup.md |
modified | +15/-3 | Documentation | Low |
docs/license-boundary.md |
modified | +8/-0 | Documentation | Low |
src/cli.ts |
modified | +15/-0 | Runtime code | Moderate: runtime path |
src/license.ts |
modified | +6/-1 | Runtime code | Elevated: validated P1 finding |
tests/license.test.ts |
modified | +37/-0 | Test coverage | Moderate: validated P2 finding |
Review Signal
Validated inline findings: 3 (P0: 0, P1: 1, P2: 2, P3: 0).
Dropped findings before posting: 0. High-severity findings: 1.
Risk Taxonomy
- Proof gap: 1
- Runtime correctness: 1
- Security boundary: 1
Validation and Proof
1 required validation/proof recommendation(s) selected from changed files.
- required: TypeScript/web build or CI proof - Runtime TypeScript/web files or package/config files changed. Proof: npm run build; typecheck; focused Vitest; green GitHub check.
Proof status: sufficient - PR metadata mentions acceptable proof for each required validation recommendation.
Profile validation hints: Current-head, App-authored, no-secret, no-duplicate, and no-repo-mutation invariants outrank style feedback.
Profile proof expectations: Require focused tests, TypeScript build, App-backed doctor, release-status proof, and launchd heartbeat before live promotion.
Related Context
Related issues/PRs: #612, #613, #610, #630, #517.
Suggested labels: bug, docs, tests.
Suggested reviewers: none from current metadata.
Review Settings Preview
- Profile: assertive
- Enabled sections: Review summary (inline_review); Walkthrough (inline_review); Changed-files table (walkthrough); Effort estimate (walkthrough); Related issues/PRs (walkthrough); Suggested labels (suggestion_only); Review status comment (sticky_status)
- Path instructions:
src/github.ts- Treat GitHub App identity or token-mode regressions as high risk. - Path instructions:
src/state.ts- Check duplicate suppression, leases, processed heads, and migration safety. - Path instructions:
src/worker.ts- Check stale-head guards, repo mutation protection, and review posting policy. - Path instructions:
src/zcode.ts- Check read-only ZCode policy, prompt constraints, and output parsing. - Label suggestions: bot, regression-hardening, release
- Reviewer suggestions: none
- Suggestion behavior: suggestions only; labels and reviewers are not auto-applied.
- Roadmap-only settings: auto-apply labels; auto-request reviewers; required status checks
Pre-merge checklist
- Inline comments target current RIGHT-side diff lines.
- No secret-like content survived into posted inline comments.
- REQUEST_CHANGES is only used when eligible P0/P1 findings survive validation.
- Required behavior proof is present or not applicable.
- Labels and reviewers are suggestions only; the bot did not auto-apply them.
evaOS review status: completedPR: #641 - feat: bind native entitlement to broker identity evaOS review completed for this PR head. Automation note: agents should wait for this comment to reach PR URL: #641 Review URL: #641 (review) |
There was a problem hiding this comment.
Walkthrough
PR: #641 - feat: bind native entitlement to broker identity
Head: 8bc02cf9bc9cc3a02f21d5ddcf41d30fb9c4b407 into main. Review event: COMMENT.
Provider: GLM/Z.ai through ZCode (zcode-glm, zcode, model GLM-5.2).
Estimated review effort: 3/5 (~46 min)
Changed Files
| File | Status | Churn | Purpose | Risk |
|---|---|---|---|---|
README.md |
modified | +9/-0 | Documentation | Low |
docs/SETUP.md |
modified | +12/-0 | Documentation | Low |
docs/github-app-setup.md |
modified | +17/-3 | Documentation | Low |
docs/license-boundary.md |
modified | +10/-0 | Documentation | Low |
src/cli.ts |
modified | +15/-0 | Runtime code | Moderate: validated P3 finding |
src/license.ts |
modified | +16/-1 | Runtime code | Moderate: runtime path |
tests/license.test.ts |
modified | +76/-0 | Test coverage | Moderate: validated P3 finding |
tests/public-cli.test.ts |
modified | +4/-0 | Test coverage | Low |
Review Signal
Validated inline findings: 2 (P0: 0, P1: 0, P2: 0, P3: 2).
Dropped findings before posting: 0. High-severity findings: 0.
Risk Taxonomy
- API compatibility: 1
- Proof gap: 1
Validation and Proof
1 required validation/proof recommendation(s) selected from changed files.
- required: TypeScript/web build or CI proof - Runtime TypeScript/web files or package/config files changed. Proof: npm run build; typecheck; focused Vitest; green GitHub check.
Proof status: sufficient - PR metadata mentions acceptable proof for each required validation recommendation.
Profile validation hints: Current-head, App-authored, no-secret, no-duplicate, and no-repo-mutation invariants outrank style feedback.
Profile proof expectations: Require focused tests, TypeScript build, App-backed doctor, release-status proof, and launchd heartbeat before live promotion.
Related Context
Related issues/PRs: #612, #613, #610, #630, #517.
Suggested labels: docs, tests.
Suggested reviewers: none from current metadata.
Review Settings Preview
- Profile: assertive
- Enabled sections: Review summary (inline_review); Walkthrough (inline_review); Changed-files table (walkthrough); Effort estimate (walkthrough); Related issues/PRs (walkthrough); Suggested labels (suggestion_only); Review status comment (sticky_status)
- Path instructions:
src/github.ts- Treat GitHub App identity or token-mode regressions as high risk. - Path instructions:
src/state.ts- Check duplicate suppression, leases, processed heads, and migration safety. - Path instructions:
src/worker.ts- Check stale-head guards, repo mutation protection, and review posting policy. - Path instructions:
src/zcode.ts- Check read-only ZCode policy, prompt constraints, and output parsing. - Label suggestions: bot, regression-hardening, release
- Reviewer suggestions: none
- Suggestion behavior: suggestions only; labels and reviewers are not auto-applied.
- Roadmap-only settings: auto-apply labels; auto-request reviewers; required status checks
Pre-merge checklist
- Inline comments target current RIGHT-side diff lines.
- No secret-like content survived into posted inline comments.
- REQUEST_CHANGES is only used when eligible P0/P1 findings survive validation.
- Required behavior proof is present or not applicable.
- Labels and reviewers are suggestions only; the bot did not auto-apply them.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8bc02cf9bc
ℹ️ 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".
evaOS review status: completedPR: #641 - feat: bind native entitlement to broker identity evaOS review completed for this PR head. Automation note: agents should wait for this comment to reach PR URL: #641 Review URL: #641 (review) |
There was a problem hiding this comment.
Walkthrough
PR: #641 - feat: bind native entitlement to broker identity
Head: 3f0252e0ee2ed3430524735d771771d975d54f5e into main. Review event: COMMENT.
Provider: GLM/Z.ai through ZCode (zcode-glm, zcode, model GLM-5.2).
Estimated review effort: 3/5 (~46 min)
Changed Files
| File | Status | Churn | Purpose | Risk |
|---|---|---|---|---|
README.md |
modified | +9/-0 | Documentation | Low |
docs/SETUP.md |
modified | +12/-0 | Documentation | Low |
docs/github-app-setup.md |
modified | +17/-3 | Documentation | Low |
docs/license-boundary.md |
modified | +10/-0 | Documentation | Low |
src/cli.ts |
modified | +19/-1 | Runtime code | Moderate: validated P3 finding |
src/license.ts |
modified | +16/-1 | Runtime code | Moderate: runtime path |
tests/license.test.ts |
modified | +76/-0 | Test coverage | Low |
tests/public-cli.test.ts |
modified | +37/-28 | Test coverage | Low |
Review Signal
Validated inline findings: 1 (P0: 0, P1: 0, P2: 0, P3: 1).
Dropped findings before posting: 0. High-severity findings: 0.
Risk Taxonomy
- Runtime correctness: 1
Validation and Proof
1 required validation/proof recommendation(s) selected from changed files.
- required: TypeScript/web build or CI proof - Runtime TypeScript/web files or package/config files changed. Proof: npm run build; typecheck; focused Vitest; green GitHub check.
Proof status: sufficient - PR metadata mentions acceptable proof for each required validation recommendation.
Profile validation hints: Current-head, App-authored, no-secret, no-duplicate, and no-repo-mutation invariants outrank style feedback.
Profile proof expectations: Require focused tests, TypeScript build, App-backed doctor, release-status proof, and launchd heartbeat before live promotion.
Related Context
Related issues/PRs: #612, #613, #610, #630, #517.
Suggested labels: docs, tests.
Suggested reviewers: none from current metadata.
Review Settings Preview
- Profile: assertive
- Enabled sections: Review summary (inline_review); Walkthrough (inline_review); Changed-files table (walkthrough); Effort estimate (walkthrough); Related issues/PRs (walkthrough); Suggested labels (suggestion_only); Review status comment (sticky_status)
- Path instructions:
src/github.ts- Treat GitHub App identity or token-mode regressions as high risk. - Path instructions:
src/state.ts- Check duplicate suppression, leases, processed heads, and migration safety. - Path instructions:
src/worker.ts- Check stale-head guards, repo mutation protection, and review posting policy. - Path instructions:
src/zcode.ts- Check read-only ZCode policy, prompt constraints, and output parsing. - Label suggestions: bot, regression-hardening, release
- Reviewer suggestions: none
- Suggestion behavior: suggestions only; labels and reviewers are not auto-applied.
- Roadmap-only settings: auto-apply labels; auto-request reviewers; required status checks
Pre-merge checklist
- Inline comments target current RIGHT-side diff lines.
- No secret-like content survived into posted inline comments.
- REQUEST_CHANGES is only used when eligible P0/P1 findings survive validation.
- Required behavior proof is present or not applicable.
- Labels and reviewers are suggestions only; the bot did not auto-apply them.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f0252e0ee
ℹ️ 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".
evaOS review status: completedPR: #641 - feat: bind native entitlement to broker identity evaOS review completed for this PR head. Automation note: agents should wait for this comment to reach PR URL: #641 Review URL: #641 (review) |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@README.md`:
- Line 149: Rephrase the paragraph beginning with “#630” so it starts with
descriptive text, such as “Issue `#630` must wire those runtime callers before the
managed path can be enabled,” preventing markdownlint MD018 from interpreting
the issue reference as a heading marker.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: bdd6b149-ce6e-4b0e-9b11-bdee532d12db
📒 Files selected for processing (10)
README.mddocs/SETUP.mddocs/github-app-setup.mddocs/license-boundary.mdservices/license-api/src/github-broker/license-entitlement.tsservices/license-api/test/github-broker-production-entitlement.test.tssrc/cli.tssrc/license.tstests/license.test.tstests/public-cli.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: Swift desktop smoke
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Build, test, and package
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use focused local tests or smoke checks by default; treat npm test and npm run build as broad GitHub CI checks unless reproducing CI, a focused harness is unavailable, or the user explicitly requests broad validation.
Files:
tests/public-cli.test.tssrc/cli.tsservices/license-api/test/github-broker-production-entitlement.test.tstests/license.test.tssrc/license.tsservices/license-api/src/github-broker/license-entitlement.ts
README.md
📄 CodeRabbit inference engine (AGENTS.md)
Keep README.md as the public landing page containing product value, installation, setup, first dry-run review, safety boundaries, and links; do not put long release ledgers, raw local command dumps, un contextualized internal paths, or active sprint churn there.
Files:
README.md
{README.md,docs/SETUP.md,docs/github-app-setup.md}
📄 CodeRabbit inference engine (AGENTS.md)
For setup or GitHub App setup changes, update README.md, docs/SETUP.md, and docs/github-app-setup.md before claiming the first-run path is fixed.
Files:
README.mddocs/SETUP.mddocs/github-app-setup.md
docs/SETUP.md
📄 CodeRabbit inference engine (AGENTS.md)
Read and maintain docs/SETUP.md before changing installation, GitHub App, provider, daemon, or first-run behavior; keep first-run detail there.
Files:
docs/SETUP.md
🧠 Learnings (1)
📚 Learning: 2026-07-18T15:32:32.800Z
Learnt from: 100yenadmin
Repo: electricsheephq/evaos-code-review-bot-neondiff PR: 641
File: services/license-api/test/github-broker-production-entitlement.test.ts:136-172
Timestamp: 2026-07-18T15:32:32.800Z
Learning: In the TypeScript GitHub broker entitlement flow tests under services/license-api/test, it’s acceptable to keep the coverage split across the existing “production license-store repo binding” assertion and the separate “HTTPS authorization/mint boundary” assertion. Treat the seam between resolver output and mint authorization as covered as long as there is no new transformation or bypass introduced between the resolver output and the mint/authorization step.
Specifically, retain the pattern where: (1) the wrong requested private repository results in `{ status: "active", coveredPrivateRepositories: [] }`, and (2) when there is empty active coverage the system denies with `entitlement_scope_insufficient` and makes zero `createInstallationAccessToken` calls. Only flag as under-tested if the change adds a new mapping/filtering/caching step or alternative path between resolver output and mint authorization.
Applied to files:
services/license-api/test/github-broker-production-entitlement.test.ts
🪛 ast-grep (0.44.1)
tests/public-cli.test.ts
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { execFile } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
src/license.ts
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawnSync } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🪛 markdownlint-cli2 (0.23.0)
README.md
[warning] 149-149: No space after hash on atx style heading
(MD018, no-missing-space-atx)
🔇 Additional comments (9)
docs/SETUP.md (1)
208-218: LGTM!docs/github-app-setup.md (1)
55-77: LGTM!docs/license-boundary.md (1)
84-92: LGTM!src/cli.ts (1)
361-370: LGTM!Also applies to: 3623-3644
src/license.ts (1)
120-259: LGTM!Also applies to: 534-534
tests/license.test.ts (1)
426-427: LGTM!Also applies to: 453-540, 626-627
services/license-api/src/github-broker/license-entitlement.ts (1)
15-53: LGTM!services/license-api/test/github-broker-production-entitlement.test.ts (1)
77-110: LGTM!Also applies to: 145-176
tests/public-cli.test.ts (1)
150-150: 🎯 Functional CorrectnessNo duplicate
failuredeclarations here. The loop declaresfailureonce and assigns it in thecatch; there’s no duplicate block-scoped declaration in this block. Confidence 98%.> Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Walkthrough
PR: #641 - feat: bind native entitlement to broker identity
Head: c091c25e13f7f9fb4000602a0af37bfd72244ec5 into main. Review event: COMMENT.
Provider: GLM/Z.ai through ZCode (zcode-glm, zcode, model GLM-5.2).
Estimated review effort: 4/5 (~56 min)
Changed Files
| File | Status | Churn | Purpose | Risk |
|---|---|---|---|---|
README.md |
modified | +9/-0 | Documentation | Low |
docs/SETUP.md |
modified | +12/-0 | Documentation | Low |
docs/github-app-setup.md |
modified | +17/-3 | Documentation | Low |
docs/license-boundary.md |
modified | +10/-0 | Documentation | Low |
src/cli.ts |
modified | +19/-1 | Runtime code | Moderate: validated P2 finding |
src/license.ts |
modified | +36/-1 | Runtime code | Moderate: runtime path |
tests/license.test.ts |
modified | +130/-0 | Test coverage | Low |
tests/public-cli.test.ts |
modified | +37/-28 | Test coverage | Moderate: validated P3 finding |
Review Signal
Validated inline findings: 2 (P0: 0, P1: 0, P2: 1, P3: 1).
Dropped findings before posting: 0. High-severity findings: 0.
Risk Taxonomy
- Auth: 1
- Runtime correctness: 1
Validation and Proof
1 required validation/proof recommendation(s) selected from changed files.
- required: TypeScript/web build or CI proof - Runtime TypeScript/web files or package/config files changed. Proof: npm run build; typecheck; focused Vitest; green GitHub check.
Proof status: sufficient - PR metadata mentions acceptable proof for each required validation recommendation.
Profile validation hints: Current-head, App-authored, no-secret, no-duplicate, and no-repo-mutation invariants outrank style feedback.
Profile proof expectations: Require focused tests, TypeScript build, App-backed doctor, release-status proof, and launchd heartbeat before live promotion.
Related Context
Related issues/PRs: #612, #613, #610, #630, #517.
Suggested labels: docs, tests.
Suggested reviewers: none from current metadata.
Review Settings Preview
- Profile: assertive
- Enabled sections: Review summary (inline_review); Walkthrough (inline_review); Changed-files table (walkthrough); Effort estimate (walkthrough); Related issues/PRs (walkthrough); Suggested labels (suggestion_only); Review status comment (sticky_status)
- Path instructions:
src/github.ts- Treat GitHub App identity or token-mode regressions as high risk. - Path instructions:
src/state.ts- Check duplicate suppression, leases, processed heads, and migration safety. - Path instructions:
src/worker.ts- Check stale-head guards, repo mutation protection, and review posting policy. - Path instructions:
src/zcode.ts- Check read-only ZCode policy, prompt constraints, and output parsing. - Label suggestions: bot, regression-hardening, release
- Reviewer suggestions: none
- Suggestion behavior: suggestions only; labels and reviewers are not auto-applied.
- Roadmap-only settings: auto-apply labels; auto-request reviewers; required status checks
Pre-merge checklist
- Inline comments target current RIGHT-side diff lines.
- No secret-like content survived into posted inline comments.
- REQUEST_CHANGES is only used when eligible P0/P1 findings survive validation.
- Required behavior proof is present or not applicable.
- Labels and reviewers are suggestions only; the bot did not auto-apply them.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c091c25e13
ℹ️ 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".
evaOS review status: completedPR: #641 - feat: bind native entitlement to broker identity evaOS review completed for this PR head. Automation note: agents should wait for this comment to reach PR URL: #641 Review URL: #641 (review) |
There was a problem hiding this comment.
Walkthrough
PR: #641 - feat: bind native entitlement to broker identity
Head: 24c57e081762ec6e2959953d4aa9476f4da47aa5 into main. Review event: COMMENT.
Provider: GLM/Z.ai through ZCode (zcode-glm, zcode, model GLM-5.2).
Estimated review effort: 4/5 (~56 min)
Changed Files
| File | Status | Churn | Purpose | Risk |
|---|---|---|---|---|
README.md |
modified | +9/-0 | Documentation | Low |
docs/SETUP.md |
modified | +12/-0 | Documentation | Low |
docs/github-app-setup.md |
modified | +17/-3 | Documentation | Low |
docs/license-boundary.md |
modified | +10/-0 | Documentation | Low |
src/cli.ts |
modified | +19/-1 | Runtime code | Moderate: validated P3 finding |
src/license.ts |
modified | +36/-1 | Runtime code | Moderate: runtime path |
tests/license.test.ts |
modified | +130/-0 | Test coverage | Low |
tests/public-cli.test.ts |
modified | +37/-28 | Test coverage | Moderate: validated P3 finding |
Review Signal
Validated inline findings: 2 (P0: 0, P1: 0, P2: 0, P3: 2).
Dropped findings before posting: 0. High-severity findings: 0.
Risk Taxonomy
- Runtime correctness: 1
- Security boundary: 1
Validation and Proof
1 required validation/proof recommendation(s) selected from changed files.
- required: TypeScript/web build or CI proof - Runtime TypeScript/web files or package/config files changed. Proof: npm run build; typecheck; focused Vitest; green GitHub check.
Proof status: sufficient - PR metadata mentions acceptable proof for each required validation recommendation.
Profile validation hints: Current-head, App-authored, no-secret, no-duplicate, and no-repo-mutation invariants outrank style feedback.
Profile proof expectations: Require focused tests, TypeScript build, App-backed doctor, release-status proof, and launchd heartbeat before live promotion.
Related Context
Related issues/PRs: #612, #613, #610, #630, #517.
Suggested labels: docs, tests.
Suggested reviewers: none from current metadata.
Review Settings Preview
- Profile: assertive
- Enabled sections: Review summary (inline_review); Walkthrough (inline_review); Changed-files table (walkthrough); Effort estimate (walkthrough); Related issues/PRs (walkthrough); Suggested labels (suggestion_only); Review status comment (sticky_status)
- Path instructions:
src/github.ts- Treat GitHub App identity or token-mode regressions as high risk. - Path instructions:
src/state.ts- Check duplicate suppression, leases, processed heads, and migration safety. - Path instructions:
src/worker.ts- Check stale-head guards, repo mutation protection, and review posting policy. - Path instructions:
src/zcode.ts- Check read-only ZCode policy, prompt constraints, and output parsing. - Label suggestions: bot, regression-hardening, release
- Reviewer suggestions: none
- Suggestion behavior: suggestions only; labels and reviewers are not auto-applied.
- Roadmap-only settings: auto-apply labels; auto-request reviewers; required status checks
Pre-merge checklist
- Inline comments target current RIGHT-side diff lines.
- No secret-like content survived into posted inline comments.
- REQUEST_CHANGES is only used when eligible P0/P1 findings survive validation.
- Required behavior proof is present or not applicable.
- Labels and reviewers are suggestions only; the bot did not auto-apply them.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 24c57e0817
ℹ️ 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".
evaOS review status: completedPR: #641 - feat: bind native entitlement to broker identity evaOS review completed for this PR head. Automation note: agents should wait for this comment to reach PR URL: #641 Review URL: #641 (review) |
There was a problem hiding this comment.
Walkthrough
PR: #641 - feat: bind native entitlement to broker identity
Head: 204fdafbcd578fb0013752de2e728b6448009a87 into main. Review event: COMMENT.
Provider: GLM/Z.ai through ZCode (zcode-glm, zcode, model GLM-5.2).
Estimated review effort: 4/5 (~56 min)
Changed Files
| File | Status | Churn | Purpose | Risk |
|---|---|---|---|---|
README.md |
modified | +9/-0 | Documentation | Low |
docs/SETUP.md |
modified | +12/-0 | Documentation | Low |
docs/github-app-setup.md |
modified | +17/-3 | Documentation | Low |
docs/license-boundary.md |
modified | +10/-0 | Documentation | Low |
src/cli.ts |
modified | +19/-1 | Runtime code | Moderate: validated P3 finding |
src/license.ts |
modified | +46/-1 | Runtime code | Moderate: validated P3 finding |
tests/license.test.ts |
modified | +166/-0 | Test coverage | Low |
tests/public-cli.test.ts |
modified | +37/-28 | Test coverage | Moderate: validated P3 finding |
Review Signal
Validated inline findings: 3 (P0: 0, P1: 0, P2: 0, P3: 3).
Dropped findings before posting: 0. High-severity findings: 0.
Risk Taxonomy
- Auth: 1
- Runtime correctness: 1
- Security boundary: 1
Validation and Proof
1 required validation/proof recommendation(s) selected from changed files.
- required: TypeScript/web build or CI proof - Runtime TypeScript/web files or package/config files changed. Proof: npm run build; typecheck; focused Vitest; green GitHub check.
Proof status: sufficient - PR metadata mentions acceptable proof for each required validation recommendation.
Profile validation hints: Current-head, App-authored, no-secret, no-duplicate, and no-repo-mutation invariants outrank style feedback.
Profile proof expectations: Require focused tests, TypeScript build, App-backed doctor, release-status proof, and launchd heartbeat before live promotion.
Related Context
Related issues/PRs: #612, #613, #610, #630, #517.
Suggested labels: docs, tests.
Suggested reviewers: none from current metadata.
Review Settings Preview
- Profile: assertive
- Enabled sections: Review summary (inline_review); Walkthrough (inline_review); Changed-files table (walkthrough); Effort estimate (walkthrough); Related issues/PRs (walkthrough); Suggested labels (suggestion_only); Review status comment (sticky_status)
- Path instructions:
src/github.ts- Treat GitHub App identity or token-mode regressions as high risk. - Path instructions:
src/state.ts- Check duplicate suppression, leases, processed heads, and migration safety. - Path instructions:
src/worker.ts- Check stale-head guards, repo mutation protection, and review posting policy. - Path instructions:
src/zcode.ts- Check read-only ZCode policy, prompt constraints, and output parsing. - Label suggestions: bot, regression-hardening, release
- Reviewer suggestions: none
- Suggestion behavior: suggestions only; labels and reviewers are not auto-applied.
- Roadmap-only settings: auto-apply labels; auto-request reviewers; required status checks
Pre-merge checklist
- Inline comments target current RIGHT-side diff lines.
- No secret-like content survived into posted inline comments.
- REQUEST_CHANGES is only used when eligible P0/P1 findings survive validation.
- Required behavior proof is present or not applicable.
- Labels and reviewers are suggestions only; the bot did not auto-apply them.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 204fdafbcd
ℹ️ 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".
evaOS review status: completedPR: #641 - feat: bind native entitlement to broker identity evaOS review completed for this PR head. Automation note: agents should wait for this comment to reach PR URL: #641 Review URL: #641 (review) |
coderabbitai
Bot
added
review-policy
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
services/license-api/src/service.ts (1)
154-167: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftMake repository binding an atomic compare-and-upsert. Confidence: 96%.
The current read/check/write sequence allows concurrent processes to return success for different repositories on the same activation, while SQLite silently retains only one binding.
services/license-api/src/service.ts#L154-L167: consume an atomic store result and returnrepositoryBindingMismatchResult()when the requested repository conflicts.services/license-api/src/store.ts#L1002-L1005: condition the conflict update on compatible repository values and report whether the write succeeded.Add a focused two-connection race test; broad
npm testor build validation can remain in GitHub CI.🤖 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 `@services/license-api/src/service.ts` around lines 154 - 167, Make repository binding atomic across services/license-api/src/service.ts lines 154-167 and services/license-api/src/store.ts lines 1002-1005: update the store upsert operation to condition writes on compatible repository values and return whether the write succeeded, then have the activation flow consume that result and return repositoryBindingMismatchResult() on conflict. Add a focused two-connection race test covering concurrent activation requests for different repositories.Source: Coding guidelines
🤖 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.
Outside diff comments:
In `@services/license-api/src/service.ts`:
- Around line 154-167: Make repository binding atomic across
services/license-api/src/service.ts lines 154-167 and
services/license-api/src/store.ts lines 1002-1005: update the store upsert
operation to condition writes on compatible repository values and return whether
the write succeeded, then have the activation flow consume that result and
return repositoryBindingMismatchResult() on conflict. Add a focused
two-connection race test covering concurrent activation requests for different
repositories.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f77ff847-3b00-4c45-9a59-103b53d80c53
📒 Files selected for processing (9)
services/license-api/src/http.tsservices/license-api/src/service.tsservices/license-api/src/store.tsservices/license-api/test/http.test.tsservices/license-api/test/service.test.tssrc/cli.tssrc/license.tstests/license.test.tstests/public-cli.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: Swift desktop smoke
- GitHub Check: Build, test, and package
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use focused local tests or smoke checks by default; treat npm test and npm run build as broad GitHub CI checks unless reproducing CI, a focused harness is unavailable, or the user explicitly requests broad validation.
Files:
services/license-api/src/store.tsservices/license-api/test/http.test.tsservices/license-api/src/http.tsservices/license-api/test/service.test.tsservices/license-api/src/service.tstests/public-cli.test.tstests/license.test.tssrc/license.tssrc/cli.ts
🪛 ast-grep (0.44.1)
tests/public-cli.test.ts
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { execFile } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { execFile } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
src/license.ts
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawnSync } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawnSync } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawnSync } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🔇 Additional comments (8)
src/cli.ts (1)
367-369: LGTM!Also applies to: 3315-3315, 3634-3637, 3693-3703
src/license.ts (1)
22-24: LGTM!Also applies to: 127-128, 161-206, 239-239, 265-265, 553-553, 570-570
tests/license.test.ts (1)
28-28: LGTM!Also applies to: 427-428, 454-644, 692-693
tests/public-cli.test.ts (1)
28-29: LGTM!Also applies to: 148-185
services/license-api/src/http.ts (1)
48-49: LGTM!Also applies to: 320-330
services/license-api/src/service.ts (1)
116-124: LGTM!services/license-api/test/http.test.ts (1)
76-92: LGTM!services/license-api/test/service.test.ts (1)
57-74: LGTM!
There was a problem hiding this comment.
Walkthrough
PR: #641 - feat: bind native entitlement to broker identity
Head: 3e836a6856780db63f373b06d002fceb12d2abd6 into main. Review event: COMMENT.
Provider: GLM/Z.ai through ZCode (zcode-glm, zcode, model GLM-5.2).
Estimated review effort: 4/5 (~56 min)
Changed Files
| File | Status | Churn | Purpose | Risk |
|---|---|---|---|---|
README.md |
modified | +9/-0 | Documentation | Low |
docs/SETUP.md |
modified | +12/-0 | Documentation | Low |
docs/github-app-setup.md |
modified | +17/-3 | Documentation | Low |
docs/license-boundary.md |
modified | +10/-0 | Documentation | Low |
src/cli.ts |
modified | +19/-1 | Runtime code | Moderate: validated P3 finding |
src/license.ts |
modified | +55/-1 | Runtime code | Moderate: validated P3 finding |
tests/license.test.ts |
modified | +196/-0 | Test coverage | Low |
tests/public-cli.test.ts |
modified | +39/-28 | Test coverage | Low |
Review Signal
Validated inline findings: 2 (P0: 0, P1: 0, P2: 0, P3: 2).
Dropped findings before posting: 0. High-severity findings: 0.
Risk Taxonomy
- Migration: 1
- Runtime correctness: 1
Validation and Proof
1 required validation/proof recommendation(s) selected from changed files.
- required: TypeScript/web build or CI proof - Runtime TypeScript/web files or package/config files changed. Proof: npm run build; typecheck; focused Vitest; green GitHub check.
Proof status: sufficient - PR metadata mentions acceptable proof for each required validation recommendation.
Profile validation hints: Current-head, App-authored, no-secret, no-duplicate, and no-repo-mutation invariants outrank style feedback.
Profile proof expectations: Require focused tests, TypeScript build, App-backed doctor, release-status proof, and launchd heartbeat before live promotion.
Related Context
Related issues/PRs: #612, #613, #610, #630, #517.
Suggested labels: docs, tests.
Suggested reviewers: none from current metadata.
Review Settings Preview
- Profile: assertive
- Enabled sections: Review summary (inline_review); Walkthrough (inline_review); Changed-files table (walkthrough); Effort estimate (walkthrough); Related issues/PRs (walkthrough); Suggested labels (suggestion_only); Review status comment (sticky_status)
- Path instructions:
src/github.ts- Treat GitHub App identity or token-mode regressions as high risk. - Path instructions:
src/state.ts- Check duplicate suppression, leases, processed heads, and migration safety. - Path instructions:
src/worker.ts- Check stale-head guards, repo mutation protection, and review posting policy. - Path instructions:
src/zcode.ts- Check read-only ZCode policy, prompt constraints, and output parsing. - Label suggestions: bot, regression-hardening, release
- Reviewer suggestions: none
- Suggestion behavior: suggestions only; labels and reviewers are not auto-applied.
- Roadmap-only settings: auto-apply labels; auto-request reviewers; required status checks
Pre-merge checklist
- Inline comments target current RIGHT-side diff lines.
- No secret-like content survived into posted inline comments.
- REQUEST_CHANGES is only used when eligible P0/P1 findings survive validation.
- Required behavior proof is present or not applicable.
- Labels and reviewers are suggestions only; the bot did not auto-apply them.
1 participant
Related: #612
Related: #613
Tracker: #610
Summary
Bounded acceptance criteria
Explicit non-goals
Failing-first proof
Focused validation
git diff --check: passedBroad repository validation is delegated to current-head GitHub Actions.
Agent-authored change. No secrets, customer data, runtime mutation, deployment, or release action is included.