Skip to content

Add browser pool profile refresh policy - #111

Merged
IlyaasK merged 7 commits into
hypeship/land-pr115-on-mainfrom
hypeship/browser-pool-refresh-on-profile-update-resource
Aug 5, 2026
Merged

Add browser pool profile refresh policy#111
IlyaasK merged 7 commits into
hypeship/land-pr115-on-mainfrom
hypeship/browser-pool-refresh-on-profile-update-resource

Conversation

@IlyaasK

@IlyaasK IlyaasK commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • expose the durable refresh_on_profile_update browser-pool policy as an optional/computed Terraform attribute
  • map explicit values through Kernel Go SDK v0.76 create and update parameters
  • preserve the prior API value during unrelated updates without overriding Kernel's default when a profile is attached, changed, or removed
  • flatten the API value into state and validate that true requires profile_id
  • retain Tighten browser pool update boundaries #115's browser-pool update-boundary behavior while removing the dropped browser-pool data-source ancestry
  • preserve the SDK production default in acceptance tests when the optional KERNEL_BASE_URL secret is unset

Why

Kernel's API and Go SDK already support this durable browser-pool setting. Terraform should manage the same desired state as the CLI without introducing browser-pool runtime operations.

The resource keeps an explicitly configured false value when a profile changes. When the attribute is omitted, profile attachment, replacement, or removal leaves the value to Kernel's profile-dependent default. Unrelated updates preserve the value already read from the API and avoid a noisy unknown plan.

Intentionally unsupported

This does not add a browser-pool data source, browser acquire/release/flush operations, leased-browser state, force deletion, profile mutation, or any other runtime/session behavior.

Verification

  • gofmt -l cmd internal
  • go test -short -timeout=2m ./...
  • go vet ./...
  • terraform fmt -check -recursive examples
  • bash scripts/check-docs.sh
  • bash scripts/check-markdown-links.sh
  • structured Codex autoreview completed clean after fixes for profile removal, attachment, and replacement planning
  • focused tests cover explicit false on create, enable/disable patches, unchanged omission, profile-dependent defaults, profile removal, validation, response flattening, malformed responses, and acceptance state assertions

Live acceptance was not rerun for this history-only restack. The browser-pool acceptance workflow remains the final live gate after the resource stack is assembled. The current live test asserts persisted state; a future live test should create SDK profile fixtures and exercise omitted-value defaults across profile attach, change, and clear transitions.


Note

Medium Risk
Touches browser-pool create/update API payloads and Terraform planning semantics for profile changes; risk is moderated by broad unit tests but live acceptance does not yet exercise omitted-default profile transitions.

Overview
Adds refresh_on_profile_update to kernel_browser_pool so Terraform can manage Kernel’s durable policy for refreshing idle browsers when a pool’s profile content changes—distinct from rebuild_idle_browsers_on_update, which still only covers launch-configuration changes through this resource.

Create/update mapping sends explicit booleans to the SDK and omits the field when the plan value is unknown (e.g. after profile attach/change/remove) so the API can apply its profile-dependent default; unrelated updates preserve the value already in state via a plan modifier. Read path flattens the API field into state, docs/architecture are updated, and validation rejects true without profile_id. Acceptance workflow now unsets an empty KERNEL_BASE_URL before tests; acceptance docs note live coverage gaps for omitted-default profile transitions.

Reviewed by Cursor Bugbot for commit 769dad1. Bugbot is set up for automated code reviews on this repo. Configure here.

@IlyaasK
IlyaasK force-pushed the hypeship/browser-pool-refresh-on-profile-update-resource branch from a342a28 to 20143f3 Compare August 3, 2026 14:31
@IlyaasK
IlyaasK force-pushed the hypeship/browser-pool-refresh-on-profile-update-resource branch 2 times, most recently from 09b0ad5 to f4021b9 Compare August 3, 2026 18:25
@IlyaasK
IlyaasK changed the base branch from hypeship/browser-pool-data-source-acceptance to fix/browser-pool-qa-boundaries August 3, 2026 18:25
@IlyaasK
IlyaasK force-pushed the hypeship/browser-pool-refresh-on-profile-update-resource branch from f4021b9 to d5f59b3 Compare August 5, 2026 13:35
@IlyaasK
IlyaasK changed the base branch from fix/browser-pool-qa-boundaries to hypeship/land-pr115-on-main August 5, 2026 13:35
@IlyaasK
IlyaasK force-pushed the hypeship/browser-pool-refresh-on-profile-update-resource branch from d5f59b3 to 25689a9 Compare August 5, 2026 13:37
@IlyaasK
IlyaasK requested a review from Sayan- August 5, 2026 15:34

@Sayan- Sayan- left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The plan modifier is the right shape for this attribute. Because Kernel's default depends on profile_id rather than being fixed, a plain UseStateForUnknown would plan the prior value and then hit "Provider produced inconsistent result after apply" every time the server re-defaults on a profile change. Gating the state reuse on profile_id being unchanged, and leaving the value unknown on attach/change/remove, avoids that while still keeping unrelated updates quiet.

Checked the behavior against packages/api:

  • The field is always present in responses (RefreshOnProfileUpdate: &pool.ProfileSync, backed by a non-nullable column), so flattenBool has no absent-field case to worry about even for pools with no profile.
  • The server rules line up with what the resource does: omitted plus a changed profile ref defaults to true, clearing the profile disables it, and true without a profile is a 400 that the validator now catches at plan time.

I also checked the interaction with the replacement modifiers this is stacked on. When a name or viewport clear forces a replace and profile_id is unchanged, the modifier plans the state value and expandCreateParams sends it explicitly, so the recreated pool matches the plan. When the profile changes too, the value stays unknown and Kernel decides. Both converge.

Two minor things:

  1. The resource now has two flush mechanisms with easily conflated names: refresh_on_profile_update (Kernel flushes idle browsers when the profile is later updated out of band) and rebuild_idle_browsers_on_update (the provider discards idle browsers when this config changes). A sentence in each description pointing at the other would save users from assuming one implies the other.

  2. Acceptance coverage is state assertions only. The conditional-default transitions are the risky part here and they're exercised against the fake, never the real API. That's a reasonable tradeoff while profiles aren't a managed resource, but it's worth listing the attach/change/clear paths explicitly as something the live gate should cover once a profile is available to test against.

IlyaasK added 5 commits August 5, 2026 11:58
Expose Kernel's durable refresh_on_profile_update setting through the browser pool resource. Preserve API defaults when omitted, retain explicit false values across profile changes, validate the profile dependency, and cover create/update/read behavior.
Keep the architecture field inventory and generated resource reference aligned with the optional/computed profile refresh behavior before the first release.
@IlyaasK
IlyaasK force-pushed the hypeship/browser-pool-refresh-on-profile-update-resource branch from 205b575 to 0d4d4fa Compare August 5, 2026 15:59
@IlyaasK
IlyaasK requested a review from Sayan- August 5, 2026 16:17

@Sayan- Sayan- left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Both notes addressed. The cross-references between refresh_on_profile_update and rebuild_idle_browsers_on_update should stop people conflating them, and writing the acceptance gap into docs/acceptance.md with the specific transitions a live test needs to cover is better than leaving it in a review thread.

The KERNEL_BASE_URL unset is unrelated to this PR but correct: env with a missing secret yields an empty string rather than an absent variable, which the SDK would take as a literal base URL instead of falling back to its default. unset in the same shell step applies to the go test that follows it.

@IlyaasK
IlyaasK merged commit 1b19580 into main Aug 5, 2026
8 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