Skip to content

The admin page works again, and the store poller runs where it can matter - #30

Merged
mspinola merged 2 commits into
mainfrom
claude/fix-admin-email-path
Aug 9, 2026
Merged

The admin page works again, and the store poller runs where it can matter#30
mspinola merged 2 commits into
mainfrom
claude/fix-admin-email-path

Conversation

@mspinola

@mspinola mspinola commented Aug 9, 2026

Copy link
Copy Markdown
Owner

Three fixes, all found by chasing two reported failures on the Admin page. Two commits: the page, then the poller.

1. Send weekly email: pointed at a file in another repo

The button ran bash scripts/generate-weekly-report-email.sh as a path relative to this repo's working directory:

Email script failed: bash: scripts/generate-weekly-report-email.sh: No such file or directory

That script is not here and never has been (git log --all finds no trace of it). It lives in cotmetrics, which it must, since it imports cotmetrics.reports.get_matrix_data and generate_matrix_html. It moved there with the rest of the metrics layer when that repo was split out, and this call site did not follow, so the button has been broken since the split. Not caused by anything merged recently: I checked #22 specifically, and none of its 10 files was this.

There were four breaks stacked, and the missing file was only the first:

  1. Wrong repo, relative path. The error above.
  2. The .sh is one line, .venv/bin/python scripts/generate-weekly-report-email.py "$@", with relative paths of its own. It needs cwd at the cotmetrics root and a cotmetrics-local .venv, which does not exist on this machine.
  3. The .py needs EMAIL_USER / RECEIVER_EMAIL_USER / EMAIL_PASSWORD from the environment.
  4. The comment here said the .sh "contains the email credentials". It contains none.

Now resolved through the installed cotmetrics package, so it tracks the editable install rather than the caller's cwd, and run directly with sys.executable (the right interpreter, since cotmetrics is installed in it). The .sh hop is dropped: it carries nothing, and skipping it removes both the cwd assumption and the missing-venv dependency. Plus a 120s timeout so a hung SMTP connect cannot wedge a Dash worker thread.

The button also reports the script's own message now instead of "Check logs". Its first act is to name whichever credential is missing, which is the likeliest failure and the one an operator can fix on the spot:

Error sending email at 12:02:38: Error: Missing required email environment
variables (EMAIL_USER, RECEIVER_EMAIL_USER, EMAIL_PASSWORD).

Verified with credentials deliberately unset, so the run stopped at the script's own guard and never opened an SMTP connection. No mail was sent at any point.

2. Login: same shape, worse message

COT_ADMIN_PASSWORD has lived only in ~/.zshrc and ~/.bash_profile. The app inherits it when launched from an interactive shell and silently does not otherwise, which is how it is running now, so validate_login short-circuits before ever comparing the typed password. This is the hazard CLAUDE.md already records for MARKETDATA_STORE ("the only place it is set, so a launchd job would not see it"), except this one gates a page.

The page said only "Admin login is not configured", which does not distinguish a missing setting from a wrong password and does not say where the setting goes. It now names the variable and points at .env, which run-local.sh sources with set -a and the deployed unit loads via EnvironmentFile, making it the one home that works in every launch context. server-side/README.md already documents it there.

Not fixed by this PR: the value still has to be added to .env by hand. That is a credential and deliberately not mine to move.

3. The store poller ran in the wrong process under --debug

Introduced by me in #23 earlier today. The poller sat inside if not is_reloader:, beside the boot options fetch and the options scheduler. That gate asks "am I the process that owns startup work?", correct for those two since they write to disk. The poller asks a different question: it mutates the CotIndexer singleton that answers HTTP, so it must run wherever that singleton lives.

With the Werkzeug reloader the answers invert. The parent has no WERKZEUG_RUN_MAIN and only supervises; the child carries the flag and serves. So not is_reloader put the poller in the parent, refreshing an index nobody reads, and left the serving child without one. The same wrong-address-space mistake that made it a thread rather than a Process in the first place.

Now gated on is_reloader or not dash_debug. Non-debug runs were always correct and are unchanged.

Verified against real instances by comparing the pid holding the listening socket with the pid in the log, rather than by re-reading the code:

--debug      parent 59728 (no WERKZEUG_RUN_MAIN), child 59738 (flag set)
             -> Store poller started in pid 59738, the one accepting connections

no --debug   single process 59961 holds the socket
             -> Store poller started in pid 59961

The pid is now in that log line, because which process this lands in is the whole correctness question and was invisible before.

Verification

131 tests pass, ruff clean, check_dep_floors.py OK. Rebased onto d7d4c3e, which added the crucible-marketdata dependency; no overlap with these files and no conflicts.

Known, not addressed here

get_indexer() is unlocked (if _indexer is None: _indexer = CotIndexer()). Two threads can both see None and both build, costing ~90s and a transient second copy. The race predates this work since Dash already serves on multiple threads, but the poller is a new caller, so exposure rises slightly. The fix is a lock in cotmetrics' indexer.py, which is a cross-repo change and a version bump.

🤖 Generated with Claude Code

mspinola and others added 2 commits August 8, 2026 23:53
…ured

Two separate failures on the same page, both of them a setting that lives
somewhere the app cannot see, and both reported as a dead end.

Send weekly email. The button ran `bash scripts/generate-weekly-report-email.sh`
as a path relative to this repo's working directory. That script is not here and
never has been: it lives in cotmetrics, which it must, since it imports
cotmetrics.reports.get_matrix_data and generate_matrix_html. It went there with
the rest of the metrics layer when that repo was split out, and this call site did
not follow, so the button has been dying on "No such file or directory" for a path
nobody would think to look for in a sibling checkout.

Locate it through the installed cotmetrics package instead, so it tracks the
editable install rather than the caller's cwd, and run the .py directly with
sys.executable. The .sh hop is dropped on purpose. The comment here claimed it
"contains the email credentials"; it contains none. It is one line,
`.venv/bin/python scripts/generate-weekly-report-email.py "$@"`, and its only
effects are to require a cotmetrics-local venv that does not exist on this machine
and to reintroduce the same cwd assumption. Credentials come from the environment,
which a subprocess inherits, and our interpreter is the right one because
cotmetrics is installed in it.

Report the script's own message rather than "Check logs". Its first act is to
check EMAIL_USER / RECEIVER_EMAIL_USER / EMAIL_PASSWORD and exit naming whichever
is missing, which is the likeliest failure and the one an operator can fix on the
spot. Losing that to a log file wastes the diagnosis. Also a 120s timeout, so a
hung SMTP connect cannot wedge a Dash worker thread.

Login. Same shape. COT_ADMIN_PASSWORD has lived only in ~/.zshrc and
~/.bash_profile, so the app inherits it when launched from an interactive shell
and silently does not otherwise, which is how it is running now. The page said
only "Admin login is not configured", which does not distinguish a missing setting
from a wrong password and does not say where the setting goes. Name the variable
and point at .env: run-local.sh sources it with `set -a` and the deployed unit
loads the same file via EnvironmentFile, so it is the one home that works in every
launch context. server-side/README.md already documents it there.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Under --debug it was starting in the wrong one, and doing nothing.

The poller sat inside `if not is_reloader:`, beside the boot options fetch and the
options scheduler. That gate asks "am I the process that owns startup work?",
which is the right question for those two: they write to disk, so duplicating
them is waste. The poller asks a different question. It mutates the CotIndexer
singleton that answers HTTP, so it has to run wherever that singleton lives.

With the Werkzeug reloader those questions give opposite answers. The parent has
no WERKZEUG_RUN_MAIN and only supervises; the CHILD carries the flag and does the
serving. So `not is_reloader` put the poller in the parent, refreshing an index
nobody reads, and left the serving child without one. Same wrong-address-space
mistake as running it in the options subprocess, which is the reason it is a
thread and not a Process in the first place.

Gate it on `is_reloader or not dash_debug` instead: true for the single process
when there is no reloader, true for the child when there is, false for the
supervising parent. Non-debug runs were always correct and are unchanged.

Verified against real instances by comparing the pid holding the listening socket
with the pid in the log, rather than by reading the code twice:

  --debug     parent 59728 (no WERKZEUG_RUN_MAIN), child 59738 (flag set)
              -> Store poller started in pid 59738, which is the one accepting
  no --debug  single process 59961 holds the socket
              -> Store poller started in pid 59961

The pid is now in that log line, because which process this lands in is the whole
correctness question and was invisible before.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@mspinola
mspinola merged commit bf87c8a into main Aug 9, 2026
3 checks passed
@mspinola
mspinola deleted the claude/fix-admin-email-path branch August 9, 2026 03:55
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.

1 participant