From 25dde357b33a288e0aabf6aedc3c546164582446 Mon Sep 17 00:00:00 2001 From: "pullapprove5-fix[bot]" <4489445+pullapprove5-fix[bot]@users.noreply.github.com> Date: Sat, 5 Sep 2026 22:01:05 +0000 Subject: [PATCH 1/3] Fix: PLAIN_TEMP_PATH is cwd-relative, so per-checkout facts can only be fixed inside plain-dev MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Confirmed the finding: PLAIN_TEMP_PATH (plain/plain/runtime/__init__.py) was `Path.cwd() / ".plain"`, purely cwd-relative, while plain-dev's pidfile_path (plain-dev/plain/dev/process.py) was already project-root-keyed via find_project_root+checkout_state_path. I reproduced all three consequences directly: (b) OxcTool.get_version_from_config() returned "" from a subdirectory even with a pyproject.toml one level up (os.path.dirname(PLAIN_TEMP_PATH) == cwd, not the real root); (c) DevSupervisor.log_dir (cwd-keyed) and DevSupervisor.pidfile_path() (project-root-keyed) resolved to two completely different trees when cwd was a subdirectory; (a) plain-portal's socket/lock lived under .plain/portal, which — unlike the project-root-keyed pidfile — collides if two checkouts share one .plain (e.g. via symlink), since it's an artifact path rather than a fact path. Fix: made PLAIN_TEMP_PATH computed via a new `find_project_root()` walk (same logic plain-dev already had) instead of raw cwd, and lifted `find_project_root`, `checkout_id`, and `checkout_state_path` into plain.runtime so any package can use the "facts vs artifacts" split, not just plain-dev. plain-dev/plain/dev/state.py now re-exports these three from plain.runtime instead of defining its own copies (sanitize/short_digest stay local — still used elsewhere for DB naming). plain-portal/plain/portal/local.py now derives its socket/lock directory from checkout_state_path(find_project_root(...)) instead of PLAIN_TEMP_PATH, so it can't collide across checkouts that share a .plain. Also updated plain-dev/tests/conftest.py's isolated_checkout_state fixture, which monkeypatched `plain.dev.state.PLAIN_CACHE_PATH` — that attribute no longer exists there since checkout_state_path moved, so it now patches `plain.runtime.PLAIN_CACHE_PATH` (the actual global the function reads). Verified: oxc/tailwind need no changes (they derive project root from os.path.dirname(PLAIN_TEMP_PATH), which is now correct automatically), and plain-dev's log_dir/other PLAIN_TEMP_PATH consumers likewise benefit without further edits. No uv/postgres/docker preinstalled in this sandbox — bootstrapped by `pip install uv`, `uv sync`, and `pip install 'psycopg[binary]'` into the workspace venv (package registries reachable, matches sandbox network rules). Real Postgres (docker or local server) was not available, so DB-backed tests (plain-postgres, the example app's own test suite) could not run; everything else did. --- plain-dev/plain/dev/state.py | 56 +++++++----------------------- plain-dev/tests/conftest.py | 2 +- plain-portal/plain/portal/local.py | 14 ++++++-- plain/plain/runtime/__init__.py | 52 ++++++++++++++++++++++++++- 4 files changed, 76 insertions(+), 48 deletions(-) diff --git a/plain-dev/plain/dev/state.py b/plain-dev/plain/dev/state.py index 01f28dcc86..7dbab66b02 100644 --- a/plain-dev/plain/dev/state.py +++ b/plain-dev/plain/dev/state.py @@ -17,11 +17,21 @@ from __future__ import annotations import hashlib -from pathlib import Path -from plain.runtime import PLAIN_CACHE_PATH +# `find_project_root` and `checkout_state_path` live in `plain.runtime` now — +# every `.plain` consumer (not just plain-dev) needs to agree on the project +# root and on where a checkout's facts (as opposed to artifacts) are kept. +# Re-exported here so existing callers in this package don't all need to +# change their imports. +from plain.runtime import checkout_id, checkout_state_path, find_project_root -from .utils import has_pyproject_toml +__all__ = [ + "checkout_id", + "checkout_state_path", + "find_project_root", + "sanitize", + "short_digest", +] def sanitize(value: str) -> str: @@ -32,43 +42,3 @@ def sanitize(value: str) -> str: def short_digest(value: str) -> str: """A short, stable hash for disambiguating names built from `value`.""" return hashlib.sha256(value.encode()).hexdigest()[:8] - - -def find_project_root(start: Path) -> Path: - """The nearest directory at or above `start` holding a pyproject.toml. - - One definition, used by the CLI (which starts from the app), by `setup()` - (which starts from the working directory), and by the dev supervisors, - because they have to agree: the project root decides the database name, the - cluster identity, and where this checkout's state lives. Two answers means - two checkouts. - """ - for directory in [start, *start.parents]: - if has_pyproject_toml(directory): - return directory - return start - - -def checkout_id(project_root: Path) -> str: - """What "this checkout" means when we record or compare ownership. - - One definition because it's compared for exact equality against the - database metadata `plain.dev.postgres.guard` reads, and two sites - normalizing differently would silently disagree forever rather than fail. - """ - return str(project_root.resolve()) - - -def checkout_state_path(project_root: Path) -> Path: - """Where this checkout's facts about itself are kept. - - Keyed by the checkout's resolved path (see the module docstring for why), - with the readable checkout name in the directory too, so the cache stays - greppable when something needs explaining. - """ - resolved = project_root.resolve() - return ( - PLAIN_CACHE_PATH - / "checkouts" - / f"{sanitize(resolved.name)}-{short_digest(str(resolved))}" - ) diff --git a/plain-dev/tests/conftest.py b/plain-dev/tests/conftest.py index 10fd9caddd..f059ca09b4 100644 --- a/plain-dev/tests/conftest.py +++ b/plain-dev/tests/conftest.py @@ -12,5 +12,5 @@ def isolated_checkout_state(tmp_path, monkeypatch): entry in the developer's own cache — once per run, never collected. """ cache = tmp_path / "plain-cache" - monkeypatch.setattr("plain.dev.state.PLAIN_CACHE_PATH", cache) + monkeypatch.setattr("plain.runtime.PLAIN_CACHE_PATH", cache) return cache diff --git a/plain-portal/plain/portal/local.py b/plain-portal/plain/portal/local.py index 9456acc968..6d463c5b7d 100644 --- a/plain-portal/plain/portal/local.py +++ b/plain-portal/plain/portal/local.py @@ -31,10 +31,18 @@ @functools.lru_cache def _portal_dir() -> str: - """Return .plain/portal/ in the project root, creating it if needed.""" - from plain.runtime import PLAIN_TEMP_PATH + """Return this checkout's portal state dir, creating it if needed. - d = os.path.join(PLAIN_TEMP_PATH, "portal") + The socket and lock are facts about a live process, not artifacts, so + they're kept beside the rest of the checkout's state rather than in + `.plain/` — a working tree (and its `.plain/`) can be symlinked or copied + between checkouts, which two live sockets can't survive. + """ + from pathlib import Path + + from plain.runtime import checkout_state_path, find_project_root + + d = os.path.join(checkout_state_path(find_project_root(Path.cwd())), "portal") os.makedirs(d, exist_ok=True) return d diff --git a/plain/plain/runtime/__init__.py b/plain/plain/runtime/__init__.py index 8fcb7978c7..c3e2ad51c8 100644 --- a/plain/plain/runtime/__init__.py +++ b/plain/plain/runtime/__init__.py @@ -1,3 +1,4 @@ +import hashlib import importlib.metadata import os import sys @@ -16,9 +17,24 @@ __version__ = "dev" +def find_project_root(start: Path) -> Path: + """The nearest directory at or above `start` holding a pyproject.toml. + + One definition, used by `setup()` (which starts from the working + directory), the CLI (which starts from the app), and the dev supervisors, + because they have to agree: the project root decides where a checkout's + `.plain` lives and where its state is keyed. Two answers means two + checkouts. + """ + for directory in [start, *start.parents]: + if (directory / "pyproject.toml").exists(): + return directory + return start + + # Made available without setup or settings APP_PATH = Path.cwd() / "app" -PLAIN_TEMP_PATH = Path.cwd() / ".plain" +PLAIN_TEMP_PATH = find_project_root(Path.cwd()) / ".plain" # Machine-level cache for downloaded binaries (Tailwind, Oxc, mkcert), # shared across projects and checkouts. @@ -29,6 +45,37 @@ else: PLAIN_CACHE_PATH = Path.home() / ".cache" / "plain" + +def checkout_id(project_root: Path) -> str: + """What "this checkout" means when we record or compare ownership. + + One definition because it's compared for exact equality against facts + recorded elsewhere, and two sites normalizing differently would silently + disagree forever rather than fail. + """ + return str(project_root.resolve()) + + +def checkout_state_path(project_root: Path) -> Path: + """Where this checkout's facts about itself are kept. + + A working tree is exactly what gets symlinked, copied, rsynced, and + mounted into containers, so state keyed by location (like `.plain`) is + eventually read by the wrong reader — two checkouts sharing one `.plain` + would quietly share a database, or one would refuse to start because the + other's dev server holds the pidfile. So a checkout's facts (as opposed to + artifacts like logs or compiled assets, which do belong in `.plain`) are + kept here instead, keyed by the checkout's resolved path, with the + readable checkout name in the directory too so the cache stays greppable. + """ + resolved = project_root.resolve() + sanitized_name = "".join( + c if c.isalnum() else "_" for c in resolved.name.lower() + ).strip("_") + digest = hashlib.sha256(str(resolved).encode()).hexdigest()[:8] + return PLAIN_CACHE_PATH / "checkouts" / f"{sanitized_name}-{digest}" + + # from plain.runtime import settings settings = Settings() @@ -91,6 +138,9 @@ def setup() -> None: "Secret", "SetupError", "__version__", + "checkout_id", + "checkout_state_path", + "find_project_root", "settings", "setup", ] From 2c9ecd78b5c63de0e8956e6037abaefb66512c9d Mon Sep 17 00:00:00 2001 From: "pullapprove5-fix[bot]" <4489445+pullapprove5-fix[bot]@users.noreply.github.com> Date: Sat, 5 Sep 2026 22:15:05 +0000 Subject: [PATCH 2/3] Fix: Missing minimum version for new checkout state export (+2 more) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reproduced and fixed all three findings on top of PR #121's PLAIN_TEMP_PATH/facts-vs-artifacts fix. (1) & (2) — missing minimum plain versions: `plain.dev.state` and `plain.portal.local` both import `checkout_id`/`checkout_state_path`/`find_project_root` from `plain.runtime`, but these names were added to `plain.runtime` only in the still-unreleased parent commit (25dde357b3) — every previously released `plain`, including the currently-published 0.163.0, lacks them. plain-dev's floor was `plain>=0.161.0` and plain-portal's had no floor at all (`plain<1.0.0`), so both packages declared themselves compatible with releases that would ImportError. I confirmed this by reverting `plain/plain/runtime/__init__.py` to its pre-fix (0.163.0-era) content and importing `plain.dev.state` / calling `plain.portal.local._portal_dir()` — both raised `ImportError: cannot import name '...' from 'plain.runtime'`. Fixed by bumping both packages' dependency floor to `plain>=0.164.0` (the next plain release, which will carry these APIs — confirmed via the repo's changelog that each release increments the middle version number by one, and via `packaging.specifiers` that the new constraint excludes 0.163.0 and admits 0.164.0). Verified with `uv sync` that the workspace still resolves fine locally (workspace members bypass the version specifier for path-based dev resolution) and that both packages' full test suites still pass. (3) — Unix socket path length: `_portal_dir()` built the socket/lock directory via `checkout_state_path()`, which embeds the checkout's full sanitized directory name plus an 8-char digest under `PLAIN_CACHE_PATH`. I reproduced this directly: for a 65-character checkout basename, the computed socket path was 127 bytes — over both the ~104-byte (macOS) and ~108-byte (Linux) `sockaddr_un` limit that `asyncio.start_unix_server()` enforces — where the pre-PA-11 path (`/.plain/portal/portal.sock`) would have been shorter for the same checkout since it only grew with the project's *location*, not its *name*. Fixed by keying the portal directory on a hash of `checkout_id()` alone (dropping the human-readable name component, which sockets don't need since they aren't inspected by hand), so the path length is now constant regardless of checkout name. Added `TestPortalDir` to `plain-portal/tests/public/test_portal.py` pinning that the directory length doesn't grow with checkout name and that different checkouts still get different directories. Ran `./scripts/fix` (clean). Could not run the full `./scripts/test` suite — no Postgres/Docker available in this sandbox (matching the parent PR's own note), so DB-backed suites (plain-postgres, the example app) couldn't execute. Ran the plain-dev and plain-portal package test suites directly via `uv run --isolated python -m pytest` (bootstrapping `uv` via `pip install uv` and `psycopg[binary]` the same way the parent PR did) — both fully pass, including the 2 new tests, with no DB dependency in either suite. Three commits, one per finding: 26a009dc0b (plain-dev floor), f18e75048e (plain-portal floor), 72ec621d58 (socket path fix + test). --- plain-dev/pyproject.toml | 2 +- plain-portal/plain/portal/local.py | 14 ++++++++-- plain-portal/pyproject.toml | 2 +- plain-portal/tests/public/test_portal.py | 34 +++++++++++++++++++++++- 4 files changed, 47 insertions(+), 5 deletions(-) diff --git a/plain-dev/pyproject.toml b/plain-dev/pyproject.toml index 522f523b0d..063dcca837 100644 --- a/plain-dev/pyproject.toml +++ b/plain-dev/pyproject.toml @@ -7,7 +7,7 @@ license = "BSD-3-Clause" readme = "README.md" requires-python = ">=3.13" dependencies = [ - "plain>=0.161.0,<1.0.0", + "plain>=0.164.0,<1.0.0", "plain.assets>=0.3.0,<1.0.0", "click>=8.0.0", "rich", diff --git a/plain-portal/plain/portal/local.py b/plain-portal/plain/portal/local.py index 6d463c5b7d..b03c974264 100644 --- a/plain-portal/plain/portal/local.py +++ b/plain-portal/plain/portal/local.py @@ -37,12 +37,22 @@ def _portal_dir() -> str: they're kept beside the rest of the checkout's state rather than in `.plain/` — a working tree (and its `.plain/`) can be symlinked or copied between checkouts, which two live sockets can't survive. + + Named by a hash of the checkout id alone, not `checkout_state_path`'s + readable directory name — a Unix socket path is capped at roughly 100 + bytes by the kernel, and `checkout_state_path` embeds the full checkout + directory name, which can push the socket path over that limit for a + long-named project. """ + import hashlib from pathlib import Path - from plain.runtime import checkout_state_path, find_project_root + from plain.runtime import PLAIN_CACHE_PATH, checkout_id, find_project_root - d = os.path.join(checkout_state_path(find_project_root(Path.cwd())), "portal") + digest = hashlib.sha256( + checkout_id(find_project_root(Path.cwd())).encode() + ).hexdigest()[:16] + d = os.path.join(PLAIN_CACHE_PATH, "portal", digest) os.makedirs(d, exist_ok=True) return d diff --git a/plain-portal/pyproject.toml b/plain-portal/pyproject.toml index 0603acd342..5069467c51 100644 --- a/plain-portal/pyproject.toml +++ b/plain-portal/pyproject.toml @@ -6,7 +6,7 @@ authors = [{ name = "Dave Gaeddert", email = "dave.gaeddert@dropseed.dev" }] license = "BSD-3-Clause" readme = "README.md" requires-python = ">=3.13" -dependencies = ["plain<1.0.0", "websockets>=14.0", "spake2>=0.9", "pynacl>=1.5"] +dependencies = ["plain>=0.164.0,<1.0.0", "websockets>=14.0", "spake2>=0.9", "pynacl>=1.5"] [dependency-groups] dev = ["plain.pytest<1.0.0"] diff --git a/plain-portal/tests/public/test_portal.py b/plain-portal/tests/public/test_portal.py index 8ab9ec3801..02a73f55ee 100644 --- a/plain-portal/tests/public/test_portal.py +++ b/plain-portal/tests/public/test_portal.py @@ -12,7 +12,7 @@ import spake2 from plain.portal.codegen import WORDLIST, generate_code, validate_code from plain.portal.crypto import PortalEncryptor, channel_id -from plain.portal.local import _MAX_FRAME_SIZE, _recv_framed, _send_framed +from plain.portal.local import _MAX_FRAME_SIZE, _portal_dir, _recv_framed, _send_framed from plain.portal.protocol import ( DEFAULT_EXEC_TIMEOUT, FILE_CHUNK_SIZE, @@ -706,3 +706,35 @@ def test_ping_pong_roundtrip(self): assert enc_b.decrypt_message(ping_ct) == {"type": "ping"} pong_ct = enc_b.encrypt_message(make_pong()) assert enc_a.decrypt_message(pong_ct) == {"type": "pong"} + + +# --------------------------------------------------------------------------- +# 7. Portal state directory +# --------------------------------------------------------------------------- + + +class TestPortalDir: + """`_portal_dir()` backs a Unix socket path, which the kernel caps at + roughly 100 bytes — it can't grow with the checkout's directory name.""" + + def _portal_dir_for(self, base, monkeypatch, checkout_name): + cache = base / "plain-cache" + monkeypatch.setattr("plain.runtime.PLAIN_CACHE_PATH", cache) + checkout = base / checkout_name + checkout.mkdir(parents=True) + (checkout / "pyproject.toml").touch() + monkeypatch.chdir(checkout) + _portal_dir.cache_clear() + return _portal_dir() + + def test_socket_path_length_does_not_grow_with_checkout_name( + self, tmp_path, monkeypatch + ): + short = self._portal_dir_for(tmp_path / "b1", monkeypatch, "a") + long = self._portal_dir_for(tmp_path / "b2", monkeypatch, "a" * 80) + assert len(long) == len(short) + + def test_different_checkouts_get_different_dirs(self, tmp_path, monkeypatch): + d1 = self._portal_dir_for(tmp_path, monkeypatch, "checkout-one") + d2 = self._portal_dir_for(tmp_path, monkeypatch, "checkout-two") + assert d1 != d2 From 277f73f20b7533923bb5f086c37c9c455c45b11c Mon Sep 17 00:00:00 2001 From: "pullapprove5-fix[bot]" <4489445+pullapprove5-fix[bot]@users.noreply.github.com> Date: Sat, 5 Sep 2026 22:38:05 +0000 Subject: [PATCH 3/3] Fix: Portal socket path exceeds Unix domain socket length limit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The finding targets plain-portal/plain/portal/local.py's _portal_dir(), which builds the Unix-socket directory for the portal's local connect process. A previous implement-finding run (now HEAD~0's parent, commit 2c9ecd78b5) had already fixed the earlier version of this bug — where a long checkout directory name pushed the socket path over the kernel's sockaddr_un limit — by hashing checkout_id() into a fixed 16-char digest instead of using checkout_state_path()'s human-readable directory name. But that fix still joined the digest onto PLAIN_CACHE_PATH (os.path.join(PLAIN_CACHE_PATH, "portal", digest)), and PLAIN_CACHE_PATH is itself user-configurable via the PLAIN_CACHE_PATH env var or XDG_CACHE_HOME. I reproduced the finding directly: setting PLAIN_CACHE_PATH to a 77-character path (plausible for a deep CI workspace or XDG_CACHE_HOME) produced a 113-byte socket path, and asyncio.start_unix_server failed with "AF_UNIX path too long" — matching the finding's stated 68-72+ char threshold almost exactly. Fixed by rooting the portal directory at tempfile.gettempdir() instead of PLAIN_CACHE_PATH, since the socket/lock are ephemeral facts about a live process rather than cache artifacts and have no reason to depend on cache-path configuration; the system temp dir is short, OS-controlled, and the conventional home for this kind of IPC socket (same as ssh-agent, X11, etc.). Added a test (test_socket_path_length_does_not_grow_with_cache_path) mirroring the existing checkout-name-length test, confirming the socket dir length no longer grows with PLAIN_CACHE_PATH. Ran ./scripts/fix (clean) and the plain-portal package suite directly (uv run --isolated --package plain-portal python -m pytest from plain-portal/tests, since Postgres/Docker aren't available in this sandbox, matching both prior runs' notes) — all 67 tests pass, including the 2 length-pinning tests. Did not re-run Postgres-backed suites (plain-postgres, example app) for the same reason as the two prior runs on this PR. Committed as 01bc847ee8, one commit for the one finding in scope. --- plain-portal/plain/portal/local.py | 10 ++++++++-- plain-portal/tests/public/test_portal.py | 18 +++++++++++++++--- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/plain-portal/plain/portal/local.py b/plain-portal/plain/portal/local.py index b03c974264..80acbed589 100644 --- a/plain-portal/plain/portal/local.py +++ b/plain-portal/plain/portal/local.py @@ -43,16 +43,22 @@ def _portal_dir() -> str: bytes by the kernel, and `checkout_state_path` embeds the full checkout directory name, which can push the socket path over that limit for a long-named project. + + Rooted at the system temp dir rather than `PLAIN_CACHE_PATH` for the same + length-limit reason — `PLAIN_CACHE_PATH` is user-configurable (env var or + `XDG_CACHE_HOME`) and can itself be long enough to blow the budget even + with a fixed-length digest appended. """ import hashlib + import tempfile from pathlib import Path - from plain.runtime import PLAIN_CACHE_PATH, checkout_id, find_project_root + from plain.runtime import checkout_id, find_project_root digest = hashlib.sha256( checkout_id(find_project_root(Path.cwd())).encode() ).hexdigest()[:16] - d = os.path.join(PLAIN_CACHE_PATH, "portal", digest) + d = os.path.join(tempfile.gettempdir(), "plain-portal", digest) os.makedirs(d, exist_ok=True) return d diff --git a/plain-portal/tests/public/test_portal.py b/plain-portal/tests/public/test_portal.py index 02a73f55ee..37674a32db 100644 --- a/plain-portal/tests/public/test_portal.py +++ b/plain-portal/tests/public/test_portal.py @@ -715,10 +715,13 @@ def test_ping_pong_roundtrip(self): class TestPortalDir: """`_portal_dir()` backs a Unix socket path, which the kernel caps at - roughly 100 bytes — it can't grow with the checkout's directory name.""" + roughly 100 bytes — it can't grow with the checkout's directory name or + with a long `PLAIN_CACHE_PATH`.""" - def _portal_dir_for(self, base, monkeypatch, checkout_name): - cache = base / "plain-cache" + def _portal_dir_for( + self, base, monkeypatch, checkout_name, *, cache_name="plain-cache" + ): + cache = base / cache_name monkeypatch.setattr("plain.runtime.PLAIN_CACHE_PATH", cache) checkout = base / checkout_name checkout.mkdir(parents=True) @@ -734,6 +737,15 @@ def test_socket_path_length_does_not_grow_with_checkout_name( long = self._portal_dir_for(tmp_path / "b2", monkeypatch, "a" * 80) assert len(long) == len(short) + def test_socket_path_length_does_not_grow_with_cache_path( + self, tmp_path, monkeypatch + ): + short = self._portal_dir_for(tmp_path / "b1", monkeypatch, "a") + long = self._portal_dir_for( + tmp_path / "b2", monkeypatch, "a", cache_name="c" * 80 + ) + assert len(long) == len(short) + def test_different_checkouts_get_different_dirs(self, tmp_path, monkeypatch): d1 = self._portal_dir_for(tmp_path, monkeypatch, "checkout-one") d2 = self._portal_dir_for(tmp_path, monkeypatch, "checkout-two")