Skip to content

doc-drift-precheck.sh: bare run prints a worklist path its own EXIT trap deletes #130

Open

Description

@clawgenti

Context

scripts/doc-drift-precheck.sh treats --worklist as optional and silently defaults it to a path inside the per-run temp dir (scripts/doc-drift-precheck.sh:41-44):

setup_workspace "doc-drift-precheck"
if [ -z "$WORKLIST" ]; then
  WORKLIST="$PROGRAM_TMPDIR/worklist.json"
fi

setup_workspace creates that directory under /tmp and registers an EXIT trap that deletes it (scripts/core.sh:27-29). So on the success path — worklist non-empty, exit 0 — the script prints worklist: /tmp/doc-drift-precheck-XXXXXX/worklist.json for a file that is removed before the process returns. The caller (a DAM schedule or orchestrator reading stdout to decide whether the model turn has any input) is handed a dangling path.

Review history (why this is still open)

Reported against rossoctl/repoman#12, which vendors this script:

  1. 2026-11-03 23:30 — must-fix review comment on skills/repoman/scripts/doc-drift-precheck.sh:43: "this default is deleted before the caller can use it … Default to a persistent location (e.g. $REPORTS_DIR/worklist.json) or require --worklist <persistent path>. Inherited from automation@fa61909, so the clean fix is upstream + re-vendor." — feat: Add doc-drift program (scanner, fixer, release assist) repoman#12 (comment)
  2. 2026-11-04 00:01 — commit 84ac95e7 "docs: Require a persistent doc-drift precheck worklist" responded by changing only skills/doc-drift-scanner/SKILL.md (+9/−2): it documents --worklist as required and passes it in the example. The script itself was not touched.
  3. 2026-11-04 00:07 — follow-up comment on SKILL.md:82: "So this doc is the right guardrail for now. The durable fix is upstream in doc-drift-precheck.sh." — feat: Add doc-drift program (scanner, fixer, release assist) repoman#12 (comment)
  4. PR head 8838271 — the review thread is still unresolved and not outdated; the vendored script blob (e529801b) is byte-identical to automation@fa619090 and to automation HEAD. No commit on the PR branch has ever changed the WORKLIST default: git log --follow shows only 8d187ae (added it) and 657d4f2 (vendored the engine; its diff touches the merged-PR fetch, not the worklist).

Reproduced on the PR-head script: a bare run exits 0 and prints worklist: /tmp/doc-drift-precheck-sAwXSS/worklist.json; the file no longer exists once the script returns.

So the documentation now tells callers to pass a persistent path, but nothing enforces it — the silent dead end remains, and the doc-only guardrail exists precisely because the code was not fixed.

Impact

  • Fails open in the worst direction. Precheck exits 0 ("there is work"), then the caller reads a file that no longer exists — either an empty model turn or an error inside the consuming skill, depending on how the caller reads it.
  • Silent. No warning, and the usage text (:8, :22) still advertises [--worklist /path] as optional.
  • Not covered by tests. tests/test-doc-drift-precheck.sh always passes --worklist (:56, :106); there is no case for the default path.

Proposal

Pick one (preferred first):

  1. Default to a persistent per-program path instead of /tmp: WORKLIST="${REPORTS_DIR:-$HOME/reports/doc-drift}/worklist.json" with mkdir -p, i.e. the same path the SKILL.md example already uses. --worklist stays as the override.
  2. Fail fast if a temp default is intentional: if [ -z "$WORKLIST" ]; then echo "ERROR: --worklist is required" >&2; exit 2; fi, and make the flag mandatory in the usage text. Exit 2 is already the "broken gate, fail-open for the caller" code.

Either way, update the header and --help to state the real contract, and add a regression case to tests/test-doc-drift-precheck.sh: a bare run must either write to the persistent default or exit non-zero — it must never print a worklist: path that does not exist once the script returns. Once this lands upstream, the re-vendor in rossoctl/repoman#12 can drop the doc-only guardrail and resolve the review thread.

Acceptance criteria

  • A bare invocation never prints a worklist: path that is gone after exit.
  • Usage / --help documents the real contract (required flag, or persistent default).
  • tests/test-doc-drift-precheck.sh covers the no-flag case.
  • The doc note in feat: Add doc-drift program (scanner, fixer, release assist) repoman#12 (skills/doc-drift-scanner/SKILL.md:82-94) can be simplified once this lands.

References

Activity

  1. self-assigned this
    on Nov 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions