Save raw metrics exposition in Phase 0 and mark ddev env tools read-only#23998
Save raw metrics exposition in Phase 0 and mark ddev env tools read-only#23998luisorofino wants to merge 1 commit into
Conversation
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e87403d36b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| "ddev_env_start": ToolSpec("shell.ddev.env_start", "DdevEnvStartTool", read_only=False), | ||
| "ddev_env_stop": ToolSpec("shell.ddev.env_stop", "DdevEnvStopTool", read_only=False), | ||
| "ddev_env_test": ToolSpec("shell.ddev.env_test", "DdevEnvTestTool", read_only=False), | ||
| "ddev_env_test": ToolSpec("shell.ddev.env_test", "DdevEnvTestTool", read_only=True), |
There was a problem hiding this comment.
Keep ddev_env_test out of read-only tools
When a goal-reviewer is limited to read-only tools, this entry now gives it ddev_env_test, but that command is not just observational for inactive environments or environment="all": I checked ddev/src/ddev/cli/env/test.py, where it invokes start before running tests and stop in finally, and the command docstring says inactive environments are started and stopped automatically. That can create/tear down E2E containers and write environment metadata/config, so it violates the registry contract that read-only tools only inspect state and never mutate it.
Useful? React with 👍 / 👎.
Validation ReportAll 21 validations passed. Show details
|
What does this PR do?
Two small, self-contained additions to the OpenMetrics generation flow's plumbing:
<phase_id>_exposition.txtsidecar, records its path in the checkpoint, and surfaces it in the phase memory — alongside the existing parsed JSONL catalog.ddev_env_showandddev_env_testare markedread_only=Truein the tool registry, since neither writes integration files.Includes unit tests for the new sidecar (path in checkpoint, verbatim contents, memory reference, and the atomic-write failure path).
Motivation
A later phase generates the integration's unit tests, which mock the metrics endpoint from a
tests/fixtures/metrics.txtcapture. Persisting the raw body at inspection time gives a fixture that exactly matches the metric catalog every downstream phase works from — no re-fetching a possibly-changed endpoint, no drift from the parsed snapshot.The
read_onlyflags let the goal-validation reviewer (which only receives read-only tools) runddev env show/ddev env testto objectively verify a generated integration's tests pass, instead of relying on the worker's self-report. Both tools only list environments / run tests; they never mutate integration files, so the classification is accurate.ddev_test/ddev_validatestay non-read-only because their fix/sync modes do write files.Review checklist (to be filled by reviewers)
qa/requiredif this PR needs QA validation, orqa/skip-qaif it does not. Exactly one of the two is required.backport/<branch-name>label to the PR and it will automatically open a backport PR once this one is merged