Skip to content

codex skill: unclosed ```bash fence at :1582 inverts the rest of the file (also in .tmpl), and the JSONL parser reports turn.failed as a disconnect #2671

Description

@dk96-creator

Repo: gstack 1.67.1.0 · File: ~/.claude/skills/gstack/codex/SKILL.md (+ its .tmpl) · Env: codex-cli 0.149.0, zsh 5.9, macOS 26.5.2 arm64

Two low-severity items in the codex skill, filed together so they can be closed together. Neither is a safety issue — filing them because they're cheap to fix and one of them has a natural home in #1667.


1. Unclosed ```bash fence at :1582 inverts every fenced region after it

SKILL.md has 95 fence lines — an odd count. The break is at :1582, the consult resumed-session block:

1581  For a **resumed session** (user chose "Continue"):
1582  ```bash
1583  _REPO_ROOT=$(git rev-parse --show-toplevel) || { echo "ERROR: not in a git repo" >&2; exit 1; }
...
1606    fi          ← the bash ends here, but no closing fence follows
1607
1608  5. Capture session ID from the streamed output. The parser prints `SESSION_ID:<id>`
1609     from the `thread.started` event. Save it for follow-ups:
1610  ```bash        ← parsed as the CLOSE of 1582, not as a new block

Two consequences:

  • Step 5's instruction prose (:1607–1609) renders inside a bash block.
  • Every fenced region after :1582 is inverted, so the tail of the file — 119 lines, :1638–1756 — renders as code. That tail includes the model-selection guidance and the --xhigh notes, i.e. prose a reader is meant to act on.

The same imbalance is in the template: codex/SKILL.md.tmpl has 55 fence lines, also odd. So this originates upstream of generation, and per your earlier note that edits to the emitted SKILL.md don't survive an upgrade, the template is the place to fix it.

Suggested fix: add the missing bare ``` after :1606. A cheap regression guard is a fence-parity assertion over each generated SKILL.md — which is arguably in scope for #1667 (validate bash syntax in SKILL.md code blocks during bun test); an unbalanced fence is the failure mode that makes that check necessary in the first place, since a swallowed block means the "bash" being validated isn't the bash that ships.


2. The JSONL parser has no turn.failed branch, so a failed turn is reported as a disconnect

turn.failed appears 0 times in the file. The stream parser handles turn.completed and nothing else at that level:

        elif t == 'turn.completed':
            turn_completed_count += 1
            usage = obj.get('usage',{})
            ...
    except: pass
# Fix 2: completeness check — warn if no turn.completed received
if turn_completed_count == 0:
    print('[codex warning] No turn.completed event received — possible mid-stream disconnect.', flush=True, file=sys.stderr)

A turn.failed event therefore falls through the elif chain, turn_completed_count stays 0, and the completeness check fires — so a turn that failed for a stated reason is reported to the user as "possible mid-stream disconnect."

In fairness, this is a good deal better than silence: "Fix 2" does catch the absence, and the run is not misreported as success. The defect is diagnostic quality, not detection — the user is pointed at the network when the event stream carried the actual cause.

Suggested fix: add an elif t == 'turn.failed': branch that surfaces the error payload, and have the completeness check distinguish "failed with a reason" from "nothing arrived." Those are different problems and want different next steps from the reader.


Both verified by reading the shipped file at 1.67.1.0. Neither affects the review gate or the sandbox — those were the subject of #2524 and both read correct now; thank you for the fix.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions