Harden existing isTimeoutError against context.DeadlineExceeded without caller obligation

[fullsend-ai-retro]

What happened

While investigating why the code agent produced the context.DeadlineExceeded bug in PR #6217, analysis revealed that the existing isTimeoutError function in internal/forge/github/github.go (around line 295) has the same vulnerability. It uses the Timeout() bool interface check without guarding against context errors.

The function's doc comment states "Callers must check ctx.Err() first," which shifts the burden to callers. The code agent replicated this pattern into the new public forge.IsTransient() function but did not replicate the caller obligation — creating the bug that the review agent caught.

The new IsTransient function now has the correct guard (errors.Is(err, context.DeadlineExceeded) || errors.Is(err, context.Canceled) before the Timeout() check). But isTimeoutError still relies on caller discipline.

What could go better

The caller-must-check-first contract on isTimeoutError is fragile. It already caused a bug by proxy — the code agent treated the function's implementation as a reusable pattern without carrying over the documented caller obligation. Any future caller (human or agent) that uses isTimeoutError without checking ctx.Err() first would silently retry context cancellations.

Confidence: Medium-high. The fragility is demonstrated by the PR #6217 bug. Making isTimeoutError self-contained (adding the context-error guard internally) eliminates the caller obligation and prevents the pattern from propagating incorrectly again.

Uncertainty: The function may have few callers, all of which correctly check ctx.Err() first. In that case the risk is low but the fix is also low-cost. There may also be a performance reason for the current design (avoiding redundant errors.Is checks), though this seems unlikely to matter in practice.

Proposed change

Add context-error guards to isTimeoutError in internal/forge/github/github.go, matching the pattern now used in forge.IsTransient():

func isTimeoutError(err error) bool {
    if errors.Is(err, context.DeadlineExceeded) || errors.Is(err, context.Canceled) {
        return false
    }
    var te interface{ Timeout() bool }
    return errors.As(err, &te) && te.Timeout()
}

Update the doc comment to remove the "Callers must check ctx.Err() first" obligation, since the function is now self-contained. Verify that existing callers still behave correctly (the added guard should be a no-op for callers that already check ctx.Err()).

Validation criteria

1. All existing tests for isTimeoutError and its callers continue to pass.
2. A new test case verifies that isTimeoutError(context.DeadlineExceeded) returns false.
3. The doc comment no longer requires callers to pre-check ctx.Err().
4. go vet and the race detector report no issues.

---

Generated by retro agent from #6217

[fullsend-ai-triage]

🤖 Finished Triage · ✅ Success · Started 5:50 PM UTC · Completed 5:53 PM UTC

Commit: 032e8c4 · View workflow run →

[fullsend-ai-triage]

Related issue: #6424 tracks documenting the Timeout() interface pitfall in Go coding guidelines — independent but complementary to this code fix.

Proposed test case:

// Add to existing isTimeoutError test table
{name: "context.DeadlineExceeded", err: context.DeadlineExceeded, want: false},
{name: "context.Canceled", err: context.Canceled, want: false},
{name: "wrapped deadline", err: fmt.Errorf("wrapped: %w", context.DeadlineExceeded), want: false},

Note: The existing caller at line 198 already checks ctx.Err() before calling isTimeoutError, so the guard will be a no-op for current usage. The value is eliminating the fragile caller obligation and preventing the pattern from propagating incorrectly again.

---

Labels: This is a Go code change in internal/forge (bug category, low severity). Labeling as bug and priority/low since the existing caller is safe and this is defensive hardening.

---

Next steps:

• /fs-code — agent creates a PR to implement this issue
• /fs-code <your instruction> — agent implements with your specific guidance

[fullsend-ai-coder]

🤖 Finished Code · ✅ Success · Started 5:55 PM UTC · Completed 6:07 PM UTC

Commit: b294e1e · View workflow run →