Skip to content

feat(cli): tripl doctor and tripl status, the read-only diagnostics (tripl-ey6j.2) - #77

Merged
vladenisov merged 8 commits into
mainfrom
feat/cli-doctor
Aug 1, 2026
Merged

feat(cli): tripl doctor and tripl status, the read-only diagnostics (tripl-ey6j.2)#77
vladenisov merged 8 commits into
mainfrom
feat/cli-doctor

Conversation

@vladenisov

Copy link
Copy Markdown
Owner

Stacked on #76 — base is feat/cli-foundation, so the diff here is only
tripl-ey6j.2. GitHub retargets this to main automatically once #76 merges.

What this is

The read-only diagnostics the 2026-07-28..31 incident needed. That week cost four
days to a scan config failing behind the generic "Scan failed due to an internal
error"
, last_seen_at frozen with no indication why, an accepted schema drift
that had silently deleted a FieldDefinition, and a retry backoff that read to
the operator as a hang. Each is now one invocation away.

doctor runs six checks in a fixed order, exactly once each — connectivity, API
key, projects, data sources, scheduled collection, schema drift. Results are
findings with stable codes and evidence, never prose; --json is assertable,
and exit 3 means "the tool worked, the checks did not", distinct from the tool
breaking (1) and from being misconfigured (2). A skip never counts as a pass, so
a run where nothing could be checked still exits non-zero.

status is the quick view. Its counts come from ProjectResponse.summary rather
than being recomputed — monitoring_signal_count is already gated server-side by
SIGNIFICANT_MIN_REL_EFFECT, so reading it makes the CLI agree with the sidebar
badge by construction instead of carrying a third copy of a constant that has
already drifted twice (tripl-yfsj.1, tripl-jfm3.89).

Decisions worth reviewing

Decision Why
/health gets its own short-lived, unauthenticated client It is at the root, not under /api/v1, and include_in_schema=False — absent from the spec by design, so the contract test cannot see it and a dedicated test pins it instead
Job history read at the API maximum (200), not schedule.py's 40 The backend reads 40 because it only needs the backoff delay, which saturates at six failures. doctor answers how long has this been broken — at 40 rows a 15m config failing four days reports 40 failures since this morning instead of 384 since Monday
A streak filling the window is a lower bound Reported as "at least N, at least since T", with consecutive_failures_truncated in the evidence
A full window with no dispatcher job warns, not fails run_scan, replay_metrics and apply_event_groups all write ScanJobs with no mode stamp, so a busy config buries its own scheduled history. Claiming "the beat scheduler may not be running" there sends the operator after a healthy scheduler
Demo projects get the scheduler's own 6h cooldown as their allowance The demo runs on a 1h interval, so the flat three-interval rule called a healthy demo broken for half of every cooldown — on the one project every new operator opens first
The drift budget is spent round-robin across projects Spent in project order, one large project consumed it entirely and the starved project appeared nowhere in the output — including, potentially, the project holding the very drift this check exists to surface
An unknown interval no longer hides a failing config Staleness and the backoff estimate need the denominator; is it failing does not

Fixes outside the CLI

  • Shared client crash (cli/src/tripl_cli/client.py, imported by tripl-mcp): a
    2xx with a non-JSON body — an SSO gateway or captive portal answering with a login
    page — raised json.JSONDecodeError, which subclasses ValueError, not
    httpx.HTTPError, and so escaped every consumer's error handling. Now a
    TriplAPIError; doctor keeps its promise to always emit a document.
  • mcp-server/pyproject.toml: its published Documentation URL carried a
    /docs/ segment the docs site does not serve (baseUrl: '/tripl/' +
    routeBasePath: '/'), so it 404s in the already-published 0.1.0 metadata.
  • "Account → API keys" corrected to "Settings → API keys" in three places
    and in the CLI's own error message. The route is /settings/api-keys, and
    nav.ts puts API keys in the Workspace group; Account holds only Profile and
    Security.

Tests

cli/tests/test_contract.py is new and mirrors the one mcp-server already had:
a backend route rename used to break the sibling package's CI loudly and this one
silently. It also scans collect.py for path literals, so the declaration cannot
go stale behind a newly added read.

Coverage was driven by review rather than by feel — eight documented finding codes
had no test at all, including the two an operator hits by typo, and each new test
was mutation-checked (assertion deliberately broken, run, confirmed red, restored).

Docs

website/docs/run/cli.md — required in the same change by AGENTS.md:467, and the
URL cli/pyproject.toml already bakes into the published wheel metadata, which
404ed until now.

Gates

cli          ruff · ruff format · mypy (18 modules) · 145 tests
mcp-server   ruff · ruff format · mypy (13 modules) ·  36 tests

Review notes

This came out of a workflow whose verify phase raised 21 findings across three
lenses. The substantive ones are fixed above; two remain open and filed rather
than silently dropped:

  1. There is no guard tying the CLI's mirrored scheduler constants
    (FAILURE_BACKOFF_*, DISPATCHER_MODE, the demo cooldown) to the backend's.
    They match today — I verified each — but the pinning test compares the CLI to
    literals inside its own package, so a backend change leaves every CLI test
    green. The right home is a drift-guard in the backend suite, whose CI job
    has the whole repo checked out.
  2. --max-event-types truncation is still reported as one instance-wide count
    rather than per project. Round-robin makes that count honest, but naming the
    affected project would be better.

Copilot AI review requested due to automatic review settings July 31, 2026 19:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Not ready to approve

The new CLI docs/examples contain a few contract-breaking inaccuracies (e.g., job history window size and ASCII-only output examples) that should be corrected to match the implemented behavior.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

Adds the read-only operator diagnostics layer to the new cli/ package, providing tripl doctor (verdict + stable machine contract) and tripl status (quick per-project snapshot) to make common incident/root-cause signals visible via HTTP without backend imports. It also tightens the shared HTTP client error behavior and updates docs/metadata so the CLI and MCP server share consistent configuration guidance.

Changes:

  • Implement tripl doctor and tripl status commands with a split collection (async, never-raise) vs checks (pure rules over snapshots) design, plus JSON document contracts.
  • Add comprehensive CLI test coverage (rule arithmetic, end-to-end mocked instance, concurrency/runner seam, and OpenAPI contract checks).
  • Publish operator documentation and cross-link it from MCP/docs; fix published Documentation URLs.
File summaries
File Description
website/docs/use-cases/llm-agent.md Updates API key navigation text to “Settings → API keys”.
website/docs/run/security.md Adjusts sidebar ordering to accommodate the new CLI doc page.
website/docs/run/cli.md New operator CLI reference (install/config/doctor/status/contracts).
website/docs/run/ai-and-search.md Adjusts sidebar ordering to accommodate the new CLI doc page.
website/docs/integrate/mcp-server.md Notes CLI/MCP share the same env vars and HTTP client.
mcp-server/README.md Updates API key navigation text to “Settings → API keys”.
mcp-server/pyproject.toml Fixes published documentation URL (removes nonexistent /docs/).
cli/tests/test_status.py Adds behavioral tests for tripl status (server-derived counts, envelope, failure behavior).
cli/tests/test_runner.py Tests sync/async seam guarantees (single pool, no sockets without creds, bounded gather).
cli/tests/test_doctor.py End-to-end tests for doctor’s exit contract, ASCII output, and key diagnostic cases.
cli/tests/test_contract.py Contract tests ensuring every read endpoint exists in committed OpenAPI + health is absent.
cli/tests/test_checks.py Pure rule tests for checks (scan streaks/backoff, drifts, data sources, exit code behavior).
cli/tests/conftest.py Adds a FakeInstance HTTP fixture and helpers to build coherent API payloads.
cli/src/tripl_cli/runner.py Adds the single asyncio.run bridge and bounded concurrency gather helper.
cli/src/tripl_cli/errors.py Introduces EXIT_CHECKS_FAILED=3 with clarified semantics.
cli/src/tripl_cli/diagnostics/scan_checks.py Implements scan scheduled-collection diagnostics (streaks, backoff, stale watermark, demo cooldown).
cli/src/tripl_cli/diagnostics/report.py Defines the machine-readable JSON document contract for doctor/status.
cli/src/tripl_cli/diagnostics/render.py Implements ASCII-only human rendering for doctor/status.
cli/src/tripl_cli/diagnostics/model.py Adds the core vocabulary/types (Fetched/Findings/Checks/Snapshots/status/exit code).
cli/src/tripl_cli/diagnostics/endpoints.py Centralizes the set of REST endpoints used for contract enforcement.
cli/src/tripl_cli/diagnostics/collect.py Async HTTP collection layer that wraps all reads into Fetched and probes /health unauthenticated.
cli/src/tripl_cli/diagnostics/checks.py Pure check rules over snapshots + short-circuiting skip behavior.
cli/src/tripl_cli/diagnostics/init.py Documents the diagnostics package split and invariants.
cli/src/tripl_cli/config.py Updates API key guidance text to “Settings -> API keys”.
cli/src/tripl_cli/commands/status.py Implements tripl status CLI surface and JSON/human output routing.
cli/src/tripl_cli/commands/doctor.py Implements tripl doctor CLI surface, check execution, exit codes, JSON/human output routing.
cli/src/tripl_cli/commands/init.py Registers doctor/status and adds shared argparse range validators.
cli/src/tripl_cli/client.py Converts 2xx non-JSON responses into TriplAPIError (prevents JSONDecodeError escapes).
cli/README.md Expands CLI README with doctor/status behavior, exit codes, and links to full docs.
cli/pyproject.toml Pins Python>=3.12, adds warning-as-error for un-awaited coroutines, fixes Documentation URL.
AGENTS.md Updates repo navigation map with CLI and MCP layering details.
Review details

Suppressed comments (5)

website/docs/run/cli.md:182

  • This CLI output example uses a Unicode em dash in the header even though the actual CLI renders an ASCII hyphen. To keep the examples byte-identical to real output, use - here.
tripl doctor — https://tripl.example.com (from $TRIPL_BASE_URL)

website/docs/run/cli.md:192

  • The scan_config_failing example line uses a Unicode em dash, but the CLI message uses an ASCII hyphen (see diagnostics/scan_checks.py _failing_finding). Using the ASCII form keeps the example faithful to real output and to the ASCII-only promise.
        Scan config 'prod events' (1h) has failed 5 consecutive scheduled runs since 2026-07-31T14:08:09Z. Last error: 'Scan failed due to an internal error.' — that is the backend's generic fallback, not the real cause, so the cause is in the worker log for job job-0.

website/docs/run/cli.md:371

  • This project-scoped-key example header uses a Unicode em dash, but the CLI output is ASCII-only and uses -. Align the example with actual output to preserve the byte-identical/logfile promise.
tripl doctor — https://tripl.example.com (from $TRIPL_BASE_URL)

website/docs/run/cli.md:452

  • The status example header uses a Unicode em dash even though the CLI output is ASCII-only and uses an ASCII hyphen. This example should match the actual output exactly.
tripl status — https://tripl.example.com (from $TRIPL_BASE_URL)

cli/README.md:68

  • This scan_config_failing example line uses a Unicode em dash, but the CLI message uses an ASCII hyphen in the same sentence. Keeping this ASCII avoids contradicting the ASCII-only guarantee and keeps examples byte-identical to actual output.
        Scan config 'prod events' (1h) has failed 5 consecutive scheduled runs since 2026-07-31T14:08:09Z. Last error: 'Scan failed due to an internal error.' — that is the backend's generic fallback, not the real cause, so the cause is in the worker log for job job-0.
  • Files reviewed: 31/31 changed files
  • Comments generated: 4
  • Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

Comment thread website/docs/run/cli.md Outdated
Comment on lines +282 to +284
For every scan config that has a collection interval, doctor reads the newest
40 jobs — the same window the dispatcher itself walks — and counts the run of
consecutive **failed dispatcher jobs** at the head of the history.
Comment thread website/docs/run/cli.md Outdated
Comment thread website/docs/run/cli.md Outdated
Comment thread cli/README.md Outdated
vladenisov added a commit that referenced this pull request Jul 31, 2026
…ce key

Copilot's review of #77 found three real inaccuracies, two of them created by
this branch's own code changes:

* website/docs/run/cli.md still described the newest 40 jobs, the window the
  dispatcher walks, after the CLI moved to the API maximum of 200. That number
  is not decoration - it is how far back "how long has this been broken" can
  see, and an operator reading 40 would misjudge every long streak. The section
  now states 200 and says why it deliberately differs from the scheduler's own.
* Every example transcript printed the header with a U+2014 em dash, while
  render.py emits an ASCII hyphen - so copy-paste diverged from real output on
  the same page that promises ASCII-only output two paragraphs later.
* The evidence table for scan_never_collected was missing jobs_seen, which the
  finding has carried since the window-saturation guard landed.

Both of the first two are documentation drifting behind code inside a single
change, which is this repo's most-repeated review finding. So they are now
tests rather than habits: test_contract.py gains a check that every finding
code raised by the rule modules appears in the docs page (found by walking the
AST for Finding(code=...), so it cannot go stale), and one that the documented
job window is the constant actually requested. Both were mutation-checked.

Gates: ruff, ruff format, mypy (18 modules), 147 tests.
@vladenisov
vladenisov changed the base branch from feat/cli-foundation to main August 1, 2026 14:24
The read-only diagnostics the 2026-07-28..31 incident needed. That week cost
four days to: a scan config failing behind the generic "Scan failed due to an
internal error", last_seen_at frozen with no indication why, an accepted schema
drift that had silently deleted a FieldDefinition, and a retry backoff that read
to the operator as a hang. Each is now one invocation away.

doctor runs six checks in a fixed order, exactly once each: connectivity, API
key, projects, data sources, scheduled collection, schema drift. Failures are
findings with stable codes and evidence, not prose; --json is assertable and
exit 3 means "the tool worked, the checks did not" so cron can tell that apart
from the tool breaking (1) and from being misconfigured (2). A skip never counts
as a pass, so a run where nothing could be checked still exits non-zero.

status is the quick view, and takes its counts from ProjectResponse.summary
rather than recomputing them. That is deliberate: monitoring_signal_count is
already gated server-side by SIGNIFICANT_MIN_REL_EFFECT, so reading it means the
CLI agrees with the sidebar badge by construction instead of carrying a third
copy of a constant that has drifted twice already.

Notable decisions and the traps behind them:

* /health is at the root, unauthenticated, and include_in_schema=False, so it is
  probed with its own short-lived client and no Authorization header, and it is
  covered by a test rather than by the contract test that cannot see it.
* Job history is read at the API maximum (200), NOT schedule.py's own 40-row
  window. The backend reads 40 because it only needs the backoff delay, which
  saturates at six failures; doctor answers "how long has this been broken", and
  at 40 rows a 15m config failing for four days reports 40 failures since this
  morning instead of 384 since Monday. A streak that fills the window is
  reported as a lower bound rather than as a measurement.
* A full window holding no dispatcher job proves nothing about "ever" - manual
  scans, replays and event-group applies all write ScanJobs without the mode
  stamp - so that case warns instead of claiming the beat scheduler is dead.
* Demo projects get the scheduler's own 6h collection cooldown as their
  staleness allowance. The demo runs on a 1h interval, so the flat three-interval
  rule called a healthy demo broken for half of every cooldown, on the one
  project every new operator opens first.
* The drift budget is spent round-robin across projects. Spent in project order,
  one large project consumed it entirely and the starved project appeared
  nowhere in the output - including, potentially, the project holding the very
  drift this check exists to surface.
* An interval this CLI cannot convert no longer hides a failing config: staleness
  and the backoff estimate need the denominator, "is it failing" does not.

Also fixes a crash in the shared client, which tripl-mcp imports too: a 2xx with
a non-JSON body (an SSO gateway or captive portal answering with a login page)
raised json.JSONDecodeError, which subclasses ValueError rather than
httpx.HTTPError and so escaped every consumer's error handling. It is now a
TriplAPIError, and doctor keeps its promise to always emit a document.

Adds cli/tests/test_contract.py, mirroring the one mcp-server already had: a
backend route rename used to break the sibling package's CI loudly and this one
silently. Documentation lands in the same change (AGENTS.md:467) at
website/docs/run/cli.md - the URL cli/pyproject.toml already baked into the
published wheel metadata, which 404ed until now. mcp-server's equivalent URL
carried a /docs/ segment the docs site does not serve; corrected here too.

Gates: cli - ruff, ruff format, mypy (18 modules), 145 tests. mcp-server - ruff,
ruff format, mypy (13 modules), 36 tests.
…ce key

Copilot's review of #77 found three real inaccuracies, two of them created by
this branch's own code changes:

* website/docs/run/cli.md still described the newest 40 jobs, the window the
  dispatcher walks, after the CLI moved to the API maximum of 200. That number
  is not decoration - it is how far back "how long has this been broken" can
  see, and an operator reading 40 would misjudge every long streak. The section
  now states 200 and says why it deliberately differs from the scheduler's own.
* Every example transcript printed the header with a U+2014 em dash, while
  render.py emits an ASCII hyphen - so copy-paste diverged from real output on
  the same page that promises ASCII-only output two paragraphs later.
* The evidence table for scan_never_collected was missing jobs_seen, which the
  finding has carried since the window-saturation guard landed.

Both of the first two are documentation drifting behind code inside a single
change, which is this repo's most-repeated review finding. So they are now
tests rather than habits: test_contract.py gains a check that every finding
code raised by the rule modules appears in the docs page (found by walking the
AST for Finding(code=...), so it cannot go stale), and one that the documented
job window is the constant actually requested. Both were mutation-checked.

Gates: ruff, ruff format, mypy (18 modules), 147 tests.
…pl-ey6j.4)

doctor answers "what is broken" at a point in time; watch answers "what is
happening right now". It prints one line per change - a replay advancing a
chunk, a job finishing, a signal opening, an alert delivery failing - and runs
until you stop it. Ctrl-C is its normal exit.

TRANSPORT IS POLLING, NOT SSE, and that is the load-bearing decision.

There is a realtime bus (GET /projects/{slug}/events/stream) and the obvious
design is to consume it. It would have been wrong. `_heartbeat_replay_progress`
(worker/tasks/metrics/tasks.py:264-291) writes replay chunk progress into
ScanJob.result_summary and commits with NO publish_project_event call, so the
one fact this command exists to surface is invisible on that bus. The bus also
no-ops entirely when Redis is absent while still reporting a healthy socket, and
its endpoint is include_in_schema=False so the contract test cannot protect it.
The revisit condition is recorded in watch/__init__.py: one publish call added to
that heartbeat makes a hybrid worth building.

Two defects that a green suite could not see, both found adversarially:

* A FAILED seeding read rendered as a zero on the first screen. follow() built
  the preamble before flushing diagnostics, and a failed read wrote nothing to
  the snapshot, so a 500 on the signals read produced "signals 0 open" and
  data.baseline.open_signals = 0 - indistinguishable from a quiet instance. The
  unread case is now representable (int | None plus an `unread` map), the
  preamble says so and names the diagnostic, the JSON emits null, and the
  degraded lines print BEFORE the screen they qualify.
* job.stalled was fabricated for a job watch could not see. absorb() carries the
  previous snapshot forward and overwrites only what was read successfully,
  while the stall tracker had no notion of which streams were refreshed - so a
  job that finished at t+30s while its read 502'd for five minutes produced, in
  the same ticks, "jobs read failed, silence does NOT mean idle" AND "job
  unchanged for 4m at chunk 5 of 18". The tracker now skips unrefreshed targets
  and rebases on recovery, so every second it reports is a second watch was
  actually looking.

The stall model was wrong at both ends and is now honest. A pending job was
excluded entirely, so a dead Celery worker - the most common way a scan appears
to hang - gave one job.queued line and then silence. A metrics_collection job
was structurally guaranteed to be reported as stalled once it outran the
threshold, because it has no progress channel and its fingerprint cannot change
while it runs. Now: a job WITH a progress channel stalls when the progress stops
(job.stalled); a job without one gets job.unchanged, whose message says only
what was observed - "still in status pending after 4m of watching (no worker has
picked it up)".

Also: BrokenPipeError on `tripl watch | head` ended the run with a traceback and
exit 1 instead of the footer, on a pipeline the docs recommend; poll.degraded
reported the unresolved path template rather than the concrete path doctor puts
under the same key; a scan config discovered mid-run past the cap was dropped
silently, which the startup path explicitly refuses to do; and clock skew could
render a job started in the future as having run that long.

Docs: the watch section had been inserted in a way that DELETED the `## Exit
codes` heading, nesting the exit-code table and the whole cron recipe inside
`## tripl watch`; and `## --json` still promised "exactly one JSON document",
which is false for a stream. Both fixed, with a `### watch lines` subsection
documenting the JSON Lines envelope that report.py declares a contract.

Gates: ruff, ruff format, mypy (25 modules), 234 tests.
Four contained items, each removing a way for two spellings of one fact to
diverge - this repo's most repeated defect class.

tripl-ey6j.6 (code half). The backend distribution is renamed `tripl` ->
`tripl-server`, per the owner's decision of 2026-08-01. `pip install tripl`
therefore means the operator CLI, which is the point of turning the reserved
name into a tool. The IMPORT package backend/src/tripl/ does NOT change and no
Python source was edited; a comment in backend/pyproject.toml says why the two
names differ, because that is exactly the kind of thing that gets "helpfully"
reverted later. An exhaustive search found only two places that referred to the
backend by its distribution name: pyproject.toml and uv.lock. Everything else
matching "tripl" in a build context is the import package (`uvicorn
tripl.main:app`), the CLI distribution, infrastructure credentials, or product
branding. The mcp-server dependency was verified to still resolve to the CLI
rather than to the backend, in a real synced venv, not assumed.

The PyPI publish workflow is deliberately NOT here: it needs a pending publisher
the owner must create first, so .6 stays open.

tripl-ey6j.7. `tripl_mcp.__version__` was a literal, a second spelling of the
version pyproject declares. It now reads the installed distribution metadata,
copying the sibling cli package exactly. Its User-Agent and --help banner follow
for free. A test pins it against pyproject, and was checked to actually bite: a
literal at 0.0.9 against pyproject's 0.1.0 fails both the pin and the
User-Agent assertion.

tripl-ey6j.8. Nothing tied the CLI's six mirrored scheduler constants to the
backend's. They matched, but the pinning test compared the CLI to literals
inside its own package, so a backend change would leave every CLI test green
while doctor's backoff estimate, its demo staleness allowance and the backoff
table published in the docs quietly became wrong. The guard now lives in the
backend suite, whose CI job has the whole repo checked out, and reads the two
CLI modules with `ast` so it costs the backend no dependency on the CLI.

Worth recording: the first version of that reader silently reported two of the
six as None, because `ast.literal_eval` rejects `24 * 3600`. A guard that passes
vacuously is worse than no guard, so the shipped test folds Mult/Add and treats
an unreadable expression or a missing name as a FAILURE, never a silent pass.
Mutation-checked: changing FAILURE_BACKOFF_AFTER in the CLI turns it red with a
message naming both files, both values and what to do.

tripl-ey6j.9. `drift_scan_truncated` was one instance-wide finding, so an
operator reading "40 of 90 event types examined" could not tell WHICH project
was only partly looked at - and if the unexamined part held the accepted
missing_field drift that deleted a FieldDefinition, the check written to surface
exactly that said nothing about not having looked there. Coverage is now carried
per project; a project examined in full raises nothing and the starved one is
named. Snapshot's two instance-wide counters became derived properties of that
map, so the headline and the per-project numbers cannot disagree.

Gates: cli - ruff, format, mypy (25 modules), 235 tests. mcp-server - ruff,
mypy (13 modules), 39 tests. backend - ruff, and a 35-test subset covering
settings load, the OpenAPI contract (which imports the whole app and router
graph, so it is the real proof the rename did not break wiring), the alembic
revision guard and the new mirror guard.
…MCP tools (tripl-ey6j.5)

The issue's acceptance criterion was the interesting part: "no request-building
logic is duplicated between the CLI and the MCP tools". The client was already
shared; what was not shared was the layer above it - the path strings, the query
parameters, and the decisions about which endpoint answers a compound question.

That layer now lives in cli/src/tripl_cli/api/ and both surfaces import it. A
request is a VALUE (ApiRequest: method, path, params, json_body) rather than a
call, which is what lets the CLI wrap every read into Fetched for doctor's
totality while tripl-mcp keeps the exception its ToolError translation needs -
and makes --dry-run honest, because the command prints the request object it is
holding rather than a reconstruction of it.

Two contract tests turn the criterion from a claim into a fact: every REST path
literal anywhere in tripl_cli or tripl_mcp must equal a declared template, and
ApiRequest is constructible only inside the shared layer. A contributor cannot
half-follow the rule.

Review found two things worth the prose:

* list_scans became a trimmed projection - defensible, since the full
  ScanConfigResponse is 30-odd fields including raw base_query SQL and an agent
  pays context for all of it - but the trimmed fields were then reachable from
  no MCP tool at all. That is not a trim, it is a capability removal. Added
  get_scan for the full config, and documented the change as breaking with the
  migration in one sentence.
* The cancel confirmation and the docs both said "In-flight work is discarded".
  The backend says the opposite, in both directions: cancel_scan_job marks the
  job cancelled and best-effort revokes the task, but "a running task is not
  killed: it polls this status at each chunk boundary and stops cooperatively,
  leaving already-written metrics intact" (scan_service.py:379-390). An operator
  deciding whether to cancel a long replay would have decided on a false premise.
  Corrected in the prompt an operator actually reads and in the docs.

Gates: cli - ruff, format, mypy (41 modules), 311 tests. mcp-server - ruff,
format, mypy (13 modules), 45 tests.
A fresh host goes from nothing to a running instance, and an existing one moves
between versions, without the operator transcribing deployment.md by hand.

The CLI gains no dependency to do it: it shells out to docker compose through an
injected runner, so the whole thing is testable without ever starting a
container - which matters on a box that has been frozen once by exactly that.

Honest about what it cannot do. `install` stops at a running, empty instance and
prints a URL, because creating the first owner account over the API would mean a
password on a command line and a Secure session cookie that a plain-HTTP first
run discards - and connecting a data source is owner-session-only, so an API key
is 403 there whatever its scope. The admin guide now says so in the same words
rather than implying the CLI finishes the job.

Three defects the adversarial pass caught, all of which a green suite missed:

* `--yes` meant BOTH "I acknowledge the backup prompt" AND "override the
  ordering guard". Automation must pass `--yes` to avoid hanging on the prompt,
  so every non-interactive upgrade silently ran with the downgrade guard off -
  and applying migrations backwards is the most destructive thing this CLI can
  do. The override is now its own flag, `--allow-unordered-tag`, and the refusal
  says outright that `--yes` alone does not grant it.
* Re-running `install` with a changed --app-url or --version reported the NEW
  values as applied. The .env is deliberately kept - it holds ENCRYPTION_KEY -
  so nothing changed, but the plan, the human output, the JSON document and the
  health poll all used the requested value. The plan now carries effective vs
  requested per setting, the table marks what was kept from .env, and an ignored
  request prints the real remedy instead of a false confirmation.
* `test_the_generated_env_defines_every_variable_compose_requires` never read
  compose.yaml, so a new mandatory interpolation would have shipped a .env
  missing it with the suite still green. It now extracts every interpolation
  from the packaged file and was mutation-checked: a fake required variable
  turns it red, restoring the file turns it green.

Also: an embedded newline in --app-url was written unescaped into the 0600 .env,
injecting arbitrary KEY=value lines that Docker Compose reads - urlsplit deletes
CR/LF before parsing and so reports such a URL as valid. Rejected at validation,
and the env renderer now refuses a value containing a line break rather than
trusting its caller. And freshly generated secrets were appended to a
world-readable .env behind nothing but a printed warning; that now refuses,
because no later chmod un-discloses a secret already written.

Docs: the issue's second acceptance criterion is that the guides reference the
command rather than duplicating it. deployment.md keeps the by-hand path - an
operator who cannot install the CLI still needs it - but each duplicated fact
now has one home and the other side links to it. The openssl recipe lives in
deployment.md and appears nowhere else.

Gates: ruff, ruff format, mypy (51 modules), 445 tests.
Mirrors publish-mcp.yml: Trusted Publishing over OIDC with no stored token, a
tag-versus-pyproject check because a PyPI version can never be reused, the full
cli gate suite repeated so a release is never the first time it runs, and a
clean-virtualenv smoke test with no --find-links.

Two things this smoke test asserts that no unit test can:

* `tripl --version` and `tripl doctor --help` run from a wheel installed off the
  index with only its declared constraints - the check that catches what a
  lockfile hides. tripl-mcp learned it expensively when an unbounded `mcp` floor
  resolved to 2.0 and the console script died on import.
* the wheel does NOT install an importable `tripl`. The import package is
  tripl_cli precisely so it cannot shadow backend/src/tripl/ in an environment
  holding both, and a packaging change could silently undo that.

release.md documented one of three release trains. It now covers all three: the
service on `v*` via bin/release.sh, and the two Python distributions on their
own tags, versioned independently. It also records the ordering constraint -
publish-mcp.yml's smoke test fails on purpose until `tripl` reaches the index,
because tripl-mcp declares tripl>=0.1,<0.2 - and the exact pending-publisher
fields, which cannot be created from here.

The first publish still needs the owner to create that pending publisher on
PyPI. workflow_dispatch also only registers once this file is on the default
branch, so the TestPyPI rehearsal is available after merge, not before.
… dies on boot

Found while packaging compose.yaml for `tripl install` (tripl-ey6j.3), and it is
a live production defect rather than tidiness.

compose.yaml forwards ~28 optional settings as `VAR: ${VAR:-}` so that a value
placed in .env actually reaches the container - the omission that made
DEMO_ENABLED (tripl-2su6.16) and then REGISTRATION_MODE (tripl-jfm3.101) inert.
But Compose's map syntax materialises an UNDEFINED variable as the empty STRING
inside the container, not as an absent one. pydantic then tried to parse "" as a
bool, an int and an enum, and `Settings()` is constructed at module import - so
the app, migrate, worker and beat containers all exited on boot of any fresh
`docker compose up` whose .env left those five blank. Reproduced: removing
`env_ignore_empty` from model_config turns the new test red with exactly five
validation errors, on exactly the five typed members of that passthrough list.

The running production instance is unaffected because its .env sets them, which
is why this survived until something tried to provision a machine from scratch.

.env.example now points at `tripl install` for secret generation instead of
publishing a third copy of the recipe, and compose.yaml records why the
`${VAR:-}` lines and this flag depend on each other - deleting either one alone
reintroduces one of the two bugs above.

Also stages the packaged compose.yaml's counterpart at the repo root, which the
previous commit missed: cli/tests/test_contract.py asserts the two are
byte-identical, and CI had the old root file against the new packaged copy.
@vladenisov
vladenisov merged commit 763266d into main Aug 1, 2026
5 checks passed
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.

2 participants