Watcher: Per-environment .env files, file creation time, API key sanitization, and Windows service auto-restart - #42
Merged
Conversation
Persist the file's on-disk creation time alongside each row in the `files` table and surface it in the run files table so users see when the instrument actually wrote the file, not when the watcher first reported it to the API. - Add nullable `files.file_created_at` column (drizzle migration 0009). - Watcher: capture `st_birthtime` (with `st_mtime` fallback) at stability time, persist it in the local SQLite manifest, and send it as ISO 8601 UTC on POST /runs, PATCH /runs/:runId, and the request-upload-url path. The latter also backfills the column when an earlier detected_files report predated this change. - API: accept `file_created_at` on all three watcher entry points and include it in every JSON file response. - UI: render `fileCreatedAt` in the "Created" column of the run files table, falling back to the row's `createdAt` for legacy rows and Lambda-created files. - Tests: round-trip coverage for the SQLite manifest, the wire payload, and a unit test for the platform-portable `file_created_at` helper.
Pasting an API key into the hidden `click.prompt` on Windows can silently introduce trailing CR/LF, non-breaking spaces, or zero-width characters from rich-text clipboards (Outlook, Teams, Word). The bad bytes flow through to the Authorization header, the server hashes the wrong value, and every request 401s — including the heartbeats that run after init persists the corrupted key to `.env.<environment>`. Add a `_clean_api_key` helper that strips invisible characters and whitespace, then validates the `dhub_` prefix and rejects any internal whitespace. Apply it to every code path in `init` that produces a key, including the value read from the environment, so a previously saved bad key is caught and re-prompted instead of silently reused. Surface a clear ClickException at init time rather than a confusing 401 from `list_instruments`. Also add an `--show-key` flag so operators on Windows terminals where hidden-input paste is unreliable can fall back to visible entry.
Merge the per-watcher status badge (status-badge.ts) into
WatcherStatusBadge so there's one component handling both the
instrument-level aggregate (online/offline/no_watcher) and the per-watcher
states (watching/stale/stopped/registered).
Aligns the per-watcher labels with the aggregate vocabulary:
watching -> "Online" (green, matches aggregate online)
stale -> "Unresponsive" (destructive, matches aggregate offline)
stopped -> "Stopped" (muted filled — intentional, not an alarm)
registered -> "Registered" (muted outlined — transient pre-heartbeat)
Tooltip with "Last online {time}" now also fires for `stale` watchers,
and watcher-header.tsx wires lastHeartbeatAt through so hovering the
header badge surfaces it too.
The service was registered with SERVICE_AUTO_START, which triggers before the network stack is up on a freshly-booted lab PC. The startup sequence in SvcDoRun would then fail its API health check and `return` cleanly, which the SCM treats as a graceful stop -- so the configured recovery actions never fired and the watcher stayed down until a human intervened. Three changes to watcher/src/data_hub_watcher/service.py: 1. install_service now passes delayedstart=True and serviceDeps= ["Tcpip", "Dnscache"] so the SCM waits for the network stack and defers the start until ~2 minutes after boot. 2. Each early-exit path in SvcDoRun (registry-read failure, transient ApiError at startup, missing watcher_id, instrument still pending) now `raise SystemExit(1)` instead of returning, so the process exits non-zero. 3. _configure_recovery additionally sets SERVICE_CONFIG_FAILURE_ACTIONS_FLAG with fFailureActionsOnNonCrashFailures=True, so the existing 60s/120s restart actions also fire on non-zero exits, not only on actual crashes. Operators must reinstall the service (`data-hub-watcher service uninstall && data-hub-watcher service install`) to pick up the new registration flags.
Extract the SvcDoRun body into a top-level `_run_service_loop` so the service startup sequence (registry read, env loading, API health check, checksum sync, runtime build/start/stop) can be exercised on any platform by injecting a mock servicemanager. Add a test suite that mocks the win32 layer via sys.modules and locks in the contract that has historically regressed on lab PCs: install kwargs (delayed-start + Tcpip/Dnscache deps), the two-restart recovery policy plus the non-crash-failure flag, and the four SystemExit branches in service startup. Also include watcher/** in the python-test.yml paths filter so watcher-only PRs actually trigger the Python test job.
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A grab-bag of small, independent watcher fixes and quality-of-life improvements observed across the staging/production fleet, plus the matching web-app changes to surface and persist the new file metadata. Each commit on the branch is self-contained; there is no single overarching feature.
Changes
Per-environment
.envfiles (60d02d3) — API keys are now saved to~/.data-hub/.env.<environment>(e.g..env.staging,.env.production,.env.preview) so operators can switch environments by re-runninginitwithout re-entering credentials. The legacy~/.data-hub/.envis still loaded first as a fallback.watcher/src/data_hub_watcher/constants.py: newenv_file_path(),SUPPORTED_ENVIRONMENTS, andload_env(environment)overlay semantics;save_api_keynow takes anenvironment.watcher/src/data_hub_watcher/cli.py:initoverlays the env-specific file, prompts to reuse a saved key when one exists, andservice install --env-pathdefaults to the per-environment file.docs/watcher.mdanddocs/guides/installing-a-watcher.md.Capture and display on-disk file creation time (
91b9e69) — the watcher now reportsst_birthtime(falling back tost_mtime) for every detected file and the run files table prefers it over the row'screated_at.watcher/src/data_hub_watcher/run_detector.py: newfile_created_at()helper,FileInfo.file_created_at, payload fieldfile_created_at.watcher/src/data_hub_watcher/api_client.py,uploader.py: sendfile_created_atonrequest_upload_url.watcher/src/data_hub_watcher/state.py: nullablefile_created_atcolumn ondetected_fileswith a migration for legacy DBs.web-app/lib/db/schema.ts+web-app/drizzle/0009_clammy_tyger_tiger.sql: new nullablefiles.file_created_atcolumn.web-app/app/api/v1/...: acceptfile_created_aton report-run, PATCH run, and request-upload-url; emit it on every file response; backfill the column when a queue-mode upload follows an olderdetected_filesreport.web-app/components/runs/run-files-table.tsx: preferfileCreatedAt, fall back tocreatedAt.Sanitize and validate API keys during
init(4007831) — strips zero-width / non-breaking-space characters that Outlook/Teams/Word frequently inject into copied keys, validates thedhub_prefix, and adds a--show-keyflag for Windows terminals where hidden paste is unreliable. Operators get a clear error instead of a confusing 401.Unify the watcher status badge (
af8f0cb) — deletesweb-app/components/watchers/status-badge.tsand folds per-watcher statuses (watching,stale,stopped,registered) into the existingWatcherStatusBadge, with shared color treatments, distinct labels (UnresponsivevsOffline), and a tooltip that shows last-online time only for unexpected silences.WatcherHeaderandWatchersTablenow use the unified component.Auto-restart the Windows service after a lab-PC reboot (
1712c70) — addresses the recurring failure mode where a freshly-rebooted PC starts the watcher before DHCP/DNS is up, the API health check fails, and the service stays stopped.delayedstart=Trueand a dependency onTcpip+Dnscache.SERVICE_CONFIG_FAILURE_ACTIONS_FLAG(fFailureActionsOnNonCrashFailures) so the existing recovery actions also fire on non-zeroSystemExit— previously the SCM only restarted on hard crashes.SvcDoRuninto a top-level, platform-agnostic_run_service_loop(stop_event, sm)so the full startup sequence (registry read, env loading, instrument check, checksum sync, runtime build/start/stop) is unit-testable on any platform via a mockedservicemanager.Unit tests for the Windows service module (
b997b65) — newwatcher/tests/test_service.pycovering_run_service_loopstartup paths and recovery exit semantics.Pytest upgrade (
f057746) — bumpspytestfrom>=8.3.5to>=9.0.3(anduv.lockto9.0.3) to address a dependabot security alert.CI —
.github/workflows/python-test.ymlnow triggers on changes underwatcher/**so the new test suite runs on every PR.Breaking changes
None for end users. A few internal contracts changed but are handled with migrations / fallbacks:
StateDB.record_detected_filesnow expects 5-tuples (addsfile_created_at); thedetected_filestable is migrated in-place viaALTER TABLE … ADD COLUMNfor existing watchers.save_api_key(api_key, environment)andload_env(environment)gained anenvironmentargument; the legacy~/.data-hub/.envis still loaded as a base layer for backwards compatibility.web-app/components/watchers/status-badge.tswas deleted. Anything importingstatusBadgefrom it must switch toWatcherStatusBadge.Driveby changes
watcher/src/data_hub_watcher/cli.py: theservice installwarning now correctly points todata-hub-watcher initinstead of the (non-existent)loginsubcommand.watcher/src/data_hub_watcher/service.py:TYPE_CHECKINGimport block removed since it was empty after the refactor;threadingimport hoisted to module scope.assert cfg.api_base_url is not Noneon the preview branch makes the existingWatcherConfigvalidator invariant visible to pyright.Testing
make check-allpasses (format, lint, type-check).uv run pytest watcher/passes, including the newwatcher/tests/test_service.py.pnpm drizzle-kit migrateapplies0009_clammy_tyger_tiger.sqlcleanly on a staging DB.data-hub-watcher initagainst staging, confirm the key is saved to~/.data-hub/.env.stagingand that re-running offers to reuse it.data-hub-watcher service install, reboot, confirm the service comes up after the network stack is ready (check Event Viewer forLogInfoMsglines).created_at) and thatfiles.file_created_atis populated in the DB.WatcherStatusBadgerenders correctly in: instruments table, instrument header, watchers table, and watcher detail header — including the tooltip foroffline/stale.