fix: Add a proper build step for fff-bun - #766
Conversation
We never build it and this seems to be not a good idea
📝 WalkthroughWalkthroughThe packages now publish compiled ChangesPackage distribution
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant ReleaseJob as npm publishing job
participant BunSetup as oven-sh/setup-bun@v2
participant Bun
participant Npm as npm publish
ReleaseJob->>BunSetup: Set up Bun
ReleaseJob->>Bun: Install frozen dependencies
ReleaseJob->>Bun: Run Bun package build
ReleaseJob->>Npm: Publish the built package
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/external-tests.yml (1)
211-214: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winDo not install floating Bun in CI.
npm install -g bunresolves a package outside the repository lockfile. Pin the exact Bun version or use a trusted, musl-compatible installer with a pinned version.Proposed fix
- name: Install bun - run: npm install -g bun + run: npm install -g bun@<pinned-version>🤖 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/external-tests.yml around lines 211 - 214, Update the “Install bun” step in the workflow to avoid the floating `npm install -g bun` dependency: install an explicitly pinned Bun version using a trusted musl-compatible installer, or pin the npm package version exactly. Keep the setup compatible with the existing fff-node build.Source: Linters/SAST tools
🤖 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/release.yaml:
- Around line 713-714: Update the release workflow’s oven-sh/setup-bun step to
use an immutable commit reference, set bun-version to "1.3.9", and enable
no-cache. Also regenerate packages/fff-bun/bun.lock so it matches the dependency
versions and names produced by make set-npm-version before bun install
--frozen-lockfile runs.
---
Nitpick comments:
In @.github/workflows/external-tests.yml:
- Around line 211-214: Update the “Install bun” step in the workflow to avoid
the floating `npm install -g bun` dependency: install an explicitly pinned Bun
version using a trusted musl-compatible installer, or pin the npm package
version exactly. Keep the setup compatible with the existing fff-node build.
🪄 Autofix
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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c07e5573-9d6f-48ab-bd8a-1bc46a8fa81a
📒 Files selected for processing (10)
.github/workflows/external-tests.yml.github/workflows/release.yamlpackages/fff-bun/package.jsonpackages/fff-bun/tsconfig.build.jsonpackages/fff-node/package.jsonpackages/fff-node/src/binary.tspackages/fff-node/test/e2e.mjspackages/fff-node/test/stress-515.mjspackages/fff-node/test/watch.mjspackages/fff-node/tsconfig.build.json
| - uses: oven-sh/setup-bun@v2 | ||
|
|
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- release workflow excerpt ---'
sed -n '670,735p' .github/workflows/release.yaml
printf '%s\n' '--- setup-bun references ---'
rg -n -C 3 'oven-sh/setup-bun|bun-version|cache:' .github package.json bun.lockb bun.lock 2>/dev/null || true
printf '%s\n' '--- version/install ordering ---'
rg -n -C 5 'npm version|yarn version|pnpm version|bun install|frozen-lockfile|packageManager|version:' .github/workflows/release.yaml package.json bunfig.toml 2>/dev/null || true
printf '%s\n' '--- tracked lockfiles and manifests ---'
git ls-files | rg '(^|/)(package\.json|bun\.lockb?|bunfig\.toml|.*lock.*)$' || trueRepository: dmtrKovalenko/fff
Length of output: 10681
🏁 Script executed:
#!/bin/bash
set -eu
sed -n '670,735p' .github/workflows/release.yaml
rg -n -C 3 'oven-sh/setup-bun|bun-version|cache:' .github package.json bun.lockb bun.lock 2>/dev/null || true
rg -n -C 5 'npm version|yarn version|pnpm version|bun install|frozen-lockfile|packageManager|version:' .github/workflows/release.yaml package.json bunfig.toml 2>/dev/null || true
git ls-files | rg '(^|/)(package\.json|bun\.lockb?|bunfig\.toml|.*lock.*)$' || trueRepository: dmtrKovalenko/fff
Length of output: 10546
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- npm publish workflow ---'
sed -n '735,790p' .github/workflows/release.yaml
printf '%s\n' '--- version mutation implementation ---'
rg -n -C 8 'set-npm-version|set_npm_version|npm.*version|VERSION' Makefile scripts .github/workflows/release.yaml
printf '%s\n' '--- Bun package manifest and lockfile header ---'
cat -n packages/fff-bun/package.json
sed -n '1,80p' packages/fff-bun/bun.lock
printf '%s\n' '--- upstream setup-bun action metadata ---'
curl -fsSL https://raw.githubusercontent.com/oven-sh/setup-bun/main/action.yml
printf '%s\n' '--- upstream setup-bun README cache/version references ---'
curl -fsSL https://raw.githubusercontent.com/oven-sh/setup-bun/main/README.md | rg -n -C 3 'bun-version|cache|pin|commit' | head -120Repository: dmtrKovalenko/fff
Length of output: 33276
🏁 Script executed:
#!/bin/bash
set -eu
sed -n '735,790p' .github/workflows/release.yaml
rg -n -C 8 'set-npm-version|set_npm_version|npm.*version|VERSION' Makefile scripts .github/workflows/release.yaml
cat -n packages/fff-bun/package.json
sed -n '1,80p' packages/fff-bun/bun.lock
curl -fsSL https://raw.githubusercontent.com/oven-sh/setup-bun/main/action.yml
curl -fsSL https://raw.githubusercontent.com/oven-sh/setup-bun/main/README.md \
| rg -n -C 3 'bun-version|cache|pin|commit' | head -120Repository: dmtrKovalenko/fff
Length of output: 33056
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- root package metadata ---'
cat -n package.json
printf '%s\n' '--- setup-bun cache implementation references ---'
curl -fsSL https://raw.githubusercontent.com/oven-sh/setup-bun/main/dist/setup/index.js \
| rg -n -o '.{0,120}(no-cache|cache|tool-cache|RUNNER_TOOL_CACHE).{0,180}' | head -80 || true
curl -fsSL https://raw.githubusercontent.com/oven-sh/setup-bun/main/dist/cache-save/index.js \
| rg -n -o '.{0,120}(no-cache|cache|tool-cache|RUNNER_TOOL_CACHE).{0,180}' | head -80 || true
printf '%s\n' '--- zizmor cache-poisoning documentation ---'
curl -fsSL https://raw.githubusercontent.com/woodruffw/zizmor/main/docs/audits.md \
| rg -n -C 8 'cache-poison|unpinned-uses|setup-bun' | head -160 || trueRepository: dmtrKovalenko/fff
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -eu
cat -n package.json
curl -fsSL https://raw.githubusercontent.com/oven-sh/setup-bun/main/dist/setup/index.js \
| rg -n -o '.{0,120}(no-cache|cache|tool-cache|RUNNER_TOOL_CACHE).{0,180}' | head -80 || true
curl -fsSL https://raw.githubusercontent.com/oven-sh/setup-bun/main/dist/cache-save/index.js \
| rg -n -o '.{0,120}(no-cache|cache|tool-cache|RUNNER_TOOL_CACHE).{0,180}' | head -80 || true
curl -fsSL https://raw.githubusercontent.com/woodruffw/zizmor/main/docs/audits.md \
| rg -n -C 8 'cache-poison|unpinned-uses|setup-bun' | head -160 || trueRepository: dmtrKovalenko/fff
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
import json
from pathlib import Path
manifest = json.loads(Path("packages/fff-bun/package.json").read_text())
lock = json.loads(Path("packages/fff-bun/bun.lock").read_text())
workspace = lock["workspaces"][""]
version = "9.9.9-test"
mutated = json.loads(json.dumps(manifest))
mutated["version"] = version
for name in mutated.get("optionalDependencies", {}):
mutated["optionalDependencies"][name] = version
locked_optional = workspace.get("optionalDependencies", {})
manifest_names = set(mutated.get("optionalDependencies", {}))
locked_names = set(locked_optional)
print("manifest version after make set-npm-version:", mutated["version"])
print("manifest optional dependency names:", sorted(manifest_names))
print("lockfile optional dependency names:", sorted(locked_names))
print("names missing from lockfile:", sorted(manifest_names - locked_names))
print("names only in lockfile:", sorted(locked_names - manifest_names))
print("manifest optional versions:", sorted(set(mutated["optionalDependencies"].values())))
print("lockfile optional versions:", sorted(set(locked_optional.values())))
print("version mismatch:", set(mutated["optionalDependencies"].values()) != set(locked_optional.values()))
PY
printf '%s\n' '--- Bun frozen-lockfile documentation ---'
curl -fsSL https://bun.sh/docs/pm/lockfile \
| rg -n -C 4 'frozen|lockfile|package.json' | head -100 || trueRepository: dmtrKovalenko/fff
Length of output: 672
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
import json
from pathlib import Path
manifest = json.loads(Path("packages/fff-bun/package.json").read_text())
lock = json.loads(Path("packages/fff-bun/bun.lock").read_text())
workspace = lock["workspaces"][""]
version = "9.9.9-test"
mutated = json.loads(json.dumps(manifest))
mutated["version"] = version
for name in mutated.get("optionalDependencies", {}):
mutated["optionalDependencies"][name] = version
locked_optional = workspace.get("optionalDependencies", {})
manifest_names = set(mutated.get("optionalDependencies", {}))
locked_names = set(locked_optional)
print("manifest version after make set-npm-version:", mutated["version"])
print("manifest optional dependency names:", sorted(manifest_names))
print("lockfile optional dependency names:", sorted(locked_names))
print("names missing from lockfile:", sorted(manifest_names - locked_names))
print("names only in lockfile:", sorted(locked_names - manifest_names))
print("manifest optional versions:", sorted(set(mutated["optionalDependencies"].values())))
print("lockfile optional versions:", sorted(set(locked_optional.values())))
print("version mismatch:", set(mutated["optionalDependencies"].values()) != set(locked_optional.values()))
PY
curl -fsSL https://bun.sh/docs/pm/lockfile \
| rg -n -C 4 'frozen|lockfile|package.json' | head -100 || trueRepository: dmtrKovalenko/fff
Length of output: 672
🏁 Script executed:
#!/bin/bash
set -u
python3 - <<'PY'
import re
from pathlib import Path
manifest_text = Path("packages/fff-bun/package.json").read_text()
lock_text = Path("packages/fff-bun/bun.lock").read_text()
def members(text, key):
match = re.search(r'"' + re.escape(key) + r'"\s*:\s*\{(.*?)\n\s*\}', text, re.S)
if not match:
raise SystemExit(f"missing {key}")
return dict(re.findall(r'"([^"]+)"\s*:\s*"([^"]+)"', match.group(1)))
manifest_optional = members(manifest_text, "optionalDependencies")
lock_optional = members(lock_text, "optionalDependencies")
version = "9.9.9-test"
manifest_names = set(manifest_optional)
mutated_versions = {name: version for name in manifest_names}
print("manifest optional dependency names:", sorted(manifest_names))
print("lockfile optional dependency names:", sorted(lock_optional))
print("names missing from lockfile:", sorted(manifest_names - set(lock_optional)))
print("names only in lockfile:", sorted(set(lock_optional) - manifest_names))
print("manifest optional versions after mutation:", sorted(set(mutated_versions.values())))
print("lockfile optional versions:", sorted(set(lock_optional.values())))
print("version mismatch:", set(mutated_versions.values()) != set(lock_optional.values()))
PY
printf '%s\n' '--- Bun frozen-lockfile documentation ---'
curl -fsSL https://bun.sh/docs/pm/lockfile 2>/dev/null \
| rg -n -C 4 'frozen|lockfile|package.json' | head -100 || trueRepository: dmtrKovalenko/fff
Length of output: 50374
🏁 Script executed:
sed -n '1,30p' /tmp/coderabbit-shell-logs/shell-output-H1S6f1Repository: dmtrKovalenko/fff
Length of output: 50373
Pin Bun and disable its cache before publishing.
oven-sh/setup-bun@v2 uses a mutable tag, installs latest, and enables its executable cache by default. Pin the action to a commit, set bun-version: "1.3.9", and use no-cache: true.
Also fix the stale packages/fff-bun/bun.lock: make set-npm-version changes dependency versions before bun install --frozen-lockfile, while the lockfile still contains 0.0.0 dependencies with different names.
🧰 Tools
🪛 zizmor (1.29.0)
[error] 713-713: runtime artifacts potentially vulnerable to a cache poisoning attack (cache-poisoning): enables caching by default
(cache-poisoning)
🤖 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/release.yaml around lines 713 - 714, Update the release
workflow’s oven-sh/setup-bun step to use an immutable commit reference, set
bun-version to "1.3.9", and enable no-cache. Also regenerate
packages/fff-bun/bun.lock so it matches the dependency versions and names
produced by make set-npm-version before bun install --frozen-lockfile runs.
Source: Linters/SAST tools
Brings in upstream fixes on top of our 0.18.0 fork: - fix(grep) dmtrKovalenko#756/dmtrKovalenko#764: literal/regex fallback now keeps an explicit FilePath scope so a top-level regex alternation can't leak matches outside the pinned path - fix dmtrKovalenko#754/dmtrKovalenko#765: missing clap attribute on no_content_indexing - fix(bun) dmtrKovalenko#766: proper build step for fff-bun - chore dmtrKovalenko#762: rename stale fff.nvim repo refs to fff Conflicts resolved preserving fork divergences: - update_check.rs / install-mcp.sh: kept our DEFAULT_REPO=abhijit-s/fff and FFF_UPDATE_REPO override; upstream's fff.nvim->fff rename does not apply since we target our own fork for update checks and installs - README.md: kept our rewritten Homebrew-tap/apt/fffctl install section; upstream only tweaked a release URL in a block our fork already removed - grep_tests.rs: add/add at EOF resolved as a union — kept our three cold-start readiness tests and adopted upstream's dmtrKovalenko#756 regex-scope test, reflowing the shared trailing braces so both functions close correctly grep.rs auto-merged; reviewed the dmtrKovalenko#756 fallback change against our surrounding fork logic and confirmed coherence. Makefile .PHONY targets and the per-root ignore feature (ignore.rs/main.rs) preserved; workspace stays at 0.18.0. Build green (fff-mcp/fff-engine/fff-ctl); fff-search + fff-engine tests 375 passed, 0 failed.
We never build it and this seems to be not a good idea
Summary by CodeRabbit
New Features
Bug Fixes