Skip to content

feat(api): expose evidence_verify as an additive op (unblocks dsh-sessionflow#9 / Core #22) - #24

Draft
SunnyRad wants to merge 1 commit into
HarryHeYu:mainfrom
SunnyRad:feat/evidence-verify-op
Draft

SunnyRad wants to merge 1 commit into
HarryHeYu:mainfrom
SunnyRad:feat/evidence-verify-op

Conversation

@SunnyRad

Copy link
Copy Markdown
Contributor

What

Add one additive evidence_verify op to voyager/api.py _OPS — a thin,
read-only pass-through to the existing evidence.verify() — plus a dedicated
bridge-contract test (tests/test_evidence_verify_op.py). Two files changed,
no existing line modified.

Closes/advances sessionFlow#22
and is the Core half of
dsh-sessionflow#9
(Option A, confirmed by the maintainer on 2026-11-12).

Why

voyager evidence verify --json already returns the five verdicts
MATCH / MISMATCH / NOT_FOUND / UNVERIFIABLE / INVALID_REFERENCE,
but it is CLI-only (docs/MAINTENANCE.md known-issue #6). The plugin's
runJson() throws CLI_FAILED on any non-zero process exit code, while
voyager/evidence.py:39-45 uses exit codes 1–4 for four normal verdicts,
so a direct CLI call (Option B) would discard them. callOp() reads a single
JSON line from voyager api and returns payload.result, so the verdict lives
in result.status, decoupled from the process exit code. A machine op is the
only way to keep all five states over the bridge without a second transport.

Scope — strictly additive (no expansion)

  • api.py: +1 import (from .evidence import verify), +1 function, +1 _OPS
    entry. BRIDGE_SCHEMA_VERSION stays 1; integration_info() derives
    ops/capabilities from _OPS, so the new op is discoverable by name.
  • verify() / EXIT_CODES / the five-state semantics / FINGERPRINT_ALGORITHM
    (sha256-8hex-json-v1) / CLI behaviour / resolveVoyager() /
    REQUIRED_CORE_REF / MCP / peer range / CI matrices: untouched.
  • No docs/*.md changes ⇒ no docs/INDEX.md registration needed.
  • MATCH remains "this reference is consistent", never "the claim is true".
  • An older Core without the op answers unknown op: 'evidence_verify' (ops: …)
    — a capability error mapped by the plugin to OP_ERROR, never confused with
    evidence NOT_FOUND and never degraded to MATCH.

Verification (level: UNIT_TESTED)

  • pytest tests/test_evidence_verify_op.py -q → 8 passed. Asserts all five
    states round-trip by name through handle_request (incl.
    INVALID_REFERENCE on malformed seq / non-8-hex digest, and
    UNVERIFIABLE with no digest), the {id, result} envelope, the verbatim
    documented result shape, MATCH ≠ "claim is true", schema stays 1,
    integration_info().ops advertises the op, old-Core unknown op
    degradation, and the real stdio voyager api surface (one JSON line).
    Fixtures follow the real voyager evidence verify --json shape (same seeded
    index the CLI tests use); no invented response fields.
  • Mutation check (4 cases: unregister op / force MATCH / drop arg / comment-only)
    → bad=0, byte-identical restore. The new test has teeth.
  • Full Core suite pytest tests/ -q --tb=short -rf:
    3 failed, 1382 passed, 8 skipped (1393 collected). The 3 failures are
    pre-existing / environment artifacts, not introduced here:
    • test_cli.py::test_version_reports_the_installed_distribution and
      ::test_dunder_version_agrees_with_the_distribution —
      importlib.metadata.PackageNotFoundError: No package metadata was found for voyager
      (the PR is built from a source tarball, not an installed dist; reproduced
      identically with the pristine api.py).
    • test_verification_harness_v2.py::test_a_successful_run_records_evidence —
      order-dependent/flaky under the full run (GBK decode warnings from a
      subprocess reader thread); passes 3/3 in isolation with this change.
      These are the same release-engineering/CI-environment issues the maintainer
      noted in the #9 division-of-labor comment
      (release workflow permission + no fixed deadline); they do not block a
      feature-branch Draft PR.
  • scripts/check_consistency.py --strict --online: 3 checks fail on
    v1.3.1 not a visible git tag (the not-yet-pushed release tag), all in
    README/version files not touched by this PR; the
    bridge schema agreement check is SKIP (no plugin root here). Same class of
    pre-existing release-environment issue.
  • No INTEGRATION_TESTED / LIVE_VERIFIED claim: those need an installable
    Core version over the real bridge, which depends on the maintainer's
    release-workflow fix.

Merge order (cross-repo, CONTRIBUTING.md)

  1. This Core op first — review + CI.
  2. An installable Core version is formed (maintainer-side release workflow).
  3. Then the plugin PR sessionflow_evidence_verify calls callOp('evidence_verify', …);
    formal acceptance gated on a real-core integration test. Plugin Draft PR /
    stub contract tests may be prepared in parallel, marked blocked.

This PR is opened as a Draft and is blocked on step 2 (installable Core
version) for formal acceptance, exactly as Core #22 / the maintainer's comment
allow. Keep dsh-sessionflow#9
as the design record; no fixed delivery time.

Author: SunnyRad (same as #7 / prior PRs).

Add one thin, read-only pass-through to evidence.verify() so the
plugin's callOp() keeps all five verdicts in result.status instead
of losing four normal non-zero states to runJson()/CLI_FAILED.
BRIDGE_SCHEMA_VERSION stays 1; integration_info() derives ops from
_OPS. No change to verify(), EXIT_CODES, the CLI or the fingerprint.
Add tests/test_evidence_verify_op.py (8 states/contract checks).
Closes Core HarryHeYu#22; Core half of dsh-sessionflow#9 (Option A).
@HarryHeYu

Copy link
Copy Markdown
Owner

Confirmed — you already have this covered, so I am not asking you to claim anything: PR #24 (feat/evidence-verify-op) is exactly the Core half of #22, two files (voyager/api.py, tests/test_evidence_verify_op.py), and its full CI matrix is green.

Status I see: OPEN, draft, MERGEABLE / CLEAN, head 856a09e. Nothing from my side touches those files, so there is no conflict with the OpenCode work.

I will review it on the merits (thin wrapper only, five states reachable by name, schema_version stays 1, integration_info().ops gains the name, and unknown op degradation) and leave the merge decision explicit. Keeping it draft until that review is the right call.

Two notes so we do not duplicate effort:

No fixed deadline, and no need to push further commits for me — I will comment on the PR itself with the review.

@HarryHeYu HarryHeYu left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Review — looks correct; verified by running it

I fetched refs/pull/24/head (856a09e) into an isolated worktree and ran the new test file:

python -m pytest tests/test_evidence_verify_op.py -q
=> 8 passed in 6.13s

What I checked against the agreed boundary:

  1. Thin wrapper only. evidence_verify() opens a read-only Store and returns evidence.verify(...) verbatim — no second implementation of the five states, no reinterpretation of message, no local digest logic. verify() and EXIT_CODES are untouched.
  2. Verdict is a field, not an exit code. result.status carries the five names and result.exit_code rides along as data, which is exactly why a callOp() client keeps MISMATCH / NOT_FOUND / UNVERIFIABLE / INVALID_REFERENCE instead of CLI_FAILED.
  3. Schema stays 1. BRIDGE_SCHEMA_VERSION is unchanged and the op is registered in _OPS, so integration_info() advertises it — feature detection by op name, no version sniffing.
  4. Old-Core degradation is a capability error. unknown op: ... is asserted as such, and the test explicitly keeps it distinct from an evidence NOT_FOUND — that is the distinction I care about most here.
  5. No scope creep. Two files, no existing line modified, no MCP tool, no CLI change, no REQUIRED_CORE_REF bump.

One note, not a blocker: evidence_verify(db=None, session_id="", seq=None, digest=None) will happily take session_id="", and verify() will answer INVALID_REFERENCE for it. That is the honest outcome for a malformed reference, so I am fine leaving argument validation to the plugin layer (which should refuse before spawning). Just flagging that the op itself does not pre-validate.

No change requested. Keeping it draft is right; the merge decision and the "installable Core" step are mine to sequence, and I will not merge it silently. Plugin sessionflow_evidence_verify can proceed as a draft PR against this shape.

@HarryHeYu HarryHeYu left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Final technical acceptance — PASS (no blocking issues)

Re-checked at 856a09e (unchanged since my earlier review). Still Draft,
CI 10/10 pass, MERGEABLE / CLEAN. I am not changing its status —
it stays Draft until you mark it Ready for Review.

I re-verified the seven points by running the op, not by reading alone. Seeded a
temporary index, called handle_request({"op": "evidence_verify", ...}, db=...)
for each verdict:

BRIDGE_SCHEMA_VERSION: 1
evidence_verify in _OPS: True
integration_info().ops has it: True

MATCH               status=MATCH               exit_code=0
MISMATCH            status=MISMATCH            exit_code=1
NOT_FOUND           status=NOT_FOUND           exit_code=2
UNVERIFIABLE        status=UNVERIFIABLE        exit_code=3
INVALID_REFERENCE   status=INVALID_REFERENCE   exit_code=4

unknown op -> error: "unknown op: 'evidence_verify_v2' (ops: [...])"   # no result
EXIT_CODES: {'MATCH': 0, 'MISMATCH': 1, 'NOT_FOUND': 2, 'UNVERIFIABLE': 3, 'INVALID_REFERENCE': 4}

Against the agreed boundary:

  1. Thin wrapper only — evidence_verify() opens a read-only Store and
    returns evidence.verify(...) verbatim. verify(), EXIT_CODES and the
    fingerprint algorithm are untouched; the diff is two files (voyager/api.py,
    tests/test_evidence_verify_op.py), no existing line modified.
  2. All five states reachable by name through the API, each with its own
    status (the run above).
  3. exit_code is a result field, not the API's process status: every call
    returned {id, result} with no error, including the four non-zero verdicts.
    This is the property that keeps callOp() from flattening them into
    CLI_FAILED.
  4. schema_version stays 1 — verified directly.
  5. integration_info().ops includes evidence_verify — verified, so a
    client can feature-detect by op name instead of sniffing versions.
  6. Old-Core behaviour is a capability error, not a verdict — an unknown op
    returns error with no result; it cannot be mistaken for evidence
    NOT_FOUND or MATCH.
  7. No scope change — no CLI change, no MCP tool, no fingerprint change, no
    REQUIRED_CORE_REF bump, no other provider touched.

Also ran the PR's own suite: tests/test_evidence_verify_op.py → 8 passed.

Non-blocking note

evidence_verify(db=None, session_id="", seq=None, digest=None) does not
pre-validate; an empty/malformed reference resolves to INVALID_REFERENCE,
which is the honest answer. Argument validation belongs in the plugin layer
(refuse before spawning) — no change requested here.

Merge order

Core first, then the plugin tool. This PR is the Core half; the plugin
sessionflow_evidence_verify (dsh-sessionflow#9) can be prepared as a draft now
but should not be accepted until an installable Core containing this op exists.

Not merged here. Merge is a separate explicit step after you mark it Ready;
no release or tag change is made in this round.

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