Skip to content

Unit 5 - Add ApprovalPolicy enum and non-interactive helpers - #5

Merged
srdjan merged 2 commits into
mainfrom
unit-5-approval-policy-enum
Apr 19, 2026
Merged

Unit 5 - Add ApprovalPolicy enum and non-interactive helpers#5
srdjan merged 2 commits into
mainfrom
unit-5-approval-policy-enum

Conversation

@srdjan

@srdjan srdjan commented Apr 19, 2026

Copy link
Copy Markdown
Owner

Summary

  • Adds ApprovalPolicy enum (ask / auto_approve / auto_reject) and matching autoApprove / autoReject helpers to packages/pi/src/loop.zig.
  • Pure-additive infrastructure so a future non-interactive worker (zigts expert --print / --mode json) can set RunOptions.approval_fn = autoApprove without blocking on stdin.
  • Folds the test-only rejectEdit helper into the public autoReject (identical semantics, single source of truth).
  • No call sites in repl.zig, tui/app.zig, agent.zig, or elsewhere are touched.

Plan

  • Plan file: /Users/srdjans/.claude/plans/study-the-plan-provided-clever-wadler.md (Unit 5).

Test plan

  • zig build test-expert-app passes
  • zig build test passes
  • zig build succeeds
  • timeout 10 bash -c 'printf "help\nquit\n" | ./zig-out/bin/zigts expert' prints /verify and Registered tools: before EOF

Add ApprovalPolicy enum with ask/auto_approve/auto_reject variants plus
matching autoApprove and autoReject helper functions to loop.zig. Pure-
additive type infrastructure that a future non-interactive worker
(zigts expert --print / --mode json) can plug into approval_fn without
blocking on stdin.

Also folds the local test-only rejectEdit into the new autoReject helper.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@srdjan
srdjan enabled auto-merge April 19, 2026 23:32
@srdjan
srdjan disabled auto-merge April 19, 2026 23:33
@srdjan
srdjan enabled auto-merge April 19, 2026 23:33
@srdjan

srdjan commented Apr 19, 2026

Copy link
Copy Markdown
Owner Author

@copilot resolve the merge conflicts in this pull request

@srdjan
srdjan disabled auto-merge April 19, 2026 23:36
Signed-off-by: srdjan <61190+srdjan@users.noreply.github.com>
@srdjan
srdjan merged commit 4e2164a into main Apr 19, 2026
@srdjan
srdjan deleted the unit-5-approval-policy-enum branch April 19, 2026 23:37
srdjan added a commit that referenced this pull request Apr 19, 2026
The PR #5 merge of unit-5-approval-policy-enum branched from a main
commit that predated `tmpWorkspacePath`, so the PR recreated the
helper. Conflict resolution kept both copies; drop the duplicate.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
srdjan added a commit that referenced this pull request May 24, 2026
The rule registry never indexed flow_checker.DiagnosticKind, so adding,
removing, or renaming a flow diagnostic did not trip the embedded
contract policyHash drift check at runtime. Data-label flow analysis
changes silently passed verifyPolicyHash on contracts produced by older
analyzer revisions.

Add flow_meta covering all eight DiagnosticKind variants with codes
ZTS400-ZTS407, mirroring the canonical mapping in
packages/tools/src/json_diagnostics.zig:151-162. A comptime block at
file scope walks @typeinfo(flow_checker.DiagnosticKind).@"enum".fields
and @CompileError's on any variant missing from flow_meta, so new flow
diagnostics cannot land without a corresponding registry row.

Extend RuleCategory with .flow, thread flow rules into the all_rules
comptime block, and fold @intFromEnum(rule.category) as a single byte
into computePolicyHash. The category byte means reclassifying a rule
between categories (e.g. .verifier -> .flow) drifts the hash even when
all text strings stay identical - a class of silent drift the previous
hash could not detect.

Three regression tests: every DiagnosticKind projects into all_rules
under .flow; the exhaustiveness gate is exercised through findByName;
two RuleEntry values identical in every field except category produce
distinct hashes. Bump the sanity lower bound from 27 to 35
(verifier+policy+property+flow).

Intent: fix-defect
Scope: zigts/rule_registry, zigts/flow_checker, runtime/policy_hash
Decided-Against: separate flow_hash sibling to policy_hash (doubles drift surface; consumers would need to verify both)
Decided-Against: hash on category label string instead of enum byte (verbose; same drift coverage)
Refs: 2026-05-23 overall-repo review, Wave 1B/2D P0 #5

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
srdjan added a commit that referenced this pull request Jun 8, 2026
Code-review cleanup follow-ups (#10, #11, #14) plus one fidelity fix (#2).

#10 - extract replayRecordedEntry(state, ctx, entry): the result-parse +
parse-failure divergence heuristic now has a single owner, shared by
makeReplayStub and makeReplayStubWithFallback instead of being copy-pasted.

#14 - add ReplayState.consumeHead(): the fallback trusts a prior peekMatches
and advances without re-checking bounds/names, dropping the redundant nextIO
re-scan and its dead orelse branch.

#2 - add ReplayState.hasEntryAhead(): when a pure call's recorded entry is not
at the head but exists further on, the fallback counts a divergence (structural
drift) before running real, so replay verification still fails closed; a
genuinely-new call with no recording stays silent. modules_all (entry at head)
and validate-external (no entries) are unaffected.

#11 - add trace.unescapeBody(allocator, body) -> {slice, owned}: test_runner
and replay_runner share it for request-body unescaping, replacing the
duplicated unescape-with-guarded-free idiom (the owned flag also retires the
misleading .ptr re-check).

New unit test covers consumeHead/hasEntryAhead. Verified: test-zigts, test-cli
(497), test-examples 23/23, test-zruntime clean; env exclusion and validate
passthrough intact.

Note: #4 (env fallback under restrictive policy) is already moot - env has
replay_pure=false and never falls through. #5 (matched-path arg checking) is
deferred: it needs the recorder's JSValue serializer extracted and reused for
byte-identical arg comparison, with real risk of false divergences against
existing fixtures; better as a dedicated, test-gated change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
srdjan added a commit that referenced this pull request Jun 8, 2026
…ializer

Code-review item #5 plus the serializer-parity refactor it required.

Parity refactor: the TraceRecorder's appendJSValueDepth was a byte-identical
twin of the free appendJSValueBuf (already used by the durable oplog). Unify
them - TraceRecorder.appendJSValue now delegates to appendJSValueBuf - so
recording, the durable oplog, and replay arg-comparison share one serializer
and stay byte-identical by construction instead of by two copies agreeing.
Recording output is unchanged (identical logic); -85 lines.

#5: serializeArgsJson builds the recorded [a,b,...] shape with that shared
serializer. Both replay stubs pass live args into replayRecordedEntry, which
counts a divergence when argsDrifted(recorded, live) is true - so a recorded
entry replayed against changed arguments fails replay verification closed. It
skips unpinned recordings ("[]"/empty), protecting zero-arg calls and
hand-written fixtures.

Validation: no false divergences (test-zigts, test-zruntime, test-examples
23/23, test-cli 497 all green; existing recorded fixtures passing is the
parity proof). New argsDrifted unit test covers same-args/changed-arg/arity/
unpinned-skip. End-to-end note: serve --replay diffs response bodies, while #5
increments the divergence counter consumed by the proof/replay_runner gate; a
replay_runner integration test asserting divergences>0 on an arg change is a
possible follow-up.

Net -29 lines despite adding the feature (the parity refactor removed the
duplicate serializer).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
srdjan added a commit that referenced this pull request Jun 8, 2026
…ergence

Belt-and-suspenders coverage for #5. replaySha256ArgDivergences builds a replay
runtime, installs a recorded crypto.sha256 entry captured with arg "AAA", runs a
handler, and returns the divergence count.

- "replay flags an argument change at the matched path (#5)": handler calls
  sha256("BBB") against the "AAA" recording -> divergences > 0.
- "replay does not flag matching arguments (#5 control)": sha256("AAA") -> 0.

The control makes it airtight: matching args give 0 and only the changed arg
gives >0, so the matched-path arg check is provably the cause. Verified by an
inversion check (flipping the assertion to ==0 fails the test), so it is not a
false pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
srdjan added a commit that referenced this pull request Jun 28, 2026
…xhigh #4/#5)

The audit was added to verify.sh to machine-check the SMT soundness boundary, but
the gate passed silently when the check did not actually run:

- #4: with z3 absent the audit skipped (available=false, zero failures, exit 0)
  and the gate printed "semantics spec gate OK" - the boundary check was a no-op.
- #5: a slow f64-associativity refutation that hit the 15s ceiling returned
  unknown -> inconclusive -> non-fatal, so a law that was never refuted still
  passed the gate with false confidence.

The command stays deliberately lenient (a z3-absent skip or an inconclusive
timeout is non-fatal, so a future genuinely un-refutable law cannot wedge plain
`spec-check --audit`). Strictness goes in the gate, where these findings live:
verify.sh now re-reads `spec-check --audit --json` and REQUIRES a complete audit
(z3 present, refuted == total, inconclusive == 0). A silent skip or timeout now
fails the gate; an explicit ZIGTTP_Z3 opt-out (off/none/0/disable) is honored.

Also bump the audit :timeout 15s -> 30s (6x the measured ~5s assoc search) so the
now-strict gate does not flake on a slow/loaded machine. The semantics hash is
unchanged (the preamble timeout is not hashed).

Verified: all four gate branches (complete pass / z3-absent fail / explicit
opt-out skip / inconclusive fail) behave correctly; real gate passes 4/4 refuted.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
srdjan added a commit that referenced this pull request Jul 12, 2026
… caching, Sonnet default, exit codes

Wave 1 of the expert-analysis recommendations (#1, #2, #4, #5, #8).

#1 Make the retry loop information-complete. The `.run_veto` arm now records
the model's draft as an `apply_edit` tool_use and closes it with the compiler
verdict as a tool_result (session-unique id keyed on transcript index). On a
failure the tool_result carries the diagnostic + SQL escalation and the
compiler-authored repair block is persisted as a follow-up system_note, so the
whole retry context lives in the transcript instead of a transient
`extra_user_text` that a mid-repair tool call would erase. Drops `diag_history`
re-accumulation; `.retry_draft` now sends only a short framing nudge. Adds a
regression test proving the failed draft + diagnostic survive a tool interleave.

#2 Raise the per-turn wall-clock timeout 60s -> 300s (the recorded convergence
for complex handlers ran 80-96s) and drop the eval harness's `turn_timeout_ms=0`
override so measurement and production share one options struct.

#4 Add a rolling cache breakpoint on the last message block so the conversation
prefix hits the prompt cache instead of re-paying full input price every
roundtrip.

#5 Flip the default model claude-haiku -> claude-sonnet-4-6 (the measured 7/7
first-draft baseline); Haiku stays available via `--model` and the harness's
ZIGTTP_CODEGEN_MODEL knob. Docs updated.

#8 Outcome-bearing `--print` exit codes: 0 applied/clean, 1 hard error,
2 veto-exhausted, 3 budget, 4 approval-denied. Documented in the user guide.

Also lands the OpenAI honest-demotion doc note (#7, docs portion).

Co-Authored-By: Claude Opus 4.8 (1M context) <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.

1 participant