Repository navigation
Stop a tab drag being stranded by the tab changing under it - #627
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
Claude diff review of 7a15e93 (no tests executed). Review of PR #627 @ 7a15e93Scope note: this is a diff-only review. I could not inspect Confirmed defects / high-confidence issues1.
|
|
@kshivang Your diagnosis of the stranded ghost and the two real Compose gesture regressions are useful, and your original commit/credit are preserved. I pushed review repairs to this branch (current head I checked Claude's current original-head review against the full source:
Added coverage for both surfaces, actual reindexed reorder results, latest callbacks, disposing an idle same-id chip versus its owner, and button identity replacement. Original-head CI had 10 successful checks; Supabase preview was skipped. These are NOT evidence that the new repairs pass. Local Gradle never started because the shared build lock stayed occupied; I interrupted only my queued command after the bounded wait. This is shared validation infrastructure, not an author defect. I am converting this PR to draft because current-head validation/review is outstanding, not because the useful change should be abandoned. Next steps: run Main has distinct cross-pane insertion behavior from merged #284; preserve its target-index behavior during eventual integration. #248/#253/#284 are related context, not duplicates. Integration #651 is confirmed merged and was read-only. @claude Please review the repaired current head once, especially gesture ownership, reindex/drop semantics, and the added regression tests. No tests have been claimed passed on this new head. |
|
Claude finished @kshivang's task in 6m 5s —— View job Review of the repaired head (
|
cd88490 to
5a2e818
Compare
|
I audited #627 against the newer #690 cross-pane reproduction and found one remaining ownership gap. The new
That cleanup is keyed by tab ID rather than the specific drag instance. During a cross-pane move, the source This is the remaining gap against #690's source-scoped ownership requirement. I suggest removing/replacing that legacy disposal path with the same gesture-owned identity semantics already used by the new
An equivalent test for I did not modify the branch or create a competing PR. |
75ebc71 to
628f152
Compare
|
Thanks for digging into the tab-drag race. The build summary and code-quality checks are currently failing. Please keep working through them until CI is green. Appreciate the interaction work! |
d4dd36a to
bdcf620
Compare
… running git (#930) * fix(git): re-validate the remote-prefix-stripped checkout name checkout() gated the picker's branch name with isSafeRefName, then stripped the remote prefix and handed the stripped result to git untouched: a branch named origin/-f - legal as a whole, plantable by anyone who can push a branch name - strips to -f, so the command assembled as `git checkout -f --` and silently discarded every uncommitted modification. The stripped name now runs through the same isSafeRefName gate; flag-shaped, blank and oversized results are refused before git sees them. BossTopBar's onCheckout was also the one write verb passing no projectPathOverride: the global path belongs to whichever window aligned it last, so a second window's checkout could act on the other window's repository. It now passes windowProjectPath like its neighboring verbs. Upstream: #919 * test(git): add the safe-name control to the checkout gate suite A slashed remote-style name with the same uncommitted work the flag-shaped refusal guards must still DWIM to its tracking branch and carry the modification - proving the gate refuses flag-shaped stripped names only, not slashed names wholesale. * test(git): pin the checkout gate's own refusal and share the clone fixture Address the Claude review on the exact prior head: - The strips-to-empty test passed against unpatched dev (git checkout "" -- fails on its own), so it pinned git's behaviour rather than the fix. Both refusal tests now assert the gate's own Error message, which only the isSafeRefName(localName) line can produce. - The flag test's HEAD check used rev-parse --abbrev-ref, which is stable under a same-branch HEAD move; it now compares the full hash. - The planted origin/-f ref gets an existence precondition so a future git rejecting that refname cannot leave the test silently modelling a phantom attack. - The duplicated clone fixture (which had diverged by one core.autocrlf line) becomes one cloneOf() helper used by all three remote-style tests. - The BossTopBar comment no longer calls checkout "the one write verb still on the global path": stash/stashPop below it still run on the global currentProjectPath because their signatures take no override. Agent repair commit (sweep 2026-09-22T0929Z-codeq); the original contribution remains the gate in DesktopGitService.checkout. * style(git): satisfy ktlint on the planted-ref precondition chain --------- Co-authored-by: Boss PR Review Agent <pr-review-agent@risa-labs.local>
…on or logging (#931) * fix(browser): gate Navigate schemes and redact URLs in the service log (#911) The browser service accepted any Navigate URL verbatim: a javascript: URL was handed to the engine as script to evaluate in the page's own context, reported back as success=true with a COMPLETED navigation event, and logged raw at INFO in the browser-service log — the same log path that printed userinfo credentials (https://user:Secret@host/) verbatim. - Navigate now refuses every scheme outside the http/https/file/ftp allowlist — javascript: and data: included — before any state is written, any event is emitted or the URL is logged, so a refused navigation leaves no COMPLETED event, no slot update and no log line carrying the URL. The scheme is read the way an engine reads it (tabs and newlines stripped first, case-insensitive, RFC 3986 syntax), so "jav\tascript:", "JAVASCRIPT:" or a schemeless string cannot walk the gate. - The navigate and getFavicon log lines route the URL through LogSanitizer.redactUrlUserInfo, so userinfo never reaches the browser-service log; the response and navigation-event contract still carries the full URL. - Reload now emits the same synchronous STARTED -> COMPLETED pair navigate has and stores isLoading=false, so getPageInfo no longer reports the window loading forever after a reload. * fix(browser): log navigations through describeUri and keep the gate http/https-only Address the Claude review on the exact prior head: - The navigate and getFavicon log lines used redactUrlUserInfo, which is weaker than the convention BrowserUrlLogConventionTest was written to enforce: it leaves the query and fragment (an OAuth code, a presigned signature) in the log, and returns input without :// unchanged, which covers https:user:secret@host. LogSanitizer.describeUri drops the userinfo, query and fragment, and the source-scan test now requires it. - file: and ftp: left the allowlist, matching the repo's existing URL policy for anything that opens in a browser tab (UrlOpenValidation is http/https only; file: is a local-file read into a window, and on Windows file://host/share is an SMB fetch; ftp: is dead in Chromium). No client of Navigate exists in-repo yet, so tightening is free now. - The refusal message caps the echoed scheme at 32 characters: the charset is validated but the length is not, and the log contract is "never the URL". - Tests added for the file:/ftp: refusals, for the refusal carrying the scheme name and no payload, and for the capped overlong scheme; the navigable set is http/https. Agent repair commit (sweep 2026-09-22T0929Z-codeq); the original contribution remains the scheme gate, refusal-before-state ordering, reload wedge fix and test matrix in this branch. * style(browser): satisfy detekt MaxLineLength in the capped-scheme test --------- Co-authored-by: Boss PR Review Agent <pr-review-agent@risa-labs.local>
* fix(logging): mask Supabase's current API key shapes Supabase replaced the legacy JWT anon and service_role keys with opaque sb_publishable_ and sb_secret_ strings. credentialShapePattern recognised a JWT, a GitHub token and a vendor sk_/pk_ key, so neither new shape matched any branch and both passed through every free-text path untouched. Measured on dev before the change, through the real sanitizer: in : connect failed: apikey sb_secret_A1b2C3d4E5f6G7h8J9k0L1m2 rejected out: connect failed: apikey sb_secret_A1b2C3d4E5f6G7h8J9k0L1m2 rejected against a JWT in the same call, which came back as eyJ...123. The free-text paths are where this lands: sanitizeExceptionMessage, sanitizeLogMessage and sanitizeStackTrace are what a client library's own message goes through, and a rejected-key message is exactly the shape that carries one. Those lines reach the Console capture that console_tail and console_search serve and that every plugin reads through PluginContext.logDataProvider. sb_secret_ is the service_role replacement, so it bypasses row-level security entirely. A value in a map position was already covered, by looksLikeSecret's length test rather than by shape. This closes the text path, which had nothing. BossConsole#870 closed the JWT case for the realtime client and recorded these shapes as follow-up scope. This is that follow-up. The two prefixes are named rather than accepting any sb_, because over-masking a diagnostic is the failure looksLikeSecret's own KDoc warns about: a path, a URL or a command is the thing a reader most needs, and sb_config_reloaded is not a credential. The pattern exists twice. McpArgumentSanitizer carries a byte-identical copy whose KDoc says it is the same rule, and the two serve one decision from opposite sides: what an operator sees before approving a mutating tool call, and what a reader sees in the log or the ledger afterwards. A shape masked in one and printed in the other is the worst of both, so both are widened here and McpArgumentSanitizerCredentialShapeTest now pins them against each other by reading the literal out of both sources. Comparing sources rather than behaviour because a behavioural sample can only prove the cases someone thought to list. Verified by running it. Red first: the three sb_ assertions fail against unmodified dev while the two guard assertions pass, so they are not passing vacuously. Green: 61 tests in plugin-logging and the composeApp sanitizer suites. Mutation: removing the branch from LogSanitizer alone fails the three shape tests, and removing it from McpArgumentSanitizer alone fails the drift test by name plus its two argument tests. Not covered: opaque JSON credentials, the other half of what #870 recorded, still need a shape nobody has proposed. Other vendors that moved to opaque prefixes are unaddressed for the same reason; this adds the one whose keys this product actually issues. BOSS still configures SUPABASE_ANON_KEY as a legacy JWT, so this is a gap closed ahead of the rotation rather than a leak observed in a log today. (cherry picked from commit 145cc1d) * fix(logging): make the drift guard compare real pattern sources; sync the shape docs Agent repair of Claude's pre-merge findings: 1. The drift guard's normalization deleted every '+' from the pattern source, so the JWT branch's [A-Za-z0-9_-]+ quantifier did not survive it and a copy that dropped the '+' in one file would normalize to the same string. The extractor now anchors on the declaration, skips // lines while counting paren depth, and concatenates the contents of the quoted string segments instead of deleting separators. A new test pins that the + quantifier survives normalization. 2. Three doc sites still described three shapes after the Supabase branch was added (LogSanitizer's property KDoc, sanitizeExceptionMessage's Removes list, McpArgumentSanitizer's object KDoc); each now names the sb_ shapes. The inline comment inside the Regex literal moved into the property KDoc where the per-branch reasoning lives. 3. SupabaseKeyShapeTest records the sbp_ personal-access-token gap in its 'Not covered' note. The test KDoc's #870 attribution is corrected: #870 recorded the shapes as follow-up scope, it did not add them. * style(logging): satisfy detekt and ktlint in the drift-guard extractor The single walk now keeps one jump statement (the break) and skips comment lines via if/else, and the multiline expression starts on a new line. The extracted pattern is unchanged. * style(logging): split the drift-guard walk so it satisfies detekt depth Move the paren-depth scan into regexArgumentListEnd so the single loop holds one break and the block depth stays within the detekt limit. The extracted pattern is unchanged. * docs(logging): break the circular attribution between the two pattern KDocs LogSanitizer's credentialShapePattern is the original; its KDoc now says so and names the guard that pins the McpArgumentSanitizer copy against it. The test KDoc's reformatting claim is scoped to line breaks between segments. * style(logging): wrap the helper signature for ktlint * fix(logging): keep the drift guard comment-aware in the extraction pass Claude's final re-review flagged that the split moved comment-skipping into the depth walk only: a //-comment holding a quoted word inside the argument list would be concatenated into that file's extracted pattern. The extraction now skips //-lines before the segment regex runs, restoring the parity the pre-split version had. The class KDoc also made the broader claim than the helper KDoc: .? does not cross newlines, so splitting one triple-quoted segment across lines mangles it. The limit is now "between its segments", in both places. The remaining nit (paren counting inside string segments) is rejected with evidence in the PR thread: the walk is comment-aware, Kotlin string literals cannot contain unescaped newlines, and a regex literal is syntactically well-formed, so no unbalanced paren can appear outside a quoted segment. * fix(logging): give the drift guard one scanning rule instead of two Claude's final re-review approved the masking and the comment-aware extraction, then corrected the recorded reasoning behind the rejected paren nit: regex syntax does not require balanced parens as raw characters - [)], [(], \) are all valid - so the old depth walk could terminate early inside a quoted segment and silently stop comparing both files from the same point. It also flagged that the whole-line comment skip missed trailing // comments, which the depth pass did skip. The extractor is now a single pass over runs - a triple-quoted segment, an ordinary quoted segment, a // comment to the line end, or a single character - that counts the parens bounding the list and collects the segment contents at the same time. One scanning rule, so a paren inside a quoted segment and a quoted word inside a comment can reach neither the counter nor the extraction. The extracted string is byte-identical on both current files (verified with an independent re-implementation: identical output, equal to the real runtime pattern, + quantifier surviving), and a new test pins that a trailing // comment containing a quoted word does not leak into the extraction. * fix(logging): make the drift guard fail loud on the paths it can silently miss Claude's re-review approved the single-pass extractor, then named the two remaining silent paths: - an argument list with no quoted segment at all (a refactor to a constant) extracted "" from both files, so the equality check compared nothing and passed. The extractor now returns null on an empty result, so the existing "no credentialShapePattern literal in <name>" error fires; a new test pins the bare-identifier case. - the trailing-comment fixture used "(a)", whose parens are balanced, so it discriminates the comment fix but not the paren fix that motivated the rewrite. A new fixture uses "[)]" (valid regex, valid Kotlin, an unbalanced closer as raw characters): a character-wise counter would stop the walk there and truncate both files at the same point, so this is red against exactly the defect class that was fixed. The class KDoc's limit now names quote style and escaping alongside line breaks: the extraction keeps raw segment source, escapes included, so a triple-quoted segment and an escaped ordinary string that compile to the same regex extract differently - loud, but not something the KDoc should promise away. * style(test): remove the doubled blank line ktlint flagged ktlintDesktopTestSourceSetCheck failed on McpArgumentSanitizerCredentialShapeTest.kt:148 for a needless blank line before the credentialShapeLiteral KDoc. Dropped one blank line, nothing else touched. --------- Co-authored-by: kshivang (review agent) <kshivang@users.noreply.github.com>
* fix(plugin-store): bound ratings pagination and stop returning rater UUIDs GET /:pluginId/ratings is unauthenticated, and it passed caller-chosen page/pageSize straight into a PostgREST .range() call: pageSize=99999999 asked for a massive range, 0/negative/NaN values produced malformed range math, and page=1e9 walked deep offsets - all on a route with no rate limiter. Every row also carried the rater's raw auth.users UUID, so the endpoint doubled as a mass identity-scraping surface at whatever scale the query allowed. - Validate page/pageSize fail-closed in the handler, before anything touches the database: integers only, page 1..100000, pageSize 1..100 (mirroring the cap /search already enforces). Anything else gets this route's fixed 400 envelope instead of becoming oversized or malformed range math. - Stop selecting and returning user_id from plugin_ratings: review rows are now anonymous (rating/review/createdAt). Nothing in-repo consumed the UUID and no display-name join exists, so the identity column had no legitimate consumer on this unauthenticated route. Adversarial coverage in tests/ratings-bounds.test.ts: oversized, zero, negative, fractional, non-numeric and beyond-cap values are rejected without a database call; the max bounds still pass; a normal page still works while the service never even requests user_id and the response body stays UUID-free. Fixes #915 * fix(ratings): tighten pagination bounds to 500/50 as promised on the thread Follow-up to the bounds discussion on #915: RATINGS_PAGE_MAX drops 100000 -> 500 and RATINGS_PAGE_SIZE_MAX 100 -> 50, the same family bound #1045 pins on /list, so the worst-case single request is a 25,000-row window and both PRs agree by construction. The handler still fails closed with the same fixed 400 envelope before any database call; only the constants (and the numbers they render into the message) move, and the tests now exercise the exact edges: 500/50 passes validation, 501/51 is rejected, and the range ceiling at max pageSize is 50 rows. Fixes #915
…route (#1541) get_popular_tags is anon-callable on purpose, because the plugin store is browsable before sign-in, so anyone holding the anon key the desktop client ships can call it through PostgREST and never pass the /tags/popular route that #1508 caps. Its body ended in LIMIT p_limit, and LIMIT NULL is LIMIT ALL. The rows are public tags only, so this is a cost bound, not a disclosure fix. The function now clamps the same way the route does: NULL takes the default of 20, anything below 1 becomes 1, and anything above 100 becomes 100. Values inside 1..100 are unchanged, and so is the signature, so the audits that name get_popular_tags(integer) still match. The replace re-grants EXECUTE to anon, authenticated and service_role, because the enforce_explicit_anon_grants event trigger fires on the CREATE FUNCTION tag that CREATE OR REPLACE also produces and revokes anon. The pgTAP test pins both the grant and the clamp, calling the function as anon the way PostgREST does, against a fixture of 150 public tags so the cap is observable. Refs #1253. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…For hop, not the caller's (#1664) clientKey used the LEFTMOST X-Forwarded-For entry, which is whatever the caller sent. Rotating it on every request bought a fresh bucket each time, so the catalogue and download-info limiters never tripped, and each request added a map entry until MAX_KEYS forced a clear that dropped every other client's count too. Writing someone else's address there spent their budget instead. Derive the key the way the passkey limiter already does (#1335): cf-connecting-ip, then x-real-ip, then the RIGHTMOST X-Forwarded-For entry - the hop the gateway appended for the connection it accepted. Fixes #1628
* fix(plugin-store): cap catalogue schemas on pageSize, query, tags, githubUrl host and popular-tags limit (fixes #1247, #1249, #1250, #1253) BossConsole#1247 (list route), #1249 (search route), #1253 (popular-tags) let callers push unbounded `page`/`pageSize`/`query`/`tags`/`limit` into the SQL RPCs and the edge function. BossConsole#1250 accepted any URL whose text contained the substring "github.com" rather than parsing the host. Bound all five to the same shape the search route already had. Co-authored-by: Claude <noreply@anthropic.com> * fix(plugin-store): bound /tags/popular, whose limit reached LIMIT unchecked GET /tags/popular is public and sends the anon key. Its limit reached `LIMIT p_limit` in get_popular_tags with nothing bounding it anywhere on the way: not the route (`z.string()...transform(Number)`), not getPopularTags, which passes it straight to the RPC, and not the SQL function, which is a bare `LIMIT p_limit` in both the 20260130 and 20260803 definitions. That allowed two things, and the second is the worse one: - an oversized integer asked for an arbitrarily large window; - a non-numeric value removed the bound entirely. Number('abc') is NaN, JSON has no NaN so the RPC payload carries null - verified: JSON.stringify({ p_limit: Number('abc') }) is '{"p_limit":null}' - and PostgreSQL documents that a LIMIT evaluating to NULL is LIMIT ALL. So ?limit=abc was less bounded than ?limit=99999999. The fix follows the shape the open ratings fixes (#949, #1045) use for their routes: take the value as a raw string so the handler owns coercion, and refuse anything that is not an integer in 1..100 with the route's own 400 before the database is touched. Number.isInteger rejects NaN, fractions and Infinity in one test. 100 is the ceiling /search already enforces on pageSize. Nothing in-repo is affected: PluginStoreClient.getPopularTags defaults to 20 and has no caller passing anything else. This route is not covered by #1045, which bounds /list and /ratings and does not touch browse.ts. Tests follow error-response.test.ts, a route-level harness already on main. A client whose rpc throws proves refusal happens before the database, since a request that got through would surface as the route's 500 instead of 400. A recording client checks the value that actually reaches p_limit, not only the status. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(plugin-store): import @std/assert through the import map The previous commit's test used the inline `jsr:@std/assert` specifier, which deno lint's no-import-prefix rule flags. The job is green because that lint step is advisory - the workflow notes it stays non-gating precisely because existing tests carry this finding - so adding one more instance grows the debt that keeps it from being gated. deno.json already maps `@std/assert` to `jsr:@std/assert@^1.0.0`, and download-hash, publish-authz and publish-org already import it bare. I had copied error-response.test.ts, which is the one route test still using the inline form. The bare specifier also pins the major version, which the inline form did not. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(plugin-store): match popular-tags validation envelope The consolidated route uses the OpenAPI Zod validation envelope, so assert the security boundary and database refusal without coupling the test to the formatter.\n\nCo-authored-by: Arjun Singla <singlaarjun28@gmail.com> * chore: retrigger CI --------- Co-authored-by: choksi2212 <manaschoksiwork@gmail.com> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Arjun Singla <singlaarjun28@gmail.com> Co-authored-by: fluck-boss <fluck-boss@users.noreply.github.com>
…it skips (#1667) An expression-bodied test takes the type of its last expression, and runBlocking<T> returns whatever its block does. So a test ending in assertIs/assertNotNull/assertFailsWith compiles to a method returning a value, and JUnit Jupiter does not execute a @test method that returns a value. It reports a WARNING-level discovery issue, which Gradle does not show, and the build stays green. Nine composeApp tests were in that state, four of them the MCP approval gate's fail-closed guarantees. Each is now declared `(): Unit =`. Eight pass once they run. The ninth asserted that a caller with no credential can unregister an unknown UI surface. That stopped being true when #776's ProcessIdentityInterceptor began authenticating every RPC, and the test never ran to notice. It now pins the current contract: a no-op for an authenticated caller, UNAUTHENTICATED without a credential. composeApp's Test tasks set junit.platform.discovery.issue.severity.critical=WARNING, so a test JUnit will not run fails the build. Reverting one of the nine fails it with "@test method ... must not return a value. It will not be executed." Full desktopTest: 6,529 tests, 0 failed. On dev, an approval bus whose full buffer returns Approved() passes the build; with this change the buffer test catches it. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…nter.post (#1666) (#1670) * fix(notifications): enforce the inbox's size bounds in NotificationCenter.post (#1666) #1644 bounded notification_post at the MCP provider, and #1660 put the snippet caps in the store. Two adjacent fixes on opposite conventions, and this one left a hole: post is documented as serving the host too, and a HOST-origin post met none of the three bounds. - MAX_TITLE_CHARS and MAX_MESSAGE_CHARS move to NotificationCenter and are required in post beside the existing blank-title check, for every origin. - A HOST source label longer than MAX_SOURCE_LABEL_CHARS is refused. An agent's label keeps #1619's truncation: an agent is not ours to fix, while a host label that long is a bug to catch where it is written. - The MCP tool keeps its check first so its refusal names the field and the limit. - Tests: over-long title and message refused from both origins with nothing stored; all three exactly at their limits accepted; host label refused while an agent's is cut. The provider test pins MAX_LIST_LIMIT < MAX_ENTRIES, which the clamp depends on. Mutation-checked: removing each of the three requires fails a named test. 416 tests across ai.rever.boss.notifications.* and ai.rever.boss.mcp.* pass; ktlint and detekt clean. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(notifications): cut a long host message instead of refusing it; flatten host labels Review follow-up on #1670. - A host message over MAX_MESSAGE_CHARS is cut, ending in an ellipsis, never splitting a surrogate pair. A host builds its message from dynamic text (an exception message, a command's output), and refusing would turn a notice about one problem into a second problem on an error path. An agent's message is still refused, because the MCP tool checks first and names the limit. Titles are still refused from either origin. - A host label is flattened to one line like an agent's, and the cap is measured on the stamped label, which is exactly what is stored. So trimmed whitespace cannot tip a label over. - post's KDoc states the rule per field and origin, with @throws for the three refusals. MAX_ENTRIES states the aggregate bound, about 860,000 characters and more once escaped, as SnippetLibraryManager's does. - Tests: the at-limit test asserts every field is stored intact. New cases cover a host label padded with whitespace at the cap, a host label with a newline, a host message cut with its ellipsis, and a cut that lands on a surrogate pair. Mutation-checked: storing the host message uncut, dropping the surrogate guard, not flattening the host label, measuring it untrimmed, and not refusing the agent message each fail a named test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…1043) (#1044) The absolute-date branch of formatRelativeTime now passes Locale.getDefault(Locale.Category.FORMAT) to SimpleDateFormat explicitly, matching the rest of the codebase's date formatters. This is a clarity/consistency change, not a behaviour fix: the bare SimpleDateFormat("MMM d") constructor already resolves to the FORMAT-category locale, so the rendered output is unchanged. Add ProjectCardLocaleTest guarding that the branch honours the FORMAT-category locale (not the general default and not a hardcoded one), so a future regression to Locale.getDefault() or a fixed locale is caught.
…ope (#1029) (#1030) SplitViewOperationsImpl launched every plugin-facing operation on a bare CoroutineScope(Dispatchers.Main): no SupervisorJob, so one child coroutine failing cancelled the scope's Job and silently killed every later launch, and the scope was never cancelled, so a closed window leaked it and any in-flight Main-dispatched work. - Use CoroutineScope(SupervisorJob() + Dispatchers.Main) so one operation's failure does not take down the others. - Implement the existing DisposableProvider seam and cancel the scope in dispose(). DefaultPlugin.dispose() now releases the plugin-facing instance (via a named lazy delegate, guarded by isInitialized() like the other providers), and the composable that owns the BossAppState instance disposes it in a DisposableEffect. The scope is an injectable constructor parameter (default preserves production) so dispose() can be tested without a Main dispatcher. The detekt baseline entry for this class is updated to match its added supertype; it is the same suppression, not a new one.
…ugh atomicWriteText (#1011) (#1022) Migrate the listed settings/state managers' JSON persistence from a direct writeText to atomicWriteText (temp file then atomic rename), so an interrupted or concurrent write cannot leave a truncated or torn file. Not every writer in the app is migrated yet - only the managers listed in this change. Rebased onto current dev, which merged #1032's saveMutex serialization for the four settings managers (FocusMode, Performance, DesktopRunner, DesktopTerminalLink). This keeps that mutex and does the atomic write inside the lock, in both saveSettings and the load-time default-file write, for all four. The mutex only orders writes; atomicWriteText is what makes each write crash-safe, so the load-time read never sees a torn file even though it stays outside the lock - the mutex comment now says so. That is the remaining half of #1031 and closes #1659. Add a test asserting the write goes through the atomic helper: after an update the settings file is pinned to owner-only 0600 permissions, which atomicWriteText sets and a plain writeText does not, so the test fails if a call site regresses to a non-atomic write (the pre-existing no-leftover-temp check passes either way).
…l meets it (#1668) (#1671) The /list route caps pageSize (#1508), but search_plugins is anon-callable through PostgREST and passed p_page and p_page_size straight to search_plugins_internal, which ends in `LIMIT p_page_size OFFSET (p_page - 1) * p_page_size`. So `POST /rest/v1/rpc/search_plugins {"p_page_size": null}` was LIMIT ALL: every visible plugin, each with its own version, rating, download and tag subqueries. A negative size or page raised a LIMIT/OFFSET error, and a huge page overflowed INT. Both wrappers now clamp before calling the internal function: page size to 1..100 (NULL -> 20), page to 1..1,000,000 (NULL -> 1). The same shape as get_popular_tags in 20260922160000. Values inside the ranges are unchanged, and the route's own ceiling is 100, so no request the route accepts changes. Clamping in the wrappers avoids restating search_plugins_internal's hundred-line body, and they are its only callers: search_plugins_internal is service_role only. search_plugins is re-granted to anon, because the enforce_explicit_anon_grants trigger revokes it on CREATE OR REPLACE. search_plugins_for_viewer's service_role-only ACL is restated. The pgTAP test uses 150 public plugins. It pins null, zero, negative, one-past and oversized page sizes, null, zero, negative and INT-max pages, total_count unchanged, both band edges, the ACL, and the viewer wrapper. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Introduces {{secret:<id>}} references in MCP tool arguments, resolved in an ordered pre-pass ahead of the policy and approval path: denied or feature-off calls never touch the vault, resolution is all-or-nothing, a secret-bearing call always asks (tool or provider ALLOW, session trust and YOLO never satisfy it), the ledger records the original arguments and names the released references, and an optional result scrubber redacts echoed values. The pre-pass's argument parses sit under dev's depth guard, and the invariant suite pins non-disclosure, atomicity, ordering, approval binding, transparency, consistency and bounded work.
… spellings (#1656) * fix(mcp): stop masking a container uid:gid, and cover the cookie flag spellings Two findings from the review of #1019 that were left open when it merged. `docker run -u 1000:1000 nginx` came back as `docker run -u [REDACTED]`. The basic-auth rule keys on the `user:password` colon after `-u`/`--user`, and a uid:gid pair has the same shape, so one of the commonest docker and podman invocations there is had the very thing approval exists to show - which uid a container is about to run as - taken out of it. No credential is two integers, so a purely numeric value is now excluded; `-u admin:1234` still redacts, because only both sides being integers makes it a uid pair. The benign corpus gains the `-u` twin of the `docker run -p 8080:80` entry that should have caught this, plus the `--user=` and podman spellings. `Cookie: session=x` was closed by the header spelling alone. The flag spellings went through untouched: after `-b` or `--cookie` the cookie's name is whatever the site chose, so no list of names helps, and the flag has to be the signal. The whole value is taken, as the long-flag rule already does for `--password`. `-b` also names a jar file, which is redacted too: telling a path from a cookie needs a filesystem the sanitizer does not have, and over-masking one curl argument is the safe direction. Both are pinned as fuzz shapes rather than examples. Against the sanitizer as it merged, the two cookie shapes leak 60 cells and the benign corpus fails on the docker line. Also from that review, without behaviour changes: the failure message counts the distinct cells it prints rather than the pre-`distinct()` total, which with `repeat(3)` reported nine for three; and the generator's KDoc states the blind spot its alphabet creates, namely that a generated secret can never contain a character that ends a value, so no cell can discover that `password=Xk9&mQ2` leaves a tail in the clear. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(mcp): gate the short cookie flag on curl, and pin the -b false positives Review follow-ups. The first version matched a bare `-b` after any program, which reintroduced exactly the over-masking this change removes for `-u`: `git checkout -b feature/x`, `ssh -b addr`, `tar -b 20` and `gradle -b file` lost their argument, and flags that take no value (`cp -b`, `grep -b`, `wget -b`) lost the next positional. The short flag is now gated on curl the way `-p` is gated on its programs; `--cookie` and `--cookie-jar` stay unconditional because the name carries the meaning. The benign corpus gains `git checkout -b`, `cp -b` and `ssh -b`. The failure count now dedupes by shape and context: each failure string embeds that repetition's random secret, so plain distinct() collapsed nothing. uidGidValue moves above basicAuthFlag's KDoc, which it had detached from its declaration. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* ci: run the authenticated-ipc.yml Windows leg only for PRs into main * ci: run the build.yml Windows leg only for PRs into main Updated the OS matrix configuration for build-test job to clarify conditions for macOS and Windows runners. * ci: run the native-file-boundaries.yml Windows leg only for PRs into main * ci: run the filesystem-limits.yml Windows leg only for PRs into main * docs: note the Windows legs skip PRs that don't target main
… warning (#991) Adds buildSrc/settings.gradle.kts with an explicit rootProject.name, so the type-safe project accessors preview stops warning on every Gradle invocation and the build identity is explicit rather than derived.
…eads it (#1658) * fix(cli): refuse a quote inside a boss.bat argument before anything reads it Every argument read in boss.bat wraps the value in quotes - if "%~1"=="" - and cmd substitutes %~1 before it parses the line, so a " in the value closes that quote early. x"=="x" calc & rem " keeps its & quoted for the caller's own parse, but turns the first check into if "x"=="x" calc & rem ""=="" and runs calc. Capture the raw %* on an echoed REM line (the one place cmd writes it out without parsing it), read it back through a FOR variable, and refuse any quote that does not open or close a whole argument. Whole-argument quotes are all a path or URL needs, and with none inside, no %~N can carry a quote into the quoted reads, so the rest of the script is unchanged. Fixes #1617 * fix(cli): scope the boss.bat quote check to what is read, and claim the capture file Review on #1658: - status, doctor, mcp and completion pass their arguments to BOSS.exe as a bare %*, which cmd re-reads exactly as the caller's line was read, and never through %~N. Check only their first argument, so `boss mcp invoke tool --args {"q":"x"}` (docs/CLI.md) works again. plugin stays strict: it reads %~2 and %~3 before it forwards. - %RANDOM% is seeded from the clock, so two calls started together could share the capture file and each check the other's arguments. Claim a directory with md, which fails when it exists, and refuse the call if none can be claimed. - Clear the capture variables before anything is forwarded. - Run the live cmd.exe probes on the Windows leg of build-test, so the mutation checks run in CI and not only on a fork. Tests: forwarded JSON arguments, first-token and plugin strictness, escaped > >> | & < on the command line, and eight parallel calls that must each check their own arguments. * test(cli): pin that a forwarded argument stays data, and that a taken capture directory is refused - A payload in argument 2..N of a forwarded command reaches the stub and runs nothing, so a future %~2 read in a forwarded command fails the test. - A copy that always draws the same capture directory name finds it taken and refuses, instead of reading the other call's arguments. - The leftover check only judges capture directories made during the test. - The for /f read is guarded again, so a missing capture file shows only the script's own refusal. - The refusal points at boss.ps1 for a terminal command that needs a quote, and docs/CLI.md says what cmd accepts. - The Windows CI step runs right after checkout, in one pwsh.
…ly (#741) A window's Save now binds its live layout to that window's own active Space identity instead of the process-global one, rebinds the window only after the write has landed, and reports a failed write instead of premature success. A per-effect latch coalesces overlapping menu saves into one re-run on the latest layout, the named dialog ignores an overlapping submit, and late callbacks are fenced against window teardown.
* docs(plugin-store): clarify audit RPC failure behavior Clarify comments regarding JSON input rejection in audit payload. * fix(plugin-store): log resolved API key audit RPC errors * test(plugin-store): cover resolved audit RPC failures * fix(plugin-store): preserve safe audit failure kind
…d both providers (#1552) * fix(plugins): resolve the Downloads folder one way for the browser and both providers The browser's save location, the in-process FileSystemDataProvider and the out-of-process FileSystemDataProviderProxy each resolved the Downloads folder differently, so a plugin was told a different folder depending on which process it was loaded into. When ~/Downloads was absent, in-process plugins got the home folder and out-of-process plugins got a path that did not exist, with mixed separators on Windows. Add DownloadsDirectory to plugin-path-utils and route all three through it. The shared XDG parser also handles ${HOME}, XDG_CONFIG_HOME and last-wins. DownloadsDirectoryConsistencyTest fails on main and passes here; DownloadsDirectoryTest covers the per-platform rules on any OS. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(plugins): keep the Downloads consistency test out of the Windows ARM64 build, and read XDG edge values as xdg-user-dirs means them Review follow-up for #1552. - DownloadsDirectoryConsistencyTest imports FileSystemDataProviderProxy from plugin-api-ipc, which Windows ARM64 does not build. It is now excluded with SkipListDriftTest when that module is absent. WindowsArm64SourceIsolationTest lists it in unconditionalExclusions, so the build-file drift check agrees, and gains ai.rever.boss.plugin.ipc. in unavailablePackages, so the next such import fails on every platform instead of at release. - XDG_DOWNLOAD_DIR: an empty last value (how xdg-user-dirs records a disabled directory) no longer revives an earlier assignment; a relative value is read against home, not the working directory; a value naming home itself falls back to the conventional folder. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(plugins): treat $HOME/. and $HOME/sub/.. as home in XDG_DOWNLOAD_DIR Re-review follow-up for #1552. absolutePath does not remove dot segments, so those forms passed the "not home itself" check and handed back home. The value and home are now compared after Path.normalize(), which needs no filesystem access, so existence stays injected in the tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…#894) * test(auth): add UserDataStorage merge, clear, and race contract tests * test(auth): harden the torn-read detector against vacuous passes A detector thread that died on an unexpected error (or never decoded a record) would have left the torn-write test green while proving nothing. Record unexpected detector failures and successful decodes, assert the thread actually stopped, and add Thread.onSpinWait so the busy-spin stops fighting the writers for the cores the overlap depends on. Also correct the lost-update round comment (path coverage is the scheduler's choice, not the test's claim) and document on clearUserData that the pre-login pending_wizard_completed marker is deliberately preserved through logout. * test(auth): satisfy detekt on the hardened torn-write test Extract the detector iteration and the writer race into private helpers (LongMethod), drop the second jump statement from the detector loop, and suppress TooGenericExceptionCaught on the deliberate sentinel catch - the same convention as BrowserFindTest. * test(auth): close the remaining vacuity and flake holes in the torn-write race - replace the broad detector catch with an uncaught-exception handler (detekt's TooGenericExceptionCaught stays clean and the net widens) - restore the transient IOException tolerance dropped in the refactor - latch the writers to the detector's first disk read: overlap becomes a guarantee, killing the successfulDecodes scheduling flake - assert the detector saw a decoded form beyond the seed, so the test cannot pass when no racing write ever landed (e.g. every atomic move failing on a platform) - assert torn-reads before the inconclusiveness guards so a real finding is never buried; correct the onSpinWait rationale (PAUSE hint, not yield) - update the PR body: no behavior changes, only a clearUserData KDoc * test(auth): fix unscoped async and detekt SwallowedException in the race test - wrap the 24 racing writers in coroutineScope (a bare async in a suspend fun selects the deprecated unscoped overload, which fails compilation) - name the transient IOException catch '_' so detekt's SwallowedException stops flagging the deliberately ignored sharing-violation window * test(auth): duty-cycle the detector, guard-order it, and always stop it - parkNanos duty cycle instead of a hot spin: a spinning detector pins a core AND keeps user_data.json open a large fraction of the race, which on Windows denies the writers' Files.move(., REPLACE_EXISTING, ATOMIC_MOVE) delete access so every write no-ops - turning the non-vacuity assertion into a misleading loud failure. Sampling on a ~0.1 ms duty cycle still takes hundreds of samples across the race, which is plenty for a negative no-torn-read invariant, and lets the moves land. - detector-health guard now precedes the decodedForms non-vacuity guard (a detector that died right after its first read would otherwise read as 'no racing write ever landed') - stop + join the daemon detector in a finally so a latch-timeout or writer throw can't leave it spinning (re-reading storageFile) for the whole shared test-worker JVM - scope the 'cannot flake on scheduling' header claim to the mutex-based race tests (2 and 3); test 4 is observation-dependent/best-effort * test(auth): mark the merge race inconclusive when Windows lands no write (per review) --------- Co-authored-by: Mihir-Rabari <mihir.rabari@users.noreply.github.com> Co-authored-by: Shivang <shivang.iitk@gmail.com>
…ngs backend (#918) * fix(auth): persist Supabase session tokens through an encrypted Settings backend Root cause: install(Auth) left the supabase-kt defaults in place, and its default Settings() resolves to java.util.prefs.Preferences on the JVM, so the session JWTs - including a live refresh token, the highest-value secret in the app - were written as plaintext under ~/.java/.userPrefs (XML), the macOS plist, or the Windows registry: a library-owned store outside the ~/.boss directory this app otherwise keeps owner-only. Fix: EncryptedSessionSettings, a Settings backend that AES-256-GCM encrypts every value with a fresh 96-bit IV per write into ~/.boss/supabase/session-store.enc, with the random 32-byte key in session-store.key written through atomicWriteText so it lands 0600 on POSIX from its first byte, mirroring the #860 hardening. It is fail-closed: crypto unavailability or a corrupt key aborts construction, never falling back to plaintext. install(Auth) now wires SettingsSessionManager and SettingsCodeVerifierCache onto this backend, and a one-shot migration copies the legacy Preferences plaintext first and then destroys it, so existing sessions survive the upgrade and the live plaintext refresh token is revoked. Test: EncryptedSessionSettingsTest adds 10 cases, all green: injection of the encrypted backend into the Auth session manager and code verifier cache (assertSame through reflection into the private settings field), string and typed-value roundtrips across a restart, persisted bytes contain no plaintext, the key file is created owner-only on POSIX, remove/clear purge disk, tampered entries decrypt to nothing, legacy Preferences content migrates in with the plaintext destroyed afterwards, and the encrypted copy wins over a stale legacy copy. Fixes #846 * fix(auth): migrate URL-qualified legacy session keys with dual compatibility * fix(auth): keep the legacy migration within the detekt return-count rule Agent review fix for #918 (not contributor credit): migrateLegacyEntry had five returns, tripping detekt's ReturnCount limit and failing the code-quality gate. Fold the empty-populated and null-value guards into one check and the two warn-and-abort arms into a small helper; the fail-closed behavior (keep the legacy plaintext until the encrypted copy is written) is unchanged. * fix(auth): fail removals closed, regenerate unusable keys, pin library defaults - remove/clear destroy the store when the revoking rewrite fails, so a sign-out that reported success can never leave a revoked refresh token on disk to be auto-loaded at the next start - an undecodable or wrong-length key file regenerates instead of throwing: a corrupt key means sign-in again, not a Supabase client that refuses to build - the key file is created exclusively (Files.createFile) so two processes starting against the same directory adopt one key instead of each orphaning the other's store - a drift test pins supabase-kt 3.8.0's default URL-qualified key names and its Preferences.userRoot node against the library itself, so a dependency bump that changes either fails the build instead of silently stranding plaintext tokens in the legacy store --------- Co-authored-by: Shivang <shivang.iitk@gmail.com> Co-authored-by: Mihir-Rabari <mihir-rabari@users.noreply.github.com>
…ce directory (#880) * fix(mcp): keep workspace ids from selecting files outside the workspace directory open_workspace and close_workspace (#794) took a caller-supplied workspaceId ending in ".json" as the file name itself, and the desktop workspace file manager joined any name onto its directory without confinement. So open_workspace {"workspaceId":"../x.json"} read and applied a file beside the directory, and close_workspace with a "workspace-disposable-/../x.json" id deleted one on Windows, where ".." resolves lexically even when the component before it does not exist. ~/.boss/mcp-tool-policy.json, the persisted MCP policy file, is one directory up from the default workspace directory's parent and is exactly such a file. WorkspaceFileManagerCommon.isBareFileName states the rule (one component, no separator of either platform, not "." or "..", no NUL) and getWorkspaceFilePath enforces it, so every read, write and delete through the manager is confined; each caller already catches and logs, so a refused name surfaces as not found or not saved. The provider derives a file name from an id the same way the file was written (fileNameForId, after stripping an echoed ".json"), for both tools. Tests: the end-to-end open case (failed before), a close case that pins the derived name and the surviving file, and file-manager cases for delete, load, loadDocument, writeDocumentBlocking and saveWorkspace with a relative path (delete, load and the writes failed before). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(mcp): make the created file name agree with the read one, and say what the bare-name rule means Review follow-ups on the file-name confinement. The create path and the read path disagreed for an id ending in `.json`. `workspaceFileNameFor` strips the suffix before deriving, so `foo.json` reads `foo.json`; `createIfAbsent` kept the id verbatim, so `saveWorkspace` derived `foo.json.json` from it. A Space created that way could never be reopened by the id that made it, and a second `createIfAbsent` silently replaced its layout. The id is now stripped once where it is minted, so both halves derive the same name. Pre-existing, but the property this change claims is that the name read is the name written, and this was the case where it was not. `isBareFileName` now judges the component Win32 resolves rather than the one it was handed. Win32 trims trailing spaces and dots, so `".. "` addressed the parent directory while passing a `!= ".."` test; trimming collapses every all-dots-and-spaces name to empty, so one emptiness test covers `.`, `..`, `...` and `".. "` together. `:` is refused (a drive, `C:x.json`, or an alternate data stream, `notes.json:evil`) instead of relying on the JDK throwing InvalidPathException at the join, and the control-character test is `isISOControl` rather than NUL only, matching `isSafeWorkspaceId`. Neither case was an escape to a file, so this is hardening, not a hole being closed. The KDoc now says which layer this is: `isSafeWorkspaceId` is the id-level rule at the tool boundary, `isBareFileName` the name-level rule at the file manager, and the two differ on purpose (a name may contain `..` inside it, an id may not), so neither implies the other. `getWorkspaceFilePath`'s `expect` declaration documents the throw, which is the contract callers outside this class meet. Tests: an id ending in `.json` creates one file, named as the read path derives it, and reopens by either spelling; the bare-name rule accepts four real names and refuses thirteen shapes including `...`, `".. "`, `C:x.json` and `x.json:stream`; an empty name addresses the workspace directory and is refused on all four entry points. The two provider tests that dev's `isSafeWorkspaceId` now refuses first say so, and assert that message, so they read as regression guards for that gate rather than as cover for this change; the file-manager test is the one that fails without `isBareFileName`. Reverting the two fixes fails exactly the two new tests (`must refuse: '...'`, `round-trip.json.json`). 255 tests green; ktlint and detekt clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Atharv Gaur <atharvanna71@gmail.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…es (#957) * fix(mcp): share the startup-command refusal across open_workspace modes open_workspace's id mode refuses to open a saved Space whose layout carries terminal initialCommand entries - those commands were not visible in the invocation's approval arguments - but the path mode re-entered the same saved-Spaces machinery with no gate: opening a project path applied any saved Space registered for it (matchExistingSpace rule 3) and typed its stored terminal startup commands into a shell, unapproved. The refusal could be walked around by passing a path instead of an id. Both modes now route through one shared refusal (initialCommandsRefusal): the same message, the same shipped-template exemption, and the same placement before anything is applied - before the path mode's reuse fast path for the same reason the id mode's gate sits before its already-active return. The operator's doors are unchanged: the workspace UI loads a command-carrying Space behind its confirmation prompt, and open_terminal still types one command an invocation the risk gate has actually seen. Adversarial coverage: a saved Space whose terminal writes a marker file is refused by path entry with the id mode's refusal word for word and is never applied to the window, while a saved Space without startup commands still re-enters with its terminal intact. Fixes #920 Windows follow-up (first CI run caught it): the two new path-mode tests failed on windows-latest because matchExistingSpace compared project paths by raw string. The request arrives canonicalized (C:\dir) while a saved Space's stored spelling can legitimately differ (C:/dir - another save surface, or another OS in a synced workspace file), so rule 3 missed the Space: the refusal silently missed with it, and the benign re-entry minted a duplicate instead of reusing. The lookup now uses path identity (matchesProjectPath over TabPaths.pathsMatch, the same definition open tabs already trust), with a spelling-insensitivity unit test; on macOS/Linux the spellings already agreed, so nothing changes there. * test(mcp): register path-mode Space fixtures after the manager's first load The workspaceManager singleton loads its picker list asynchronously once per process, and that first load REPLACES the whole list. A test that registered a fixture right after first touching the singleton could have its registration swapped out: matchExistingSpace then saw nothing for the project path, path mode minted a fresh Space, and the test saw "opened" where a refusal or reuse was expected. Linux runners usually finished the load first; the Windows CI run (35360957645) did not, which is the flake behind that red check. Both path-mode tests that register a Space now await the singleton's first non-empty workspaces value before registering, so the registration lands after the load and can no longer be replaced under it. No assertion is weakened, and the gate logic is untouched. The registered ids are also dropped from the process-wide singleton in tearDown so later test classes in the same JVM inherit no fixture. WorkspaceManager has no unregister door - deleteWorkspaceById removes a row only when its own file manager deleted the file, which is not where a test fixture lives - so the helper reaches the backing flow directly, the same way other desktop tests reset otherwise-final private state. * test(mcp): wait for the merged first load, not a non-empty list The first-load wait `first { it.isNotEmpty() }` can be satisfied by a pre-load registration: the create_workspace tests in this class register into the workspaceManager singleton with no wait and JUnit method order is not fixed, so if one runs first the wait returns before the async disk load lands, and that load then replaces the list - dropping the fixture exactly as in the Windows flake. Wait instead until every shipped layout id is present, which only the merged post-load list satisfies. Review follow-up from the 957c15e pass.
) Adds TerminalService.CloseInput, backed by TerminalSession.closeStdin(), which delivers EOF on a session's stdin without terminating it. A stdin-consuming one-shot command (sort, grep, cat with no args, or a pipeline ending in one) otherwise blocks on read() forever, and its run_command exit chunk is never produced. Rebased onto dev's reworked input path (#939, #1527): - closeStdin() takes the same input mutex as send(), through the same bounded acquireInputLock, so a close never races a write on the same fd, and a caller behind a stalled writer is answered RESOURCE_EXHAUSTED "Terminal input is busy" with a retry-after, exactly as a queued send is. - A deliberate close is tracked apart from dev's inputClosed (set when a write fails), so a send after it reads "Terminal input has been closed", not a broken pipe. - IpcVersion.CURRENT moves to 1.4.0 (1.3.0 is now #1144's NodeSkipped), and :boss-ipc's module version follows, so verifyIpcVersionConsistency passes. The history note says the bump is not a capability signal: an older host answers CloseInput with UNIMPLEMENTED, which callers must treat as "not supported". - inputBusy() moved to file scope and requireUsableInput() folded to one throw, to stay within detekt's TooManyFunctions and ThrowsCount limits. Tests: TerminalCloseInputTest (5) covers EOF delivery with and without input, an idempotent second close, a send refused after close while the process is still alive (drain-stdin-hold fixture), and the busy answer while a 1 MiB write holds the lock, bounded so a regression fails instead of hanging. Mutation-checked: an unbounded wait in closeStdin, or dropping the closed-on-request check, each fail a test. Refs #928
…che writes (#1679) * docs(ci): record dev-push cache seeding and read-only PR caches * ci: seed shared Gradle caches from dev, read-only caches on PRs PRs stop writing PR-scoped Gradle caches (cache-read-only on pull_request); a ubuntu-only build on dev pushes seeds the shared caches PRs into dev restore. * build(test): run desktop tests in two forks * Revert maxParallelForks: forks would share test-home (per-fork homes needed first) * docs(ci): fix stale validation sentences, pushes-only cache writes * ci: only pushes write Gradle caches (dispatch runs read-only too)
|
Claude diff review of 46e4e95 (no tests executed). Review of PR #627 @
|
20 participants
Changing a tab's title or index during a drag restarted pointer input and could leave a ghost following the cursor after release. Both tab surfaces now key gestures by tab identity and component, read callbacks and indices live, and use the current dev shared drag handler for owner-scoped cancellation and release.
The legacy full-button disposal hook is removed: an idle rendering of the same tab must not cancel a gesture owned elsewhere. Sahvendra's fix and regression from #1614 are incorporated with original authorship. Earlier implementation and review-repair history is retained.
Validation on
d409ff3e6: 27 focused tests passed (tab changes, mouse cancellation/native release, and reorder behavior); common-main/desktop-test ktlint and composeApp detekt passed. Current-head GitHub checks remain the merge gate. No application was launched; hardware-browser cross-pane smoke testing remains manual.#692 is the follow-up that extracts the shared session abstraction and extends mouse-input recovery coverage. Merge this PR first.