Skip to content

feat(config): support ready_http status codes#437

Merged
jdx merged 2 commits into
jdx:mainfrom
jkker:feat/ready-http-status
May 17, 2026
Merged

feat(config): support ready_http status codes#437
jdx merged 2 commits into
jdx:mainfrom
jkker:feat/ready-http-status

Conversation

@jkker

@jkker jkker commented May 14, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add a structured ready_http form that preserves the existing URL string shorthand
  • allow exact accepted HTTP status codes, e.g. ready_http = { url = "http://localhost:3000/health", status = [200, 401] }
  • update docs, generated JSON schema, and readiness tests
  • isolate e2e test XDG config/state paths so tests do not connect to a live local supervisor

Validation

  • mise x rust@stable -- cargo check
  • mise x rust@stable -- cargo test --test test_pitchfork_toml
  • mise x rust@stable bun@latest -- cargo test --test test_e2e ready_http -- --nocapture

This PR was generated with AI assistance.

@greptile-apps

greptile-apps Bot commented May 14, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds structured ready_http config support, allowing users to specify exact accepted HTTP status codes (e.g. { url = \"...\", status = [200, 401] }) alongside the existing plain-URL string shorthand, and updates the CLI override logic to preserve configured status codes when --http supplies a new URL.

  • New ReadyHttp type (src/config_types.rs) implements the StringOrStruct serde pattern so both TOML forms round-trip cleanly; accepts_status and range validation in from_raw are correct.
  • merge_ready_http_override (src/ipc/batch.rs) replaces the previous plain assignment at all three call sites, preserving the daemon's configured status list when only the URL is overridden from the CLI.
  • TUI, web, lifecycle, and schema updated consistently; e2e and TOML-parse tests cover the new path including invalid-status rejection.

Confidence Score: 5/5

Safe to merge; all changed paths behave correctly and the new config type is well-tested.

The implementation is thorough: the new type, serialization, CLI merge logic, TUI preservation, lifecycle check, and schema are all consistent. The two observations in comments are minor edge cases that don't cause incorrect runtime behaviour with valid configurations.

src/config_types.rs — minor parse-time validation gaps (empty URL, explicit empty status array) worth a follow-up.

Important Files Changed

Filename Overview
src/config_types.rs Adds ReadyHttp type with StringOrStruct serde pattern, status validation (100-599), and correct accepts_status logic; two minor edge cases: empty URL passes parse-time validation, and an explicit empty status array silently maps to 2xx.
src/ipc/batch.rs Adds merge_ready_http_override helper that correctly preserves configured status codes when CLI --http overrides the URL; all three call sites updated consistently, with unit tests covering the key invariants.
src/supervisor/lifecycle.rs HTTP readiness check now routes the response status through accepts_status instead of is_success(); change is minimal and correct.
src/tui/app.rs Introduces preserved_ready_http_status to round-trip status codes through the URL-only form field; correctly re-attaches preserved codes when the user saves.
tests/common/mod.rs Sets XDG_CONFIG_HOME and XDG_STATE_HOME env vars in all test commands so e2e tests no longer connect to a live supervisor on the developer's machine.
tests/test_pitchfork_toml.rs New tests cover structured ready_http parse, accepts_status logic, and invalid status code rejection; good coverage of the new validation path.
tests/test_e2e.rs Adds end-to-end test for non-2xx readiness (401) using the updated http_server.ts; structure mirrors existing ready_http tests.
src/pitchfork_toml.rs Re-exports ReadyHttp and updates PitchforkTomlDaemon/PitchforkTomlDaemonRaw fields from Option to Option; straightforward type migration.
docs/public/schema.json Schema updated with ReadyHttp oneOf (string

Reviews (2): Last reviewed commit: "fix(ipc): preserve ready_http status on ..." | Re-trigger Greptile

Comment thread src/ipc/batch.rs Outdated
@jdx jdx enabled auto-merge (squash) May 17, 2026 14:16
@jdx jdx merged commit ec885d8 into jdx:main May 17, 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