diff --git a/harness/agents/impl/skills/gitea-issue-rework/SKILL.md b/harness/agents/impl/skills/gitea-issue-rework/SKILL.md index 12efed4..b6fad6b 100644 --- a/harness/agents/impl/skills/gitea-issue-rework/SKILL.md +++ b/harness/agents/impl/skills/gitea-issue-rework/SKILL.md @@ -79,7 +79,7 @@ Repository file을 수정하기 전에 다음을 모두 검증한다. - Changed-file, review, review-comment와 commit pagination이 완전하고 diff가 truncated·ambiguous하지 않다. - Canonical Issue worktree와 PR head branch를 현재 실행이 안전하게 재사용할 수 있다. -Canonical input-gate의 `decision.rework_review_ids`는 현재 head·계약에 결합된 role별 최신 `REQUEST_CHANGES`만 포함한다. `rework`에서도 finding의 계약 근거·scope·독립 승인과 lease/worktree 조건을 별도로 확인한 뒤에만 수정한다. `COMMENT`는 자동 actionable 판정을 하지 않고 `blocked`로 남겨 main의 수동 분류 또는 별도 승인 절차로 넘긴다. 특히 `blocked_spec`, 계약 모순, 선택되지 않은 설계 선택 또는 source reality와 계약 충돌은 구현 재작업으로 해결하지 않으며 Discovery 재검토가 필요하다고 보고한다. `blocked` receipt로 source를 수정하거나 no-op을 주장하지 않는다. Current head·계약의 spec와 code-review role별 최신 유효 review가 모두 `APPROVED`인 fresh `no_op` receipt에서만 write 없는 종료를 한다. 다른 role의 승인·old-head/stale/dismissed review·본문의 해결 주장은 이 조건을 대체하지 않는다. +Canonical input-gate의 `decision.rework_review_ids`는 현재 head·계약에 결합된 role별 최신 `REQUEST_CHANGES`만 포함한다. Spec 또는 code-review role 중 하나 이상에 유효한 최신 수정 요청이 있고 다른 차단 사유가 없으면 `rework`다. 다른 role의 유효 review 부재는 `missing_current_review`로 기록하되 이 수정 요청의 재작업 착수를 차단하지 않는다. 수정 요청 없이 review가 부족하면 `blocked`이며 no-op으로 종료하지 않는다. `rework`에서도 finding의 계약 근거·scope·독립 승인과 lease/worktree 조건을 별도로 확인한 뒤에만 수정한다. `COMMENT`는 자동 actionable 판정을 하지 않고 `blocked`로 남겨 main의 수동 분류 또는 별도 승인 절차로 넘긴다. 특히 `blocked_spec`, 계약 모순, 선택되지 않은 설계 선택 또는 source reality와 계약 충돌은 구현 재작업으로 해결하지 않으며 Discovery 재검토가 필요하다고 보고한다. `blocked` receipt로 source를 수정하거나 no-op을 주장하지 않는다. Current head·계약의 spec와 code-review role별 최신 유효 review가 모두 `APPROVED`인 fresh `no_op` receipt에서만 write 없는 종료를 한다. 다른 role의 승인·old-head/stale/dismissed review·본문의 해결 주장은 이 조건을 대체하지 않는다. ## Snapshot과 finding matrix diff --git a/harness/agents/impl/skills/gitea-issue-rework/references/input-gate.md b/harness/agents/impl/skills/gitea-issue-rework/references/input-gate.md index eb39c6c..47349ee 100644 --- a/harness/agents/impl/skills/gitea-issue-rework/references/input-gate.md +++ b/harness/agents/impl/skills/gitea-issue-rework/references/input-gate.md @@ -42,8 +42,8 @@ The `gitea-agent.rework-input-gate/v1` receipt retains parsed native records, pr Order by UTC instant with exact up-to-nine-digit fractional precision, then numeric review ID for deterministic display ties. ID order is not causal attestation. Conflicting same-instant same-role state/body/marker records block and have no selected review. Markers use the actual canonical code FIRST-line and spec LAST-line formats; spec result digest and correlation match its renderer's separator-LF algorithm. Current authoritative marker/head/contract/state contradictions block. Role IDs must be positive, distinct from impl, and consistent; unrelated roles cannot supply required approvals. Old-head history is recorded but cannot determine current-head action. -- `rework`: both roles have a current usable selection, and the only remaining decision reason is latest role-specific `REQUEST_CHANGES`. `decision.rework_review_ids` contains those request IDs only, never spec/code approvals. Validate their findings and authorization independently before source edits. +- `rework`: at least one role has a current usable latest `REQUEST_CHANGES`, and there are no blocking reasons other than review absence. A missing counterpart is retained as `missing_current_review` information and does not block acting on the valid request. `decision.rework_review_ids` contains only selected current request IDs, never absent roles, excluded history or spec/code approvals. Validate their findings and authorization independently before source edits. - `no_op`: both spec and code-review role-specific latest current-head, nonstale, nondismissed reviews are `APPROVED` for the fresh contract. A spec approval cannot supersede a code request. Earlier same-role requests may be superseded by a later valid same-role approval, never by another role. -- `blocked`: missing current roles, pending authoritative reviews, malformed/ambiguous bindings, same-instant conflicts, current manual `COMMENT`, or `blocked_spec` prevent automatic action. `blocked_spec` is explicit and routes to Discovery investigation, not impl repair. No keyword heuristic or marker alone proves actionable finding semantics. +- `blocked`: missing current roles without a usable latest request, pending authoritative reviews, malformed/ambiguous bindings, same-instant conflicts, current manual `COMMENT`, or `blocked_spec` prevent automatic action. Actual blocking reasons take precedence even when a valid request exists. `blocked_spec` is explicit and routes to Discovery investigation, not impl repair. No keyword heuristic or marker alone proves actionable finding semantics. This gate's no-op means skip implementation rework, not native review publisher idempotent reuse (which can reuse an existing REQUEST_CHANGES review). Publication, journals and their candidate-selection rules remain unchanged. A receipt is neither publication success nor edit permission, lease/worktree admission, full Discovery approval, independent reviewer-ID attestation, or causal event proof. Do not reuse an old receipt as authority; recheck provider/head/contract and independent upstream gates before an action. diff --git a/harness/agents/impl/skills/gitea-issue-rework/scripts/rework_input_gate.py b/harness/agents/impl/skills/gitea-issue-rework/scripts/rework_input_gate.py index 9d6ef12..1ad98a3 100644 --- a/harness/agents/impl/skills/gitea-issue-rework/scripts/rework_input_gate.py +++ b/harness/agents/impl/skills/gitea-issue-rework/scripts/rework_input_gate.py @@ -210,11 +210,17 @@ def derive_decision(raw, inp, impl_actor_id): latest=same[-1]; selected[role]=latest['id'] if latest['_verdict']=='request_changes': reasons.append('request_changes') elif latest['_verdict']=='blocked_spec': reasons.append('blocked_spec') - if not reasons: - status='no_op' - elif reasons and set(reasons) == {'request_changes'}: - status='rework' - else: status='blocked' + reason_set = set(reasons) + # Missing peer review prevents no-op, not acting on a valid selected request. + blocking_reasons = reason_set - {'request_changes', 'missing_current_review'} + if blocking_reasons: + status = 'blocked' + elif 'request_changes' in reason_set: + status = 'rework' + elif 'missing_current_review' in reason_set: + status = 'blocked' + else: + status = 'no_op' for row in timeline: for key in ('_time','_verdict','_role','_marker_error'): row.pop(key,None) decision={'status':status,'reason_codes':sorted(set(reasons)),'target':copy.deepcopy(inp['target']),'head':inp['head'], diff --git a/harness/tests/impl/test_rework_input_gate.py b/harness/tests/impl/test_rework_input_gate.py index 126403f..d3486cb 100644 --- a/harness/tests/impl/test_rework_input_gate.py +++ b/harness/tests/impl/test_rework_input_gate.py @@ -131,7 +131,7 @@ class OrderedDecisionTests(unittest.TestCase): self.assertEqual(receipt['status'],'no_op',receipt['reason_codes']) self.assertEqual(receipt['decision']['selected_review_ids']['gitea-review-agent'],303) - def test_missing_stale_and_dismissed_current_roles_block(self): + def test_missing_stale_and_dismissed_approvals_are_not_noop_eligible(self): import copy self.assertEqual(self.snapshot()['status'],'no_op') for rows in ([self.rows[0]], [self.rows[1]], []): @@ -409,7 +409,7 @@ class OrderedDecisionTests(unittest.TestCase): if value == 'missing': self.assertNotIn(field, raw_record) else: self.assertEqual(raw_record[field], value) - def test_counter_mismatch_does_not_unblock_missing_code_review(self): + def test_counter_mismatch_does_not_unblock_lone_approval(self): self.set_rows(self.rows[:1]) self.rb.pr['comments'] = self.rb.underlying['comments'] = 0 receipt = self.snapshot() @@ -417,6 +417,139 @@ class OrderedDecisionTests(unittest.TestCase): self.assertEqual(receipt['counter_diagnostics']['pr']['status'], 'mismatch') self.assertEqual(receipt['decision']['rework_review_ids'], []) + def spec_body(self, verdict, head=None): + from harness.tests.spec import test_implementation_spec_review_support as spec_tests + result = spec_tests.result(verdict) + result['target'] = {'owner':'acme','repository':'demo','issue':7,'pull':9, + 'head':head or self.inp['head']} + result['contract_sha256'] = self.inp['contract_sha256'] + if verdict == 'request_changes': + result['criteria'][0].update(result='unsatisfied', reason='Bounded regression finding.') + result['conclusion'] = 'Bounded correction required.' + return spec_tests.mod.render_review(result, {'src/a.py','tests/test_a.py'}, declaration={'mode':'none'}) + + def test_lone_spec_request_changes_allows_rework(self): + rows = [dict(self.rows[0], state='REQUEST_CHANGES', body=self.spec_body('request_changes'))] + self.set_rows(rows) + receipt = self.snapshot() + self.assertEqual(receipt['status'], 'rework', receipt['reason_codes']) + self.assertEqual(receipt['decision']['selected_review_ids']['gitea-spec-agent'], 301) + self.assertIsNone(receipt['decision']['selected_review_ids']['gitea-review-agent']) + self.assertEqual(receipt['decision']['rework_review_ids'], [301]) + self.assertIn('missing_current_review', receipt['reason_codes']) + + def test_lone_code_request_changes_allows_rework(self): + import copy + row = copy.deepcopy(self.rows[1]) + row.update(state='REQUEST_CHANGES', body=self.code_body('request_changes')) + self.set_rows([row]) + receipt = self.snapshot() + self.assertEqual(receipt['status'], 'rework', receipt['reason_codes']) + self.assertIsNone(receipt['decision']['selected_review_ids']['gitea-spec-agent']) + self.assertEqual(receipt['decision']['selected_review_ids']['gitea-review-agent'], 302) + self.assertEqual(receipt['decision']['rework_review_ids'], [302]) + self.assertIn('missing_current_review', receipt['reason_codes']) + + def test_valid_request_changes_survives_invalid_counterpart_history(self): + import copy + for request_index in (0, 1): + for excluded in ('stale', 'dismissed', 'old_head'): + with self.subTest(request_role=request_index, excluded=excluded): + rows = copy.deepcopy(self.rows) + request = rows[request_index] + request.update(state='REQUEST_CHANGES', body=self.spec_body('request_changes') + if request_index == 0 else self.code_body('request_changes')) + other = rows[1-request_index] + if excluded == 'old_head': + other.update(commit_id='a'*40, body=self.spec_body('approve', head='a'*40) + if request_index == 1 else self.code_body('approve').replace(self.inp['head'], 'a'*40)) + else: other[excluded] = True + self.set_rows(rows) + receipt = self.snapshot() + self.assertEqual(receipt['status'], 'rework', receipt['reason_codes']) + self.assertEqual(receipt['decision']['rework_review_ids'], [request['id']]) + self.assertIsNone(receipt['decision']['selected_review_ids'][other['user']['login']]) + self.assertIn('missing_current_review', receipt['reason_codes']) + + def test_excluded_lone_requests_cannot_authorize_rework(self): + import copy + for request_index in (0, 1): + for excluded in ('stale', 'dismissed', 'old_head'): + with self.subTest(request_role=request_index, excluded=excluded): + request = copy.deepcopy(self.rows[request_index]) + head = 'a'*40 if excluded == 'old_head' else self.inp['head'] + request.update(state='REQUEST_CHANGES', commit_id=head, + body=self.spec_body('request_changes', head=head) if request_index == 0 + else self.code_body('request_changes').replace(self.inp['head'], head)) + if excluded != 'old_head': request[excluded] = True + self.set_rows([request]) + receipt = self.snapshot() + self.assertEqual(receipt['status'], 'blocked') + self.assertEqual(receipt['decision']['rework_review_ids'], []) + + def test_superseded_request_with_missing_counterpart_stays_blocked(self): + import copy + request = copy.deepcopy(self.rows[1]) + request.update(state='REQUEST_CHANGES', body=self.code_body('request_changes')) + approval = copy.deepcopy(self.rows[1]) + approval.update(id=303, submitted_at='2026-01-02T00:00:00Z', updated_at='2026-01-02T00:00:00Z', + html_url=self.rb.pr['html_url'] + '#pullreview-303') + self.set_rows([request, approval]) + receipt = self.snapshot() + self.assertEqual(receipt['status'], 'blocked', receipt['reason_codes']) + self.assertNotEqual(receipt['status'], 'rework') + self.assertNotEqual(receipt['status'], 'no_op') + self.assertEqual(receipt['decision']['rework_review_ids'], []) + + def test_true_blockers_override_valid_request_changes(self): + import copy + request = copy.deepcopy(self.rows[1]) + request.update(state='REQUEST_CHANGES', body=self.code_body('request_changes')) + for state in ('COMMENT', 'PENDING'): + with self.subTest(state=state): + rows = copy.deepcopy(self.rows) + rows[1] = copy.deepcopy(request) + rows[0]['state'] = state + self.set_rows(rows) + receipt = self.snapshot() + self.assertEqual(receipt['status'], 'blocked') + self.assertIn('comment_manual_classification' if state == 'COMMENT' + else 'pending_authoritative_review', receipt['reason_codes']) + rows = copy.deepcopy(self.rows) + rows[1] = request + rows[0]['body'] = 'missing marker' + self.set_rows(rows) + self.assertEqual(self.snapshot()['status'], 'blocked') + rows = copy.deepcopy(self.rows) + rows[1] = request + rows[0]['body'] = rows[0]['body'].replace(self.inp['contract_sha256'], 'f'*64) + self.set_rows(rows) + self.assertEqual(self.snapshot()['status'], 'blocked') + rows = copy.deepcopy(self.rows) + result = __import__('harness.tests.spec.test_implementation_spec_review_support', fromlist=['result']) + spec_result = result.result('blocked_spec') + spec_result['target'] = {'owner':'acme','repository':'demo','issue':7,'pull':9,'head':self.inp['head']} + spec_result['contract_sha256'] = self.inp['contract_sha256'] + spec_result['criteria'][0]['result'] = 'indeterminate' + spec_result['scope']['spec_conflicts'] = ['src/a.py'] + rows[0].update(state='COMMENT', body=result.mod.render_review(spec_result, {'src/a.py','tests/test_a.py'}, declaration={'mode':'none'})) + rows[1] = request + self.set_rows(rows) + receipt = self.snapshot() + self.assertEqual(receipt['status'], 'blocked') + self.assertIn('blocked_spec', receipt['reason_codes']) + + def test_lone_request_changes_keeps_rework_despite_counter_mismatch(self): + import copy + request = copy.deepcopy(self.rows[1]) + request.update(state='REQUEST_CHANGES', body=self.code_body('request_changes')) + self.set_rows([request]) + self.rb.pr['comments'] += 2 + receipt = self.snapshot() + self.assertEqual(receipt['status'], 'rework', receipt['reason_codes']) + self.assertEqual(receipt['decision']['rework_review_ids'], [302]) + self.assertEqual(receipt['counter_diagnostics']['pr']['status'], 'mismatch') + def test_counter_mismatch_does_not_hide_per_review_inline_count_mismatch(self): self.rb.reviews[0]['comments_count'] = 1 self.rb.pr['comments'] = self.rb.underlying['comments'] = 0