spec-kitty / spec-kitty/spec-kitty

[squad] MINOR findings from PR#4172 (kernel.clock door swap + pre-install bootstrap)

Open
#4,176 0 comments 0 reactions 0 assignees View on GitHub
from:squad priority:P3 type:finding
Dominant language
Python
Stars
1.6k
Forks
165
Avg merge
14h 52m
Merged PRs (30d)
303

Description

Consolidated MINOR findings from the squad pass 2 review of **spec-kitty/spec-kitty#4172** (`fix(ci): import datetime via the kernel.clock single door (FR-012(a))`, head `da9135e9b9435aad33252fac1bc5ad90d607ccc4`).

PR: https://github.com/spec-kitty/spec-kitty/pull/4172
Squad verdict: https://github.com/spec-kitty/spec-kitty/pull/4172#issuecomment-5607864742 (`[squad] passed`, no MAJOR findings)

**The first item below references code that does not exist on `main` until #4172 merges.** The remaining two are PR-body evidence defects — actionable only as a body edit on that PR before merge, or as a process note afterwards; there is nothing to change in the tree for them.

Per PROGRAM.md §7 these are non-blocking: they did not hold the merge of a `priority:P0` red-main fix.

- [ ] `scripts/ci/select_source_artifacts.py:14` — unconditional `sys.path.insert(0, str(Path(__file__).resolve().parents[2] / "src"))` diverges from both in-repo bootstrap precedents, which guard for idempotency: `scripts/docs/glossary_linker.py:73` and `tests/architectural/surface_resolution_audit/audit.py:83` both use `if str(_REPO_ROOT) not in sys.path:`. `tests/ci/test_aggregate_attempts.py:174` imports this module in-process, so the entry is prepended as a session-wide side effect of any suite run that touches it, duplicating a path the editable install already supplies. Verified harmless today (`uv run --frozen pytest tests/ci/` → 203 passed at that head). Recommendation: wrap the insert in the same `not in sys.path` guard, and annotate the bare `# noqa: E402` with its reason as the precedents do (`# noqa: E402 (sys.path bootstrap above)`).
- [ ] PR#4172 body, `## Blast radius` — the `Discovery:` line records `gh run view 34385207968 --log-failed | grep -E 'test_clock|E '`, which structurally cannot produce the second path in its own `Files:` list: it is a CI-log grep that named only `scripts/ci/select_source_artifacts.py:8`, never `tests/ci/test_aggregate_attempts.py`. It is also not re-runnable by a reviewer on an exe.dev VM, because `gh run view` is not one of the `github.int.exe.xyz` proxy's supported paths (`bin/GH-API.md`). This is the planning#1188 shape (a `Files:` list not re-derived from its own stated discovery command). The union itself is correct — re-derived independently with the PROGRAM.md §6 recipe (`grep -rln` for both changed paths and their basename stems over `tests/`) it is exactly `tests/ci/test_aggregate_attempts.py`, which the PR listed and ran. Recommendation: record the §6 file-discovery command as `Discovery:` and keep the CI-log grep as supporting root-cause evidence elsewhere in the body.
- [ ] PR#4172 body, `## Tests run` — records file paths plus counts (`tests/ci/test_aggregate_attempts.py 28/28`, `tests/architectural/test_clock_import_ban.py 10/10`) instead of PROGRAM.md §5's `- → /` form, so a reviewer must infer the invocation before re-running it. Both counts are honest — re-ran them and got 28 passed and 10 passed exactly. `bin/validate-pr-body.py --labels type:fix` accepts the body as-is (exit 0), so this is §5's letter rather than a seam failure. Recommendation: restate as the literal commands with their counts.

Filed as one issue per PR per planning#1313, not one per finding.

Contributor guide

Open the contributing guide

Research direction

Read scripts/ci/select_source_artifacts.py alongside scripts/docs/glossary_linker.py, tests/architectural/surface_resolution_audit/audit.py, and tests/ci/test_aggregate_attempts.py. Run uv run --frozen pytest tests/ci/ and inspect PR#4172's Blast radius and Tests run sections against PROGRAM.md §§5–6. Done means the bootstrap evidence and PR-body commands match the stated precedents and validation format.

Written by the indexing model from the issue text.

Assessment

Tech stack
github-actions, python
Domain
ci-cd, tooling
Issue type
Refactor
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.