Skip to content

fix(server): list binary files outside Git repositories - #10686

Closed
StiensWout wants to merge 2 commits into
pingdotgg:mainfrom
StiensWout:t3code/list-non-git-binary-files
Closed

StiensWout wants to merge 2 commits into
pingdotgg:mainfrom
StiensWout:t3code/list-non-git-binary-files

Conversation

@StiensWout

@StiensWout StiensWout commented Sep 8, 2026 •

Copy link
Copy Markdown
Contributor

Non-Git projects omit images and other known binary file extensions because FFF 0.9.4 drops them while building its path index. The Files sidebar and file search cannot return entries that never reach that index.

Upgrade FFF to 0.10.6, where non-Git scans include those files. The existing Electron native-library patch now covers the package’s bundled ESM and CommonJS entries, and the Windows packaged-app probe follows the new entry path. Workspace test spies use the CommonJS entry loaded by the service.

Validation:

  • 133 focused workspace-index, desktop-artifact, and external-package tests passed.
  • The regression covers PNG, JPEG, PDF, .bin, and text files in a non-Git workspace.
  • Server typecheck passed with existing Effect suggestions.
  • Targeted formatting and lint passed, with three existing warnings in cli-external-packages.test.ts.
  • The server bundle built, and FFF loaded its native library through both ESM and CommonJS.

This changes server-side indexing and package staging, so screenshots are not applicable.

Closes #10628

Model: GPT-5.6 Sol. Harness: Codex in T3 Code. Rebase and CommonJS test integration by GPT-6 in T3 Code. Rebased onto current main by Claude Opus 5.5 (Claude Code in T3 Code).

Note

Fix listing binary files outside Git repos via ASAR unpacked path resolution

  • Updates @ff-labs/fff-node from 0.9.4 to 0.10.6 and replaces the old version-specific patch with a new one in patches/@ff-labs__fff-node@0.10.6.patch
  • The new patch adds ASAR binary-path resolution to both the CJS and ESM distributions: when a resolved binary sits under a .asar archive, the resolver checks for the corresponding .asar.unpacked path and uses it when present, otherwise keeps the original path
  • Updates the Windows native-load probe in scripts/build-desktop-artifact.ts to probe the fff-node entrypoint at its new top-level distribution path
  • Adds a non-git workspace fixture with JPEG, PNG, BIN, PDF, and Markdown files in WorkspaceEntries.test.ts to verify binary file listings outside Git repos
  • Risk: the ASAR resolver change affects all fff-node binary loading when packaged under Electron; verify that .asar.unpacked paths exist for every binary the resolver now selects, since missing unpacked files will cause load failures

Macroscope summarized f041f71.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Sep 8, 2026
macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Sep 8, 2026
@macroscopeapp

macroscopeapp Bot commented Sep 8, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — The PR changes production workspace indexing and packaged native-binary resolution through an FFF upgrade, with focused regression and packaging tests. It also adds a file-level static-analysis suppression, requiring human review.

Notes:

  • Diff unchanged. Approvability was decided on eligibility alone.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Sep 8, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: pingdotgg/t3code/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 65b2cffb-6c9a-4929-9406-faae6e42df4e

📥 Commits

Reviewing files that changed from the base of the PR and between 763ddd4 and ba1cd08.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (5)
  • apps/server/package.json
  • pnpm-workspace.yaml
  • scripts/build-desktop-artifact.test.ts
  • scripts/build-desktop-artifact.ts
  • scripts/lib/cli-external-packages.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.


📝 Walkthrough

Walkthrough

The PR upgrades @ff-labs/fff-node to version 0.10.6, updates its ASAR resolution patch and desktop package paths, and adds a non-Git workspace test for binary file listing.

Changes

FFF dependency and workspace update

Layer / File(s) Summary
FFF version and ASAR resolution
apps/server/package.json, pnpm-workspace.yaml, patches/@ff-labs__fff-node@0.10.6.patch
The project uses @ff-labs/fff-node version 0.10.6. Its CJS and ESM builds resolve to an existing .asar.unpacked binary path when one is available.
Desktop package integration
scripts/build-desktop-artifact.ts, scripts/build-desktop-artifact.test.ts, scripts/lib/cli-external-packages.test.ts
Desktop dependency fixtures and assertions use version 0.10.6. The Windows native-load probe and package path expectations use dist/index.js.
Workspace binary listing regression
apps/server/src/workspace/WorkspaceEntries.test.ts, apps/server/src/workspace/WorkspaceSearchIndex.test.ts
Workspace tests load the CJS package entry. The listing test checks that JPEG, PNG, BIN, PDF, and text files appear as file entries in a non-Git workspace.

Priority: ⬇️ Low

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

Change: Bug fix · Severity of issue fixed: Low

Suggested reviewers: juliusmarminge, t3dotgg

Merge Risk: ⚪ Minimal · up to ba1cd

The dependency and lockfile agree, and the ASAR patch applies to the selected package. No specific merge-blocking risk remains; proceed with normal checks.

Architecture Summary

Architecture risk: 🔵 Low · up to ba1cd

The change affects 4 systems.

Changed systems: scripts, apps/server, patches, pnpm-workspace.yaml

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — scripts (service) was modified; 3 changed files map to changed impact.
  • observed — apps/server (service) was modified; 3 changed files map to changed impact.
  • observed — patches (service) was modified; 1 changed file maps to changed impact.
  • observed — pnpm-workspace.yaml (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

  • observed — Modified behavior in patches/@ff-labs__fff-node@0.10.6.patch: The CJS build (dist/index.cjs) adds the resolveUnpackedAsarPath function, which rewrites the last .asar path segment to .unpacked and returns that path only when it exists, otherwise returning the original path.
  • observed — Modified behavior in patches/@ff-labs__fff-node@0.10.6.patch: resolveFromNpmPackage in the CJS build now returns resolveUnpackedAsarPath(binaryPath) instead of returning binaryPath directly when the binary exists.
  • observed — Modified behavior in patches/@ff-labs__fff-node@0.10.6.patch: The ESM build (dist/index.js) adds sep to the path import list.
  • observed — Modified behavior in patches/@ff-labs__fff-node@0.10.6.patch: The ESM build adds the same resolveUnpackedAsarPath function, using the imported sep and existsSync.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The PR satisfies the coding requirements in [#10628]. It upgrades @ff-labs/fff-node to 0.10.6. The non-Git WorkspaceEntries regression test checks JPEG, PNG, .bin, PDF, and text files. The patch…
Out of Scope Changes check ✅ Passed The changes remain within [#10628]. Dependency patching, native-load probes, package-entry updates, and test changes support the FFF upgrade and Electron packaging compatibility. The excluded `pnpm-lo…
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 5…
Title check ✅ Passed The title clearly and concisely describes the primary change: listing binary files outside Git repositories.
Description check ✅ Passed The description explains what changed, why it changed, validation performed, issue linkage, and why screenshots do not apply. It omits the template headings and checklist, but it provides the required…
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

Comment @coderabbitai help to get the list of available commands.

@StiensWout
StiensWout force-pushed the t3code/list-non-git-binary-files branch from 763ddd4 to ba1cd08 Compare September 30, 2026 06:34
@juliusmarminge juliusmarminge added the macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews label Oct 1, 2026 — with ChatGPT Codex Connector
@maria-rcks

Copy link
Copy Markdown
Collaborator

Note

Written by claude-opus-5-5 on behalf of Maria

Hi! We are cleaning up open PRs, and this one appears to have been created with an older model (gpt-5.6-sol). If this change is really important, we recommend rebuilding the PR with a newer model if possible.

@maria-rcks maria-rcks closed this Oct 11, 2026
@maria-rcks

Copy link
Copy Markdown
Collaborator

Note

Written by claude-opus-5-5 on behalf of Maria

Reopening, this was closed by mistake. Sorry for the noise!

@maria-rcks maria-rcks reopened this Oct 11, 2026
@maria-rcks

Copy link
Copy Markdown
Collaborator

Note

Written by claude-opus-5-5 on behalf of Maria

Closing again after a second look, sorry for the back and forth. This PR has merge conflicts with main and was opened more than two weeks ago. If this change is still important, please rebuild it on current main with a newer model and note the model in the PR description.

@maria-rcks maria-rcks closed this Oct 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews size:L 100-499 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Images and other binary file types are missing from Files sidebar in non-Git projects

3 participants