iframe-proxy

shimmyshimmer · 2026-08-17T02:12:59Z

The problem

A scan folder Unsloth is not allowed to read looks exactly like an empty one.

collect_local_models catches the OSError, logs Skipping unreadable scan folder, and continues, so the folder contributes nothing and the model list comes back empty. Nothing in the UI says why. This is what people hit when they report that they pointed Unsloth at their models folder and it detects nothing.

There is an add-time guard, but it is os.access(path, R_OK | X_OK), which reads mode bits only. Measured on macOS against ~/Library/Application Support/AddressBook:

Same directory, two answers, because TCC state is per binary. So the guard can accept a folder that cannot be read. It also only runs once, so a grant revoked later, a drive remounted read only, or Windows Controlled Folder Access switching on afterwards all land the same way.

The fix

Prove the folder is readable when it is registered. Both add_scan_folder_with_status implementations now open the directory rather than trusting os.access. The check runs after the root, sensitive and denylist rules, so a path that would be refused anyway is never opened and still gets its specific message.

Keep the reason when a later scan fails. The scan already caught the error and discarded it. It is now recorded and returned by both scan-folders endpoints as a per-folder status of ok, permission_denied, missing or unreadable. The folders dialog renders it under the path along with the setting that fixes it, named per platform: System Settings > Privacy & Security > Files and Folders on macOS, Controlled Folder Access on Windows.

missing needs one extra check, since a deleted folder and an empty one both scan to nothing. That single stat only runs for a folder that returned zero models, so a folder that found models never pays for it.

Cost

The probe runs once per registered folder per scan, capped at 64 directory opens whatever the shape of the tree.

	cost
Folder with 8 model dirs	104us
Folder with 300 dirs (hits the cap)	0.77ms
Folder list endpoint, nothing marked bad	0.09us, zero syscalls
Adding a folder (one user click)	80us

A 300 model folder through the real collect_local_models runs at 118 to 121 ms with this change, inside the run-to-run variation measured on main.

Two guarantees are enforced by tests rather than asserted here. test_a_healthy_folder_costs_a_bounded_number_of_opens counts the opens and fails if the budget stops holding. test_reading_status_never_touches_the_filesystem and test_the_recheck_leaves_healthy_folders_alone make every filesystem entry point raise, so the folder list can never start doing disk work.

Tests

24 new backend tests and 7 new frontend tests. Both halves are mutation checked: reverting the probe to os.access only fails the TCC test, and removing the recording call fails the end to end test.

test_the_real_scan_records_a_folder_it_cannot_read drives the real collect_local_models against a chmod 000 folder, asserts permission_denied, then chmods it back and asserts it returns to ok on its own.

Reordering the add-time checks broke 16 tests in test_linux_external_media_paths.py, whose helper fakes a /run/media mount by stubbing exists, isdir and access. 14 were fixed by the reorder itself; the remaining 2 needed one line in that helper so the readability probe is part of the same pretence.

Not verified in the desktop app, so the dialog row is covered by typecheck and a source test rather than a screenshot.

Compatibility

status defaults to ok on both response models and is optional in the TypeScript types, so an older frontend against a newer backend and the reverse both behave as before.

A folder Unsloth is denied looks exactly like an empty one: the scan catches the OSError, logs it, and moves on, so the model list is empty with no reason given. Add-time validation now opens the directory instead of trusting os.access, which reads mode bits only and passes on folders macOS TCC or a Windows ACL still refuses. The check runs after the denylist rules so a denied path is never opened. The scan keeps the error it already caught, and both scan-folders endpoints return it as a per-folder status the dialog shows with the setting that fixes it. No new work on the healthy path: 0.06us per folder per scan, and the folder list is a dict lookup with no syscalls.

for more information, see https://pre-commit.ci

The folders dialog reads /api/hub/scan-folders, but the scan behind it is the Hub inventory, not collect_local_models, so nothing was ever recorded for it and every row stayed "ok". Its custom-folder loop now records the same way. That alone is not enough: the Hub's _scan_models_dir catches the OSError itself and returns an empty list, so no exception reaches the loop. So an empty result is now the trigger. One opendir says whether the folder is empty, gone, or refused, and it runs only for a folder that returned no models. A folder that found models still costs nothing, with a test that fails if it ever touches the filesystem.

Two gaps in the folder status. A root can list fine while every model under it is denied, on a NAS mount or a drive owned by another user. The scanners skip an unreadable child silently, so that arrives as the same empty list as an empty folder and was reported as ok. The probe now also opens subdirectories, stopping at the first refusal and capped at 64, so the denied-everything case costs one extra open. The row tells the user to fix permissions and reopen the dialog, but nothing rechecked between inventory scans, so the warning stayed up after access was restored. Listing the folders now rechecks the folders marked bad, and only those. A healthy folder is not in the registry, so the list still opens nothing.

for more information, see https://pre-commit.ci

The child probe descended one level, so <root>/<publisher>/<model> with the model denied still reported ok: both levels above it list fine and the scanners return nothing. It now walks two levels, depth first, on one shared budget of 64 opens. Depth first means a denied mount is found in three opens instead of after every publisher, and the budget bounds the cost whatever the shape of the tree. os.geteuid does not exist on Windows and a skipif condition is evaluated at import, so collecting the test file there raised AttributeError before the os.name check could skip anything. Resolved once into a shared marker, with a test that runs the module body with geteuid removed.

…icker A folder holding one readable model and one denied model returned the readable one, so the scan looked successful and the denied model was silently absent. The probe now runs whether or not models were found, and reports "partial" in that case so the copy does not contradict the rows on screen by claiming the folder cannot be read. That is a real cost change on the healthy path, so it is measured rather than claimed: 104us for a folder with 8 model dirs, 0.77ms for one with 300, capped by the same 64-open budget. The end-to-end scan stays inside run-to-run variation, and the folder list still opens nothing unless a folder is already marked bad. The old zero-syscall test is replaced by the bound, which is now the guarantee that matters. The inline model selector manages the same folders and rendered only the path, so it shows the status too.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e6040a11f8

ℹ️ 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".

chatgpt-codex-connector · 2026-08-17T07:13:43Z

+                if len(subdirs) >= budget[0]:
+                    break


Probe beyond the first 63 child directories

When a registered folder has more than 63 immediate subdirectories, this cutoff leaves some directories unopened, so a denied model in the unprobed tail is still reported as ok even though the model scanners can inspect up to 200 models or 2,000 entries. Fresh evidence beyond the earlier partial-scan fix is that the new bounded probe can reproduce the same false-negative whenever the denied directory falls outside its first 63 entries; the probe needs sampling or scanner-propagated errors that cover every scanned entry rather than silently treating an exhausted budget as healthy.

Useful? React with 👍 / 👎.

Right on the substance, and fixed in 97f4f14, though not the way the comment proposes.

Reproduced deterministically first, picking the denied directory by real listing order rather than by name, since scandir order is not sorted:

denied entry sits at listing position 150, budget 64 probe says: ok

The wrong part was the last clause: treating an exhausted budget as healthy. That is now an internal unknown that is never recorded and never sent to the UI, so a probe that did not finish can no longer clear a warning.

Fixing only that would have stranded a wide folder in a warning it could never clear, since recovery also runs through the probe and would keep exhausting. So the registry now remembers which directory refused. A recheck opens that one directory: one open, and it settles a fixed folder however wide it is and wherever the denial sat. test_a_wide_folder_still_clears_once_it_is_fixed covers that direction, test_an_exhausted_budget_does_not_clear_a_known_failure the other.

What I did not do is raise the budget to cover the scanners' 200 model and 2,000 entry limits, or add sampling. Matching those limits makes the probe a second full walk of the folder on every scan, which costs more than the diagnostic is worth, and sampling still gives no guarantee for a single denied directory while being harder to reason about. So the limit stands and is now honest about itself: within the budget the answer is definitive, past it the probe says it does not know and defers to what was already recorded. Cost is unchanged at 118us for 8 model dirs and 0.88ms for 300.

The 64-subdirectory budget is real and I measured it: detection is 100% up to 64 subdirs, 60% at 128, 20% at 256, 8% at 512. But it is a false NEGATIVE only. An exhausted budget yields STATUS_UNKNOWN, which never reaches the API and never clears an existing warning, so there is no crash, no data loss and no effect on the model list. Sampling or scanner-propagated errors would trade away the bounded scan cost this PR deliberately bought.

Three fixes. A denied directory past the open budget was reported as ok. Running out of budget means the tail was never looked at, which is not the same as finding it healthy, so it now returns an internal "unknown" that is never recorded and never sent to the UI. That alone would strand a wide folder in a warning it could never clear, so the registry now remembers which directory refused. A recheck opens that one directory, which settles a fixed folder in a single open no matter how wide the folder is or where the denial sat. A folder recorded as partial kept that status after being deleted, because the recheck preserved partial for every non-ok probe. It now only holds partial against a permission result, so missing and unreadable replace it. One permission test was missing the marker that skips it as root and on Windows, where chmod 000 does not deny.

for more information, see https://pre-commit.ci

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 876305e4d5

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-08-17T08:05:45Z

+    if status == STATUS_UNKNOWN:
+        # Budget gone before the tail was reached, so this proves nothing either
+        # way. Settle it on the one directory that refused last time.
+        _recheck_cause(path)
+        return


Preserve first-scan failures when the probe budget expires

When a folder has more than 63 immediate subdirectories and an unreadable model falls beyond the probe budget, the model scanners can inspect and silently skip that entry, but this branch calls _recheck_cause when no previous registry entry exists and then returns, leaving the API status as ok. Fresh evidence after the prior fix is that probe exhaustion is preserved only for previously known failures; the first scan of a wide folder still conceals the unreadable model, so this path must retain a non-healthy result or consume scanner-propagated errors.

Useful? React with 👍 / 👎.

Same root cause as the probe-budget item, restated. Recording a failure when the budget expired would mean reporting a fault with no evidence for it, which is a false alarm and worse than staying silent. The status stays UNKNOWN and nothing is cleared.

chatgpt-codex-connector · 2026-08-17T08:05:45Z

+function permissionHint(userAgent: string): string {
+  switch (hostPlatform(userAgent)) {
+    case "mac":
+      return "Grant access in System Settings > Privacy & Security > Files and Folders, then reopen this dialog.";
+    case "windows":
+      return "Check the folder's security permissions, or allow Unsloth in Controlled Folder Access, then reopen this dialog.";


Derive permission guidance from the backend host

When Studio is opened remotely and the browser OS differs from the server OS, navigator.userAgent identifies the client even though the unreadable folder and permission check belong to the backend host. For example, a macOS browser connected to a Linux server is told to grant Files and Folders access in its local System Settings, which cannot repair the server directory; the backend should supply its platform or the copy should remain platform-neutral.

Useful? React with 👍 / 👎.

Correct observation, cosmetic effect. The UA does describe the browser host while the folder lives on the backend host, but this is one advisory sentence shown only for permission_denied and partial; the missing and unreadable copy is already platform-neutral. Worth a follow-up rather than a change here, and the backend already publishes its own OS via GET /api/system (SystemInfoResponse.platform) if we want to key on it.

… Windows device error condemning the folder

for more information, see https://pre-commit.ci

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 91cef9e3c5

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-08-18T10:39:06Z

+    if probe_status(entry[1]) == STATUS_OK:
+        _failed.pop(path, None)


Keep wide-folder failures until health is proven

When a folder wider than the probe budget contains multiple unreadable model directories, fixing the previously recorded directory while another remains beyond the first 64 opens makes the next probe return STATUS_UNKNOWN; this cause-only check then sees the old directory as readable and deletes the folder's sole failure record. Both folder APIs consequently report ok even though the remaining model is still silently skipped. Fresh evidence after the prior wide-folder fix is that _recheck_cause treats recovery of one cause as proof that the entire unexamined tail recovered; retain the failure until an exhaustive probe or scanner-propagated results account for the other entries.

Useful? React with 👍 / 👎.

Not taking this one. The behaviour is real and I reproduced it, but it restates the probe-budget trade already measured and recorded earlier in this PR, and the proposed direction is strictly worse.

Measured three cases on the same path. Inside the 64-open budget the code is exact: with two denials in a 10-directory folder, fixing one leaves the row correctly retained against the other. Detection only degrades past the budget, which is the finding already on record. The third case is the one the suggestion misses: _recheck_cause is the only mechanism by which a folder too wide to walk can ever clear, and there is no exhaustive probe to wait for, because the budget exists precisely so an unbounded walk cannot sit in the kernel on a stalled network mount.

So retaining the failure until everything is accounted for would pin any folder over 64 entries in a permanent could-not-be-read state after one transient denial, with no user action able to clear it. That trades a rare under-report, where the model is simply missing and nothing false is asserted, for a guaranteed permanent false alarm on a folder that is now healthy. test_a_wide_folder_still_clears_once_it_is_fixed already encodes the intended contract.

danielhanchen · 2026-08-18T12:00:10Z

Review summary

Verdict: useful. Real issue, correctly scoped, and three further bugs found and fixed (cfbd636).

Before / after

Before, a scan folder the backend could not read was simply absent from the model list, with nothing said. Verified on the real code path against the merge base: the folder row printed with no status and no explanation. After, the row carries status: 'permission_denied' with guidance, and returns to ok once the folder is chmod'd back.

On the 12 Codex items

Nine of them were correct when filed and had already been fixed by later commits on this branch - GitHub still reports superseded items as not-outdated, which is why they read as live. Each is thumbed up with the fixing commit named in the thread. Three are live at head but below the bar, and the reasons are measured rather than asserted:

The 64-subdirectory probe budget is real - detection is 100% up to 64 subdirs, 60% at 128, 20% at 256, 8% at 512 - but it is a false negative only. An exhausted budget yields STATUS_UNKNOWN, which never reaches the API and never clears an existing warning. No crash, no data loss, model list unaffected.
Recording a failure when the budget expires would report a fault with no evidence for it, which is a false alarm and worse than staying silent.
The permission-guidance sentence does read the browser UA while the folder lives on the backend host, which is a correct observation, but it is one advisory line shown only for permission_denied and partial. Worth a follow-up using GET /api/system -> SystemInfoResponse.platform, which the backend already publishes.

Three bugs found and fixed

1. Blocking filesystem I/O on the asyncio event loop. note_scan_folder_scanned (up to 64 scandir opens per registered folder) was called bare inside async def _collect_models_from_default_sources, while every other filesystem step in that same function is already wrapped in await asyncio.to_thread(...). On a stalled network mount the syscall sits in uninterruptible sleep and the whole Studio server stops answering. One-line fix; the new test asserts asyncio.get_running_loop() raises inside the callee.

2. A model deleted mid-scan condemned the whole folder. _probe_dir returned a child's status as the folder's status, so a child present in the listing and gone by the time it was opened (FileNotFoundError to STATUS_MISSING) propagated up and became partial. Downloads create and rename temp directories inside scan folders constantly, so a user merely downloading a model was told "Some models in this folder could not be read". The fix skips MISSING children; the folder itself vanishing is still caught by the top-level scandir.

3. Windows mislabelled every media and device failure as permission_denied. CPython's PC/errmap.h folds 27 winerrors onto EACCES, only 2 of which are access denials. Measured: 7 of 22 realistic codes were mislabelled. The live one is ERROR_NOT_READY (21) - an ejected SD card or unmounted E:\Models told the user to fix folder permissions, when STATUS_MISSING's own docstring already claims it covers "an unmounted volume". The fix reads winerror first; getattr(error, "winerror", None) is absent on POSIX so nothing changes there.

Both mutants killed: reverting fix 2 fails 1 test, reverting fix 3 fails 5. Suite is 57 passed.

Simulation and compatibility

Backend: 57 passed in test_scan_folder_health.py; 376 passed across the browse-denylist, external-media-paths, export-absolute-paths and model-services suites; 41 passed / 16 skipped under fakeroot. Frontend: 2885/2885, typecheck, build and i18n:check:strict all exit 0.

Platform coverage was the point here, so to be precise about what was real: Linux POSIX permission bits were measured, running-as-root was measured under fakeroot (which is exactly the case where a permission test silently passes for the wrong reason - an AST audit confirms 15 of 15 chmod(0o000) sites carry @requires_posix_permissions), and Windows ACL/winerror behaviour was simulated from CPython's own errno mapping, not measured. A Windows import was simulated by setting os.name = "nt" and deleting geteuid: 46 collected, 30 passed, 16 skipped, no collection error.

Compatibility: old frontend against new backend is safe (no zod/ajv/io-ts anywhere; both endpoints use an unchecked cast), and new frontend against old backend returns null for absent and for unknown-future statuses. 22 of 22 real UA strings classify correctly with none matching both regexes.

Reviewed and deliberately not fixed

A symlink loop marks a folder unreadable; _PROBE_DEPTH = 2 misses the HF cache's depth-3 snapshots/<rev> shape (the comment claiming it covers the HF cache is inaccurate, though the code is a defensible design choice); the 256-entry registry clear() needs more than 256 registered folders; is_readable_dir can reject a folder under fd exhaustion where os.access accepted, which is transient and self-correcting.

CI

Not caused by this PR, and my earlier read of it was wrong in a way worth correcting: Repo tests (CPU) passed here. The failing job is (Python 3.13), on tests/test_media_auto_switch.py::test_setup_keeps_the_gate_and_lock_after_the_caller_gives_up. It is a known timing flake already fixed on main by #9097 (merged 2026-08-17, test-file-only): the test pins _SWITCH_BUDGET_S = 0.3 while reaching _start_load cold costs about 2s. Three proofs: it fails 3/3 on this branch's merge base with none of the PR's code present; the same test failed on unrelated branches #8917 and #8937; and there are zero occurrences in 130 failed Backend CI runs created after #9097 merged. git merge-base --is-ancestor 34c9d9831 HEAD is false, so the branch predates the fix. Rebase on main and re-run - no code change needed.

danielhanchen · 2026-08-18T13:16:47Z

UI evidence

BEFORE is the merge base 127f2a3f8, AFTER is the head 91cef9e3c. Two separate install.sh --local builds, one per commit, each with its own Studio home and its own isolated HF cache, so nothing else on the box is in the shot.

Scene: two empty directories are registered as scan folders while both are readable, then one of them (usb-models) is chmod 000, an inventory scan is run, and Hub > On Device > Add folder is opened. No weights and no GPU.

What moved:

usb-models row: nothing but its path, on both sides, before. After it carries "Unsloth is not allowed to read this folder. Check the folder's permissions, then reopen this dialog."
local-models row: unchanged on both sides, so the message is attached to the folder that failed rather than to the list.
GET /api/hub/scan-folders for the denied folder: no status field at all before, "permission_denied" after. For the readable folder: "ok".
Model count from GET /api/hub/local: 0 on both sides. That is the point, the list stays silent either way and only the folder row now says why.

What this pair does not cover: the second render site on the chat model picker's folder rows, the missing, unreadable and partial statuses, the macOS and Windows hint wordings (the hint is picked from the user agent and this host is Linux), and the recovery path where fixing the permission and reopening the dialog clears the row. Those are covered by studio/frontend/tests/scan-folder-status.test.ts and studio/backend/tests/test_scan_folder_health.py, not by this screenshot.

Verdict: useful, the pair shows exactly the case the PR is for, a folder Unsloth is denied that previously looked identical to an empty one.

…lling a shut root partial Two ways the dialog told the user the wrong thing. The cache layout is models--org--name/snapshots/<commit>/, three levels under a registered root, so a probe that stops at two never opens the one directory the weights live in: a denied commit dir made the model vanish from the list while the folder still reported ok, which is the silent empty case this PR exists to remove. The extra level is bought only for a directory named snapshots, since a blanket third level would spend the open budget descending into diffusers component directories. And when the root itself becomes denied, the partial branch restored partial regardless, so a folder none of which can be read kept saying some models in it could not be read, sending the user hunting for one bad model. The cause the probe already returns is the discriminator: it is the root path itself for the root's own refusal and a nested entry path otherwise.

danielhanchen · 2026-08-18T16:57:10Z

@codex review

danielhanchen · 2026-08-18T16:57:21Z

@codex review

danielhanchen · 2026-08-18T17:34:59Z

Merged current main in to clear a stale base. The branch was 98 commits behind, and that is what the red checks were: I reproduced tests/studio/test_deep_research_frontend_contract.py::test_research_presentation_is_integrated failing at the old head and passing after the merge (1 failed -> 12 passed), which is main's #8986, not anything this PR touches. Same tell as #8999 and #9053.

A merge rather than a rebase, so nobody's local copy breaks. No conflicts.

After the merge: the PR own backend suites 87 passed, frontend typecheck clean, npm run build clean, frontend suite 3786 passed with one failure, tests/queued-model-capabilities.test.ts, which is red on main today for an unrelated extensionless import that #9192 fixes. That single failure also explains the red Frontend build + bundle sanity and Frontend unit tests (Windows), neither of which this PR can reach: its own diff is backend-only.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 569921ca74

ℹ️ 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".

chatgpt-codex-connector · 2026-08-19T12:09:34Z

+                    if entry.is_dir():
+                        subdirs.append((entry.name, entry.path))


Verify directory search permission during scans

When a POSIX folder retains read permission but loses execute/search permission after registration (for example, mode 0400) and contains a loose .gguf file, os.scandir can still enumerate the filename and DirEntry.is_dir() classifies it as a file without touching the path, so this probe returns ok. The real custom-folder scanner uses Path.is_dir()/Path.is_file(), which raises EACCES for that file and skips the model; consequently the model disappears while both folder APIs still report the folder as healthy. The probe must also verify search access for every directory it treats as readable.

Useful? React with 👍 / 👎.

Every denial a real session actually produces takes read and search away together: chmod 000, an ownership change, an ACL deny, macOS TCC, Windows Controlled Folder Access. In all of those os.scandir itself raises EACCES and the probe already reports permission_denied. The split you describe needs a deliberate r-without-x mode such as 0400, which nothing in a session creates, so I am leaving the per-entry access check out rather than paying a syscall per file on every scan.

chatgpt-codex-connector · 2026-08-19T12:09:34Z

+# Deep enough for <root>/<publisher>/<model>, the LM Studio and HF cache shape.
+_PROBE_DEPTH = 2


Probe deeper into registered Ollama roots

When a registered scan folder is an Ollama root and access is denied below manifests/<host>—for example at the usual <namespace>/<model> level—this depth opens only the root, manifests, and the host directory. The custom-folder loop explicitly invokes _scan_ollama_dir, whose recursive traversal suppresses an unreadable manifest subtree and returns no model, but note_scan_folder_scanned then receives found=False and this probe reports ok. Add Ollama-specific traversal comparable to the snapshots exception, or propagate the scanner's skipped-entry error, so these supported models do not disappear without a warning.

Useful? React with 👍 / 👎.

This needs access to be denied specifically at manifests//, inside a tree Ollama creates itself at 0755 and that the user never touches. The denials a real session hits are the registered root or the models directory itself, and the current depth already catches both. A healthy Ollama root probes ok correctly, so I am not spending extra opens on a level that only matters for a hand-made permission state.

danielhanchen · 2026-08-19T12:21:07Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 569921ca74

ℹ️ 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".

chatgpt-codex-connector · 2026-08-19T12:24:27Z

+def _record(path: str, status: str, cause: str) -> None:
+    if len(_failed) >= _MAX_TRACKED:
+        _failed.clear()


Evict one entry instead of clearing all failures

When a scan records a 257th distinct failing path, this clears the statuses of all previously failing folders, so both folder APIs immediately report those unreadable locations as ok. There is no corresponding 256-folder database limit, and removed folders are never purged from _failed, so the threshold can also be reached through folder churn; evict a single stale/old entry or reconcile the registry against active folder rows rather than discarding every known failure.

Useful? React with 👍 / 👎.

Every key in _failed is a registered scan folder row path (record_scan_failure and note_scan_folder_scanned are only ever called with the row's path, never a per-model subpath), so reaching 257 distinct failing paths would mean a user registered and broke that many folders inside one backend process. The 256 entry cap is a leak guard for a state no real session reaches, not a path a running install takes.

danielhanchen · 2026-08-19T12:31:34Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 569921ca74

ℹ️ 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".

chatgpt-codex-connector · 2026-08-19T12:34:40Z

+# Each value is (status, the directory that refused), so a recheck can settle a
+# folder in one open instead of walking it again. Bounded by the folder count.
+_MAX_TRACKED = 256
+_failed: dict[str, tuple[str, str]] = {}


Rebuild folder health after backend restarts

When the backend restarts while a registered folder is missing or unreadable, this process-local registry starts empty; refresh_failed_scan_folders() then returns immediately and annotate_scan_folders() reports every row as ok. Consequently, opening the folder dialog directly after a restart hides the problem until a separate model-inventory scan happens to repopulate the registry. Persist the last scan result or probe/rebuild registered-folder health on cold start before returning statuses.

Useful? React with 👍 / 👎.

A status is only ever rendered from a view that has already run an inventory scan: the hub page refreshes the inventory on mount and the folders dialog opens on a later click, so the registry is populated before any row is drawn, and a folder that is still unreadable is re-probed by refresh_failed_scan_folders on that open. STATUS_OK is documented as readable or not scanned yet, and probing every registered folder on cold start is exactly the stalled-network-mount cost this module keeps off the folder-list path.

for more information, see https://pre-commit.ci

shimmyshimmer requested a review from danielhanchen as a code owner August 17, 2026 02:12

[pre-commit.ci] auto fixes from pre-commit.com hooks

a97c72f

for more information, see https://pre-commit.ci

This comment was marked as resolved.

Sign in to view

shimmyshimmer and others added 2 commits August 16, 2026 20:11

[pre-commit.ci] auto fixes from pre-commit.com hooks

279a0d6

for more information, see https://pre-commit.ci

This comment was marked as resolved.

Sign in to view

chatgpt-codex-connector Bot reviewed Aug 17, 2026

View reviewed changes

shimmyshimmer and others added 2 commits August 17, 2026 00:59

[pre-commit.ci] auto fixes from pre-commit.com hooks

876305e

for more information, see https://pre-commit.ci

chatgpt-codex-connector Bot reviewed Aug 17, 2026

View reviewed changes

Probe scan folders off the event loop, and stop a vanished model or a…

cfbd636

… Windows device error condemning the folder

This comment was marked as resolved.

Sign in to view

[pre-commit.ci] auto fixes from pre-commit.com hooks

91cef9e

for more information, see https://pre-commit.ci

chatgpt-codex-connector Bot reviewed Aug 18, 2026

View reviewed changes

unslothai deleted a comment from chatgpt-codex-connector Bot Aug 18, 2026

Merge remote-tracking branch 'origin/main' into r9053

4ed413c

Merge remote-tracking branch 'origin/main' into r9053

569921c

chatgpt-codex-connector Bot reviewed Aug 19, 2026

View reviewed changes

unslothai deleted a comment from chatgpt-codex-connector Bot Aug 19, 2026

unslothai deleted a comment from shimmyshimmer Aug 19, 2026

unslothai deleted a comment from chatgpt-codex-connector Bot Aug 19, 2026

unslothai deleted a comment from shimmyshimmer Aug 19, 2026

unslothai deleted a comment from chatgpt-codex-connector Bot Aug 19, 2026

unslothai deleted a comment from shimmyshimmer Aug 19, 2026

unslothai deleted a comment from chatgpt-codex-connector Bot Aug 19, 2026

unslothai deleted a comment from shimmyshimmer Aug 19, 2026

unslothai deleted a comment from chatgpt-codex-connector Bot Aug 19, 2026

unslothai deleted a comment from shimmyshimmer Aug 19, 2026

unslothai deleted a comment from chatgpt-codex-connector Bot Aug 19, 2026

chatgpt-codex-connector Bot reviewed Aug 19, 2026

View reviewed changes

danielhanchen and others added 2 commits August 19, 2026 12:37

Merge remote-tracking branch 'origin/main' into r9053

2652462

[pre-commit.ci] auto fixes from pre-commit.com hooks

9303927

for more information, see https://pre-commit.ci

danielhanchen merged commit 0eb6b6c into main Aug 19, 2026
41 of 47 checks passed

danielhanchen deleted the studio/scan-folder-permission-status branch August 19, 2026 13:11

interpreter	`os.access(R_OK\|X_OK)`	actual read
system python 3.9	True	`PermissionError [Errno 1] Operation not permitted`
venv python 3.13	False	same EPERM

		if probe_status(entry[1]) == STATUS_OK:
		_failed.pop(path, None)

		# Deep enough for <root>/<publisher>/<model>, the LM Studio and HF cache shape.
		_PROBE_DEPTH = 2

Sunbelt Computer Software

PL/B Language Development and Support

Uh oh!

Uh oh!

Conversation

shimmyshimmer commented Aug 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

The problem

The fix

Cost

Tests

Compatibility

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Aug 17, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Aug 17, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Aug 17, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Aug 18, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danielhanchen commented Aug 18, 2026

Review summary

Before / after

On the 12 Codex items

Three bugs found and fixed

Simulation and compatibility

Reviewed and deliberately not fixed

CI

Uh oh!

danielhanchen commented Aug 18, 2026

UI evidence

Uh oh!

danielhanchen commented Aug 18, 2026

Uh oh!

danielhanchen commented Aug 18, 2026

Uh oh!

danielhanchen commented Aug 18, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Aug 19, 2026

shimmyshimmer commented Aug 17, 2026 •

edited

Loading