feat(logseq-cli): build the OCaml/Melange CLI front-end via opam-nix#45
Conversation
|
Claude finished @Bad3r's task in 5m 23s —— View job ReviewNo issues found. The prior review threads (gemini, codex, claude) cover the substantive concerns this diff raises:
Spot checks against the working tree:
|
There was a problem hiding this comment.
Code Review
This pull request migrates the Logseq CLI build from ClojureScript to OCaml compiled via Melange and bundled with Vite. It integrates opam-nix to resolve OCaml dependencies, introduces a separate pnpm workspace for the CLI bundle, and updates the loopback address binding patch to target the new OCaml implementation. The review feedback recommends explicitly adding and forwarding sqlite and zstd as build inputs to ensure that sqlite3 and tar --zstd are available in the sandboxed build environment.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5ead90d34e
ℹ️ 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".
|
/gemini review |
|
@codex review |
There was a problem hiding this comment.
Code Review
This pull request migrates the Logseq CLI build from ClojureScript (shadow-cljs) to OCaml compiled via Melange and bundled with Vite, aligning with upstream changes. It introduces opam-nix to resolve the OCaml/Melange dependency closure, adds a new cliBundlePnpmDepsHash field to the manifest to handle the separate cli/ pnpm workspace, and updates the build process, wrapper script, and checks accordingly. Additionally, the loopback binding patch is rewritten to target the new OCaml implementation in cli_platform.ml. There are no review comments, so no further feedback is provided.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 919f1bae87
ℹ️ 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".
Review feedback resolvedImplemented
Not changed (rationale)
Validation: |
logseq/logseq dbd220c95d moved the CLI front-end to OCaml/Melange under cli/. The melange packages and the humanize git pin are not supplied by the existing nixpkgs input, so the flake now pins opam-nix and opam-repository for the CLI package to resolve cli/logseq-cli.opam against OCaml 5.4.0. The lockfile records opam-nix's own nixpkgs pins, so this change stays behind the flake.lock sensitive-change gate.
logseq/logseq dbd220c95d replaced the shadow-cljs CLI front-end with cli/, dune build @Bundle, Melange, and a Vite bundle. The db-worker-node process remains a shadow-cljs release, so build.nix now combines the opam-nix OCaml closure, the root pnpm store, the cli/ pnpm store, and the Clojure dependency FOD. Add cliBundlePnpmDepsHash to the manifest validator, package scope, nightly manifest producer, and generated manifest data. The wrapper also pins LOGSEQ_DB_WORKER_NODE_SCRIPT and Node.js on PATH so doctor and graph commands can spawn the packaged worker from any current directory. The loopback OAuth patch moves to cli/lib/platform/node/cli_platform.ml because the pinned revision no longer compiles src/main/logseq/cli/auth.cljs. The port keeps both loopback address families and flushes the callback response before the login task settles. Validation from the pre-cleanup tree: nix build .#logseq-cli and git apply --check for patches/logseq-cli-auth-bind-loopback-address-families.patch against logseqRev 1d1ca70d13368dafdd1f4181d5a6d0dc3946e577.
The desktop tarballs ship static/js/logseq-cli.js, and logseq/logseq dbd220c95d now produces that file with opam exec -- pnpm cli:release instead of the old shadow-cljs CLI target. Mirror upstream build-desktop-release.yml by installing OCaml 5.4.0, fetching cli/ pnpm dependencies, and running the CLI release step between db-worker-node:bundle and webpack-app-build. Assert static/js/logseq-cli.js and the staged Logseq CLI skill after desktop:prepare-runtime-js so a dropped runtime staging step fails before the packaging jobs publish artifacts. Validation from the pre-cleanup tree: actionlint .github/workflows/build-desktop.yml.
The OCaml CLI's doctor command exits 0 with a per-check status table and does not print the old Doctor: ok summary. Keep the smoke check strict by requiring the db-worker-script row, then leave the graph create and list probe to exercise the packaged db-worker-node.js process. Validation from the pre-cleanup tree: nix build .#checks.x86_64-linux.logseq-cli-help.
The pinned Logseq runtime staging script no longer provides root-level runtime entries to compare against, so the old check failed before it could validate the ASAR contents. Derive each non-optional copyPairs destination under static/js/ and require the packaged ASAR to ship it as /js/<name>, which is the layout the desktop app loads at startup. Validation from the pre-cleanup tree: nix build .#checks.x86_64-linux.logseq-runtime-assets.
The repo documentation still described logseq-cli as a shadow-cljs CLI build with a single pnpm dependency hash. Since logseq/logseq dbd220c95d the CLI front-end is OCaml/Melange, while db-worker-node remains the shadow-cljs sidecar. Document the opam-nix closure, the separate cli/ pnpm bundle hash, the updated nightly build sequence, and the import-from-derivation effect that means --no-build and --offline evaluation still need the opam closure built or cached. Validation from the pre-cleanup tree: nix fmt.
Unresolved constants in non-optional repoRoot spread copyPairs made the ASAR drift guard skip required static/js entries. The parser now exits with the identifier and asks for a parser update instead of shrinking required-runtime-names. Validation: nixfmt modules/_checks/runtime-assets.nix nix-instantiate --parse modules/_checks/runtime-assets.nix >/dev/null nix build .#checks.x86_64-linux.logseq-runtime-assets
Review feedback resolvedAccepted and implemented:
Rejected:
Reliability impact:
Validation:
|
Summary
Tracks upstream Logseq's CLI rewrite. Since
logseq/logseqdbd220c95d, the CLI front-end lives undercli/, compiles OCaml 5.4 through Melange, and bundles with Vite. This PR builds that front-end inside Nix withopam-nix, keeps the spawneddb-worker-nodeas a shadow-cljs release, and updates the desktop nightly workflow so desktop tarballs stage the same OCaml CLI runtime.Commit stack
feat(flake): add opam-nix inputs for OCaml CLIPins
opam-nixandopam-repositoryso the flake can resolvecli/logseq-cli.opamwith OCaml 5.4.0.feat(logseq-cli): build OCaml CLI with opam-nixReworks the Nix CLI package around
dune build @bundle, thecli/pnpm workspace, the root pnpm runtime closure, the db-worker Clojure deps, the new manifest hash, the update script, the wrapper, and the ported OAuth callback patch.ci(build-desktop): compile OCaml CLI in nightly buildMirrors upstream's OCaml CLI release step in the reusable desktop build workflow and verifies the staged runtime files.
test(logseq-cli): match OCaml doctor outputUpdates the CLI smoke check to assert the OCaml
doctorstatus table while keeping the db-worker spawn probe.test(desktop): require staged runtime JS assetsDerives required ASAR
/js/*runtime entries fromprepare-desktop-runtime-js.mjsinstead of accepting the obsolete root-entry layout.docs(logseq-cli): document OCaml CLI packagingUpdates
README.mdandAGENTS.mdfor the OCaml/Melange CLI,cliBundlePnpmDepsHash, the desktop release step, and opam-nix IFD during evaluation.What changed
opam-nixand a pinnedopam-repositoryinput for the OCaml CLI closure.modules/_packages/logseq-cli/opam-deps.nixresolves OCaml 5.4.0, dune,melange*, andhumanize, and links Melange's stdlib to the path thatmelc -whereresolves underopam-nix.cliBundlePnpmDepsHashmanifest field plusmodules/_packages/logseq-cli/cli-pnpm-deps.nixfor the separatecli/pnpm workspace (viteandtransit-js). The manifest producer, validator, manifest data, and package scope were updated together.modules/_packages/logseq-cli/build.nixto mirror upstream's release flow: install root pnpm dependencies offline, install thecli/pnpm store offline, rundune build @bundle, stagestatic/logseq-cli.js, build and bundle the shadow-cljsdb-worker-node, and runprepare-cli-package.mjs.keytar.node, prunes build-only node-gyp intermediates, preserves the LinuxlibsecretRPATH, and putssqlite3onPATHfor thecli/pnpm store extraction.LOGSEQ_DB_WORKER_NODE_SCRIPTto the packaged worker and prepending the bundled Node.js toPATHbefore executingdist/logseq.js.localhostredirect URI, binds explicit127.0.0.1and::1loopback listeners, and flushes the HTTP callback response before settling the login task..github/workflows/build-desktop.ymlto install the OCaml andcli/pnpm dependencies, runopam exec -- pnpm cli:release, and verify the staged desktop runtime files (static/js/logseq-cli.js,static/js/db-worker-node.js, and the bundled Logseq CLI skill).logseq-runtime-assetsderives required ASAR/js/*entries fromprepare-desktop-runtime-js.mjs; the CLI help check matches the OCaml doctor's status table and boots the db-worker via a graph create/list probe.Validation
History cleanup validation on the rewritten head:
git diff --exit-code backup/feat-ocaml-cli-pre-cleanup-20260618T082948Z..HEADnix fmt -- --ciorigin/main..HEADorigin/main..HEADThe rewritten branch preserves the same final file tree as the pre-cleanup head. Earlier local validation recorded for that same tree:
nix build .#logseq-clinix build .#checks.x86_64-linux.logseq-runtime-assetsnix build .#checks.x86_64-linux.logseq-cli-helpnix build .#checks.x86_64-linux.logseq-cli-graph-querynix build .#checks.x86_64-linux.logseq-cli-login-callbackgit apply --checkandpatch -p1 --dry-runforpatches/logseq-cli-auth-bind-loopback-address-families.patchagainstlogseqRev1d1ca70d13368dafdd1f4181d5a6d0dc3946e577actionlint .github/workflows/build-desktop.ymlnix-instantiate --parse,nixfmt --check, anddeadnix --failon changed Nix files, plusnix fmtGitHub checks rerun on the force-pushed head
ce0d01426ff428bdc3c93c0d57b1b2f417ec1ce8.Review notes
flake.lockand.github/workflows/build-desktop.yml. The requiredstatus(security-review-approved)label is already applied.opam-nixintroduces IFD for the CLI dependency closure. CLI evaluation and builds can compile the OCaml 5.4.0 toolchain per system if those paths are not already cached.opam-nixvendors additional nixpkgs pins through locked inputs (nixpkgs-llvm17,nixpkgs-python38, andnixpkgs-python39), soaudit-flake-inputsremains the gate for unsupported or unexpected input drift.