fix(code): ISS-5963 — zero-byte plan.json, absent-workspace fail-open, and a per-command predicate in detect_spurious_complete

plugins/code/.claude-plugin/plugin.json

@@ -1,7 +1,7 @@
 {
   "name": "code",
   "description": "Code and planning framework plugin",
-  "version": "1.14.8",
+  "version": "1.14.9",
   "author": {
     "name": "ClosedLoop",
     "email": "support@closedloop.ai"

plugins/code/scripts/run-loop.sh

@@ -132,6 +132,78 @@ fail_loop_user_visible() {
   exit 1
 }
 
+# Loop commands whose result bundle declares plan.json REQUIRED. Mirrors the
+# ResultBundle manifest that the cloud side validates against (PLAN and
+# REQUEST_CHANGES both require plan.json).
+#
+# EXECUTE is deliberately absent, and that is the load-bearing part: its
+# required artifact is execution-result.json, which is only written after a
+# successful commit AND push, so a legitimate no-changes EXECUTE run ends
+# without it. A blanket "required artifact missing => spurious" rule would fail
+# every one of those runs. Listing the non-plan commands by name below keeps
+# EXECUTE excluded on purpose rather than by the accident of it not carrying a
+# --prd.
+LOOP_COMMANDS_OWING_PLAN="PLAN REQUEST_CHANGES"
+LOOP_COMMANDS_NOT_OWING_PLAN="EXECUTE CHAT EXPLORE REQUEST_PRD_CHANGES DECOMPOSE EVALUATE_PRD GENERATE_PRD EVALUATE_PLAN EVALUATE_CODE EVALUATE_FEATURE BOOTSTRAP MANUAL"
+
+# Normalize a command label to the canonical LoopCommand spelling.
+# The desktop sets CLOSEDLOOP_COMMAND to the wire spelling (PLAN, EXECUTE,
+# REQUEST_CHANGES); a local CLI run resolves it from --prompt instead, whose
+# values are the prompt file names (plan-prompt, execute-prompt). Both map to
+# the same command, so normalize rather than special-case the caller.
+normalize_loop_command() {
+  local raw="${1:-}"
+  raw="${raw%-prompt}"
+  raw="${raw%_prompt}"
+  printf '%s' "$raw" | tr '[:lower:]-' '[:upper:]_'
+}
+
+# Does this run owe a plan.json? Echoes nothing; returns 0 (owes) or 1 (does not).
+# Args: $1 = command label (raw), $2 = prd_file
+#
+# Version skew, both directions: an unrecognized command -- an older desktop
+# sending nothing, or a newer one sending a command this script has never heard
+# of -- is not an error and never blocks. It falls back to the --prd proxy,
+# which is exactly the behaviour before commands were modelled at all.
+run_owes_plan_json() {
+  local command
+  command=$(normalize_loop_command "${1:-}")
+  local prd_file="${2:-}"
+
+  case " $LOOP_COMMANDS_OWING_PLAN " in
+    *" $command "*) return 0 ;;
+  esac
+  case " $LOOP_COMMANDS_NOT_OWING_PLAN " in
+    *" $command "*) return 1 ;;
+  esac
+
+  [[ -n "$prd_file" ]]
+}
+
+# Classify the plan artifact at $1 as: missing | empty | unparseable | present.
+#
+# "Exists" is not "was produced". The incident evidence is literally a 0-byte
+# plan.json: [[ -f ]] passes on it, jq yields nothing, and every pendingTasks
+# check below reads 0 pending tasks and calls the run clean. A file that is
+# absent, zero-byte, or not parseable JSON carries the same fact -- no plan was
+# written -- and all three must be treated as unproduced.
+classify_plan_artifact() {
+  local plan_file="$1"
+  if [[ ! -f "$plan_file" ]]; then
+    printf 'missing'
+    return
+  fi
+  if [[ ! -s "$plan_file" ]]; then
+    printf 'empty'
+    return
+  fi
+  if ! jq -e . "$plan_file" >/dev/null 2>&1; then
+    printf 'unparseable'
+    return
+  fi
+  printf 'present'
+}
+
 # Detect a spurious COMPLETE: the orchestrator's Phase 7 contract forbids
 # emitting <promise>COMPLETE</promise> when plan.json has pending tasks, but
 # it sometimes violates that contract -- typically when tasks are blocked by
@@ -143,6 +215,10 @@ fail_loop_user_visible() {
 #   {"subcode": "...", "message": "..."}   when a violation is detected
 #   {}                                      otherwise
 #
+# Args: $1 = workdir
+#       $2 = prd file    (defaults to $PRD_FILE; tests pass it explicitly)
+#       $3 = command      (defaults to $CLOSEDLOOP_COMMAND; ditto)
+#
 # Caller is responsible for telemetry, cleanup, and invoking
 # fail_loop_user_visible.
 detect_spurious_complete() {
@@ -152,9 +228,23 @@ detect_spurious_complete() {
   # draft a plan from a PRD, which is what makes a missing plan.json a broken
   # promise rather than a run that never owed one.
   local prd_file="${2-${PRD_FILE:-}}"
+  # Defaults to the global the desktop exports; passed explicitly by tests.
+  local command="${3-${CLOSEDLOOP_COMMAND:-}}"
   local plan_file="$workdir/plan.json"
   local state_file="$workdir/state.json"
 
+  # Fail OPEN when the workspace itself is gone. "The artifact was not produced"
+  # and "the workspace no longer exists" are different facts, and only the first
+  # one is evidence of a spurious completion. Live-exit and boot-recovery paths
+  # delete the temp workdir right after finalization, so adjudicating a run
+  # whose directory has already been reclaimed would flip a genuine success into
+  # a failure -- a permanent divergence that no re-run can repair. A missing
+  # workdir means "cannot judge", so judge nothing.
+  if [[ ! -d "$workdir" ]]; then
+    echo '{}'
+    return
+  fi
+
   # Skip the check when the orchestrator emitted COMPLETE as part of an
   # AWAITING_USER_SEQUENCE hard stop (e.g., the Phase 1.1 plan review
   # checkpoint). In those cases pending tasks and open questions are
@@ -170,12 +260,15 @@ detect_spurious_complete() {
     fi
   fi
 
-  if [[ ! -f "$plan_file" ]]; then
-    # A --prd run exists to produce plan.json. Claiming COMPLETE without one is
-    # the strongest spurious-completion signal there is, and this branch used to
-    # wave it through: the checks below only validate pendingTasks INSIDE an
-    # existing plan, so "no plan at all" -- the case that actually happens --
-    # was the one case nothing could catch.
+  local plan_state
+  plan_state=$(classify_plan_artifact "$plan_file")
+
+  if [[ "$plan_state" != "present" ]]; then
+    # A PLAN or REQUEST_CHANGES run exists to produce plan.json. Claiming
+    # COMPLETE without one is the strongest spurious-completion signal there is,
+    # and this branch used to wave it through: the checks below only validate
+    # pendingTasks INSIDE an existing plan, so "no plan at all" -- the case that
+    # actually happens -- was the one case nothing could catch.
     #
     # Observed: the orchestrator launched plan-draft-writer in the BACKGROUND,
     # said "Plan-draft-writer is running in the background. Waiting for
@@ -185,12 +278,30 @@ detect_spurious_complete() {
     # nothing was examined"), and the run exited 0 having produced nothing. The
     # user got an implementation-plan artifact that looked done and was empty.
     #
-    # Scoped to runs that were asked for a plan: a run with no PRD never owed
-    # one, and is left alone.
-    if [[ -n "$prd_file" ]]; then
+    # Scoped per-command: a run that never owed a plan is left alone.
+    #
+    # REQUEST_CHANGES is included, with a known limit: the harness seeds
+    # plan.json before an amend run, so PRESENCE PROVES NOTHING there. This
+    # branch still catches "no plan at all" (including a seeded file truncated
+    # to zero bytes), but it cannot see "the amend ran and produced nothing" --
+    # that needs a pre-run baseline the detector is not given. Do not read a
+    # clean result on a REQUEST_CHANGES run as proof the amend did work.
+    if run_owes_plan_json "$command" "$prd_file"; then
+      local plan_detail
+      case "$plan_state" in
+        empty)
+          plan_detail="plan.json is zero bytes -- the file exists but no plan was ever written into it"
+          ;;
+        unparseable)
+          plan_detail="plan.json is not parseable JSON -- the file exists but no usable plan was written into it"
+          ;;
+        *)
+          plan_detail="no plan.json was ever written"
+          ;;
+      esac
       jq -n -c \
         --arg subcode "PLAN_MISSING_AT_COMPLETION" \
-        --arg message "Loop emitted COMPLETE but no plan.json was ever written. The planning phase did not finish -- a background plan-draft-writer that is still running when the completion promise fires is abandoned. Inspect state.json and the loop output, then re-run /code:code to continue." \
+        --arg message "Loop emitted COMPLETE but $plan_detail. The planning phase did not finish -- a background plan-draft-writer that is still running when the completion promise fires is abandoned. Inspect state.json and the loop output, then re-run /code:code to continue." \
         '{subcode:$subcode,message:$message}'
       return
     fi

plugins/code/scripts/tests/test_spurious_complete.sh

@@ -56,6 +56,11 @@ assert_not_spurious() {
   fi
 }
 
+# The two-argument call sites below default the command to $CLOSEDLOOP_COMMAND,
+# so an ambient value -- this suite may well be run from inside a live loop --
+# would silently decide the per-command branch for them. Clear it first.
+unset CLOSEDLOOP_COMMAND PRD_FILE || true
+
 # shellcheck source=/dev/null
 source "$RUN_LOOP"
 
@@ -106,6 +111,91 @@ assert_subcode "pending tasks blocked by open questions still flagged" \
   "PENDING_TASKS_BLOCKED_BY_QUESTIONS"
 rm -rf "$WORKDIR"
 
+# --- A file that EXISTS but holds no plan is still an unproduced plan --------
+# The incident evidence is literally a 0-byte plan.json. [[ -f ]] passes on it,
+# jq yields nothing, and the pendingTasks checks then read 0 pending tasks and
+# call the run clean -- so the exact condition this guard exists to catch was
+# reported as success.
+WORKDIR=$(mktemp -d)
+: > "$WORKDIR/plan.json"
+assert_subcode "a zero-byte plan.json is treated as missing" \
+  "$(detect_spurious_complete "$WORKDIR" "/tmp/prd.md")" \
+  "PLAN_MISSING_AT_COMPLETION"
+rm -rf "$WORKDIR"
+
+WORKDIR=$(mktemp -d)
+printf '%s' '{"pendingTasks": [' > "$WORKDIR/plan.json"
+assert_subcode "a plan.json that is not parseable JSON is treated as missing" \
+  "$(detect_spurious_complete "$WORKDIR" "/tmp/prd.md")" \
+  "PLAN_MISSING_AT_COMPLETION"
+rm -rf "$WORKDIR"
+
+WORKDIR=$(mktemp -d)
+printf '%s' '   ' > "$WORKDIR/plan.json"
+assert_subcode "a whitespace-only plan.json is treated as missing" \
+  "$(detect_spurious_complete "$WORKDIR" "/tmp/prd.md")" \
+  "PLAN_MISSING_AT_COMPLETION"
+rm -rf "$WORKDIR"
+
+# A run that never owed a plan is still left alone, empty file or not.
+WORKDIR=$(mktemp -d)
+: > "$WORKDIR/plan.json"
+assert_not_spurious "a zero-byte plan.json on a run that owed no plan is not spurious" \
+  "$(detect_spurious_complete "$WORKDIR" "")"
+rm -rf "$WORKDIR"
+
+# --- An absent WORKSPACE is not an unproduced artifact: fail open ------------
+# Live-exit and boot-recovery delete the temp workdir right after finalization.
+# Adjudicating a run whose directory is already reclaimed would flip a genuine
+# success to FAILED, and no re-run repairs that. Missing workdir = cannot judge.
+WORKDIR=$(mktemp -d)
+rm -rf "$WORKDIR"
+assert_not_spurious "an absent workspace fails open rather than reporting spurious" \
+  "$(detect_spurious_complete "$WORKDIR" "/tmp/prd.md" "PLAN")"
+
+# --- Per-command contract: EXECUTE is excluded ------------------------------
+# EXECUTE's required artifact is execution-result.json, written only after a
+# successful commit AND push, so a legitimate no-changes run ends without it --
+# and without a plan.json either. It must never be flagged.
+WORKDIR=$(mktemp -d)
+printf '%s' '{"phase":"Phase 7","status":"IN_PROGRESS"}' > "$WORKDIR/state.json"
+assert_not_spurious "a legitimate no-changes EXECUTE run is not spurious" \
+  "$(detect_spurious_complete "$WORKDIR" "" "EXECUTE")"
+assert_not_spurious "EXECUTE is excluded by name even when a PRD was passed" \
+  "$(detect_spurious_complete "$WORKDIR" "/tmp/prd.md" "EXECUTE")"
+assert_not_spurious "the execute-prompt CLI spelling resolves to EXECUTE" \
+  "$(detect_spurious_complete "$WORKDIR" "/tmp/prd.md" "execute-prompt")"
+rm -rf "$WORKDIR"
+
+# --- Per-command contract: REQUEST_CHANGES owes a plan ----------------------
+# Its result bundle requires plan.json, so "no plan at all" is still caught,
+# with or without a PRD. Presence, however, proves nothing on an amend run --
+# the harness seeds plan.json before it starts -- so a zero-byte seed must be
+# caught too, and a clean result must not be read as proof the amend did work.
+WORKDIR=$(mktemp -d)
+assert_subcode "REQUEST_CHANGES with no plan.json is spurious even without a PRD" \
+  "$(detect_spurious_complete "$WORKDIR" "" "REQUEST_CHANGES")" \
+  "PLAN_MISSING_AT_COMPLETION"
+: > "$WORKDIR/plan.json"
+assert_subcode "REQUEST_CHANGES with a zero-byte seeded plan.json is spurious" \
+  "$(detect_spurious_complete "$WORKDIR" "" "REQUEST_CHANGES")" \
+  "PLAN_MISSING_AT_COMPLETION"
+rm -rf "$WORKDIR"
+
+# --- Version skew, both directions -----------------------------------------
+# A command this script has never heard of (an older desktop sending nothing, a
+# newer one sending a command added after this release) must not crash or block.
+# It falls back to the --prd proxy, i.e. the behaviour before commands existed.
+WORKDIR=$(mktemp -d)
+assert_subcode "an unknown command with a PRD falls back to the --prd proxy" \
+  "$(detect_spurious_complete "$WORKDIR" "/tmp/prd.md" "SOME_FUTURE_COMMAND")" \
+  "PLAN_MISSING_AT_COMPLETION"
+assert_not_spurious "an unknown command with no PRD is left alone" \
+  "$(detect_spurious_complete "$WORKDIR" "" "SOME_FUTURE_COMMAND")"
+assert_not_spurious "an empty command with no PRD is left alone" \
+  "$(detect_spurious_complete "$WORKDIR" "" "")"
+rm -rf "$WORKDIR"
+
 echo
 echo "passed: $PASS_COUNT  failed: $FAIL_COUNT"
 [[ "$FAIL_COUNT" -eq 0 ]]