feat(config): core/process.run — one guarded exec, and the logic in Python
Clarifies the rewrite decision to what it actually meant: rewriting the bash in Python does not mean reimplementing the operating system. A guarded exec is the right answer for rustup, curl, unzip, git, godot, blender. What must become Python is the LOGIC — which version is wanted, whether it is already present, what the output means, what to do when it fails. The test of a correct port is not whether it calls anything external, but whether the decisions can be exercised without performing them. Delivered ahead of the remaining ports because every one of them needs it. core/process.run is the single sanctioned exec, and each of its guards exists because a per-domain subprocess call is precisely where that guard goes missing: - An argv list, never a shell string. A string is rejected outright rather than helpfully split, since the helpful split is the vulnerability. - shell=False always. - A non-zero exit becomes a ReachError naming the command, carrying its output, and preserving its exit code — not a CalledProcessError traceback at someone who wanted to know the next step. - A missing binary reports what to install. FileNotFoundError names the path that was not found, which is the less useful half of the answer. All four verified against real commands, including a genuine git failure relaying exit 128. A conformance invariant keeps the door single: nothing outside core/process.py may import subprocess or call os.system/popen/exec*. Proven to fail by importing subprocess into a domain service. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Claude Opus 5
@@ -2088,3 +2088,48 @@ SCOPE CORRECTION 2026-08-31, found on starting the first port and materially lar
|
||||
INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G5GWCTS89VSST07DFVBVG064', 'description', 'Finish what T-1262 started. domains/check/ currently has one verb (client-version); the other four gates join it: check-canvas-version, check-systems-db-stamp, check-fact-ids and check-dataflow-graph.py. Per the domain map, canvas_sources.py and generator_sources.py move alongside as module-level REGISTRIES, not verbs — nothing types them, and putting them in reach --help would answer a question nobody has. schema_version.py goes to the db domain where it is defined; check imports it, which is what a service layer is for. STANDARD PORT ACCEPTANCE, applying to every per-domain ticket under this epic: (a) service.py holds the logic and is transport-agnostic — no printing, no sys.exit, no typer; (b) router.py holds no logic and every command wears @command; (c) a parity test per gate, copying the pattern in tooling/test_check_parity.py, which builds a throwaway fixture repo and runs BOTH implementations — exit codes must match exactly and no fact the old message carried may be lost; (d) the old scripts are RETIRED once parity passes, not left alongside; (e) that domain''s tooling make targets are retired too rather than left as wrappers, per the D-263 make/reach split — build orchestration stays with make. Note check-dataflow-graph.py and check-canvas-version both parse git output, so their fixtures need a git repo rather than a bare directory; that is the one thing here harder than client-version was.', 'Finish what T-1262 started. domains/check/ currently has one verb (client-version); the other four gates join it: check-canvas-version, check-systems-db-stamp, check-fact-ids and check-dataflow-graph.py. Per the domain map, canvas_sources.py and generator_sources.py move alongside as module-level REGISTRIES, not verbs — nothing types them, and putting them in reach --help would answer a question nobody has. schema_version.py goes to the db domain where it is defined; check imports it, which is what a service layer is for. STANDARD PORT ACCEPTANCE, applying to every per-domain ticket under this epic: (a) service.py holds the logic and is transport-agnostic — no printing, no sys.exit, no typer; (b) router.py holds no logic and every command wears @command; (c) a parity test per gate, copying the pattern in tooling/test_check_parity.py, which builds a throwaway fixture repo and runs BOTH implementations — exit codes must match exactly and no fact the old message carried may be lost; (d) the old scripts are RETIRED once parity passes, not left alongside; (e) that domain''s tooling make targets are retired too rather than left as wrappers, per the D-263 make/reach split — build orchestration stays with make. Note check-dataflow-graph.py and check-canvas-version both parse git output, so their fixtures need a git repo rather than a bare directory; that is the one thing here harder than client-version was.
|
||||
|
||||
PROGRESS 2026-08-31 — fact-ids ported, the first bash-to-Python rewrite under the D-263 decision. It was 89 lines of grep/sed pipeline; it is now service.fact_ids() returning a FactIdCheck, with a router that renders it. PARITY IS EXACT ON THE LIVE TREE: both implementations print ''check-fact-ids: OK — 6 references validated against 61 canonical facts'' and exit 0. The matching COUNTS are the real evidence — if the line-matching regex differed from the grep chain even slightly, 6 and 61 would diverge. Kept line-matched rather than YAML-parsed on purpose: parsing properly would change which lines count (anchors, merge keys, multi-document files would start contributing ids the old check never saw), which is a different check wearing the same name. Three parity cases added to test_check_parity.py — ok, unknown fact_id, and the advisory mode where catalogs are unpopulated. Proven to fail: removing the entity-attributes.yaml exclusion was caught, and interestingly by the ADVISORY case rather than the exclusion assertion — including that file made the catalog non-empty, so the new implementation went to enforcing mode while the old stayed advisory. A real behavioural divergence, caught by exit code. RETIREMENT DEFERRED to the end of this ticket, and there is a reason worth recording. The rule is retire per DOMAIN, and three gates remain. It also resolves a tension: the parity test COPIES the old script into its fixture, so deleting the script early would delete the test''s own subject. A parity test is scaffolding with a defined lifetime — when the old script goes, its parity case goes with it and is replaced by a unit test of the new behaviour. Same will apply to every port under T-1250. NOTE the pre-commit hook calls tooling/check-fact-ids directly, so retirement means editing that hook — and a mistake there is felt on every commit rather than every push. Do it together with validate-content (T-1282), which is in the same hook, so it is edited once.', NULL, '2026-08-31 15:38:42', '2026-08-31 15:38:42.109', '2026-08-31 15:38:42.109', NULL, '0ea9d87f8863bc38b0020df38923641c', 2) ON CONFLICT(hash) DO NOTHING;
|
||||
INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G1S3D0M1TQW0GMFBBPQZG3ZM', 'description', 'The mechanical bulk. Move every Python file into tooling/domains/<name>/ split router/service/schemas/helpers, normalising names on the way in. The split is DISCOVERED not invented — the domains are already encoded as filename prefixes (blender x14, atlas x8, generate x7, check x7, visual/validate/test x3, godot/garment/pql/install x2), which is the main evidence this is safe to do mechanically. Pieces that become tickets, roughly one per domain: atlas, check, validate, generate, visual, godot, wiki, db, dev. Cross-cutting pieces: (1) naming normalisation — modules snake_case, CLI verbs kebab-case, replacing the four conventions currently coexisting (assign-astro-ids.py, canvas_sources.py, check-canvas-version, blender_ prefix-as-namespace); (2) the four hyphenated directories planet-gen, economy-db, garment-fit, pql-migrate are not importable and must be renamed; (3) tooling/econ-sim is a Rust crate — excluded from package discovery, not moved; (4) the Blender carve-out — tooling/blender stays a bash wrapper and the 35 blender_*.py payload files move to tooling/scripts/blender/, executed under Blender bundled Python and never imported, with reach fronting them; (5) ruff clean under the existing config, and the E402/E702/F841 ignores revisited since the script-style sys.path.insert that forced E402 disappears with the package. Watch for: a domain that only has a router and a service does not need empty schemas.py and dependencies.py — the layering is a vocabulary, not a quota.
|
||||
|
||||
SURVEY 2026-08-31, before refining. The description above is right about the SHAPE and wrong about three counts. Re-measured:
|
||||
|
||||
RUST CRATES: THREE, not one. The description names only tooling/econ-sim. There are also tooling/line-previewer and tooling/test-client — both Cargo.toml + src/ with zero .py files. All three are excluded from the move and from package discovery, and none needs renaming, since a hyphen only matters for something Python has to import.
|
||||
|
||||
HYPHENATED PYTHON DIRECTORIES TO RENAME: FIVE, not four. economy-db (17 .py), garment-fit (23), garment-qa (1), planet-gen (30), pql-migrate (5). The description misses garment-qa. Already-importable subdirectories: db (9), wiki (1), plus the new core/ and domains/.
|
||||
|
||||
FILE COUNT: ~128 legacy .py files plus ~33 extensionless executables, not 123 total. Top level alone holds 42 .py and ~33 executables; the subdirectories hold 76 more .py.
|
||||
|
||||
WHAT THE PREFIX EVIDENCE ACTUALLY SUPPORTS — and this is the part that matters for how this epic is decomposed. Re-counted at the top level: blender 14, atlas 8, generate 7, check 7, test 5, visual 3, validate 3, install 2, godot 2. That is ~51 files with an obvious home, and the description is right that the split is discovered rather than invented for them.
|
||||
BUT there are 28 SINGLETON prefixes with no group to join: assign, canvas, clerk, convert, fill, generator, glb, inspect, migrate, patch, perf, pql, pr, process, render, schema, sculpt, setup, star, synth, tea, trellis, tune, wipe, worktree. Each needs a judgment call about which domain it belongs to, and several are genuinely ambiguous (canvas_sources.py and generator_sources.py are registries consumed by gates; schema_version.py is consumed by the DB importer; tea-comment and pr-watchlist-diff are PR workflow, which is not in the description''s domain list at all).
|
||||
|
||||
CONSEQUENCE FOR REFINEMENT: the description proposes "roughly one ticket per domain: atlas, check, validate, generate, visual, godot, wiki, db, dev". Filing those nine now would be guessing, because ~28 files have no assigned domain and at least one domain (PR/workflow) is missing from the list. THE DOMAIN MAP IS THE BLOCKING DESIGN WORK and it gets its own ticket; per-domain port tickets are filed once it exists, so their boundaries are read off the map rather than invented and then renegotiated mid-move.
|
||||
|
||||
REFINED 2026-08-31 — four cross-cutting tasks filed; the per-domain port tickets are DELIBERATELY NOT filed yet. T-1271 the domain map (blocking design — every file assigned before any file moves), T-1272 rename the five hyphenated Python directories, T-1273 the Blender carve-out, T-1274 retire the ruff ignores the package makes unnecessary. T-1272 and T-1273 are blocked on T-1271; T-1274 runs LAST, since re-enabling E402 mid-move means fixing violations in files that are about to move again. The per-domain tickets get filed from the map rather than from the epic''s proposed list, because that list is missing at least one domain (PR/workflow — tea-comment, pr-watchlist-diff) and leaves 28 singleton-prefix files unassigned. Filing nine tickets now and renegotiating their boundaries mid-move is the failure this ordering avoids.
|
||||
|
||||
SEQUENCING SET 2026-08-31 by decision. (1) T-1264 (streaming, as decorators) lands BEFORE the per-domain ports, so every ported command arrives already streaming. (2) Old scripts retire PER DOMAIN, the moment that domain''s port passes a parity test — not in one sweep at T-1253. So each per-domain port ticket owns its own retirement, and the tree shrinks continuously rather than existing twice for months while edits risk landing in the dead copy. (3) Domain names as mapped are confirmed — no renames. (4) Per D-263''s amended make/reach split, a per-domain port also RETIRES that domain''s tooling make targets rather than leaving wrappers; build/test orchestration targets stay with make.
|
||||
|
||||
WAVE 1 FILED 2026-08-31: T-1281 check (four remaining gates), T-1282 validate, T-1283 godot and visual together. Deliberately NOT filing all fourteen domains at once — the standard port acceptance is defined once on T-1281 and referenced, and the remaining tickets (atlas, starmap, planet, db, wiki, assets, character, generate, dev, pr) get written after wave 1 lands, when the real friction of a port is known rather than guessed. Four descriptions written from experience beat ten written from a map. T-1272 was also RESCOPED today: the hyphenated directories are emptied by the domain moves rather than renamed, so it becomes a closing verification instead of an opening step.
|
||||
|
||||
SCOPE CORRECTION 2026-08-31, found on starting the first port and materially larger than the epic assumed. SEVENTEEN of the 33 extensionless executables are BASH, not Python — about 900 lines. This description and the domain map both assumed a Python tree, so those are REWRITES, not moves. Decided with Jeroen: rewrite all of them in Python rather than wrap any. Wrapping would achieve one door while leaving half the CLI surface outside the contract — no @command, no remedy on failure, no streaming, no testable service — so reach --help would list verbs that behave differently from the ones beside them, which is worse than two doors because the inconsistency is invisible until something fails. D-263 amended. The bash inventory, by shape: THIN WRAPPERS that mostly invoke something else (atlas-systems-done 9, atlas-names 17, generate-brands 23, atlas 24, generate-corporations 23, blender 28) — these become routers calling the same thing, nearly free. LOGIC that computes a verdict (check-fact-ids 89, validate-ron 137, godot-parse-sweep 64, godot-cold-parse 96, pr-watchlist-diff 37, atlas-update-field 59, atlas-commit-and-sync 60, tea-comment 41) — these gain the most from becoming services. ENVIRONMENT scripts (install-godot 108, install-rust 33, worktree-setup 49) — these gain least and carry the most regression risk, because downloading and unzipping a Godot build or driving rustup is awkward to exercise in a gate; port their DECISION logic into a testable service and keep the irreducible external calls behind core/process. Per-domain port tickets must state which of their sources are bash, since a bash source changes a port from mechanical to a rewrite needing its own parity evidence.', 'The mechanical bulk. Move every Python file into tooling/domains/<name>/ split router/service/schemas/helpers, normalising names on the way in. The split is DISCOVERED not invented — the domains are already encoded as filename prefixes (blender x14, atlas x8, generate x7, check x7, visual/validate/test x3, godot/garment/pql/install x2), which is the main evidence this is safe to do mechanically. Pieces that become tickets, roughly one per domain: atlas, check, validate, generate, visual, godot, wiki, db, dev. Cross-cutting pieces: (1) naming normalisation — modules snake_case, CLI verbs kebab-case, replacing the four conventions currently coexisting (assign-astro-ids.py, canvas_sources.py, check-canvas-version, blender_ prefix-as-namespace); (2) the four hyphenated directories planet-gen, economy-db, garment-fit, pql-migrate are not importable and must be renamed; (3) tooling/econ-sim is a Rust crate — excluded from package discovery, not moved; (4) the Blender carve-out — tooling/blender stays a bash wrapper and the 35 blender_*.py payload files move to tooling/scripts/blender/, executed under Blender bundled Python and never imported, with reach fronting them; (5) ruff clean under the existing config, and the E402/E702/F841 ignores revisited since the script-style sys.path.insert that forced E402 disappears with the package. Watch for: a domain that only has a router and a service does not need empty schemas.py and dependencies.py — the layering is a vocabulary, not a quota.
|
||||
|
||||
SURVEY 2026-08-31, before refining. The description above is right about the SHAPE and wrong about three counts. Re-measured:
|
||||
|
||||
RUST CRATES: THREE, not one. The description names only tooling/econ-sim. There are also tooling/line-previewer and tooling/test-client — both Cargo.toml + src/ with zero .py files. All three are excluded from the move and from package discovery, and none needs renaming, since a hyphen only matters for something Python has to import.
|
||||
|
||||
HYPHENATED PYTHON DIRECTORIES TO RENAME: FIVE, not four. economy-db (17 .py), garment-fit (23), garment-qa (1), planet-gen (30), pql-migrate (5). The description misses garment-qa. Already-importable subdirectories: db (9), wiki (1), plus the new core/ and domains/.
|
||||
|
||||
FILE COUNT: ~128 legacy .py files plus ~33 extensionless executables, not 123 total. Top level alone holds 42 .py and ~33 executables; the subdirectories hold 76 more .py.
|
||||
|
||||
WHAT THE PREFIX EVIDENCE ACTUALLY SUPPORTS — and this is the part that matters for how this epic is decomposed. Re-counted at the top level: blender 14, atlas 8, generate 7, check 7, test 5, visual 3, validate 3, install 2, godot 2. That is ~51 files with an obvious home, and the description is right that the split is discovered rather than invented for them.
|
||||
BUT there are 28 SINGLETON prefixes with no group to join: assign, canvas, clerk, convert, fill, generator, glb, inspect, migrate, patch, perf, pql, pr, process, render, schema, sculpt, setup, star, synth, tea, trellis, tune, wipe, worktree. Each needs a judgment call about which domain it belongs to, and several are genuinely ambiguous (canvas_sources.py and generator_sources.py are registries consumed by gates; schema_version.py is consumed by the DB importer; tea-comment and pr-watchlist-diff are PR workflow, which is not in the description''s domain list at all).
|
||||
|
||||
CONSEQUENCE FOR REFINEMENT: the description proposes "roughly one ticket per domain: atlas, check, validate, generate, visual, godot, wiki, db, dev". Filing those nine now would be guessing, because ~28 files have no assigned domain and at least one domain (PR/workflow) is missing from the list. THE DOMAIN MAP IS THE BLOCKING DESIGN WORK and it gets its own ticket; per-domain port tickets are filed once it exists, so their boundaries are read off the map rather than invented and then renegotiated mid-move.
|
||||
|
||||
REFINED 2026-08-31 — four cross-cutting tasks filed; the per-domain port tickets are DELIBERATELY NOT filed yet. T-1271 the domain map (blocking design — every file assigned before any file moves), T-1272 rename the five hyphenated Python directories, T-1273 the Blender carve-out, T-1274 retire the ruff ignores the package makes unnecessary. T-1272 and T-1273 are blocked on T-1271; T-1274 runs LAST, since re-enabling E402 mid-move means fixing violations in files that are about to move again. The per-domain tickets get filed from the map rather than from the epic''s proposed list, because that list is missing at least one domain (PR/workflow — tea-comment, pr-watchlist-diff) and leaves 28 singleton-prefix files unassigned. Filing nine tickets now and renegotiating their boundaries mid-move is the failure this ordering avoids.
|
||||
|
||||
SEQUENCING SET 2026-08-31 by decision. (1) T-1264 (streaming, as decorators) lands BEFORE the per-domain ports, so every ported command arrives already streaming. (2) Old scripts retire PER DOMAIN, the moment that domain''s port passes a parity test — not in one sweep at T-1253. So each per-domain port ticket owns its own retirement, and the tree shrinks continuously rather than existing twice for months while edits risk landing in the dead copy. (3) Domain names as mapped are confirmed — no renames. (4) Per D-263''s amended make/reach split, a per-domain port also RETIRES that domain''s tooling make targets rather than leaving wrappers; build/test orchestration targets stay with make.
|
||||
|
||||
WAVE 1 FILED 2026-08-31: T-1281 check (four remaining gates), T-1282 validate, T-1283 godot and visual together. Deliberately NOT filing all fourteen domains at once — the standard port acceptance is defined once on T-1281 and referenced, and the remaining tickets (atlas, starmap, planet, db, wiki, assets, character, generate, dev, pr) get written after wave 1 lands, when the real friction of a port is known rather than guessed. Four descriptions written from experience beat ten written from a map. T-1272 was also RESCOPED today: the hyphenated directories are emptied by the domain moves rather than renamed, so it becomes a closing verification instead of an opening step.
|
||||
|
||||
SCOPE CORRECTION 2026-08-31, found on starting the first port and materially larger than the epic assumed. SEVENTEEN of the 33 extensionless executables are BASH, not Python — about 900 lines. This description and the domain map both assumed a Python tree, so those are REWRITES, not moves. Decided with Jeroen: rewrite all of them in Python rather than wrap any. Wrapping would achieve one door while leaving half the CLI surface outside the contract — no @command, no remedy on failure, no streaming, no testable service — so reach --help would list verbs that behave differently from the ones beside them, which is worse than two doors because the inconsistency is invisible until something fails. D-263 amended. The bash inventory, by shape: THIN WRAPPERS that mostly invoke something else (atlas-systems-done 9, atlas-names 17, generate-brands 23, atlas 24, generate-corporations 23, blender 28) — these become routers calling the same thing, nearly free. LOGIC that computes a verdict (check-fact-ids 89, validate-ron 137, godot-parse-sweep 64, godot-cold-parse 96, pr-watchlist-diff 37, atlas-update-field 59, atlas-commit-and-sync 60, tea-comment 41) — these gain the most from becoming services. ENVIRONMENT scripts (install-godot 108, install-rust 33, worktree-setup 49) — these gain least and carry the most regression risk, because downloading and unzipping a Godot build or driving rustup is awkward to exercise in a gate; port their DECISION logic into a testable service and keep the irreducible external calls behind core/process. Per-domain port tickets must state which of their sources are bash, since a bash source changes a port from mechanical to a rewrite needing its own parity evidence.
|
||||
|
||||
GUARDED EXEC RULE, clarified by Jeroen 2026-08-31 and now the operative reading of the rewrite decision: ''for proper OS level interactions I am okay with system calls in a well guarded exec here or there, but the logic needs to go into python''. So rewrite-in-Python does NOT mean reimplementing the operating system. A guarded exec is correct for rustup, curl, unzip, git, godot, blender. What must become Python is the LOGIC — which version is wanted, whether it is already present, what the output means, what to do when it fails. THE TEST OF A CORRECT PORT is not whether it calls anything external but whether the DECISIONS can be exercised without performing them. Delivered ahead of the remaining ports, since every one of them needs it: core/process.run() is the single sanctioned exec, with four guards that each exist because a per-domain subprocess call is where they go missing — (1) an argv LIST, never a shell string, with a string rejected outright rather than helpfully split, since the helpful split is the vulnerability; (2) shell=False always; (3) a non-zero exit becomes a ReachError naming the command, carrying its output and PRESERVING its exit code; (4) a missing binary reports WHAT TO INSTALL rather than raising FileNotFoundError at someone who wanted to know the next step. All four verified against real commands. New conformance invariant enforces the single door: nothing outside core/process.py may import subprocess or call os.system/popen/exec*. Proven to fail by importing subprocess into a domain service.', NULL, '2026-08-31 15:48:10', '2026-08-31 15:48:10.137', '2026-08-31 15:48:10.137', NULL, '62fd6c01a7403e4134ec19473d8893fa', 2) ON CONFLICT(hash) DO NOTHING;
|
||||
|
||||
@@ -2438,3 +2438,27 @@ SCOPE CORRECTION 2026-08-31, found on starting the first port and materially lar
|
||||
INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G5GWCTS89VSST07DFVBVG064', 'task', '06G1S3D0M1TQW0GMFBBPQZG3ZM', 'Port the check domain — the four remaining gates', 'Finish what T-1262 started. domains/check/ currently has one verb (client-version); the other four gates join it: check-canvas-version, check-systems-db-stamp, check-fact-ids and check-dataflow-graph.py. Per the domain map, canvas_sources.py and generator_sources.py move alongside as module-level REGISTRIES, not verbs — nothing types them, and putting them in reach --help would answer a question nobody has. schema_version.py goes to the db domain where it is defined; check imports it, which is what a service layer is for. STANDARD PORT ACCEPTANCE, applying to every per-domain ticket under this epic: (a) service.py holds the logic and is transport-agnostic — no printing, no sys.exit, no typer; (b) router.py holds no logic and every command wears @command; (c) a parity test per gate, copying the pattern in tooling/test_check_parity.py, which builds a throwaway fixture repo and runs BOTH implementations — exit codes must match exactly and no fact the old message carried may be lost; (d) the old scripts are RETIRED once parity passes, not left alongside; (e) that domain''s tooling make targets are retired too rather than left as wrappers, per the D-263 make/reach split — build orchestration stays with make. Note check-dataflow-graph.py and check-canvas-version both parse git output, so their fixtures need a git repo rather than a bare directory; that is the one thing here harder than client-version was.
|
||||
|
||||
PROGRESS 2026-08-31 — fact-ids ported, the first bash-to-Python rewrite under the D-263 decision. It was 89 lines of grep/sed pipeline; it is now service.fact_ids() returning a FactIdCheck, with a router that renders it. PARITY IS EXACT ON THE LIVE TREE: both implementations print ''check-fact-ids: OK — 6 references validated against 61 canonical facts'' and exit 0. The matching COUNTS are the real evidence — if the line-matching regex differed from the grep chain even slightly, 6 and 61 would diverge. Kept line-matched rather than YAML-parsed on purpose: parsing properly would change which lines count (anchors, merge keys, multi-document files would start contributing ids the old check never saw), which is a different check wearing the same name. Three parity cases added to test_check_parity.py — ok, unknown fact_id, and the advisory mode where catalogs are unpopulated. Proven to fail: removing the entity-attributes.yaml exclusion was caught, and interestingly by the ADVISORY case rather than the exclusion assertion — including that file made the catalog non-empty, so the new implementation went to enforcing mode while the old stayed advisory. A real behavioural divergence, caught by exit code. RETIREMENT DEFERRED to the end of this ticket, and there is a reason worth recording. The rule is retire per DOMAIN, and three gates remain. It also resolves a tension: the parity test COPIES the old script into its fixture, so deleting the script early would delete the test''s own subject. A parity test is scaffolding with a defined lifetime — when the old script goes, its parity case goes with it and is replaced by a unit test of the new behaviour. Same will apply to every port under T-1250. NOTE the pre-commit hook calls tooling/check-fact-ids directly, so retirement means editing that hook — and a mistake there is felt on every commit rather than every push. Do it together with validate-content (T-1282), which is in the same hook, so it is edited once.', 'in_progress', 'high', NULL, NULL, 'D-263', '2026-08-31 15:30:35.338', '2026-08-31 15:38:42.109', NULL, '9e8af2a6085ac6f63a902ce84ce8619d', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= tickets.updated_at;
|
||||
INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06G1S3D0M1TQW0GMFBBPQZG3ZM', 'epic', '06G1S37Y1ARMV68RJT802Z3VPG', 'E3 — domains/: move the 123-file tree into the layered package', 'The mechanical bulk. Move every Python file into tooling/domains/<name>/ split router/service/schemas/helpers, normalising names on the way in. The split is DISCOVERED not invented — the domains are already encoded as filename prefixes (blender x14, atlas x8, generate x7, check x7, visual/validate/test x3, godot/garment/pql/install x2), which is the main evidence this is safe to do mechanically. Pieces that become tickets, roughly one per domain: atlas, check, validate, generate, visual, godot, wiki, db, dev. Cross-cutting pieces: (1) naming normalisation — modules snake_case, CLI verbs kebab-case, replacing the four conventions currently coexisting (assign-astro-ids.py, canvas_sources.py, check-canvas-version, blender_ prefix-as-namespace); (2) the four hyphenated directories planet-gen, economy-db, garment-fit, pql-migrate are not importable and must be renamed; (3) tooling/econ-sim is a Rust crate — excluded from package discovery, not moved; (4) the Blender carve-out — tooling/blender stays a bash wrapper and the 35 blender_*.py payload files move to tooling/scripts/blender/, executed under Blender bundled Python and never imported, with reach fronting them; (5) ruff clean under the existing config, and the E402/E702/F841 ignores revisited since the script-style sys.path.insert that forced E402 disappears with the package. Watch for: a domain that only has a router and a service does not need empty schemas.py and dependencies.py — the layering is a vocabulary, not a quota.
|
||||
|
||||
SURVEY 2026-08-31, before refining. The description above is right about the SHAPE and wrong about three counts. Re-measured:
|
||||
|
||||
RUST CRATES: THREE, not one. The description names only tooling/econ-sim. There are also tooling/line-previewer and tooling/test-client — both Cargo.toml + src/ with zero .py files. All three are excluded from the move and from package discovery, and none needs renaming, since a hyphen only matters for something Python has to import.
|
||||
|
||||
HYPHENATED PYTHON DIRECTORIES TO RENAME: FIVE, not four. economy-db (17 .py), garment-fit (23), garment-qa (1), planet-gen (30), pql-migrate (5). The description misses garment-qa. Already-importable subdirectories: db (9), wiki (1), plus the new core/ and domains/.
|
||||
|
||||
FILE COUNT: ~128 legacy .py files plus ~33 extensionless executables, not 123 total. Top level alone holds 42 .py and ~33 executables; the subdirectories hold 76 more .py.
|
||||
|
||||
WHAT THE PREFIX EVIDENCE ACTUALLY SUPPORTS — and this is the part that matters for how this epic is decomposed. Re-counted at the top level: blender 14, atlas 8, generate 7, check 7, test 5, visual 3, validate 3, install 2, godot 2. That is ~51 files with an obvious home, and the description is right that the split is discovered rather than invented for them.
|
||||
BUT there are 28 SINGLETON prefixes with no group to join: assign, canvas, clerk, convert, fill, generator, glb, inspect, migrate, patch, perf, pql, pr, process, render, schema, sculpt, setup, star, synth, tea, trellis, tune, wipe, worktree. Each needs a judgment call about which domain it belongs to, and several are genuinely ambiguous (canvas_sources.py and generator_sources.py are registries consumed by gates; schema_version.py is consumed by the DB importer; tea-comment and pr-watchlist-diff are PR workflow, which is not in the description''s domain list at all).
|
||||
|
||||
CONSEQUENCE FOR REFINEMENT: the description proposes "roughly one ticket per domain: atlas, check, validate, generate, visual, godot, wiki, db, dev". Filing those nine now would be guessing, because ~28 files have no assigned domain and at least one domain (PR/workflow) is missing from the list. THE DOMAIN MAP IS THE BLOCKING DESIGN WORK and it gets its own ticket; per-domain port tickets are filed once it exists, so their boundaries are read off the map rather than invented and then renegotiated mid-move.
|
||||
|
||||
REFINED 2026-08-31 — four cross-cutting tasks filed; the per-domain port tickets are DELIBERATELY NOT filed yet. T-1271 the domain map (blocking design — every file assigned before any file moves), T-1272 rename the five hyphenated Python directories, T-1273 the Blender carve-out, T-1274 retire the ruff ignores the package makes unnecessary. T-1272 and T-1273 are blocked on T-1271; T-1274 runs LAST, since re-enabling E402 mid-move means fixing violations in files that are about to move again. The per-domain tickets get filed from the map rather than from the epic''s proposed list, because that list is missing at least one domain (PR/workflow — tea-comment, pr-watchlist-diff) and leaves 28 singleton-prefix files unassigned. Filing nine tickets now and renegotiating their boundaries mid-move is the failure this ordering avoids.
|
||||
|
||||
SEQUENCING SET 2026-08-31 by decision. (1) T-1264 (streaming, as decorators) lands BEFORE the per-domain ports, so every ported command arrives already streaming. (2) Old scripts retire PER DOMAIN, the moment that domain''s port passes a parity test — not in one sweep at T-1253. So each per-domain port ticket owns its own retirement, and the tree shrinks continuously rather than existing twice for months while edits risk landing in the dead copy. (3) Domain names as mapped are confirmed — no renames. (4) Per D-263''s amended make/reach split, a per-domain port also RETIRES that domain''s tooling make targets rather than leaving wrappers; build/test orchestration targets stay with make.
|
||||
|
||||
WAVE 1 FILED 2026-08-31: T-1281 check (four remaining gates), T-1282 validate, T-1283 godot and visual together. Deliberately NOT filing all fourteen domains at once — the standard port acceptance is defined once on T-1281 and referenced, and the remaining tickets (atlas, starmap, planet, db, wiki, assets, character, generate, dev, pr) get written after wave 1 lands, when the real friction of a port is known rather than guessed. Four descriptions written from experience beat ten written from a map. T-1272 was also RESCOPED today: the hyphenated directories are emptied by the domain moves rather than renamed, so it becomes a closing verification instead of an opening step.
|
||||
|
||||
SCOPE CORRECTION 2026-08-31, found on starting the first port and materially larger than the epic assumed. SEVENTEEN of the 33 extensionless executables are BASH, not Python — about 900 lines. This description and the domain map both assumed a Python tree, so those are REWRITES, not moves. Decided with Jeroen: rewrite all of them in Python rather than wrap any. Wrapping would achieve one door while leaving half the CLI surface outside the contract — no @command, no remedy on failure, no streaming, no testable service — so reach --help would list verbs that behave differently from the ones beside them, which is worse than two doors because the inconsistency is invisible until something fails. D-263 amended. The bash inventory, by shape: THIN WRAPPERS that mostly invoke something else (atlas-systems-done 9, atlas-names 17, generate-brands 23, atlas 24, generate-corporations 23, blender 28) — these become routers calling the same thing, nearly free. LOGIC that computes a verdict (check-fact-ids 89, validate-ron 137, godot-parse-sweep 64, godot-cold-parse 96, pr-watchlist-diff 37, atlas-update-field 59, atlas-commit-and-sync 60, tea-comment 41) — these gain the most from becoming services. ENVIRONMENT scripts (install-godot 108, install-rust 33, worktree-setup 49) — these gain least and carry the most regression risk, because downloading and unzipping a Godot build or driving rustup is awkward to exercise in a gate; port their DECISION logic into a testable service and keep the irreducible external calls behind core/process. Per-domain port tickets must state which of their sources are bash, since a bash source changes a port from mechanical to a rewrite needing its own parity evidence.
|
||||
|
||||
GUARDED EXEC RULE, clarified by Jeroen 2026-08-31 and now the operative reading of the rewrite decision: ''for proper OS level interactions I am okay with system calls in a well guarded exec here or there, but the logic needs to go into python''. So rewrite-in-Python does NOT mean reimplementing the operating system. A guarded exec is correct for rustup, curl, unzip, git, godot, blender. What must become Python is the LOGIC — which version is wanted, whether it is already present, what the output means, what to do when it fails. THE TEST OF A CORRECT PORT is not whether it calls anything external but whether the DECISIONS can be exercised without performing them. Delivered ahead of the remaining ports, since every one of them needs it: core/process.run() is the single sanctioned exec, with four guards that each exist because a per-domain subprocess call is where they go missing — (1) an argv LIST, never a shell string, with a string rejected outright rather than helpfully split, since the helpful split is the vulnerability; (2) shell=False always; (3) a non-zero exit becomes a ReachError naming the command, carrying its output and PRESERVING its exit code; (4) a missing binary reports WHAT TO INSTALL rather than raising FileNotFoundError at someone who wanted to know the next step. All four verified against real commands. New conformance invariant enforces the single door: nothing outside core/process.py may import subprocess or call os.system/popen/exec*. Proven to fail by importing subprocess into a domain service.', 'in_progress', 'medium', NULL, NULL, 'D-263', '2026-08-20 00:23:58.880', '2026-08-31 15:48:10.137', NULL, '400f022495449b2a3a4136f8aa5ceb71', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at >= tickets.updated_at;
|
||||
|
||||
@@ -2624,6 +2624,8 @@ tooling/
|
||||
- **Why not wrap them.** Wrapping achieves one door while leaving half the surface outside the contract: no `@command`, no remedy on failure, no streaming, no testable service. `reach --help` would then list verbs that behave differently from the ones beside them, which is worse than two doors because the inconsistency is invisible until a failure.
|
||||
- **The cost is real and lands unevenly.** The grep-pipeline scripts (`check-fact-ids`, `validate-ron`, `godot-cold-parse`) compute verdicts and gain the most from becoming services — testable, with failures that teach. The environment scripts (`install-godot`, `install-rust`, `worktree-setup`) gain least and carry the most regression risk, because what they do — download and unzip a specific build, drive `rustup`, manipulate git worktrees — is awkward to exercise in a gate.
|
||||
- **So the risk is named rather than absorbed:** for those three, port the *decision* logic (which version, is it already installed, what does the tree look like) into a testable service and keep the irreducible external calls behind `core/process`. A rewrite that cannot be tested is a rewrite that has to be trusted, and trusting an installer is how a working environment becomes an unreproducible one.
|
||||
- **"Rewrite in Python" does not mean reimplementing the operating system** *(clarified 2026-08-31)*. A guarded `exec` is the right answer for genuine OS-level work — `rustup`, `curl`, `unzip`, `git`, `godot`, `blender`. **What must become Python is the LOGIC**: which version is wanted, whether it is already present, what the output means, what to do when it fails. The test of a correct port is not "does it call anything external" but **"can the decisions be exercised without performing them"**.
|
||||
**Guarded means, specifically:** an argv *list* and never a shell string, so nothing can be injected by a filename; `shell=False` always; a non-zero exit turned into a `ReachError` naming the command and carrying a remedy; and a missing binary reported as *what to install* rather than as `FileNotFoundError`. All of that belongs in one place — `core/process.run` — because a per-domain `subprocess` call is where each of those guards quietly goes missing.
|
||||
|
||||
**`make` and `reach` split by kind, not by preference** *(settled 2026-08-31)*. The Makefile has 84 targets and is today's front door, so "one CLI for all repo tooling" is not true until that relationship is stated. The boundary is **what the target actually does**, not who calls it:
|
||||
|
||||
|
||||
@@ -38,6 +38,7 @@ from pathlib import Path
|
||||
from typing import Any
|
||||
|
||||
from tooling.core import config, jobs
|
||||
from tooling.core.errors import ReachError
|
||||
|
||||
# Under .cache/, which is gitignored and already the repo's scratch space — so a
|
||||
# wrong answer about retention costs disk, never data.
|
||||
@@ -62,6 +63,69 @@ def meta_path(job_id: str) -> Path:
|
||||
return jobs_dir() / f"{job_id}.json"
|
||||
|
||||
|
||||
def run(
|
||||
argv: list[str],
|
||||
*,
|
||||
cwd: Path | None = None,
|
||||
env: dict[str, str] | None = None,
|
||||
capture: bool = True,
|
||||
check: bool = True,
|
||||
fix: str | None = None,
|
||||
missing_fix: str | None = None,
|
||||
) -> subprocess.CompletedProcess[str]:
|
||||
"""Run an external program, guarded. The one sanctioned `exec` in reach.
|
||||
|
||||
"Rewrite the bash in Python" does not mean reimplementing the operating
|
||||
system (D-263). A guarded exec is right for `git`, `godot`, `rustup`,
|
||||
`curl`; what must be Python is the *logic* around it — which version is
|
||||
wanted, whether it is already there, what the output means. The test of a
|
||||
good port is whether the decisions can be exercised without performing them.
|
||||
|
||||
Every guard lives here rather than at each call site, because a per-domain
|
||||
`subprocess.run` is exactly where one of them quietly goes missing:
|
||||
|
||||
- **An argv list, never a shell string.** `shell=False` always, so a
|
||||
filename containing a space or a semicolon is an argument and not a
|
||||
command. Passing a string here is rejected outright rather than helpfully
|
||||
split, since the helpful split is the vulnerability.
|
||||
- **A non-zero exit becomes a `ReachError`** naming the command and carrying
|
||||
a remedy — not a `CalledProcessError` traceback at someone who wanted to
|
||||
know what to do next.
|
||||
- **A missing binary reports what to install.** `FileNotFoundError` names
|
||||
the path that was not found, which is the least useful half of the answer.
|
||||
"""
|
||||
if isinstance(argv, str): # type: ignore[unreachable]
|
||||
raise ReachError(
|
||||
"process.run was given a string, not an argument list",
|
||||
fix='pass a list — ["git", "status"] — so nothing goes through a shell',
|
||||
)
|
||||
|
||||
try:
|
||||
result = subprocess.run(
|
||||
argv,
|
||||
cwd=cwd,
|
||||
env=env,
|
||||
capture_output=capture,
|
||||
text=True,
|
||||
shell=False,
|
||||
)
|
||||
except FileNotFoundError as exc:
|
||||
raise ReachError(
|
||||
f"{argv[0]} is not installed or not on PATH",
|
||||
fix=missing_fix or f"install {argv[0]}, or check PATH in a non-interactive shell",
|
||||
) from exc
|
||||
|
||||
if check and result.returncode != 0:
|
||||
detail = (result.stderr or result.stdout or "").strip()
|
||||
tail = f"\n{detail}" if detail else ""
|
||||
raise ReachError(
|
||||
f"{' '.join(argv)} exited {result.returncode}{tail}",
|
||||
fix=fix or f"run `{' '.join(argv)}` directly to see the full output",
|
||||
exit_code=result.returncode,
|
||||
)
|
||||
return result
|
||||
|
||||
|
||||
def spawn_detached(argv: list[str]) -> str:
|
||||
"""Run `reach <argv>` in a detached child. Returns the job id immediately.
|
||||
|
||||
|
||||
@@ -144,6 +144,41 @@ def check_jobs_stay_ambient(failures: list[str]) -> None:
|
||||
)
|
||||
|
||||
|
||||
def check_one_guarded_exec(failures: list[str]) -> None:
|
||||
"""Only core/process.py may touch subprocess or os.system.
|
||||
|
||||
A guarded exec is the right answer for genuine OS work (D-263) — but the
|
||||
guards only hold if there is one door. Every per-domain `subprocess.run` is
|
||||
a place where the argv-list rule, the missing-binary message, or turning a
|
||||
non-zero exit into a remedy quietly goes missing, and none of those
|
||||
omissions announces itself.
|
||||
|
||||
core/process.py is exempt because it IS the door. Test scripts are outside
|
||||
the package scope already.
|
||||
"""
|
||||
for path in _package_files():
|
||||
if path.name == "process.py" and path.parent.name == "core":
|
||||
continue
|
||||
tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path))
|
||||
if "subprocess" in _imports(tree):
|
||||
failures.append(
|
||||
f"[exec] {path.relative_to(REPO_ROOT)} imports subprocess — go through "
|
||||
"core/process.run, which is where the argv-list rule, the "
|
||||
"missing-binary message and the failure remedy live"
|
||||
)
|
||||
for node in ast.walk(tree):
|
||||
if (
|
||||
isinstance(node, ast.Call)
|
||||
and isinstance(node.func, ast.Attribute)
|
||||
and node.func.attr in {"system", "popen", "execv", "execvp"}
|
||||
):
|
||||
failures.append(
|
||||
f"[exec] {path.relative_to(REPO_ROOT)}:{node.lineno} calls "
|
||||
f"os.{node.func.attr} — unguarded, and shell-interpreting in the "
|
||||
"case of system()"
|
||||
)
|
||||
|
||||
|
||||
def check_single_output_path(failures: list[str]) -> None:
|
||||
"""(2) Nothing prints but console."""
|
||||
for path in _package_files():
|
||||
@@ -271,6 +306,7 @@ def main() -> int:
|
||||
failures: list[str] = []
|
||||
check_transport_isolation(failures)
|
||||
check_jobs_stay_ambient(failures)
|
||||
check_one_guarded_exec(failures)
|
||||
check_single_output_path(failures)
|
||||
check_commands_decorated(failures)
|
||||
check_errors_name_a_remedy(failures)
|
||||
|
||||
Reference in New Issue
Block a user