[Bindy-lbb]

Canonical requirement

• Canonical Issue URL: feat(approvals): add policy-gated batch decisions for homogeneous low-risk requests roleweave#403
• Consumed revision: R1
• Consumed decision reference: feat(approvals): add policy-gated batch decisions for homogeneous low-risk requests roleweave#403 (current canonical Issue body; no revision ledger is published)
• No automatic close keywords: acknowledged

Delivery ownership

• implementationOwner: Codex
• automatedPreReviewOwner: local governance precheck and GitHub CI
• humanReviewOwner: @Bindy-lbb
• Separation of duties: Codex authored the implementation; @Bindy-lbb is the required human reviewer and merge ledger owner.

Requirement trace

REQ/AC IDs	Changed files / domain	Tests or review evidence
REQ-001 / AC-001	packages/engine/src/contracts.ts, turn-executor.ts: sealed plural approval-recovery contract	tests/engine/approval-events.test.ts atomically validates the complete homogeneous set before model consumption
REQ-002 / AC-002	apps/cli/turn/*, configs/turn-envelope.schema.json: envelope boundary and recovery hand-off	tests/apps/turn-envelope-schema.test.ts rejects mixed and duplicate batch members
REQ-003 / AC-003	packages/engine/src/turn-evidence.ts: plural approval evidence	focused engine evidence assertions in tests/engine/approval-events.test.ts
REQ-004 / AC-004	engine preflight and terminal settlement	malformed, mixed, and expired batches emit no partial verdict in the focused test suite

File domains

• Engine contracts, executor, and evidence own the atomic batch settlement semantics and recovery proof.
• CLI envelope builder, parser, schema, and turn runner carry only the sealed pendingApprovals boundary.
• Focused engine and envelope tests cover all-or-nothing validation and backward compatibility.

Scope and non-goals

This PR supplies the engine dependency for RoleWeave #403: a recovery turn can settle 2–32 homogeneous approval verdicts as one validated set before execution resumes. It preserves the legacy singular approval path.

It does not provide queue UI behavior, policy classification, external network enforcement, or a broad approval scope; those remain in RoleWeave PR #410 and its policy layer.

Validation

• Exact commands: npm run typecheck --silent; npx tsx --test tests/apps/turn-envelope-schema.test.ts tests/engine/approval-events.test.ts; npm run build --silent; node ./scripts/governance-precheck.js --body-file pr-337-body.md --base-sha origin/main --head-sha HEAD --pr-number 337 --repo bytefolk/digital-employee
• Observed counts/results: PASS 1/1 typecheck; PASS 27/27 focused tests; PASS 1/1 build; PASS 1/1 governance precheck.
• Check URLs: https://github.com/bytefolk/digital-employee/actions/runs/35499048667

ID	REQ/AC	Observable acceptance criterion	Command or manual steps	Environment	Expected	Observed	Status
V1	REQ-001 / AC-001	A complete homogeneous batch is validated before model consumption	npx tsx --test tests/engine/approval-events.test.ts	Node 24, Linux	27 focused tests pass	27/27 tests passed	PASS
V2	REQ-002 / AC-002	Mixed or duplicate members fail at the sealed envelope boundary	npx tsx --test tests/apps/turn-envelope-schema.test.ts	Node 24, Linux	envelope cases pass	included in 27/27 focused tests	PASS
V3	REQ-003 / AC-003	Plural evidence is recorded without changing legacy approval handling	npm run typecheck --silent	Node 24, Linux	typecheck passes	1/1 command passed	PASS
V4	REQ-004 / AC-004	Malformed or expired batches emit no partial verdict	npx tsx --test tests/engine/approval-events.test.ts	Node 24, Linux	negative cases pass	included in 27/27 focused tests	PASS

Security and compatibility

• No credentials, personal identifiers, private messages, internal URLs, or generated indexes are included.
• Source/tool access and public evidence are explicitly bounded.
• Logs redact content, identities, and credential-bearing URLs.
• Compatibility is preserved: legacy singular pendingApproval remains supported while plural recovery is opt-in.
• No dependency changed.
• Typecheck, focused tests, build, and local governance precheck pass; GitHub CI is being re-run after this body repair.
• No documentation delta is needed in this repository: the consuming approval product documentation is in RoleWeave PR #410.

The protocol is fail-closed: malformed, expired, mixed, or duplicate members never produce a partial set of approval events or reach model consumption.

Known limitations

• The engine accepts only homogeneous decisions in a 2–32 member recovery set.
• End-to-end policy classification and the operator queue remain owned by RoleWeave PR #410.
• The earlier GitHub test (22) result predates this repaired implementation trace; its rerun is required before merge.

Risk and rollback

The primary risk is cross-version consumers that do not recognize pendingApprovals; the sealed schema fails those consumers closed. Roll back by reverting this single commit, returning callers to the legacy singular recovery path. No persistent migration or irreversible data operation is introduced.

Product review handoff

• Automated pre-review result: PREFLIGHT PASS
• Human final review: PENDING
• Merge ledger owner: @Bindy-lbb
• Product reviewer: @Bindy-lbb
• Milestone or release packet: feat(approvals): add policy-gated batch decisions (#403) roleweave#410
• Merge, CI, release, and model judgment do not accept or close the Issue: acknowledged

[waterbro-8]

Review (digital-employee #337, supports roleweave #403)

REQUEST_CHANGES — not a product 验收.

CI test (22) / governance:check failed on this PR body:

• missing required headings: Canonical requirement, Requirement trace, File domains, Scope and non-goals, Validation, Security and compatibility, Known limitations, Risk and rollback, Product review handoff
• canonical Issue must be a full GitHub Issue URL
• consumed revision must match R
• no-auto-close acknowledgement is missing
• requirement trace / validation ledger table headers missing
• exact commands / observed counts / check URLs need copyable evidence or NOT VERIFIED: <reason>
• product handoff needs merge ledger owner, product reviewer, packet URL or N/A: <reason>

Do not merge while test (22) is red. I did not treat this as product 验收.

After the PR body matches the governance ledger, ping me for a second pass. CODEOWNERS merge still requires @PeterGuy326 / @Bindy-lbb / @waterbro-8 and I cannot merge my own CODEOWNER PRs.

[PeterGuy326]

Re-review of head eba9e74e. The governance ledger waterbro-8 asked for is now in the body, and the red test (22) in the rollup is the superseded 08:15 run — the 08:32 re-run on this same head is green across the matrix including test (22). The batch gate itself is sound. One blocking finding remains, and it is in the evidence contract rather than the settlement path.

Blocking: batch recovery turns produce evidence that fails the repo's own conformance checker

turn-evidence.ts gains the approvalRefs?: TurnEvidenceApprovalRef[] property, and turn-executor.ts spreads it into every record (...(approvalRefs !== undefined ? { approvalRefs } : {})). But ALLOWED_TOP_LEVEL_KEYS in the same file is not updated, and validateTurnEvidenceRecord fails closed on any unrecognised top-level key — by design, per its own docstring: "the evidence record is a sealed digest-only artefact, and an unrecognised field signals producer drift, not forward compatibility."

So every atomic-batch recovery turn — granted, denied and expired alike — writes a turn-evidence.v1 record that the published #140 conformance checker rejects. Verified by execution against the validator (unchanged by this PR, so main is equivalent):

singular approvalRef  -> true  []
batch approvalRefs    -> false [{"field":"approvalRefs","code":"unknown_field",
                                 "message":"unrecognised top-level field: approvalRefs"}]

This is not a theoretical consumer: scripts/w1-acceptance-harness.mjs runs records through validateTurnEvidenceRecord, and docs/verification.md AC-004b states "all 8 archived records pass validateTurnEvidenceRecord". The PR ships a producer whose output cannot pass. The existing suites stay green only because no test feeds a batch-settled record to the validator — tests/engine/approval-events.test.ts asserts evidenceSink.records[0].approvalRefs directly and never round-trips it through conformance.

Needed:

1. Add "approvalRefs" to ALLOWED_TOP_LEVEL_KEYS.
2. Add a validator branch mirroring the approvalRef rules — array, 2–32 members, each with a non-empty approvalId, optional string previewId, and outcome in the existing enum — with dotted violation fields (approvalRefs[0].approvalId, …) so the harness can name the offending member.
3. Add a conformance test for a conformant batch record plus at least one rejecting case, and extend the #403 engine test to pass its settled record through validateTurnEvidenceRecord so producer and checker cannot drift apart again.

Verified as correct

• validateTurnRequest runs at turn-executor.ts:367, long before the approval gate, and it calls validateApprovalRequestFields — so the 2–32 bound, the approvalId uniqueness check and the single-decision homogeneity check are all enforced before the gate reads pendingApprovals[0].decision. Reading element 0 for the branch is therefore safe.
• Atomicity holds at the boundary: expiry is evaluated with .some() across the whole set before any event is yielded, so one expired member produces zero approval.granted / approval.denied events and the model port is never touched. Matches AC-004.
• Three-way mutual exclusion (approvalAction / pendingApproval / pendingApprovals) is enforced in both the engine and parseTurnEnvelope, and the envelope parser re-checks length, duplicate ids and homogeneity independently of the JSON Schema. Fail-closed at both layers.
• The batch branches mirror the singular ones exactly — evidence before event, engine.internal_error on a sink failure, cancelled terminal with the existing APPROVAL_EXPIRED_CODE / APPROVAL_DENIED_CODE, no new terminal reasons.
• Keeping pendingApprovals as a second field rather than overloading pendingApproval leaves legacy envelopes byte-identical; the #205 v1 byte-exactness test still covers that.

Non-blocking

• GitHub code search across the org's default branches finds no producer or consumer of pendingApprovals outside this branch, so the engine leg lands ahead of its only caller. Fine for a dependency PR, but #403 is not wired end to end by this merge and should not be reported as such.
• The expired branch stamps outcome: "expired" on every member, including the ones that were still valid. That is the right atomic semantics, but the record then does not say which verdict expired, so diagnosing a batch failure means recomputing expiry from the stored verdicts. Consider carrying the offending id(s).
• uniqueItems: true in both turn-envelope.schema.json and buildTurnEnvelopeSchema() compares whole objects, not approvalId, so two members with the same id but different reason pass the Schema layer. The parser catches it; the Schema assertion is just weaker than it looks.

Not product 验收. Merge also stays blocked on waterbro-8 re-reviewing or dismissing his CHANGES_REQUESTED — that review is on this same head, so GitHub still counts it.

[Bindy-lbb]

Addressed the evidence-contract blocker in d763d4a: approvalRefs is now allowlisted, validated as a 2–32 member batch with dotted member violations, covered by conformance tests, and the #403 producer test now validates its emitted record end-to-end. Local npm run typecheck, targeted conformance/approval tests, governance:check, and security:check pass. @PeterGuy326 @waterbro-8 please re-review this head; the remaining CHANGES_REQUESTED reviews still block merge.

[waterbro-8]

Review (digital-employee #337, supports roleweave #403)

APPROVE — not a product 验收. Do not merge from this review.

Re-review of head d763d4a. Previous blockers are gone:

• PR body now has Canonical requirement, Requirement trace, and the rest of the governance ledger. Canonical Issue is the full RoleWeave #403 URL; consumed revision is R1; no-auto-close is acknowledged.
• CI is green on this head, including test (20/22/24) (governance:check runs inside test (22)), coverage, commit-metadata, CodeQL.
• Evidence-contract gap from PeterGuy326 / 胡奕舟: approvalRefs is in ALLOWED_TOP_LEVEL_KEYS, 2–32 members are validated with dotted fields, conformance covers accept + reject, and the #403 producer record round-trips through validateTurnEvidenceRecord.

Atomic batch settlement holds: the complete homogeneous set is validated before any verdict event or model consumption; mixed, duplicate, and expired batches emit no partial approval events.

Not product 验收. I cannot merge my own CODEOWNER PRs.

Withdrawing my eba9e74 change-request: the approvalRefs evidence gap is closed on d763d4a. Re-review posted separately. Not product 验收; no merge judgment.

[PeterGuy326]

Re-review (digital-employee #337, head d763d4a)

My previous CHANGES_REQUESTED was on head eba9e74, blocking solely on the evidence-contract gap. That finding is resolved, and I am withdrawing the change request. Not product 验收, and no merge judgment from this review — @Bindy-lbb remains merge ledger owner.

What closed it, checked against d763d4a:

• approvalRefs added to ALLOWED_TOP_LEVEL_KEYS, so an atomic-batch recovery turn no longer trips the validator's fail-closed unknown-field path — which was the concrete breakage (every granted/denied/expired batch record was being rejected by the published #140 conformance checker).
• validateTurnEvidenceRecord now validates the array itself: 2–32 length bound, per-member object_required, non-empty approvalId, previewId string-when-present, and outcome against APPROVAL_OUTCOMES — reported with dotted field paths (approvalRefs[0].outcome), matching how the singular approvalRef errors are shaped.
• The producer is pinned, not just the validator: approval-events.test.ts now asserts validateTurnEvidenceRecord(evidenceSink.records[0]).violations is [] on the real batch settlement, so this cannot silently regress.
• turn-evidence-conformance.test.ts covers accept and reject in both directions.

Scope discipline is right: 54 added lines, all in the evidence contract and its tests, nothing touched in the settlement path that was already sound.

State now: 16/16 checks green on this head including test (20/22/24), coverage, commit-metadata, CodeQL; @waterbro-8 approved on d763d4a.