Skip to content

fix(doctor): close review findings F1/F5/F6/F2/F7 — evidence, ledgers, YAML, verify surface, file targets - #27

Merged
SollanSystems merged 5 commits into
mainfrom
fix/doctor-hardening
Jul 5, 2026
Merged

SollanSystems merged 5 commits into
mainfrom
fix/doctor-hardening

Conversation

@SollanSystems

Copy link
Copy Markdown
Owner

Summary

Closes the doctor-side enforcement gaps confirmed from the external review (findings F1, F5, F6, F2, F7). Every fix was pinned with a failing test reproducing the exploit first, then verified adversarially against the original repros plus variants in both validation modes (structural-fallback and jsonschema).

  • F1 (roadmap G1/QW2): a Succeeded terminal with empty evidence[] now fails doctor in both modes (contradictory_terminal), mirroring the write-time G1 check in loop/emit.py. The one touched fixture previously encoded the exploit (asserted an evidence-free Succeeded validated clean).
  • F5a: doctor validates only the canonical rollout ledger (.loop/rollout.jsonl) as rollout@1; a foreign .loop/notes.jsonl no longer false-FAILs a healthy contract. rollout enters schemas_checked only when actually validated.
  • F5b: jsonl ledgers strict-decode; invalid UTF-8 fails closed with invalid_encoding instead of silently passing under errors="ignore".
  • F6: the fallback YAML parser preserves # inside quoted scalars (goal: "reach #1") and now agrees with the PyYAML path.
  • F2: doctor flags a missing verification surface (missing_verify_surface) and a path-shaped task.verify that doesn't resolve (unresolved_task_verify). Plain commands (pytest -q) are not path-checked. Fresh-scaffold-doctor-clean invariant preserved (stub-marker substance grading stays in the inspector, per the triage correction).
  • F7: a file target (loop doctor .loop/state.json) resolves to its owning workspace instead of producing garbage paths.

Test plan

  • fallback mode: 265 passed, 11 skipped (+16 new; zero regressions)
  • jsonschema mode: 270 passed, 6 skipped
  • fresh scaffold doctor-clean; examples/coverage-repair doctor-clean (both modes)
  • independent adversarial verifier: 6/6 original exploits now caught, diff reviewed for weakened gates (none)

🤖 Generated with Claude Code

SollanSystems and others added 5 commits July 5, 2026 18:28
_check_terminal_contradiction checked false_completion and criteria_met but
never evidence, so a schema-valid Succeeded terminal with an empty evidence[]
passed doctor in both validation modes — a success claim with nothing behind
it. Mirror the write-time refusal in loop/emit.py: a Succeeded terminal now
requires a non-empty evidence list, emitting a contradictory_terminal issue
otherwise.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ode jsonl

F5a: _validate_optional_records globbed every .loop/*.jsonl and validated each
as a rollout@1 record, so a foreign notes.jsonl false-FAILed an otherwise
healthy contract. Validate only the canonical rollout ledger (rollout.jsonl)
and skip unknown jsonl; "rollout" enters schemas_checked only when that ledger
was actually validated.

F5b: _validate_jsonl read with errors="ignore", so a ledger line carrying raw
0xff bytes decoded lossily and silently validated clean (a false PASS). Decode
strictly and fail the file closed with an invalid_encoding issue on
UnicodeDecodeError.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ments

_fallback_yaml did raw.split("#", 1)[0] first, so goal: "reach #1" truncated to
'"reach' — a '#' inside a quoted scalar was treated as a comment. Add
_strip_comment, which cuts a trailing comment only when the '#' is unquoted and
(YAML-style) at line start or after whitespace, so a quoted '#' survives and the
fallback agrees with PyYAML.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…lves

doctor never noticed a loop with no verification surface: deleting every
scripts/verify-* left it green, and a task whose verify pointed at a
nonexistent script was never existence-checked. Add _check_verify_surface,
run in both modes: flag missing_verify_surface when no verify-* script exists
and no task declares a verify command, and unresolved_task_verify when a task
verify's first token is path-shaped but does not resolve under the workspace.
Plain commands like pytest -q are not path-checked. _check_stub_verify_scripts
is unchanged; its docstring now states the stub markers are an opt-in convention.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
_workspace_from only handled directory targets, so `loop doctor .loop/state.json`
or a root-level TASKS.json resolved the workspace to the file itself and every
path underneath it was garbage (a wall of missing_file issues). When the target
is an existing file, resolve from its parent directory: .loop/state.json now
resolves to the owning workspace, and a root-level TASKS.json to the workspace
root — matching the directory-target report.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 5, 2026 22:54
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@SollanSystems
SollanSystems merged commit e190d3f into main Jul 5, 2026
6 checks passed
@SollanSystems
SollanSystems deleted the fix/doctor-hardening branch July 5, 2026 22:55
SollanSystems added a commit that referenced this pull request Jul 8, 2026
* feat(st2): the contract is a versioned, tool-agnostic standard

Promote reference/repo-os-contract.md to the normative spec: stability
note and versioning model (§0), artifact/schema table across all 7
published schemas with required keys verbatim from schemas/ (§11),
lifecycle vocabulary + terminal-file-iff rule (§12), repair-record vs
rollout-record two-shape clarification (§13), and the A1-E1 conformance
checklist (§14).

- doctor lifecycle line: validate_contract reports
  lifecycle: planned | running | terminated:<State> | unknown —
  additive reporting only, never an issue source; DG-3 regressions pin
  both directions in both validation modes.
- scripts/test_template_roundtrip.py: every templates/* artifact,
  filled with schema-valid values, passes validate_contract with zero
  issues in both modes (in-flight + terminated scaffolds).
- scripts/test_conformance.py: executes checklist A1-E1 in CI against
  examples/coverage-repair and a fresh template scaffold, incl.
  additive-key tolerance (D2), lifecycle honesty (E1), and a doc-parity
  guard binding every checklist ID to the normative doc.
- README: pointer subsection for the versioned standard.

ST2 spec: docs/superpowers/specs/2026-06-30-st2-portable-contract-spec.md
(DG-1/DG-2/DG-3-core/M5/QW11 had already landed via #27-#30).
Suite: 372 passed / 10 skipped (jsonschema), 361 / 21 (structural).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* chore(release): cut 0.7.0 — portable standard

CHANGELOG 0.7.0 collects the ST2 standard work, the adoption slices
(A1/B1/C1/PR5, previously Unreleased), and the external-review patch
set #27-#30 (previously unchangelogged). Version 0.6.1 -> 0.7.0 in
pyproject.toml, plugin.json, README badge + Status; docs-version gate
updated. The README Adopt-section @v0.7.0 action pin becomes accurate
at this release's tag.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants