Skip to content

feat: make QuickJS the default plugin engine everywhere - #2775

Merged
steipete merged 1 commit into
mainfrom
steipete/quickjs-default
Aug 8, 2026
Merged

feat: make QuickJS the default plugin engine everywhere#2775
steipete merged 1 commit into
mainfrom
steipete/quickjs-default

Conversation

@steipete

@steipete steipete commented Aug 8, 2026

Copy link
Copy Markdown
Owner

Summary

  • make QuickJS the default provider-plugin engine on every platform
  • retain JavaScriptCore as an Apple-only rollback engine and in A/B CI coverage
  • isolate the portable TypeScript transpiler and preserve true in-engine interruption, heap, and stack limits
  • add committed engine benchmark instrumentation and document the decision

Benchmark

August 8, 2026 baseline from a Swift debug build on an Apple M3 Ultra. Fetch values are 50 fixture-backed iterations reusing one context; memory is the rough macOS physical-footprint delta per retained context.

Engine Poe (50) OpenRouter (50) Crof (50) Rough peak delta/context
JavaScriptCore 321.237 ms 36.424 ms 23.917 ms 806.4 KiB
QuickJS 1000.066 ms 90.610 ms 38.119 ms 117.3 KiB

QuickJS is slower in relative terms, but the absolute fetch deltas remain below 10 ms per fetch at minutes-cadence polling. That trade is preferable to JavaScriptCore's roughly 7x larger measured context footprint, especially because QuickJS also provides true in-engine interrupt safety instead of abandoning an uninterruptible evaluation thread after timeout.

Rollback

On Apple platforms, set CODEXBAR_PLUGIN_ENGINE=jsc or enable the JavaScriptCore rollback in Settings → Debug → Provider Plugins, then restart CodexBar. An explicit environment selection overrides the persisted Debug setting.

Proof

  • full suite: 822/822 under the QuickJS default
  • full suite: 822/822 under the JavaScriptCore default
  • focused A/B engine runs: 64/64 for QuickJS and 64/64 for JavaScriptCore
  • post-rebase local gate: make check, make build, full make test, and swift build --target CodexBarLinuxTests
  • JavaScriptCore remains retained in A/B CI to detect drift
  • autoreview clean
  • coordinator live proof: no-flag Poe fetch succeeded with the new QuickJS default

🤖 Generated with Claude Code

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. P2 Normal priority bug or improvement with limited blast radius. labels Aug 8, 2026
@clawsweeper

clawsweeper Bot commented Aug 8, 2026

Copy link
Copy Markdown

Codex review: found issues before merge. Reviewed August 8, 2026, 5:13 PM ET / 21:13 UTC.

ClawSweeper review

What this changes

Changes provider plugins to use QuickJS by default, retains JavaScriptCore as an Apple rollback, and adds safety, test, CI, settings, benchmark, and documentation updates.

Merge readiness

⚠️ Needs maintainer review before merge - 4 items remain

This owner-authored PR remains useful but is not merge-ready: it contains unresolved changelog conflict markers and still documents a 2 MiB QuickJS limit while the implementation uses 1 MiB.

Priority: P2
Reviewed head: 0f26b232a8b7a43346ee8fed29ccee394d7f2aca
Owner decision: Required. See Decision needed.

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) Real-behavior evidence and coverage are useful, but concrete merge and documentation defects remain.
Proof confidence 🐚 platinum hermit (4/6) Sufficient (live_output): The PR body reports an after-fix no-flag Poe fetch under the QuickJS default, supplemented by QuickJS and JavaScriptCore A/B suites.
Patch quality 🦐 gold shrimp (3/6) 2 actionable review findings remain.

Verification

Check Result Evidence
Real behavior Verified Sufficient (live_output): The PR body reports an after-fix no-flag Poe fetch under the QuickJS default, supplemented by QuickJS and JavaScriptCore A/B suites.
Evidence reviewed 3 items Unresolved merge content: The submitted changelog hunk contains conflict markers, leaving competing release-note text and matching GitHub's dirty merge state.
Stack-limit history: The preceding QuickJS safety commit describes a 4 MiB native stack and 1 MiB JavaScript limit, while the changed documentation says 2 MiB.
Area provenance: QuickJS portability and the JavaScript provider cutover are established main-branch work in this runtime area.
Findings 2 actionable findings [P2] Resolve the committed changelog conflict
[P3] Document the 1 MiB QuickJS stack limit
Security None None.

How this fits together

Provider plugins execute JavaScript to collect provider usage data. Engine selection chooses QuickJS or the Apple rollback engine before limits and watchdogs govern execution and results reach CodexBar.

flowchart LR
  A[Provider plugin source] --> B[Engine selection]
  B --> C[QuickJS default]
  B --> D[JavaScriptCore rollback]
  C --> E[Watchdog and limits]
  D --> E
  E --> F[Usage fetch result]
  F --> G[CodexBar display]
Loading

Decision needed

Question Recommendation
Should the QuickJS default land after the conflict and documentation mismatch are repaired, with JavaScriptCore retained as the compatibility rollback? Land with rollback safeguards: Repair the two defects, then merge with the Apple rollback and A/B coverage retained.

Why: The runtime-default change is an intentional compatibility tradeoff that automated coverage cannot fully settle for third-party plugins.

Before merge

  • Resolve the committed changelog conflict (P2) - CHANGELOG.md contains <<<<<<<, =======, and >>>>>>> in the PR diff. This leaves competing release-note entries and prevents a clean merge; retain one accurate entry before rebasing.
  • Document the 1 MiB QuickJS stack limit (P3) - The implementation sets a 1 MiB JavaScript stack limit, but the changed plugin documentation says 2 MiB. Correct the public limit so plugin authors can reason about recursion failures.
  • Resolve merge risk (P1) - Existing provider plugins may behave differently after the engine default changes; the Apple-only JavaScriptCore rollback mitigates but does not eliminate that upgrade risk.

Findings

  • [P2] Resolve the committed changelog conflict — CHANGELOG.md:6-12
  • [P3] Document the 1 MiB QuickJS stack limit — docs/plugin-prototype.md:22-23
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Changed surface 35 files, +787/-155 lines The runtime default changes across execution, settings, CI, tests, documentation, and localization.

Merge-risk options

Maintainer options:

  1. Repair and revalidate before merge (recommended)
    Resolve the conflicted changelog hunk, correct the stack-limit documentation, rebase, and rerun both engine paths.
  2. Defer the default switch
    Keep JavaScriptCore as the Apple default if maintainers want additional compatibility observation.
Copy recommended automerge instruction
@clawsweeper automerge

Special instructions:
Resolve the CHANGELOG.md conflict to one accurate entry, document the 1 MiB QuickJS JavaScript stack limit, rebase onto main, and rerun both engine paths.

Technical review

Best possible solution:

Resolve the exceptional release-note conflict, correct the documented 1 MiB QuickJS limit, rebase, and retain the JavaScriptCore rollback plus A/B coverage when deciding whether to land the default switch.

Do we have a high-confidence way to reproduce the issue?

Not applicable as a bug report; the PR intentionally changes a runtime default and reports a live no-flag Poe fetch plus A/B engine coverage.

Is this the best way to solve the issue?

Unclear until the merge conflict and documentation mismatch are corrected; retaining the rollback is the narrowest compatibility safeguard for the proposed default.

Full review comments:

  • [P2] Resolve the committed changelog conflict — CHANGELOG.md:6-12
    CHANGELOG.md contains <<<<<<<, =======, and >>>>>>> in the PR diff. This leaves competing release-note entries and prevents a clean merge; retain one accurate entry before rebasing.
    Confidence: 0.99
  • [P3] Document the 1 MiB QuickJS stack limit — docs/plugin-prototype.md:22-23
    The implementation sets a 1 MiB JavaScript stack limit, but the changed plugin documentation says 2 MiB. Correct the public limit so plugin authors can reason about recursion failures.
    Confidence: 0.98

Overall correctness: patch is incorrect
Overall confidence: 0.97

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against af9bc9382f4c.

Labels

Label changes:

  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR body reports an after-fix no-flag Poe fetch under the QuickJS default, supplemented by QuickJS and JavaScriptCore A/B suites.

Label justifications:

  • P2: This broad runtime-default change has normal-priority compatibility impact without an emergency regression.
  • merge-risk: 🚨 compatibility: Existing provider plugins change JavaScript engines on upgrade, with JavaScriptCore available only as an Apple rollback.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🐚 platinum hermit and patch quality is 🦐 gold shrimp.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (live_output): The PR body reports an after-fix no-flag Poe fetch under the QuickJS default, supplemented by QuickJS and JavaScriptCore A/B suites.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body reports an after-fix no-flag Poe fetch under the QuickJS default, supplemented by QuickJS and JavaScriptCore A/B suites.

Evidence

What I checked:

Likely related people:

  • steipete: Authored the QuickJS portability and provider cutover work as well as all commits in this PR. (role: recent area contributor; confidence: high; commits: cee0fd07, 8fc67d7f, 8bf4041fbebc; files: Sources/CodexBarCore/Plugins/ProviderPluginRuntime.swift, Sources/CodexBarCore/Plugins/QuickJSProviderPluginEngine.swift)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Resolve the committed changelog conflict.
  • Correct the public QuickJS stack-limit value and refresh validation after rebasing.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (6 earlier review cycles)
  • reviewed 2026-08-08T17:50:34.145Z sha fc23e69 :: needs changes before merge. :: [P1] Expose the result box to the extracted transpiler
  • reviewed 2026-08-08T18:22:38.138Z sha fc23e69 :: needs changes before merge. :: [P1] Expose the result box to the extracted transpiler
  • reviewed 2026-08-08T18:51:48.048Z sha 919a309 :: needs maintainer review before merge. :: none
  • reviewed 2026-08-08T19:41:01.441Z sha 2b0d93d :: found issues before merge. :: [P3] Document the reduced QuickJS stack limit
  • reviewed 2026-08-08T19:50:46.795Z sha 2b0d93d :: needs changes before merge. :: [P3] Document the reduced QuickJS stack limit
  • reviewed 2026-08-08T20:56:23.344Z sha 064b22d :: found issues before merge. :: [P2] Remove the unresolved changelog conflict markers | [P3] Document the 1 MiB QuickJS stack limit

@clawsweeper clawsweeper Bot added merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Aug 8, 2026
@steipete
steipete force-pushed the steipete/quickjs-default branch from d635b4b to 064b22d Compare August 8, 2026 20:52
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Aug 8, 2026
Benchmark (test-target, both engines), QuickJS default on all platforms
with CODEXBAR_PLUGIN_ENGINE=jsc rollback (env + Debug pane), dedicated
4 MiB worker threads with true-interrupt watchdog, native stack headroom
for the overflow guard, CI crash-report collection, and the overflow
regression pinned to production stack geometry.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant