Skip to content

feat(port-management): impl proxy for local dev#293

Merged
jdx merged 4 commits into
jdx:mainfrom
gaojunran:feat-proxy-2
Apr 10, 2026
Merged

feat(port-management): impl proxy for local dev#293
jdx merged 4 commits into
jdx:mainfrom
gaojunran:feat-proxy-2

Conversation

@gaojunran

Copy link
Copy Markdown
Contributor

No description provided.

@gaojunran gaojunran marked this pull request as draft March 24, 2026 10:46
@gemini-code-assist

Copy link
Copy Markdown

Warning

Gemini is experiencing higher than usual traffic and was unable to create the summary. Please try again in a few hours by commenting /gemini summary.

@greptile-apps

greptile-apps Bot commented Mar 24, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR implements a local-dev reverse proxy for pitchfork, routing <slug>.<tld> URLs to daemon ports. It adds a new proxy CLI subcommand (trust, add, remove, status), a full-featured HTTP/HTTPS proxy server with SNI-based dynamic TLS cert issuance, per-daemon active_port runtime detection, and slug registry management via the global config. It also fixes several sudo-related permission issues in the supervisor and IPC socket setup. Prior review concerns (CA key exposure, world-writable socket, state-file vs config-file routing) are addressed in this revision.

Confidence Score: 4/5

Safe to merge with the two P2 items addressed; all prior security concerns have been resolved in this revision.

The major prior concerns (CA key exposure via blanket chmod, world-writable IPC socket, slug routing via config files instead of state file, daemon dir ignored in routing) are all resolved. Two new P2 findings remain: is_daemon_slug_target ignores namespace causing unnecessary listeners::get_all() polling for same-named daemons in other projects, and non-deterministic routing when namespace resolution silently fails. Neither causes incorrect routing but both can cause confusing behavior in multi-project setups.

src/supervisor/lifecycle.rs (is_daemon_slug_target namespace scoping) and src/proxy/server.rs (multi-match fallback warning)

Important Files Changed

Filename Overview
src/proxy/server.rs New 1337-line proxy server: handles HTTP/HTTPS routing with SNI TLS, slug cache with 2s TTL, loop detection, WebSocket forwarding. resolve_target_port now correctly uses the state file with namespace scoping.
src/supervisor/lifecycle.rs Adds active_port detection via detect_and_store_active_port with PID-guarded retry backoff. has_port_config uses is_daemon_slug_target which only matches by name (not namespace), causing unnecessary detection on same-named daemons in unrelated projects.
src/supervisor/mod.rs Starts proxy server at supervisor startup, fixes sudo ownership via chown_recursive with explicit proxy/ skip to protect ca-key.pem.
src/cli/proxy.rs New proxy CLI subcommands: trust (macOS/Linux cert install), status (slug listing with runtime state), add/remove (global config registry management). Slug validation correctly rejects dots and non-alphanumeric characters.
src/pitchfork_toml.rs Adds SlugEntry/SlugEntryRaw types, [slugs] section in global config, add_slug/remove_slug with proper file locking across read-modify-write, and slug resolution in daemon ID lookup.
src/ipc/server.rs Replaces the previous 0o666 world-writable socket approach with chown back to SUDO_UID/SUDO_GID — a significant security improvement from prior revision.
src/daemon.rs Adds active_port, slug, proxy fields to Daemon/RunOptions; changes mise from bool to Option for global setting inheritance. Schema compatibility note is thorough and accurate.
tests/test_e2e_proxy.rs Comprehensive e2e tests covering slug start/stop, cross-directory resolution, add/remove CLI, multi-slug disambiguation. Good coverage for the new feature surface.

Sequence Diagram

sequenceDiagram
    participant Browser
    participant Proxy as Proxy Server
    participant Cache as Slug Cache (2s TTL)
    participant StateFile as State File
    participant Daemon as Backend Daemon

    Browser->>Proxy: GET https://api.localhost:7777/path
    Proxy->>Proxy: Strip port → host = api.localhost
    Proxy->>Cache: cached_slug_lookup(api)
    alt Cache expired
        Cache->>Cache: build_slug_entries() reads global config
        Cache-->>Proxy: CachedSlugEntry namespace project-a daemon_name api
    else Cache hit
        Cache-->>Proxy: CachedSlugEntry from memory
    end
    Proxy->>StateFile: lock and clone daemons
    StateFile-->>Proxy: IndexMap DaemonId Daemon
    Proxy->>Proxy: filter by name + namespace to active_port
    alt Daemon running active_port set
        Proxy->>Daemon: GET http://localhost:3000/path
        Daemon-->>Proxy: 200 OK
        Proxy-->>Browser: 200 OK plus x-pitchfork header
    else Daemon not running or no port
        Proxy-->>Browser: 502 Bad Gateway
    end
    Note over Daemon,StateFile: On daemon start detect_and_store_active_port polls listeners every 500 1000 2000 4000 ms and writes active_port to state file
Loading

Reviews (9): Last reviewed commit: "fix: misc" | Re-trigger Greptile

Comment thread src/proxy/server.rs
Comment thread src/supervisor/lifecycle.rs
Comment thread src/pitchfork_toml.rs Outdated
@jdx

jdx commented Mar 24, 2026

Copy link
Copy Markdown
Owner

@gaojunran I am still thinking about this but I think that maybe we should define the slugs only in the global pitchfork.toml which would point to project directories. I'm thinking that would be a more obvious way to configure it when later we get auto-start when trying to connect to one. Otherwise you'd need to first run the daemon in a project without auto-start to get it populated into state.toml which I think isn't obvious to users.

@gaojunran

gaojunran commented Mar 24, 2026

Copy link
Copy Markdown
Contributor Author

@gaojunran I am still thinking about this but I think that maybe we should define the slugs only in the global pitchfork.toml which would point to project directories. I'm thinking that would be a more obvious way to configure it when later we get auto-start when trying to connect to one. Otherwise you'd need to first run the daemon in a project without auto-start to get it populated into state.toml which I think isn't obvious to users.

@jdx I understand your thoughts. And I'm hesitating about if it's complicated or bothersome for users to define twice in two pitchfork.toml. And in a specific project, every user will need to configure once in global pitchfork.toml.

What about just introducing pitchfork trust and trusted configs will be recorded. Then pitchfork will search for the configs and find out the slug.

Is that acceptable? I think that's more user friendly. And using trust to maintain a mapping from namespace to project dir is useful. Just an idea and I can also follow your idea if you don't prefer this.

@jdx

jdx commented Mar 25, 2026

Copy link
Copy Markdown
Owner

I would definitely prefer to avoid trust if at all possible, it's a pretty annoying thing about mise. I suppose if pitchfork is auto-launching we may need it though.

@greptile-apps

greptile-apps Bot commented Apr 9, 2026

Copy link
Copy Markdown
Contributor

Tip:

Greploop — Automatically fix all review issues by running /greploops in Claude Code. It iterates: fix, push, re-review, repeat until 5/5 confidence.

Use the Greptile plugin for Claude Code to query reviews, search comments, and manage custom context directly from your terminal.

Comment thread src/proxy/server.rs Outdated
Comment thread src/supervisor/mod.rs Outdated
Comment thread src/ipc/server.rs Outdated
@gaojunran gaojunran force-pushed the feat-proxy-2 branch 2 times, most recently from dcc5aca to 5317271 Compare April 9, 2026 11:04
@gaojunran

gaojunran commented Apr 9, 2026

Copy link
Copy Markdown
Contributor Author

@jdx Ready for a review!

@gaojunran gaojunran marked this pull request as ready for review April 9, 2026 12:12
@jdx

jdx commented Apr 9, 2026

Copy link
Copy Markdown
Owner

I've been thinking more about the slug architecture. I think slugs should be defined only in the global config (~/.config/pitchfork/config.toml), not in per-project pitchfork.toml files. The global config becomes the single source of truth for slug→project mappings.

Proposed format

# ~/.config/pitchfork/config.toml

[slugs]
api = { dir = "/home/user/my-api", daemon = "server" }
frontend = { dir = "/home/user/my-app", daemon = "dev" }
# or if daemon name matches slug:
docs = { dir = "/home/user/docs-site" }  # defaults daemon = "docs"

Each slug entry maps to a project directory and (optionally) a specific daemon name within that project.

Why this is better than the current approach

  1. Single source of truth — no pitchfork proxy register sync step needed, no risk of global registry getting stale when project configs change
  2. Auto-start becomes trivial — when a request hits api.localhost, the proxy knows the dir, can load the project config, and start the daemon automatically. With per-project slugs you'd need to have already run the daemon once (or run register) before this works.
  3. Cross-directory resolution just works — no fallback logic needed, no reading config files from other projects
  4. Explicit and auditable — one file shows all your proxied services. You can see conflicts immediately.

What this means for the PR

  • Remove slug and proxy fields from PitchforkTomlDaemon / per-project config
  • Remove pitchfork proxy register subcommand
  • The [slugs] section in global config replaces the current registry
  • Proxy server resolves slugs by reading global config → finding the dir → loading that project's config to get daemon details → checking state file for the running daemon's port
  • pitchfork proxy status should show all registered slugs and their current state (running/stopped, port, etc.)
  • Could add a convenience command like pitchfork proxy add <slug> that adds an entry to global config for the current directory

Auto-start (future, not needed in this PR)

With this design, auto-start later is straightforward: proxy gets a request for api.localhost → looks up slug in global config → finds dir → loads project pitchfork.toml from that dir → starts the daemon → routes request once ready.

This comment was generated by Claude Code.

@gaojunran

Copy link
Copy Markdown
Contributor Author

@jdx I like this approach! This resolves all the problems.

Comment thread src/proxy/server.rs Outdated
feat(port-management): impl url reverse proxy

[autofix.ci] apply automated fixes
Update src/proxy/server.rs

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
[autofix.ci] apply automated fixes
fix: misc

Merge branch 'main' into feat-proxy
refactor: explicitly ask user to register slugs

chore: conflict

fix: misc

[autofix.ci] apply automated fixes
@socket-security

Copy link
Copy Markdown

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Addedcargo/​rcgen@​0.13.29610093100100
Addedcargo/​rustls-pemfile@​2.2.010010093100100

View full report

@jdx jdx merged commit ef6e340 into jdx:main Apr 10, 2026
5 checks passed
@jdx jdx mentioned this pull request Apr 10, 2026
jdx added a commit that referenced this pull request Apr 10, 2026
## 🤖 New release

* `pitchfork-cli`: 2.4.0 -> 2.5.0

<details><summary><i><b>Changelog</b></i></summary><p>

<blockquote>

## [2.5.0](v2.4.0...v2.5.0) -
2026-04-10

### Added

- *(port-management)* impl proxy for local dev
([#293](#293))

### Other

- *(deps)* update rust crate indexmap to v2.14.0
([#321](#321))
- *(deps)* update rust crate xx to v2.5.3
([#322](#322))
- *(deps)* update rust crate tokio to v1.51.1
([#320](#320))
- update inconsistencies in docs
([#312](#312))
</blockquote>


</p></details>

---
This PR was generated with
[release-plz](https://github.com/release-plz/release-plz/).

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> <sup>[Cursor Bugbot](https://cursor.com/bugbot) is generating a
summary for commit d305742. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
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