workflow-fix: AST lint for discarded upload-helper returns (scripts/)
Highlight text on any card (body, plan, or an event) to anchor a comment, or leave a whole-task comment here. Mention @claude to summon a reply.
No comments yet.
Overview / Motivation
Auto-filed by the workflow-fix-on-bug protocol from a workflow-fix candidate
raised on task #2054 (emitting agent: code-reviewer, review round 10, marker
epm:code-review v6).
Goal
Add an AST lint check (bundled into the no-flags default run) that flags
Expr-statement (discarded-return) calls to _upload_folder_filtered /
_upload under scripts/, with a waiver comment for deliberate fail-soft
callers.
Workflow gap
- Bug observed: six discard-shaped upload-helper call sites reached main across issue2054 phase scripts despite full review rounds; a fail-soft-by-return upload helper whose return is discarded logs success and exits 0 on silent durability loss.
- Why it is a workflow gap:
src/explore_persona_space/orchestrate/hub.py's_upload_folder_filteredis fail-SOFT by return on every failure shape (missing token → warn +""; incomplete expected-set verify →logger.error+""; terminalexcept Exception→""). Any caller that discards the return converts a durability failure into a false-success exit 0 — the exact class.claude/rules/upload-policy.mdbans ("'upload returned no path' is a TRACKED GAP ... never a warning-and-continue"). Reviews caught the class only by hand (task #2054 review rounds 9-10, which fixed 3 sites and enumerated 6 more pre-existing on main); no mechanical check exists. The adjacentcheck_no_upload_or_true(#841) covers the SHELL-side suppression (|| trueon upload lines) but not this Python-side discarded-return shape. - Confidence (emitter): high (reviewer marked the finding
Mechanizable: yeswith the check sketch; 6 live offenders enumerated by exact grep + git-provenance-probed as pre-existing on main). - verified-at-filing:
grep -rnE '^\s*_upload_folder_filtered\(|^\s*_upload\(' scripts/→ 12+ bare-statement call-shape hits in 11 files at the main checkout (2026-08-05; issue2054_capture.py:943, issue2054_phase_a.py:1067/:1089, issue2054_phase_c.py:445, plus issue2054_phase_b/phase_d/fits/ladder sites enumerated inepm:code-reviewv6 on #2054, plus issue1481_marker.py / issue825_gen_conversations.py sites whose local_uploadhelpers need per-site fail-soft classification — the grep is a line-anchored approximation; the AST check is the true classifier). Absence probe:grep -inE 'upload.*(discard|return)|discard.*upload' scripts/workflow_lint.py→ 1 incidental hit at :4309, the docstring of the SHELL-sidecheck_no_upload_or_true(#841) — no Python-discarded-return check exists. Landed-fix history:git log --oneline --since='7 days ago' -- scripts/workflow_lint.py→ 5 commits, none touching this class.
Proposed change (candidate diff sketch — refine in planning)
+ def check_upload_return_discard(paths: list[Path]) -> list[str]:
+ # AST pass over scripts/**.py: flag ast.Expr nodes whose value is a
+ # Call to _upload_folder_filtered / _upload (name or attribute form) —
+ # the return carries the success/failure verdict and MUST be consumed
+ # (capture-and-raise per hub.py upload_raw_completions_to_data_repo).
+ # Waiver: a trailing `# lint: upload-fail-soft-ok — <reason>` comment
+ # on the call line exempts deliberate fail-soft callers.
+ # Grandfather: seed an allowlist from today's offenders (or fix them
+ # in the same round) so the no-flags run stays green on main.
# bundle into the no-flags default run + add pin tests in
# tests/test_workflow_lint.py (live-tree pass + synthetic offender FAIL)
Scope / surfaces
- Primary target:
scripts/workflow_lint.py - Secondary:
tests/test_workflow_lint.py(pin tests: synthetic offender FAILs, waivered caller passes, live tree green). - Grep the workflow surface for the pattern before editing
(
grep -rnE '^\s*_upload_folder_filtered\(|^\s*_upload\(' scripts/) and decide per site: fix (capture-and-raise), waiver, or grandfather-allowlist; list them in the plan. NOTE: the 6 issue2054 sites may already be fixed by task #2054's in-flight r11 round by the time this session runs — re-grep at plan time; the lint must be green on the tree it lands on (the #1145/#931 class), so fix-or-waive every residual offender in the same round.
Constraints / invariants
- Workflow-surface only — the lint + its tests; residual offender fixes in
scripts/issue*_*.pyare allowed ONLY as the minimal capture-and-raise one-liners needed to land the check green (the #2054 r10 pattern, hub.py:2161), never behavioral restructures. scripts/workflow_lint.py --check-askspasses; ruff on touched files passes; the no-flags default run stays green on the landing tree.- This session runs under
EPM_WORKFLOW_FIX_SESSION=1and carries aworkflow_fix_target:Provenance line — it MUST NOT auto-route any of its own subagents' workflow-fix candidates (recursion guard,.claude/rules/workflow-fix-on-bug.md§ Recursion guard).
Provenance
- workflow_fix_target: scripts/workflow_lint.py
- fingerprint: f4319be4f5f9
Origin candidate (synthesized from epm:code-review v6 on task #2054, Major
finding + "Mechanizable: yes" line): "the v5-sketched AST lint (flag
Expr-statement calls to _upload_folder_filtered / _upload under
scripts/, waiver comment for deliberate fail-soft callers) would have caught
all 6; still recommended as a workflow-fix candidate for whoever owns the next
lint pass."