Skip to content

fix(proxy): preserve preview activity updates#983

Open
G4614 wants to merge 1 commit into
boxlite-ai:mainfrom
G4614:agent/fix-activity-keepalive
Open

fix(proxy): preserve preview activity updates#983
G4614 wants to merge 1 commit into
boxlite-ai:mainfrom
G4614:agent/fix-activity-keepalive

Conversation

@G4614

@G4614 G4614 commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Preserve preview activity updates after request completion while bounding each detached update attempt to 10 seconds.

Test plan:

  • cd apps && go test ./proxy/pkg/proxy
  • cd apps && go test -race ./proxy/pkg/proxy
  • make lint:fix

Summary by CodeRabbit

  • Bug Fixes
    • Last-activity updates now continue when a proxy request is canceled.
    • Updates are bounded by a timeout to prevent them from running indefinitely.
  • Tests
    • Added coverage confirming activity updates are sent after request cancellation.

@G4614
G4614 force-pushed the agent/fix-activity-keepalive branch from 11e3d68 to fd6b1ff Compare July 13, 2026 12:41
@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 7b044807-25c9-405c-8640-d23927aff0a4

📥 Commits

Reviewing files that changed from the base of the PR and between d029764 and 6000dbf.

📒 Files selected for processing (2)
  • apps/proxy/pkg/proxy/get_box_target.go
  • apps/proxy/pkg/proxy/get_box_target_test.go

📝 Walkthrough

Walkthrough

The proxy's last-activity update logic now uses a bounded, cancellation-ignoring context created via context.WithTimeout with a new activityUpdateTimeout constant, applied to cache checks, the BoxAPI update call, and cache set. A new test verifies the update completes despite request cancellation.

Changes

Activity update timeout handling

Layer / File(s) Summary
Timeout constant and cancellation-safe context wiring
apps/proxy/pkg/proxy/get_box_target.go
Adds activityUpdateTimeout constant, spawns updateLastActivity with context.WithoutCancel, and creates a timed updateCtx used for the cache check, BoxAPI.UpdateLastActivity call, and cache set.
Test coverage for cancellation resilience
apps/proxy/pkg/proxy/get_box_target_test.go
Adds blockingActivityCache test double and TestGetProxyTargetUpdatesActivityAfterRequestCancellation, which cancels the request mid-update and asserts the activity endpoint is still hit within the timeout.

Estimated code review effort: 2 (Simple) | ~12 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant GetProxyTarget
  participant updateLastActivity
  participant Cache
  participant BoxAPI

  Client->>GetProxyTarget: request (context ctx)
  GetProxyTarget->>updateLastActivity: spawn goroutine with WithoutCancel(ctx)
  Client--)GetProxyTarget: cancel request context
  updateLastActivity->>updateLastActivity: create updateCtx = WithTimeout(activityUpdateTimeout)
  updateLastActivity->>Cache: Has(updateCtx)
  updateLastActivity->>BoxAPI: UpdateLastActivity(updateCtx)
  updateLastActivity->>Cache: Set(updateCtx)
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description skips the required Summary/Changes/How to verify structure and only provides a brief note plus test plan. Add the template headings with a short Summary, a Changes list, and a How to verify section; include Risks / rollout if applicable.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly names the main behavior change: preserving preview activity updates.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@G4614 G4614 changed the title fix(proxy): keep active connections alive fix(proxy): preserve preview activity updates Jul 13, 2026
@G4614
G4614 force-pushed the agent/fix-activity-keepalive branch from fd6b1ff to 6000dbf Compare July 13, 2026 12:53
@G4614
G4614 marked this pull request as ready for review July 13, 2026 12:56
@G4614
G4614 requested a review from a team as a code owner July 13, 2026 12:56
@boxlite-agent

boxlite-agent Bot commented Jul 13, 2026

Copy link
Copy Markdown

📦 BoxLite review — looks good · 6000dbf

Review evidence

  • git diff --numstat origin/main...HEAD && git diff origin/main...HEAD — 2 files, +145/-4, proxy activity-update fix
  • go build ./... (apps workspace) — builds clean, go1.25.4 toolchain
  • go vet ./... — no issues
  • go test ./pkg/proxy/... -v (incl. new test) — TestGetProxyTargetUpdatesActivityAfterRequestCancellation passes
  • go test -race — CGO_ENABLED=0 in sandbox, race detector unavailable

Risk notes

  • concurrency/lifecycle — context.WithoutCancel decouples activity update from request ctx cancellation; new context.WithTimeout(10s) bounds it so goroutine can't leak indefinitely; verified via repro test that update still fires after request cancel
  • API contract — updateLastActivity signature/behavior unchanged externally; recursive poll call at line 338 already used WithoutCancel pre-PR, now double-wrapped but idempotent, no behavior change
  • test coverage — new test directly exercises the regression scenario (cancel request mid-flight, assert activity POST still reaches backend) with a custom blocking cache to control timing
apps/proxy/pkg/proxy/get_box_target.go
  GetProxyTarget  +3/-1  detach activity ctx from request
  updateLastActivity  +3/-3  add 10s timeout wrapper
apps/proxy/pkg/proxy/get_box_target_test.go
  TestGetProxyTargetUpdatesActivityAfterRequestCancellation  +135/-0  new regression test, blocking fake cache

reviewed 6000dbf in a BoxLite microVM · @boxlite-agent review to re-run · powered by BoxLite

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