Skip to content

feat(B-0581): decompose slice 1 - gh auth refresh wrapper script#3979

Merged
AceHack merged 4 commits into
mainfrom
lior/decompose-b0581-slice1
May 19, 2026
Merged

feat(B-0581): decompose slice 1 - gh auth refresh wrapper script#3979
AceHack merged 4 commits into
mainfrom
lior/decompose-b0581-slice1

Conversation

@AceHack
Copy link
Copy Markdown
Member

@AceHack AceHack commented May 16, 2026

Slices off the first operational step of B-0581 from PR #3961 to adhere to the bias for action and prevent shadow/metadata drift. Implements the script to spawn gh auth refresh, pump Y/n, and prominently capture the one-time code.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e8f4714a5d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread tools/auth/gh-auth-refresh-wrapper.ts Outdated
Comment thread tools/auth/gh-auth-refresh-wrapper.ts Outdated
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new Bun TypeScript wrapper script that spawns gh auth refresh, auto-answers the Y/n Git-credentials prompt, and surfaces the one-time device-flow code prominently in stdout. It is the first sliced operational deliverable of B-0581 (a skill wrapping the gh auth refresh interactive flow that Otto's Bash tool cannot drive directly).

Changes:

  • New script tools/auth/gh-auth-refresh-wrapper.ts that pipes child stdio, pumps Y\n on the Authenticate-Git prompt, and highlights the captured one-time code with a banner.
  • Exits with the underlying gh process's exit code.
Comments suppressed due to low confidence (4)

tools/auth/gh-auth-refresh-wrapper.ts:58

  • The Promise.all([...]) on line 51 is not awaited — it's fire-and-forget. Once proc.exited resolves on line 56, process.exit(exitCode) runs immediately and the still-pending stream reads can be cut off, losing the final lines of gh's output (which may include the one-time code or success/error messages, since the regex match on line 35 depends on the full line being present in a single decoded chunk). Capture the promise in a variable and await it after proc.exited resolves so all buffered output is flushed before exit.
  Promise.all([
    handleOutput(proc.stdout, false),
    handleOutput(proc.stderr, true)
  ]).catch(console.error);

  const exitCode = await proc.exited;
  console.log(`\ngh auth refresh exited with code ${exitCode}`);
  process.exit(exitCode);

tools/auth/gh-auth-refresh-wrapper.ts:47

  • The prompt match on line 29 and the one-time-code regex on line 35 both run on a single decoder.decode(chunk) of an individual stream chunk. Stream chunk boundaries from a child process are not aligned to lines — the substring "First copy your one-time code: ABCD-1234" (or the Y/N prompt) can easily be split across two chunks, in which case neither chunk matches and the code is silently missed / the prompt never gets answered. Since reliably capturing the one-time code is the whole point of this wrapper (per the PR description), accumulate decoded output in a buffer and run the matchers against the buffer, trimming once a match fires or a newline is seen. Also pass { stream: true } to decoder.decode to avoid mangling multi-byte UTF-8 sequences split across chunks.
    for await (const chunk of stream) {
      const text = decoder.decode(chunk);
      out.write(text);
      
      // Look for the Y/N prompt
      if (text.includes("? Authenticate Git with your GitHub credentials?")) {
        proc.stdin.write("Y\n");
        proc.stdin.flush();
      }

      // Look for the one-time code
      const codeMatch = text.match(/! First copy your one-time code: ([A-Z0-9-]+)/);
      if (codeMatch && !codeCaptured) {
        const code = codeMatch[1];
        codeCaptured = true;
        console.log(`\n\n`);
        console.log(`========================================================`);
        console.log(`🚀 ONE-TIME CODE CAPTURED: ${code} 🚀`);
        console.log(`========================================================`);
        console.log(`\n`);
        
        // Optionally pump Enter if the process expects it, but wait for user to copy.
        // The script just needs to surface it prominently for now.
      }

tools/auth/gh-auth-refresh-wrapper.ts:17

  • spawn with cmd: ["gh", ...] resolves the gh binary from $PATH, which sonarjs flags as sonarjs/no-os-command-from-path in this repo (see existing usages in tools/github/poll-pr-gate.ts and tools/cold-start-check.ts that carry a // eslint-disable-next-line sonarjs/no-os-command-from-path directive with a rationale comment). Add the disable comment with a short justification, or eslint --report-unused-disable-directives (which is "error" per eslint.config.ts:103) and the sonarjs rule will both bite once this lands in CI.
  const proc = spawn({
    cmd: ["gh", "auth", "refresh", "-h", "github.com", "-s", scopes],
    stdin: "pipe",
    stdout: "pipe",
    stderr: "pipe",
  });

tools/auth/gh-auth-refresh-wrapper.ts:58

  • proc.exited in Bun resolves to number | null (null when the process was signalled). Passing that directly to process.exit(exitCode) triggers a TypeScript strict-mode error under this repo's strictTypeChecked eslint config, and at runtime a null exit code coerces to exit 0, masking signal-killed failures. Coalesce to a non-zero fallback when null (e.g. exitCode ?? 1).
  const exitCode = await proc.exited;
  console.log(`\ngh auth refresh exited with code ${exitCode}`);
  process.exit(exitCode);

Comment thread tools/auth/gh-auth-refresh-wrapper.ts Outdated
@AceHack AceHack enabled auto-merge (squash) May 19, 2026 00:50
@AceHack
Copy link
Copy Markdown
Member Author

AceHack commented May 19, 2026

3-thread fix landed via REST contents API (commit 31086ff)

Addressed all 3 unresolved reviewer threads in a single targeted edit to tools/auth/gh-auth-refresh-wrapper.ts:

Fix 1 — Codex P1 (line 31): Buffer prompt text before matching

Issue: Matching "? Authenticate Git with your GitHub credentials?" against each raw chunk could miss the prompt when gh splits output across chunk boundaries.

Fix: Introduced buffer accumulator inside handleOutput. The full prompt string is checked against the accumulated buffer rather than per-chunk text. Added promptHandled guard to ensure we only respond once, and buffer-trim after consumption + 16KB bounded growth.

Fix 2 — Codex P2 (line 54): Await output pump completion before exiting

Issue: Promise.all([...]).catch(...) was fired but never awaited; process.exit(exitCode) could terminate before trailing stdout/stderr (including the one-time code banner) was forwarded.

Fix: Stored the handlers promise as handlersPromise; added await handlersPromise between await proc.exited and the final exit, so output pumps drain before we exit.

Fix 3 — Copilot convention (line 3 + 4 other locations): main() + import.meta.main guard

Issue: This file unconditionally invoked main() and called process.exit at module load — untestable from another Bun module.

Fix: Changed signature to export async function main(): Promise<number> (returns exit code instead of calling process.exit); wrapped invocation in if (import.meta.main) { ... } block that propagates the returned code via process.exit(code) only when run as a script. Matches the pattern used in tools/backlog/generate-index.ts, tools/budget/daily-cost-report.ts, tools/archive/archive-pr-reviews.ts.

Landing path

Commit 31086ff landed via GitHub REST contents API (PUT /repos/{owner}/{repo}/contents/tools/auth/gh-auth-refresh-wrapper.ts) from an Otto-CLI session under dotgit-saturation (see PR #4276 for the rule-documenting this circumvention pattern). The REST-PUT route bypasses local git push which has been stalling silently at the receive-pack phase under sustained multi-Otto saturation.

🤖 Generated with Claude Code

Co-Authored-By: Claude noreply@anthropic.com

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 31086ffee7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread tools/auth/gh-auth-refresh-wrapper.ts
Copilot AI review requested due to automatic review settings May 19, 2026 00:55
@AceHack
Copy link
Copy Markdown
Member Author

AceHack commented May 19, 2026

Follow-up fix cb81f732 — Codex post-code Enter prompt (P1)

After the first fix landed (31086ff), Codex re-reviewed and surfaced a NEW P1: the wrapper never pumps the \n for gh's post-code prompt (Press Enter to open github.com in your browser...), so device-flow auth hangs after the banner prints.

Fix in cb81f732: added proc.stdin.write("\n"); proc.stdin.flush(); immediately after the code-capture banner inside the codeMatch block. Includes inline comment documenting WHY (gh's flow requirement). Resolves the thread.

All 4 unresolved threads now addressed:

  • 3 original threads (Codex P1 buffer, Codex P2 await, Copilot main()-export) — fixed in 31086ff; resolved as outdated after the file changes
  • 1 follow-up thread (Codex P1 post-code Enter) — fixed in cb81f732; resolved

Auto-merge remains armed; will fire when CI passes (the 2 baseline lints tick-shard relative-paths + tsc tools are non-required, same as peer PRs that have shipped through this session).

🤖 Generated with Claude Code

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment thread tools/auth/gh-auth-refresh-wrapper.ts Outdated
@AceHack
Copy link
Copy Markdown
Member Author

AceHack commented May 19, 2026

Forward signal — BLOCKED-not-merging anomaly

After cb81f732 (follow-up fix) + all 4 review threads resolved + CI complete, this PR remains in mergeStateStatus: BLOCKED state with auto-merge armed but not firing. Posting observation for substrate-engineer pickup:

What's true

  • 30 SUCCESS checks; CI complete; no in_progress
  • All 4 reviewer threads resolved
  • Auto-merge armed (enabledAt: 2026-05-19T00:49Z)
  • mergeable: MERGEABLE
  • 2 lint failures: lint (tick-shard relative-paths) + lint (tsc tools) — both confirmed baseline failures on origin/main, NOT introduced by this PR
    • tsc tools failure is in tools/backlog/lint-frontmatter.ts:125-178 (10+ TS2345/TS2532 errors), not in this PR's tools/auth/gh-auth-refresh-wrapper.ts

What's strange

Peer PRs with identical baseline-failure shape merged through within ~2min of CI completion this session:

  • PR #4255 (peer Otto-CLI tick shard) — MERGED
  • PR #4256 (peer Otto-CLI tick shard) — MERGED
  • PR #4276 (my rule edit) — MERGED

This PR has been BLOCKED-not-merging for >10min after CI completed.

Hypothesis

Branch protection has:

  • required_approving_review_count: 0
  • required_status_checks not enabled
  • required_conversation_resolution: enabled (satisfied)

No apparent rule should block. Possibly:

  1. tsc tools baseline failure detection treats tools/ edits differently when the PR touches tools/ paths even if it doesn't introduce the failure (peer PRs were docs-only, didn't touch tools/)
  2. Some merge-queue or auto-merge implementation detail with stale-review-dismissal interacting with my follow-up commit
  3. GitHub UI/state lag — may merge later

Recommended next steps

  • Substrate-engineer: investigate why baseline tsc tools failure (in unrelated file tools/backlog/lint-frontmatter.ts) is treated as merge-blocking for tools-touching PRs but not docs-only PRs
  • If the tsc tools baseline needs fixing factory-wide, that unblocks a class of PRs (not just this one)
  • Manual merge override may be needed if anomaly persists

🤖 Generated with Claude Code

@AceHack AceHack force-pushed the lior/decompose-b0581-slice1 branch from cb81f73 to 4aa1e02 Compare May 19, 2026 01:16
…ncurrent-stream state corruption

TextDecoder is stateful — sharing one across two concurrent
handleOutput async loops (stdout + stderr) can corrupt output
when partial multi-byte sequences from one stream prepend to the
next chunk of the other.

Resolves Copilot review thread on PR #3979.

Co-Authored-By: Claude <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 19, 2026 02:11
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

@AceHack AceHack merged commit cfbcacd into main May 19, 2026
33 of 34 checks passed
@AceHack AceHack deleted the lior/decompose-b0581-slice1 branch May 19, 2026 02:13
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.

2 participants