Skip to content

[AUTOMATION] fix(clawpatch): address daily finding#273

Open
michiosw wants to merge 1 commit into
mainfrom
fix/clawpatch-daemon-cancel
Open

[AUTOMATION] fix(clawpatch): address daily finding#273
michiosw wants to merge 1 commit into
mainfrom
fix/clawpatch-daemon-cancel

Conversation

@michiosw

Copy link
Copy Markdown
Contributor

Where We Are

The local Guard daemon keeps running after the caller cancels context. runDaemon started the runtime service with context.Background() and the HTTP server had no shutdown path tied to the caller context, so tests or supervisors could hang until process exit.

Where We Want To Go

The daemon should stop when the caller context is canceled. kontext start should return cleanly after shutdown instead of leaving the socket and HTTP listener alive.

How do we get there

Thread the caller context into the local runtime service and the HTTP server, treat http.ErrServerClosed as a normal shutdown, and add a regression test that starts the daemon, waits for it to come up, cancels context, and asserts clean return. Verified with go test ./..., go vet ./..., npm exec --yes --package pnpm@10.0.0 -- pnpm install --frozen-lockfile, npm exec --yes --package pnpm@10.0.0 -- pnpm --dir web/guard-dashboard typecheck, and git diff --check.

Copy link
Copy Markdown
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@greptile-apps

greptile-apps Bot commented Jun 10, 2026

Copy link
Copy Markdown

Greptile Summary

This PR makes the local Guard daemon stop when its caller context is canceled. The main changes are:

  • Threads the daemon context into the local runtime service.
  • Adds context-aware shutdown to the Guard HTTP server.
  • Treats normal HTTP server shutdown as a clean return.
  • Adds a daemon cancellation regression test.

Confidence Score: 4/5

This is close, but I would harden the daemon context handling before merging.

  • The shutdown flow is generally wired through the changed server and daemon paths.

  • A nil context from an embedded caller can now panic in runtime service startup.

  • No security-specific issue was found in the changed code.

  • internal/guard/cli/cli.go should normalize the daemon context before forwarding it.

Important Files Changed

Filename Overview
internal/guard/app/server/server.go Adds context-driven HTTP server shutdown and normalizes http.ErrServerClosed.
internal/guard/cli/cli.go Passes the caller context through daemon startup and serving paths.
internal/guard/cli/cli_test.go Adds coverage that canceling the daemon context returns cleanly.

Comments Outside Diff (1)

  1. internal/guard/cli/cli.go, line 83-84 (link)

    P2 Normalize daemon context

    Run accepts a context.Context, and this path now forwards it directly into localruntime.Service.Start. That method calls context.WithCancel(ctx), which panics when ctx is nil. Before this change, runDaemon used context.Background() for the runtime service and the server path now has its own nil guard, so an embedded caller passing nil can crash before the daemon returns an error.

Reviews (1): Last reviewed commit: "fix(clawpatch): address daily finding" | Re-trigger Greptile

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.

1 participant