Skip to content

Merge upstream/main into the fork: 158 commits, fork work intact - #5

Merged
ArturLauche merged 160 commits into
mainfrom
sync/upstream-2026-09-29
Sep 29, 2026
Merged

ArturLauche merged 160 commits into
mainfrom
sync/upstream-2026-09-29

Conversation

@ArturLauche

Copy link
Copy Markdown
Owner

What

Syncs the fork with pingdotgg/t3code main at d2c9281b8 — 158 upstream commits since the fork point f5ef0ddb9 — with all fork work kept intact.

The fork is now 0 behind upstream/main and 18 ahead (17 pre-existing fork commits + this merge). Nothing from the fork was rebased, squashed, or dropped:

Fork work Status
Cloud runtimes (feat/freebuff-cloud-runtimes) intact
Freebuff provider (fix/freebuff-terminal-integration) intact
Fork CI (Windows preview, Docker GHCR) intact
Cline ACP provider (feat/cline-provider-acp) intact

How

Nine files needed resolution. The pattern throughout was: take upstream's refactor, re-apply the fork's cloud-execution transport and capability gating on top.

  • apps/server/package.json — upstream's node-pty bump and @napi-rs/keyring land alongside the fork's cloud-runtime SDKs (@daytona/sdk, e2b, novita-sandbox, @xterm/headless).
  • Drivers/CodexDriver.ts / Drivers/GrokDriver.ts — upstream renamed codexResetCredit to resetCreditCoordinator and moved adapter construction after the managed snapshot so the adapter receives models. The fork's cloud transport is re-applied: childProcessSpawner / remoteCwdFor are threaded into the adapter, and textGeneration still runs on the cloud spawner when a cloud target is enabled.
  • serverSettings.ts — upstream's Bitbucket secret handling joins the fork's cloud credential rotation; both run in the same locked update transaction.
  • server.ts — upstream's managed-tunnel startup joins the fork's CloudRuntimeServiceLive.
  • ComposerAttachmentButton.tsx — upstream's Android control sizing joins the fork's attachment capability filter.
  • ProviderInstanceRegistryLive.test.ts — both the fork's cloud-target fail-closed case and upstream's reset-credit case are kept.
  • pnpm-lock.yaml — regenerated with pnpm 11.10.0, then verified with pnpm install --frozen-lockfile (no drift).

One upstream refactor needed real re-anchoring

Upstream moved queued-message sending out of ChatView into sendQueuedMessage / QueuedMessageSender (#13764), so the queuedMessage parameter the fork's provider-capability gate branched on no longer exists. The gate is now anchored on sendCtx in the composer, and the same check is asserted in sendQueuedMessage — otherwise the fork's protection would have silently stopped covering the queue-drain path, which now sends without any chat view open. A rejected send stays held at the head of the queue, so nothing is lost. Two focused tests cover both directions.

One upstream invariant the fork's new drivers had to satisfy

Upstream requires a bundled compatibility policy for every built-in harness. cline and freebuff are added to the model manifest: Cline is marked supported from 3.0.65 (the version its ACP path was verified against, per docs/internals/providers.md) and unknown below, since nothing is known to be broken. Freebuff's ranges are left empty — its versions are uncharacterized, and an empty policy renders no advisory rather than an invented one.

Verification

  • Typecheck: clean across contracts, shared, client-runtime, server, web, mobile, desktop, marketing (0 errors).

  • Format / lint: vp check and vp lint clean on every hand-resolved file (a pre-existing formatter pass reflowed one call in CodexDriver.ts).

  • Tests:

    Workspace Result
    web 413 files / 5480 tests — all pass
    client-runtime 98 / 1680 — all pass
    mobile 190 / 1730 — all pass
    shared 69 / 978 — all pass
    contracts 26 / 467 — all pass
    desktop 110 / 1413 — 1 failure, see below
    server 366 / 5628 — 8 failures, see below

server — 8 failures, none caused by this merge. Seven are chmod 0o000 permission tests (cli/theme, keybindings, server, terminal/Manager) that cannot fail as expected when the suite runs as uid 0; eight of the nine were already failing before the compatibility fix. The ninth, serviceLauncher > restores the database when a migrating trial exits, is a timing/IPC test whose entire module graph (serviceLauncher.ts and its only project import, cloud/serviceProtocol.ts) is byte-identical to this branch's base — verified with git diff --cached origin/main.

desktop — 1 failure: browser-secret-native.test.mjs needs the libsecret-1 system package, which is not installed in this container.

Known pre-existing issue (not from this sync)

knip:check reports three unused exports, all in fork-only files the merge does not touch: CLINE_PRESENTATION, clineModelsFromSetup (ClineProvider.ts) and providerShowsInteractionModeToggle (providerCapabilities.ts). All three are already unused on main — confirmed by git grep against origin/main — so this check was failing before the sync. Left alone to keep the PR to one concern; happy to clean them up separately.

No UI change, so no before/after images.

🤖 Generated with OpenCode (opencode/space-bunny-free).

juliusmarminge and others added 30 commits September 23, 2026 15:01
…case screenshots (pingdotgg#13316)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: t3-code[bot] <269035359+t3-code[bot]@users.noreply.github.com>
Co-authored-by: Exotic <118054752+extoci@users.noreply.github.com>
…otgg#11580)

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Simone <185146821+Lucenx9@users.noreply.github.com>
…3355)

Co-authored-by: Yordis Prieto <yordis.prieto@gmail.com>
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
…2613)

Co-authored-by: shivam <91240327+shivamhwp@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…otgg#13397)

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…otgg#13363)

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…gdotgg#13454)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: GPT-6 Sol <noreply@openai.com>
Co-authored-by: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-authored-by: Julius Marminge <julius0216@outlook.com>
…ops startup (pingdotgg#13469)

Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
…gdotgg#13388)

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…otgg#13386)

Co-authored-by: adeebahmad01 <52380344+adeebahmad01@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…otgg#13060)

Co-authored-by: Julius Marminge <julius0216@outlook.com>
Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Yash-Singh1 and others added 22 commits September 26, 2026 16:23
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
…#13736)

Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
…otgg#13935)

Co-authored-by: t3-code[bot] <269035359+t3-code[bot]@users.noreply.github.com>
Co-authored-by: Exotic <118054752+extoci@users.noreply.github.com>
Co-authored-by: Utkarsh Patil <73941998+UtkarshUsername@users.noreply.github.com>
…ncy improvement (pingdotgg#13884)

Co-authored-by: GPT-6 Astra <noreply@openai.com>
… account registration (pingdotgg#14127)

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Adds claude-sonnet-5-5 to the Claude catalog (reuses the sonnet-5 profile, requires Claude Code 2.1.284) and features it in currentModels. Sonnet 5 stays current.
…ingdotgg#13927)

Co-authored-by: UtkarshUsername <putkarsh184@gmail.com>
Co-authored-by: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com>
…ingdotgg#8673)

Co-authored-by: shivam <91240327+shivamhwp@users.noreply.github.com>
…tion (pingdotgg#13463)

Co-authored-by: Julius Marminge <julius0216@outlook.com>
Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
… before submission (pingdotgg#12003)

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: shivam <91240327+shivamhwp@users.noreply.github.com>
…tgg#14103)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com>
…dotgg#14007)

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Brings in 158 upstream commits since the fork point f5ef0dd, with the
fork's own work — cloud runtimes, the Freebuff provider, the Cline ACP
provider, and the fork CI workflows — kept intact.

Nine files needed resolution:

- apps/server/package.json: upstream's node-pty bump and @napi-rs/keyring
  land alongside the fork's cloud-runtime SDKs.
- CodexDriver/GrokDriver: upstream renamed the reset-credit coordinator to
  resetCreditCoordinator and moved adapter construction after the managed
  snapshot (so the adapter receives `models`). The fork's cloud execution
  transport is re-applied on top: the spawner and remote cwd resolver are
  threaded through, and Grok keeps its maintenance resolution.
- serverSettings/server: upstream's Bitbucket secret handling and managed
  tunnel startup land alongside the fork's cloud credential rotation.
- ComposerAttachmentButton: upstream's Android control sizing joins the
  fork's attachment capability filter.
- ProviderInstanceRegistryLive.test: both the fork's cloud-target
  fail-closed case and upstream's reset-credit case are kept.
- pnpm-lock.yaml regenerated with pnpm 11.10.0; verified with
  pnpm install --frozen-lockfile.

Upstream refactored queued-message sending out of ChatView
(295d7cb), so the fork's provider-capability send gate is re-anchored on
sendCtx in the composer and the same check is asserted in
sendQueuedMessage, which now owns the queue drain path.

Upstream also started requiring a bundled compatibility policy for every
built-in harness; cline and freebuff are added to the model manifest
(3.0.65 is the version the Cline ACP path was verified against; Freebuff's
ranges are left empty because its versions are uncharacterized).
@coderabbitai

coderabbitai Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Repository: ArturLauche/t3code/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 16c2a4ba-fbda-4880-a29e-536f4c8f2a8e


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XXL labels Sep 29, 2026
…orts

The upstream sync enabled `shadcn/no-arbitrary-values`, which also reads
brand strokes — not just fills. `ClineIcon` (added on `main` by the Cline
ACP work) strokes two of its rects, so it was the only file in the tree
failing the rule.

Third-party marks already carry a `no-raw-colors` exemption for exactly
this reason: a brand color is part of the logo, not a theme surface. The
stroked case is the same argument, so `Icons.tsx` and `JetBrainsIcons.tsx`
get the matching `no-arbitrary-values` exemption. It goes after the rule
is enabled, alongside the existing `AuthSurfaceShell` exemption, because a
later block wins.

Also removes three exports that nothing imports, which takes
`knip:check` from 50 unused exports on `main` to zero:

- `CLINE_PRESENTATION` and `clineModelsFromSetup` are only read inside
  `ClineProvider.ts`, so they lose the `export` keyword.
- `providerShowsInteractionModeToggle` had no callers at all and is gone.
@ArturLauche

Copy link
Copy Markdown
Owner Author

Follow-up after full verification

Ran the repo-wide gates rather than only the touched files, and found two things worth fixing here. Both are pushed as 062ad7619.

1. Lint: the sync itself turned a rule on, and it caught the fork's Cline icon

vp lint failed with 2 errors in apps/web/src/components/Icons.tsx:

stroke-[#0F0F0F] hardcodes a color and no declared theme color is close to it
dark:stroke-[#F5F5F5] hardcodes a color

This is a genuine consequence of the sync, not pre-existing noise. The merge brought in upstream's shadcn/no-arbitrary-values rule, which reads brand strokes as well as fills. ClineIcon (added on main by the Cline ACP work) strokes two <rect strokeWidth="8"> elements, so it is the only file in the tree the new rule rejects — upstream's own Icons.tsx has 3 fill-[#…] and zero stroke-[#…].

The fix follows the precedent already in vite.config.ts: third-party marks carry a no-raw-colors exemption because "a brand color is part of the logo, not a theme surface." A brand stroke is the same argument, so Icons.tsx and JetBrainsIcons.tsx get the matching no-arbitrary-values exemption, placed after the rule is enabled next to the existing AuthSurfaceShell exemption (a later block wins — my first attempt sat before it and had no effect).

vp lint is now 0 errors. The 784 warnings are pre-existing React-compiler advisories.

2. knip: 50 → 0

Worth reporting because the direction is the opposite of what you'd expect: the merge reduced unused exports from 50 to 3, since upstream now consumes most of what the fork had exported without callers. I removed the last 3, which is zero-risk:

Export Action
CLINE_PRESENTATION only read inside ClineProvider.ts → dropped export
clineModelsFromSetup only read inside ClineProvider.ts → dropped export
providerShowsInteractionModeToggle no callers anywhere → removed

knip:check now passes. (Correcting my earlier note: I had left this as "pre-existing, happy to fix separately" — it was two one-word edits, so it was not worth deferring.)

Corrected: the build-desktop-artifact failures are NOT from this sync

My first pass reported 7 failures there and I attributed them to the environment. Checking against a clean upstream/main worktree shows the failure set is byte-identical on both — all 7 fail on upstream too:

diff <(FAIL on upstream) <(FAIL on merge) → IDENTICAL — no regression

Cause: a /tmp/node_modules directory (created 2026-09-28, before this task) makes the self-containment probe refuse to run — "Refusing to report success: /tmp/node_modules is visible from the probe directory." That is a container artifact, not a code problem. I worked around it by pointing TMPDIR at a clean directory rather than touching anything under /tmp.

update-release-package-versions and serviceLauncher > restores the database when a migrating trial exits also fail identically on upstream/main.

Gates after both commits

Gate Result
vp run -r typecheck (15 workspaces) 0 errors
vp lint 0 errors
vp check (format) 4150 files formatted, 0 errors
knip:check pass (was 50 unused exports on main)
server Cline + Freebuff + compatibility + registry tests 85 passed
shared providerCapabilities 21 passed
web QueuedMessageSender 8 passed

pnpm install --frozen-lockfile verifies the regenerated lockfile with no drift.

🤖 Generated with OpenCode (opencode/space-bunny-free).

@ArturLauche
ArturLauche merged commit cbdd167 into main Sep 29, 2026
10 of 19 checks passed
Comment thread apps/server/src/server.ts
if (!hasCloudPublicConfig) {
yield* Deferred.succeed(cloudLinkParked, undefined).pipe(Effect.orDie);
return;
}
const releaseManagedTunnel = releaseManagedTunnelOnShutdown().pipe(

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

CRITICAL: The merge resolution dropped the fork's hasCloudPublicConfig early-exit guard, so the managed-tunnel reconcile fiber now runs on every host.

Parent 1 (95df343d) opened this layer with:

if (!hasCloudPublicConfig) {
  yield* Deferred.succeed(cloudLinkParked, undefined).pipe(Effect.orDie);
  return;
}

The resolution replaced that block with upstream's version verbatim. After the merge hasCloudPublicConfig is only consulted, at line 823, to decide wantsCliLink — everything else in the body executes unconditionally, and cloudLinkParked is only succeeded at the very end (line 950), so the guard can no longer be reinstated by the surviving code.

Failure scenario on a self-hosted or air-gapped install (no T3CODE_RELAY_URL, no Clerk keys — exactly the case that guard existed for): the fiber registers the shutdown finalizer, forks the recovery consumer over endpointRuntime.recoveryRequests, and then reaches the registration path. startManagedCloudTunnelIfOriginConfirmed fails and is caught to false, so startedConfirmed is false and line 878 hands startStoredManagedTunnel to retryManagedTunnelRegistration as onRetryWindowExhausted. In apps/server/src/cloud/managedTunnelStartup.ts that fallback re-enters an unbounded Effect.retry (capped at 30s jittered backoff) that runs for the process lifetime. Independently, startStoredManagedTunnel (line 856, requireConfirmedOrigin: false) can start a stored managed tunnel on a host that is explicitly configured without cloud.

This code is byte-identical to upstream, so the underlying behaviour may be an accepted upstream trade-off — but removing the fork's opt-out is a behaviour change this merge makes, and it is easy to miss because the file is dominated by upstream's refactor. Restoring an early return before line 742 reinstates the protection without touching upstream's happy path.


Reply with @kilocode-bot fix it to have Kilo Code address this issue.

// at the head of the queue and the user edits the draft in the composer.
const assertProviderInputAllowed = () => {
const provider =
readConfig()?.providers.find(

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

WARNING: This gate fails open when the provider snapshot cannot be resolved — and it is the only assertion in this function that does.

readConfig()?.providers.find(...) ?? null collapses two very different states into null: (a) readConfig() is undefined because the environment's serverConfig has not loaded yet, and (b) the instance genuinely is absent from the list. getUnsupportedProviderModeReason early-returns null for a null provider (providerCapabilities.ts:126) and providerSupportsImageAttachments treats absent as supported (:143), so both states allow the send.

The sibling assertFilesAllowed deliberately threads attachmentUploadsCapabilityKnown: config !== undefined (line 69) and blocks with "Waiting for the server…" when the config is unknown. The two disagree on what "capability unknown" means, and the new one is the lenient one.

Failure scenario: serverConfig is momentarily null (a reconnect) while the thread timeline is still rendered. The queued row's "Send now" button is enabled unconditionally, and QueuedMessageSender guards on a loaded config but the steer path does not. A queued message carrying an image then bypasses the gate, useUploads (line 122) falls to false, and the image is inlined as a data URL and shipped to a provider that declares supportsImageAttachments: false — the exact silent content drop this gate exists to prevent. The same hole covers a stale instanceId (instance renamed or removed in Settings).

Mirroring the assertFilesAllowed shape — resolving the provider as undefined while the config is still unknown and rejecting — would close it.


Reply with @kilocode-bot fix it to have Kilo Code address this issue.

};
// The composer's send gate cannot cover a queue that drains on its own, so
// the same provider-capability rules are asserted here. A rejected send stays
// at the head of the queue and the user edits the draft in the composer.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

SUGGESTION: The documented recovery does not work, and the matching comment in ChatView.tsx oversells what this gate is.

"the user edits the draft in the composer" implies that switching the composer to a provider that accepts the image and retrying resolves the block. It does not: this gate resolves the provider from sendSettings.modelSelection.instanceId — frozen on the queued message — so "Send now" re-runs the identical check against the identical instance and rejects again. The only working route is "Cancel and return to the composer", which the toast (line 224) never mentions. Worth restating the recovery accurately in the comment.

Relatedly, ChatView.tsx:7442-7444 says queued messages "assert the same capability gate before it dispatches". They do not assert the same gate: ChatView resolves the provider via activeProviderStatus (the live composer selection) and adds directAnnotation?.image to the count, while this function resolves by the message's own instanceId and counts only the queue's attachments. Related but not a defect: the ChatView path is now composer-only, since no queued message reaches its onSend.


Reply with @kilocode-bot fix it to have Kilo Code address this issue.

attachmentCount: attachments.length,
fileCount: message.files.length,
});
if (reason !== null) throw new Error(reason.reason);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

SUGGESTION: throw new Error(reason.reason) discards reason.kind, so a capability restriction surfaces as a red failure toast instead of the actionable warning the composer shows.

The composer path renders the same condition through getUnsupportedProviderInputBannerCopy as a warning titled "Image attachments unavailable" / "Provider mode unavailable". Here the throw collapses into the generic catch at line 221, producing type: "error", title "Queued message not sent in <thread>" — a failure tone for a pre-flight block the user can resolve, with a message that names no recovery route. Carrying kind through (or branching on it in the catch) would let this path match the composer's treatment.


Reply with @kilocode-bot fix it to have Kilo Code address this issue.

});

it("sends attachments when the selected provider accepts them", async () => {
config.providers = [

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

SUGGESTION: Both directions are asserted, but the accept case does not actually guard the new assertion, and three pre-existing tests silently depend on the fail-open behaviour flagged at sendQueuedMessage.ts:81.

The reject test (line 257) is real — it pins commandsRun() === [] and the held head-of-queue entry. The accept test (line 290) only pins that a start command runs, so it would still pass if assertProviderInputAllowed were deleted outright; as written it catches over-blocking, not an unwired assertion. A test that omits the provider from config.providers but still expects a send (or one that mutates the entry to supportsImageAttachments: false after enqueue) would pin the wiring in the other direction.

The second gap is the coupling: the shared beforeEach sets config.providers = [] (line 93), and the pre-existing tests at lines 132, 149 and 196 all run under it. Each of those exercises the provider: null path, which is precisely the fail-open branch. Closing the sendQueuedMessage.ts:81 hole will change their behaviour, and nothing in this file currently signals that — they will keep passing or start failing for reasons unrelated to what they assert.


Reply with @kilocode-bot fix it to have Kilo Code address this issue.

{
"driver": "freebuff",
"t3CodeRange": ">=0.0.42",
"ranges": []

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

SUGGESTION: "ranges": [] does not render no advisory — it renders an unknown one, which makes the upstream invariant test pass vacuously for Freebuff.

resolveProviderCompatibility returns a truthy object for every version when the policy exists but no range matches: policy.ranges.find(...)?.status ?? "unknown" (providerCompatibility.ts:80), with message: null and both recommendation fields null. applyProviderCompatibility then attaches that object to every enabled+installed Freebuff snapshot (:118-126) — whereas a driver with no policy at all would get no compatibilityAdvisory key. So the manifest entry is not the neutral "render nothing" case the rationale assumes; it is a defined advisory with status: "unknown".

There is no user-visible symptom today, because every consumer gates on .message or explicitly excludes "unknown". The cost is the invariant: the test asserting every built-in harness has a bundled policy (providerCompatibility.test.ts) uses assert.isDefined, which a ranges: [] entry satisfies. A maintainer sees Freebuff covered, ships a Freebuff release with a breaking protocol change, no advisory is ever raised, and CI stays green — plus any future if (provider.compatibilityAdvisory) check would now fire on a truthy-but-empty advisory.

Real ranges, or an explicit marker that is distinguishable from a satisfied invariant, would make the entry mean what it claims.


Reply with @kilocode-bot fix it to have Kilo Code address this issue.

Comment thread vite.config.ts
// Same reasoning for the third-party marks that are stroked rather than
// filled: a brand stroke is part of the logo, so it keeps its exact color.
files: ["apps/web/src/components/Icons.tsx", "apps/web/src/components/JetBrainsIcons.tsx"],
rules: { "shadcn/no-arbitrary-values": "off" },

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

SUGGESTION: This turns off the whole no-arbitrary-values rule for two entire files, which is broader than the stated reason.

The no-raw-colors exemption above it (line 207) is narrow by construction — that rule only reads colors, so scoping it to the mark files costs nothing else. shadcn/no-arbitrary-values also governs spacing, sizing, radii and typography, and its own block already carries a long explicit allow list, so it is a rule with real coverage to protect. The motivating case is two stroke-[#0F0F0F] / dark:stroke-[#F5F5F5] values in ClineIcon; from here on, any arbitrary [12px] or [7px] added to Icons.tsx ships with a green lint.

Icons.tsx is a large shared file that new icons land in constantly, so this is the place where the loss of coverage compounds. Adding the two color values to the rule's allow list (or an equivalent targeted allowance) would keep the rest of the rule live for these files.


Reply with @kilocode-bot fix it to have Kilo Code address this issue.

searchableEnvironmentId,
selectedEnvironment,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

SUGGESTION: selectedEnvironment is an unstable dependency — it is an object from options.find(...) (line 304), so the effect re-runs on every rebuild of options even when nothing it branches on has changed.

The neighbouring dep selectedEnvironmentCanRenderSettings (line 351) is a boolean derived from the same object precisely so it is referentially stable. The platform.os === "darwin" half of the guard at line 331 is also a boolean, so the whole predicate could be lifted into a selectedEnvironmentIsMac value alongside line 306 and used as the dep.

It converges rather than looping — the branch sets the id and the next render satisfies the darwin half and stops — so this is effect churn on a panel that already re-derives three search ids, not a defect. Worth noting while the dependency list is open.


Reply with @kilocode-bot fix it to have Kilo Code address this issue.

@kilo-code-bot

kilo-code-bot Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Code Review Summary

Status: 8 Issues Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 1
WARNING 2
SUGGESTION 5

Scope

This PR is a 158-commit upstream merge: 841 files, +76,762 / -27,740. The overwhelming majority of that is verbatim upstream, so review scope was set to the authored content — the 44 files the merge resolution changed relative to both parents, plus the 3 files touched by the follow-up commit 062ad7619. Upstream code the resolution did not touch was deliberately excluded.

What checked out clean

  • Capability-gate re-anchoring (the riskiest resolution). No queued message reaches ChatView's onSend any more — the only queue interaction there is enqueue; "Send now" routes to sendQueuedMessage and the row's X routes to restoreQueuedMessagesToComposer. The queuedMessage parameter is fully gone, so re-reading sendCtx is equivalent. The new gate's throw path is correct: it runs before any metadata command, and queue.failSend returns the entry to the head with holdUntilUserAction.
  • Renames. codexResetCredit → resetCreditCoordinator and codexResetCredit.ts → resetCreditCoordinator.ts are complete — zero dangling references, and ResetCreditCoordinator.layerTest is a real implementation, not a stub.
  • Cloud-transport unions. Every provideService(ChildProcessSpawner…) site across the four cloud drivers is a clean 1:1 union of both parents; the only additions are upstream's resolveMaintenance sites and the fork's textGeneration sites. With executionTarget unset the local path is behaviourally identical to pre-merge.
  • models threading in CodexDriver. The const models = … binding and the adapter-construction-after-snapshot reordering match upstream exactly; no read-before-define.
  • Test survival. All upstream it.live cases in ProviderInstanceRegistryLive.test.ts survive byte-identical alongside the three fork cases, with no duplicate names. ProviderRegistry.test.ts is upstream plus two sorted-list entries — and it incidentally fixes a duplicated Layer.provideMerge(CodexResetCredit.layerTest) on the fork side.
  • Contracts / state. Zero field deletions in contracts/settings.ts (every addition is withDecodingDefault, so older clients keep decoding); mergeProviderInstancePatches preserves an existing executionTarget when a patch omits it, so older clients cannot strip cloud routing. client-runtime/state/server.ts is a pure union. acp-mock-agent.ts is a pure union and AcpJsonRpcConnection.test.ts still matches its emission order.
  • Manifest integrity. All 8 driverKinds from builtInDrivers.ts have entries, no duplicate slugs, all profile refs resolve (otherwise hasValidProviderCatalogReferences throws at import).
  • Mobile. ComposerAttachmentButton.tsx keeps the fork's capability filter alongside upstream's Android sizing; useAndroidControlSizing returns scale === 1 on iOS and is called before any early return, so no iOS no-op and no hook-order hazard.
  • Dead-export removal in 062ad7619. Verified that CLINE_PRESENTATION and clineModelsFromSetup are read only inside ClineProvider.ts and that providerShowsInteractionModeToggle had no callers — the knip:check claim holds.

Issues

Issue Details (click to expand)

CRITICAL

File Line Issue
apps/server/src/server.ts 742 The resolution dropped the fork's hasCloudPublicConfig early-exit guard. hasCloudPublicConfig is now only consulted at line 823 to pick wantsCliLink; the rest of the reconcile fiber runs unconditionally, and cloudLinkParked is only succeeded at line 950, so the guard can no longer be reinstated by surviving code. On a self-hosted/air-gapped install this reaches retryManagedTunnelRegistration with startStoredManagedTunnel as its unbounded-retry fallback, for the process lifetime, and can start a stored managed tunnel on a host explicitly configured without cloud. The code is upstream's verbatim — the behaviour change is this merge removing the fork's opt-out.

WARNING

File Line Issue
apps/web/src/components/chat/sendQueuedMessage.ts 81 The new gate fails open when the provider snapshot cannot be resolved. ?? null collapses "config not loaded yet" and "instance genuinely absent" into null, and getUnsupportedProviderModeReason / providerSupportsImageAttachments both treat null as permissive. Its sibling assertFilesAllowed (line 69) deliberately does the opposite and blocks on unknown capability. During a reconnect the queued row's "Send now" is enabled unconditionally, so an image can be inlined as a data URL and shipped to a provider declaring supportsImageAttachments: false — the exact silent content drop the gate exists to prevent.
apps/web/src/components/chat/sendQueuedMessage.ts 78 The documented recovery does not work. "the user edits the draft in the composer" implies switching providers and retrying fixes it; it does not, because the gate resolves the provider from the queued message's frozen sendSettings.modelSelection.instanceId, so "Send now" rejects identically. The only working route is "Cancel and return to the composer", which the toast never mentions. ChatView.tsx:7442-7444 also overstates this as "the same capability gate" — ChatView resolves via activeProviderStatus and counts directAnnotation.image; this resolves by instanceId and counts only queue attachments.

SUGGESTION

File Line Issue
apps/web/src/components/chat/sendQueuedMessage.ts 91 throw new Error(reason.reason) discards reason.kind, collapsing a capability restriction into the generic catch as a red "Queued message not sent" failure toast, instead of the actionable warning the composer renders via getUnsupportedProviderInputBannerCopy.
apps/web/src/components/QueuedMessageSender.test.tsx 268 The reject test is real; the accept test only pins that a start command runs, so it would still pass if assertProviderInputAllowed were deleted outright. Separately, beforeEach sets config.providers = [] (line 93) and the pre-existing tests at 132 / 149 / 196 all depend on the null-provider fail-open — closing the WARNING above changes their behaviour with nothing in the file signalling it.
apps/server/src/provider/model-manifest.json 72 "ranges": [] does not render no advisory — resolveProviderCompatibility falls through to status: "unknown" with message: null and returns a truthy object, which applyProviderCompatibility attaches to every enabled+installed Freebuff snapshot. The "every built-in harness has a policy" invariant uses assert.isDefined, so a ranges: [] entry satisfies it vacuously: a maintainer sees Freebuff covered, ships a breaking release, no advisory fires, CI stays green. No user-visible symptom today — every consumer gates on .message or explicitly excludes "unknown".
vite.config.ts 289 Disables the entire shadcn/no-arbitrary-values rule for two whole files, which is broader than the stated reason (two stroke-[#…] values in ClineIcon). The no-raw-colors exemption at line 207 is narrow by construction; this rule also governs spacing, sizing, radii and typography and already carries an explicit allow list. Icons.tsx is a large file new icons land in constantly, so future arbitrary values there will ship green.
apps/web/src/components/settings/ProviderSettingsPanel.tsx 350 selectedEnvironment is an object from options.find(...) (line 304), so the effect re-runs on every rebuild of options even when nothing it branches on changed. The neighbouring boolean dep at line 351 exists to be stable; the platform.os === "darwin" half could be lifted to a boolean alongside line 306. Converges rather than looping, so this is churn, not a defect.
Files Reviewed (47 files)

Merge resolutions — server drivers (5)

  • apps/server/src/provider/Drivers/AntigravityDriver.ts - 0 issues
  • apps/server/src/provider/Drivers/ClaudeDriver.ts - 0 issues
  • apps/server/src/provider/Drivers/CodexDriver.ts - 0 issues
  • apps/server/src/provider/Drivers/CursorDriver.ts - 0 issues
  • apps/server/src/provider/Drivers/GrokDriver.ts - 0 issues

Merge resolutions — adapters and registry (7)

  • apps/server/src/provider/Layers/ClaudeAdapter.ts - 0 issues
  • apps/server/src/provider/Layers/CodexAdapter.ts - 0 issues
  • apps/server/src/provider/Layers/CodexSessionRuntime.ts - 0 issues
  • apps/server/src/provider/Layers/GrokAdapter.ts - 0 issues
  • apps/server/src/provider/Layers/ProviderInstanceRegistryLive.test.ts - 0 issues
  • apps/server/src/provider/Layers/ProviderRegistry.test.ts - 0 issues
  • apps/server/src/provider/Layers/ProviderService.ts - 0 issues

Merge resolutions — server core (9)

  • apps/server/src/server.ts - 1 issue
  • apps/server/src/serverSettings.ts - 0 issues
  • apps/server/src/serverSettings.test.ts - 0 issues
  • apps/server/src/terminal/Manager.ts - 0 issues
  • apps/server/src/terminal/Manager.test.ts - 0 issues
  • apps/server/src/ws.ts - 0 issues
  • apps/server/package.json - 0 issues
  • pnpm-workspace.yaml - 0 issues
  • packages/shared/package.json - 0 issues

Merge resolutions — web (8)

  • apps/web/src/components/ChatView.tsx - 0 issues
  • apps/web/src/components/chat/ChatComposer.tsx - 0 issues
  • apps/web/src/components/chat/sendQueuedMessage.ts - 3 issues
  • apps/web/src/components/QueuedMessageSender.test.tsx - 1 issue
  • apps/web/src/components/Icons.tsx - 0 issues
  • apps/web/src/components/settings/ProviderInstanceCard.tsx - 0 issues
  • apps/web/src/components/settings/ProviderSettingsPanel.tsx - 1 issue
  • apps/web/src/components/settings/settingsSearch.ts - 0 issues

Merge resolutions — mobile, contracts, ACP, provider policy (14)

  • apps/mobile/src/Stack.tsx - 0 issues
  • apps/mobile/src/components/ComposerAttachmentButton.tsx - 0 issues
  • apps/mobile/src/components/ProviderIcon.tsx - 0 issues
  • apps/mobile/src/features/threads/NewTaskDraftScreen.tsx - 0 issues
  • apps/mobile/src/features/threads/ThreadComposer.tsx - 0 issues
  • packages/client-runtime/src/state/server.ts - 0 issues
  • packages/contracts/src/settings.ts - 0 issues
  • packages/contracts/src/settings.test.ts - 0 issues
  • apps/server/src/provider/acp/AcpSessionRuntime.ts - 0 issues
  • apps/server/src/provider/acp/AcpJsonRpcConnection.test.ts - 0 issues
  • apps/server/src/provider/RuntimeInstructions.ts - 0 issues
  • apps/server/src/provider/RuntimeInstructions.test.ts - 0 issues
  • apps/server/src/provider/model-manifest.json - 1 issue
  • apps/server/scripts/acp-mock-agent.ts - 0 issues

Follow-up commit 062ad7619 (3)

  • apps/server/src/provider/Layers/ClineProvider.ts - 0 issues
  • packages/shared/src/providerCapabilities.ts - 0 issues
  • vite.config.ts - 1 issue

Not in inline-comment scope (checked)

  • README.md - the provider enumeration at lines 5 and 16 omits Freebuff, but neither line is touched by this PR (its only change is the .deb install section at line 58), so there is no valid diff line to anchor to. Flagged here for awareness.
  • docs/user/install.md - same: the table change lands at lines 61-71, not the provider list further down.

Fix these issues in Kilo Cloud


Reviewed by space-bunny-alpha · Input: 0 · Output: 0 · Cached: 0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XXL vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.