Skip to content

fix(starlark): the store authenticated with a key nothing writes - #541

Merged
ChuckBuilds merged 1 commit into
mainfrom
claude/starklark-store-loading-5308da
Sep 9, 2026
Merged

fix(starlark): the store authenticated with a key nothing writes#541
ChuckBuilds merged 1 commit into
mainfrom
claude/starklark-store-loading-5308da

Conversation

@ChuckBuilds

Copy link
Copy Markdown
Owner

#535 restored the thirteen Starlark routes, so the store stopped answering 404 — and it still would not load. This is why, confirmed against a running device before anything was changed.

What the device said

/api/v3/starlark/repository/browse answers 200 with 1000 apps in 27 seconds. The routes are fine. Two things underneath them are not:

/api/v3/plugins/store/github-status -> authenticated: true, rate_limit: 5000
/api/v3/starlark/repository/browse -> rate_limit: {limit: 60, remaining: 18, used: 42}

The plugin store is authenticated. The Starlark store, on the same device at the same moment, is not — and it is spending an unauthenticated 60/hour budget shared with 48 installed plugins' update checks. When that budget runs out the store goes blank, reports success, and says nothing.

The store never used the token the user configured

The three repository routes read github_token off config.json. Nothing writes that key — it is not in config.template.json, no setting offers it, and it appears nowhere else in the codebase. The token the user actually configures goes to config_secrets.json as github.api_token, which PluginStoreManager loads and every other GitHub caller uses.

So the store could never be authenticated, no matter what the user set. 60 requests/hour, against a budget 48 update checks are also drawing on, when 5000 was already configured and sitting unused.

_starlark_github_token() now prefers the store manager's token and keeps the config.json key as a fallback for a hand-edited config.

Every failure looked identical, including to whoever was debugging it

A failed fetch was reported as an empty repository.list_all_apps_cached turned any listing failure — rate limit, DNS, timeout, non-200 — into {'apps': [], 'count': 0}, and the route sent that out as status: 'success'. A rate limit, a network failure and a genuinely empty repository all drew the same blank grid with no error anywhere. It now returns the reason, the route answers 502 with it, and a failure is no longer cached as an empty repository for two hours.

The guard against a bad response was itself a crash._make_request catches (json.JSONDecodeError, ValueError), but json was never imported in that module — evaluating the tuple raises NameError, so the guard written for exactly this case never ran. Reachable whenever something on the path answers with HTML: a captive portal, a proxy error page, a DNS-hijacking router. Relevant here given the DNS problems #538 documented.

Seventeen handlers answered 5xx with no detail.browse_tronbyte_repository said only "Failed to browse repository". test_no_api_v3_handler_discards_its_exception is supposed to prevent this across api_v3, but it matched one exact message string — handlers that wrote their own wording were never checked, and all thirteen Starlark routes wrote their own. The test now keys on the shape that matters: if it returns 5xx, it says why. The 15 pre-existing non-Starlark functions are listed explicitly as a set that may shrink and never grow, rather than fixed here.

The listing was capped at 1000 and did not say so

The contents API caps a directory at 1000 entries silently. tronbyt/apps has 1075 app directories (counted over git protocol) — the store was showing a truncated repository and looking complete doing it. Now listed via the git trees API, which reports truncated, with the contents API kept as a fallback so a trees-only outage cannot take the store down.

Not addressed

The 27-second cold load. Confirmed on the device, and probably the largest part of what "doesn't load" feels like: 1075 manifests fetched five at a time, with nothing on screen but skeleton placeholders. The fix is a wider thread pool and a message saying what it is doing — out of scope here, and worth its own change.

One judgement call worth flagging: the upload_starlark_app OSError arm deliberately withheld its detail because it names absolute paths. I made it report like the others — a full disk and a bad permission are indistinguishable without it, describe_exception redacts and truncates, and the project's own test already asserts that shape on an OSError. Easy to revert if you disagree.

Verification

  • On the device before any change: browse 200/1000 apps/27s, limit: 60 while the plugin store had 5000.
  • tronbyt/apps app count via git ls-tree: 1075.
  • The missing json import reproduced directly: RAISED: NameError name 'json' is not defined.
  • 25 new tests, and the widened guard. Whole suite: no new failures against main.

🤖 Generated with Claude Code

#535 restored the thirteen routes, so the store stopped answering 404 --
and still would not load. Confirmed against a running device before
anything was changed: /repository/browse answers 200 with 1000 apps in
27s, so the routes are fine. Two things underneath them are not.
**The store never used the token the user configured.** The three
repository routes read `github_token` off config.json. Nothing writes
that key -- it is not in config.template.json, no setting offers it, and
it appears nowhere else in the codebase. The configured token goes to
config_secrets.json as `github.api_token`, which PluginStoreManager
loads and every other GitHub caller uses. So the store could never be
authenticated: 60 requests/hour, on the same per-IP budget 48 installed
plugins spend on update checks, while the 5000 the user had already
configured sat unused. On the device, /plugins/store/github-status
reported authenticated with a limit of 5000 at the same moment
/starlark/repository/browse reported 60, with 18 left. The store going
blank was that 60 running out.
**Every failure looked identical.** list_all_apps_cached turned any
listing failure -- rate limit, DNS, timeout, non-200 -- into an empty
app list, and the route sent that out as `status: success`, so a rate
limit and an empty repository drew the same blank grid with no error
anywhere. It now returns the reason, the route answers 502 with it, and
a failure is no longer cached as an empty repository for two hours.
The guard for a bad response was itself a crash: _make_request catches
`(json.JSONDecodeError, ValueError)` but `json` was never imported, so
evaluating the tuple raises NameError and the guard written for exactly
this case never ran. Reachable whenever something on the path answers
with HTML -- a captive portal, a proxy page, a DNS-hijacking router.
Seventeen handlers answered 5xx with no detail at all.
test_no_api_v3_handler_discards_its_exception is meant to prevent that
across api_v3, but it matched one exact message string, and all thirteen
Starlark routes wrote their own wording. The guard now keys on the shape
that matters: if it returns 5xx, it says why. The 15 pre-existing
non-Starlark functions are listed as a set that may shrink, never grow.
**The listing was capped at 1000 and did not say so.** The contents API
truncates a directory silently; tronbyt/apps has 1075 app directories,
so the store showed a truncated repository and looked complete doing it.
Now listed via the git trees API, which reports `truncated`, with the
contents API kept as a fallback.
Not addressed: the 27-second cold load -- 1075 manifests fetched five at
a time behind skeleton placeholders -- which is probably the largest part
of what "does not load" feels like, and wants its own change.
25 new tests.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@coderabbitai

coderabbitaiBot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

Next included review available in 59 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 83f0acd7-bae4-49dd-a67e-185187d0b91e

📥 Commits

Reviewing files that changed from the base of the PR and between 968b953 and 35d5e26.

📒 Files selected for processing (4)
  • plugin-repos/starlark-apps/tronbyte_repository.py
  • test/test_web_error_detail.py
  • test/web_interface/test_starlark_pixlet_routes.py
  • web_interface/blueprints/api_v3.py

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@codacy-production

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues0 issues

Results:
0 new issues

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

Comment threadweb_interface/blueprints/api_v3.py Dismissed
Comment threadweb_interface/blueprints/api_v3.py Dismissed
Comment threadweb_interface/blueprints/api_v3.py Dismissed
Comment threadweb_interface/blueprints/api_v3.py Dismissed
Comment threadweb_interface/blueprints/api_v3.py Dismissed
Comment threadweb_interface/blueprints/api_v3.py Dismissed
Comment threadweb_interface/blueprints/api_v3.py Dismissed
Comment threadweb_interface/blueprints/api_v3.py Dismissed
Comment threadweb_interface/blueprints/api_v3.py Dismissed
Comment threadweb_interface/blueprints/api_v3.py Dismissed
@ChuckBuilds

Copy link
Copy Markdown
OwnerAuthor

@coderabbitai review

@coderabbitai

coderabbitaiBot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor
⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@ChuckBuilds
ChuckBuilds merged commit 26769ee into mainSep 9, 2026
26 of 27 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@ChuckBuilds@github-advanced-security