fix(noema-review): bound both jobs to a job-level timeout-minutes #1715
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,105 @@ | ||
| name: Source Fix PR 1715 No Model Job Timeout | ||
|
|
||
| on: | ||
| push: | ||
| branches: | ||
| - fix/noema-review-job-timeout-minutes | ||
| paths: | ||
| - scripts/ci/source_fix_pr1715_no_model_job_timeout.py | ||
| - .github/workflows/source-fix-pr1715-no-model-job-timeout.yml | ||
|
|
||
| concurrency: | ||
| group: source-fix-pr1715-${{ github.repository }}-${{ github.ref_name }} | ||
| cancel-in-progress: true | ||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
||
| jobs: | ||
| repair: | ||
| runs-on: ubuntu-slim | ||
| steps: | ||
| - name: Checkout exact writer head | ||
| uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 | ||
| with: | ||
| ref: ${{ github.sha }} | ||
| fetch-depth: 0 | ||
| persist-credentials: false | ||
|
|
||
| - name: Revalidate exact remote head | ||
| shell: bash | ||
| run: | | ||
| set -euo pipefail | ||
| remote_head="$(git ls-remote origin refs/heads/fix/noema-review-job-timeout-minutes | cut -f1)" | ||
| test -n "$remote_head" | ||
| test "$remote_head" = "$GITHUB_SHA" | ||
|
|
||
| - name: Set up Python 3.14 | ||
| uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 | ||
| with: | ||
| python-version: "3.14" | ||
| cache: pip | ||
|
|
||
| - name: Install exact test toolchain | ||
| shell: bash | ||
| run: | | ||
| set -euo pipefail | ||
| python -m pip install --require-hashes -r requirements-opencode-review-ci-hashes.txt | ||
|
|
||
| - name: Apply causal-owner repair | ||
| shell: bash | ||
| run: | | ||
| set -euo pipefail | ||
| python scripts/ci/source_fix_pr1715_no_model_job_timeout.py | ||
| python -m py_compile scripts/ci/source_fix_pr1715_no_model_job_timeout.py | ||
| git diff --check | ||
|
|
||
| - name: Verify Noema timeout authority contract | ||
| shell: bash | ||
| run: | | ||
| set -euo pipefail | ||
| python -m pytest \ | ||
| tests/test_noema_orchestrator_workflow_contract.py \ | ||
| tests/test_required_workflow_queue_contract.py \ | ||
| tests/test_noema_review_gate.py \ | ||
| tests/test_noema_review_handoff.py \ | ||
| tests/test_noema_two_phase_handoff.py \ | ||
| -q | ||
| python -m compileall -q scripts tests .github/actions/noema-review | ||
| git diff --check | ||
|
|
||
| - name: Retire one-shot repair artifacts and verify scope | ||
| shell: bash | ||
| run: | | ||
| set -euo pipefail | ||
| rm scripts/ci/source_fix_pr1715_no_model_job_timeout.py | ||
| rm .github/workflows/source-fix-pr1715-no-model-job-timeout.yml | ||
| allowed='^(.github/workflows/noema-review.yml|tests/test_noema_orchestrator_workflow_contract.py|CHANGELOG.md|docs/product-technical-gap-baseline.md|scripts/ci/source_fix_pr1715_no_model_job_timeout.py|.github/workflows/source-fix-pr1715-no-model-job-timeout.yml)$' | ||
| bad="$(git status --short | sed -E 's/^.. //' | grep -Ev "$allowed" || true)" | ||
| test -z "$bad" | ||
| test ! -e scripts/ci/source_fix_pr1715_no_model_job_timeout.py | ||
| test ! -e .github/workflows/source-fix-pr1715-no-model-job-timeout.yml | ||
| remote_head="$(git ls-remote origin refs/heads/fix/noema-review-job-timeout-minutes | cut -f1)" | ||
| test "$remote_head" = "$GITHUB_SHA" | ||
|
Comment on lines
+71
to
+83
Contributor
There was a problem hiding this comment. 🔍 Reviewed head remains transitional The one-shot workflow defers the final contract to a later bot commit. Merge evidence generated before that push validates a tree the workflow intends to replace. 
Was this helpful? React with 👍 or 👎 to provide feedback. |
||
|
|
||
| - name: Publish normal non-force repair commit | ||
| env: | ||
| PRIMARY_PUSH_TOKEN: ${{ secrets.PR_REVIEW_MERGE_TOKEN }} | ||
| FALLBACK_PUSH_TOKEN: ${{ secrets.OPENCODE_APPROVE_TOKEN }} | ||
| shell: bash | ||
| run: | | ||
| set -euo pipefail | ||
| workflow_push_token="${PRIMARY_PUSH_TOKEN:-${FALLBACK_PUSH_TOKEN:-}}" | ||
| if [ -z "$workflow_push_token" ]; then | ||
| echo "::error::No workflow-starting mutation credential is configured; refusing github.token publication." | ||
| exit 1 | ||
| fi | ||
| remote_head="$(git ls-remote origin refs/heads/fix/noema-review-job-timeout-minutes | cut -f1)" | ||
| test "$remote_head" = "$GITHUB_SHA" | ||
| git config user.name "github-actions[bot]" | ||
| git config user.email "41898282+github-actions[bot]@users.noreply.github.com" | ||
| git add -A | ||
| git diff --cached --check | ||
| git commit -m "fix(noema-review): remove model wall-clock termination" | ||
| git remote set-url origin "https://x-access-token:${workflow_push_token}@github.com/${GITHUB_REPOSITORY}.git" | ||
| git push origin HEAD:fix/noema-review-job-timeout-minutes | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,110 @@ | ||
| """One-shot exact-head repair for PR #1715's Noema model timeout contract.""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import re | ||
| from pathlib import Path | ||
|
|
||
| WORKFLOW = Path(".github/workflows/noema-review.yml") | ||
| TEST = Path("tests/test_noema_orchestrator_workflow_contract.py") | ||
| CHANGELOG = Path("CHANGELOG.md") | ||
| BASELINE = Path("docs/product-technical-gap-baseline.md") | ||
|
|
||
|
|
||
| def replace_once(text: str, old: str, new: str, label: str) -> str: | ||
| """Replace one literal block and fail closed when branch contents moved.""" | ||
| count = text.count(old) | ||
| if count != 1: | ||
| raise SystemExit(f"PR1715 {label}: expected one literal block, found {count}") | ||
| return text.replace(old, new, 1) | ||
|
Comment on lines
+14
to
+19
Contributor
There was a problem hiding this comment. 🔍 Repair helper bypasses coverage gate No test executes the new
Was this helpful? React with 👍 or 👎 to provide feedback. |
||
|
|
||
|
|
||
| def patch_workflow() -> None: | ||
| """Keep bounded cleanup but remove elapsed-time authority from model work.""" | ||
| text = WORKFLOW.read_text(encoding="utf-8") | ||
| old = ''' # Bound this job well short of GitHub's 360-minute platform default. Its | ||
| # "Prepare Noema model verdict" step calls into two_phase.py's call_llm | ||
| # via the same contextual-orchestrator gateway whose unbounded wait was | ||
| # confirmed to stall runs for 7-20 hours in opencode-review.yml before | ||
| # PR #1707's fix -- and noema_review_gate.py's own comment says that | ||
| # step "remains governed by contextual-orchestrator rather than a fixed | ||
| # inference timeout", so nothing upstream of this job bounds it either. | ||
| # 210 minutes gives that step the same ~180-minute (3-hour) allowance | ||
| # PR #1707 set for its analogous model-wait deadline -- comfortably | ||
| # above this org's documented "accommodate over 2 hours per model" | ||
| # policy (docs/product-goal-directive.md #8) -- plus a 30-minute buffer | ||
| # for this job's other steps (tarball fetch, credential mint, the | ||
| # superseded-run cleanup sweep, visibility-lookup retries, sidecar | ||
| # provisioning, publication), while staying well under GitHub's default. | ||
| timeout-minutes: 210 | ||
| ''' | ||
| new = ''' # Model-backed Noema intentionally has no job-level wall-clock timeout. | ||
| # contextual-orchestrator/orchestrator/free owns provider termination; | ||
| # GitHub admission must not stop reasoning, streaming, or tool work only | ||
| # because elapsed time crossed a repository-side deadline. Stale heads, | ||
| # closed/draft PRs, provider completion, and explicit cancellation remain | ||
| # authoritative termination signals. The non-model cleanup job above is | ||
| # independently bounded because it performs only GitHub API housekeeping. | ||
| ''' | ||
| WORKFLOW.write_text( | ||
| replace_once(text, old, new, "model job timeout block"), encoding="utf-8" | ||
| ) | ||
|
|
||
|
|
||
| def patch_test() -> None: | ||
| """Replace the stale timeout-positive assertion with the owner contract.""" | ||
| text = TEST.read_text(encoding="utf-8") | ||
| marker = "def test_noema_review_job_has_a_bounded_runtime_above_the_two_hour_model_allowance() -> None:\n" | ||
| start = text.find(marker) | ||
| if start < 0 or text.find(marker, start + 1) >= 0: | ||
| raise SystemExit("PR1715 stale model-timeout test marker moved or duplicated") | ||
| replacement = '''def test_noema_review_model_job_has_no_elapsed_time_termination() -> None: | ||
| """Model-backed Noema delegates termination to orchestrator/provider authority.""" | ||
| workflow = workflow_text("noema-review.yml") | ||
| job = workflow.split(" noema-review:\\n", 1)[1] | ||
|
|
||
| assert re.search(r"^ timeout-minutes:", job, flags=re.MULTILINE) is None | ||
| assert "contextual-orchestrator/orchestrator/free" in workflow | ||
| assert "Model-backed Noema intentionally has no job-level wall-clock timeout" in job | ||
| assert "timeout-minutes: 20" in workflow.split( | ||
| " cancel-closed-pr-runs:\\n", 1 | ||
| )[1].split("\\n noema-review:\\n", 1)[0] | ||
| ''' | ||
| TEST.write_text(text[:start] + replacement, encoding="utf-8") | ||
|
|
||
|
|
||
| def append_traceability() -> None: | ||
| """Record why support housekeeping may be bounded while model work may not.""" | ||
| changelog_note = ( | ||
| "\n- PR #1715: keep the non-model Noema close-cleanup job bounded, but remove " | ||
| "the proposed 210-minute job timeout from model-backed `noema-review`; " | ||
| "`orchestrator/free`/provider completion, live PR/head state, or explicit " | ||
| "cancellation are the termination authorities rather than elapsed time.\n" | ||
| ) | ||
| changelog = CHANGELOG.read_text(encoding="utf-8") | ||
| if "PR #1715: keep the non-model Noema close-cleanup job bounded" not in changelog: | ||
| CHANGELOG.write_text(changelog + changelog_note, encoding="utf-8") | ||
|
|
||
| baseline_note = ''' | ||
|
|
||
| ### Noema model-job timeout authority — PR #1715 | ||
|
|
||
| - **Root cause:** a queue-operability repair proposed `timeout-minutes: 210` on the model-backed `noema-review` job, turning elapsed wall time into an admission/model termination authority. | ||
| - **Contract:** the lightweight closed-PR Actions cleanup remains bounded, while Noema model work has no repository-owned wall-clock cutoff. `orchestrator/free` and its upstream provider own normal model completion; live PR/head validation, provider end, or explicit cancellation remain authoritative stop conditions. | ||
| - **Regression:** `test_noema_review_model_job_has_no_elapsed_time_termination` rejects a job-level timeout on the model job while retaining the 20-minute bound on non-model cleanup. | ||
| - **Status:** Implemented on the PR #1715 writer branch; exact-head CI/review must be regenerated after the one-shot repair commit. | ||
| ''' | ||
| baseline = BASELINE.read_text(encoding="utf-8") | ||
| if "### Noema model-job timeout authority — PR #1715" not in baseline: | ||
| BASELINE.write_text(baseline + baseline_note, encoding="utf-8") | ||
|
|
||
|
|
||
| def main() -> None: | ||
| """Apply the minimal owner repair and its permanent regression/docs.""" | ||
| patch_workflow() | ||
| patch_test() | ||
| append_traceability() | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| main() | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📝 Info: Cleanup timeout scope
The
cancel-closed-pr-runsdeadline wraps only a bounded three-pass status sweep. It never covers model inference, so the no-fixed-inference-timeout contract does not apply.Was this helpful? React with 👍 or 👎 to provide feedback.