fix: target owned monitor repository#2
Merged
Merged
Conversation
yaacovcorcos
added a commit
that referenced
this pull request
Jul 26, 2026
…findings Ensure the provider updater child process is only ever spawned against a target that was probed, certified, and re-validated under the settings write lock, closing six concurrency/security windows in the confirmed-update boundary: - #1 Immutable probe/settings snapshot threaded through refresh; a single serialized refresh (refreshSemaphore) captures one snapshot and revalidates confirmed targets at the commit boundary. - #2 updateProvider re-validates the confirmed target and captures the exact updater command into immutable locals while holding withSettingsWriteLock, so a concurrent settings write cannot change what gets spawned between validation and capture. The child is spawned OUTSIDE the lock (spawn + await share one update-timeout budget), so a slow or hung spawner.spawn — whose acquire is uninterruptible — can only stall its own request and can never pin the global settings write lock. - #3 Request-owned update state (per-request owner token); a losing duplicate cannot clobber the in-flight update's running state. - #4 Interruption-safe cleanup lands a terminal failed state and kills the child via a scoped finalizer. - #5 Hot getStatuses/stream reads re-derive only a cheap authority key (revision counters) instead of the full maintenance context, so reads never re-probe CLIs or re-resolve the runtime. - #6 Runtime target identity tracked by a monotonic per-provider revision counter (ProviderRuntimeManager.getRevision), excluded from transient install-progress churn; PROVIDER_KINDS derived from ProviderKind.literals. Adds deterministic regression tests for each finding (no sleeps; barriers via Deferred / TestClock.withLive / scheduler drains), including a mutation- sensitive hot-read guard, a mutation-sensitive guard that the settings write lock is released before the unlocked spawn, a hung-process timeout guard, succeeded/unchanged post-update re-probe guards, and a per-provider revision-isolation test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
yaacovcorcos
added a commit
that referenced
this pull request
Jul 26, 2026
Follow-ups to Codex's review of the project-removal turnstile (fc43522): P1 #1 — drop the `projectOperationAlreadyHeld` bypass in the slash-command creators. Every project-mutating creator now re-acquires its own removal-coordination lease via `tryBeginProjectOperation`, which fails closed once removal is reserved, so a concurrent lease can no longer let a creator skip the turnstile and orphan the thread it creates. The composer draft is cleared only when the operation actually starts. Adds a regression test that holds a concurrent lease across the reservation and asserts the creator is refused by the turnstile (not the "Side is unavailable" early guard). P1 #2 — derive the removal-confirmation thread count from the live store (`getThreadsFromState(useStore.getState())`) at click time instead of the captured `sidebarThreads` render snapshot, so the count the user consents to never lags a stale render. Browser-suite isolation — the ChatView suite reset only 4 of the store's data fields between tests. Tests that dispatch a real `project.delete` tombstone the project id in `deletedProjectIdsById`; left behind, the next test's project route resolved to "deleted" and its thread data never loaded (33 cascading failures). `resetAppStoreForTests` now resets the full store (shallow-merge, preserving actions), and `resetChatViewDispatchGatesForTests` clears the outside-React send/lease gates. Full ChatView browser suite: 102 passed | 11 skipped, 0 failed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
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.
Fix
Pins GitHub CLI issue and label operations to
github.repositoryafter the workflow adds the official Synara remote.The first manual monitor dispatch proved that remote inference otherwise targeted official Synara and failed safely with HTTP 403. No upstream or product state changed.
Verification
git diff --checkcleanreviewedThroughequals the current official tip