Skip to content

fix(agent): Make Ctrl+C actually work: Do not use context.TODO - #225

Merged
indradhanush merged 4 commits into
mainfrom
dg/context-todo-agent
Aug 21, 2026
Merged

fix(agent): Make Ctrl+C actually work: Do not use context.TODO#225
indradhanush merged 4 commits into
mainfrom
dg/context-todo-agent

Conversation

@indradhanush

@indradhanush indradhanush commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Same motivation as #222. But targeted for the agent. The agent now propagates a ctx down into the controllers and everything else. So a clean exit is to be expected.

Also similar to #222, refactor main so that we can unit test it.

Note to reviewers

Reviewing one commit at a time will make it easier to absorb the changes.

@codecov

codecov Bot commented Aug 20, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 33.33333% with 76 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
agent/main.go 30.55% 75 Missing ⚠️
agent/registration/host_registrar.go 0.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@indradhanush
indradhanush force-pushed the dg/context-todo-agent branch 4 times, most recently from 4ba2185 to 5bf5ac2 Compare August 20, 2026 21:07
@sebastian-pf9 sebastian-pf9 changed the title fix(agent): Do not use context.TODO fix(agent): Make Ctrl+C actually work: Do not use context.TODO Aug 21, 2026
@indradhanush
indradhanush force-pushed the dg/context-todo-agent branch from 5bf5ac2 to cbe7e1c Compare August 21, 2026 10:36
Base automatically changed from dg/context-todo-main to main August 21, 2026 11:32
indradhanush and others added 2 commits August 21, 2026 07:33
The host reconciler was registered with context.TODO() while the manager ran
on the signal context. Its event filter therefore held a context that no
signal could cancel.

Build the signal context once and use it for both. Manager construction and
reconciler registration move out of main() so a test can reach them, which
nothing could before.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every command the executor ran got a fresh context.TODO, so cancelling a
reconcile left those commands running with nothing able to stop them.

The reconciler's call site moves in this commit rather than its own. It is the
only caller, and the package would not build split across two.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@indradhanush
indradhanush force-pushed the dg/context-todo-agent branch from cbe7e1c to 63a6ad5 Compare August 21, 2026 11:36
indradhanush and others added 2 commits August 21, 2026 08:12
One package-level context.TODO was shared by every spec, so the API calls a
spec made outlived it. A spec left behind referred to a ctx that no longer
existed, which broke the build for the whole package.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Registration made its own context.TODO. The CSR wait rooted its approval
timeout on a second one. Neither could be cancelled when the agent shut down.

The signal context now reaches both, threaded through the bootstrap and
rotation helpers in main().

The agent suite moves onto Ginkgo's SpecContext here rather than separately,
because its specs call the helpers whose signatures change. Two constraints
shaped that. Docker calls made through the host runner span a spec and its
AfterEach, so they take a suite-scoped context that Ginkgo will not cancel
when a node body returns. Gomega drops its default Eventually timeout as soon
as an assertion carries a context, so that default is pinned back on.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@indradhanush
indradhanush force-pushed the dg/context-todo-agent branch from 63a6ad5 to 72bd890 Compare August 21, 2026 12:12
@indradhanush

Copy link
Copy Markdown
Collaborator Author

Changelog: Fix bad merge conflict. No code changes.

@indradhanush
indradhanush merged commit bbaa640 into main Aug 21, 2026
17 of 18 checks passed
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