Skip to content

fix: publish parses gh JSON when color is forced - #91

Merged
lukemaj merged 1 commit into
mainfrom
fix/88-publish-error
Oct 14, 2026
Merged

lukemaj merged 1 commit into
mainfrom
fix/88-publish-error

Conversation

@lukemaj

@lukemaj lukemaj commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

What: publish reads plain JSON from gh when the worker forces terminal color, and names the command and its stderr if that output is empty or not JSON.

Why: This Grok shell sets CLICOLOR_FORCE=1. gh api then colorizes JSON even on a pipe and even with NO_COLOR=1, so json.loads raises Expecting value: line 1 column 1 (char 0).

So what: This head is ready for exact-head Sol and Grok review. The parent merges when those reviews and the checks pass. That merge publishes 1.2.1 through the existing release workflow. Installation stays out of scope.

Requirements and who asked: Issue #88 (user, 2026-10-14): reproduce the worker failure, fix the cause, and make empty or non-JSON subprocess output name the command and stderr. Parent continuation: rebase onto origin/main 69c0f07 (v1.2.0), set the next patch with versionctl, rerun proof, and force-with-lease. Cost-comment text stays as it is. Workflow and repository settings stay as they are.

Deleted: Workflow edits, a live-ledger cost comment, install, nested delegation, an ANSI-stripping parser, and the same change in other gh callers. Only publish failed.

Bottleneck: Rebase version identity. The old commit was 1.1.6; main is 1.2.0. versionctl applied 1.2.1. release-check needed the remote tag v1.2.0, which was not in the local tag list.

Checked myself: Rebased parent is 69c0f07b64d6bc4620ae88f692066d491bd2f2ad. Head is 6e8472b7bae30999e483096b39cb83fa61fb8209. Diff against that base is publish.py, tests/test_publish_gh.py, and the 1.2.1 version files only. Plugin JSON diffs change only version. python3 -m unittest discover -s tests on that tree: 695 tests OK in 87.213s. versionctl release-check --json: ok, version 1.2.1, sha 6e8472b7bae30999e483096b39cb83fa61fb8209. Main checkout remains 67c4221 and was not written.

Fixes #88

Website impact: none. This repository has no site in this change.

Handoff

  • Branch: fix/88-publish-error
  • Workspace: /home/luke/dev/toolboxmd/agent-observer-worktrees/88-publish-error
  • Base: origin/main 69c0f07b64d6bc4620ae88f692066d491bd2f2ad (v1.2.0)
  • Head: 6e8472b7bae30999e483096b39cb83fa61fb8209 (v1.2.1)
  • Proof: full unittest above; publish tests use a fake gh and do not touch GitHub or a ledger. versionctl release-check passed after git fetch origin tag v1.2.0.
  • Review: Opus and Sol passed only 74745c5. Those verdicts are stale. No review/independent success on this head. Parent owns the new Sol and Grok reviews.
  • Authority: commit and push only. No merge, install, workflow edit, or live Observer publish.
  • Delivery: not merged, not released, not installed. Cleanup stays with the parent.
  • Blockers: none in this repair. Independent review and CI on this head are still open.
  • Next: parent exact-head review, green checks, and the authorized merge.

Proof limit: this pass did not POST a cost comment. The earlier live check was gh api user on the same _gh change, before the rebase, and publish.py did not conflict.

@lukemaj

lukemaj commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

Review: Opus (claudeAgent, claude-opus-5-5, medium)

Verdict: pass on 74745c5fe5eb4cb18c755b696a225c6b3fecbf07. No blocking findings.

Opus checked the cause with gh 2.102.0: CLICOLOR_FORCE=1 colorizes gh api JSON and json.loads raises Expecting value; CLICOLOR_FORCE=0 returns plain JSON. Callers of _gh all expect a JSON body on success. The three new tests reject a colored parse, an empty {} success, and a bare Expecting value error.

Notes:

  • GH_FORCE_TTY=1 might still color output when CLICOLOR_FORCE=0.
  • dispatch._gh_json has the same parse and is out of this PR.
  • The _gh docstring is choppy. Style only.

Dispositions

  • GH_FORCE_TTY, rejected. Under the same subprocess capture publish uses, CLICOLOR_FORCE=0 with GH_FORCE_TTY=1 returned plain JSON (1199 bytes, no ANSI, exit 0). This worker does not set GH_FORCE_TTY. No change.
  • dispatch._gh_json, left out of this PR. Same class of bug at agent_observer/dispatch.py (json.loads of gh stdout, no CLICOLOR_FORCE=0). Not part of publish fails with 'Expecting value: line 1 column 1' when run from a delegated Grok worker #88. No live fetch_approval failure was recorded here.
  • Docstring, rejected. No behavior change.

Sol's review of this head is still running. review/independent stays pending until that verdict is recorded.

@lukemaj

lukemaj commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

Review: Sol (codex, gpt-6.1-sol, high)

Verdict: pass on 74745c5fe5eb4cb18c755b696a225c6b3fecbf07 only. No blocking findings.

Sol confirmed the worktree HEAD and base, reviewed the whole diff and every _gh caller, and saw comment contents and adapters unchanged. The focused publish tests passed. Faulty implementations that still parse colored stdout, return {} for empty stdout, or raise only Expecting value each failed the matching test. Sol did not repeat a live post. The regression tests use a fake gh.

Dispositions

  • No change. The notes confirm the diff and the tests. Nothing to fix.

Both reviewers pass this head: Opus (claudeAgent, claude-opus-5-5, medium) and Sol (codex, gpt-6.1-sol, high). No blocking findings on either.

@lukemaj

lukemaj commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

Agent work on this PR

Estimated cost unknown · 32 responses · 37 sessions · 2.4 h wall time

Model Responses Tokens Estimated cost
grok-4.7 (grok, medium) 28 40,921,226 unknown
grok-4.7 (grok, xhigh) 2 7,585,054 unknown
grok-4.7 (grok, high) 2 unknown unknown

Flags: 2 human corrections · 12 large tool outputs · 1 permission request · 1 repeated command · 1 repeated failure · 117 repeated reads · 14 repeated skill loads · 1 test edited after a failure · 2 sessions without usage records · 32 unpriced responses

Details: snapshot, prices, coverage, counters
  • Task toolboxmd/agent-observer#88: outcome unknown (recorded acceptance only; a finished process never implies it).
  • Proof: publish fails with 'Expecting value: line 1 column 1' when run from a delegated Grok worker #88
  • Snapshot 09b21210e5203467c80731e56c8c1ccaaa5ef145d55b940bda8dbd9e3d28696e, records up to 2026-10-14 13:50 UTC.
  • 37 sessions on claude, codex, grok; AgentsMD unknown.
  • The total includes work shared with 8 other tasks, counted whole in each, so sums across PRs overlap.
  • Not counted: 669 responses (at least $49.26) in sessions shared with other PRs that worked in no single PR's checkout.
  • 80 responses in these sessions worked on other PRs and are counted there.
  • Totals reconcile with the measured sessions: yes. Evidence complete: no.
  • Prices: list-price estimate from T3 local rate table (path withheld) as of 2026-10-12, schedule 5a9558d9512b93fd1fc1405312fe3fdc2a438448a15846a05c354fe2851d74e6. Unknown prices stay unknown, never zero.
    • T3 LiteLLM rate table when present; bundled schedule covers the rest.
    • Model price sources: T3 local rate table (path withheld).
    • Unpriced: unknown model (32).
  • Native session usage or worker ownership is unavailable.
  • Harness-reported cost: none reported.
  • Usage totals are not billing. Subscription spending is separate and is never posted as spend.
  • Crashed runs are counted separately: 0.

Token counters by model (native counter semantics; never added across semantics):

  • grok-4.7 (grok, medium), grok counters: input includes cached, output includes reasoning: input 40,486,791; cache-read 37,995,264; cache-write 0; output 434,435; reasoning 278,118; harness total 40,921,226.
  • grok-4.7 (grok, xhigh), grok counters: input includes cached, output includes reasoning: input 7,479,479; cache-read 7,243,136; cache-write 0; output 105,575; reasoning 69,338; harness total 7,585,054.
  • grok-4.7 (grok, high), grok counters: input includes cached, output includes reasoning: input unknown; cache-read unknown; cache-write unknown; output unknown; reasoning unknown; harness total unknown.

Generated tokens (reasoning and other output are disjoint; other output includes code, tool calls and replies):

Model Reasoning Other output Reasoning share Split known for
grok-4.7 (grok, medium) unknown unknown unknown 0 of 28
grok-4.7 (grok, xhigh) unknown unknown unknown 0 of 2
grok-4.7 (grok, high) unknown unknown unknown 0 of 2

Selected rates (USD per million tokens). These rates value the report at the selected schedule date; they do not establish historical prices or subscription spending.

Model Input tier Input Cache read Cache write Other output Reasoning
grok-4.7 all unknown unknown unknown unknown unknown

Other output and reasoning are priced without double counting inclusive native output. Missing rates remain unknown.

Local measurement from native records; usage totals are not billing. Updated in place by agent-observer publish.

CLICOLOR_FORCE makes gh colorize API JSON on a pipe, so publish raised
Expecting value. Turn that force off, and name the command and stderr
when the output is empty or not JSON.
@lukemaj
lukemaj force-pushed the fix/88-publish-error branch from 74745c5 to 6e8472b Compare October 14, 2026 18:55
@lukemaj

lukemaj commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

What: PASS for PR #91, with no supported blocking findings.
Why: The exact candidate fixes forced-color JSON parsing, preserves comment content, and reports empty or invalid output with command and stderr.
So what: Parent can aggregate this unchanged verdict with the second review and publish the combined comment/status.

Reviewer: gpt-6.1-sol, high effort, Codex harness through T3 Code.

  • Base: 69c0f07b64d6bc4620ae88f692066d491bd2f2ad
  • Head: 6e8472b7bae30999e483096b39cb83fa61fb8209
  • Version: 1.2.1
  • Inspected the complete diff, Issue/PR bodies and comments, repository instructions/direction/README/glossary, and every reachable publish caller. Unrelated gh callers are unchanged.

Independent proof:

  • PYTHONDONTWRITEBYTECODE=1 python3 -m unittest -v tests.test_publish_gh tests.test_publish tests.test_cli.PublishDryRunJsonTest tests.test_cli.PackageVersionTest tests.test_semantics: 34 tests passed.
  • Ran the three new regressions against base publish.py, loaded in memory: one failure and two errors, reproducing silent empty-output success and bare JSON errors. All three pass on head.
  • Real, read-only gh api repos/toolboxmd/agent-observer checks using gh 2.102.0 reproduced ANSI-contaminated JSON under CLICOLOR_FORCE=1, including with NO_COLOR=1. Candidate _gh returned plain, parseable JSON in all four tested combinations of forced color, NO_COLOR, and GH_FORCE_TTY, preserving the parent environment.
  • Exercised real post/_gh code with the subprocess boundary mocked: PR creation/update and commit-comment creation preserve the exact body. Empty, whitespace-only and non-JSON results include command, stderr or (no stderr).
  • Every publish function except _gh is AST-identical to base.
  • Canonical versionctl release-check --json: ok, exact head and all three plugin mirrors at 1.2.1.
  • CI log: 695 tests passed, compilation passed, version 1.2.1. Its merge candidate 157b5b8ff7ff1794d8f1a33cef442c24e7526875 has the requested base/head parents and the same tree as head: 189a00379f3bb979ff3358daa7015110985b0fb2.
  • Final worktree status is clean; local and live PR identities remain unchanged.

Limits: No live-ledger execution, GitHub POST/PATCH, delegated Grok publish, or local full-suite rerun. No repository or GitHub mutations; publication acceptance remains outside this review’s proof.

Requirements and who asked: User and Issue #88 require reliable publication, actionable JSON errors, preserved comment content, and independent exact-head review, supporting trustworthy GitHub aggregates.
Deleted: Broader caller changes, implementation, nested delegation and unauthorized live publication.
Bottleneck: Fresh proof for the rebased candidate.
Checked myself: Complete diff/callers, baseline failures, 34 passing tests, real gh color behavior, payload preservation, version validation, CI tree identity and clean final state.

@lukemaj

lukemaj commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

What: PASS on 6e8472b. publish reads plain JSON from gh when this Grok shell forces color, and an empty or non-JSON response names the command and its stderr.

Why: CLICOLOR_FORCE=1 makes gh 2.102.0 colorize gh api JSON on a pipe, even with NO_COLOR=1, so json.loads raises Expecting value: line 1 column 1 (char 0). _gh now sets CLICOLOR_FORCE=0 on that child process.

So what: No blocking finding. The parent records this verdict and review/independent success on this exact SHA. Two non-blocking follow-ups are below. Merge, release, and install stay with the parent.

Requirements and who asked: Independent exact-head review of PR 91 for Issue #88 (user, 2026-10-14): plain JSON under forced color, command and stderr on empty or non-JSON output, cost-comment text preserved, no edits to other gh callers, version 1.2.1. Read-only.

Deleted: Reuse of the 74745c5 reviews, a live ledger, a GitHub publish POST, GitHub writes, new worktrees, and implementation.

Bottleneck: Whether CLICOLOR_FORCE=0 makes installed gh 2.102.0 emit JSON in this color-forced shell.

Checked myself: Exact base and head, the full diff, every _gh caller, a live gh api user comparison, focused tests, CI run 38619294833 on this SHA, and versionctl release-check.

Model and harness: grok-4.7, Grok harness in T3 Code. High effort.

Verdict: PASS

Base: 69c0f07b64d6bc4620ae88f692066d491bd2f2ad
Head: 6e8472b7bae30999e483096b39cb83fa61fb8209
Worktree: /home/luke/dev/toolboxmd/agent-observer-worktrees/88-publish-error, clean, matching origin/fix/88-publish-error.

Checked behavior:

  • This shell sets CLICOLOR_FORCE=1, NO_COLOR=1, CLICOLOR=1, FORCE_COLOR=1, and GH_PAGER=cat. GH_FORCE_TTY is unset. Raw gh api user exits 0 with 2599 ANSI bytes and JSONDecodeError: Expecting value: line 1 column 1 (char 0) on empty stderr. publish._gh(["user"]) returns the user object for the authenticated login.
  • tests.test_publish_gh plus tests.test_publish: 17 tests OK in 0.120s on Python 3.14.7. The fake gh covers colored stdout, empty stdout (gh api user and keyring locked), and non-JSON stdout (color forced). The messages do not contain Expecting value.
  • The base-to-head diff changes _gh and adds that test. render() and the comment body passed to post() are unchanged, and the existing render assertions passed. The other diff files are VERSION, CHANGELOG.md, and the three plugin mirrors at 1.2.1.
  • Reachable _gh callers are login, comment listing, PATCH, and POST. A comments list still stops on a parsed []. A truly empty body now fails closed with the command and stderr. dispatch._gh_json is outside the diff.
  • bin/agent-observer --version prints 1.2.1. versionctl release-check --version 1.2.1 --sha 6e8472b7bae30999e483096b39cb83fa61fb8209 --json is ok: mirrors consistent, changelog date 2026-10-14, tag v1.2.1 not created yet. v1.2.0 points at the base.
  • CI proof run 38619294833 is success on this SHA. That job runs python3 -m unittest discover -s tests, compileall, and bin/agent-observer --version on Python 3.13.

Follow-ups for the parent, both non-blocking:

Limitations: no ledger run and no GitHub publish POST. Empty and non-JSON coverage uses a fake gh. The full 695-test suite was inspected via CI for this SHA and was not re-run here. Local proof used Python 3.14.7. The worktree was left clean.

@lukemaj

lukemaj commented Oct 14, 2026

Copy link
Copy Markdown
Contributor Author

Both exact-head reviews PASS on 6e8472b: Sol #91 (comment) and Grok #91 (comment). Non-blocking remaining color combinations are tracked in #93. All checks are green; proceeding with the user-authorized matched-head squash merge and automatic release.

@lukemaj
lukemaj merged commit a4ba7ce into main Oct 14, 2026
2 checks passed
@lukemaj
lukemaj deleted the fix/88-publish-error branch October 14, 2026 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

publish fails with 'Expecting value: line 1 column 1' when run from a delegated Grok worker

1 participant