Ben Carr

Scope soft-error policy to each command Summary: Parse `BUCK2_HARD_ERROR` and `BUCK2_SHOW_SOFT_ERRORS` into immutable per-command soft-error contexts carried by `EventDispatcher`. Move emission quotas into the same context so concurrent commands cannot reset or consume one another’s allowance. Keep a process-lifetime fallback for work without a command dispatcher. Preserve command context across task, thread, version-control, page-out, completion, heartbeat, blocking-I/O, deferred-materializer, persisted dep-file, worker, debugger, test-orchestrator, and detailed-metrics boundaries. When a command-scoped soft error reaches a worker without an ambient dispatcher, route it only to the active command that owns the context instead of broadcasting it to unrelated clients. If the owning command has already exited, the event is still written to Scribe but is deliberately not injected into another command’s event stream. Reviewed By: scottcao Differential Revision: D120195703 fbshipit-source-id: 720554d1178e50516630368b25f02993791b7246

Coordinate concurrency admission through an actor Summary: Move repository-local admission state into a Tokio coordinator task while preserving the `ConcurrencyHandler::enter` facade and existing policy. The coordinator owns complete pending commands, active commands, DICE status, cleanup epochs, and blocked waiters. Encode pending and active ownership as distinct, non-cloneable handles so admission atomically consumes pending ownership into active ownership and lifecycle responsibilities cannot be duplicated. Command tasks retain the update permit, updater, `DiceTransaction`, observer, telemetry, and command execution so the coordinator never awaits external work. Replace the condition variable and separate queued registry with targeted wake channels and a cancellation-safe command lease. Normal completion waits for release acknowledgement; cancellation uses the lease drop path. Prepare cleanup in the coordinator before the triggering transaction can be dropped. If an after-update reply is dropped, restore any preemption senders borrowed from active commands before releasing the arriving command, so cancellation cannot silently disable later preemption. The actor lifetime is channel-owned: dropping the final handle closes blocked wake channels and lets the coordinator task terminate without retaining itself. Reviewed By: JakobDegen Differential Revision: D120751935 fbshipit-source-id: 5db7ed2f2af4c1c7afb805ccb9a0a923df65c348

Tag Starlark runtime type errors for telemetry Summary: Runtime Starlark type failures currently collapse into the broad STARLARK_FUNCTION, STARLARK_VALUE, or STARLARK_ERROR categories. That makes them indistinguishable from unrelated Starlark failures without fragile message matching, and prevents reliable measurement of the work wasted before a type error terminates a build. As a rough indication of scale, the existing broad proxy found 3,878 local failures (17.36 summed client-hours and 301,082 action attempts) and 72,795 CI failures (731.44 summed client-hours and 26.3M action attempts) over 28 days. Those numbers are intentionally only a proxy; this change creates the durable, exact cohort needed to replace it and to evaluate static-checker shadow mode. Introduce ErrorKind::RuntimeType with stable subtypes for function calls, parameters, value unpacking, operations, attributes, and annotations. Emit the secondary STARLARK_RUNTIME_TYPE_ERROR tag and a starlark_runtime_type_error=<SUBTYPE> category-key component while preserving the existing primary tags so current dashboards and classification remain stable. How to use the data after rollout: 1. Select exact failures in scuba_buck2_errors with contains(tags, 'STARLARK_RUNTIME_TYPE_ERROR'), or invocation rows with contains(error_tags_normvector, 'STARLARK_RUNTIME_TYPE_ERROR'). 2. Catalog failure modes by the starlark_runtime_type_error=<SUBTYPE> component of category_key. 3. Join distinct (ds, uuid) rows to scuba_buck2_builds for client wall time, command/client/CI dimensions, action counts, and total execution time. 4. Join (ds, uuid) to scuba_buck2_actions and sum execution_duration_ms by action kind (for example CXX compilation) to estimate executed resource waste rather than cache hits or scheduled work. 5. Split local human and agent traffic with is_agent_command in infrastructure.buck2_builds_user; identify CI with sandcastle_alias IS NOT NULL, deduplicating on (ds, uuid). 6. Correlate the exact runtime cohort with the static checker's shadow result by invocation UUID or normalized error fingerprint. The matched share and its client/action cost estimate the failures and waste static checking could prevent. Reviewed By: akrieger Differential Revision: D121638353 fbshipit-source-id: c2aacc33274362eec2843b6ec437db557cac8734

Stop naming cancelled commands as the exclusive lock's owner Summary: Track exclusive-command waiters with RAII tickets instead of a positional name queue. A waiter is registered before awaiting the write lock and removes its own entry if it is cancelled or completes. This prevents cancelled commands from being reported later as the exclusive lock owner. The change affects telemetry only; admission decisions do not read this data. Reviewed By: akrieger, JakobDegen Differential Revision: D115625056 fbshipit-source-id: f0f0feedcd5ac757c4fc4843121cb8c254cd9093

Route commands and debug RPCs by explicit tenant identity Summary: Add a stable `(project_root, isolation)` tenant identity to `ClientContext` and populate it from client invocation paths. The server validates that identity into `TenantPaths`, then uses `TenantStateRegistry::get_or_create` and `RepoStateFactory` to initialize the selected `RepoState` exactly once. Concurrent commands for the same tenant share initialization. Command handling, profile and allocative diagnostics, dep-file flushing, and DICE debug RPCs route to the selected tenant; context-free daemon RPCs explicitly use the initial tenant. Clients without the new field retain compatibility while the daemon has one tenant and receive an error once it serves multiple tenants. Reviewed By: JakobDegen Differential Revision: D120611547 fbshipit-source-id: f1b104ea874a5c0a465752be65fe731267cec39b

Separate tenant and repository state Summary: Introduce a concrete `TenantState` that owns an `Arc<RepoState>` and the tenant-specific `ConcurrencyHandler`. Rename tenant construction and lookup APIs accordingly, and route command, paging, snapshot, and debug DICE access through the selected tenant. Repository services remain behaviorally unchanged behind the nested `RepoState`. Reviewed By: akrieger Differential Revision: D121489109 fbshipit-source-id: ddda5705ce417a3371e8c3a3bfc4552f75029e8d

Invert the buckconfig telemetry dependency out of the concurrency manager Summary: First step towards extracting `ConcurrencyHandler` into its own crate. Pure dependency inversion — no behavioural change, and deliberately no fix to the critical-section problem the moved code has. `wait_for_others` performed buckconfig-derived telemetry inline: compare the command's config against the previous command's, emit experiment tags, emit the config values. That block was the sole reason `concurrency.rs` depended on `buck2_common`, and the sole user of two of `enter`'s twelve parameters (`previous_command_data`, `project_root`) and of both of the file's intra-crate imports (`crate::ctx::LockedPreviousCommandData`, `crate::experiment_util::get_experiment_tags`). Replaced with a `CommandTransactionObserver` trait defined in `concurrency.rs` and implemented by `BuckconfigTelemetry` in `ctx.rs`, which closes over the dispatcher, project root, previous-command data, argv and trace id. `enter` takes `&dyn CommandTransactionObserver` in place of the two parameters, so its arity drops from 12 to 11. The observer is invoked from exactly where the inline block was: under the concurrency lock, after the transaction is committed, before the command is registered. That ordering is load-bearing — the previous-command comparison reads state shared between commands and is currently serialised by that lock — so the trait's doc comment records it, and moving the call later is called out as a behavioural change rather than a refactor. Returning `Result` preserves the two `?` sites, so a failing config read still fails the command. The test helper `make_default_dice` also loses its `buck2_common` use. It called `set_none_legacy_config_external_data` to inject `LegacyExternalBuckConfigDataKey`, which existed only because the inline block read that key. With the block behind the observer and tests supplying a no-op one, nothing in the test path reads it, so the helper reduces to `Dice::builder().build(...)` and is no longer async. That detail matters more than it looks. `test_deps` are linked into the `-unittest` binary, which is what coverage instruments, so leaving `buck2_common` as a test-only dependency would have pulled configerator (1,221 targets) and thrift (1,064) back into the coverage closure and undone most of the win the extraction is for. Net effect: `concurrency.rs` contains zero references to `buck2_common`, `ProjectRoot`, or anything under `crate::`, in production or test code. The file is self-contained within the crate, which is what makes the subsequent move a pure relocation. Deliberately NOT done here: the redundant full blocking buckconfig parse this code performs under the global mutex is untouched. It is a real problem — a second parse of the whole repo, on `std::fs`, inside the critical section — but fixing it changes behaviour and belongs in its own diff, backed by coverage. Note `buck2_server_ctx` still depends on `buck2_common`, correctly, because `ctx.rs` now uses it. The dependency-tree win only materialises when `concurrency.rs` moves to its own crate. Reviewed By: scottcao Differential Revision: D115445067 fbshipit-source-id: 719c2ead4044227644f53c9f4569f230f198b5fd

Answer --exit-when=notidle before doing the work Summary: Evaluate `--exit-when=notidle` from `active_commands` in the first critical section, before running the file-watcher update and DICE commit. A busy daemon now rejects immediately instead of doing expensive work first. The check reflects state when the command arrives, so it can reject even if the other commands would have finished during the update. Five related tests also gain bounded waits. Reviewed By: JakobDegen Differential Revision: D115607484 fbshipit-source-id: 952f46861357f48ad8a8a396fc6c143e37d58b8f

Scope revision interpolation to the selected repo Summary: Replace the process-global version-control revision cache with a lazy cache owned by each command executor factory and rooted at its selected repository. Both Hg and Git revision commands now receive that root explicitly. Keep `$(revision)` deferred while parsing remote-execution dependencies, then resolve it when the repository-specific remote executor prepares the request. This preserves per-invocation refresh behavior without allowing concurrent tenants to share revision state. Remove the obsolete thread-local test shim and cover independent repository roots, per-instance caching, and enabled or disabled interpolation. Reviewed By: JakobDegen Differential Revision: D119574221 fbshipit-source-id: bde5c59fc01fbf727c9868b734aecd2430f035f2

Split DaemonStateData into per-repo and daemon-wide halves Summary: Groundwork for serving multiple repos from one daemon. Split `DaemonStateData` into daemon-wide state and a new `RepoState` containing repo paths, configuration, and repo-scoped services. The split keeps `re_client_manager`, `blocking_executor`, and `forkserver` on `DaemonStateData` because their intended lifetime is daemon-wide. Their constructors still use the sole repository paths or root, so additional work is required before multiple tenants can be enabled. The daemon still owns exactly one `RepoState`, so behavior is unchanged; later diffs make command-scoped repo selection explicit. Reviewed By: akrieger Differential Revision: D115649549 fbshipit-source-id: 2e845f02bad059730be89a8b13464fe817fd80c2