Add ranked next-action diagnostics for detect / doctor#47
Add ranked next-action diagnostics for detect / doctor#47pengfei-threemoonslab merged 5 commits intomainfrom
Conversation
…ures A coding agent that hits a common first-run failure (no shipgate.yaml, zero tools, MCP/OpenAPI artifact-only repo, dynamic toolsets, missing source file, unresolved CHANGE_ME, non-agent workspace, pure prompt experiment) now gets a ranked, structured recovery hint in JSON without having to consult human-facing docs. - New cli/diagnostics.py with NextAction / Diagnostic models, a 10-entry catalog, and pure-functional resolvers. - detect --json and doctor --json gain diagnostics[] and next_actions[] alongside the existing single-string next_action (which now projects from the rank-1 action so it stays string-typed even for stop / edit kinds). - AGENTS_SHIPGATE_AGENT_MODE=1 errors carry the same next_actions array. Audit covered all emit sites in main.py, scenario.py, and apply_patches.py. - Behavior change: doctor --json no longer raises InputParseError(3) on a required tool_sources path that doesn't resolve. It now exits 0 with unresolved_sources[] and a SHIP-DIAG-MISSING-SOURCE-FILE diagnostic. scan is unchanged — still raises 3 on the same condition. - _collect_placeholders extracted to discovery/placeholders.py and enriched with line numbers so edit actions can target shipgate.yaml:<line>. - DetectResult gains a workspace_signals block (Python file count, pyproject/requirements/prompts/tools dir hits) so the resolver can discriminate the three negative-control cases. - 41 new tests across test_diagnostics.py and test_cli.py covering model invariants, catalog stability, every resolver, precedence, cross-command consistency, and the doctor behavior change. Full suite green (463 passed). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
P1-1 — doctor's non-JSON output now surfaces unresolved required
sources and any diagnostics (id, severity, rank-1 action) and exits 3
when a required tool_sources path is unresolved. The --json contract
is unchanged: agents still get exit 0 with structured diagnostics.
P1-2 — Ruff: sort imports in diagnostics.py and test_diagnostics.py,
add strict=True to the doctor zip().
P2-1 — diagnose_missing_manifest now receives the workspace derived
from --config / --workspace, not Path.cwd(). New helper
_missing_manifest_workspace centralises the rule. An agent that runs
"agents-shipgate scan -c /tmp/repo/shipgate.yaml" from elsewhere now
gets a rank-1 detect command targeting /tmp/repo.
P2-2 — _resolve_source_paths catches the containment failure case
(declared path resolves outside base_dir) in addition to the missing-
file case. Each unresolved entry carries a `reason` field
("missing" | "outside_manifest_dir"). diagnose_doctor uses the reason
to tailor the SHIP-DIAG-MISSING-SOURCE-FILE diagnostic message.
P2-3 — diagnose_doctor edit-action paths now use str(manifest_path)
instead of manifest_path.name, so workspace and nested-manifest runs
emit "subdir/shipgate.yaml:<line>" or absolute paths instead of an
ambiguous "shipgate.yaml:<line>".
Plus 4 regression tests in test_cli.py covering each finding.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Thanks for the review — all five findings addressed in 0393fd3. [P1] Human [P1] Ruff — fixed. Imports re-sorted in [P2] Missing-manifest recovery uses caller cwd — fixed. New [P2] Missing-source detection misses containment failures — fixed. [P2] Edit actions drop manifest directories — fixed. Full local: |
P1 — distinguish missing-manifest from invalid-manifest in agent mode. ConfigError covers two failure shapes: file-not-found and exists-but-unparseable (invalid YAML, schema validation failure, unsupported version, etc.). Both used to dispatch to SHIP-DIAG-MISSING-MANIFEST whose rank-1 action is `detect / init` — which is the wrong recovery for an existing-but-invalid file (init refuses to overwrite, so the agent would loop). New SHIP-DIAG-INVALID-MANIFEST diagnostic with an `edit <path>` rank-1 action; new `_diagnose_config_error` helper dispatches by file existence in both scan and doctor handlers. P2 — POSIX-shell-quote dynamically-interpolated paths in `command` fields so a coding-agent shell runner doesn't word-split workspaces or manifest paths containing spaces. Applied to `diagnose_missing_manifest` (workspace), `diagnose_doctor`'s zero-tools re-run command (manifest path), and apply_patches' malformed_patch re-run command (--out parent). `next_actions[].command` is still a single string per the v1 contract; argv-style structured commands remain a future option. P3 — clarify the human-vs-JSON exit code split in CHANGELOG.md, docs/diagnostics.md, and AGENTS.md. The doctor behavior change is scoped narrowly: `--json` exits 0 with diagnostics (agent contract); non-JSON exits 3 (human contract); scan is unchanged regardless. Docs now spell this out and call out that diagnostics influence exit codes only on `doctor` + `MISSING-SOURCE-FILE`. Plus regression tests: - invalid-manifest dispatches to INVALID-MANIFEST (both schema-invalid and unparseable-YAML), not MISSING-MANIFEST. - workspace-with-spaces command round-trips through shlex.split(). - diagnose_invalid_manifest unit test confirms edit-action target. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Round 2 addressed in 0699069. [P1] Invalid manifests dispatched to MISSING-MANIFEST — fixed. New [P2] Path quoting — fixed. New
Regression test [P3] Stale exit-code docs — fixed. The
Updated CHANGELOG.md, docs/diagnostics.md (both the "advisory exit codes" preamble and the "Doctor behavior change" section now describe the divergence and call out that it's bounded to Verification: |
P1 — extend the missing-vs-invalid manifest dispatch to cover --workspace and glob configs. `doctor` now catches `ConfigError` separately for the discovery phase (no candidate manifests) and the per-path inspect phase (a specific discovered manifest is invalid). The inner handler dispatches with the failing path in scope, so workspace and glob runs surface SHIP-DIAG-INVALID-MANIFEST pointing at the exact file. `scan` was already correct after the v2 refactor (it uses the shared `_diagnose_config_error` dispatcher), but the dispatcher itself now walks every candidate manifest path — direct `-c <file>`, `--workspace` discovery, or glob expansion — instead of only recognising the bare `-c <file>` case. New `_candidate_manifest_paths` helper centralises the enumeration; it never raises so the agent-mode dispatch path remains panic-proof. `_missing_manifest_workspace` now falls back to `cwd` when the config is a glob, so the rank-1 detect command no longer carries literal `*` characters. P2 — `_quote_path` applied to the SHIP-DIAG-MCP-OPENAPI-ARTIFACT-ONLY rank-1 command. The detect command for an artifact-only workspace with spaces in the path now round-trips through shlex.split() like the other generated commands. Plus 4 regression tests: - doctor --workspace with invalid shipgate.yaml dispatches to INVALID-MANIFEST with the correct path - scan with a glob `*/shipgate.yaml` against an invalid file dispatches to INVALID-MANIFEST - glob with no matches falls back to cwd-based MISSING-MANIFEST, not a workspace argument containing literal `*` - artifact-only detect command for a spaced workspace shell-quotes correctly Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Round 3 addressed in 0b83fd5. [P1] Invalid manifests in The The
[P2] MCP-OPENAPI-ARTIFACT-ONLY workspace not quoted — fixed. Verification: |
P1 — split scan's CLI option-parsing into its own try/except so flag errors (`--format txt`, `--ci-mode yolo`, `--fail-on banana`, `--packet-format docx`) don't reach the manifest dispatch helper. They now emit a `kind="review"` action with guidance to fix the flag value and re-run, instead of misreporting the manifest as invalid. P2 — `_missing_manifest_workspace` now derives the longest non-glob path prefix for glob configs instead of unconditionally falling back to cwd. New `_glob_non_glob_prefix` helper walks `Path.parts` and stops at the first component containing a glob metacharacter. So `scan -c /tmp/repo/*/shipgate.yaml` from /tmp/elsewhere now routes recovery at /tmp/repo. Purely-relative globs with no leading non-glob component keep the existing cwd-fallback (there's no useful prefix to route to). Plus 4 regression tests: - bad `--fail-on` flag does not dispatch to INVALID-MANIFEST - parametrized coverage for `--format`, `--ci-mode`, `--packet-format` - absolute glob with no matches routes to the glob prefix, not cwd - relative glob with no matches still falls back to cwd Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Round 4 addressed in 80ae697. [P2] Scan option errors misreported as invalid manifests — fixed. CLI option parsing ( [P2] Absolute glob no-match recovery targets caller cwd — fixed. New Verification: |
Summary
shipgate.yaml, zero tools, MCP/OpenAPI artifact-only repo, dynamic toolsets, missing source file, unresolvedCHANGE_ME, non-agent / pure-prompt workspace, production target without permissions) gets a routable next step in JSON without having to consult docs.cli/diagnostics.pymodule:NextAction+DiagnosticPydantic models with strict validators, a 10-entry catalog under stableSHIP-DIAG-*ids, and pure-functional resolvers (diagnose_detect,diagnose_doctor,diagnose_missing_manifest,top_next_actions).detect --jsonand eachdoctor --jsonpayload now carrydiagnostics: [...]andnext_actions: [...]blocks alongside the existing single-stringnext_action. The legacy field stays string-typed for every kind by projecting from the rank-1 action (Edit <path>,Stop: <why>, etc.), so consumers that only readnext_actionkeep working.AGENTS_SHIPGATE_AGENT_MODE=1error JSON gains the samenext_actionsarray. Audited every emit site: 7 inmain.py, 2 inscenario.py, and the local helper inapply_patches.py.Type
Behavior change (deliberate, one)
agents-shipgate doctorno longer raisesInputParseError(3)when a requiredtool_sources[].pathdoes not resolve. It now exits 0 with:unresolved_sources: [{id, declared_path, line}]in the per-manifest payloadSHIP-DIAG-MISSING-SOURCE-FILEdiagnostic whose rank-1 action is aneditpointing atshipgate.yaml:<line>agents-shipgate scanis unchanged — it still raisesInputParseError(3)on the same condition. Documented inAGENTS.md,CHANGELOG.md, anddocs/diagnostics.md. A regression-guard test assertsscanstill exits 3.Notes for reviewers
report.jsonschema bump. Diagnostics are pre-scan recovery hints; per-finding remediation already lives in v0.7 fields (autofix_safe,suggested_patch_kind,docs_url).DetectResultextension is additive — newworkspace_signalsblock (Python file count, pyproject/requirements/prompts/tools dir presence). Existing fields and JSON output are unchanged._collect_placeholdersextracted tocli/discovery/placeholders.pysoinitand the newdoctordiagnostic share one implementation. Now also returnslineso edit actions can point atshipgate.yaml:<line>.PURE_PROMPT_EXPERIMENT > NON_AGENT_LIBRARY > NO_AGENT_SURFACE. Asserted in tests.NextActionvalidates kind/field correlation via amodel_validator:kind="command"requirescommand;kind="edit"requirespath;kind="stop"rejectscommand. Emptynext_actionslists are rejected at the Diagnostic level (min_length=1).Verification
CI is authoritative. Local checks run:
python -m pytest -q— 463 passed (35 new intests/test_diagnostics.py, 6 new integration tests intests/test_cli.py)scananddoctorproduce the same rank-1 next action for the missing-manifest caseRelease-readiness notes
docs/checks.md— N/A (these areSHIP-DIAG-*diagnostics, documented indocs/diagnostics.md)STABILITY.md—report.jsonschema is unchanged. TheDetectResultandinspect_sourcespayload additions are additive only.Test plan
next_action(string) stays present in every JSON output for back-compatdoctor --jsonexits 0 on unresolved required source;scanexits 3 on the same input🤖 Generated with Claude Code