chore: shrink the fork's conflict surface in upstream-owned files - #90
Open
NicolasWalter wants to merge 1 commit into
Open
chore: shrink the fork's conflict surface in upstream-owned files#90NicolasWalter wants to merge 1 commit into
NicolasWalter wants to merge 1 commit into
Conversation
Two changes, both targeting files that conflicted or nearly conflicted during recent syncs. 1. Route every sandbox creation through create_primo_sandbox() manager.py is the fork's hottest patch target — 34 upstream commits in six months. Our patch sat in six places, two of them *inside* upstream's create_kwargs dict literal, which is exactly where upstream keeps adding fields. The 2026-07-31 sync conflicted on the import block for the same reason. A fork-owned create_primo_sandbox() wrapper takes over the argv and the per-repo VM-runtime kwargs, so each call site becomes one line and upstream's create_kwargs dict goes back to being untouched. The import drops from 5 lines to 1. Precedence is preserved: caller kwargs override the Core defaults, so explicit cpuCores/memoryMib from session settings still win. 2. Move fork tests out of upstream test files Test files churn hardest and merge worst, so they are the last place a fork wants a permanent edit — the lesson from #82, applied to the three upstream test files we were still editing. Their fork assertions move to new fork-owned files, which never conflict: - tests/test_build_sandbox.py, tests/test_sandbox_resources.py -> tests/test_primo_sandbox_creation.py - slack-bot classifier/index.test.ts -> classifier/index.primo.test.ts All three upstream test files are now byte-identical to upstream. No behavior change. Verified the new tests actually guard: bypassing the wrapper makes two of them fail. Verification: pytest 225 passed, ruff check/format clean, slack-bot 383 passed, npm run typecheck / lint / prettier clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Terraform Validation Results
Pushed by: @NicolasWalter, Action: |
Terraform Plan ResultsStatus: ✅ Success Show Planterraform_data.access_control_gate: Refreshing state... [id=841ab6bc-98a7-018c-1031-ebad4f8b62bc]
terraform_data.cloudflare_custom_domain_gate: Refreshing state... [id=fa456fac-6c14-16e4-a484-3338d1a3718d]
data.external.modal_source_hash[0]: Reading...
terraform_data.sign_in_provider_gate: Refreshing state... [id=7f4a67d1-6978-0b23-899d-c2a9004643bd]
local_file.web_app_wrangler_production[0]: Refreshing state... [id=4fd11b54fb693eced4839d030233ab1911f26ea1]
random_password.image_callback_token_pepper: Refreshing state... [id=none]
random_password.service_auth_secret_modal: Refreshing state... [id=none]
random_password.service_auth_secret_linear_bot: Refreshing state... [id=none]
random_password.service_auth_secret_slack_bot: Refreshing state... [id=none]
random_password.service_auth_secret_web: Refreshing state... [id=none]
random_password.service_auth_secret_github_bot: Refreshing state... [id=none]
null_resource.control_plane_build: Refreshing state... [id=704864366885380594]
cloudflare_d1_database.main: Refreshing state... [id=dba95b03-ace9-47a8-81e9-6e39d8d694c5]
module.slack_kv[0].cloudflare_workers_kv_namespace.this: Refreshing state... [id=729b357dbb5e4c9d99ec9212cc45766e]
module.session_index_kv.cloudflare_workers_kv_namespace.this: Refreshing state... [id=7f18644fbed34121bbe3a196f373ea93]
null_resource.slack_bot_build[0]: Refreshing state... [id=4260025910565342222]
module.github_kv[0].cloudflare_workers_kv_namespace.this: Refreshing state... [id=87dbfaa1d9ce4a37a42e04c57c434a72]
cloudflare_r2_bucket.media: Refreshing state... [id=open-inspect-media-primo]
cloudflare_queue.image_build_finalization: Refreshing state... [id=1ca823a150c54578a9ad1814325147a3]
null_resource.web_app_cloudflare_build[0]: Refreshing state... [id=8017869950953229760]
cloudflare_queue.image_build_finalization_dlq: Refreshing state... [id=cbbc2d8794c04396a550996e7f0cc129]
module.linear_kv[0].cloudflare_workers_kv_namespace.this: Refreshing state... [id=d003f1ad81384910a1f48a0a33f18c09]
cloudflare_queue.slack_completion_delivery[0]: Refreshing state... [id=56ef0f3e13bd46a3a39f30c79ec547fa]
data.external.modal_source_hash[0]: Read complete after 0s [id=-]
null_resource.linear_bot_build[0]: Refreshing state... [id=314863585638450347]
null_resource.github_bot_build[0]: Refreshing state... [id=4488115135763450793]
cloudflare_queue.slack_completion_delivery_dlq[0]: Refreshing state... [id=06ce03d2663f4aea937b0c0c1c379c17]
module.modal_app[0].null_resource.modal_secrets[0]: Refreshing state... [id=2786381799123975256]
module.linear_bot_worker[0].cloudflare_worker.this: Refreshing state... [id=33782d80e8ff4af9b30b92870084b674]
module.modal_app[0].null_resource.modal_deploy: Refreshing state... [id=397431337699639711]
module.slack_bot_worker[0].cloudflare_worker.this: Refreshing state... [id=5200e96d69804ea296e1f3a6b39e4243]
null_resource.d1_migrations: Refreshing state... [id=7999417390868468622]
module.linear_bot_worker[0].cloudflare_worker_version.this: Refreshing state... [id=1fd58cf1-169a-4fb2-85cb-f25f356b6238]
module.slack_bot_worker[0].cloudflare_worker_version.this: Refreshing state... [id=3d40339d-4a86-4e98-b81a-e0593b878fb8]
module.linear_bot_worker[0].cloudflare_workers_deployment.this: Refreshing state... [id=54fc0ddd-80d5-44c5-862c-70f0ff08a8c3]
module.slack_bot_worker[0].cloudflare_workers_deployment.this: Refreshing state... [id=ac636d3e-d748-4ef4-8a2c-57f479d26ac7]
module.control_plane_worker.cloudflare_worker.this: Refreshing state... [id=c208a60c393e45e38eb502346bb7ce1e]
cloudflare_queue_consumer.slack_completion_delivery[0]: Refreshing state...
module.control_plane_worker.cloudflare_workers_cron_trigger.this[0]: Refreshing state... [id=open-inspect-control-plane-primo]
null_resource.web_app_cloudflare_secrets[0]: Refreshing state... [id=8867783181576424643]
module.github_bot_worker[0].cloudflare_worker.this: Refreshing state... [id=4b5e2696491a41eaaa124f4e2a9855f2]
Terraform used the selected providers to generate the following execution
plan. Resource actions are indicated with the following symbols:
+ create
~ update in-place
-/+ destroy and then create replacement
Terraform will perform the following actions:
# cloudflare_queue_consumer.image_build_finalization will be created
+ resource "cloudflare_queue_consumer" "image_build_finalization" {
+ account_id = "bf66240843ed90d19b82e4b90916d29a"
+ consumer_id = (known after apply)
+ created_on = (known after apply)
+ dead_letter_queue = "open-inspect-image-build-finalization-dlq-primo"
+ queue_id = "1ca823a150c54578a9ad1814325147a3"
+ queue_name = (known after apply)
+ script_name = "open-inspect-control-plane-primo"
+ settings = {
+ batch_size = 1
+ max_concurrency = 5
+ max_retries = 12
+ max_wait_time_ms = 1000
+ retry_delay = 15
+ visibility_timeout_ms = (known after apply)
}
+ type = "worker"
}
# local_file.web_app_wrangler_production[0] will be created
+ resource "local_file" "web_app_wrangler_production" {
+ content = <<-EOT
name = "open-inspect-web-primo"
main = ".open-next/worker.js"
compatibility_date = "2025-08-15"
compatibility_flags = ["nodejs_compat", "global_fetch_strictly_public"]
# A custom-domain deployment has one canonical browser origin.
workers_dev = true
[vars]
CONTROL_PLANE_URL = "https://open-inspect-control-plane-primo.primo-bf6.workers.dev"
NEXT_PUBLIC_WS_URL = "wss://open-inspect-control-plane-primo.primo-bf6.workers.dev"
NEXT_PUBLIC_SANDBOX_PROVIDER = "modal"
NEXT_PUBLIC_APP_NAME = "Primo"
NEXT_PUBLIC_APP_SHORT_NAME = "Primo"
NEXT_PUBLIC_APP_ICON_URL = ""
[assets]
directory = ".open-next/assets"
binding = "ASSETS"
[[services]]
binding = "CONTROL_PLANE_WORKER"
service = "open-inspect-control-plane-primo"
EOT
+ content_base64sha256 = (known after apply)
+ content_base64sha512 = (known after apply)
+ content_md5 = (known after apply)
+ content_sha1 = (known after apply)
+ content_sha256 = (known after apply)
+ content_sha512 = (known after apply)
+ directory_permission = "0777"
+ file_permission = "0777"
+ filename = "../../..//packages/web/wrangler.production.toml"
+ id = (known after apply)
}
# null_resource.control_plane_build must be replaced
-/+ resource "null_resource" "control_plane_build" {
~ id = "704864366885380594" -> (known after apply)
~ triggers = { # forces replacement
~ "always_run" = "2026-07-31T12:45:57Z" -> (known after apply)
}
}
# null_resource.github_bot_build[0] must be replaced
-/+ resource "null_resource" "github_bot_build" {
~ id = "4488115135763450793" -> (known after apply)
~ triggers = { # forces replacement
~ "always_run" = "2026-07-31T12:45:57Z" -> (known after apply)
}
}
# null_resource.linear_bot_build[0] must be replaced
-/+ resource "null_resource" "linear_bot_build" {
~ id = "314863585638450347" -> (known after apply)
~ triggers = { # forces replacement
~ "always_run" = "2026-07-31T12:45:57Z" -> (known after apply)
}
}
# null_resource.slack_bot_build[0] must be replaced
-/+ resource "null_resource" "slack_bot_build" {
~ id = "4260025910565342222" -> (known after apply)
~ triggers = { # forces replacement
~ "always_run" = "2026-07-31T12:45:57Z" -> (known after apply)
}
}
# null_resource.web_app_cloudflare_build[0] must be replaced
-/+ resource "null_resource" "web_app_cloudflare_build" {
~ id = "8017869950953229760" -> (known after apply)
~ triggers = { # forces replacement
~ "always_run" = "2026-07-31T12:45:57Z" -> (known after apply)
}
}
# null_resource.web_app_cloudflare_deploy[0] will be created
+ resource "null_resource" "web_app_cloudflare_deploy" {
+ id = (known after apply)
+ triggers = {
+ "always_run" = (known after apply)
}
}
# module.control_plane_worker.cloudflare_worker.this will be updated in-place
~ resource "cloudflare_worker" "this" {
id = "c208a60c393e45e38eb502346bb7ce1e"
name = "open-inspect-control-plane-primo"
~ observability = {
~ logs = {
+ destinations = (known after apply)
# (4 unchanged attributes hidden)
}
~ traces = {
+ destinations = (known after apply)
# (3 unchanged attributes hidden)
}
# (2 unchanged attributes hidden)
}
~ references = {
~ dispatch_namespace_outbounds = [] -> (known after apply)
~ domains = [] -> (known after apply)
~ durable_objects = [
- {
- namespace_id = "4c77239db3614a6aac69a90e1fbd8955" -> null
- namespace_name = "open-inspect-control-plane-primo_SessionDO" -> null
- worker_id = "c208a60c393e45e38eb502346bb7ce1e" -> null
- worker_name = "open-inspect-control-plane-primo" -> null
},
- {
- namespace_id = "bf3328c8ebcb4039855ed7fcca6eb7e9" -> null
- namespace_name = "open-inspect-control-plane-primo_SchedulerDO" -> null
- worker_id = "c208a60c393e45e38eb502346bb7ce1e" -> null
- worker_name = "open-inspect-control-plane-primo" -> null
},
] -> (known after apply)
~ queues = [] -> (known after apply)
~ workers = [
- {
- id = "33782d80e8ff4af9b30b92870084b674" -> null
- name = "open-inspect-linear-bot-primo" -> null
},
- {
- id = "5200e96d69804ea296e1f3a6b39e4243" -> null
- name = "open-inspect-slack-bot-primo" -> null
},
- {
- id = "ac07332f8b0f4cdfa4ca04f966a9fa61" -> null
- name = "open-inspect-web-primo" -> null
},
- {
- id = "4b5e2696491a41eaaa124f4e2a9855f2" -> null
- name = "open-inspect-github-bot-primo" -> null
},
] -> (known after apply)
} -> (known after apply)
tags = []
~ updated_on = "2026-07-30T19:37:01Z" -> (known after apply)
# (6 unchanged attributes hidden)
}
# module.control_plane_worker.cloudflare_worker_version.this will be created
+ resource "cloudflare_worker_version" "this" {
+ account_id = "bf66240843ed90d19b82e4b90916d29a"
+ annotations = (known after apply)
+ bindings = (sensitive value)
+ compatibility_date = "2024-09-23"
+ compatibility_flags = [
+ "nodejs_compat",
]
+ created_on = (known after apply)
+ id = (known after apply)
+ limits = (known after apply)
+ main_module = "index.js"
+ main_script_base64 = (known after apply)
+ migration_tag = (known after apply)
+ modules = [
+ {
+ content_file = "../../..//packages/control-plane/dist/index.js"
+ content_sha256 = "a53f4bb4c0c7ef23b75abf36035650563bb6234748fe70488cdc6aba05f0720b"
+ content_type = "application/javascript+module"
+ name = "index.js"
},
]
+ number = (known after apply)
+ source = (known after apply)
+ startup_time_ms = (known after apply)
+ urls = (known after apply)
+ usage_model = "standard"
+ worker_id = "c208a60c393e45e38eb502346bb7ce1e"
}
# module.control_plane_worker.cloudflare_workers_deployment.this will be created
+ resource "cloudflare_workers_deployment" "this" {
+ account_id = "bf66240843ed90d19b82e4b90916d29a"
+ annotations = (known after apply)
+ author_email = (known after apply)
+ created_on = (known after apply)
+ id = (known after apply)
+ script_name = "open-inspect-control-plane-primo"
+ source = (known after apply)
+ strategy = "percentage"
+ versions = [
+ {
+ percentage = 100
+ version_id = (known after apply)
},
]
}
# module.github_bot_worker[0].cloudflare_worker.this will be updated in-place
~ resource "cloudflare_worker" "this" {
id = "4b5e2696491a41eaaa124f4e2a9855f2"
name = "open-inspect-github-bot-primo"
~ observability = {
~ logs = {
+ destinations = (known after apply)
# (4 unchanged attributes hidden)
}
~ traces = {
+ destinations = (known after apply)
# (3 unchanged attributes hidden)
}
# (2 unchanged attributes hidden)
}
~ references = {
~ dispatch_namespace_outbounds = [] -> (known after apply)
~ domains = [] -> (known after apply)
~ durable_objects = [] -> (known after apply)
~ queues = [] -> (known after apply)
~ workers = [] -> (known after apply)
} -> (known after apply)
tags = []
~ updated_on = "2026-07-30T19:37:10Z" -> (known after apply)
# (6 unchanged attributes hidden)
}
# module.github_bot_worker[0].cloudflare_worker_version.this will be created
+ resource "cloudflare_worker_version" "this" {
+ account_id = "bf66240843ed90d19b82e4b90916d29a"
+ annotations = (known after apply)
+ bindings = (sensitive value)
+ compatibility_date = "2024-09-23"
+ compatibility_flags = [
+ "nodejs_compat",
]
+ created_on = (known after apply)
+ id = (known after apply)
+ limits = (known after apply)
+ main_module = "index.js"
+ main_script_base64 = (known after apply)
+ migration_tag = (known after apply)
+ modules = [
+ {
+ content_file = "../../..//packages/github-bot/dist/index.js"
+ content_sha256 = "d5f1aeb4820ce5ccd8eb0802ee3cc63654ad5b7c919711ae450c7abd776d9cde"
+ content_type = "application/javascript+module"
+ name = "index.js"
},
]
+ number = (known after apply)
+ source = (known after apply)
+ startup_time_ms = (known after apply)
+ urls = (known after apply)
+ usage_model = "standard"
+ worker_id = "4b5e2696491a41eaaa124f4e2a9855f2"
}
# module.github_bot_worker[0].cloudflare_workers_deployment.this will be created
+ resource "cloudflare_workers_deployment" "this" {
+ account_id = "bf66240843ed90d19b82e4b90916d29a"
+ annotations = (known after apply)
+ author_email = (known after apply)
+ created_on = (known after apply)
+ id = (known after apply)
+ script_name = "open-inspect-github-bot-primo"
+ source = (known after apply)
+ strategy = "percentage"
+ versions = [
+ {
+ percentage = 100
+ version_id = (known after apply)
},
]
}
# module.linear_bot_worker[0].cloudflare_worker.this will be updated in-place
~ resource "cloudflare_worker" "this" {
id = "33782d80e8ff4af9b30b92870084b674"
name = "open-inspect-linear-bot-primo"
~ observability = {
~ logs = {
+ destinations = (known after apply)
# (4 unchanged attributes hidden)
}
~ traces = {
+ destinations = (known after apply)
# (3 unchanged attributes hidden)
}
# (2 unchanged attributes hidden)
}
~ references = {
~ dispatch_namespace_outbounds = [] -> (known after apply)
~ domains = [] -> (known after apply)
~ durable_objects = [] -> (known after apply)
~ queues = [] -> (known after apply)
~ workers = [
- {
- id = "c208a60c393e45e38eb502346bb7ce1e" -> null
- name = "open-inspect-control-plane-primo" -> null
},
] -> (known after apply)
} -> (known after apply)
tags = []
~ updated_on = "2026-07-31T12:45:58Z" -> (known after apply)
# (6 unchanged attributes hidden)
}
# module.linear_bot_worker[0].cloudflare_worker_version.this must be replaced
-/+ resource "cloudflare_worker_version" "this" {
~ annotations = {
+ workers_message = (known after apply)
+ workers_tag = (known after apply)
~ workers_triggered_by = "create_version_api" -> (known after apply)
} -> (known after apply)
~ bindings = (sensitive value) # forces replacement
~ created_on = "2026-07-31T12:45:59Z" -> (known after apply)
~ id = "1fd58cf1-169a-4fb2-85cb-f25f356b6238" -> (known after apply)
+ limits = (known after apply)
+ main_script_base64 = (known after apply)
+ migration_tag = (known after apply)
~ number = 58 -> (known after apply)
~ source = "terraform" -> (known after apply)
~ startup_time_ms = 36 -> (known after apply)
~ urls = [
- "https://1fd58cf1-open-inspect-linear-bot-primo.primo-bf6.workers.dev",
] -> (known after apply)
# (7 unchanged attributes hidden)
}
# module.linear_bot_worker[0].cloudflare_workers_deployment.this must be replaced
-/+ resource "cloudflare_workers_deployment" "this" {
~ annotations = {
+ workers_message = (known after apply)
~ workers_triggered_by = "deployment" -> (known after apply)
} -> (known after apply)
~ author_email = "nicolas@primo.la" -> (known after apply)
~ created_on = "2026-07-31T12:46:01Z" -> (known after apply)
~ id = "54fc0ddd-80d5-44c5-862c-70f0ff08a8c3" -> (known after apply)
~ source = "terraform" -> (known after apply)
~ versions = [ # forces replacement
~ {
~ version_id = "1fd58cf1-169a-4fb2-85cb-f25f356b6238" -> (known after apply)
# (1 unchanged attribute hidden)
},
]
# (3 unchanged attributes hidden)
}
# module.modal_app[0].null_resource.modal_deploy is tainted, so must be replaced
-/+ resource "null_resource" "modal_deploy" {
~ id = "397431337699639711" -> (known after apply)
~ triggers = {
~ "source_hash" = "8e036b68e92a17fb5f7cb78b68d814e9c1d4e4630a7fe36d5446af83073be284" -> "5dc083ec793cc4616c3aadfe853e1cf083f43613117f957b50370a1c2c6ce66c"
# (3 unchanged elements hidden)
}
}
# module.slack_bot_worker[0].cloudflare_worker.this will be updated in-place
~ resource "cloudflare_worker" "this" {
id = "5200e96d69804ea296e1f3a6b39e4243"
name = "open-inspect-slack-bot-primo"
~ observability = {
~ logs = {
+ destinations = (known after apply)
# (4 unchanged attributes hidden)
}
~ traces = {
+ destinations = (known after apply)
# (3 unchanged attributes hidden)
}
# (2 unchanged attributes hidden)
}
~ references = {
~ dispatch_namespace_outbounds = [] -> (known after apply)
~ domains = [] -> (known after apply)
~ durable_objects = [] -> (known after apply)
~ queues = [
- {
- queue_consumer_id = "a755a290fd92417fb11c298f9c1d1f40" -> null
- queue_id = "56ef0f3e13bd46a3a39f30c79ec547fa" -> null
- queue_name = "open-inspect-slack-completion-primo" -> null
},
] -> (known after apply)
~ workers = [
- {
- id = "c208a60c393e45e38eb502346bb7ce1e" -> null
- name = "open-inspect-control-plane-primo" -> null
},
] -> (known after apply)
} -> (known after apply)
tags = []
~ updated_on = "2026-07-31T12:45:57Z" -> (known after apply)
# (6 unchanged attributes hidden)
}
# module.slack_bot_worker[0].cloudflare_worker_version.this must be replaced
-/+ resource "cloudflare_worker_version" "this" {
~ annotations = {
+ workers_message = (known after apply)
+ workers_tag = (known after apply)
~ workers_triggered_by = "create_version_api" -> (known after apply)
} -> (known after apply)
~ bindings = (sensitive value) # forces replacement
~ created_on = "2026-07-31T12:45:59Z" -> (known after apply)
~ id = "3d40339d-4a86-4e98-b81a-e0593b878fb8" -> (known after apply)
+ limits = (known after apply)
+ main_script_base64 = (known after apply)
+ migration_tag = (known after apply)
~ number = 61 -> (known after apply)
~ source = "terraform" -> (known after apply)
~ startup_time_ms = 41 -> (known after apply)
~ urls = [
- "https://3d40339d-open-inspect-slack-bot-primo.primo-bf6.workers.dev",
] -> (known after apply)
# (7 unchanged attributes hidden)
}
# module.slack_bot_worker[0].cloudflare_workers_deployment.this must be replaced
-/+ resource "cloudflare_workers_deployment" "this" {
~ annotations = {
+ workers_message = (known after apply)
~ workers_triggered_by = "deployment" -> (known after apply)
} -> (known after apply)
~ author_email = "nicolas@primo.la" -> (known after apply)
~ created_on = "2026-07-31T12:46:00Z" -> (known after apply)
~ id = "ac636d3e-d748-4ef4-8a2c-57f479d26ac7" -> (known after apply)
~ source = "terraform" -> (known after apply)
~ versions = [ # forces replacement
~ {
~ version_id = "3d40339d-4a86-4e98-b81a-e0593b878fb8" -> (known after apply)
# (1 unchanged attribute hidden)
},
]
# (3 unchanged attributes hidden)
}
Plan: 17 to add, 4 to change, 10 to destroy.
─────────────────────────────────────────────────────────────────────────────
Saved the plan to: tfplan
To perform exactly these actions, run the following command to apply:
terraform apply "tfplan"Pushed by: @NicolasWalter |
📝 WalkthroughWalkthroughThe PR centralizes Primo sandbox creation, applies repository runtime defaults, and preserves caller overrides across sandbox flows. It adds isolated sandbox tests and moves Primo classifier prompt coverage into a dedicated test suite. ChangesPrimo sandbox creation
Primo classifier prompt tests
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant SandboxManager
participant create_primo_sandbox
participant ModalSandbox
SandboxManager->>create_primo_sandbox: pass repository identity and creation options
create_primo_sandbox->>create_primo_sandbox: merge Primo runtime defaults with caller options
create_primo_sandbox->>ModalSandbox: call Sandbox.create.aio with Primo command
ModalSandbox-->>SandboxManager: return created sandbox
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed due to a network error. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/modal-infra/tests/test_primo_sandbox_creation.py`:
- Around line 50-52: In the test_primo_sandbox_creation.py file, replace the
hardcoded values in the assertions at lines 50-52 and 88-90 with imported
constants from src.images.primo_overlay. First, define a named constant for the
VM runtime options default (the dict with "vm_runtime": True) in the
primo_overlay module, then import that constant along with the existing Core
resource constants (cpu and memory defaults) into the test file. Replace the
assertions checking captured["kwargs"]["cpu"] == 2.0,
captured["kwargs"]["memory"] == 8192, and
captured["kwargs"]["experimental_options"] == {"vm_runtime": True} to use these
imported constants instead of hardcoded values, ensuring the test assertions
stay synchronized with the factory configuration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0a4666c8-9037-4209-8e10-d939c9c89c69
📒 Files selected for processing (7)
packages/modal-infra/src/images/primo_overlay.pypackages/modal-infra/src/sandbox/manager.pypackages/modal-infra/tests/test_build_sandbox.pypackages/modal-infra/tests/test_primo_sandbox_creation.pypackages/modal-infra/tests/test_sandbox_resources.pypackages/slack-bot/src/classifier/index.primo.test.tspackages/slack-bot/src/classifier/index.test.ts
💤 Files with no reviewable changes (3)
- packages/slack-bot/src/classifier/index.test.ts
- packages/modal-infra/tests/test_build_sandbox.py
- packages/modal-infra/tests/test_sandbox_resources.py
Comment on lines
+50
to
+52
| assert captured["kwargs"]["cpu"] == 2.0 | ||
| assert captured["kwargs"]["memory"] == 8192 | ||
| assert captured["kwargs"]["experimental_options"] == {"vm_runtime": True} |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Reuse the named Core runtime defaults in these assertions.
Lines 50-52 duplicate the factory resource defaults. Line 90 duplicates the VM runtime options default. Define a named Core VM-options constant in src.images.primo_overlay, then import that constant and the existing resource constants here. This keeps the tests synchronized with the factory configuration.
As per coding guidelines, “Define each default value exactly once in a named constant and import or reuse that constant everywhere.”
Also applies to: 88-90
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/modal-infra/tests/test_primo_sandbox_creation.py` around lines 50 -
52, In the test_primo_sandbox_creation.py file, replace the hardcoded values in
the assertions at lines 50-52 and 88-90 with imported constants from
src.images.primo_overlay. First, define a named constant for the VM runtime
options default (the dict with "vm_runtime": True) in the primo_overlay module,
then import that constant along with the existing Core resource constants (cpu
and memory defaults) into the test file. Replace the assertions checking
captured["kwargs"]["cpu"] == 2.0, captured["kwargs"]["memory"] == 8192, and
captured["kwargs"]["experimental_options"] == {"vm_runtime": True} to use these
imported constants instead of hardcoded values, ensuring the test assertions
stay synchronized with the factory configuration.
Source: Coding guidelines
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #82, aimed at the files that actually conflicted (or nearly did) in the last two syncs — including #89 an hour ago.
Fork lines living inside upstream-owned files: 95 across 4 files → 11 in 1 file.
1. Route every sandbox creation through
create_primo_sandbox()manager.pyis the fork's hottest patch target — 34 upstream commits in six months. Our patch sat in six places, two of them inside upstream'screate_kwargsdict literal, which is exactly where upstream keeps adding fields. #89 conflicted on the import block for the same reason.A fork-owned
create_primo_sandbox()wrapper now owns the argv and the per-repo VM-runtime kwargs, so each call site is a single line and upstream'screate_kwargsdict goes back to untouched:create_kwargsdictsPrecedence is preserved: caller kwargs override the Core defaults, so explicit
cpuCores/memoryMibfrom session settings still win — covered by a test.2. Move fork tests out of upstream test files
Test files churn hardest and merge worst, so they're the last place a fork wants a permanent edit. That was #82's lesson for
session-launcher.test.ts; this applies it to the three upstream test files we were still editing.modal-infra/tests/test_build_sandbox.pymodal-infra/tests/test_sandbox_resources.pyslack-bot/src/classifier/index.test.tsCoverage moves to
tests/test_primo_sandbox_creation.pyandclassifier/index.primo.test.ts— new files never conflict. The Python file gains two cases the old placement didn't have (session-path entrypoint, alongside the build path).No behavior change
Same argv, same kwargs, same precedence. Verified the new tests actually guard rather than just pass: bypassing the wrapper's kwargs merge makes two of them fail.
Verification
pytest tests/— 255 passed;ruff check+ruff formatcleannpm test -w @open-inspect/slack-bot— 392 passed across 33 files, including upstream's unmodifiedindex.test.tsnpm run typecheck,npm run lint,prettier --check .— cleanWhat I deliberately left alone
images/base.pysplits upstream'sbase_imagechain in two. Moving the overlay to the end of the file would shrink the diff but put the expensive Go/AWS-CLI layers afteradd_local_dir, so everysandbox_runtimesource change would rebuild them. Not worth it.README.md(+5 at line 1) and.gitignore(+3): tiny inserts far from upstream's edit sites.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests