Tag issues deferred from PR reviews

[LeanAndMean]

Summary

• Add a lazy, batch-scoped PR review deferral label decision to deferred PR-review issue creation.
• Preserve duplicate handling and isolate label lookup, creation, and application failures from issue publication.
• Validate newly created issue identities before applying labels, and prevent issue mutation while structured confirmation is cancelled.
• Add semantic contract tests for label discovery and authorization, mutation ordering, duplicate exclusion, and graceful failure behavior.

Test plan

• Run npx vitest run packages/scramjet/tests/mach12-wiring.test.ts
• Run npm run typecheck
• Run npx biome check packages/scramjet/tests/mach12-wiring.test.ts packages/scramjet/mach12/commands/mach12:pr-review-assessment.md
• Manually verify both deferred issue-creation paths share one label decision and all-duplicate batches do not prompt

Fixes #462

[LeanAndMean]

PR Review: Tag issues deferred from PR reviews

Critical

None.

Important

F1: Option 3 can block issue creation after recoverable label-resolution failures (packages/scramjet/mach12/commands/mach12:pr-review-assessment.md:254). The shared batch contract explicitly says a failed/malformed lookup or failed label creation continues issue creation without the label, but Option 3 allows mutation only after the decision “succeeds or is explicitly declined.” Those failure outcomes satisfy neither condition, so an agent following the narrower instruction can stop instead of preserving issue publication. Permit mutation once label handling is resolved by success, explicit decline, lookup failure, or creation failure; cancellation should remain the sole unresolved/paused state. This was independently identified by mach12:code-reviewer and mach12:comment-analyzer.

F2: The new issue-publication contract does not define the outcome when gh issue create itself fails or when successful output contains zero or multiple candidate URLs (packages/scramjet/mach12/commands/mach12:pr-review-assessment.md:160-170). It requires exactly one candidate and defines failed canonical identity validation, but does not require surfacing the create error, prohibit retries for ambiguous output, or return a non-completed status for those paths. The established analogous contract in packages/scramjet/mach12/commands/mach12:issue-create.md:244-252 handles these cases explicitly. Adopt that distinction here so definite command failure is surfaced, ambiguous successful output is reported as “may have succeeded” without retry or metadata mutation, and either path cannot be reported complete. Per mach12:silent-failure-hunter and mach12:test-analyzer.

Suggestions

S1: Strengthen the semantic contract tests at packages/scramjet/tests/mach12-wiring.test.ts:917-958. Current phrase-presence assertions can still pass if Option 3 adds a second lookup/prompt or contradicts the shared failure behavior. Assert lookup/label-create command cardinality and the complete resolved-decision-to-mutation relationship, plus explicit create-failure and ambiguous-output outcomes. Per mach12:test-analyzer.

Strengths

• The label decision is lazy and shared across the batch, so all-duplicate batches avoid unnecessary lookup and authorization.
• Exact case-sensitive paginated lookup distinguishes proven absence from unavailable or malformed lookup results.
• Clear duplicates remain excluded from label mutation, while existing PR and F/S provenance is preserved.
• Issue publication is decoupled from label application, with canonical identity validation before guarded metadata edits.
• Focused verification passes: 104/104 tests, and git diff --check reports no whitespace errors.
• The change stays within the existing command contract and adds no runtime state, dependencies, or unnecessary abstractions.

Reviewed by GPT-5.6 Sol

This is an automated review.

[LeanAndMean]

Independent Review Assessment

Assessing review comment: #464 (comment)

Classifications

F1 — Genuine issue: Option 3 can block issue creation after recoverable label-resolution failures. The shared contract permits publication after lookup or label-creation failure (packages/scramjet/mach12/commands/mach12:pr-review-assessment.md:154-156), while Option 3 permits mutation only after success or explicit decline (packages/scramjet/mach12/commands/mach12:pr-review-assessment.md:254). Replace that gate with an exhaustive resolved-state rule covering label found or created, explicit No, lookup failure, and creation failure; cancellation remains the sole paused state.

F2 — Genuine issue: The issue-publication contract does not define failed creation or ambiguous successful output. It requires one candidate but specifies consequences only for identity-validation failure (packages/scramjet/mach12/commands/mach12:pr-review-assessment.md:163-170); unlike the established contract in packages/scramjet/mach12/commands/mach12:issue-create.md:244-252, it does not explicitly surface gh issue create failure or classify zero or multiple output lines as potentially successful. Add those outcomes, prohibit retries and metadata mutation after ambiguous output, and require a non-completed status.

S1 — Genuine issue: The semantic contract tests need stronger cardinality and failure assertions. They primarily assert phrase presence and currently require the contradictory Option 3 wording (packages/scramjet/tests/mach12-wiring.test.ts:917-958), so duplicate lookup or prompt instructions and missing publication-failure semantics could pass. Add exact command cardinality assertions and explicit coverage for every terminal label decision, cancellation, create failure, and ambiguous successful output.

Staged Implementation Plan

Stage 1 — Correct deferred issue-publication control flow (F1, F2)

Required before merge.

• Define all terminal label-resolution outcomes and make cancellation the only mutation pause.
• Specify guarded gh issue create failure handling and zero or multiple output-line handling, including no retry, no metadata mutation, and non-completed status.
• Affected file: packages/scramjet/mach12/commands/mach12:pr-review-assessment.md

Stage 2 — Strengthen semantic contract coverage (S1)

Required before merge.

• Assert one label lookup and at most one label-creation instruction per batch.
• Cover each resolved decision’s mutation permission and definite and ambiguous issue-creation failures.
• Affected file: packages/scramjet/tests/mach12-wiring.test.ts

Assessed by GPT-5.6 Sol

[LeanAndMean]

Addressed the selected findings from originating review ID 5174500866.

• F1: made label lookup failure and label-creation failure resolved outcomes that permit issue publication; cancellation remains the sole paused state.
• F2: distinguished definite gh issue create failure from ambiguous successful output, with no retry or metadata mutation and a required non-completed status.
• S1: enforced batch command cardinality and branch-specific terminal outcome semantics in the wiring tests.

Commit: f05b5f5 (Clarify deferred issue publication failures)
Verification: 105 focused tests passed; typecheck, Biome, and git diff --check passed.

[LeanAndMean]

PR Review: Tag issues deferred from PR reviews

Critical

None.

Important

F1: Label lookup can treat malformed entries as proof that PR review deferral is absent (packages/scramjet/mach12/commands/mach12:pr-review-assessment.md:151-154). The contract validates only that the response is an array of page arrays before comparing each entry's name; entries such as null, scalars, or objects with a non-string name therefore have no required malformed-result path and can fall through to the “exact label absent” branch, prompting unauthorized metadata creation from incomplete authority. Require every flattened entry consumed by the comparison to be an object with a string name; otherwise classify the lookup as malformed/unknown and continue unlabelled without prompting. Per mach12:type-design-analyzer.

Suggestions

S1: Extend the semantic contract tests at packages/scramjet/tests/mach12-wiring.test.ts:917-975 to pin complete label-entry shape validation and the malformed-entry fallback. The current tests require only the outer “array of page arrays” phrase, so F1's unsafe classification can remain while all 105 focused tests pass. Per mach12:test-analyzer and mach12:type-design-analyzer.

S2: In Option 3, refer directly to the Shared issue-creation batch contract instead of saying it is “described in Option 1” (packages/scramjet/mach12/commands/mach12:pr-review-assessment.md:255). The contract precedes Option 1, and a direct reference avoids drift between the shared rules and option-specific prose. Per mach12:code-simplifier.

Strengths

• The earlier review's failure-state defects are resolved: recoverable label outcomes permit publication, while cancellation alone pauses mutation.
• Definite issue-creation failure, ambiguous successful output, and canonical identity-validation failure avoid unsafe retries and metadata mutation.
• Label resolution is lazy and batch-scoped, and clear duplicates remain excluded from label mutation.
• Canonical issue identity validation precedes independent label application, preserving confirmed issues when optional metadata fails.
• PR and F/S provenance, overlap notes, and deferred-item dispositions remain intact.
• Focused verification passes: 105/105 tests, and git diff --check is clean.

Reviewed by GPT-5.6 Sol

This is an automated review.

[LeanAndMean]

Independent Review Assessment

Assessing review comment: #464 (comment)

Classifications

F1 — Genuine issue: The lookup contract validates only the outer page-array structure before interpreting label names (packages/scramjet/mach12/commands/mach12:pr-review-assessment.md:153-155). A malformed entry can therefore be treated as a non-match and incorrectly prove absence, triggering metadata-creation authorization. Require every flattened entry to be an object with a string name; otherwise classify the lookup as malformed/unknown and continue unlabelled without prompting.

S1 — Genuine issue: The test asserts only the phrase “array of page arrays” (packages/scramjet/tests/mach12-wiring.test.ts:917-929), so it does not enforce entry validation or the safe malformed-entry outcome. Add assertions requiring object/string-name validation and malformed-entry fallback without authorization; this is a contained, behavior-preserving improvement.

S2 — Nitpick: Option 3 says the shared decision is “described in Option 1” (packages/scramjet/mach12/commands/mach12:pr-review-assessment.md:253-257), although the authoritative contract precedes Option 1 at lines 139-180. Referring directly to the Shared issue-creation batch contract is a safe clarity improvement, but the existing reference remains understandable and does not change behavior.

No later commit or discussion resolves these findings; PR HEAD remains f05b5f5.

Staged Implementation Plan

Stage 1 — Complete malformed-label validation (required; F1, S1)

• Extend the lookup contract to validate every flattened label entry as an object with a string name.
• Treat any invalid entry as malformed/unknown, continuing issue creation unlabelled without prompting.
• Add semantic assertions for complete entry validation and this fallback.
• Affected files:
  • packages/scramjet/mach12/commands/mach12:pr-review-assessment.md
  • packages/scramjet/tests/mach12-wiring.test.ts

Stage 2 — Correct the shared-contract reference (optional; S2)

• Replace “described in Option 1” with a direct reference to the Shared issue-creation batch contract.
• Affected file:
  • packages/scramjet/mach12/commands/mach12:pr-review-assessment.md

Assessed by GPT-5.6 Sol

[LeanAndMean]

Addressed findings from originating review ID 5174615503.

• F1: require every flattened label lookup entry to be an object with a string name; malformed entries now make availability unknown and cannot authorize a label-creation prompt.
• S1: pin complete entry-shape validation, the unlabelled malformed-entry fallback, and the prohibition on prompting or creating metadata.
• S2: point Option 3 directly to the Shared issue-creation batch contract.

Commit: d9cc6b5 (Reject malformed deferred-label lookup entries)
Verification: 105 focused tests passed; Biome, git diff --check, and a scoped correctness review passed.

[LeanAndMean]

PR Review: Tag issues deferred from PR reviews

Critical

None.

Important

F1: The test named “authorizes creation once” does not enforce one authorization prompt (packages/scramjet/tests/mach12-wiring.test.ts:917-934). It asserts only that type: "confirm" appears, so a second confirmation elsewhere in Step 6—or one prompt per finding—would still pass despite violating the batch-scoped authorization contract. Assert exactly one structured confirmation instruction within deferredSection, parallel to the existing lookup and label-creation command cardinality checks. Per mach12:test-analyzer.

F2: Canonical identity validation and its failure outcome are not semantically pinned (packages/scramjet/tests/mach12-wiring.test.ts:937-952). The test checks that gh issue view precedes label application, but it would still pass if the command stopped requiring a positive integer number, a non-empty canonical url, no label or retry after validation failure, the “may have succeeded” warning, or a non-completed status (packages/scramjet/mach12/commands/mach12:pr-review-assessment.md:171-176). Add assertions for those invariants so unsafe metadata mutation or duplicate issue creation cannot regress unnoticed. Per mach12:test-analyzer and mach12:type-design-analyzer.

Suggestions

S1: Require failed label lookup or label creation to identify the failed operation and include concise error context in the CLI summary (packages/scramjet/mach12/commands/mach12:pr-review-assessment.md:154-156,178-180). The current generic batch guidance can leave authentication, permission, rate-limit, network, or malformed-response failures unactionable even though unlabelled issue publication correctly continues. Preserve the non-blocking behavior, but surface the relevant failure cause and assert it in the semantic contract tests. Per mach12:silent-failure-hunter.

Strengths

• The implementation fully satisfies linked issue Tag issues created from deferred review findings #462 and its latest plan: label resolution is lazy, exact, validated, and shared across the batch.
• Clear duplicates remain unlabelled, while existing PR/F/S provenance, overlap notes, and dispositions are preserved.
• Cancellation is the sole unresolved state and prevents issue mutation until a resumed user turn.
• Issue publication, canonical identity validation, and optional label application are correctly ordered and independently guarded.
• Earlier review findings are resolved at current HEAD d9cc6b5; no stale findings were repeated.
• The focused suite passes 105/105 tests, git diff --check is clean, and no premature version or changelog change is present.

Reviewed by GPT-5.6 Sol

This is an automated review.

[LeanAndMean]

Independent Review Assessment

Assessing review comment: #464 (comment)

Classifications

F1 — Genuine issue: The “authorizes creation once” test does not enforce exactly one structured confirmation prompt. The command requires one batch-scoped confirmation at packages/scramjet/mach12/commands/mach12:pr-review-assessment.md:155, but the test only checks that type: "confirm" appears at packages/scramjet/tests/mach12-wiring.test.ts:930. Replace the presence assertion with a cardinality assertion confirming exactly one such instruction in deferredSection.

F2 — Genuine issue: Tests do not pin canonical issue identity validation or its required failure outcome. The command correctly requires a positive integer number, non-empty canonical URL, no metadata or retry after validation failure, a “may have succeeded” warning, and non-completed status at packages/scramjet/mach12/commands/mach12:pr-review-assessment.md:165-175, but packages/scramjet/tests/mach12-wiring.test.ts:937-952 verifies only command ordering. Add semantic assertions for each predicate and failure outcome while retaining the existing create → view → edit ordering checks.

S1 — Genuine issue: Failed label lookup or creation should identify the failed operation and provide concise error context in CLI output. The branches at packages/scramjet/mach12/commands/mach12:pr-review-assessment.md:154-156 continue without the label but do not require surfacing the underlying command failure, which can hide authentication, permission, network, or malformed-response diagnostics. Require operation-specific concise error context while preserving non-blocking issue publication, and pin that behavior in semantic tests.

Staged Implementation Plan

Stage 1 — Strengthen authorization and identity-validation contracts (F1, F2)

Required before merge.

• Assert exactly one structured confirmation instruction in deferredSection.
• Assert the positive-integer number and non-empty canonical URL requirements.
• Assert that identity-validation failure forbids labelling and creation retry, warns that creation may have succeeded, and returns a non-completed status.
• Affected file: packages/scramjet/tests/mach12-wiring.test.ts

Stage 2 — Surface deferred-label operation failures (S1)

Required before merge.

• Require operation-specific concise error context for failed label lookup and label creation without blocking issue publication.
• Pin both diagnostic requirements in semantic contract tests.
• Affected files:
  • packages/scramjet/mach12/commands/mach12:pr-review-assessment.md
  • packages/scramjet/tests/mach12-wiring.test.ts

Assessed by GPT-5.6 Sol

[LeanAndMean]

Addressed the selected findings from originating review ID 5181223641.

• F1: enforced exactly one batch-scoped structured confirmation instruction.
• F2: pinned positive canonical issue identity requirements and the no-label, no-retry, may-have-succeeded, non-completed validation-failure outcome.
• S1: required operation-specific concise CLI error context for failed label lookup and label creation while preserving unlabelled issue publication.

Commit: 430e8cb (Strengthen deferred issue contract coverage)
Verification: 105 focused tests passed; Biome and git diff --check passed; scoped correctness review found no actionable issues.