From 81a96f97af336039bb93faafc388cf603ebddd53 Mon Sep 17 00:00:00 2001 From: kikashy Date: Thu, 22 Oct 2026 15:25:57 -0400 Subject: [PATCH 1/2] Retain check results and carry repository notes Preflight the check log destination and preserve completed results when log persistence fails so infrastructure errors do not erase the summary. Admit repository notes in the manifest, print them in build brief templates, and require their heading when checking a build brief. Co-Authored-By: Codex Signed-off-by: kikashy --- bin/jpdev | 50 +++++++++++++++++++++++++++++--- docs/adopting.md | 6 ++++ lib/validate_manifest.py | 8 +++++ scripts/dev/check.sh | 48 +++++++++++++++++++++++++----- templates/check.sh | 48 +++++++++++++++++++++++++----- templates/dev.schema.json | 9 ++++++ tests/run.sh | 61 +++++++++++++++++++++++++++++++++++++++ 7 files changed, 212 insertions(+), 18 deletions(-) diff --git a/bin/jpdev b/bin/jpdev index 98cb281..f9b910d 100755 --- a/bin/jpdev +++ b/bin/jpdev @@ -82,7 +82,9 @@ cmd_check() { export JPDEV_LOGDIR=$logdir "$script" --manifest "$manifest" "${passthru[@]}" local status=$? - jpdev_log "full output of each check: $logdir" + if compgen -G "$logdir/*.log" > /dev/null; then + jpdev_log "full output of each check: $logdir" + fi exit "$status" } @@ -475,7 +477,7 @@ codex_review() { # codex_check_brief [--build] FILE: check the build heading and the filter-list words in FILE. codex_check_brief() { - local file="" build=0 + local file="" build=0 root manifest while [ $# -gt 0 ]; do case $1 in --build) build=1; shift ;; @@ -492,6 +494,25 @@ codex_check_brief() { fi echo "jpdev codex check-brief: warning: $file has no line exactly '## Before you write msg.txt'" >&2 fi + if [ "$build" -eq 1 ]; then + root=$(git rev-parse --show-toplevel 2>/dev/null || true) + manifest=${root:+$root/dev.json} + if [ -n "$manifest" ] && [ -f "$manifest" ] && python3 -I - "$manifest" <<'PY' +import json, sys +try: + notes = json.load(open(sys.argv[1], encoding='utf-8')).get('notes', []) +except (OSError, ValueError, AttributeError): + notes = [] +if not isinstance(notes, list): notes = [] +sys.exit(0 if notes else 1) +PY + then + if ! grep -qxF "## This repository's notes" "$file"; then + echo "jpdev codex check-brief: refused: $file has no line exactly '## This repository's notes', required by $manifest" >&2 + exit 1 + fi + fi + fi local out status out=$(python3 -I "$kit/lib/codex_words.py" "$file" "$kit/config/codex-words.txt" "$JPDEV_CODEX_WORDS") status=$? @@ -588,7 +609,7 @@ codex_run() { done fi brief=$(cd "$(dirname "$brief")" && pwd)/$(basename "$brief") - codex_check_brief --build "$brief" + (cd "$dir" && codex_check_brief --build "$brief") || exit $? if [ -z "$log" ]; then mkdir -p "$JPDEV_SCRATCH/logs" log=$JPDEV_SCRATCH/logs/codex-$(basename "$dir")-$(date +%Y%m%dT%H%M%S).log @@ -837,7 +858,28 @@ codex_publish() { # codex_brief_template prints the brief skeleton a Codex build or review starts from. codex_brief_template() { case ${1:-} in - "") cat "$kit/templates/codex-brief.md" ;; + "") + cat "$kit/templates/codex-brief.md" + local root manifest notes + root=$(git rev-parse --show-toplevel 2>/dev/null || true) + manifest=${root:+$root/dev.json} + if [ -n "$manifest" ] && [ -f "$manifest" ]; then + notes=$(python3 -I - "$manifest" <<'PY' +import json, sys +try: + notes = json.load(open(sys.argv[1], encoding='utf-8')).get('notes', []) +except (OSError, ValueError, AttributeError): + notes = [] +if not isinstance(notes, list): notes = [] +for note in notes: + if isinstance(note, str): print('- ' + note) +PY +) + if [ -n "$notes" ]; then + printf "\n## This repository's notes\n\n%s\n" "$notes" + fi + fi + ;; --review) [ $# -eq 1 ] || jpdev_die "codex brief-template: --review takes no value"; cat "$kit/templates/codex-review-brief.md" ;; *) jpdev_die "codex brief-template: unknown argument $1" ;; esac diff --git a/docs/adopting.md b/docs/adopting.md index 83f9a0b..694b9f9 100644 --- a/docs/adopting.md +++ b/docs/adopting.md @@ -19,6 +19,12 @@ Start from `templates/dev.json` and keep what the repository runs. The manifest - `setup`: steps `jpdev setup` and CI run once after a clone (`npm ci`, the python requirements). - `checks`: the check list, in the order it should run (cheap checks first). +- `notes`: optional short sentences about repository-specific traps. For example, + `"run every Python command with the venv on the PATH"` and + `"a change to a file listed in scripts/parity.sh updates its row"`. They are plain text: + `jpdev codex brief-template` prints them under `## This repository's notes`, and + `jpdev codex check-brief --build` requires that heading when notes are present; no note is + executed. - `mutants`: the command `jpdev mutants` runs; `{rows}` becomes one `--row 'TEXT'` per row. - `parity`: an optional last check that compares the repository against another one; it names the environment variable it needs in `requires_env`, and is skipped, and said to be skipped, diff --git a/lib/validate_manifest.py b/lib/validate_manifest.py index 54a3bec..3690803 100644 --- a/lib/validate_manifest.py +++ b/lib/validate_manifest.py @@ -42,6 +42,14 @@ def main(): problems.append("(root): %r is required" % key) if manifest.get("version") != 1: problems.append("version: must be 1") + if "notes" in manifest: + notes = manifest["notes"] + if not isinstance(notes, list): + problems.append("notes: not an array") + else: + for index, note in enumerate(notes): + if not isinstance(note, str) or not note or len(note) > 240: + problems.append("notes/%d: must be a non-empty string of at most 240 characters" % index) for kind in ("setup", "checks"): for index, step in enumerate(manifest.get(kind, [])): if not isinstance(step, dict) or not step.get("name") or not step.get("run"): diff --git a/scripts/dev/check.sh b/scripts/dev/check.sh index 9d7fbb9..e13d82c 100755 --- a/scripts/dev/check.sh +++ b/scripts/dev/check.sh @@ -18,6 +18,23 @@ while args: elif a in ('--all', '--setup'): opt[a[2:]] = True else: sys.exit('check.sh: unknown argument ' + a) m = json.load(open(opt['manifest'], encoding='utf-8')) +logdir = os.environ.get('JPDEV_LOGDIR') +if logdir: + probe = os.path.join(logdir, '.jpdev-write-probe-%d' % os.getpid()) + try: + os.makedirs(logdir, exist_ok=True) + descriptor = os.open(probe, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o600) + try: os.write(descriptor, b'x') + finally: os.close(descriptor) + os.unlink(probe) + except OSError as error: + try: + if os.path.exists(probe): os.unlink(probe) + except OSError: + pass + print('check.sh: infrastructure failure: cannot create and write the check log directory %s: %s; set JPDEV_LOGDIR or JPDEV_SCRATCH to a writable directory' % + (logdir, error.strerror or error), file=sys.stderr) + sys.exit(2) python = m.get('toolchains', {}).get('python', {}) venv = python.get('venv') run_env = dict(os.environ) @@ -59,10 +76,11 @@ if venv is not None: steps = m.get('setup', []) if opt['setup'] else m['checks'] + ([dict(name='parity', **m['parity'])] if 'parity' in m else []) unknown = (opt['only'] | opt['skip']) - set(s['name'] for s in steps) if unknown: print('check.sh: no step named ' + ', '.join(sorted(unknown)), file=sys.stderr); sys.exit(2) -wrap, logdir, rows, failed = os.environ.get('JPDEV_WRAP'), os.environ.get('JPDEV_LOGDIR'), [], 0 +wrap, rows, failed, infrastructure = os.environ.get('JPDEV_WRAP'), [], 0, '' for c in steps: n = c['name'] if (opt['only'] and n not in opt['only']) or n in opt['skip']: continue + if infrastructure: rows.append((n, 'NOT RUN', '', [])); continue if failed and not opt['all']: rows.append((n, 'NOT RUN', '', [])); continue if c.get('requires_env') and not os.environ.get(c['requires_env']): # A skip is a failure under CI (CI is set) unless the step says optional: true. @@ -85,18 +103,34 @@ for c in steps: found = subprocess.run([executable, '--version'], text=True, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, env=step_env).stdout.strip() took += ', python %s (%s)' % (executable, found) - if logdir: os.makedirs(logdir, exist_ok=True); open('%s/%s.log' % (logdir, n), 'w').write(out) hit = [l.strip() for l in lines if c.get('summary') and re.search(c['summary'], l)] note = [' ' + l for l in (hit if len(hit) <= 3 else ['%d lines matched; last: %s' % (len(hit), hit[-1])])] - if p.returncode == 0: rows.append((n, 'PASS', took, note)); continue - failed += 1 - rows.append((n, 'FAIL', '%s, exit %d' % (took, p.returncode), note + [' --'] + [' ' + l for l in lines[:40]] + ([' ... %d more lines' % (len(lines) - 40)] if len(lines) > 40 else []))) + if p.returncode == 0: + rows.append((n, 'PASS', took, note)) + else: + failed += 1 + rows.append((n, 'FAIL', '%s, exit %d' % (took, p.returncode), note + [' --'] + [' ' + l for l in lines[:40]] + ([' ... %d more lines' % (len(lines) - 40)] if len(lines) > 40 else []))) + if logdir: + log = '%s/%s.log' % (logdir, n) + pending = log + '.pending-%d' % os.getpid() + try: + with open(pending, 'w', encoding='utf-8') as handle: handle.write(out) + os.replace(pending, log) + except OSError as error: + try: + if os.path.exists(pending): os.unlink(pending) + except OSError: + pass + infrastructure = 'cannot persist the completed check log %s: %s' % (log, error.strerror or error) print('== %s summary (%s) ==' % ('setup' if opt['setup'] else 'check', os.path.basename(opt['manifest']))) if not opt['setup'] and 'handover' in m: h = m['handover'] print('handover: boundary %d paths, shared %d paths; see AGENTS.md "Before you hand over"' % (len(h.get('boundary_paths', [])), len(h.get('shared_paths', [])))) +if infrastructure: print('infrastructure failure: ' + infrastructure) for n, s, head, extra in rows: print('\n'.join(['%-8s %-24s %s' % (s, n, head)] + extra)) -print('== %d passed, %d failed, %d skipped, %d not run ==' % tuple(sum(1 for r in rows if r[1] == k) for k in ('PASS', 'FAIL', 'SKIP', 'NOT RUN'))) -sys.exit(min(failed, 255)) +totals = tuple(sum(1 for r in rows if r[1] == k) for k in ('PASS', 'FAIL', 'SKIP', 'NOT RUN')) +if infrastructure: print('== %d passed, %d failed, %d skipped, %d not run; infrastructure failure ==' % totals) +else: print('== %d passed, %d failed, %d skipped, %d not run ==' % totals) +sys.exit(min(failed + bool(infrastructure), 255)) PY diff --git a/templates/check.sh b/templates/check.sh index 9d7fbb9..e13d82c 100755 --- a/templates/check.sh +++ b/templates/check.sh @@ -18,6 +18,23 @@ while args: elif a in ('--all', '--setup'): opt[a[2:]] = True else: sys.exit('check.sh: unknown argument ' + a) m = json.load(open(opt['manifest'], encoding='utf-8')) +logdir = os.environ.get('JPDEV_LOGDIR') +if logdir: + probe = os.path.join(logdir, '.jpdev-write-probe-%d' % os.getpid()) + try: + os.makedirs(logdir, exist_ok=True) + descriptor = os.open(probe, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o600) + try: os.write(descriptor, b'x') + finally: os.close(descriptor) + os.unlink(probe) + except OSError as error: + try: + if os.path.exists(probe): os.unlink(probe) + except OSError: + pass + print('check.sh: infrastructure failure: cannot create and write the check log directory %s: %s; set JPDEV_LOGDIR or JPDEV_SCRATCH to a writable directory' % + (logdir, error.strerror or error), file=sys.stderr) + sys.exit(2) python = m.get('toolchains', {}).get('python', {}) venv = python.get('venv') run_env = dict(os.environ) @@ -59,10 +76,11 @@ if venv is not None: steps = m.get('setup', []) if opt['setup'] else m['checks'] + ([dict(name='parity', **m['parity'])] if 'parity' in m else []) unknown = (opt['only'] | opt['skip']) - set(s['name'] for s in steps) if unknown: print('check.sh: no step named ' + ', '.join(sorted(unknown)), file=sys.stderr); sys.exit(2) -wrap, logdir, rows, failed = os.environ.get('JPDEV_WRAP'), os.environ.get('JPDEV_LOGDIR'), [], 0 +wrap, rows, failed, infrastructure = os.environ.get('JPDEV_WRAP'), [], 0, '' for c in steps: n = c['name'] if (opt['only'] and n not in opt['only']) or n in opt['skip']: continue + if infrastructure: rows.append((n, 'NOT RUN', '', [])); continue if failed and not opt['all']: rows.append((n, 'NOT RUN', '', [])); continue if c.get('requires_env') and not os.environ.get(c['requires_env']): # A skip is a failure under CI (CI is set) unless the step says optional: true. @@ -85,18 +103,34 @@ for c in steps: found = subprocess.run([executable, '--version'], text=True, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, env=step_env).stdout.strip() took += ', python %s (%s)' % (executable, found) - if logdir: os.makedirs(logdir, exist_ok=True); open('%s/%s.log' % (logdir, n), 'w').write(out) hit = [l.strip() for l in lines if c.get('summary') and re.search(c['summary'], l)] note = [' ' + l for l in (hit if len(hit) <= 3 else ['%d lines matched; last: %s' % (len(hit), hit[-1])])] - if p.returncode == 0: rows.append((n, 'PASS', took, note)); continue - failed += 1 - rows.append((n, 'FAIL', '%s, exit %d' % (took, p.returncode), note + [' --'] + [' ' + l for l in lines[:40]] + ([' ... %d more lines' % (len(lines) - 40)] if len(lines) > 40 else []))) + if p.returncode == 0: + rows.append((n, 'PASS', took, note)) + else: + failed += 1 + rows.append((n, 'FAIL', '%s, exit %d' % (took, p.returncode), note + [' --'] + [' ' + l for l in lines[:40]] + ([' ... %d more lines' % (len(lines) - 40)] if len(lines) > 40 else []))) + if logdir: + log = '%s/%s.log' % (logdir, n) + pending = log + '.pending-%d' % os.getpid() + try: + with open(pending, 'w', encoding='utf-8') as handle: handle.write(out) + os.replace(pending, log) + except OSError as error: + try: + if os.path.exists(pending): os.unlink(pending) + except OSError: + pass + infrastructure = 'cannot persist the completed check log %s: %s' % (log, error.strerror or error) print('== %s summary (%s) ==' % ('setup' if opt['setup'] else 'check', os.path.basename(opt['manifest']))) if not opt['setup'] and 'handover' in m: h = m['handover'] print('handover: boundary %d paths, shared %d paths; see AGENTS.md "Before you hand over"' % (len(h.get('boundary_paths', [])), len(h.get('shared_paths', [])))) +if infrastructure: print('infrastructure failure: ' + infrastructure) for n, s, head, extra in rows: print('\n'.join(['%-8s %-24s %s' % (s, n, head)] + extra)) -print('== %d passed, %d failed, %d skipped, %d not run ==' % tuple(sum(1 for r in rows if r[1] == k) for k in ('PASS', 'FAIL', 'SKIP', 'NOT RUN'))) -sys.exit(min(failed, 255)) +totals = tuple(sum(1 for r in rows if r[1] == k) for k in ('PASS', 'FAIL', 'SKIP', 'NOT RUN')) +if infrastructure: print('== %d passed, %d failed, %d skipped, %d not run; infrastructure failure ==' % totals) +else: print('== %d passed, %d failed, %d skipped, %d not run ==' % totals) +sys.exit(min(failed + bool(infrastructure), 255)) PY diff --git a/templates/dev.schema.json b/templates/dev.schema.json index 4523eff..51f4b99 100644 --- a/templates/dev.schema.json +++ b/templates/dev.schema.json @@ -14,6 +14,15 @@ "version": { "const": 1 }, + "notes": { + "type": "array", + "description": "short repository-specific sentences printed in Codex build briefs as plain text", + "items": { + "type": "string", + "minLength": 1, + "maxLength": 240 + } + }, "handover": { "type": "object", "additionalProperties": false, diff --git a/tests/run.sh b/tests/run.sh index 08ac44a..85c1ff9 100755 --- a/tests/run.sh +++ b/tests/run.sh @@ -51,6 +51,34 @@ out=$("$kit/templates/check.sh" --manifest "$fixture" --only first,python-versio expect "check.sh --only runs the named checks and exits 0" 0 '== 2 passed, 0 failed, 0 skipped, 0 not run ==' "$out" "$s" out=$("$kit/templates/check.sh" --manifest "$fixture" --skip fails,after-failure,needs-variable 2>/dev/null) && s=0 || s=$? expect "check.sh --skip leaves the rest" 0 '== 5 passed' "$out" "$s" + +# A log destination is proved writable before a command starts, and a later persistence failure +# keeps the completed result while stopping the rest of the list. +mkdir -p "$work/read-only-scratch" +chmod 500 "$work/read-only-scratch" +cat > "$work/log-preflight.json" <&1) && s=0 || s=$? +chmod 700 "$work/read-only-scratch" +expect "check.sh refuses an unwritable log destination before a check" 2 'infrastructure failure: cannot create and write the check log directory.*JPDEV_LOGDIR.*JPDEV_SCRATCH' "$out" "$s" +if [ ! -e "$work/preflight-ran" ]; then ok "a log preflight refusal starts no check"; else bad "a log preflight refusal starts no check"; fi +absent "a log preflight refusal does not imply that logs exist" 'full output of each check:' "$out" + +mkdir -p "$work/mid-run-logs" +cat > "$work/log-persistence.json" <<'JSON' +{"version": 1, "checks": [ + {"name": "completed", "run": "echo retained; chmod a-w \"$JPDEV_LOGDIR\""}, + {"name": "after-persistence-failure", "run": "touch should-not-run"} +]} +JSON +out=$(JPDEV_ROOT=$work JPDEV_LOGDIR=$work/mid-run-logs "$kit/templates/check.sh" --manifest "$work/log-persistence.json" --all 2>&1) && s=0 || s=$? +chmod u+w "$work/mid-run-logs" +expect "check.sh reports a mid-run log persistence failure as infrastructure" 1 'infrastructure failure: cannot persist the completed check.*completed\.log' "$out" "$s" +expect "a persistence failure retains the completed step status" 1 'PASS +completed' "$out" "$s" +expect "a persistence failure marks later checks not run" 1 'NOT RUN +after-persistence-failure' "$out" "$s" +if [ ! -e "$work/should-not-run" ]; then ok "a persistence failure does not run another check"; else bad "a persistence failure does not run another check"; fi + out=$(JPDEV_FIXTURE_UNSET=1 "$kit/templates/check.sh" --manifest "$fixture" --only needs-variable 2>/dev/null) && s=0 || s=$? expect "check.sh runs a step whose variable is set" 0 'PASS +needs-variable' "$out" "$s" out=$("$kit/templates/check.sh" --manifest "$fixture" --setup 2>/dev/null) && s=0 || s=$? @@ -91,6 +119,11 @@ expect "jpdev check prints the handover path counts" 1 'handover: boundary 2 pat if grep -q 'go version go' <<< "$out" && grep -q 'Python 3' <<< "$out"; then ok "jpdev check prints the counts the checks printed"; else bad "jpdev check prints the counts the checks printed" "$out"; fi out=$("$jpdev" check --manifest "$fixture" --skip fails,after-failure,needs-variable 2>/dev/null) && s=0 || s=$? expect "jpdev check exits 0 when every check passes" 0 '== 5 passed' "$out" "$s" +stored_scratch=$work/stored-scratch +out=$(JPDEV_SCRATCH=$stored_scratch "$jpdev" check --manifest "$fixture" --only first 2>&1) && s=0 || s=$? +expect "jpdev check stores a step log under the configured scratch logs directory" 0 'full output of each check:' "$out" "$s" +stored_log=$(find "$stored_scratch/logs" -type f -name first.log -print -quit) +if [ -n "$stored_log" ] && grep -qxF 'Tests 3 passed' "$stored_log"; then ok "the stored check log holds the step output"; else bad "the stored check log holds the step output" "${stored_log:-no first.log}"; fi out=$(JPDEV_GO_ULIMIT_KB=1000 "$jpdev" check --manifest "$fixture" --only go-version 2>/dev/null) && s=0 || s=$? expect "jpdev check applies the Go address-space cap (a 1 MB cap fails the step)" 1 'FAIL +go-version' "$out" "$s" out=$("$jpdev" check --manifest "$fixture" --only go-env 2>/dev/null) && s=0 || s=$? @@ -375,6 +408,20 @@ out=$(python3 "$kit/lib/validate_manifest.py" "$kit/templates/dev.json" "$kit/te expect "the template manifest is valid" 0 "valid" "$out" "$s" out=$(python3 "$kit/lib/validate_manifest.py" "$kit/examples/judgment-pack-agent.dev.json" "$kit/templates/dev.schema.json" 2>&1) && s=0 || s=$? expect "the agent repository's example manifest is valid" 0 "valid" "$out" "$s" +printf '{"version": 1, "notes": ["run every Python command with the venv on the PATH", "a change to a file listed in scripts/parity.sh updates its row"], "checks": [{"name": "a", "run": "true"}]}\n' > "$work/notes.json" +out=$(python3 "$kit/lib/validate_manifest.py" "$work/notes.json" "$kit/templates/dev.schema.json" 2>&1) && s=0 || s=$? +expect "the schema accepts repository notes" 0 "valid" "$out" "$s" +out=$(python3 -S "$kit/lib/validate_manifest.py" "$work/notes.json" "$kit/templates/dev.schema.json" 2>&1) && s=0 || s=$? +expect "the standard-library manifest validator accepts repository notes" 0 "valid" "$out" "$s" +printf '{"version": 1, "notes": [""], "checks": [{"name": "a", "run": "true"}]}\n' > "$work/empty-note.json" +out=$(python3 "$kit/lib/validate_manifest.py" "$work/empty-note.json" "$kit/templates/dev.schema.json" 2>&1) && s=0 || s=$? +expect "the schema refuses an empty repository note" 1 "dev.json:" "$out" "$s" +python3 - "$work/long-note.json" <<'PY' +import json, sys +json.dump({"version": 1, "notes": ["x" * 241], "checks": [{"name": "a", "run": "true"}]}, open(sys.argv[1], "w", encoding="utf-8")) +PY +out=$(python3 "$kit/lib/validate_manifest.py" "$work/long-note.json" "$kit/templates/dev.schema.json" 2>&1) && s=0 || s=$? +expect "the schema refuses an oversized repository note" 1 "dev.json:" "$out" "$s" printf '{"version": 1, "checks": [{"name": "a", "run": "true"}], "handover": {"boundary_paths": ["gate"], "shared_paths": ["contract"]}}\n' > "$work/handover.json" out=$(python3 "$kit/lib/validate_manifest.py" "$work/handover.json" "$kit/templates/dev.schema.json" 2>&1) && s=0 || s=$? expect "the schema accepts handover path lists" 0 "valid" "$out" "$s" @@ -1147,6 +1194,20 @@ if [ "$out" = "$(cat "$kit/templates/codex-brief.md")" ]; then ok "brief-templat expect "the brief template tells codex to write msg.txt" 0 'msg\.txt' "$out" "$s" expect "the brief template tells codex to write pr.md" 0 'pr\.md' "$out" "$s" expect "the brief template tells codex not to commit" 0 'do not run .git commit.' "$out" "$s" +notesrepo=$work/notesrepo +mkdir -p "$notesrepo" +git init -q -b main "$notesrepo" +cp "$work/notes.json" "$notesrepo/dev.json" +out=$(cd "$notesrepo" && "$jpdev" codex brief-template 2>&1) && s=0 || s=$? +expect "brief-template prints the repository notes heading" 0 '^## This repository.s notes$' "$out" "$s" +expect "brief-template prints the first repository note" 0 '^- run every Python command with the venv on the PATH$' "$out" "$s" +expect "brief-template prints the second repository note" 0 '^- a change to a file listed in scripts/parity.sh updates its row$' "$out" "$s" +printf 'A build brief.\n\n## Before you write msg.txt\n' > "$notesrepo/build-brief.md" +out=$(cd "$notesrepo" && "$jpdev" codex check-brief --build build-brief.md 2>&1) && s=0 || s=$? +expect "check-brief --build refuses missing repository notes" 1 'has no line exactly.*This repository.s notes.*required by .*dev.json' "$out" "$s" +printf '{"version": 1, "checks": [{"name": "a", "run": "true"}]}\n' > "$notesrepo/dev.json" +out=$(cd "$notesrepo" && "$jpdev" codex check-brief --build build-brief.md 2>&1) && s=0 || s=$? +expect "a manifest without notes requires no repository notes heading" 0 'no word of the filter list' "$out" "$s" out=$("$jpdev" codex check-brief --build "$kit/templates/codex-brief.md" 2>&1) && s=0 || s=$? expect "the brief template passes check-brief --build" 0 'no word of the filter list' "$out" "$s" out=$("$jpdev" codex brief-template --review 2>&1) && s=0 || s=$? From d71c9cdfbbbb1bfb548f5ef971fdf2c10ef08635 Mon Sep 17 00:00:00 2001 From: kikashy Date: Thu, 22 Oct 2026 15:39:30 -0400 Subject: [PATCH 2/2] Preserve results when check logs cannot be written Let checks run after a log-directory probe fails and retain their summary when the completed output cannot be persisted. Keep infrastructure failures separate from check-derived exit statuses and document the log location, contents, and persistence-failure behavior. Co-Authored-By: Codex Signed-off-by: kikashy --- docs/adopting.md | 6 ++++++ scripts/dev/check.sh | 10 +++++----- templates/check.sh | 10 +++++----- tests/run.sh | 27 ++++++++++++++++++--------- 4 files changed, 34 insertions(+), 19 deletions(-) diff --git a/docs/adopting.md b/docs/adopting.md index 694b9f9..d0a0845 100644 --- a/docs/adopting.md +++ b/docs/adopting.md @@ -75,6 +75,12 @@ step that invokes `python3`. `jpdev check` uses the vendored copy when it exists `JPDEV_WRAP` so that each step runs under its toolchain's cap. `jpdev env` shows the interpreter selected for the current repository's manifest. +`jpdev check` stores each step's complete step output (stdout and stderr combined) at +`$JPDEV_SCRATCH/logs/-/.log`. If that output cannot be persisted, the +summary keeps the completed step's result and reports the persistence failure separately, naming +the intended log target. Remaining steps are marked not run, and the infrastructure failure does +not change the check-derived exit status. No log path is advertised unless a complete log exists. + ## 3. Add the agent instructions Copy `templates/AGENTS.md` to the repository root and generate `CLAUDE.md` from it with diff --git a/scripts/dev/check.sh b/scripts/dev/check.sh index e13d82c..fcdd251 100755 --- a/scripts/dev/check.sh +++ b/scripts/dev/check.sh @@ -19,6 +19,7 @@ while args: else: sys.exit('check.sh: unknown argument ' + a) m = json.load(open(opt['manifest'], encoding='utf-8')) logdir = os.environ.get('JPDEV_LOGDIR') +log_probe_error = '' if logdir: probe = os.path.join(logdir, '.jpdev-write-probe-%d' % os.getpid()) try: @@ -28,13 +29,11 @@ if logdir: finally: os.close(descriptor) os.unlink(probe) except OSError as error: + log_probe_error = error.strerror or str(error) try: if os.path.exists(probe): os.unlink(probe) except OSError: pass - print('check.sh: infrastructure failure: cannot create and write the check log directory %s: %s; set JPDEV_LOGDIR or JPDEV_SCRATCH to a writable directory' % - (logdir, error.strerror or error), file=sys.stderr) - sys.exit(2) python = m.get('toolchains', {}).get('python', {}) venv = python.get('venv') run_env = dict(os.environ) @@ -114,6 +113,7 @@ for c in steps: log = '%s/%s.log' % (logdir, n) pending = log + '.pending-%d' % os.getpid() try: + os.makedirs(logdir, exist_ok=True) with open(pending, 'w', encoding='utf-8') as handle: handle.write(out) os.replace(pending, log) except OSError as error: @@ -121,7 +121,7 @@ for c in steps: if os.path.exists(pending): os.unlink(pending) except OSError: pass - infrastructure = 'cannot persist the completed check log %s: %s' % (log, error.strerror or error) + infrastructure = 'cannot persist the completed check log %s: %s' % (log, error.strerror or log_probe_error or error) print('== %s summary (%s) ==' % ('setup' if opt['setup'] else 'check', os.path.basename(opt['manifest']))) if not opt['setup'] and 'handover' in m: h = m['handover'] @@ -132,5 +132,5 @@ for n, s, head, extra in rows: print('\n'.join(['%-8s %-24s %s' % (s, n, head)] totals = tuple(sum(1 for r in rows if r[1] == k) for k in ('PASS', 'FAIL', 'SKIP', 'NOT RUN')) if infrastructure: print('== %d passed, %d failed, %d skipped, %d not run; infrastructure failure ==' % totals) else: print('== %d passed, %d failed, %d skipped, %d not run ==' % totals) -sys.exit(min(failed + bool(infrastructure), 255)) +sys.exit(min(failed, 255)) PY diff --git a/templates/check.sh b/templates/check.sh index e13d82c..fcdd251 100755 --- a/templates/check.sh +++ b/templates/check.sh @@ -19,6 +19,7 @@ while args: else: sys.exit('check.sh: unknown argument ' + a) m = json.load(open(opt['manifest'], encoding='utf-8')) logdir = os.environ.get('JPDEV_LOGDIR') +log_probe_error = '' if logdir: probe = os.path.join(logdir, '.jpdev-write-probe-%d' % os.getpid()) try: @@ -28,13 +29,11 @@ if logdir: finally: os.close(descriptor) os.unlink(probe) except OSError as error: + log_probe_error = error.strerror or str(error) try: if os.path.exists(probe): os.unlink(probe) except OSError: pass - print('check.sh: infrastructure failure: cannot create and write the check log directory %s: %s; set JPDEV_LOGDIR or JPDEV_SCRATCH to a writable directory' % - (logdir, error.strerror or error), file=sys.stderr) - sys.exit(2) python = m.get('toolchains', {}).get('python', {}) venv = python.get('venv') run_env = dict(os.environ) @@ -114,6 +113,7 @@ for c in steps: log = '%s/%s.log' % (logdir, n) pending = log + '.pending-%d' % os.getpid() try: + os.makedirs(logdir, exist_ok=True) with open(pending, 'w', encoding='utf-8') as handle: handle.write(out) os.replace(pending, log) except OSError as error: @@ -121,7 +121,7 @@ for c in steps: if os.path.exists(pending): os.unlink(pending) except OSError: pass - infrastructure = 'cannot persist the completed check log %s: %s' % (log, error.strerror or error) + infrastructure = 'cannot persist the completed check log %s: %s' % (log, error.strerror or log_probe_error or error) print('== %s summary (%s) ==' % ('setup' if opt['setup'] else 'check', os.path.basename(opt['manifest']))) if not opt['setup'] and 'handover' in m: h = m['handover'] @@ -132,5 +132,5 @@ for n, s, head, extra in rows: print('\n'.join(['%-8s %-24s %s' % (s, n, head)] totals = tuple(sum(1 for r in rows if r[1] == k) for k in ('PASS', 'FAIL', 'SKIP', 'NOT RUN')) if infrastructure: print('== %d passed, %d failed, %d skipped, %d not run; infrastructure failure ==' % totals) else: print('== %d passed, %d failed, %d skipped, %d not run ==' % totals) -sys.exit(min(failed + bool(infrastructure), 255)) +sys.exit(min(failed, 255)) PY diff --git a/tests/run.sh b/tests/run.sh index 85c1ff9..aab98f8 100755 --- a/tests/run.sh +++ b/tests/run.sh @@ -52,33 +52,42 @@ expect "check.sh --only runs the named checks and exits 0" 0 '== 2 passed, 0 fai out=$("$kit/templates/check.sh" --manifest "$fixture" --skip fails,after-failure,needs-variable 2>/dev/null) && s=0 || s=$? expect "check.sh --skip leaves the rest" 0 '== 5 passed' "$out" "$s" -# A log destination is proved writable before a command starts, and a later persistence failure -# keeps the completed result while stopping the rest of the list. +# An unwritable log destination does not stop the check. Its completed result remains in the +# summary, the infrastructure failure names the intended log, and no log is advertised. mkdir -p "$work/read-only-scratch" chmod 500 "$work/read-only-scratch" cat > "$work/log-preflight.json" <&1) && s=0 || s=$? chmod 700 "$work/read-only-scratch" -expect "check.sh refuses an unwritable log destination before a check" 2 'infrastructure failure: cannot create and write the check log directory.*JPDEV_LOGDIR.*JPDEV_SCRATCH' "$out" "$s" -if [ ! -e "$work/preflight-ran" ]; then ok "a log preflight refusal starts no check"; else bad "a log preflight refusal starts no check"; fi -absent "a log preflight refusal does not imply that logs exist" 'full output of each check:' "$out" +expect "check.sh reports an unwritable log destination without changing a passing status" 0 'infrastructure failure: cannot persist the completed check log .*sentinel\.log' "$out" "$s" +expect "an unwritable log destination retains the completed status" 0 'PASS +sentinel' "$out" "$s" +expect "an unwritable log destination retains the selected output" 0 '^ retained$' "$out" "$s" +if [ -e "$work/preflight-ran" ]; then ok "an unwritable log destination still runs the check"; else bad "an unwritable log destination still runs the check"; fi +absent "an unwritable log destination does not imply that logs exist" 'full output of each check:' "$out" mkdir -p "$work/mid-run-logs" cat > "$work/log-persistence.json" <<'JSON' {"version": 1, "checks": [ - {"name": "completed", "run": "echo retained; chmod a-w \"$JPDEV_LOGDIR\""}, + {"name": "completed", "run": "echo retained; chmod a-w \"$JPDEV_LOGDIR\"; exit 7"}, {"name": "after-persistence-failure", "run": "touch should-not-run"} ]} JSON out=$(JPDEV_ROOT=$work JPDEV_LOGDIR=$work/mid-run-logs "$kit/templates/check.sh" --manifest "$work/log-persistence.json" --all 2>&1) && s=0 || s=$? chmod u+w "$work/mid-run-logs" -expect "check.sh reports a mid-run log persistence failure as infrastructure" 1 'infrastructure failure: cannot persist the completed check.*completed\.log' "$out" "$s" -expect "a persistence failure retains the completed step status" 1 'PASS +completed' "$out" "$s" +expect "check.sh reports a mid-run log persistence failure without adding to a failed status" 1 'infrastructure failure: cannot persist the completed check.*completed\.log' "$out" "$s" +expect "a persistence failure retains the failed step status" 1 'FAIL +completed +.*exit 7' "$out" "$s" expect "a persistence failure marks later checks not run" 1 'NOT RUN +after-persistence-failure' "$out" "$s" if [ ! -e "$work/should-not-run" ]; then ok "a persistence failure does not run another check"; else bad "a persistence failure does not run another check"; fi +adopting=$(cat "$kit/docs/adopting.md") +expect "the adopting guide gives the check log path" 0 '[$]JPDEV_SCRATCH/logs/-/\.log' "$adopting" 0 +expect "the adopting guide says a log holds the complete step output" 0 'complete step output' "$adopting" 0 +expect "the adopting guide says a persistence failure is separate" 0 'reports the persistence failure separately' "$adopting" 0 +expect "the adopting guide says later steps are not run" 0 'Remaining steps are marked not run' "$adopting" 0 +expect "the adopting guide says infrastructure does not change the status" 0 'infrastructure failure does' "$adopting" 0 + out=$(JPDEV_FIXTURE_UNSET=1 "$kit/templates/check.sh" --manifest "$fixture" --only needs-variable 2>/dev/null) && s=0 || s=$? expect "check.sh runs a step whose variable is set" 0 'PASS +needs-variable' "$out" "$s" out=$("$kit/templates/check.sh" --manifest "$fixture" --setup 2>/dev/null) && s=0 || s=$?