Skip to content

Improve context management#507

Merged
gtrrz-victor merged 7 commits intomainfrom
improve-context-management
Feb 26, 2026
Merged

Improve context management#507
gtrrz-victor merged 7 commits intomainfrom
improve-context-management

Conversation

@pfleidi
Copy link
Contributor

@pfleidi pfleidi commented Feb 26, 2026

Summary

  • Thread context.Context consistently through all function call chains, replacing ~25 instances where context.Background() or context.TODO() was used despite a caller-provided ctx being available
  • Add ctx context.Context as the first parameter to all internal functions that previously created their own background contexts, ensuring cancellation signals and context values propagate correctly from Cobra command handlers through helpers, strategy, checkpoint, session, and settings packages
  • Preserve 2 justified exceptions annotated with //nolint:contextcheck: sync.Once initialization in manual_commit.go and a SetLogLevelGetter callback in config.go where no caller context is available

Motivation

The contextcheck linter (disabled until now) flagged ~25 broken context chains where functions had access to a caller-provided ctx but instead created context.Background(). This means cancellation signals, deadlines, and tracing values from the CLI's Cobra commands would not propagate to lower-level git operations,
session state management, or settings loading. Fixing these chains is a prerequisite to enabling the contextcheck linter in CI.

What changed

Core pattern applied across the codebase:

  // Before: broken chain
  func helper() error {
      s, err := settings.Load(context.Background())
      ...
  }

  // After: ctx threaded through
  func helper(ctx context.Context) error {
      s, err := settings.Load(ctx)
      ...
  }

Packages updated:

  • cmd/entire/cli/ — command handlers, lifecycle, hooks, state management, setup, status, explain, resume
  • cmd/entire/cli/strategy/ — manual commit strategy, condensation, hooks, content overlap, rewind, reset, logs, git operations
  • cmd/entire/cli/checkpoint/ — temporary and committed checkpoint storage
  • cmd/entire/cli/session/ — state store, phase management
  • cmd/entire/cli/settings/ — settings loading
  • All corresponding *_test.go files updated to pass context.Background() to changed function signatures

Test plan

  • mise run fmt — no formatting issues
  • mise run lint — 0 lint issues (including contextcheck on changed code)
  • mise run test:ci — all unit and integration tests pass
  • Merged with latest main and resolved all conflicts (main had parallel optimization work adding variadic opts params and pre-resolved tree caching)

Entire-Checkpoint: 827909577c94
Entire-Checkpoint: 40c25a7c8270
Entire-Checkpoint: 56150bf41459
…ement

# Conflicts:
#	cmd/entire/cli/checkpoint/temporary.go
#	cmd/entire/cli/root.go
#	cmd/entire/cli/strategy/common.go
#	cmd/entire/cli/strategy/content_overlap.go
#	cmd/entire/cli/strategy/content_overlap_test.go
#	cmd/entire/cli/strategy/manual_commit_condensation.go
#	cmd/entire/cli/strategy/manual_commit_hooks.go
@pfleidi pfleidi requested a review from a team as a code owner February 26, 2026 01:50
Copilot AI review requested due to automatic review settings February 26, 2026 01:50
@cursor
Copy link

cursor bot commented Feb 26, 2026

PR Summary

Medium Risk
Broad signature changes across core CLI, agent, and checkpoint code raise integration risk (missed call sites/incorrect ctx plumbing), but behavior is largely unchanged aside from improved cancellation/log context propagation.

Overview
Threads context.Context through the agent abstraction and registry, hook install/uninstall/detection, transcript chunking/prep, and OpenCode CLI invocations, replacing internal context.Background() usage so deadlines/cancellation propagate.

Propagates ctx through checkpoint writing/reading paths (including transcript chunking/reassembly and shadow-branch operations) and through clean/settings helpers (LoadEntireSettings, Save*, IsEnabled, GetAgentsWithHooksInstalled), updating all affected tests/benchmarks to pass a context; remaining context.TODO() usage is limited to callback-only call sites.

Written by Cursor Bugbot for commit 72dde33. Configure here.

Copy link
Contributor

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

This PR improves context management by threading context.Context consistently through all function call chains in the Entire CLI, replacing approximately 25 instances where context.Background() or context.TODO() was used despite a caller-provided context being available. This ensures proper propagation of cancellation signals, deadlines, and context values from Cobra command handlers through the entire call stack.

Changes:

  • Added ctx context.Context as the first parameter to ~50 internal functions across cli, strategy, checkpoint, session, settings, paths, logging, and agent packages
  • Updated all function call sites to pass the caller's context instead of creating new background contexts
  • Preserved 2 justified exceptions with //nolint:contextcheck annotations (sync.Once initialization and callback without available context)

Reviewed changes

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

Show a summary per file
File Description
cmd/entire/cli/versioncheck/versioncheck.go Added ctx parameter to CheckAndNotify and fetchLatestVersion, threaded through HTTP requests
cmd/entire/cli/versioncheck/versioncheck_test.go Updated test calls to pass context.Background() to modified functions
cmd/entire/cli/transcript.go Added ctx parameter to resolveTranscriptPath
cmd/entire/cli/root.go Threaded cmd.Context() through LoadEntireSettings, GetAgentsWithHooksInstalled, and CheckAndNotify
cmd/entire/cli/status.go Added ctx parameter to runStatus and related functions, threaded through settings and session operations
cmd/entire/cli/resume.go, reset.go, doctor.go, clean.go Added ctx parameters and threaded through strategy operations
cmd/entire/cli/config.go Added ctx parameters to settings functions, preserved context.TODO() exception for callback
cmd/entire/cli/hooks_git_cmd.go, hook_registry.go Threaded cmd.Context() through hook handlers and lifecycle dispatchers
cmd/entire/cli/strategy/*.go Added ctx parameters across all strategy functions (session, state, reset, push, logs, cleanup, hooks)
cmd/entire/cli/settings/settings.go Added ctx parameters to Load, Save, and helper functions
cmd/entire/cli/session/*.go Added ctx parameters to StateStore, state loading, and phase transitions
cmd/entire/cli/paths/paths.go Added ctx parameter to WorktreeRoot and AbsPath
cmd/entire/cli/logging/logger.go Added ctx parameter to Init function
cmd/entire/cli/checkpoint/temporary.go Threaded ctx through tree building and metadata operations
cmd/entire/cli/agent/*.go Added ctx parameters to Agent interface methods (DetectPresence, ChunkTranscript, WriteSession, ParseHookEvent, InstallHooks, UninstallHooks, AreHooksInstalled, PrepareTranscript) and all implementations

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable autofix in the Cursor dashboard.

Comment @cursor review or bugbot run to trigger another review on this PR

toothbrush
toothbrush previously approved these changes Feb 26, 2026
Copy link
Contributor

@toothbrush toothbrush left a comment

Choose a reason for hiding this comment

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

This looks great and i think it's an excellent start. I'd be happy to merge as-is and improve from here, it doesn't have to be perfect on day 1.

I am very far from the details, but i noticed a few functions did not get context.Context:

  • ReassembleTranscript(chunks [][]byte) ([]byte, error)
  • GetSessionID(input *HookInput) string
  • GetSessionDir(repoPath string) (string, error)
  • ResolveSessionFile(sessionDir, agentSessionID string) string

Is there any chance these operations might be slow (like, >500ms)? If so i think it'd be good for them to be cancelable too. But that can be a follow-up PR IMHO.

Finally i hoped this would fix the thing where running entire explain on main with many checkpoints can't be killed with C-c, but it seems that's still broken. I'm investigating, but again, happy for that to be a followup.

Thanks!!

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Entire-Checkpoint: f7a9907f2f22
@gtrrz-victor gtrrz-victor merged commit 97fd6ab into main Feb 26, 2026
3 checks passed
@gtrrz-victor gtrrz-victor deleted the improve-context-management branch February 26, 2026 04:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants