From f8a8bc7317a5540f2ba10b50aa1c7a4798d39f89 Mon Sep 17 00:00:00 2001 From: AntonOresten Date: Sat, 19 Sep 2026 19:40:49 +0200 Subject: [PATCH 1/2] CI: Bound upstream reviews independently and retain failure progress Two review attempts exhausted the job deadline without reaching diagnostic upload. Stop only the marked review action before the outer timeout, retain per-commit progress and a non-sensitive process snapshot, and provide a short non-publishing action probe. Failed run: https://github.com/jool-space/joolia/actions/runs/35449954934/attempts/2 Assisted-by: Codex (GPT-6) --- .github/workflows/upstream-sync.yml | 79 ++++++++++++++- contrib/upstream-sync/README.md | 24 ++++- contrib/upstream-sync/prompt.md | 12 +++ contrib/upstream-sync/review_watchdog.py | 97 +++++++++++++++++++ contrib/upstream-sync/test_review_watchdog.py | 75 ++++++++++++++ 5 files changed, 281 insertions(+), 6 deletions(-) create mode 100644 contrib/upstream-sync/review_watchdog.py create mode 100644 contrib/upstream-sync/test_review_watchdog.py diff --git a/.github/workflows/upstream-sync.yml b/.github/workflows/upstream-sync.yml index 7a6277f976fb6..3febbb9d62ef0 100644 --- a/.github/workflows/upstream-sync.yml +++ b/.github/workflows/upstream-sync.yml @@ -6,9 +6,9 @@ on: workflow_dispatch: inputs: mode: - description: Plan only, or ask Luna to prepare a draft sync PR + description: Plan, prepare a sync PR, or probe the review action without publishing type: choice - options: [plan, propose] + options: [plan, propose, probe] default: plan permissions: @@ -29,7 +29,9 @@ env: jobs: plan: - if: github.ref == 'refs/heads/master' && (github.event_name != 'schedule' || vars.JOOLIA_UPSTREAM_SYNC_ENABLED == 'true') + if: >- + (github.ref == 'refs/heads/master' || (github.event_name == 'workflow_dispatch' && inputs.mode == 'probe')) && + (github.event_name != 'schedule' || vars.JOOLIA_UPSTREAM_SYNC_ENABLED == 'true') runs-on: ubuntu-24.04 timeout-minutes: 10 permissions: @@ -62,7 +64,9 @@ jobs: agent: needs: plan - if: needs.plan.outputs.status == 'ready' && (github.event_name == 'schedule' || inputs.mode == 'propose') + if: >- + (needs.plan.outputs.status == 'ready' && (github.event_name == 'schedule' || inputs.mode == 'propose')) || + (github.event_name == 'workflow_dispatch' && inputs.mode == 'probe') runs-on: ubuntu-24.04 timeout-minutes: 40 steps: @@ -83,12 +87,55 @@ jobs: exit 1 fi - name: Prepare the pinned source tree and trusted prompt + if: inputs.mode != 'probe' run: | UPSTREAM_SHA=$(python3 -c 'import json,os; print(json.load(open(os.environ["SYNC_DIR"]+"/plan.json"))["fetched_sha"])') git fetch --no-tags https://github.com/JuliaLang/julia.git "$UPSTREAM_SHA" cp contrib/upstream-sync/review.schema.json "$SYNC_DIR/review.schema.json" python3 contrib/upstream-sync/sync.py prepare --out "$SYNC_DIR" + - name: Prepare a minimal action probe + if: inputs.mode == 'probe' + run: | + python3 - <<'PYCODE' + import json, os + from pathlib import Path + folder = Path(os.environ['SYNC_DIR']) + (folder / 'prompt.md').write_text('Do not run tools or inspect files. Return exactly {"status":"ok"}.') + schema = {'type': 'object', 'properties': {'status': {'type': 'string', 'enum': ['ok']}}, + 'required': ['status'], 'additionalProperties': False} + (folder / 'review.schema.json').write_text(json.dumps(schema)) + PYCODE + - name: Arm independent review deadline + env: + WATCH_SECONDS: ${{ inputs.mode == 'probe' && '180' || '1500' }} + run: | + WATCH_DIR=$(sudo mktemp -d /tmp/joolia-review-watchdog.XXXXXX) + sudo chmod 755 "$WATCH_DIR" + sudo install -m 555 contrib/upstream-sync/review_watchdog.py "$WATCH_DIR/watchdog.py" + echo "WATCH_DIR=$WATCH_DIR" >> "$GITHUB_ENV" + sudo -n setsid python3 "$WATCH_DIR/watchdog.py" \ + --marker "$GITHUB_RUN_ID:$GITHUB_RUN_ATTEMPT" --seconds "$WATCH_SECONDS" \ + --status "$WATCH_DIR/status.json" --stop "$SYNC_DIR/watchdog.stop" \ + /dev/null 2>&1 & + WATCH_DIR="$WATCH_DIR" python3 - <<'PYCODE' + import json, os, time + from pathlib import Path + status = Path(os.environ['WATCH_DIR']) / 'status.json' + deadline = time.monotonic() + 10 + while not status.exists(): + if time.monotonic() > deadline: + raise SystemExit('Review watchdog did not start') + time.sleep(0.1) + plan = json.loads((Path(os.environ['SYNC_DIR']) / 'plan.json').read_text()) + progress = Path('.cache/upstream-sync-progress.json') + progress.parent.mkdir(exist_ok=True) + progress.write_text(json.dumps({'target_sha': plan['target_sha'], 'current_sha': None, 'commits': []})) + print('Independent review deadline armed for', os.environ['WATCH_SECONDS'], 'seconds.') + PYCODE - name: Review every commit and adapt the batch + id: review + env: + JOOLIA_REVIEW_MARKER: ${{ github.run_id }}:${{ github.run_attempt }} timeout-minutes: 30 uses: openai/codex-action@86365089eb2b84e0a8fb0717b304f8bdcb13b20e # v1 with: @@ -102,10 +149,29 @@ jobs: prompt-file: ${{ env.SYNC_DIR }}/prompt.md output-schema-file: ${{ env.SYNC_DIR }}/review.schema.json output-file: ${{ env.SYNC_DIR }}/review.json - - name: Export adaptations without executing them + - name: Retain progress and stop the deadline + if: always() + env: + REVIEW_OUTCOME: ${{ steps.review.outcome }} run: | + touch "$SYNC_DIR/watchdog.stop" + if [ -n "${WATCH_DIR:-}" ] && [ -f "$WATCH_DIR/status.json" ]; then + cp "$WATCH_DIR/status.json" "$SYNC_DIR/watchdog.json" + fi + if [ -f .cache/upstream-sync-progress.json ]; then + cp .cache/upstream-sync-progress.json "$SYNC_DIR/progress.json" + fi git add --intent-to-add -- . git diff --binary HEAD > "$SYNC_DIR/adaptations.patch" + python3 - <<'PYCODE' + import json, os + from pathlib import Path + folder = Path(os.environ['SYNC_DIR']) + outcome = os.environ['REVIEW_OUTCOME'] + (folder / 'action-result.json').write_text(json.dumps({'outcome': outcome})) + with open(os.environ['GITHUB_STEP_SUMMARY'], 'a') as summary: + summary.write('Review action outcome: **' + outcome + '**. Progress and watchdog diagnostics are in upstream-review.\n') + PYCODE - name: Retain the review and proposed patch if: always() uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 @@ -115,6 +181,9 @@ jobs: ${{ env.SYNC_DIR }}/review.json ${{ env.SYNC_DIR }}/adaptations.patch ${{ env.SYNC_DIR }}/merge.json + ${{ env.SYNC_DIR }}/progress.json + ${{ env.SYNC_DIR }}/watchdog.json + ${{ env.SYNC_DIR }}/action-result.json if-no-files-found: warn retention-days: 30 diff --git a/contrib/upstream-sync/README.md b/contrib/upstream-sync/README.md index ec3f59877e63d..93f217a4462d7 100644 --- a/contrib/upstream-sync/README.md +++ b/contrib/upstream-sync/README.md @@ -50,7 +50,29 @@ gh variable set JOOLIA_UPSTREAM_SYNC_ENABLED --body true --repo jool-space/jooli Delete that variable or set it to `false` to stop scheduled proposals. Manual runs remain available. This switch controls scheduled proposals and post-merge continuation; automatic merging has a separate switch below. Each new batch permits one agent invocation, -with a 30-minute timeout; this is a runtime bound, not a dollar/token cap. +with an independent 25-minute deadline including action setup. A root-owned +watchdog stops only processes carrying the review action's exact marker, before +the outer job limit, leaving time to upload diagnostics. This is a runtime bound, +not a dollar/token cap. The 30-minute step and 40-minute job limits remain backups. + +A reviewer writes completed per-commit records and its current SHA to an ignored +progress file. On success or failure, `upstream-review` retains that file, the +working patch, action outcome and watchdog process counters. These diagnostics +never substitute for a complete schema-validated review and cannot authorize a +PR or merge. They contain no process arguments, environment contents or Codex +authentication files. A killed runner can still prevent artifact retention. + +To check the action/model path independently of a large review, manually run: + +```sh +gh workflow run upstream-sync.yml --ref master -f mode=probe --repo jool-space/joolia +``` + +The probe asks only for a tiny JSON response, has a three-minute independent +deadline, and never publishes a PR or advances a checkpoint. Maintainers can +also dispatch it on a same-repository workflow branch to validate a fix before +merging. It still makes an API call. Failed review batches are not retried in an +unbounded loop; inspect the artifact before deciding whether to retry. ## What happens diff --git a/contrib/upstream-sync/prompt.md b/contrib/upstream-sync/prompt.md index f8971764d2c9a..99c83a8f03d63 100644 --- a/contrib/upstream-sync/prompt.md +++ b/contrib/upstream-sync/prompt.md @@ -8,6 +8,18 @@ or newly imported files. Do not access credentials, publish, push, open PRs, change remotes, create commits, or edit automation, agent instructions, the checkpoint or reports. The trusted workflow handles Git and publication. +Keep a diagnostic checkpoint at `.cache/upstream-sync-progress.json` (the only +exception to the report-writing restriction above). Before inspecting each +commit, set `current_sha`; after finishing it, append the complete per-commit +review object to `commits`. Preserve `target_sha` and earlier entries. Write via +a temporary file and rename so cancellation cannot leave a half-written JSON. +This ignored file is retained on failure but never authorizes publication or +replaces the final schema-validated review. Do not put credentials in it. + +The independent deadline stops this action after 25 minutes including setup. +Aim to finish within 20 minutes. Avoid exhaustive repository-wide output; read +bounded diffs and relevant surrounding definitions. Do not build or run tests. + For EVERY SHA in the batch's commits list, inspect the individual diff against its first parent, surrounding upstream code and Joolia's corresponding code. Return one structured review per SHA, including indexing implications, affected diff --git a/contrib/upstream-sync/review_watchdog.py b/contrib/upstream-sync/review_watchdog.py new file mode 100644 index 0000000000000..4ccd0f703ad87 --- /dev/null +++ b/contrib/upstream-sync/review_watchdog.py @@ -0,0 +1,97 @@ +#!/usr/bin/env python3 +"""Bound the review action without terminating the Actions worker. + +The workflow runs this from a root-owned directory before drop-sudo. Only +processes with this action's exact environment marker can receive a signal. +Diagnostics contain process identities/counters, never argv or environment. +This is a liveness mechanism, not a security boundary against the reviewer. +""" +import argparse +import json +import os +from pathlib import Path +import signal +import time + +MARKER = 'JOOLIA_REVIEW_MARKER' +KINDS = {'node', 'bash', 'sh', 'sudo', 'codex', 'codex-responses-api-proxy', 'bwrap', 'npm'} + + +def marked_processes(marker, proc=Path('/proc')): + expected = (MARKER + '=' + marker).encode() + found = [] + for entry in proc.iterdir(): + if not entry.name.isdecimal() or int(entry.name) <= 1: + continue + try: + if expected not in (entry / 'environ').read_bytes().split(b'\0'): + continue + fields = (entry / 'stat').read_text().rpartition(')')[2].split() + if fields[0] == 'Z': + continue + executable = (entry / 'exe').readlink().name + found.append({'pid': int(entry.name), 'start_ticks': int(fields[19]), + 'kind': executable if executable in KINDS else 'other', + 'cpu_ticks': int(fields[11]) + int(fields[12]), + 'rss_pages': int(fields[21])}) + except (OSError, ValueError, IndexError): + # A process can exit between any two /proc reads. + continue + return sorted(found, key=lambda item: item['pid']) + + +def signal_marked(marker, processes, sig): + identities = {(p['pid'], p['start_ticks']) for p in processes} + for current in marked_processes(marker): + if (current['pid'], current['start_ticks']) in identities: + try: + os.kill(current['pid'], sig) + except ProcessLookupError: + pass + + +def watch(marker, seconds, status, stop, *, interval=2, grace=5): + started = time.monotonic() + record = {'state': 'watching', 'deadline_seconds': seconds, 'action_seen': False} + + def save(): + record['elapsed_seconds'] = round(time.monotonic() - started, 2) + temporary = status.with_suffix('.tmp') + temporary.write_text(json.dumps(record, indent=2) + '\n') + temporary.replace(status) + + while True: + processes = marked_processes(marker) + record['processes'] = processes + record['action_seen'] |= bool(processes) + if stop.exists(): + record['state'] = 'stopped' + save() + return + if time.monotonic() - started >= seconds: + record['state'] = 'timed_out' + save() + signal_marked(marker, processes, signal.SIGTERM) + time.sleep(grace) + signal_marked(marker, processes, signal.SIGKILL) + record['remaining_processes'] = marked_processes(marker) + save() + return + save() + time.sleep(interval) + + +def main(): + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument('--marker', required=True) + parser.add_argument('--seconds', type=int, required=True) + parser.add_argument('--status', type=Path, required=True) + parser.add_argument('--stop', type=Path, required=True) + args = parser.parse_args() + if not 1 <= args.seconds <= 1500: + parser.error('deadline must be between 1 and 1500 seconds') + watch(args.marker, args.seconds, args.status, args.stop) + + +if __name__ == '__main__': + main() diff --git a/contrib/upstream-sync/test_review_watchdog.py b/contrib/upstream-sync/test_review_watchdog.py new file mode 100644 index 0000000000000..ee1b458b8e220 --- /dev/null +++ b/contrib/upstream-sync/test_review_watchdog.py @@ -0,0 +1,75 @@ +"""Check the deadline against real child processes without API calls.""" +import json +import os +from pathlib import Path +import signal +import subprocess +import sys +import tempfile +import unittest +import uuid +from unittest.mock import patch + +import review_watchdog as watchdog + + +@unittest.skipUnless(sys.platform == 'linux', 'the Actions watchdog uses /proc') +class WatchdogTests(unittest.TestCase): + def setUp(self): + self.temp = tempfile.TemporaryDirectory() + self.addCleanup(self.temp.cleanup) + self.root = Path(self.temp.name) + self.marker = str(uuid.uuid4()) + + def child(self, marker, ignore_term=False): + code = ('import signal,time; ' + + ('signal.signal(signal.SIGTERM, signal.SIG_IGN); ' if ignore_term else '') + + 'print("ready", flush=True); time.sleep(60)') + child = subprocess.Popen([sys.executable, '-c', code], + env=dict(os.environ, JOOLIA_REVIEW_MARKER=marker, + NEVER_LOG_THIS_SECRET='sentinel-secret'), + stdout=subprocess.PIPE, text=True) + def cleanup(): + if child.poll() is None: + child.kill() + child.wait() + child.stdout.close() + self.addCleanup(cleanup) + self.assertEqual(child.stdout.readline().strip(), 'ready') + return child + + def test_deadline_kills_only_marked_process_and_escalates(self): + target = self.child(self.marker, ignore_term=True) + unrelated = self.child(self.marker + '-other') + status = self.root / 'status.json' + watchdog.watch(self.marker, 0, status, self.root / 'stop', interval=.01, grace=.05) + self.assertEqual(target.wait(timeout=5), -signal.SIGKILL) + self.assertIsNone(unrelated.poll()) + record = json.loads(status.read_text()) + self.assertEqual(record['state'], 'timed_out') + self.assertTrue(record['action_seen']) + self.assertNotIn('sentinel-secret', status.read_text()) + self.assertNotIn('argv', status.read_text()) + + def test_stop_disarms_deadline(self): + target = self.child(self.marker) + stop = self.root / 'stop' + stop.touch() + status = self.root / 'status.json' + watchdog.watch(self.marker, 0, status, stop) + self.assertIsNone(target.poll()) + self.assertEqual(json.loads(status.read_text())['state'], 'stopped') + + def test_reused_pid_is_not_signalled(self): + with patch.object(watchdog, 'marked_processes', return_value=[{'pid': 1234, 'start_ticks': 20}]), \ + patch.object(watchdog.os, 'kill') as kill: + watchdog.signal_marked(self.marker, [{'pid': 1234, 'start_ticks': 10}], signal.SIGTERM) + kill.assert_not_called() + + def test_timeout_before_action_started_is_visible(self): + status = self.root / 'status.json' + watchdog.watch(self.marker, 0, status, self.root / 'stop', grace=0) + record = json.loads(status.read_text()) + self.assertEqual(record['state'], 'timed_out') + self.assertFalse(record['action_seen']) + self.assertEqual(record['processes'], []) From 1aea52b970ee0ba7045a922e901b4c61779ad225 Mon Sep 17 00:00:00 2001 From: AntonOresten Date: Sat, 19 Sep 2026 19:45:16 +0200 Subject: [PATCH 2/2] CI: Verify sandbox tool execution in the review probe Require the probe to read a random local challenge so a successful API response alone cannot hide a broken tool execution path. Assisted-by: Codex (GPT-6) --- .github/workflows/upstream-sync.yml | 25 +++++++++++++++++++++---- contrib/upstream-sync/README.md | 5 +++-- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/.github/workflows/upstream-sync.yml b/.github/workflows/upstream-sync.yml index 3febbb9d62ef0..f5e586cb5dbd4 100644 --- a/.github/workflows/upstream-sync.yml +++ b/.github/workflows/upstream-sync.yml @@ -97,12 +97,17 @@ jobs: if: inputs.mode == 'probe' run: | python3 - <<'PYCODE' - import json, os + import json, os, secrets from pathlib import Path folder = Path(os.environ['SYNC_DIR']) - (folder / 'prompt.md').write_text('Do not run tools or inspect files. Return exactly {"status":"ok"}.') - schema = {'type': 'object', 'properties': {'status': {'type': 'string', 'enum': ['ok']}}, - 'required': ['status'], 'additionalProperties': False} + challenge = Path('.cache/upstream-sync-probe.txt') + challenge.parent.mkdir(exist_ok=True) + challenge.write_text(secrets.token_hex(16)) + (folder / 'prompt.md').write_text('Use one shell tool to run `cat .cache/upstream-sync-probe.txt`. ' + 'Do not change files or run anything else. Return JSON with status="ok" and proof equal to the file contents. ' + 'If the tool fails, return status="failed" and proof=""; do not retry.') + schema = {'type': 'object', 'properties': {'status': {'type': 'string', 'enum': ['ok', 'failed']}, + 'proof': {'type': 'string'}}, 'required': ['status', 'proof'], 'additionalProperties': False} (folder / 'review.schema.json').write_text(json.dumps(schema)) PYCODE - name: Arm independent review deadline @@ -149,6 +154,18 @@ jobs: prompt-file: ${{ env.SYNC_DIR }}/prompt.md output-schema-file: ${{ env.SYNC_DIR }}/review.schema.json output-file: ${{ env.SYNC_DIR }}/review.json + - name: Verify the probe exercised a shell tool + if: inputs.mode == 'probe' && steps.review.outcome == 'success' + run: | + python3 - <<'PYCODE' + import json, os + from pathlib import Path + result = json.loads((Path(os.environ['SYNC_DIR']) / 'review.json').read_text()) + expected = Path('.cache/upstream-sync-probe.txt').read_text() + if result != {'status': 'ok', 'proof': expected}: + raise SystemExit('The model responded, but the sandboxed shell probe failed') + print('Model response and sandboxed shell challenge verified.') + PYCODE - name: Retain progress and stop the deadline if: always() env: diff --git a/contrib/upstream-sync/README.md b/contrib/upstream-sync/README.md index 93f217a4462d7..4f106f38a749e 100644 --- a/contrib/upstream-sync/README.md +++ b/contrib/upstream-sync/README.md @@ -68,8 +68,9 @@ To check the action/model path independently of a large review, manually run: gh workflow run upstream-sync.yml --ref master -f mode=probe --repo jool-space/joolia ``` -The probe asks only for a tiny JSON response, has a three-minute independent -deadline, and never publishes a PR or advances a checkpoint. Maintainers can +The probe asks for one sandboxed shell read of a random challenge file and a +tiny JSON response. It verifies the returned challenge, has a three-minute +independent deadline, and never publishes a PR or advances a checkpoint. Maintainers can also dispatch it on a same-repository workflow branch to validate a fix before merging. It still makes an API call. Failed review batches are not retried in an unbounded loop; inspect the artifact before deciding whether to retry.