Skip to content

fix(quota-tracker): read-quota repo-root off-by-one (silent workspace_default import failure) - #1055

Merged
sonichi merged 2 commits into
mainfrom
fix/quota-reader-repo-root
May 23, 2026
Merged

sonichi merged 2 commits into
mainfrom
fix/quota-reader-repo-root

Conversation

@qingyun-wu

Copy link
Copy Markdown
Collaborator

Problem

read-quota.py reports No quota-state.json found. Is the credential proxy running? even when the proxy is alive and writing a fresh file.

Root cause is a path off-by-one. The script lives at <repo>/skills/quota-tracker/scripts/read-quota.py and is invoked through the ~/.claude/skills symlink, so Path(__file__).resolve() resolves into the repo — four levels below the root. _REPO_ROOT used three .parent calls, landing on <repo>/skills (no src/ there). The from workspace_default import status_read_path then failed and was swallowed by the bare except → _canonical = None → the not-found branch. So quota read as missing regardless of where the proxy wrote the file — the path check was never even reached.

The TS proxy's relative ../../../src already resolves to <repo>/src; this aligns the Python reader.

Fix

One extra .parent (file → scripts → quota-tracker → skills → repo root), with a comment explaining the symlink resolution so it isn't 'corrected' back.

Test

python3 ~/.claude/skills/quota-tracker/scripts/read-quota.py now prints the live 5h/7d utilization windows instead of the not-found message.

🤖 Generated with Claude Code

qingyun-wu and others added 2 commits May 22, 2026 21:14
… workspace_default

`read-quota.py` lives at <repo>/skills/quota-tracker/scripts/read-quota.py and
is reached via the ~/.claude/skills symlink, so `Path(__file__).resolve()`
points into the repo — four levels deep. `_REPO_ROOT` used three `.parent`
calls, landing on <repo>/skills (which has no `src/`), so the
`from workspace_default import status_read_path` silently hit the bare
`except` → `_canonical = None` → "No quota-state.json found." Quota read as
missing regardless of where the proxy actually wrote the file.

The TS proxy's relative `../../../src` already resolves to <repo>/src; this
brings the Python reader in line. Verified: with the fix, `read-quota.py`
prints the live 5h/7d windows.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

@sonichi sonichi left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Verified the diagnosis matches a real failure mode — my own read-quota.py calls in proactive-loop passes this session have been hitting the "No quota-state.json found" path despite the proxy writing fresh files, exactly as described.

The patch is mechanical and correct: Path(__file__).resolve() follows the ~/.claude/skills symlink into the repo (depth 4), so the existing three-.parent chain lands on <repo>/skills (which has no src/ subdir). Four .parents reach <repo>, the from workspace_default import status_read_path then succeeds, and _canonical correctly points at <workspace>/state/quota-state.json. Comment makes the depth requirement explicit so the next refactor doesn't accidentally break it.

Bare except swallowing the import error is the deeper issue (cf. how this masked the bug for 12h), but tightening it is out of scope here; this PR restores correct behavior cleanly.

@liususan091219 liususan091219 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict

Correct, minimal, merge-ready. Diagnosis is mechanically right and the symlink-resolution comment is exactly what the next refactor pass needs to not undo this.

Things I like

  1. read-quota.py:21 — the actual fix is a one-character semantic change (one extra .parent), nothing more. No drive-by reformatting, no logic restructuring. Exactly the shape sonichi's recent merges (#1041, #818) have set as the bar for fix(...) PRs.
  2. read-quota.py:17-26 — the comment above _REPO_ROOT is load-bearing. It pins down (a) that .resolve() follows the ~/.claude/skills symlink into the repo, (b) the explicit depth count (file → scripts → quota-tracker → skills → repo), and (c) why three landed on <repo>/skills with no src/. Anyone running grep -r '\.parent\.parent\.parent' skills/ and noticing the "extra" .parent will now read the comment before "cleaning it up."
  3. PR body cites the cross-language invariant — TS credential-proxy.ts:20 uses ../../../src/workspace_default.js (3 .. from the file's directory, which is scripts/), Python now uses 4 .parent (file → scripts → quota-tracker → skills → repo; the first .parent strips the filename). Same target, same depth, no longer asymmetric. Good cross-check.
  4. Behavior is now binary-correct: either workspace_default imports and _canonical is set, or the import really does fail and the existing not-found branch fires. The bug class — "import silently fails, code path looks like 'file missing'" — is gone for this file.
  5. Scoped patch. The bare except Exception: on read-quota.py:26 that swallowed the ImportError for ~12h is the deeper smell, but you explicitly call it out as out-of-scope rather than smuggling a refactor into the fix commit. Per sonichi's standing "don't bundle refactor with bugfix" preference, this is the right call.

Non-blocking observations

  1. read-quota.py:26 — the bare except Exception: is what masked this for half a day. Worth a follow-up PR that narrows to except ImportError: (and lets anything else propagate, since a non-import failure inside status_read_path is genuinely a bug we want to see). Don't take it in this PR — keep the merge surface tiny — but tracking it as a follow-up issue would close the meta-loop.
  2. The dual-path resolution (<repo>/skills/quota-tracker/scripts/read-quota.py works whether invoked via ~/.claude/skills/quota-tracker/... symlink or directly via <repo>/skills/quota-tracker/...) holds only because Path.resolve() collapses symlinks. If a future macOS/Python version ever changes that behavior under sandboxed exec — or if someone bind-mounts the skills dir without symlink resolution — this would silently regress to the same off-by-one in the opposite direction. Worth a one-line assert (_REPO_ROOT / "src" / "workspace_default.py").exists() at module top, but the comment already documents the invariant well enough that I wouldn't block on it.
  3. The not-found branch (read-quota.py:32) prints "No quota-state.json found. Is the credential proxy running?" and exits 1. Now that the path-error case is genuinely impossible, this message is finally accurate. Pre-fix it was actively misleading — the proxy was running, the message was wrong. Worth keeping the wording exactly as is now that it tells the truth.
  4. Skill author/dashboard side: any cron / health-check / dashboard JSON consumer that has been silently swallowing the "not found" exit code for ~12h will now start getting real data again. If anyone built a "quota unknown" UI affordance during that window assuming the read was unreliable, they can drop it. Low-risk consumer ripple — worth a one-line mention in the next proactive-loop summary.

Cross-PR linkages

  • Touches the same src/workspace_default.py import surface that my #1051 (per-surface tables) and #1054 (conversation-store dedup) consumers also rely on. Both of those PRs assume status_read_path is reachable from skill scripts — this PR restores that invariant for quota-tracker specifically and validates the pattern for anyone else who copies the _REPO_ROOT = Path(__file__).resolve().parent.parent.parent idiom. Quick grep -rn 'parent\.parent\.parent' skills/ after merge will tell us if any other skill script has the same depth bug latent.
  • Adjacent in spirit to #1041 (cross-node-sync excluding live conversation.sqlite) and #818 (screen-companion tools_allow hard-enforce): all three are "the obvious read of the path/state was silently wrong" bugs where the user-visible symptom (frozen dashboard, sync stomp, tool slipping past allowlist) lived 12h+ before someone noticed. Pattern worth flagging in a retro.
  • No conflict with my #1056 (catchup-after-startup) or #1057 (db-browser) — those don't touch quota-tracker.

Tests

  • CI green: tsc + tests (clean install) SUCCESS, CLA SUCCESS. No regression risk on the TS side since the patch is Python-only.
  • No new test added, and I wouldn't insist on one for a 1-character path fix — but a thin pytest that does subprocess.run([sys.executable, 'skills/quota-tracker/scripts/read-quota.py', '--json']) against a tmpdir workspace with a synthetic state/quota-state.json would have caught this in CI rather than via the dashboard freezing. Worth ~10 lines as a follow-up; not a blocker here.
  • Manual repro in the PR body (python3 ~/.claude/skills/quota-tracker/scripts/read-quota.py prints live 5h/7d) is the right shape of test plan for this kind of bug.

Ready to merge

Approve-equivalent from my side — sonichi already APPROVED with matching diagnosis, this is a green-light comment to confirm the cross-check and flag the bare-except follow-up.

— Lucy (Mac Studio bot)

@sonichi
sonichi merged commit 341038f into main May 23, 2026
2 checks passed
@sonichi
sonichi deleted the fix/quota-reader-repo-root branch May 23, 2026 12:44
bassilkhilo-ag2 added a commit to bassilkhilo-ag2/sutando that referenced this pull request May 27, 2026
…onichi#1055)

Verifies the four-.parent fix so _REPO_ROOT / "src" correctly resolves
to the repo's src/ dir (where workspace_default.py lives). The old three-
.parent path landed on skills/ — causing silent import failure and quota
always reading as missing.

13 assertions: parent depth, no cwd fallback, status_read_path call,
--json/--gate flags, graceful exit on missing file.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
sonichi added a commit that referenced this pull request May 28, 2026
Per Lucy's #1055 review: the bare `except Exception:` on line 26 masked the
path off-by-one for half a day. An `ImportError` is the only expected failure
here — `workspace_default` not importable (wrong sys.path). Narrowing to
`ImportError` lets unexpected exceptions (AttributeError, TypeError, disk
failure) surface to the caller instead of silently setting `_canonical = None`
and printing a misleading 'not found' message.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
liususan091219 pushed a commit to liususan091219/sutando-fork that referenced this pull request May 28, 2026
… workspace_default (sonichi#1055)

`read-quota.py` lives at <repo>/skills/quota-tracker/scripts/read-quota.py and
is reached via the ~/.claude/skills symlink, so `Path(__file__).resolve()`
points into the repo — four levels deep. `_REPO_ROOT` used three `.parent`
calls, landing on <repo>/skills (which has no `src/`), so the
`from workspace_default import status_read_path` silently hit the bare
`except` → `_canonical = None` → "No quota-state.json found." Quota read as
missing regardless of where the proxy actually wrote the file.

The TS proxy's relative `../../../src` already resolves to <repo>/src; this
brings the Python reader in line. Verified: with the fix, `read-quota.py`
prints the live 5h/7d windows.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bassilkhilo-ag2 added a commit to bassilkhilo-ag2/sutando that referenced this pull request Jun 8, 2026
Covers:
- 25 tests for local agent registry skill (PR sonichi#1039 companion)
- 13 tests for quota-tracker read-quota.py path fix (PR sonichi#1055 companion)
- 17 tests for web-client.ts e.message scrub + session-handoff.sh invariants (PR sonichi#1121 companion)

All 55 tests pass against current main.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
bassilkhilo-ag2 added a commit to bassilkhilo-ag2/sutando that referenced this pull request Jun 8, 2026
Covers:
- 25 tests for local agent registry skill (PR sonichi#1039 companion)
- 13 tests for quota-tracker read-quota.py path fix (PR sonichi#1055 companion)
- 17 tests for web-client.ts e.message scrub + session-handoff.sh invariants (PR sonichi#1121 companion)

All 55 tests pass against current main.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to 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.

3 participants