|
|
|
@@ -1725,3 +1725,49 @@ First accidentally: the initial version asserted the yaml version appears on eve
|
|
|
|
|
Then deliberately: mutated the router to print a literal instead of result.godot_version. The test failed and named the dropped fact with both outputs side by side. Restored, green again.
|
|
|
|
|
|
|
|
|
|
make test-tooling: PASS end to end with both new tests wired in.', 'in_progress', 'high', NULL, NULL, 'D-263', '2026-08-20 00:36:41.192', '2026-08-31 11:45:25.451', NULL, 'ad72b0871a63801dc2597549712689f3', 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 ('06G1S6A2D22PZVQJ0S0HT91E7W', 'task', '06G1S392DEG41MRGEFSYWJWP2W', 'Reference domain: check client-version, end to end through router and service', 'The one domain ported end to end as the pattern every later port copies, so it is worth more care than its size suggests. Port tooling/check-client-version (72 lines, reads project.yaml and client/project.godot, compares the mirrored version, prints a drift message naming the remedy) into tooling/domains/check/. Deliverables: service.py holding the logic, TRANSPORT-AGNOSTIC — it returns a result and never prints, never calls sys.exit, never imports typer, so it can be called by a test or another service directly; router.py holding the Typer sub-app, which parses, delegates and formats, with ZERO logic; schemas.py holding the result shape as a STDLIB frozen dataclass, not pydantic. That last point is deliberate and is half the value of this ticket as a reference: pydantic costs 87 ms against a whole gate check of 20 ms, so domains/check/ carries no pydantic at all, and schemas.py existing while being pydantic-free demonstrates both the layering and the budget rule at once. Keep the existing tooling/check-client-version script IN PLACE and still wired to the pre-push hook — retiring old paths is E6, and the deprecation window is the point. Acceptance: reach check client-version matches the old script byte for byte on both the OK and the drift path, including the exit code.
|
|
|
|
|
|
|
|
|
|
AMENDED 2026-08-20 — the pydantic carve-out is DROPPED. The description above justifies a stdlib dataclass in schemas.py on the grounds that pydantic costs 87 ms against a 20 ms check; that reasoning came from the timing-parity budget which D-263 withdrew the same day. Pydantic is now available to every domain including check. Use pydantic in schemas.py so the reference domain is a TRUE reference — the pattern every later port copies should be the normal one, not an exception carrying a footnote. Everything else in this ticket stands unchanged, and the acceptance criterion is now the primary one: byte-for-byte output parity with the old script on both the OK and drift paths, exit code included.
|
|
|
|
|
|
|
|
|
|
RESCOPED 2026-08-25 by T-1260. This is now a HARDENING ticket, not a port. T-1260''s acceptance test needed a real domain as its subject — a stub raising NotImplementedError would have been committed dead code — so tooling/domains/check/ already exists with router.py, service.py and schemas.py, and service.client_version() is ported from tooling/check-client-version and working. reach check client-version runs green. WHAT REMAINS YOURS, unchanged: (1) convert schemas.py from the stdlib frozen dataclass to pydantic, so the reference domain shows the normal pattern rather than an exception; (2) BYTE-FOR-BYTE OUTPUT PARITY against the old script on BOTH paths — the OK path and the drift path — including the exit code, which is the acceptance criterion and has NOT been verified yet, only the OK path has been eyeballed; (3) the failure-path tests, including a deliberately drifted client/project.godot, since a gate that has only ever passed has never been tested. Note the old tooling/check-client-version script is still in place and still wired to the pre-push hook — the deprecation window is deliberate and retiring it is T-1253.
|
|
|
|
|
|
|
|
|
|
DONE 2026-08-31. All three remaining deliverables landed, and one of them changed a definition that applies well beyond this ticket.
|
|
|
|
|
|
|
|
|
|
(1) schemas.py IS NOW PYDANTIC.
|
|
|
|
|
VersionCheck is a frozen pydantic BaseModel. Frozen deliberately: a result object is a statement about what was found, and nothing downstream should be able to edit the finding on its way to being reported. Verified pydantic stays OFF the --help path — test_lazy_domains still passes, which is the assertion that pydantic loads with the domain rather than with the CLI. The reference domain is now the normal pattern with no footnote.
|
|
|
|
|
|
|
|
|
|
(2) BYTE-FOR-BYTE PARITY IS IMPOSSIBLE, AND SHOULD NOT BE ATTEMPTED. This is the real finding.
|
|
|
|
|
The acceptance criterion in the description predates the D-263 streaming amendment (2026-08-20, written after this ticket). Streaming puts the verdict on STDERR as an event; the old script writes its success line to STDOUT. Measured both: the TEXT is byte-identical in text mode — `check-client-version: OK — 0.4.13` from each — but the STREAM differs. Matching both would mean abandoning the streaming model or special-casing every ported gate, and neither is worth it.
|
|
|
|
|
Replaced with a definition that is stronger where it counts, now recorded in D-263 and carried to T-1251 because it governs every gate port in that epic:
|
|
|
|
|
(a) EXIT CODES MATCH EXACTLY — what the hook gates on, and the only part a caller can act on programmatically.
|
|
|
|
|
(b) NO FACT IS LOST — every version number, path and reason the old message carried appears in the new one. A migration that silently drops a detail makes a failure HARDER to fix than it was before, which is the opposite of the point.
|
|
|
|
|
(c) FAILURES NAME A REMEDY as a structured field, which the old scripts did only in prose.
|
|
|
|
|
|
|
|
|
|
(3) FAILURE-PATH TESTS — tooling/test_check_parity.py, wired into make test-tooling.
|
|
|
|
|
Three paths, each run through BOTH implementations and compared: ok, drift, missing-godot-file. Builds a throwaway fixture repo per case (sentinel project.yaml with commentary quoting OTHER version numbers, mirroring the real file so an unanchored regex would be caught) and copies the OLD script into fixture/tooling/ so its __file__-relative root resolves there. The new command is pointed at the same fixture with SR_REPO_ROOT. Worth noting the asymmetry that exposed: the old scripts have NO root override, which is exactly why they are hard to test and part of why the port earns its keep.
|
|
|
|
|
The test also asserts drift and missing-file actually exit NON-ZERO — without that, "the exit codes matched" would be vacuous for a pair of checks that both silently pass.
|
|
|
|
|
Invokes `reach` by BARE NAME per T-1261''s negative criterion, and fails with "Fix: make install-reach" if it is not on PATH.
|
|
|
|
|
|
|
|
|
|
PROVEN TO FAIL, twice, which is the only reason the green means anything.
|
|
|
|
|
First accidentally: the initial version asserted the yaml version appears on every failing path, but the old script does not report it when the client file is missing — so the test was wrong, not the code. Fixed by deriving expected facts from what the OLD OUTPUT ACTUALLY CONTAINS rather than a hardcoded list, which is a better statement of the rule anyway ("nothing the old message said is lost" — so the old message defines it).
|
|
|
|
|
Then deliberately: mutated the router to print a literal instead of result.godot_version. The test failed and named the dropped fact with both outputs side by side. Restored, green again.
|
|
|
|
|
|
|
|
|
|
make test-tooling: PASS end to end with both new tests wired in.', 'done', 'high', NULL, NULL, 'D-263', '2026-08-20 00:36:41.192', '2026-08-31 11:45:42.408', NULL, '03b4b4d4756c7727d38d66395de47585', 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 ('06G1S7EZ0HZSZT5HYHJ5HPBQ2G', 'task', '06G1S392DEG41MRGEFSYWJWP2W', 'Permission entry: Bash(reach *) replaces per-script prompting', 'Add Bash(reach *) to .claude/settings.json. This single line is the friction Q-124 was actually filed about — the permission gate prefix-matches whole command strings, a blanket Bash(python3 *) grant is explicitly forbidden as an unbounded write grant, so today ten separate hand-written Bash(tooling/...) entries each cover one script and every unlisted tool prompts. One bare command with subcommands is one entry covering the whole surface, which is exactly why pql is frictionless today. Scope note: ADD the reach entry here, do not remove the ten tooling entries — the old paths stay live through the deprecation window and removing their permissions early would make them prompt during E3 to E5 when they are still the working tools. The removal is E6. Verify the rule actually matches by making a real reach call through the permission gate rather than by reading the JSON, since a rule that looks right and does not match is the failure mode this whole initiative exists to fix.', 'in_progress', 'medium', NULL, NULL, 'D-263', '2026-08-20 00:41:43.428', '2026-08-31 11:48:52.107', NULL, '0a1bd26b50d16fa9a290d10fec2d61a7', 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 ('06G1S7EZ0HZSZT5HYHJ5HPBQ2G', 'task', '06G1S392DEG41MRGEFSYWJWP2W', 'Permission entry: Bash(reach *) replaces per-script prompting', 'Add Bash(reach *) to .claude/settings.json. This single line is the friction Q-124 was actually filed about — the permission gate prefix-matches whole command strings, a blanket Bash(python3 *) grant is explicitly forbidden as an unbounded write grant, so today ten separate hand-written Bash(tooling/...) entries each cover one script and every unlisted tool prompts. One bare command with subcommands is one entry covering the whole surface, which is exactly why pql is frictionless today. Scope note: ADD the reach entry here, do not remove the ten tooling entries — the old paths stay live through the deprecation window and removing their permissions early would make them prompt during E3 to E5 when they are still the working tools. The removal is E6. Verify the rule actually matches by making a real reach call through the permission gate rather than by reading the JSON, since a rule that looks right and does not match is the failure mode this whole initiative exists to fix.', 'in_progress', 'medium', NULL, NULL, 'D-263', '2026-08-20 00:41:43.428', '2026-08-31 11:48:59.985', NULL, '8923b296996cf054e2ddcdb44ec3be2a', 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 ('06G1S7EZ0HZSZT5HYHJ5HPBQ2G', 'task', '06G1S392DEG41MRGEFSYWJWP2W', 'Permission entry: Bash(reach *) replaces per-script prompting', 'Add Bash(reach *) to .claude/settings.json. This single line is the friction Q-124 was actually filed about — the permission gate prefix-matches whole command strings, a blanket Bash(python3 *) grant is explicitly forbidden as an unbounded write grant, so today ten separate hand-written Bash(tooling/...) entries each cover one script and every unlisted tool prompts. One bare command with subcommands is one entry covering the whole surface, which is exactly why pql is frictionless today. Scope note: ADD the reach entry here, do not remove the ten tooling entries — the old paths stay live through the deprecation window and removing their permissions early would make them prompt during E3 to E5 when they are still the working tools. The removal is E6. Verify the rule actually matches by making a real reach call through the permission gate rather than by reading the JSON, since a rule that looks right and does not match is the failure mode this whole initiative exists to fix.
|
|
|
|
|
|
|
|
|
|
DONE 2026-08-31, with an honest limit on the verification — read that part before trusting this.
|
|
|
|
|
|
|
|
|
|
DELIVERED — TWO entries, not one.
|
|
|
|
|
Added both Bash(reach) and Bash(reach *) to .claude/settings.json, placed beside the pql pair. The ticket asked only for Bash(reach *), but a rule ending in " *" does NOT match the bare word, and bare `reach` is a real invocation now that it prints the domain list. The established convention here confirms it: pql, make, cargo test and ruff check each carry a bare-form entry alongside the wildcard one, for exactly this reason. Adding only the wildcard would have left `reach` prompting while `reach check ...` did not — a confusing half-fix.
|
|
|
|
|
The ten tooling/ entries were left in place per the scope note; removing them is T-1253, and taking their permissions away while they are still the working tools would make them prompt through T-1250 to T-1252.
|
|
|
|
|
|
|
|
|
|
VERIFICATION — WHAT WAS AND WAS NOT PROVEN. This matters given the ticket''s own warning.
|
|
|
|
|
Ran real calls through the gate: `reach check client-version` (exit 0) and `reach --help` (domain list). Both succeeded with no prompt.
|
|
|
|
|
BUT THAT IS NOT EVIDENCE THE RULE WORKED. Those same calls succeeded throughout this session BEFORE the rule existed — I checked, and there was no Bash(reach ...) entry in either .claude/settings.json or .claude/settings.local.json, and no blanket Bash(*) grant in either. So something else (session permission mode) was already allowing them, and the observation is confounded. A passing call here cannot distinguish "the rule matched" from "the rule was never consulted".
|
|
|
|
|
WHAT WOULD ACTUALLY PROVE IT: a session started AFTER this commit, in a mode that prompts, where `reach check client-version` runs without a prompt. settings.json is read at session start, so this cannot be self-verified from inside the session that wrote it. Flagging rather than claiming a green.
|
|
|
|
|
What IS established: the JSON parses, both entries are present in permissions.allow, and the rule shape is byte-identical in form to the pql pair, which is empirically frictionless in this repo. That is inference from a working precedent, not proof.
|
|
|
|
|
|
|
|
|
|
FINDING — env-prefixed calls will still prompt, and that is accepted.
|
|
|
|
|
Permission rules prefix-match the WHOLE command string (root CLAUDE.md: `PATH=/opt/bin make build` does not match `Bash(make *)`). So `SR_REPO_ROOT=... reach ...` and `SR_OUTPUT_FORMAT=text reach ...` do not match Bash(reach *) and will prompt. Deliberately not worked around: an environment override IS a genuine departure from normal invocation, and it is the ordinary form that needs to be frictionless. Recorded in docs/DEVOPS.md with the mitigation for tests — pass overrides through the subprocess environment rather than the command string, as tooling/test_check_parity.py does.', 'in_progress', 'medium', NULL, NULL, 'D-263', '2026-08-20 00:41:43.428', '2026-08-31 11:52:32.631', NULL, '0f1abd699ed3f113681cd62d89bef251', 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;
|
|
|
|
|
Claude Opus 5