feat: scaffold snake-eyes JSON-RPC server with initialize/shutdown lifecycle - #7
Conversation
…fecycle - Add package scaffold (pyproject.toml, src layout, NOTICE attribution) - Implement protocol types with stdlib dataclasses and JSON-RPC 2.0 error codes - Implement line-delimited stdio server loop with injectable streams and dispatch - Add CI workflow (ruff, mypy, pytest with 85% coverage gate) - Add tests driving through injected stdin/stdout (100% coverage) Assisted-by: deepseek-v4-pro Generated with AI assistance (deepseek-v4-pro)
jflowers
left a comment
jflowers
left a comment
There was a problem hiding this comment.
Council Verdict: APPROVE
Reviewers: adversary, architect, curator, envoy, guard, herald, scribe, sre, testing
Iterations: 2
All nine reviewers APPROVE after one fix pass (bool JSON-RPC id, README/AGENTS current-state, initialize/config tests, to_dict wording, checkout tag comment).
5 LOW findings omitted.
Note: Could not post as APPROVE due to insufficient permissions. Posted as COMMENT instead. Original verdict: APPROVE.
This review was generated by /review-council (AI-assisted).
jflowers
left a comment
jflowers
left a comment
There was a problem hiding this comment.
Note: Could not post as APPROVE — GitHub prohibits self-review (PR author). Posted as COMMENT instead. Original verdict: APPROVE.
Council Verdict: APPROVE
Reviewers: Adversary, Architect, Guard, Tester, SRE, Curator, Envoy, Herald, Scribe
Iterations: 2 (hardening fix round + spec doc-sync round)
All 9 reviewers approve. All CRITICAL/HIGH/MEDIUM findings resolved in the fix loop; two findings retracted as false positives on git evidence. Gates: ruff + mypy strict + pytest — 69 tests, 100% statement+branch coverage (85% gate untouched).
Adversary (APPROVE)
- [MEDIUM] Stdin parse path unhardened (RecursionError escape, no size cap) → FIXED (-32700/-32600 + stay-alive regression tests)
- 5 LOW fixed or deferred to issues #3-#6 with rationale
Architect (APPROVE — Alignment 92→96/100)
- [MEDIUM] BrokenPipeError teardown could exit 120 on Python 3.12+ → FIXED (OSError catch + devnull redirect)
- 4 LOW fixed (helper extraction, conftest.py, annotations, docstrings)
Guard (APPROVE)
- [MEDIUM] Constitution citations — RETRACTED (false positive; .specify/memory/constitution.md tracked on main)
- Full spec→code→test traceability; zero scope creep; gates intact
Tester (APPROVE)
- [HIGH] Transport liveness unverifiable with StringIO fixtures → FIXED (flush-spy exact-count test; shutdown+trailing-request test)
- [MEDIUM] No process-boundary test → FIXED (two subprocess smoke tests)
- 4 LOW fixed (branch coverage enabled — gate value not lowered)
SRE (APPROVE)
- [MEDIUM] Locale-dependent stdio encoding → FIXED (UTF-8 reconfigure + UnicodeDecodeError→-32700)
- [MEDIUM] Version dual source of truth → FIXED (dynamic version + drift-pinning test)
- 9 LOW fixed/verified (CI hardening, NOTICE-in-wheel via uv build) or deferred
Curator (APPROVE)
- [MEDIUM] Public-seam docstrings underdocumented → FIXED (dispatch-replacement seam, exit codes, field semantics)
Envoy (APPROVE) — 3 LOW (1 retracted artifact, 2 deferred)
Herald (APPROVE)
- [MEDIUM] NOTICE present-tense attribution ahead of the gaze-py lift → DEFERRED TO HUMAN (see below)
- 5 LOW fixed or deferred
Scribe (APPROVE)
- [CRITICAL] readme/LICENSE missing — RETRACTED (false positives; both tracked on main at 204ec28)
- 4 LOW fixed (AGENTS.md tree, spec wording)
Round-2 residuals — resolved
- [MEDIUM] Spec text lagged hardened implementation → FIXED (server/cli spec amendments, design decision 9, tasks.md metadata)
Open item for human decision (non-blocking)
- [MEDIUM] NOTICE:4-6 — present-tense "includes software originally developed in gaze-py" while the lift is a scheduled follow-up. (a) Reword to future intent (amend NOTICE + spec + test together), or (b) keep and disclose in 0.1.0 release notes. Herald recommends (b).
This review was generated by /review-council (AI-assisted).
| snake-eyes | ||
| Copyright 2026 zero-dot-force | ||
|
|
||
| This product includes software originally developed in gaze-py |
There was a problem hiding this comment.
[MEDIUM] Present-tense "includes" asserts gaze-py code ships in this change; the lift is a scheduled follow-up (design.md Non-Goal). Council deferred to human: (a) reword to future intent (amend NOTICE + spec + test together), or (b) keep and disclose in 0.1.0 release notes. (Herald/Scribe)
jflowers
left a comment
jflowers
left a comment
There was a problem hiding this comment.
Council Verdict: APPROVE
Note: Could not post as APPROVE — GitHub prohibits self-review (PR author). Posted as COMMENT instead. Original verdict: APPROVE.
Reviewers: Adversary, Architect, Guard, Tester, SRE, Curator, Envoy, Herald, Scribe
Iterations: 1 (unanimous APPROVE on first pass)
Adversary (APPROVE)
No blocking findings. 3 LOW: internal error message leaks exception string (benign — trusted subprocess), _devnull_stdout bare pass needs comment, ubuntu-latest mutable runner.
Architect (APPROVE — Alignment 9/10)
No blocking findings. 4 LOW: .gitignore boilerplate, conftest.py missing from manifest, test stub docstring inconsistency, conftest.py bare module import.
Guard (APPROVE)
Full spec→code→test traceability verified. Zero scope creep. All governance gates intact (85% coverage, mypy strict, ruff). 2 LOW: .gitignore boilerplate, CI naming convention.
Tester (APPROVE)
2 MEDIUM: to_dict scalar passthrough not directly tested (see inline), subprocess timeout messaging. 2 LOW: conftest.py id shadows builtin, test_initialize_roundtrip relies on helper correctness.
SRE (APPROVE)
4 LOW: dev deps unpinned (lockfile protects), ubuntu-latest mutable runner, no Dependabot/Renovate, README missing troubleshooting.
Curator (APPROVE)
1 MEDIUM: platform pattern blog opportunity — recommend filing when analysis capabilities land.
Envoy (APPROVE)
3 LOW omitted.
Herald (APPROVE)
3 MEDIUM: notification semantics (see inline), root_path not validated as absolute (see inline), CI action version comment drift. 5 LOW omitted.
Scribe (APPROVE)
4 LOW omitted.
Summary
| Severity | Count |
|---|---|
| CRITICAL | 0 |
| HIGH | 0 |
| MEDIUM | 5 |
| LOW | 22 |
22 LOW findings omitted.
All findings are non-blocking. This is a clean, well-structured scaffold with strong protocol fidelity, comprehensive test coverage (69 tests, 100% coverage, 85% gate preserved), and proper CI hardening.
This review was generated by /review-council (AI-assisted).
|
|
||
| def test_to_dict_handles_list_values() -> None: | ||
| request = JsonRpcRequest("2.0", 1, "initialize", {"items": [1, 2]}) | ||
| assert to_dict(request)["params"]["items"] == [1, 2] |
There was a problem hiding this comment.
[MEDIUM] Missing direct test for to_dict scalar passthrough. The scalar branch (return obj) is exercised transitively through dataclass field serialization but has no dedicated test. Consider adding:
def test_to_dict_scalar_passthrough() -> None:
assert to_dict(42) == 42
assert to_dict("hello") == "hello"
assert to_dict(None) is None(Testing)
| request_id = data.get("id") | ||
| if isinstance(request_id, (int, str)): | ||
| return request_id | ||
| return None |
There was a problem hiding this comment.
[MEDIUM] _extract_id returns None for absent id, causing the server to respond to all requests including JSON-RPC 2.0 notifications (§4.1 says notifications with no id should not receive responses). Acceptable simplification since Gaze never sends notifications — worth noting for future protocol compliance. (Herald)
| if not isinstance(params, dict): | ||
| raise RpcError(INVALID_PARAMS, "Invalid params") | ||
| root_path = params.get("root_path") | ||
| if not isinstance(root_path, str): |
- Add oversize line rejection (16 MiB cap) with stay-alive behavior - Add UnicodeDecodeError handling at readline level (-32700) - Add RecursionError guard for deeply nested JSON payloads - Add bool-is-int guard in _extract_id (Python bool subclasses int) - Add broken pipe / OSError teardown with devnull stdout redirect - Add method name truncation (64 chars) in -32601 responses - Add UTF-8 stream reconfiguration in CLI entry point - Add conftest.py with shared req()/responses() test helpers - Add subprocess smoke tests for initialize/shutdown roundtrip - Improve error messages with specific field names - Enhance docstrings across protocol types and server - Amend spec artifacts to reflect hardened implementation - Update CI: concurrency group, persist-credentials, timeout - Consolidate coverage config; single-source version via hatch
PR Cost Report: #7Session: Propose new change artifactsID:
Timeline: 2026-08-25 17:02:24 — 2026-08-27 13:03:42 Child sessions: 56 — additional cost: $14.39 Child session breakdown
Session: Python project review - skip gaze testingID:
Timeline: 2026-08-27 13:04:33 — 2026-08-27 14:17:20 Child sessions: 9 — additional cost: $11.79 Child session breakdown
Session: Review council: skip gaze for Python projectID:
Timeline: 2026-08-27 15:20:01 — 2026-08-27 16:51:01 Child sessions: 9 — additional cost: $7.67 Child session breakdown
Grand Total: $54.82 across 3 session tree(s) (77 sessions) |
1 participant
Summary
snake-eyes is the Python backend for the Gaze analyzer, but it had no Python source, package metadata, tests, or CI — it could not boot. This change delivers the foundation: the package scaffold, JSON-RPC 2.0 protocol types, a line-delimited stdio server loop implementing the
initialize/shutdownlifecycle, CI, and tests — matching Gaze's analyzer protocol v1.1.0 exactly.pyproject.toml(stdlib-only runtime,src/layout,snake-eyesentry point),__init__.py(__version__ = "0.1.0"),__main__.py(--stdioonly), andNOTICE.-32700…-32603),initializeresult schema (4 capability flags allfalse), anddata-omission-when-Noneserialization.initialize/shutdown), sequential processing, clean teardown on shutdown/EOF/broken-pipe.permissions: contents: read, ruff + mypy + pytest with 85% coverage gate.Out of scope (later issues): analysis, complexity, coverage, discovery, streaming, gaze-py source lift, and astroid/radon/coverage.py runtime deps.
How to Test
Manual lifecycle check:
How to Demo
Run
uv run snake-eyes --stdioand observe it waits on stdin. Send aninitializerequest and confirm the response carriesanalyzer_name,language,language_version,protocol_version: "1.1.0", and fourfalsecapability flags. Sendshutdownand observe the{}result and clean exit 0. Runninguv run snake-eyeswithout--stdioprintssnake-eyes --stdioto stderr and exits 2.Key Files Changed
src/snake_eyes/protocol.py— JSON-RPC envelope dataclasses, error codes,initialize/shutdownresults, serialization.src/snake_eyes/server.py— line-delimited stdio loop with injectable streams/dispatch and full error taxonomy.src/snake_eyes/__main__.py—--stdio-only CLI entry point.src/snake_eyes/__init__.py— package version.pyproject.toml— build config, stdlib-only runtime, dev deps, ruff/mypy/pytest config..github/workflows/ci.yml— 4-gate CI (ruff, mypy, pytest with 85% coverage).tests/test_{protocol,server,cli}.py— 42 tests driving through injected stdin/stdout (100% coverage).NOTICE— gaze-py attribution.openspec/changes/scaffold-and-protocol/— proposal, design, specs, tasks.Known Issues
The following review-council findings (all LOW, non-blocking) were acknowledged but not resolved:
_extract_idtreats JSON booleans as valid ids (Pythonboolsubclassesint);{"id": true}is echoed rather than rejected.BrokenPipeErrorteardown may emit "Exception ignored" noise on interpreter shutdown.requires-python = ">=3.11"is unbounded while CI only verifies 3.11/3.12; build backendhatchlingis unpinned.This PR was generated by /uf.finale (AI-assisted).