Route a failing required-work gate step instead of crashing on correction_rounds.gate_type #841

Closed
opened 2026-09-30 13:35:21 +00:00 by jared · 4 comments
Owner

Problem

A failed gate step with no violation parser crashes the gate. The Runner ends the run with verdict:error and does not route the failure to repair.

Failed step Parser category (StepOutcome#gate_type) Current result Correct result
lint, test-lint "lint" FAIL, round recorded, repair same
test, diff-test, suite "test" FAIL, round recorded, repair same
required-work nil verdict:error, implementation_failed FAIL, no round, map fail edge (suite-check → programmer-repair)
retired-tests nil verdict:error, implementation_failed FAIL, no round, map fail edge (test-writer)

Evidence:

plugins/os-sdlc/lib/os_sdlc/model/gate/step_outcome.rb:17-22        gate_type returns nil for any step outside LINT_STEPS and TEST_STEPS
plugins/os-sdlc/lib/os_sdlc/command/gate/violation_recorder.rb:20    record_round(outcome) if %i[failed broken].include?(classification)
plugins/os-sdlc/lib/os_sdlc/command/gate/violation_recorder.rb:35-37 record_correction_round!(gate_type: outcome.gate_type, ...); PARSERS.fetch(outcome.gate_type)
plugins/os-sdlc/lib/os_sdlc/adapter/db.rb:214                        t.String :gate_type, null: false
plugins/os-sdlc/lib/os_sdlc/command/gate/run.rb:24-27,107-111        rescue -> record_failure -> Verdict::ERROR, raise Failed

What happened (implementation 4, #836, dispatch 30; implementation 10, #843, dispatch 61):

  1. A required-work (or retired-tests) step exits non-zero. StepOutcome#classify returns :failed.
  2. ViolationRecorder#call calls record_round. The insert sends gate_type: nil.
  3. SQLite raises Sequel::NotNullConstraintViolation: NOT NULL constraint failed: correction_rounds.gate_type.
  4. Gate::Run#call rescues it and writes Verdict::ERROR on a synthetic gate step.

What should have happened: Gate#verdict returns FAIL for the step. It uses @last_classification only and does not read the correction_rounds row. The map fail edge routes the failure.

Out of this ticket: the Errno::ENOENT for a slash-command check (/os-adr:find). ADR-0255 (#843) replaces shell-run checks with typed present/absent checks, which removes that crash class.

Approach

ViolationRecorder records a correction round only when the failed step has a parser category. A failed step with gate_type == nil records its step_result as today, and no round.

# before (violation_recorder.rb:20)
record_round(outcome) if %i[failed broken].include?(classification)

# after
record_round(outcome) if %i[failed broken].include?(classification) && outcome.gate_type

gate_type keeps its ADR-0149 meaning: a parser category (lint or test), not a gate node name. Do not store the step name in gate_type. Do not relax the NOT NULL column.

No other reader depends on a round for a failed dispatch. The repair brief (query/brief/facts.rb:52-69) reads step_results, so the retry brief still shows the required-work output. lesson_relay (run.rb:101) accepts an empty violation set. Loop bounds use LoopLedger and ProgressWitness, not rounds.

Amend ADR-0149 with one line: only a step with a parser category records a correction round. Amend ADR-0244 with one line: a failed required-work step records no correction round and routes through the suite-check fail edge.

Required work: none beyond the acceptance criteria.

Acceptance criteria

  1. In tests/command/gate/violation_recorder_test.rb, a failed required-work step (exit 1, classification :failed) records no correction round and raises no error.
  2. In the same file, a failed retired-tests step records no correction round and raises no error.
  3. A gate run whose required-work step fails returns a FAIL verdict on step required-work, not verdict:error on step gate.
  4. The existing lint and test round-recording tests in violation_recorder_test.rb stay green without edits.
  5. ADR-0149 and ADR-0244 each carry the one-line amendment named in Approach.
  6. docs/implementation-status/os-sdlc.md has one History row for #841.

Owner sketch

OsSdlc::Command::Gate::ViolationRecorder#call owns the decision to record a round. StepOutcome#gate_type stays the single source for the parser category. Gate#verdict and the map fail edges own routing; they do not change.

VERDICT: CREATE
REASON: A complete, in-policy /os-sdlc:implement run hit this. Any failing required-work step crashes the gate (correction_rounds.gate_type NOT NULL) instead of routing to a correction round. The Origin chain ends in DESIGN, not IMPROVISATION, this ticket is the root, and it blocks #836.

## Problem A failed gate step with no violation parser crashes the gate. The Runner ends the run with `verdict:error` and does not route the failure to repair. | Failed step | Parser category (`StepOutcome#gate_type`) | Current result | Correct result | | --- | --- | --- | --- | | `lint`, `test-lint` | `"lint"` | FAIL, round recorded, repair | same | | `test`, `diff-test`, `suite` | `"test"` | FAIL, round recorded, repair | same | | `required-work` | `nil` | `verdict:error`, `implementation_failed` | FAIL, no round, map `fail` edge (suite-check → programmer-repair) | | `retired-tests` | `nil` | `verdict:error`, `implementation_failed` | FAIL, no round, map `fail` edge (test-writer) | Evidence: ``` plugins/os-sdlc/lib/os_sdlc/model/gate/step_outcome.rb:17-22 gate_type returns nil for any step outside LINT_STEPS and TEST_STEPS plugins/os-sdlc/lib/os_sdlc/command/gate/violation_recorder.rb:20 record_round(outcome) if %i[failed broken].include?(classification) plugins/os-sdlc/lib/os_sdlc/command/gate/violation_recorder.rb:35-37 record_correction_round!(gate_type: outcome.gate_type, ...); PARSERS.fetch(outcome.gate_type) plugins/os-sdlc/lib/os_sdlc/adapter/db.rb:214 t.String :gate_type, null: false plugins/os-sdlc/lib/os_sdlc/command/gate/run.rb:24-27,107-111 rescue -> record_failure -> Verdict::ERROR, raise Failed ``` What happened (implementation 4, #836, dispatch 30; implementation 10, #843, dispatch 61): 1. A `required-work` (or `retired-tests`) step exits non-zero. `StepOutcome#classify` returns `:failed`. 2. `ViolationRecorder#call` calls `record_round`. The insert sends `gate_type: nil`. 3. SQLite raises `Sequel::NotNullConstraintViolation: NOT NULL constraint failed: correction_rounds.gate_type`. 4. `Gate::Run#call` rescues it and writes `Verdict::ERROR` on a synthetic `gate` step. What should have happened: `Gate#verdict` returns FAIL for the step. It uses `@last_classification` only and does not read the `correction_rounds` row. The map `fail` edge routes the failure. Out of this ticket: the `Errno::ENOENT` for a slash-command check (`/os-adr:find`). ADR-0255 (#843) replaces shell-run checks with typed `present`/`absent` checks, which removes that crash class. ## Approach `ViolationRecorder` records a correction round only when the failed step has a parser category. A failed step with `gate_type == nil` records its step_result as today, and no round. ```ruby # before (violation_recorder.rb:20) record_round(outcome) if %i[failed broken].include?(classification) # after record_round(outcome) if %i[failed broken].include?(classification) && outcome.gate_type ``` `gate_type` keeps its ADR-0149 meaning: a parser category (`lint` or `test`), not a gate node name. Do not store the step name in `gate_type`. Do not relax the NOT NULL column. No other reader depends on a round for a failed dispatch. The repair brief (`query/brief/facts.rb:52-69`) reads step_results, so the retry brief still shows the required-work output. `lesson_relay` (`run.rb:101`) accepts an empty violation set. Loop bounds use `LoopLedger` and `ProgressWitness`, not rounds. Amend ADR-0149 with one line: only a step with a parser category records a correction round. Amend ADR-0244 with one line: a failed required-work step records no correction round and routes through the suite-check `fail` edge. Required work: none beyond the acceptance criteria. ## Acceptance criteria 1. In `tests/command/gate/violation_recorder_test.rb`, a failed `required-work` step (exit 1, classification `:failed`) records no correction round and raises no error. 2. In the same file, a failed `retired-tests` step records no correction round and raises no error. 3. A gate run whose `required-work` step fails returns a FAIL verdict on step `required-work`, not `verdict:error` on step `gate`. 4. The existing lint and test round-recording tests in `violation_recorder_test.rb` stay green without edits. 5. ADR-0149 and ADR-0244 each carry the one-line amendment named in Approach. 6. `docs/implementation-status/os-sdlc.md` has one History row for #841. ## Owner sketch `OsSdlc::Command::Gate::ViolationRecorder#call` owns the decision to record a round. `StepOutcome#gate_type` stays the single source for the parser category. `Gate#verdict` and the map `fail` edges own routing; they do not change. VERDICT: CREATE REASON: A complete, in-policy /os-sdlc:implement run hit this. Any failing required-work step crashes the gate (correction_rounds.gate_type NOT NULL) instead of routing to a correction round. The Origin chain ends in DESIGN, not IMPROVISATION, this ticket is the root, and it blocks #836.
Author
Owner

Scope additions from review (Codex second opinion, 2026-09-30):

  • retired-tests has the same gap. The test-writer gate is [diff-test, test-lint, retired-tests] (poodr-ticket-implementation.yaml:53). A failed retired-tests step reaches ViolationRecorder#record_round through normal gate settlement (gate.rb:65) and hits the same NOT NULL crash.
  • Fix direction: skip violation recording when a failed step has no parser (StepOutcome#gate_type is nil). Do not store the node name in gate_type. ADR-0149 limits it to lint and test, and those are parser categories, not gate nodes.
  • Tests: add a failure-recording test for each of required-work and retired-tests.
  • The slash-command check is a ticket-authoring problem. The Required-work parser accepts any non-empty backticked text (required_work.rb:10). A separate ticket covers the scope rule and the rejection of / checks at implementation-open.
Scope additions from review (Codex second opinion, 2026-09-30): - `retired-tests` has the same gap. The test-writer gate is `[diff-test, test-lint, retired-tests]` (poodr-ticket-implementation.yaml:53). A failed `retired-tests` step reaches `ViolationRecorder#record_round` through normal gate settlement (gate.rb:65) and hits the same NOT NULL crash. - Fix direction: skip violation recording when a failed step has no parser (`StepOutcome#gate_type` is nil). Do not store the node name in `gate_type`. ADR-0149 limits it to `lint` and `test`, and those are parser categories, not gate nodes. - Tests: add a failure-recording test for each of `required-work` and `retired-tests`. - The slash-command check is a ticket-authoring problem. The Required-work parser accepts any non-empty backticked text (required_work.rb:10). A separate ticket covers the scope rule and the rejection of `/` checks at `implementation-open`.
Author
Owner

Decomposed 2026-09-30. Scope narrowed to the gate_type crash; the slash-command ENOENT half is covered by ADR-0255 (#843). Blast radius: no pinning tests to retire; ADR-0149 and ADR-0244 get one-line amendments.

Decomposed 2026-09-30. Scope narrowed to the gate_type crash; the slash-command ENOENT half is covered by ADR-0255 (#843). Blast radius: no pinning tests to retire; ADR-0149 and ADR-0244 get one-line amendments.
Author
Owner

Work started via /os-sdlc:implement on branch ticket-841.

Work started via /os-sdlc:implement on branch ticket-841.
Author
Owner

Resolution

Done: ViolationRecorder records a correction round only when the failed step has a parser category. Failed required-work and retired-tests steps return FAIL on their step and route by the map fail edge, instead of verdict:error on correction_rounds.gate_type NOT NULL. ADR-0149 and ADR-0244 amended; os-sdlc History row added.

Evidence: Branch ticket-841: da7c2643 (guard, two recorder tests, run_required_work_test.rb), 1fdfeefb (ADR amendments, History row). Implementation 11 complete: suite-check gate required-work exit 0, suite exit 0.

Follow-ups: none (repair-node required-work gating declined by user 2026-09-30)

## Resolution **Done:** ViolationRecorder records a correction round only when the failed step has a parser category. Failed required-work and retired-tests steps return FAIL on their step and route by the map fail edge, instead of verdict:error on correction_rounds.gate_type NOT NULL. ADR-0149 and ADR-0244 amended; os-sdlc History row added. **Evidence:** Branch ticket-841: da7c2643 (guard, two recorder tests, run_required_work_test.rb), 1fdfeefb (ADR amendments, History row). Implementation 11 complete: suite-check gate required-work exit 0, suite exit 0. **Follow-ups:** none (repair-node required-work gating declined by user 2026-09-30)
jared closed this issue 2026-09-30 17:25:58 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
jared/cc-os#841
No description provided.