diff --git a/.github/workflows/desktop-release.yml b/.github/workflows/desktop-release.yml new file mode 100644 index 00000000000..9ad25e6f124 --- /dev/null +++ b/.github/workflows/desktop-release.yml @@ -0,0 +1,377 @@ +name: Desktop Release + +on: + push: + tags: + - "v*.*.*" + - "!v*.*.*-*" + workflow_dispatch: + inputs: + version: + description: "Release version, for example 0.0.26 or v0.0.26" + required: true + type: string + +permissions: + contents: read + +jobs: + preflight: + name: Preflight + runs-on: ubuntu-24.04 + timeout-minutes: 15 + outputs: + version: ${{ steps.release_meta.outputs.version }} + tag: ${{ steps.release_meta.outputs.tag }} + release_name: ${{ steps.release_meta.outputs.release_name }} + ref: ${{ steps.release_meta.outputs.ref }} + steps: + - name: Checkout + uses: actions/checkout@v6 + with: + fetch-depth: 0 + + - name: Setup Bun + uses: oven-sh/setup-bun@v2 + with: + bun-version-file: package.json + + - name: Setup Node + uses: actions/setup-node@v6 + with: + node-version-file: package.json + + - name: Install dependencies + run: bun install --frozen-lockfile + + - id: release_meta + name: Resolve release version + shell: bash + env: + INPUT_VERSION: ${{ inputs.version }} + run: | + set -euo pipefail + + if [[ "${GITHUB_EVENT_NAME}" == "workflow_dispatch" ]]; then + raw="${INPUT_VERSION}" + else + raw="${GITHUB_REF_NAME}" + fi + + version="${raw#v}" + if [[ ! "$version" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]]; then + echo "Invalid stable release version: $raw" >&2 + echo "Stable desktop releases must use x.y.z or vx.y.z, for example 0.0.26 or v0.0.26." >&2 + exit 1 + fi + + tag="v$version" + release_ref="$(git rev-list -n 1 "$tag" 2>/dev/null || true)" + if [[ -z "$release_ref" ]]; then + echo "Release tag $tag does not exist. Create the tag before running the desktop release workflow." >&2 + exit 1 + fi + + echo "version=$version" >> "$GITHUB_OUTPUT" + echo "tag=$tag" >> "$GITHUB_OUTPUT" + echo "release_name=T3 Code v$version" >> "$GITHUB_OUTPUT" + echo "ref=$release_ref" >> "$GITHUB_OUTPUT" + + - name: Format check + run: bun run fmt:check + + - name: Lint + run: bun run lint + + - name: Typecheck + run: bun run typecheck + + - name: Test + run: bun run test + + build: + name: Build ${{ matrix.label }} + needs: preflight + runs-on: ${{ matrix.runner }} + timeout-minutes: 35 + strategy: + fail-fast: false + matrix: + include: + - label: macOS arm64 + runner: macos-latest + platform: mac + target: dmg + arch: arm64 + - label: macOS x64 + runner: macos-latest + platform: mac + target: dmg + arch: x64 + - label: Linux x64 + runner: ubuntu-24.04 + platform: linux + target: AppImage + arch: x64 + - label: Windows x64 + runner: windows-latest + platform: win + target: nsis + arch: x64 + steps: + - name: Checkout + uses: actions/checkout@v6 + with: + ref: ${{ needs.preflight.outputs.ref }} + fetch-depth: 0 + + - name: Setup Bun + uses: oven-sh/setup-bun@v2 + with: + bun-version-file: package.json + + - name: Setup Node + uses: actions/setup-node@v6 + with: + node-version-file: package.json + + - name: Install dependencies + run: bun install --frozen-lockfile + + - name: Align package versions to release version + run: node scripts/update-release-package-versions.ts "${{ needs.preflight.outputs.version }}" + + - name: Install Spectre-mitigated MSVC libs + if: matrix.platform == 'win' + shell: pwsh + run: | + $ErrorActionPreference = "Stop" + $vswhere = "${env:ProgramFiles(x86)}\Microsoft Visual Studio\Installer\vswhere.exe" + $installPath = & $vswhere -products * -latest -property installationPath + $setupExe = "${env:ProgramFiles(x86)}\Microsoft Visual Studio\Installer\setup.exe" + $proc = Start-Process -FilePath $setupExe ` + -ArgumentList "modify", "--installPath", "`"$installPath`"", "--add", ` + "Microsoft.VisualStudio.Component.VC.Tools.x86.x64.Spectre", "--quiet", "--norestart" ` + -Wait -PassThru -NoNewWindow + if ($null -eq $proc -or $proc.ExitCode -ne 0) { + $code = if ($null -ne $proc) { $proc.ExitCode } else { 1 } + Write-Error "Visual Studio Installer failed with exit code $code" + exit $code + } + + - name: Prepare Azure Trusted Signing + if: matrix.platform == 'win' + shell: pwsh + env: + AZURE_TENANT_ID: ${{ secrets.AZURE_TENANT_ID }} + AZURE_CLIENT_ID: ${{ secrets.AZURE_CLIENT_ID }} + AZURE_CLIENT_SECRET: ${{ secrets.AZURE_CLIENT_SECRET }} + AZURE_TRUSTED_SIGNING_ENDPOINT: ${{ secrets.AZURE_TRUSTED_SIGNING_ENDPOINT }} + AZURE_TRUSTED_SIGNING_ACCOUNT_NAME: ${{ secrets.AZURE_TRUSTED_SIGNING_ACCOUNT_NAME }} + AZURE_TRUSTED_SIGNING_CERTIFICATE_PROFILE_NAME: ${{ secrets.AZURE_TRUSTED_SIGNING_CERTIFICATE_PROFILE_NAME }} + AZURE_TRUSTED_SIGNING_PUBLISHER_NAME: ${{ secrets.AZURE_TRUSTED_SIGNING_PUBLISHER_NAME }} + run: | + $ErrorActionPreference = "Stop" + + $requiredSecrets = @( + $env:AZURE_TENANT_ID, + $env:AZURE_CLIENT_ID, + $env:AZURE_CLIENT_SECRET, + $env:AZURE_TRUSTED_SIGNING_ENDPOINT, + $env:AZURE_TRUSTED_SIGNING_ACCOUNT_NAME, + $env:AZURE_TRUSTED_SIGNING_CERTIFICATE_PROFILE_NAME, + $env:AZURE_TRUSTED_SIGNING_PUBLISHER_NAME + ) + if ($requiredSecrets | Where-Object { [string]::IsNullOrWhiteSpace($_) }) { + Write-Host "Azure Trusted Signing disabled; skipping TrustedSigning module preparation." + exit 0 + } + + try { + Install-PackageProvider ` + -Name NuGet ` + -MinimumVersion 2.8.5.201 ` + -Force ` + -Scope CurrentUser ` + -ErrorAction Stop + } catch { + Write-Warning "Could not bootstrap NuGet package provider. Continuing because the runner may already have a usable provider. $($_.Exception.Message)" + } + + Install-Module ` + -Name TrustedSigning ` + -MinimumVersion 0.5.0 ` + -Force ` + -AllowClobber ` + -Repository PSGallery ` + -Scope CurrentUser ` + -ErrorAction Stop + + Import-Module TrustedSigning -MinimumVersion 0.5.0 -Force + Get-Command Invoke-TrustedSigning -ErrorAction Stop + + - name: Build desktop artifact + shell: bash + env: + T3CODE_DESKTOP_UPDATE_REPOSITORY: ${{ github.repository }} + CSC_LINK: ${{ secrets.CSC_LINK }} + CSC_KEY_PASSWORD: ${{ secrets.CSC_KEY_PASSWORD }} + APPLE_API_KEY: ${{ secrets.APPLE_API_KEY }} + APPLE_API_KEY_ID: ${{ secrets.APPLE_API_KEY_ID }} + APPLE_API_ISSUER: ${{ secrets.APPLE_API_ISSUER }} + AZURE_TENANT_ID: ${{ secrets.AZURE_TENANT_ID }} + AZURE_CLIENT_ID: ${{ secrets.AZURE_CLIENT_ID }} + AZURE_CLIENT_SECRET: ${{ secrets.AZURE_CLIENT_SECRET }} + AZURE_TRUSTED_SIGNING_ENDPOINT: ${{ secrets.AZURE_TRUSTED_SIGNING_ENDPOINT }} + AZURE_TRUSTED_SIGNING_ACCOUNT_NAME: ${{ secrets.AZURE_TRUSTED_SIGNING_ACCOUNT_NAME }} + AZURE_TRUSTED_SIGNING_CERTIFICATE_PROFILE_NAME: ${{ secrets.AZURE_TRUSTED_SIGNING_CERTIFICATE_PROFILE_NAME }} + AZURE_TRUSTED_SIGNING_PUBLISHER_NAME: ${{ secrets.AZURE_TRUSTED_SIGNING_PUBLISHER_NAME }} + run: | + set -euo pipefail + + args=( + --platform "${{ matrix.platform }}" + --target "${{ matrix.target }}" + --arch "${{ matrix.arch }}" + --build-version "${{ needs.preflight.outputs.version }}" + --verbose + ) + + has_all() { + for value in "$@"; do + if [[ -z "$value" ]]; then + return 1 + fi + done + return 0 + } + + if [[ "${{ matrix.platform }}" == "mac" ]]; then + if has_all "$CSC_LINK" "$CSC_KEY_PASSWORD" "$APPLE_API_KEY" "$APPLE_API_KEY_ID" "$APPLE_API_ISSUER"; then + key_path="$RUNNER_TEMP/AuthKey_${APPLE_API_KEY_ID}.p8" + printf '%s' "$APPLE_API_KEY" > "$key_path" + export APPLE_API_KEY="$key_path" + echo "macOS signing enabled." + args+=(--signed) + else + echo "macOS signing secrets are required for stable desktop releases." >&2 + echo "Missing one or more of CSC_LINK, CSC_KEY_PASSWORD, APPLE_API_KEY, APPLE_API_KEY_ID, APPLE_API_ISSUER." >&2 + echo "Unsigned macOS builds can download updates, but Squirrel.Mac rejects them during install." >&2 + exit 1 + fi + elif [[ "${{ matrix.platform }}" == "win" ]]; then + if has_all \ + "$AZURE_TENANT_ID" \ + "$AZURE_CLIENT_ID" \ + "$AZURE_CLIENT_SECRET" \ + "$AZURE_TRUSTED_SIGNING_ENDPOINT" \ + "$AZURE_TRUSTED_SIGNING_ACCOUNT_NAME" \ + "$AZURE_TRUSTED_SIGNING_CERTIFICATE_PROFILE_NAME" \ + "$AZURE_TRUSTED_SIGNING_PUBLISHER_NAME"; then + echo "Windows signing enabled (Azure Trusted Signing)." + args+=(--signed) + else + echo "Windows signing disabled (missing one or more Azure Trusted Signing secrets)." + fi + else + echo "Signing disabled for ${{ matrix.platform }}." + fi + + bun run dist:desktop:artifact -- "${args[@]}" + + - name: Collect release assets + shell: bash + run: | + set -euo pipefail + mkdir -p release-publish + + shopt -s nullglob + for pattern in \ + "release/*.dmg" \ + "release/*.zip" \ + "release/*.AppImage" \ + "release/*.exe" \ + "release/*.blockmap" \ + "release/*.yml"; do + for file in $pattern; do + cp "$file" release-publish/ + done + done + + if [[ "${{ matrix.platform }}" == "mac" && "${{ matrix.arch }}" != "arm64" ]]; then + shopt -s nullglob + for manifest in release-publish/*-mac.yml; do + mv "$manifest" "${manifest%.yml}-${{ matrix.arch }}.yml" + done + fi + + - name: Upload build artifacts + uses: actions/upload-artifact@v7 + with: + name: desktop-${{ matrix.platform }}-${{ matrix.arch }} + path: release-publish/* + if-no-files-found: error + + release: + name: Publish GitHub Release + needs: [preflight, build] + runs-on: ubuntu-24.04 + timeout-minutes: 10 + permissions: + contents: write + steps: + - name: Checkout + uses: actions/checkout@v6 + with: + ref: ${{ needs.preflight.outputs.ref }} + + - name: Setup Bun + uses: oven-sh/setup-bun@v2 + with: + bun-version-file: package.json + + - name: Setup Node + uses: actions/setup-node@v6 + with: + node-version-file: package.json + + - name: Install release tooling dependencies + run: bun install --frozen-lockfile --filter=@t3tools/scripts + + - name: Download all desktop artifacts + uses: actions/download-artifact@v8 + with: + pattern: desktop-* + merge-multiple: true + path: release-assets + + - name: Merge macOS updater manifests + run: | + set -euo pipefail + shopt -s nullglob + for x64_manifest in release-assets/*-mac-x64.yml; do + arm64_manifest="${x64_manifest%-x64.yml}.yml" + if [[ -f "$arm64_manifest" ]]; then + node scripts/merge-update-manifests.ts --platform mac "$arm64_manifest" "$x64_manifest" + rm -f "$x64_manifest" + fi + done + + - name: Publish release + uses: softprops/action-gh-release@v2 + with: + tag_name: ${{ needs.preflight.outputs.tag }} + target_commitish: ${{ needs.preflight.outputs.ref }} + name: ${{ needs.preflight.outputs.release_name }} + generate_release_notes: true + prerelease: false + make_latest: true + files: | + release-assets/*.dmg + release-assets/*.zip + release-assets/*.AppImage + release-assets/*.exe + release-assets/*.blockmap + release-assets/*.yml + fail_on_unmatched_files: true + token: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index d90479087cf..2228133b7d5 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -3,8 +3,8 @@ name: Release on: push: tags: - - "v*.*.*" - - "!v*-nightly.*" + # Stable desktop tags are published by desktop-release.yml in this fork. + - "v*-nightly.*" schedule: - cron: "0 */3 * * *" workflow_dispatch: @@ -347,7 +347,10 @@ jobs: echo "macOS signing enabled." args+=(--signed) else - echo "macOS signing disabled (missing one or more Apple signing secrets)." + echo "macOS signing secrets are required for desktop releases." >&2 + echo "Missing one or more of CSC_LINK, CSC_KEY_PASSWORD, APPLE_API_KEY, APPLE_API_KEY_ID, APPLE_API_ISSUER." >&2 + echo "Unsigned macOS builds can download updates, but Squirrel.Mac rejects them during install." >&2 + exit 1 fi elif [[ "${{ matrix.platform }}" == "win" ]]; then if has_all \ diff --git a/.reviews/chat-composer-steering-review.md b/.reviews/chat-composer-steering-review.md new file mode 100644 index 00000000000..3c1b47ad7f2 --- /dev/null +++ b/.reviews/chat-composer-steering-review.md @@ -0,0 +1,88 @@ +# Review: Chat Composer Steering + +## Project context + +| Field | Value | +| -------------- | ---------------------------------------- | +| **Repository** | `declancowen/t3code` | +| **Remote** | `origin` | +| **Branch** | `main` | +| **Stack** | TypeScript, React/Vite, Zustand, Lexical | + +## Scope + +- `apps/web/src/components/chat/ChatComposer.tsx` — composer imperative handle and prompt snapshot synchronization. +- `apps/web/src/components/ChatView.tsx` — send, plan follow-up, and plan implementation caller behavior. + +## Hotspots + +- Lexical editor state vs persisted composer draft/ref state. +- Running-turn same-thread steering through the existing `thread.turn.start` dispatch path. +- Plan follow-up and implementation flows that read model state after draft text has been cleared. +- Failure rollback that restores prompt, attachments, and terminal contexts after send failure. + +## Review status + +| Field | Value | +| --------------------- | -------------------- | +| **Review started** | 2026-05-21 11:10 BST | +| **Last reviewed** | 2026-05-21 11:10 BST | +| **Total turns** | 1 | +| **Open findings** | 0 | +| **Resolved findings** | 0 | +| **Accepted findings** | 0 | + +## Turn 1 — 2026-05-21 11:10 BST + +| Field | Value | +| --------------- | ------------ | +| **Commit** | working tree | +| **IDE / Agent** | Codex | + +**Summary:** Reviewed the local diff that fixes Enter submission for running-thread steering by reading a fresh Lexical composer snapshot at the send boundary. +**Outcome:** No findings. +**Risk score:** Medium — the change touches a shared composer handle, keyboard/form submission, async optimistic send, and plan follow-up helper paths. +**Change archetypes:** shared-ui, optimistic-state, fallback-state. +**Intended change:** Make Enter submit use the current visible editor text while preserving the existing send/steering dispatch path and avoiding stale `promptRef` reads. +**Intent vs actual:** The main send path now obtains a fresh `prompt` through `getSendContext()` and uses that value directly. Model-only callers pass `{ syncPrompt: false }`, preventing stale editor content from being reintroduced after plan follow-up clears the draft. +**Confidence:** Medium-high — the main bug class and sibling callers were traced; no browser smoke was run in this shell. +**Coverage note:** TypeScript and diff whitespace checks passed. Existing lint warnings are unrelated. Exact Bun wrapper commands remain blocked because `bun` is not on PATH. +**Finding triage:** No live findings. The earlier overly broad submit-level sync was removed before this review and is no longer in the diff. +**Static/analyzer evidence:** `oxlint` was run earlier on this diff and reported existing warnings only; no architecture/static analyzer policy changes are present. +**Architecture impact:** The authoritative prompt snapshot stays in the composer/editor owner and is exposed through the existing narrow imperative composer boundary. `ChatView` continues to own orchestration/send semantics and does not read Lexical directly. +**Bug classes / invariants checked:** keyboard submit uses visible editor text; plan follow-up does not resurrect cleared prompt text; implement-in-new-thread reads model state without mutating prompt state; failure rollback restores the sent prompt snapshot; image/terminal context snapshots still come from composer refs. +**Branch totality:** Reviewed the full current local diff for the two changed files and checked the existing `.reviews` ledger for prior related context. +**Sibling closure:** Checked `onSend`, `onSubmitPlanFollowUp`, `onImplementPlanInNewThread`, composer pending-input guard, composer draft store `setPrompt`/`clearComposerContent`, and all `getSendContext` callers. +**Remediation impact surface:** The change is contained to web presentation/application state. No contract schema, server orchestration, provider adapter, or persistence migration surface changed. +**Residual risk / unknowns:** The live UI should still be browser-smoked against a running agent turn to confirm Lexical command ordering in the actual app shell. + +### Validation + +- `./node_modules/.bin/oxfmt` — passed. +- `./node_modules/.bin/oxlint --report-unused-disable-directives` — passed with existing warnings only. +- `./node_modules/.bin/tsc -p apps/web/tsconfig.json --noEmit` — passed. +- `git diff --check` — passed. +- `bun fmt`, `bun lint`, `bun typecheck` — not run because `bun` is not available in this shell. + +### Branch-totality proof + +- **Non-delta files/systems re-read:** diff-review gates, architecture-standards review checklist, `ChatView`, `ChatComposer`, `composerDraftStore`, existing review ledger. +- **Prior open findings rechecked:** No prior open findings applied to this new review area. +- **Prior resolved/adjacent areas revalidated:** Prior ledger notes that running composer visibility intentionally remains stop-only while keyboard/form submit routes through `onSend`; this diff preserves that. +- **Hotspots or sibling paths revisited:** main send, plan follow-up, implement-in-new-thread, pending user input early return, draft clear/set actions, send failure rollback. +- **Dependency/adjacent surfaces revalidated:** No server/provider contract change; Codex steering remains on the existing `thread.turn.start` path. +- **Why this is enough:** The risky ownership boundary is the composer snapshot crossing into `ChatView`; all current callers were audited and the non-primary model-only callers explicitly opt out of prompt sync. + +### Challenger pass + +- Not required for Medium risk. The likely miss was prompt sync leaking into plan follow-up after draft clearing; the current diff prevents that with `{ syncPrompt: false }`. + +### Resolved / Carried / New findings + +- None. + +### Recommendations + +1. **Fix first:** none. +2. **Then address:** browser-smoke the running-agent Enter steering path once the app is running with Bun available. +3. **Patterns noticed:** if more model-only callers appear, split the imperative handle into explicit prompt-send and model-selection readers instead of growing the `syncPrompt` option. diff --git a/.reviews/desktop-updates-release-review.md b/.reviews/desktop-updates-release-review.md new file mode 100644 index 00000000000..c6d02d53b49 --- /dev/null +++ b/.reviews/desktop-updates-release-review.md @@ -0,0 +1,182 @@ +# Review: Desktop Updates + Release + +## Project context + +| Field | Value | +| -------------- | -------------------------------------------- | +| **Repository** | `declancowen/t3code` | +| **Remote** | `origin` | +| **Branch** | `codex/setup-fork-desktop-release` | +| **Stack** | TypeScript, Effect, Electron, GitHub Actions | + +## Scope + +- `.github/workflows/desktop-release.yml` — fork-safe desktop release workflow. +- `apps/desktop/src/window/DesktopApplicationMenu.ts` — manual update-check dialog copy. +- `apps/desktop/src/**/*.test.ts` — menu regression coverage and Electron test shims. +- `apps/web/src/components/PlanSidebar.tsx` and `apps/web/src/components/chat/ChatComposer.tsx` — plan sidebar accent color polish. + +## Hotspots + +- Stable-only GitHub release behavior matching upstream release/update semantics. +- Local update-track settings accidentally pointing at `nightly` without nightly prereleases in the fork. +- Native menu feedback for manual update checks. +- Release workflow publication permissions, tag/version resolution, artifact upload, and updater manifests. + +## Review status + +| Field | Value | +| --------------------- | -------------------- | +| **Review started** | 2026-05-21 12:45 BST | +| **Last reviewed** | 2026-05-21 14:14 BST | +| **Total turns** | 3 | +| **Open findings** | 0 | +| **Resolved findings** | 0 | +| **Accepted findings** | 0 | + +## Turn 3 — 2026-05-21 14:14 BST + +| Field | Value | +| --------------- | ------------ | +| **Commit** | working tree | +| **IDE / Agent** | Codex | + +**Summary:** Re-reviewed after the updater install handoff fix, macOS signing workflow guard, shared update-downloaded toast helper, install-error toast surfacing, and connecting-status color change. +**Outcome:** All clear. +**Risk score:** Medium — the patch changes a desktop updater lifecycle handoff and release workflow failure behavior. +**Change archetypes:** desktop updater lifecycle, release automation, shared UI helper, presentation polish. +**Intended change:** Keep the app alive while Electron/Squirrel performs install handoff, surface install failures, prevent unsigned macOS stable releases, avoid duplicate stable tag publishing, and make update/connecting UI use the lobster primary color. +**Intent vs actual:** The diff matches the intent. `DesktopUpdates.install` now delegates directly to `quitAndInstall`, leaves the UI/window/backend running until Electron takes over, rejects duplicate install clicks, and lets updater `error` events reduce state to install failure. The release workflows now fail stable macOS builds when signing/notarization secrets are missing, and the old upstream release workflow no longer handles stable tags in this fork. The update-downloaded toast icon and connecting thread status now use `text-primary` / `bg-primary`. +**Confidence:** High for the code path and workflow guard; medium for packaged-app install until a Developer ID signed/notarized build is published and manually installed at least once. +**Coverage note:** Current-tree `bun fmt`, `bun lint`, `bun typecheck`, focused desktop updater tests, focused web updater tests, sidebar logic tests, and `git diff --check` passed. Lint warnings remain the existing unrelated warnings. +**Finding triage:** No live findings. +**Static/analyzer evidence:** No static-analysis policy changed. Fallow remains unavailable in the repo/PATH. +**Architecture impact:** The lifecycle invariant stays owned by `apps/desktop/src/updates/DesktopUpdates.ts`; UI surfaces consume state and toast helpers without duplicating updater install policy. The signing requirement is encoded at release workflow boundaries, where the release asset invariant belongs. +**Bug classes / invariants checked:** duplicate install clicks do not call `quitAndInstall` twice; install handoff no longer pre-destroys the UI; install failures keep the downloaded state retryable and surface a toast; stable macOS release assets cannot be unsigned; stable tags are owned by one workflow; primary color is applied through the existing sidebar status helper. +**Branch totality:** Rechecked the working tree plus the branch-total release/updater/UI diff against `origin/main`. +**Sibling closure:** Revisited desktop install failure handling, updater event error handling, settings/sidebar update toast consumers, release workflow tag ownership, and sidebar status color resolution. +**Residual risk / unknowns:** Existing unsigned/ad-hoc installs may not validate an update signed with a future Developer ID certificate. A manual install of the first signed/notarized build may be required before automatic updates can install reliably. + +### Validation + +- `bun run --cwd apps/desktop test src/updates/DesktopUpdates.test.ts src/updates/updateMachine.test.ts` — passed, 12 tests. +- `bun run --cwd apps/web test src/components/desktopUpdate.logic.test.ts` — passed, 24 tests. +- `bun run --cwd apps/web test src/components/Sidebar.logic.test.ts` — passed, 48 tests. +- `bun fmt` — passed. +- `bun lint` — passed with 9 existing warnings. +- `bun typecheck` — passed, 13 packages. +- `git diff --check` — passed. + +### Branch-totality proof + +- **Non-delta files/systems re-read:** diff-review gates, architecture bridge, desktop update install/error paths, toast rendering data contract, sidebar status resolver, release workflow stable tag triggers. +- **Prior open findings rechecked:** None. +- **Prior resolved/adjacent areas revalidated:** Stable-only release policy still holds; new workflow guard prevents publishing macOS artifacts that can download but not install. +- **Hotspots or sibling paths revisited:** settings update button, sidebar update pill, manual/downloaded update toasts, updater event listeners, macOS release signing gate. +- **Why this is enough:** The live failure was traced to Squirrel.Mac code-signature validation. The code now preserves UI visibility for failure reporting, and CI now blocks the artifact class that caused the install failure. + +### Challenger pass + +- Not required for Medium risk. The most plausible remaining issue is packaged-app signature transition behavior from ad-hoc builds to a future signed build; this is carried as residual release/install risk rather than hidden code risk. + +### Resolved / Carried / New findings + +- None. + +## Turn 2 — 2026-05-21 12:59 BST + +| Field | Value | +| --------------- | ------------ | +| **Commit** | working tree | +| **IDE / Agent** | Codex | + +**Summary:** Re-reviewed after the stable-only release clarification and tightened the desktop release workflow to reject prerelease versions entirely. +**Outcome:** All clear. +**Risk score:** Medium — release automation still affects the updater feed and GitHub release metadata. +**Change archetypes:** release automation, updater feed configuration, desktop menu regression coverage, presentation polish. +**Intended change:** Publish only stable desktop releases for the fork and keep the app on the upstream stable updater path. +**Intent vs actual:** The workflow now ignores normal semver prerelease tags, accepts only `x.y.z` / `vx.y.z` in preflight, always publishes `prerelease: false`, and always marks the release as latest. The app code still relies on electron-updater's normal stable behavior instead of mapping GitHub prerelease-feed errors to no-update. +**Confidence:** High for the workflow guard and menu copy; medium for live packaged-app behavior until the installed app is restarted or reinstalled with the updated settings. +**Coverage note:** Current-tree `bun fmt`, `bun lint`, `bun typecheck`, `git diff --check`, and focused desktop updater/menu tests passed. Lint warnings are the existing non-error warnings in unrelated files. +**Finding triage:** No live findings. +**Static/analyzer evidence:** No static-analysis policy changed. Fallow remains unavailable in the repo/PATH. +**Architecture impact:** Stable-vs-nightly ownership is encoded at the release workflow and persisted update-channel setting. No new updater abstraction or provider-specific error bypass was introduced. +**Bug classes / invariants checked:** stable-only version gate rejects prereleases; GitHub release metadata stays non-prerelease/latest; stable update checks use the latest stable release; same-version stable checks surface `up-to-date`; real updater failures remain errors. +**Branch totality:** Rechecked the working tree plus the branch-total release workflow diff against `origin/main`. +**Sibling closure:** Revisited the release workflow preflight/publish jobs, native menu update-check path, desktop update state contract, settings/sidebar update UI paths, and the user-driven primary-color presentation edits. +**Residual risk / unknowns:** A same-version `0.0.25` install can only prove no-update behavior. A later stable release such as `0.0.26` is needed to prove download/install update behavior end to end. + +### Validation + +- `bun fmt` — passed. +- `bun lint` — passed with 9 existing warnings. +- `bun typecheck` — passed, 13 packages. +- `bun run test src/electron/ElectronUpdater.test.ts src/updates/DesktopUpdates.test.ts src/window/DesktopApplicationMenu.test.ts` from `apps/desktop` — passed, 9 tests. +- `git diff --check` — passed. + +### Branch-totality proof + +- **Non-delta files/systems re-read:** diff-review gates, all-clear antipatterns, architecture enforcement guidance, desktop release workflow, updater menu tests, updater state consumers. +- **Prior open findings rechecked:** None. +- **Prior resolved/adjacent areas revalidated:** Turn 1 stable/nightly diagnosis still holds; workflow now prevents accidental prerelease publication for this fork. +- **Hotspots or sibling paths revisited:** GitHub release `prerelease`/`make_latest` metadata, tag/version parsing, manual update check dialog, settings/sidebar update button states. +- **Why this is enough:** The branch now encodes stable-only release policy in CI and keeps runtime update behavior aligned with upstream stable electron-updater semantics. + +### Resolved / Carried / New findings + +- None. + +## Turn 1 — 2026-05-21 12:45 BST + +| Field | Value | +| --------------- | ------------ | +| **Commit** | working tree | +| **IDE / Agent** | Codex | + +**Summary:** Reviewed the current local diff for stable-only desktop updater setup, native menu copy, release workflow branch state, and plan/sidebar color polish. +**Outcome:** All clear with low-risk unknowns. +**Risk score:** Medium — the workflow affects release publication and the local updater configuration determines whether electron-updater asks GitHub for stable releases or nightly prereleases. +**Change archetypes:** release automation, desktop updater configuration, native menu copy, presentation polish. +**Intended change:** Keep the fork on stable releases only, match upstream stable updater behavior, make the manual update-check success dialog say `No updates available`, and push the latest source changes. +**Intent vs actual:** The diff matches the intent. The app-side special case for GitHub `No published versions on GitHub` was removed so `ElectronUpdater` and `DesktopUpdates` remain aligned with upstream. The local desktop settings were reset from `nightly` to `latest`, which makes electron-updater use the upstream stable path. +**Confidence:** Medium-high — the stable-vs-nightly mismatch was confirmed from local settings and release lists; full repo gates pass. Live packaged-app verification still requires restarting or reinstalling so the running app reloads the settings/source revision. +**Coverage note:** Targeted desktop tests, `bun fmt`, `bun lint`, `bun typecheck`, and `git diff --check` passed. Lint still reports 9 pre-existing warnings outside this diff. +**Finding triage:** No live findings. +**Static/analyzer evidence:** No static-analysis policy changed. Fallow was not available in the repo/PATH, so no Fallow evidence was used. +**Architecture impact:** No new updater abstraction was added. Stable behavior remains owned by electron-updater plus the existing update state reducer; the fork-specific setup is release configuration and local `updateChannel` state, not a provider error workaround. +**Bug classes / invariants checked:** stable track uses GitHub stable latest release; nightly track requires nightly prereleases and is out of scope for this fork; same-version stable release emits the upstream no-update path; real check errors stay retryable check errors; menu check displays the no-update dialog; release workflow keeps write permissions only on the publish job. +**Branch totality:** Reviewed local working changes plus the branch-total diff against `origin/main`, including the existing desktop release workflow commit. +**Sibling closure:** Checked upstream `ElectronUpdater`/`DesktopUpdates`, local desktop settings, fork/upstream release lists, native menu dialog path, settings/sidebar update UI consumers, update status contract, release workflow artifact/manifest publish path, and plan/sidebar presentation-only edits. +**Remediation impact surface:** Desktop settings and menu copy only. No IPC schema, package version script, server/provider runtime, updater state machine, or release asset binary was changed in this turn. +**Residual risk / unknowns:** The currently running app may not reload the edited desktop settings until restart. Same-version `0.0.25` releases are still not a valid end-to-end auto-update download/install test; a later stable version such as `0.0.26` is needed for that. + +### Validation + +- `bun run test src/electron/ElectronUpdater.test.ts src/updates/DesktopUpdates.test.ts src/window/DesktopApplicationMenu.test.ts` from `apps/desktop` — passed, 9 tests. +- `bun fmt` — passed. +- `bun lint` — passed with 9 existing warnings. +- `bun typecheck` — passed, 13 packages. +- `git diff --check` — passed. + +### Branch-totality proof + +- **Non-delta files/systems re-read:** diff-review gates, architecture-standards implementation checklist, upstream updater/release files, desktop update UI helpers, native application menu, updater state reducer, IPC update state contract, desktop release workflow. +- **Prior open findings rechecked:** No prior open findings applied to this content area. +- **Prior resolved/adjacent areas revalidated:** The earlier composer steering fix is not changed; the sidebar color edit is presentation-only and does not alter send/runtime behavior. +- **Hotspots or sibling paths revisited:** stable vs nightly update channel selection, settings update check, sidebar update pill/button helpers, GitHub release workflow manifest merge. +- **Dependency/adjacent surfaces revalidated:** `DesktopUpdateState` status contract still carries `up-to-date`; menu copy consumes the existing state; release workflow uses `GITHUB_TOKEN` only in the release job. +- **Why this is enough:** The live issue was traced to a stable/nightly release-channel mismatch, and the repo now avoids adding an app-side workaround that would diverge from upstream behavior. + +### Challenger pass + +- Not required for Medium risk. The most plausible miss was treating the upstream GitHub provider error as an app bug. The current conclusion is configuration/release-channel mismatch: stable-only forks must keep `updateChannel` on `latest`. + +### Resolved / Carried / New findings + +- None. + +### Recommendations + +1. **Fix first:** none. +2. **Then address:** restart the installed app so it reloads `updateChannel: latest`, then check updates again. +3. **Patterns noticed:** if this fork stays stable-only, avoid selecting/publishing nightly channels unless the upstream nightly prerelease pipeline is also adopted. diff --git a/.reviews/kiro-provider-appearance-review.md b/.reviews/kiro-provider-appearance-review.md new file mode 100644 index 00000000000..631f2fc7ed6 --- /dev/null +++ b/.reviews/kiro-provider-appearance-review.md @@ -0,0 +1,484 @@ +# Review: Kiro Provider + Appearance + +## Project context + +| Field | Value | +| -------------- | ---------------------------------------- | +| **Repository** | `declancowen/t3code` | +| **Remote** | `origin` | +| **Branch** | `merge-upstream-main-check` | +| **Stack** | TypeScript, Effect, React/Vite, Electron | + +## Scope + +- `apps/server/src/provider/acp/StandardAcpAdapter.ts` — ACP prompt lifecycle and active-prompt steering. +- `apps/server/src/provider/Layers/KiroAdapter.ts` — Kiro `_message/send` payload mapping. +- `packages/effect-acp/src/protocol.ts` — ACP JSON-RPC transport compatibility for provider-originated requests. +- `packages/effect-acp/src/client.test.ts`, `packages/effect-acp/src/protocol.test.ts` — ACP transport error-channel regression coverage. +- `apps/server/src/provider/acp/AcpAdapterSupport.ts`, `apps/server/src/provider/acp/AcpRuntimeModel.ts` — ACP permission outcome mapping and tool-call classification. +- `apps/server/src/provider/acp/StandardAcpAdapter.test.ts` — ACP steering regression coverage. +- `apps/web/src/components/ChatView.tsx` — running-turn image send guard removal. +- `apps/web/src/components/AppSidebarLayout.tsx`, `apps/web/src/components/NoActiveThreadState.tsx`, `apps/web/src/index.css`, `apps/web/src/routes/*` — sidebar/background appearance changes. +- `assets/*`, `apps/*/public/*`, `apps/desktop/resources/*` — generated app icon assets. + +## Hotspots + +- ACP active-turn lifecycle ownership and duplicate `session/prompt` prevention. +- Kiro cancel behavior because Kiro currently rejects `session/cancel`. +- ACP permission requests with provider-owned UUID request IDs and provider-owned option IDs. +- Active-prompt steering payload compatibility for text and image attachments. +- ACP JSON-RPC response error normalization and provider error data surfacing. +- Running-turn UI send behavior across provider adapters. +- Sidebar/translucency surface consistency across route wrappers. +- macOS app icon visual bounds, corner radius, and generated package assets. + +## Review status + +| Field | Value | +| --------------------- | -------------------- | +| **Review started** | 2026-05-20 | +| **Last reviewed** | 2026-05-23 21:24 BST | +| **Total turns** | 9 | +| **Open findings** | 0 | +| **Resolved findings** | 6 | +| **Accepted findings** | 0 | + +## Turn 9 — 2026-05-23 21:24 BST + +| Field | Value | +| --------------- | ------------ | +| **Commit** | working tree | +| **IDE / Agent** | Codex | + +**Summary:** Reviewed the Kiro/ACP image attachment fix that preserves native ACP image prompts while surfacing provider JSON-RPC errors as request errors instead of Effect decode defects. +**Outcome:** No findings. +**Risk score:** High — this changes shared ACP transport response handling, the shared ACP adapter attachment path, and Kiro provider-specific attachment validation. +**Change archetypes:** protocol compatibility, provider capability guard, attachment validation, error-channel preservation, regression coverage. +**Intended change:** Keep image upload support for Kiro, reject unsupported image paths clearly, and preserve provider error `data` so invalid images no longer show the low-level `Internal error at decode` stack. +**Intent vs actual:** The diff matches the intent. JSON-RPC response errors that the Effect RPC layer decoded as `Die` defects are normalized back to protocol failures before reaching core RPC callers or generic extension callers. `mapAcpToAdapterError` can now read the preserved `data`. Shared ACP content-block construction checks the agent's advertised image prompt capability and lets providers supply their own image MIME allowlist. Kiro wires the allowlist to PNG/JPEG/GIF/WebP without moving provider policy into the transport layer. +**Confidence:** High for transport and adapter behavior because both core RPC and extension request error paths now have direct tests. Medium for Kiro's private active-prompt `_message/send` image payload contract because that path is covered by the shared adapter unit test, while the live provider probe covered normal `session/prompt` image handling and invalid image error surfacing. +**Coverage note:** Added focused tests for ACP JSON-RPC error surfacing, extension error surfacing, image-capability rejection, and provider MIME allowlist rejection. +**Finding triage:** No open findings. The main candidate failures were checked directly: protocol errors still route through typed `AcpRequestError`, non-image-capable agents cannot receive image blocks, and Kiro rejects unsupported MIME types before file read/provider dispatch. +**Static/analyzer evidence:** No analyzer policy changes. Fallow is not available in this repo context. Lint passed with the existing 9 warnings unrelated to this diff. +**Architecture impact:** The capability invariant lives in the shared ACP adapter, where prompt content is materialized for all standard ACP providers. Kiro's narrower MIME policy remains in `KiroAdapter`, preserving provider ownership. Transport normalization stays in `effect-acp` and is not Kiro-specific. +**Bug classes / invariants checked:** provider JSON-RPC errors preserve code/message/data; core RPC and generic extension errors use the request error channel; image content is only sent to agents advertising image prompt support; provider MIME allowlists reject unsupported image types before dispatch; Kiro image types align with the existing Claude/common vision set. +**Branch totality:** Reviewed the scoped local diff for the ACP/Kiro image fix. The working tree still contains unrelated `.gitignore`, app-logo, public, and release artifact changes that are intentionally outside this review and commit scope. +**Sibling closure:** Checked `AcpAdapterSupport` error formatting, ACP error classes, attachment path resolution, Claude/Codex/Cursor image attachment patterns, shared ACP prompt construction, Kiro active-prompt send mapping, and web send attachment handoff. +**Remediation impact surface:** Affects Kiro and any future provider using `makeStandardAcpAdapter` with image attachments. Cursor remains on its separate adapter path. Existing text-only ACP prompt behavior is unchanged. +**Residual risk / unknowns:** Kiro's private `_message/send` method is not an ACP standard contract; the shared adapter payload shape is tested, but provider behavior could still change outside this repo. + +### Validation + +- `bun run test src/client.test.ts src/protocol.test.ts` from `packages/effect-acp` — passed, 17 tests. +- `bun run test src/provider/acp/StandardAcpAdapter.test.ts` from `apps/server` — passed, 10 tests. +- `bun fmt` — passed. +- `bun lint` — passed with 9 existing warnings. +- `bun typecheck` — passed, 13 packages. +- `git diff --check -- apps/server/src/provider/Layers/KiroAdapter.ts apps/server/src/provider/acp/StandardAcpAdapter.test.ts apps/server/src/provider/acp/StandardAcpAdapter.ts packages/effect-acp/src/client.test.ts packages/effect-acp/src/protocol.test.ts packages/effect-acp/src/protocol.ts` — passed. + +### Branch-totality proof + +- **Non-delta files/systems re-read:** diff-review gates, all-clear antipatterns, architecture implementation checklist, `AcpAdapterSupport`, `effect-acp` error classes, attachment store/mime helpers, Claude/Codex/Cursor attachment handling, web send attachment handoff. +- **Prior open findings rechecked:** No open findings remained from Turn 8. +- **Prior resolved/adjacent areas revalidated:** Active-prompt attachment steering remains routed through the existing Kiro `_message/send` hook; Kiro stop/cancel behavior is unchanged by this turn. +- **Hotspots or sibling paths revisited:** Shared ACP protocol response routing, generic extension pending requests, normal `session/prompt`, active-prompt content block construction, provider-specific MIME policy, attachment path resolution. +- **Dependency/adjacent surfaces revalidated:** `mapAcpToAdapterError` already formats preserved provider `data`, so the transport fix now feeds the UI-facing detail path. +- **Why this is enough:** The riskiest invariant is error-channel classification, and both the core RPC path and extension path are directly covered. The attachment support checks are enforced before dispatch and have unit coverage for both capability and MIME rejection. + +### Challenger pass + +Done — assumed the fix could either swallow true provider defects or block valid image-capable providers. The normalization only rewrites failure entries shaped like JSON-RPC protocol errors, and the image guard follows ACP `promptCapabilities.image` with Kiro-specific MIME policy isolated to the Kiro adapter. + +### Resolved / Carried / New findings + +- None. + +### Recommendations + +1. **Fix first:** none. + +## Turn 8 — 2026-05-21 07:08 BST + +| Field | Value | +| --------------- | ------------ | +| **Commit** | working tree | +| **IDE / Agent** | Codex | + +**Summary:** Re-reviewed the full local diff after the Kiro ACP permission/stop fixes, send/icon/sidebar polish, and the corrected macOS-style app icon corner radius. +**Outcome:** No findings. +**Risk score:** High — this turn touches the shared ACP transport, shared ACP adapter lifecycle, provider-specific Kiro behavior, and generated release assets. +**Change archetypes:** protocol compatibility, provider lifecycle, permission mapping, visual asset replacement, shared UI presentation. +**Intended change:** Keep Kiro steering as active-prompt `_message/send`, make Kiro tool approvals complete by preserving UUID JSON-RPC request IDs and provider option IDs, make Stop actually stop Kiro when `session/cancel` is unsupported, and refresh the generated app icon assets with a rounder macOS-style boundary. +**Intent vs actual:** The diff matches the intent. Steering remains isolated to `sendMessageWhilePromptActive` in `KiroAdapter` and is not routed through the stop fallback. The stop fallback is Kiro opt-in only through `stopSessionOnInterruptCancelUnsupported`. Permission responses now use provider-supplied option IDs, and the ACP transport preserves non-numeric request IDs without making Kiro-specific protocol branches. +**Confidence:** High for protocol/unit behavior and package output; medium for final visual preference until the rebuilt DMG is inspected in Finder/Dock. +**Coverage note:** Targeted ACP protocol, ACP adapter, ACP runtime-model, sidebar, and composer tests passed. Full `bun fmt`, `bun lint`, `bun typecheck`, and `git diff --check` passed. Electron macOS arm64 packaging passed and rebuilt the DMG/ZIP artifacts. +**Finding triage:** No open findings. The main suspected issues were checked directly: Kiro Stop no longer depends on Kiro honoring `session/cancel`, and active-prompt send/attachment steering stays on the `_message/send` path. +**Architecture impact:** The protocol fix lives in `effect-acp` as transport compatibility, not in the Kiro provider. Shared ACP adapter behavior remains opt-in for providers that cannot cancel. Kiro-specific wiring is limited to the existing Kiro adapter layer. UI polish stays in owning sidebar/composer helpers and source icon assets. +**Bug classes / invariants checked:** nonnumeric JSON-RPC IDs round-trip; provider permission option IDs are honored; missing ACP tool kinds are inferred conservatively; Kiro Stop closes the ACP session after cancel write/failure; project row toggle behavior remains after chevron removal; generated icons preserve expected formats and dimensions. +**Branch totality:** Reviewed all local changes in the dirty tree. The branch is still local and dirty on `main`, one commit behind `origin/main`. +**Sibling closure:** Rechecked shared ACP transport/client tests, ACP adapter tests, Cursor adapter permission mapping, Kiro adapter active-prompt send path, provider runtime ingestion of `session.exited`, sidebar status helpers, composer primary action rendering, and desktop/web/marketing icon targets. +**Residual risk / unknowns:** The local dev server must be restarted before live Kiro Stop reflects this patch. Kiro Stop now terminates the Kiro ACP session because Kiro does not support a soft `session/cancel`; a fresh Kiro session may be needed after stopping. + +### Validation + +- `bun --filter t3 test src/provider/acp/AcpRuntimeModel.test.ts src/provider/acp/AcpAdapterSupport.test.ts src/provider/acp/StandardAcpAdapter.test.ts` — passed, 19 tests. +- `bun --filter effect-acp test src/client.test.ts src/protocol.test.ts` — passed, 15 tests. +- `bun --filter @t3tools/web test src/components/Sidebar.logic.test.ts src/components/chat/ComposerPrimaryActions.test.ts src/components/ui/sidebar.test.tsx` — passed, 59 tests. +- `bun fmt` — passed. +- `bun lint` — passed with 9 existing warnings. +- `bun typecheck` — passed, 13 packages. +- `git diff --check` — passed. +- `file`, `sips`, and `iconutil -c iconset` checks verified updated desktop/web/marketing icon file types and dimensions. +- `bun run dist:desktop:dmg:arm64` — passed after rerunning outside the sandbox temp-dir restriction; rebuilt `release/T3-Code-0.0.24-arm64.dmg` and `.zip`. + +### Branch-totality proof + +- **Non-delta files/systems re-read:** diff-review gates, architecture-standards build-mode guidance, ACP transport protocol tests, ACP adapter/runtime helpers, Kiro adapter, Cursor adapter, sidebar helpers, composer primary actions, brand asset outputs. +- **Prior open findings rechecked:** No open findings remained from Turn 7. The new Kiro stop regression was handled and covered. +- **Prior resolved/adjacent areas revalidated:** Running composer remains stop-only visually while the send/steer path is still available through active-prompt dispatch. Generated icon assets were refreshed again after the radius change. +- **Hotspots or sibling paths revisited:** Provider approval request handling, active prompt send hook, interrupt/stop path, JSON-RPC request ID translation, project row toggle handlers, status color aggregation, macOS `.icns` decode. +- **Why this is enough:** The risky runtime changes are covered by focused unit tests in both the shared transport and server adapter layers, and the generated assets were validated by type/dimension/package checks. + +### Challenger pass + +Done — the most plausible miss was conflating Stop with steering. The code paths are separate: active-prompt user messages go through Kiro `_message/send`, while only `interruptTurn` uses the Kiro opt-in session-stop fallback. + +### Resolved / Carried / New findings + +- None. + +### Recommendations + +1. **Fix first:** none. +2. **Then address:** restart the local backend/web process before live-testing Kiro Stop, because the old server process will still have the old adapter behavior. + +## Turn 7 — 2026-05-21 06:24 BST + +| Field | Value | +| --------------- | ------------ | +| **Commit** | working tree | +| **IDE / Agent** | Codex | + +**Summary:** Re-reviewed the full local diff after the app icon inset, repo-list chevron removal, send-icon centering adjustment, and Lobster-colored working indicator. +**Outcome:** No findings. +**Risk score:** Medium — this is presentation and generated asset work across desktop/web/marketing targets, with shared sidebar status logic touched, but no provider runtime or hook architecture changes. +**Change archetypes:** visual asset replacement, shared UI presentation, sidebar thread-status logic. +**Intended change:** Make the packaged app icon visually match normal Dock icon sizing, remove the visible project/repo chevron without removing row toggle behavior, center the send icon, and make the active working marker use the Lobster primary color. +**Intent vs actual:** The diff matches the stated intent. The project row click/keyboard handlers remain in place after removing the chevron. `resolveThreadStatusPill` still owns thread status presentation and now maps only the `Working` state to `text-primary`/`bg-primary`. The app icon source SVGs now keep transparent outer padding and all generated icon targets were refreshed. +**Confidence:** High for code behavior and asset file validity; medium for final visual preference until the rebuilt Electron app is inspected in the Dock. +**Coverage note:** Targeted sidebar/composer/brand tests passed, full repo fmt/lint/typecheck passed, generated icon formats and dimensions were checked, and the Electron macOS arm64 artifacts were rebuilt. +**Finding triage:** No open findings. The previous provider/ACP and CORS hotspots are unchanged by this turn. +**Architecture impact:** Presentation behavior remains in the existing owning components/helpers: sidebar status policy in `Sidebar.logic`, project row rendering in `Sidebar`, and composer primary action rendering in `ComposerPrimaryActions`. Provider hooks/adapters and ACP runtime architecture are untouched. +**Bug classes / invariants checked:** project row remains toggleable without visual chevron; working status priority and folded project indicator still flow through shared status logic; send button still uses the existing submit path; icon assets preserve expected public dimensions while reducing opaque alpha bounds. +**Branch totality:** Reviewed all local changes in the dirty tree. The branch is still local and dirty on `main`, which is one merge commit behind `origin/main` but had no tree delta from origin before these edits. +**Sibling closure:** Checked sidebar project header, hidden-thread status label path, command-palette thread status consumers, composer primary action path, web/marketing/desktop icon targets, and brand asset tests. +**Residual risk / unknowns:** The browser visual smoke for the sidebar row itself was not rerun in this turn; the packaged app was rebuilt for Dock-icon inspection. + +### Validation + +- `bun run test src/components/Sidebar.logic.test.ts src/components/chat/ComposerPrimaryActions.test.ts src/components/ui/sidebar.test.tsx` — passed, 59 tests. +- `bun run test lib/brand-assets.test.ts` — passed, 5 tests. +- `bun fmt` — passed. +- `bun lint` — passed with 9 existing warnings. +- `bun typecheck` — passed, 13 packages. +- `git diff --check` — passed. +- `file` and `sips` checks verified updated desktop/web/marketing icon file types and dimensions. +- Alpha-bounds check verified `assets/prod/black-macos-1024.png` opaque content is inset to `832x832` within the `1024x1024` canvas. +- `bun run dist:desktop:dmg:arm64` — passed after rerunning outside the local packaging temp-dir restriction; rebuilt `release/T3-Code-0.0.24-arm64.dmg` and `.zip`. + +### Branch-totality proof + +- **Non-delta files/systems re-read:** diff-review gates, architecture-standards build-mode guidance, `ThreadStatusIndicators`, `Sidebar.logic`, `Sidebar`, `ComposerPrimaryActions`, brand asset outputs. +- **Prior open findings rechecked:** No open findings remained. Provider hook and ACP adapter paths are untouched by this turn. +- **Prior resolved/adjacent areas revalidated:** Running composer behavior remains stop-only visually while submit behavior is unchanged outside this visual icon edit. +- **Hotspots or sibling paths revisited:** Project row toggle handlers, status priority aggregation, compact status label, generated macOS/Windows/web icon targets. +- **Why this is enough:** The changed code is narrow presentation logic with direct unit coverage; binary asset changes were regenerated from the source SVGs and checked for validity/dimensions; desktop packaging succeeded. + +### Challenger pass + +Done — the main plausible miss was removing the chevron in a way that also removed expand/collapse affordance behavior. The row's click and keyboard toggle handlers remain wired, and tests still cover sidebar UI primitives. + +### Resolved / Carried / New findings + +- None. + +### Recommendations + +1. **Fix first:** none. +2. **Then address:** inspect the rebuilt DMG in the Dock to confirm the new icon inset has the desired visual size. + +## Turn 6 — 2026-05-21 05:45 BST + +| Field | Value | +| --------------- | ------------ | +| **Commit** | working tree | +| **IDE / Agent** | Codex | + +**Summary:** Re-reviewed the full current local diff after the new icon and chat UI adjustments, including staged CORS/auth changes and unstaged branding/chat presentation changes. +**Outcome:** All clear after restoring the `ProjectFavicon` rendering invariant. +**Risk score:** Medium — the current tree spans browser auth headers, app branding assets, and chat composer/timeline presentation, but the high-risk provider lifecycle paths remain unchanged by this turn. +**Change archetypes:** auth/API contract, visual asset replacement, chat presentation behavior, shared UI rendering. +**Intended change:** Keep credentialed browser auth working, preserve Kiro/provider architecture, keep the running composer control stop-only by design, update app icon assets, and polish chat message/header visuals. +**Intent vs actual:** The current tree matches the stated intent. The running composer primary action intentionally shows only Stop while active; keyboard/form submit still routes through the existing `onSend` path for follow-up steering. Provider hooks and ACP adapter architecture are untouched in this turn. `ProjectFavicon` keeps the new tighter radius without cropping non-square icons. +**Confidence:** High for CORS/auth and TypeScript/lint health; medium for visual presentation because final polish remains product judgment. +**Coverage note:** Targeted tests covered browser CORS/auth, composer send-state helpers, primary-action label helpers, brand asset mapping, and project favicon resolution. Full repo fmt/lint/typecheck also passed. +**Finding triage:** Rechecked F-006 in the current unstaged tree and restored the fix. +**Architecture impact:** The browser CORS rule remains centralized at the HTTP edge. Chat control presentation stays in `ComposerPrimaryActions`/`ChatComposer` without adding provider-specific UI branches. Provider runtime and hook architecture were not changed. +**Bug classes / invariants checked:** credentialed CORS exact-origin contract; preflight parity; running-turn stop-only visible control; hidden submit path remains the single send path; project favicon non-cropping invariant; icon file validity. +**Branch totality:** Reviewed staged and unstaged local changes plus cumulative branch hotspots from the prior Kiro/appearance work. The branch is still dirty, with staged server/review/favicon files and unstaged icon/UI assets. +**Sibling closure:** Checked auth success/failure/preflight routes, OTLP CORS, composer primary actions, collapsed/mobile submit entrypoint, keyboard Enter submit, message timeline user-row controls, favicon resolver, and generated icon targets. +**Residual risk / unknowns:** Browser visual smoke was not rerun in this turn after the tiny favicon class fix; the local app was already running and the user reported the flow working. Product review should still decide whether the new icon set and dark-surface adjustments are final. + +### Validation + +- `bun run test src/server.test.ts -t "CORS|auth success|auth failures|environment descriptor|OTLP trace|reports unauthenticated session|bootstraps a bearer session" --testTimeout 10000` — passed, 10 tests, rerun with local bind permission after sandbox blocked test-server listen. +- `bun run test src/components/ChatView.logic.test.ts src/components/chat/ComposerPrimaryActions.test.ts` — passed, 33 tests. +- `bun run test lib/brand-assets.test.ts` — passed, 5 tests. +- `bun run test src/project/Layers/ProjectFaviconResolver.test.ts` — passed, 3 tests. +- `bun fmt` — passed. +- `bun lint` — passed with 9 existing warnings. +- `bun typecheck` — passed, 13 packages. +- `git diff --check` — passed. + +### Branch-totality proof + +- **Non-delta files/systems re-read:** diff-review and architecture-standards gates, `ChatComposer`, `ComposerPrimaryActions`, `ComposerPromptEditor`, `ChatView.logic`, `MessagesTimeline`, auth CORS routes, brand asset tests, favicon resolver tests. +- **Prior open findings rechecked:** No open findings remained. F-006 was rechecked against the current local diff and fixed where it had drifted. +- **Prior resolved/adjacent areas revalidated:** CORS/auth contract and Kiro active-prompt review hotspots remain covered by tests or unchanged from the last passing turn. +- **Hotspots or sibling paths revisited:** Running composer visible controls, keyboard submit, collapsed mobile submit button, auth route success/failure, browser preflight, icon asset outputs. +- **Why this is enough:** The current high-risk runtime contracts are covered by targeted tests and typecheck; the remaining changes are presentation/asset polish with file validity and relevant rendering invariants checked. + +### Challenger pass + +Done — the most plausible miss was interpreting the stop-only running composer as a regression. The user clarified that behavior is intentional, so the review checked for broken submit routing and attachment eligibility instead of reintroducing a second visible running button. + +### Resolved / Carried / New findings + +- **F-006 — Resolved in current tree:** `ProjectFavicon` keeps `object-contain` with the new `rounded-[3px]` radius, so non-square project icons are not cropped. + +### Recommendations + +1. **Fix first:** none. +2. **Then address:** keep the current dirty-tree commit split deliberate; the staged CORS/review files and unstaged icon/UI files are different change groups. + +## Turn 5 — 2026-05-21 05:14 BST + +| Field | Value | +| --------------- | ------------ | +| **Commit** | working tree | +| **IDE / Agent** | Codex | + +**Summary:** Re-reviewed all local git changes, including staged CORS/auth fixes, unstaged icon/favicon assets, the untracked `assets/app-logo.svg`, and `ProjectFavicon`. +**Outcome:** All clear after one local fix. +**Risk score:** Medium — the dirty tree includes browser auth contract changes and app branding assets, but the runtime code changes are narrow and verified. +**Change archetypes:** auth/API contract, shared middleware, visual asset replacement, small presentation rendering tweak. +**Intended change:** Fix the browser credentialed-CORS failure, preserve provider architecture, update app/marketing/web icon assets, and keep project favicons visually polished without changing favicon resolver semantics. +**Intent vs actual:** The diff now matches the intent. CORS logic is centralized at the HTTP edge; provider hooks remain untouched; generated icon assets have valid file types and expected public dimensions; `ProjectFavicon` keeps the new radius but preserves `object-contain` so non-square project icons are not cropped. +**Confidence:** High for the CORS fix and local icon asset validity; medium for visual-brand preference because asset aesthetics are product judgment. +**Coverage note:** Direct route tests cover the auth/CORS header contract. Asset checks covered file types and dimensions for the committed icon families. The desktop icon was visually inspected. +**Finding triage:** Found and fixed F-006. +**Architecture impact:** The auth fix stays in the API/transport layer, and the favicon display fix stays in presentation. No provider hook or adapter architecture was changed. +**Bug classes / invariants checked:** credentialed CORS exact-origin contract; preflight parity; route success/failure headers; asset dimension/file validity; non-square project icon preservation. +**Branch totality:** Reviewed staged and unstaged local changes. The dirty tree still has unstaged asset files and `assets/app-logo.svg`; review included them, but staging/commit scope should remain intentional. +**Sibling closure:** Checked web, marketing, desktop icon targets, brand asset mapping, project favicon resolver, and all auth CORS sibling routes. +**Residual risk / unknowns:** The icon set should still be checked in the packaged app for final product fit, but no file-format or code-path blocker remains. + +### Validation + +- `node node_modules/vitest/vitest.mjs run scripts/lib/brand-assets.test.ts apps/server/src/project/Layers/ProjectFaviconResolver.test.ts` — passed, 8 tests. +- `node node_modules/vitest/vitest.mjs run apps/server/src/server.test.ts -t "CORS|auth success|auth failures|environment descriptor|OTLP trace|reports unauthenticated session|bootstraps a bearer session" --testTimeout 10000` — passed, 10 tests. +- `curl` against the restarted backend verified `/api/auth/session` returns exact-origin CORS plus `Access-Control-Allow-Credentials: true`. +- `file`/`sips` checks verified updated icon file types and dimensions; `apps/desktop/resources/icon.png` was visually inspected. +- `bun fmt` — passed. +- `bun lint` — passed with 9 existing warnings. +- `bun typecheck` — passed with Bun directory added to `PATH` for Turbo package-manager resolution. +- `git diff --check` — passed. + +### Branch-totality proof + +- **Non-delta files/systems re-read:** `scripts/lib/brand-assets.ts`, brand asset tests, project favicon resolver tests, web/marketing icon references, CORS middleware, auth routes, web auth fetches. +- **Prior open findings rechecked:** Kiro steering/cancel provider paths are untouched by the CORS and asset changes. +- **Prior resolved/adjacent areas revalidated:** Credentialed browser auth now works on the live restarted server. +- **Hotspots or sibling paths revisited:** Auth preflight, auth success/failure, browser telemetry CORS, web/marketing/favicon icon targets, desktop resource icon targets. +- **Why this is enough:** The changed runtime contracts are tested directly; binary assets are validated by file type/dimensions plus visual inspection; the remaining risk is product visual preference rather than correctness. + +### Challenger pass + +Done — the most likely missed issue was an innocuous-looking favicon class change cropping non-square project logos. That was fixed by restoring `object-contain` while keeping the new corner radius. + +### Resolved / Carried / New findings + +- **F-006 — Resolved:** `ProjectFavicon` used `object-cover`, which could crop non-square project icons/logos returned by the resolver. Fixed by keeping `rounded-[3px]` but restoring `object-contain`. + +### Recommendations + +1. **Fix first:** none. +2. **Then address:** decide whether the unstaged icon asset set should be included in the next commit or separated from the CORS fix. + +## Turn 4 — 2026-05-21 05:04 BST + +| Field | Value | +| --------------- | ------------ | +| **Commit** | working tree | +| **IDE / Agent** | Codex | + +**Summary:** Reviewed the local auth CORS fix after the browser rejected `/api/auth/session` because the backend returned `Access-Control-Allow-Origin: *` while the web client fetches with credentials. +**Outcome:** All clear for the CORS/auth patch. +**Risk score:** Medium — browser auth routes and the shared CORS middleware are public HTTP contract surfaces, but the change is centralized and covered by route-level tests. +**Change archetypes:** auth/API contract, transport compatibility, shared middleware. +**Intended change:** Preserve the existing broad browser API CORS behavior while making credentialed browser auth requests legal by echoing valid request origins and sending `Access-Control-Allow-Credentials: true`. +**Intent vs actual:** The diff matches the intent. CORS policy remains centralized in `httpCors.ts`/`http.ts`; auth success and failure responses now derive headers from the current request; provider hooks and Kiro adapter code are untouched. +**Confidence:** High for the observed local browser/server contract; medium for arbitrary hosted-origin deployments because this preserves the previous broad origin policy rather than adding a new origin allowlist. +**Coverage note:** Server tests now assert exact-origin credentialed CORS for environment, auth bootstrap/session/ws-token success and failure, websocket token preflight, and browser OTLP CORS paths. +**Finding triage:** No new blocking findings found. +**Architecture impact:** The invariant is owned by the HTTP/API edge, not by provider runtime code. The change avoids scattering CORS fixes across individual providers or UI fetch wrappers. +**Bug classes / invariants checked:** credentialed CORS wildcard rejection; preflight response parity; auth success/failure header consistency; non-provider architecture isolation. +**Branch totality:** Rechecked the current local code delta for `auth/http.ts`, `http.ts`, `httpCors.ts`, and `server.test.ts`; unrelated icon/logo asset changes remain outside this fix. +**Sibling closure:** Auth session, bootstrap, bearer bootstrap, websocket token, pairing/client management, environment descriptor, and OTLP browser routes were considered. +**Residual risk / unknowns:** Origin reflection intentionally preserves the repo's previous broad browser API accessibility. If product direction changes toward a stricter remote-client allowlist, this should be tightened as a separate auth/network-access policy change. + +### Validation + +- `node node_modules/vitest/vitest.mjs run apps/server/src/server.test.ts -t "CORS|auth success|auth failures|environment descriptor|OTLP trace|reports unauthenticated session|bootstraps a bearer session" --testTimeout 10000` — passed, 10 tests. +- `bun fmt` — passed. +- `bun lint` — passed with 9 existing warnings. +- `bun typecheck` — passed with Bun directory added to `PATH` for Turbo package-manager resolution. + +### Branch-totality proof + +- **Non-delta files/systems re-read:** web auth fetch calls, server CORS middleware, auth route handlers, existing CORS tests, Effect HTTP middleware behavior. +- **Prior open findings rechecked:** Kiro provider hooks and ACP prompt state are not changed by this patch. +- **Prior resolved/adjacent areas revalidated:** Local browser auth bootstrap/session flow now has exact-origin CORS coverage. +- **Hotspots or sibling paths revisited:** Auth route success and failure branches, preflight handling, browser telemetry route. +- **Why this is enough:** The observed browser failure is a serialized HTTP header contract bug, and route tests now assert the headers the browser requires for credentialed requests. + +### Challenger pass + +Done — the most likely missed issue was that fixing `GET /api/auth/session` only would leave JSON `POST` auth routes blocked at preflight. The global CORS middleware now sends exact-origin credentialed preflight responses, and auth success/failure routes use the same request-origin helper. + +### Resolved / Carried / New findings + +No new findings. + +### Recommendations + +1. **Fix first:** none. +2. **Then address:** restart the local backend/web pair and confirm the browser console no longer shows the wildcard credentialed CORS rejection. + +## Turn 3 — 2026-05-21 04:43 BST + +| Field | Value | +| --------------- | ------------ | +| **Commit** | working tree | +| **IDE / Agent** | Codex | + +**Summary:** Re-reviewed after live Kiro logs showed `Prompt already in progress` from a second full `session/prompt` and later output being attributed to the failed overlap turn. +**Outcome:** All clear for the local fix. +**Risk score:** Medium — this is shared ACP prompt lifecycle state, but the patch is contained to the standard ACP adapter and regression coverage targets the observed failure mode. +**Change archetypes:** async lifecycle, provider adapter contract, state reconciliation. +**Intended change:** Keep running Kiro output attached to the real active provider turn, route active follow-ups through the active-prompt hook when any local session state still marks a turn active, and clear stale active-turn state after normal completion. +**Intent vs actual:** The diff matches the intent. Completed prompts now remove `session.activeTurnId`; failed overlapping prompts restore the previous active prompt/session marker instead of leaving the failed overlap as active; active-hook routing also checks the session snapshot active marker. +**Confidence:** High for adapter state behavior; medium for live Kiro extension behavior until restarted and smoked against the real CLI. +**Coverage note:** Added regression coverage for post-completion active state clearing and overlap-failure active state restoration. +**Finding triage:** No new blocking findings found. +**Architecture impact:** The prompt lifecycle invariant stays owned by `StandardAcpAdapter`; provider-specific Kiro steering remains isolated in `KiroAdapter`. +**Bug classes / invariants checked:** stale active-turn marker; overlapping prompt failure rollback; duplicate `session/prompt` prevention fallback; ACP cancel path remains wired through `thread.turn.interrupt` -> provider interrupt -> `session/cancel`. +**Branch totality:** Rechecked the current local diff plus adjacent interrupt paths in `ChatView`, `ProviderCommandReactor`, `ProviderService`, `StandardAcpAdapter`, and effect-acp cancel transport tests. +**Sibling closure:** Cursor/Codex/Claude/OpenCode use separate adapter paths for turn execution; Kiro remains the standard ACP adapter consumer for this active-hook flow. +**Residual risk / unknowns:** Typing `stop` into the composer is a normal message, not a cancel command. The square Stop control is the cancel path and should produce `session/cancel` in provider logs after restart. + +### Validation + +- `node node_modules/vitest/vitest.mjs run apps/server/src/provider/acp/StandardAcpAdapter.test.ts` — passed, 6 tests. +- `bun fmt` — passed. +- `bun lint` — passed with 9 existing warnings. +- `bun typecheck` — passed with Bun directory added to `PATH` for Turbo package-manager resolution. +- `git diff --check` — passed. + +### Branch-totality proof + +- **Non-delta files/systems re-read:** `ProviderCommandReactor` interrupt dispatch, `ProviderService.interruptTurn`, `ChatView.onInterrupt`, Kiro adapter active hook, effect-acp `session/cancel` transport tests. +- **Prior open findings rechecked:** Active steering attachments remain covered; ACP cancellation remains covered by existing adapter and orchestration tests. +- **Prior resolved/adjacent areas revalidated:** The new regression covers the remaining state reconciliation hole found in live logs. +- **Hotspots or sibling paths revisited:** Active prompt lifecycle state, provider session snapshot state, and cancel/interrupt routing. +- **Why this is enough:** The observed live failure was incorrect adapter state around overlapping prompts; the new test reproduces that class directly and the required repo checks pass. + +### Challenger pass + +Done — the most likely missed issue was that cancelling via the UI Stop button might still not call the provider. Existing code/test coverage confirms the UI command reaches `providerService.interruptTurn`, and the standard ACP adapter always forwards `session/cancel`, including when no local active prompt is registered. + +### Resolved / Carried / New findings + +No new findings. + +### Recommendations + +1. **Fix first:** none. +2. **Then address:** restart backend/web and confirm the next live Kiro run shows `_message/send` for steering and `session/cancel` when pressing the square Stop control. + +## Turn 2 — 2026-05-20 22:17 BST + +| Field | Value | +| --------------- | ---------- | +| **Commit** | `33128fea` | +| **IDE / Agent** | Codex | + +**Summary:** Re-reviewed the local diff after the Kiro running-turn steering fix was extended from text-only to text plus image attachments. +**Outcome:** All clear with low-risk unknowns. +**Risk score:** Medium — shared ACP adapter lifecycle behavior and provider-specific Kiro payload mapping changed, but the surface is narrow and directly covered by regression tests. +**Change archetypes:** async lifecycle, provider adapter contract, attachment/content contract, shared UI guard. +**Intended change:** While a Kiro ACP prompt is active, sending another message, including image attachments, should steer the active prompt instead of starting a second `session/prompt` or requiring stop/interruption. +**Intent vs actual:** The diff matches the intent. `StandardAcpAdapter` now materializes the same ACP content blocks for initial prompts and active-prompt steering, uses the active-prompt hook when a prompt is in flight, and clears the internal active turn marker when the prompt resolves so later messages start fresh prompts. +**Confidence:** High for local adapter behavior; medium for live Kiro private extension compatibility because `_message/send` is not a public typed contract in this repo. +**Coverage note:** The focused ACP test asserts text steering, attachment steering, no duplicate prompt while active, and fresh prompt after completion. +**Finding triage:** No new blocking findings found. +**Static/analyzer evidence:** `bun lint` passed with 9 existing warnings unrelated to this change. +**Architecture impact:** The shared ACP layer owns content-block materialization and active prompt lifecycle. Kiro-specific private method payload shape stays isolated in `KiroAdapter`, preserving the provider hook architecture. +**Bug classes / invariants checked:** Duplicate active prompt prevention; ACP prompt lifecycle authority; attachment materialization parity; post-completion fresh prompt behavior; UI no longer pre-blocks running image sends. +**Branch totality:** Rechecked the current local diff across ACP adapter, Kiro adapter, ChatView send path, and appearance wrappers. +**Sibling closure:** `rg` confirms `makeStandardAcpAdapter` is used by Kiro only; other providers keep their own adapter paths. +**Remediation impact surface:** No public schema changes. The provider hook signature widened internally to include structured ACP content blocks while preserving the plain text string for text-only hooks. +**Residual risk / unknowns:** A live Kiro browser smoke should still be run after restarting the dev servers because `_message/send` is a Kiro private extension and the repo cannot type-check its runtime payload schema. + +### Validation + +- `node node_modules/vitest/vitest.mjs run apps/server/src/provider/acp/StandardAcpAdapter.test.ts` — passed, 5 tests. +- `bun fmt` — passed. +- `bun lint` — passed with 9 existing warnings. +- `bun typecheck` — passed with Bun directory added to `PATH` for Turbo package-manager resolution. +- `git diff --check` — passed. + +### Branch-totality proof + +- **Non-delta files/systems re-read:** `ProviderService.sendTurn`, `ProviderCommandReactor`, `ChatView.onSend`, Kiro adapter hook, ACP content block schema. +- **Prior open findings rechecked:** Previous interrupt/cancel findings remain covered by existing `StandardAcpAdapter` tests. +- **Prior resolved/adjacent areas revalidated:** Active prompt steering now covers both text-only and attachment variants. +- **Hotspots or sibling paths revisited:** Provider hook usage was searched; Kiro remains the only standard ACP adapter consumer. +- **Dependency/adjacent surfaces revalidated:** UI image send guard removal checked against backend attachment materialization and provider routing. +- **Why this is enough:** The high-risk behavior is adapter routing, and the tests directly prove the routing invariant under active and completed prompt states. + +### Challenger pass + +- Done — the most likely missed issue was attachment sends still being blocked in the UI or rejected by the text-only active-prompt helper. Both paths were removed/reworked and covered with a regression test. + +### Resolved / Carried / New findings + +No new findings. + +### Recommendations + +1. **Fix first:** none. +2. **Then address:** restart local backend/web and smoke test Kiro text + image steering against the real CLI. +3. **Patterns noticed:** Kiro private ACP extensions should remain isolated behind provider hook options, not spread into orchestration or UI. + +## Turn 1 — 2026-05-20 + +**Outcome:** No open blocking findings remained after the original Kiro provider and appearance review. + +### Findings Resolved + +- F-001: Active ACP prompt registration happened after `turn.started`, leaving a short window where a Kiro follow-up could be routed as a second `session/prompt` instead of `_message/send`. +- F-002: Kiro active-prompt follow-ups are intentionally attached to the existing turn, so the UI local-dispatch guard did not clear when the server acknowledged a follow-up on the same running turn. +- F-003: The mobile collapsed composer send button lost the environment-unavailable disable guard while enabling running follow-ups. +- F-004: ACP interrupt completion was locally raced against `session/prompt`, so an interrupted turn could be marked cancelled before the provider acknowledged prompt termination. +- F-005: ACP interrupt skipped `session/cancel` when no local active prompt was registered, leaving resumed/desynced remote prompts unstoppable. diff --git a/.reviews/pr2793-automation-feedback-review.md b/.reviews/pr2793-automation-feedback-review.md new file mode 100644 index 00000000000..ab9f362d8c7 --- /dev/null +++ b/.reviews/pr2793-automation-feedback-review.md @@ -0,0 +1,198 @@ +# Review: PR 2793 Automation Feedback + +## Project context + +| Field | Value | +| -------------- | ---------------------------------------- | +| **Repository** | `declancowen/t3code` | +| **Remote** | `origin` | +| **Branch** | `codex/kiro-acp-image-errors` | +| **Stack** | TypeScript, Effect, React/Vite, Electron | + +## Scope + +- `packages/effect-acp/src/protocol.ts` — provider-originated ACP request-id normalization and response restoration. +- `packages/shared/src/shell.ts` — POSIX login-shell environment capture. +- `apps/desktop/src/updates/DesktopUpdates.ts` — desktop updater install failure recovery. +- `.github/workflows/desktop-release.yml` — manual desktop release tag/ref resolution. +- `apps/web/src/components/chat/ComposerPrimaryActions.tsx` — compact idle send button rendering. +- `apps/server/src/httpCors.ts` / `apps/server/src/http.ts` — credentialed browser API CORS trust boundary. + +## Hotspots + +- ACP JSON-RPC request identity preservation when Effect RPC requires numeric server request IDs. +- Optional login-shell environment variables under `set -e`. +- Desktop update install handoff partial-failure recovery after the backend is stopped. +- Manual release artifact provenance for versioned desktop releases. +- Compact composer layout parity. +- Credentialed browser API CORS origin reflection. + +## Review status + +| Field | Value | +| --------------------- | -------------------- | +| **Review started** | 2026-05-23 22:15 BST | +| **Last reviewed** | 2026-05-23 22:35 BST | +| **Total turns** | 2 | +| **Open findings** | 0 | +| **Resolved findings** | 6 | +| **Accepted findings** | 0 | + +## Automation + +| Field | Value | +| -------------------- | ---------------------------------- | +| **Mode** | `pr-review-automation` | +| **PR** | `pingdotgg/t3code#2793` | +| **State authority** | GitHub review threads | +| **Review file role** | Human-readable local review ledger | + +## Turn 2 — 2026-05-23 22:35 BST + +| Field | Value | +| --------------- | ------------ | +| **Commit** | working tree | +| **IDE / Agent** | Codex | + +### Automation context + +| Field | Value | +| ------------------------------ | ------------------------------------------ | +| **Trigger** | Fresh `@codex review` after push | +| **PR** | `pingdotgg/t3code#2793` | +| **Base ref** | `main` | +| **Head SHA** | `dd6e413d243d0c521128713c5aaf7ba902b02c2d` | +| **Previous reviewed head SHA** | `657f253c1d2daebc5ad7268d1bf8277d281397c8` | +| **Trusted state source** | `gh api graphql pullRequest.reviewThreads` | +| **Verification policy** | Focused CORS/server tests, then repo gates | + +**Summary:** Imported and fixed Codex's fresh P1 finding that credentialed browser API CORS reflected arbitrary valid HTTP(S) origins. +**Outcome:** Resolved in the current tree; pending commit/push when required gates pass. +**Risk score:** High — this is a credentialed cross-origin API boundary on authenticated local/remote server APIs. +**Change archetypes:** API/transport security, auth-adjacent browser compatibility, hosted/loopback variant hardening. +**Intended change:** Preserve legitimate hosted-app and local-dev browser access while preventing arbitrary websites from receiving credentialed CORS headers. +**Intent vs actual:** The server CORS predicate now accepts only loopback origins and the known hosted app origins (`app.t3.codes`, `latest.app.t3.codes`, `nightly.app.t3.codes`). The global CORS middleware was replaced with a server-owned middleware so `access-control-allow-credentials: true` is emitted only for trusted origins, including preflight responses. +**Confidence:** High for the reviewed boundary and covered variants; hosted custom-domain CORS remains intentionally unsupported without a future explicit configuration surface. +**Coverage note:** Added focused helper coverage for trusted/hostile origins and an integration regression proving untrusted auth failures are not reflected and do not receive credentialed CORS. +**Finding triage:** Live in `dd6e413d`; fixed in the current working tree. +**Static/analyzer evidence:** No analyzer policy changed. Fallow remains unavailable in this repo/PATH. +**Architecture impact:** The credentialed CORS invariant is now owned by `httpCors.ts` and applied at the HTTP transport middleware boundary in `http.ts`; route handlers keep using the same helper instead of duplicating origin policy. +**Bug classes / invariants checked:** auth-adjacent cross-origin trust boundary; loopback vs hosted vs hostile origin variants; credential header emission must be tied to explicit trusted origins. +**Branch totality:** Rechecked the PR automation threads after the first push, resolved stale/outdated prior Codex threads, and imported the only fresh current-SHA Codex finding. +**Sibling closure:** Checked auth success, auth failure, websocket-token preflight, public environment descriptor, and browser OTLP preflight paths that receive browser API CORS headers. +**Remediation impact surface:** Affects all browser API routes through the global route layer. Local loopback dev and known hosted app origins continue to work; arbitrary hosted origins receive no credentialed CORS. +**Residual risk / unknowns:** Custom hosted app origins would need an explicit future server config field rather than falling through to arbitrary origin reflection. + +### External Finding Import + +| Source | Finding | Current status | Bug class | Missed invariant/variant | Action | +| ------ | ---------------------------------------------------- | -------------- | ----------------------------------------- | ------------------------------------------------------- | -------------------------------------------------- | +| Codex | Credentialed CORS reflects arbitrary HTTP(S) origins | resolved | Auth-Adjacent Cross-Origin Trust Boundary | credentials must only be allowed for trusted UI origins | fixed with trusted-origin predicate and middleware | + +### Validation + +- `bun run --cwd apps/server test src/http.test.ts src/server.test.ts` — passed, 70 tests. + +### Branch-totality proof + +- **Non-delta files/systems re-read:** fresh Codex thread, Effect HTTP CORS middleware implementation, server CORS/auth tests, hosted pairing docs, release hosted app domain docs, auth route CORS helper. +- **Prior open findings rechecked:** First five imported findings remain fixed; their old GitHub threads are resolved or outdated. +- **Prior resolved/adjacent areas revalidated:** Macroscope correctness passed on `dd6e413d`; Cursor Bugbot passed on `dd6e413d`; stale Codex ACP/shell threads were resolved after current-tree proof. +- **Hotspots or sibling paths revisited:** credentialed auth responses, auth failures, preflight handling, public descriptor response headers, OTLP trace preflight, local loopback dev redirect helper. +- **Dependency/adjacent surfaces revalidated:** Hosted pairing expects direct backend reachability from the browser, and hosted release docs define the known app origins that remain trusted. +- **Why this is enough:** The original defect was arbitrary-origin reflection with credentials; the current middleware no longer reflects untrusted origins and the regression test asserts the hostile-origin failure path. + +### Challenger pass + +Done — assumed the generic Effect CORS middleware could still leak credential headers even after tightening `allowedOrigins`. The first focused test caught that exact companion issue, so the fix now owns both origin reflection and credential-header emission. + +### Resolved / Carried / New findings + +- `PR2793-CORS-001` — resolved in the current working tree. + - Fingerprint: `apps/server/src/httpCors.ts:isBrowserApiCorsOriginAllowed:credentialed-cors-trust-boundary` + - Evidence: trusted origin allowlist and custom middleware conditional credential headers. + - Verification: `bun run --cwd apps/server test src/http.test.ts src/server.test.ts`. + +### Recommendations + +1. **Fix first:** commit and push the CORS remediation after repo gates pass. +2. **Then address:** monitor PR checks again; only Vercel authorization should remain external if no new findings appear. + +## Turn 1 — 2026-05-23 22:15 BST + +| Field | Value | +| --------------- | ------------ | +| **Commit** | working tree | +| **IDE / Agent** | Codex | + +### Automation context + +| Field | Value | +| ------------------------------ | ---------------------------------------------------- | +| **Trigger** | Manual import of unresolved PR review threads | +| **PR** | `pingdotgg/t3code#2793` | +| **Base ref** | `main` | +| **Head SHA** | `657f253c1d2daebc5ad7268d1bf8277d281397c8` | +| **Previous reviewed head SHA** | `657f253c1d2daebc5ad7268d1bf8277d281397c8` | +| **Trusted state source** | `gh api graphql pullRequest.reviewThreads` | +| **Verification policy** | Focused regressions, full repo tests, format/lint/ts | + +**Summary:** Imported and fixed all five unresolved automated review findings from Cursor Bugbot, Macroscope, and Codex. +**Outcome:** All clear for the imported finding scope. +**Risk score:** High — one finding was a shared ACP transport identity bug, two were partial-failure/release-provenance bugs, and the remaining two touched shared shell/UI behavior. +**Change archetypes:** protocol compatibility, async lifecycle recovery, release automation, shared utility compatibility, shared UI parity. +**Intended change:** Close the unresolved PR findings without moving ownership boundaries or weakening the Kiro image attachment fix. +**Intent vs actual:** The diff addresses the current-tree failure modes: ACP aliases no longer collide with native numeric IDs; missing optional env vars are non-fatal; updater async install errors restart the backend through the same helper as sync failures; manual desktop release builds checkout the commit pointed to by the requested tag; compact composer mode reaches the send button. +**Confidence:** High — each imported finding now has a direct code change plus focused regression coverage where the repo can test it, and the full repo test script passed after repairing the local Electron binary install. +**Coverage note:** Focused tests were added or updated for ACP ID collision, shell env capture command shape, desktop async install error recovery, and compact composer send button rendering. +**Finding triage:** All imported findings were live in the pre-fix tree and resolved in the current tree. +**Static/analyzer evidence:** No analyzer policy changed. Fallow is unavailable in this repo/PATH. `bun lint` exits 0 with existing unrelated warnings in `ChatView.tsx`, `catalog.test.ts`, and `clientPersistenceStorage.ts`. +**Architecture impact:** Ownership remains in the correct layers: ACP transport owns request-id normalization, shared shell owns environment capture, desktop updater owns backend restart policy, GitHub Actions owns release provenance, and composer presentation owns compact button layout. +**Bug classes / invariants checked:** Identity/uniqueness for ACP request IDs; variant state for unset env vars and compact UI; lifecycle/partial failure for updater install errors; contract/provenance for release tag-to-artifact commit mapping. +**Branch totality:** Reviewed the local fix delta plus the branch Kiro/ACP image diff against `origin/main`. Pre-existing dirty `.gitignore` and untracked app-logo/public/release artifacts remain outside this fix scope. +**Sibling closure:** Checked native numeric ID cleanup and alias response restoration, multi-variable env capture, sync and async updater install failure paths, release checkout consumers of `needs.preflight.outputs.ref`, and the only `SendButton` call site. +**Remediation impact surface:** The ACP fix affects all provider-originated core requests using `effect-acp`; the shell fix affects desktop environment hydration; desktop release/update fixes affect manual releases and failed install recovery; composer fix affects compact render surfaces only. +**Residual risk / unknowns:** The workflow tag lookup is script-reviewed and type/lint/test-covered only through repository gates, not by executing GitHub Actions. Live PR automation will re-evaluate after push. + +### External Finding Import + +| Source | Finding | Current status | Bug class | Missed invariant/variant | Action | +| ------------- | ---------------------------------------------------------------------------- | -------------- | ------------------------------------------------------------------ | ---------------------------------------------------------------- | -------------------------------- | +| Cursor Bugbot | Manual desktop release builds `github.sha` instead of the version tag commit | resolved | Contract Encoding | release version/tag must own build provenance | fixed in workflow preflight | +| Macroscope | Async updater install error does not restart stopped backend | resolved | Lifecycle And Transient Containers / Atomicity And Partial Failure | event-driven install failure must mirror sync failure recovery | fixed with shared restart helper | +| Macroscope | Compact composer does not pass compact state to `SendButton` | resolved | Variant State / Affordance Parity | compact mode must reach final idle send control | fixed and covered | +| Codex | ACP aliased request ID can collide with native numeric provider request IDs | resolved | Identity And Uniqueness | internal server request IDs must not overlap active provider IDs | fixed and covered | +| Codex | Login shell `printenv` aborts optional env capture under `set -e` | resolved | Variant State / Compatibility | unset optional vars must be non-fatal | fixed and covered | + +### Validation + +- `bun run --cwd packages/effect-acp test src/protocol.test.ts` — passed, 12 tests. +- `bun run --cwd packages/shared test src/shell.test.ts` — passed, 25 tests. +- `bun run --cwd apps/desktop test src/updates/DesktopUpdates.test.ts` — passed, 8 tests. +- `bun run --cwd apps/web test src/components/chat/ComposerPrimaryActions.test.ts` — passed, 9 tests. +- `bun fmt` — passed. +- `bun lint` — passed with 9 existing warnings. +- `bun typecheck` — passed, 13 packages. +- `bun run test` — passed, 13 packages, after running Electron's package postinstall to restore the missing local Electron binary. + +### Branch-totality proof + +- **Non-delta files/systems re-read:** PR review threads, diff-review gates, architecture enforcement guidance, ACP protocol tests, shell tests, desktop updater tests, composer action tests, desktop release workflow checkout path. +- **Prior open findings rechecked:** All five unresolved GitHub review threads were mapped to current-tree behavior and fixed. +- **Prior resolved/adjacent areas revalidated:** Kiro image attachment handling and provider error surfacing still use the shared ACP adapter/transport path; desktop sync install failure recovery still restarts the backend; composer submit semantics are otherwise unchanged. +- **Hotspots or sibling paths revisited:** ACP alias cleanup vs native ID cleanup, missing vs present shell env vars, sync vs event-driven updater install failures, release workflow tag-push vs manual dispatch, compact vs non-compact send button. +- **Dependency/adjacent surfaces revalidated:** `needs.preflight.outputs.ref` consumers, `DesktopBackendManager.start` restart behavior, `readEnvironmentFromLoginShell` callers, and `ComposerPrimaryActions` idle state rendering. +- **Why this is enough:** The strongest failure modes now have direct regression tests, and the non-testable workflow branch uses the tag commit as the single release provenance source before build checkout. + +### Challenger pass + +- Done — assumed one serious issue remained in request-id aliasing. The current code aliases colliding numeric provider requests instead of letting them pass through, so two active requests cannot share the same internal request ID even when one original ID is already numeric. + +### Resolved / Carried / New findings + +- No open findings remain for this imported PR feedback set. + +### Recommendations + +1. **Fix first:** none. +2. **Then address:** push and let GitHub mark the old review threads outdated or rerun automation on the new commit. diff --git a/apps/desktop/resources/icon.icns b/apps/desktop/resources/icon.icns index da16d12a0c7..782e123f772 100644 Binary files a/apps/desktop/resources/icon.icns and b/apps/desktop/resources/icon.icns differ diff --git a/apps/desktop/resources/icon.ico b/apps/desktop/resources/icon.ico index 8298f70d8b3..d6999105fba 100644 Binary files a/apps/desktop/resources/icon.ico and b/apps/desktop/resources/icon.ico differ diff --git a/apps/desktop/resources/icon.png b/apps/desktop/resources/icon.png index 37f3f756a55..bfaad191b9c 100644 Binary files a/apps/desktop/resources/icon.png and b/apps/desktop/resources/icon.png differ diff --git a/apps/desktop/src/settings/DesktopClientSettings.test.ts b/apps/desktop/src/settings/DesktopClientSettings.test.ts index f666e692860..ca76214f1bf 100644 --- a/apps/desktop/src/settings/DesktopClientSettings.test.ts +++ b/apps/desktop/src/settings/DesktopClientSettings.test.ts @@ -1,6 +1,10 @@ import * as NodeServices from "@effect/platform-node/NodeServices"; import { assert, describe, it } from "@effect/vitest"; -import { ClientSettingsSchema, type ClientSettings } from "@t3tools/contracts"; +import { + ClientSettingsSchema, + DEFAULT_CLIENT_SETTINGS, + type ClientSettings, +} from "@t3tools/contracts"; import * as Effect from "effect/Effect"; import * as FileSystem from "effect/FileSystem"; import * as Layer from "effect/Layer"; @@ -12,6 +16,7 @@ import * as DesktopEnvironment from "../app/DesktopEnvironment.ts"; import * as DesktopClientSettings from "./DesktopClientSettings.ts"; const clientSettings: ClientSettings = { + ...DEFAULT_CLIENT_SETTINGS, autoOpenPlanSidebar: false, confirmThreadArchive: true, confirmThreadDelete: false, diff --git a/apps/desktop/src/updates/DesktopUpdates.test.ts b/apps/desktop/src/updates/DesktopUpdates.test.ts index 34d18f11a77..5e74522d907 100644 --- a/apps/desktop/src/updates/DesktopUpdates.test.ts +++ b/apps/desktop/src/updates/DesktopUpdates.test.ts @@ -9,6 +9,20 @@ import * as Fiber from "effect/Fiber"; import * as Layer from "effect/Layer"; import * as Option from "effect/Option"; import * as TestClock from "effect/testing/TestClock"; +import { vi } from "vitest"; + +vi.mock("electron", () => ({ + app: { focus: () => undefined }, + BrowserWindow: Object.assign( + function BrowserWindow() { + return undefined; + }, + { + getAllWindows: () => [], + getFocusedWindow: () => null, + }, + ), +})); import * as DesktopBackendManager from "../backend/DesktopBackendManager.ts"; import * as DesktopConfig from "../app/DesktopConfig.ts"; @@ -24,6 +38,7 @@ interface UpdatesHarnessOptions { void, ElectronUpdater.ElectronUpdaterCheckForUpdatesError >; + readonly quitAndInstall?: Effect.Effect; readonly env?: Record; } @@ -31,6 +46,10 @@ const flushCallbacks = Effect.yieldNow; function makeHarness(options: UpdatesHarnessOptions = {}) { let checkCount = 0; + let destroyAllCount = 0; + let quitAndInstallCount = 0; + let startCount = 0; + let stopCount = 0; let allowDowngrade = false; const feedUrls: ElectronUpdater.ElectronUpdaterFeedUrl[] = []; const listeners = new Map void>>(); @@ -70,9 +89,12 @@ function makeHarness(options: UpdatesHarnessOptions = {}) { setDisableDifferentialDownload: () => Effect.void, checkForUpdates: Effect.sync(() => { checkCount += 1; - }).pipe(Effect.andThen(options.checkForUpdates ?? Effect.void)), + }).pipe(Effect.flatMap(() => options.checkForUpdates ?? Effect.void)), downloadUpdate: Effect.void, - quitAndInstall: () => Effect.void, + quitAndInstall: () => + Effect.sync(() => { + quitAndInstallCount += 1; + }).pipe(Effect.flatMap(() => options.quitAndInstall ?? Effect.void)), on: (eventName, listener) => Effect.acquireRelease( Effect.sync(() => { @@ -97,13 +119,20 @@ function makeHarness(options: UpdatesHarnessOptions = {}) { Effect.sync(() => { sentStates.push(state as DesktopUpdateState); }), - destroyAll: Effect.void, + destroyAll: Effect.sync(() => { + destroyAllCount += 1; + }), syncAllAppearance: () => Effect.void, } satisfies ElectronWindow.ElectronWindowShape); const backendLayer = Layer.succeed(DesktopBackendManager.DesktopBackendManager, { - start: Effect.void, - stop: () => Effect.void, + start: Effect.sync(() => { + startCount += 1; + }), + stop: () => + Effect.sync(() => { + stopCount += 1; + }), currentConfig: Effect.succeed(Option.none()), snapshot: Effect.succeed({ desiredRunning: false, @@ -159,7 +188,11 @@ function makeHarness(options: UpdatesHarnessOptions = {}) { return { layer, checkCount: () => checkCount, + destroyAllCount: () => destroyAllCount, feedUrls: () => feedUrls, + quitAndInstallCount: () => quitAndInstallCount, + startCount: () => startCount, + stopCount: () => stopCount, listenerCount: () => Array.from(listeners.values()).reduce( (total, eventListeners) => total + eventListeners.size, @@ -222,6 +255,101 @@ describe("DesktopUpdates", () => { ).pipe(Effect.provide(Layer.merge(TestClock.layer(), harness.layer))); }); + it.effect("stops the desktop backend before installing a downloaded update", () => { + const harness = makeHarness(); + + return Effect.scoped( + Effect.gen(function* () { + const updates = yield* DesktopUpdates.DesktopUpdates; + yield* updates.configure; + + harness.emit("update-available", { version: "1.2.4" }); + yield* flushCallbacks; + harness.emit("update-downloaded", { version: "1.2.4" }); + yield* flushCallbacks; + + const result = yield* updates.install; + + assert.equal(result.accepted, true); + assert.equal(result.completed, false); + assert.equal(harness.stopCount(), 1); + assert.equal(harness.quitAndInstallCount(), 1); + assert.equal(harness.destroyAllCount(), 0); + + const duplicateResult = yield* updates.install; + assert.equal(duplicateResult.accepted, false); + assert.equal(harness.stopCount(), 1); + assert.equal(harness.quitAndInstallCount(), 1); + }), + ).pipe(Effect.provide(Layer.merge(TestClock.layer(), harness.layer))); + }); + + it.effect("restarts the desktop backend when the downloaded update install handoff fails", () => { + const error = new ElectronUpdater.ElectronUpdaterQuitAndInstallError({ + cause: new Error("boom"), + }); + const failingHarness = makeHarness({ + quitAndInstall: Effect.fail(error), + }); + + return Effect.scoped( + Effect.gen(function* () { + const updates = yield* DesktopUpdates.DesktopUpdates; + yield* updates.configure; + + failingHarness.emit("update-available", { version: "1.2.4" }); + yield* flushCallbacks; + failingHarness.emit("update-downloaded", { version: "1.2.4" }); + yield* flushCallbacks; + + const result = yield* updates.install; + + assert.equal(result.accepted, true); + assert.equal(result.completed, false); + assert.equal(result.state.status, "downloaded"); + assert.equal(result.state.errorContext, "install"); + assert.equal( + result.state.message, + "Electron updater failed to quit and install the update.", + ); + assert.equal(failingHarness.stopCount(), 1); + assert.equal(failingHarness.quitAndInstallCount(), 1); + assert.equal(failingHarness.startCount(), 1); + }), + ).pipe(Effect.provide(Layer.merge(TestClock.layer(), failingHarness.layer))); + }); + + it.effect("restarts the desktop backend when an updater install error arrives later", () => { + const harness = makeHarness(); + + return Effect.scoped( + Effect.gen(function* () { + const updates = yield* DesktopUpdates.DesktopUpdates; + yield* updates.configure; + + harness.emit("update-available", { version: "1.2.4" }); + yield* flushCallbacks; + harness.emit("update-downloaded", { version: "1.2.4" }); + yield* flushCallbacks; + + const result = yield* updates.install; + assert.equal(result.accepted, true); + assert.equal(result.completed, false); + assert.equal(harness.stopCount(), 1); + assert.equal(harness.startCount(), 0); + + harness.emit("error", new Error("install handoff failed")); + yield* flushCallbacks; + + const state = yield* updates.getState; + assert.equal(state.status, "downloaded"); + assert.equal(state.errorContext, "install"); + assert.equal(state.message, "install handoff failed"); + assert.equal(harness.startCount(), 1); + }), + ).pipe(Effect.provide(Layer.merge(TestClock.layer(), harness.layer))); + }); + it.effect("persists channel changes through the settings service", () => { const harness = makeHarness(); diff --git a/apps/desktop/src/updates/DesktopUpdates.ts b/apps/desktop/src/updates/DesktopUpdates.ts index 8c0acd2e8a6..d5717163f36 100644 --- a/apps/desktop/src/updates/DesktopUpdates.ts +++ b/apps/desktop/src/updates/DesktopUpdates.ts @@ -43,6 +43,7 @@ import { const AUTO_UPDATE_STARTUP_DELAY = "15 seconds"; const AUTO_UPDATE_POLL_INTERVAL = "4 minutes"; +const INSTALL_BACKEND_STOP_TIMEOUT = Duration.seconds(10); const AppUpdateYmlConfig = Schema.Record(Schema.String, Schema.String); type AppUpdateYmlConfig = typeof AppUpdateYmlConfig.Type; @@ -187,8 +188,8 @@ function isArm64HostRunningIntelBuild(runtimeInfo: DesktopRuntimeInfo): boolean const make = Effect.gen(function* () { const config = yield* DesktopConfig.DesktopConfig; - const backendManager = yield* DesktopBackendManager.DesktopBackendManager; const desktopState = yield* DesktopState.DesktopState; + const backendManager = yield* DesktopBackendManager.DesktopBackendManager; const electronUpdater = yield* ElectronUpdater.ElectronUpdater; const electronWindow = yield* ElectronWindow.ElectronWindow; const environment = yield* DesktopEnvironment.DesktopEnvironment; @@ -354,9 +355,22 @@ const make = Effect.gen(function* () { ); }).pipe(Effect.withSpan("desktop.updates.downloadAvailableUpdate")); + const restartBackendAfterInstallFailure = Effect.fn( + "desktop.updates.restartBackendAfterInstallFailure", + )(function* () { + yield* backendManager.start.pipe( + Effect.catchCause((cause) => + logUpdaterError("failed to restart desktop backend after update install failure", { + cause: Cause.pretty(cause), + }), + ), + ); + }); + const installDownloadedUpdate = Effect.gen(function* () { const state = yield* Ref.get(updateStateRef); if ( + (yield* Ref.get(updateInstallInFlightRef)) || (yield* Ref.get(desktopState.quitting)) || !(yield* Ref.get(updaterConfiguredRef)) || state.status !== "downloaded" @@ -364,12 +378,20 @@ const make = Effect.gen(function* () { return { accepted: false, completed: false }; } - yield* Ref.set(desktopState.quitting, true); yield* Ref.set(updateInstallInFlightRef, true); return yield* Effect.gen(function* () { - yield* backendManager.stop({ timeout: Duration.seconds(5) }); - yield* electronWindow.destroyAll; + yield* updateState((current) => ({ + ...current, + message: null, + errorContext: null, + })); + yield* Ref.set(desktopState.quitting, true); + yield* logUpdaterInfo("installing downloaded update", { + version: state.downloadedVersion ?? state.availableVersion ?? "unknown", + }); + yield* logUpdaterInfo("stopping desktop backend before update install"); + yield* backendManager.stop({ timeout: INSTALL_BACKEND_STOP_TIMEOUT }); yield* electronUpdater.quitAndInstall({ isSilent: true, isForceRunAfter: true, @@ -383,6 +405,7 @@ const make = Effect.gen(function* () { reduceDesktopUpdateStateOnInstallFailure(current, error.message), ); yield* Ref.set(desktopState.quitting, false); + yield* restartBackendAfterInstallFailure(); yield* logUpdaterError("failed to install update", { message: error.message }); return { accepted: true, completed: false }; }), @@ -458,6 +481,7 @@ const make = Effect.gen(function* () { yield* Ref.set(updateInstallInFlightRef, false); yield* Ref.set(desktopState.quitting, false); yield* updateState((current) => reduceDesktopUpdateStateOnInstallFailure(current, message)); + yield* restartBackendAfterInstallFailure(); yield* logUpdaterError("updater error", { message }); return; } diff --git a/apps/desktop/src/window/DesktopApplicationMenu.test.ts b/apps/desktop/src/window/DesktopApplicationMenu.test.ts index fc589b3e39b..dc7fb6b9025 100644 --- a/apps/desktop/src/window/DesktopApplicationMenu.test.ts +++ b/apps/desktop/src/window/DesktopApplicationMenu.test.ts @@ -4,9 +4,30 @@ import * as Deferred from "effect/Deferred"; import * as Effect from "effect/Effect"; import * as Layer from "effect/Layer"; import * as Option from "effect/Option"; +import { vi } from "vitest"; import type * as Electron from "electron"; +vi.mock("electron", () => ({ + app: { + on: () => undefined, + removeListener: () => undefined, + }, + Menu: { + buildFromTemplate: () => ({ + popup: () => undefined, + }), + setApplicationMenu: () => undefined, + }, + nativeImage: { + createFromNamedImage: () => ({ + resize: () => ({ + isEmpty: () => true, + }), + }), + }, +})); + import * as ElectronApp from "../electron/ElectronApp.ts"; import * as ElectronDialog from "../electron/ElectronDialog.ts"; import * as ElectronMenu from "../electron/ElectronMenu.ts"; @@ -129,4 +150,98 @@ describe("DesktopApplicationMenu", () => { assert.equal(yield* Deferred.await(selectedAction), "open-settings"); }), ); + + it.effect("shows the stable no-updates dialog from the native menu", () => + Effect.gen(function* () { + const selectedAction = yield* Deferred.make(); + const applicationMenuTemplate = + yield* Deferred.make(); + const dialogOptions = yield* Deferred.make(); + const desktopWindowLayer = Layer.succeed(DesktopWindow.DesktopWindow, { + createMain: Effect.die("unexpected createMain"), + ensureMain: Effect.succeed({} as Electron.BrowserWindow), + revealOrCreateMain: Effect.die("unexpected revealOrCreateMain"), + activate: Effect.void, + createMainIfBackendReady: Effect.void, + handleBackendReady: Effect.void, + dispatchMenuAction: (action) => + Deferred.succeed(selectedAction, action).pipe(Effect.asVoid), + syncAppearance: Effect.void, + } satisfies DesktopWindow.DesktopWindowShape); + const dialogLayer = Layer.succeed(ElectronDialog.ElectronDialog, { + pickFolder: () => Effect.succeed(Option.none()), + confirm: () => Effect.succeed(false), + showMessageBox: (options) => + Deferred.succeed(dialogOptions, options).pipe( + Effect.as({ response: 0, checkboxChecked: false }), + ), + showErrorBox: () => Effect.void, + } satisfies ElectronDialog.ElectronDialogShape); + const updatesLayer = Layer.succeed(DesktopUpdates.DesktopUpdates, { + getState: Effect.die("unexpected getState"), + emitState: Effect.void, + disabledReason: Effect.succeed(Option.none()), + configure: Effect.void, + setChannel: () => Effect.die("unexpected setChannel"), + check: () => + Effect.succeed({ + checked: true, + state: { + enabled: true, + status: "up-to-date", + channel: "latest", + currentVersion: "1.2.3", + hostArch: "arm64", + appArch: "arm64", + runningUnderArm64Translation: false, + availableVersion: null, + downloadedVersion: null, + downloadPercent: null, + checkedAt: "2026-05-21T12:00:00.000Z", + message: null, + errorContext: null, + canRetry: false, + }, + }), + download: Effect.die("unexpected download"), + install: Effect.die("unexpected install"), + } satisfies DesktopUpdates.DesktopUpdatesShape); + + yield* Effect.gen(function* () { + const menu = yield* DesktopApplicationMenu.DesktopApplicationMenu; + yield* menu.configure; + }).pipe( + Effect.provide( + DesktopApplicationMenu.layer.pipe( + Layer.provideMerge(makeElectronMenuLayer(applicationMenuTemplate)), + Layer.provideMerge(desktopWindowLayer), + Layer.provideMerge(updatesLayer), + Layer.provideMerge(dialogLayer), + Layer.provideMerge(electronAppLayer), + Layer.provideMerge( + DesktopEnvironment.layer(environmentInput).pipe( + Layer.provide(Layer.mergeAll(NodeServices.layer, DesktopConfig.layerTest({}))), + ), + ), + ), + ), + ); + + const template = yield* Deferred.await(applicationMenuTemplate); + const helpMenu = template.find((item) => item.role === "help"); + assert.isDefined(helpMenu); + if (!Array.isArray(helpMenu.submenu)) { + throw new Error("Expected Help menu submenu to be an array."); + } + const updateItem = helpMenu.submenu.find((item) => item.label === "Check for Updates..."); + assert.isDefined(updateItem); + const updateClick = updateItem.click; + if (typeof updateClick !== "function") { + throw new Error("Expected Check for Updates menu item to have a click handler."); + } + + updateClick({} as Electron.MenuItem, {} as Electron.BrowserWindow, {} as KeyboardEvent); + assert.equal((yield* Deferred.await(dialogOptions)).title, "No updates available"); + }), + ); }); diff --git a/apps/desktop/src/window/DesktopApplicationMenu.ts b/apps/desktop/src/window/DesktopApplicationMenu.ts index 5e65d81910a..b4f566ca25e 100644 --- a/apps/desktop/src/window/DesktopApplicationMenu.ts +++ b/apps/desktop/src/window/DesktopApplicationMenu.ts @@ -52,7 +52,7 @@ const checkForUpdatesFromMenu: Effect.Effect< if (updateState.status === "up-to-date") { yield* electronDialog.showMessageBox({ type: "info", - title: "You're up to date!", + title: "No updates available", message: `T3 Code ${updateState.currentVersion} is currently the newest version available.`, buttons: ["OK"], }); diff --git a/apps/marketing/public/apple-touch-icon.png b/apps/marketing/public/apple-touch-icon.png index 9c593ce352c..3136c1b44e8 100644 Binary files a/apps/marketing/public/apple-touch-icon.png and b/apps/marketing/public/apple-touch-icon.png differ diff --git a/apps/marketing/public/apple-touch-icon.webp b/apps/marketing/public/apple-touch-icon.webp index fc990561902..5f38f8c7c1a 100644 Binary files a/apps/marketing/public/apple-touch-icon.webp and b/apps/marketing/public/apple-touch-icon.webp differ diff --git a/apps/marketing/public/favicon-16x16.png b/apps/marketing/public/favicon-16x16.png index f85017caf75..27dd3c847aa 100644 Binary files a/apps/marketing/public/favicon-16x16.png and b/apps/marketing/public/favicon-16x16.png differ diff --git a/apps/marketing/public/favicon-16x16.webp b/apps/marketing/public/favicon-16x16.webp index b09dc155aef..d317fee5d97 100644 Binary files a/apps/marketing/public/favicon-16x16.webp and b/apps/marketing/public/favicon-16x16.webp differ diff --git a/apps/marketing/public/favicon-32x32.png b/apps/marketing/public/favicon-32x32.png index fae2d285b7a..93769ba3001 100644 Binary files a/apps/marketing/public/favicon-32x32.png and b/apps/marketing/public/favicon-32x32.png differ diff --git a/apps/marketing/public/favicon-32x32.webp b/apps/marketing/public/favicon-32x32.webp index 72c9243de42..539a0c9c416 100644 Binary files a/apps/marketing/public/favicon-32x32.webp and b/apps/marketing/public/favicon-32x32.webp differ diff --git a/apps/marketing/public/favicon.ico b/apps/marketing/public/favicon.ico index e3ab4ae5e02..d6999105fba 100644 Binary files a/apps/marketing/public/favicon.ico and b/apps/marketing/public/favicon.ico differ diff --git a/apps/marketing/public/icon.png b/apps/marketing/public/icon.png index 0a6e1cbfcff..91e79035cc9 100644 Binary files a/apps/marketing/public/icon.png and b/apps/marketing/public/icon.png differ diff --git a/apps/marketing/public/icon.webp b/apps/marketing/public/icon.webp index aa6826dd36d..01673e1c76e 100644 Binary files a/apps/marketing/public/icon.webp and b/apps/marketing/public/icon.webp differ diff --git a/apps/server/src/auth/http.ts b/apps/server/src/auth/http.ts index 670ff5abbff..bfba0fba249 100644 --- a/apps/server/src/auth/http.ts +++ b/apps/server/src/auth/http.ts @@ -14,10 +14,14 @@ import { HttpRouter, HttpServerRequest, HttpServerResponse } from "effect/unstab import { AuthError, ServerAuth } from "./Services/ServerAuth.ts"; import { SessionCredentialService } from "./Services/SessionCredentialService.ts"; import { deriveAuthClientMetadata } from "./utils.ts"; -import { browserApiCorsHeaders } from "../httpCors.ts"; +import { browserApiCorsHeadersForOrigin } from "../httpCors.ts"; + +const authCorsHeadersForRequest = (request: HttpServerRequest.HttpServerRequest) => + browserApiCorsHeadersForOrigin(request.headers.origin); export const respondToAuthError = (error: AuthError) => Effect.gen(function* () { + const request = yield* HttpServerRequest.HttpServerRequest; if ((error.status ?? 500) >= 500) { yield* Effect.logError("auth route failed", { message: error.message, @@ -28,7 +32,7 @@ export const respondToAuthError = (error: AuthError) => { error: error.message, }, - { status: error.status ?? 500, headers: browserApiCorsHeaders }, + { status: error.status ?? 500, headers: authCorsHeadersForRequest(request) }, ); }); @@ -41,7 +45,7 @@ export const authSessionRouteLayer = HttpRouter.add( const session = yield* serverAuth.getSessionState(request); return HttpServerResponse.jsonUnsafe(session, { status: 200, - headers: browserApiCorsHeaders, + headers: authCorsHeadersForRequest(request), }); }), ); @@ -87,7 +91,7 @@ export const authBootstrapRouteLayer = HttpRouter.add( return yield* HttpServerResponse.jsonUnsafe(result.response, { status: 200, - headers: browserApiCorsHeaders, + headers: authCorsHeadersForRequest(request), }).pipe( HttpServerResponse.setCookie(sessions.cookieName, result.sessionToken, { expires: DateTime.toDate(result.response.expiresAt), @@ -121,7 +125,7 @@ export const authBearerBootstrapRouteLayer = HttpRouter.add( ); return HttpServerResponse.jsonUnsafe(result satisfies AuthBearerBootstrapResult, { status: 200, - headers: browserApiCorsHeaders, + headers: authCorsHeadersForRequest(request), }); }).pipe(Effect.catchTag("AuthError", (error) => respondToAuthError(error))), ); @@ -136,7 +140,7 @@ export const authWebSocketTokenRouteLayer = HttpRouter.add( const result = yield* serverAuth.issueWebSocketToken(session); return HttpServerResponse.jsonUnsafe(result satisfies AuthWebSocketTokenResult, { status: 200, - headers: browserApiCorsHeaders, + headers: authCorsHeadersForRequest(request), }); }).pipe(Effect.catchTag("AuthError", (error) => respondToAuthError(error))), ); @@ -177,7 +181,10 @@ export const authPairingCredentialRouteLayer = HttpRouter.add( ) : {}; const result = yield* serverAuth.issuePairingCredential(payload); - return HttpServerResponse.jsonUnsafe(result, { status: 200 }); + return HttpServerResponse.jsonUnsafe(result, { + status: 200, + headers: authCorsHeadersForRequest(request), + }); }).pipe(Effect.catchTag("AuthError", (error) => respondToAuthError(error))), ); @@ -191,16 +198,19 @@ const authenticateOwnerSession = Effect.gen(function* () { status: 403, }); } - return { serverAuth, session } as const; + return { request, serverAuth, session } as const; }); export const authPairingLinksRouteLayer = HttpRouter.add( "GET", "/api/auth/pairing-links", Effect.gen(function* () { - const { serverAuth } = yield* authenticateOwnerSession; + const { request, serverAuth } = yield* authenticateOwnerSession; const pairingLinks = yield* serverAuth.listPairingLinks(); - return HttpServerResponse.jsonUnsafe(pairingLinks, { status: 200 }); + return HttpServerResponse.jsonUnsafe(pairingLinks, { + status: 200, + headers: authCorsHeadersForRequest(request), + }); }).pipe(Effect.catchTag("AuthError", (error) => respondToAuthError(error))), ); @@ -208,7 +218,7 @@ export const authPairingLinksRevokeRouteLayer = HttpRouter.add( "POST", "/api/auth/pairing-links/revoke", Effect.gen(function* () { - const { serverAuth } = yield* authenticateOwnerSession; + const { request, serverAuth } = yield* authenticateOwnerSession; const payload = yield* HttpServerRequest.schemaBodyJson(AuthRevokePairingLinkInput).pipe( Effect.mapError( (cause) => @@ -220,7 +230,13 @@ export const authPairingLinksRevokeRouteLayer = HttpRouter.add( ), ); const revoked = yield* serverAuth.revokePairingLink(payload.id); - return HttpServerResponse.jsonUnsafe({ revoked }, { status: 200 }); + return HttpServerResponse.jsonUnsafe( + { revoked }, + { + status: 200, + headers: authCorsHeadersForRequest(request), + }, + ); }).pipe(Effect.catchTag("AuthError", (error) => respondToAuthError(error))), ); @@ -228,9 +244,12 @@ export const authClientsRouteLayer = HttpRouter.add( "GET", "/api/auth/clients", Effect.gen(function* () { - const { serverAuth, session } = yield* authenticateOwnerSession; + const { request, serverAuth, session } = yield* authenticateOwnerSession; const clients = yield* serverAuth.listClientSessions(session.sessionId); - return HttpServerResponse.jsonUnsafe(clients, { status: 200 }); + return HttpServerResponse.jsonUnsafe(clients, { + status: 200, + headers: authCorsHeadersForRequest(request), + }); }).pipe(Effect.catchTag("AuthError", (error) => respondToAuthError(error))), ); @@ -238,7 +257,7 @@ export const authClientsRevokeRouteLayer = HttpRouter.add( "POST", "/api/auth/clients/revoke", Effect.gen(function* () { - const { serverAuth, session } = yield* authenticateOwnerSession; + const { request, serverAuth, session } = yield* authenticateOwnerSession; const payload = yield* HttpServerRequest.schemaBodyJson(AuthRevokeClientSessionInput).pipe( Effect.mapError( (cause) => @@ -250,7 +269,13 @@ export const authClientsRevokeRouteLayer = HttpRouter.add( ), ); const revoked = yield* serverAuth.revokeClientSession(session.sessionId, payload.sessionId); - return HttpServerResponse.jsonUnsafe({ revoked }, { status: 200 }); + return HttpServerResponse.jsonUnsafe( + { revoked }, + { + status: 200, + headers: authCorsHeadersForRequest(request), + }, + ); }).pipe(Effect.catchTag("AuthError", (error) => respondToAuthError(error))), ); @@ -258,8 +283,14 @@ export const authClientsRevokeOthersRouteLayer = HttpRouter.add( "POST", "/api/auth/clients/revoke-others", Effect.gen(function* () { - const { serverAuth, session } = yield* authenticateOwnerSession; + const { request, serverAuth, session } = yield* authenticateOwnerSession; const revokedCount = yield* serverAuth.revokeOtherClientSessions(session.sessionId); - return HttpServerResponse.jsonUnsafe({ revokedCount }, { status: 200 }); + return HttpServerResponse.jsonUnsafe( + { revokedCount }, + { + status: 200, + headers: authCorsHeadersForRequest(request), + }, + ); }).pipe(Effect.catchTag("AuthError", (error) => respondToAuthError(error))), ); diff --git a/apps/server/src/http.test.ts b/apps/server/src/http.test.ts index de861cc6645..dafd04db23b 100644 --- a/apps/server/src/http.test.ts +++ b/apps/server/src/http.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "vitest"; import { isLoopbackHostname, resolveDevRedirectUrl } from "./http.ts"; +import { isBrowserApiCorsOriginAllowed } from "./httpCors.ts"; describe("http dev routing", () => { it("treats localhost and loopback addresses as local", () => { @@ -24,4 +25,18 @@ describe("http dev routing", () => { "http://127.0.0.1:5173/pair?token=test-token", ); }); + + it("allows credentialed browser API CORS only for loopback and hosted app origins", () => { + expect(isBrowserApiCorsOriginAllowed("http://localhost:5173")).toBe(true); + expect(isBrowserApiCorsOriginAllowed("http://127.0.0.1:5173")).toBe(true); + expect(isBrowserApiCorsOriginAllowed("http://[::1]:5173")).toBe(true); + expect(isBrowserApiCorsOriginAllowed("https://app.t3.codes")).toBe(true); + expect(isBrowserApiCorsOriginAllowed("https://latest.app.t3.codes")).toBe(true); + expect(isBrowserApiCorsOriginAllowed("https://nightly.app.t3.codes")).toBe(true); + + expect(isBrowserApiCorsOriginAllowed("https://evil.example")).toBe(false); + expect(isBrowserApiCorsOriginAllowed("http://remote-client.test:3773")).toBe(false); + expect(isBrowserApiCorsOriginAllowed("https://app.t3.codes.evil.example")).toBe(false); + expect(isBrowserApiCorsOriginAllowed("https://user@app.t3.codes")).toBe(false); + }); }); diff --git a/apps/server/src/http.ts b/apps/server/src/http.ts index f287b4c59bf..a59df6f88a5 100644 --- a/apps/server/src/http.ts +++ b/apps/server/src/http.ts @@ -10,6 +10,8 @@ import { HttpBody, HttpClient, HttpClientResponse, + HttpEffect, + HttpMiddleware, HttpRouter, HttpServerResponse, HttpServerRequest, @@ -29,29 +31,42 @@ import { ServerAuth } from "./auth/Services/ServerAuth.ts"; import { respondToAuthError } from "./auth/http.ts"; import { ServerEnvironment } from "./environment/Services/ServerEnvironment.ts"; import { - browserApiCorsAllowedHeaders, - browserApiCorsAllowedMethods, + browserApiCorsHeadersForOrigin, + browserApiCorsPreflightHeadersForOrigin, browserApiCorsHeaders, + isLoopbackHostname, } from "./httpCors.ts"; const PROJECT_FAVICON_CACHE_CONTROL = "public, max-age=3600"; const FALLBACK_PROJECT_FAVICON_SVG = ``; const OTLP_TRACES_PROXY_PATH = "/api/observability/v1/traces"; -const LOOPBACK_HOSTNAMES = new Set(["127.0.0.1", "::1", "localhost"]); -export const browserApiCorsLayer = HttpRouter.cors({ - allowedMethods: [...browserApiCorsAllowedMethods], - allowedHeaders: [...browserApiCorsAllowedHeaders], - maxAge: 600, -}); +export { isLoopbackHostname } from "./httpCors.ts"; -export function isLoopbackHostname(hostname: string): boolean { - const normalizedHostname = hostname - .trim() - .toLowerCase() - .replace(/^\[(.*)\]$/, "$1"); - return LOOPBACK_HOSTNAMES.has(normalizedHostname); -} +export const browserApiCorsLayer = HttpRouter.middleware( + HttpMiddleware.make((httpApp) => + Effect.gen(function* () { + const request = yield* HttpServerRequest.HttpServerRequest; + if (request.method === "OPTIONS") { + return HttpServerResponse.empty({ + status: 204, + headers: browserApiCorsPreflightHeadersForOrigin(request.headers.origin), + }); + } + + yield* HttpEffect.appendPreResponseHandler((request, response) => + Effect.succeed( + HttpServerResponse.setHeaders( + response, + browserApiCorsHeadersForOrigin(request.headers.origin), + ), + ), + ); + return yield* httpApp; + }), + ), + { global: true }, +); export function resolveDevRedirectUrl(devUrl: URL, requestUrl: URL): string { const redirectUrl = new URL(devUrl.toString()); diff --git a/apps/server/src/httpCors.ts b/apps/server/src/httpCors.ts index e44486d3c4b..2c5a41f3f5b 100644 --- a/apps/server/src/httpCors.ts +++ b/apps/server/src/httpCors.ts @@ -6,8 +6,70 @@ export const browserApiCorsAllowedHeaders = [ "content-type", ] as const; +const LOOPBACK_HOSTNAMES = new Set(["127.0.0.1", "::1", "localhost"]); +const BROWSER_API_TRUSTED_ORIGINS = new Set([ + "https://app.t3.codes", + "https://latest.app.t3.codes", + "https://nightly.app.t3.codes", +]); + export const browserApiCorsHeaders = { "access-control-allow-origin": "*", "access-control-allow-methods": browserApiCorsAllowedMethods.join(", "), "access-control-allow-headers": browserApiCorsAllowedHeaders.join(", "), } as const; +export const browserApiCorsMaxAgeSeconds = 600; + +export function isLoopbackHostname(hostname: string): boolean { + const normalizedHostname = hostname + .trim() + .toLowerCase() + .replace(/^\[(.*)\]$/, "$1"); + return LOOPBACK_HOSTNAMES.has(normalizedHostname); +} + +export function isBrowserApiCorsOriginAllowed(origin: string | undefined): origin is string { + if (origin === undefined || origin.trim().length === 0) { + return false; + } + + try { + const url = new URL(origin); + if ( + url.origin !== origin || + (url.protocol !== "http:" && url.protocol !== "https:") || + url.username.length !== 0 || + url.password.length !== 0 + ) { + return false; + } + + return isLoopbackHostname(url.hostname) || BROWSER_API_TRUSTED_ORIGINS.has(url.origin); + } catch { + return false; + } +} + +export function browserApiCorsHeadersForOrigin(origin: string | undefined) { + if (!isBrowserApiCorsOriginAllowed(origin)) { + return browserApiCorsHeaders; + } + + return { + ...browserApiCorsHeaders, + "access-control-allow-origin": origin, + "access-control-allow-credentials": "true", + vary: "Origin", + } as const; +} + +export function browserApiCorsPreflightHeadersForOrigin(origin: string | undefined) { + if (!isBrowserApiCorsOriginAllowed(origin)) { + return {}; + } + + return { + ...browserApiCorsHeadersForOrigin(origin), + "access-control-max-age": String(browserApiCorsMaxAgeSeconds), + } as const; +} diff --git a/apps/server/src/localUserEnvironment.test.ts b/apps/server/src/localUserEnvironment.test.ts new file mode 100644 index 00000000000..aee469087ad --- /dev/null +++ b/apps/server/src/localUserEnvironment.test.ts @@ -0,0 +1,22 @@ +import * as NodeOS from "node:os"; +import { describe, expect, it } from "vitest"; + +import { resolveLocalUserHome, withLocalUserHome } from "./localUserEnvironment.ts"; + +describe("localUserEnvironment", () => { + it("preserves normal HOME values", () => { + const env = { HOME: "/Users/someone" }; + + expect(resolveLocalUserHome(env)).toBe("/Users/someone"); + expect(withLocalUserHome(env)).toBe(env); + }); + + it("maps the Codex-launched T3 temp HOME back to the OS account home", () => { + const env = { HOME: "/private/tmp/t3code-home" }; + + expect(resolveLocalUserHome(env)).toBe(NodeOS.userInfo().homedir); + expect(withLocalUserHome(env)).toEqual({ + HOME: NodeOS.userInfo().homedir, + }); + }); +}); diff --git a/apps/server/src/localUserEnvironment.ts b/apps/server/src/localUserEnvironment.ts new file mode 100644 index 00000000000..81e41c1e123 --- /dev/null +++ b/apps/server/src/localUserEnvironment.ts @@ -0,0 +1,27 @@ +import * as NodeOS from "node:os"; + +const T3CODE_TEMP_HOME_PATTERN = /^\/(?:private\/)?tmp\/t3code-home(?:\/|$)/; + +function shouldUseOsAccountHome(home: string | undefined): boolean { + return typeof home === "string" && T3CODE_TEMP_HOME_PATTERN.test(home); +} + +export function resolveLocalUserHome(baseEnv: NodeJS.ProcessEnv = process.env): string | undefined { + if (!shouldUseOsAccountHome(baseEnv.HOME)) { + return baseEnv.HOME; + } + + return NodeOS.userInfo().homedir || baseEnv.HOME; +} + +export function withLocalUserHome(baseEnv: NodeJS.ProcessEnv = process.env): NodeJS.ProcessEnv { + const localHome = resolveLocalUserHome(baseEnv); + if (!localHome || localHome === baseEnv.HOME) { + return baseEnv; + } + + return { + ...baseEnv, + HOME: localHome, + }; +} diff --git a/apps/server/src/provider/Drivers/KiroDriver.ts b/apps/server/src/provider/Drivers/KiroDriver.ts new file mode 100644 index 00000000000..7a27b9d5776 --- /dev/null +++ b/apps/server/src/provider/Drivers/KiroDriver.ts @@ -0,0 +1,166 @@ +/** + * KiroDriver — `ProviderDriver` for Kiro CLI's ACP runtime. + * + * Kiro exposes Agent Client Protocol over `kiro-cli acp`, so the driver + * composes the existing provider instance model with the shared ACP adapter + * instead of introducing a provider-specific transport path. + * + * @module provider/Drivers/KiroDriver + */ +import { KiroSettings, ProviderDriverKind, type ServerProvider } from "@t3tools/contracts"; +import * as Duration from "effect/Duration"; +import * as Effect from "effect/Effect"; +import * as FileSystem from "effect/FileSystem"; +import * as Path from "effect/Path"; +import * as Schema from "effect/Schema"; +import * as Stream from "effect/Stream"; +import { HttpClient } from "effect/unstable/http"; +import { ChildProcessSpawner } from "effect/unstable/process"; + +import { ServerConfig } from "../../config.ts"; +import { withLocalUserHome } from "../../localUserEnvironment.ts"; +import { makeKiroTextGeneration } from "../../textGeneration/KiroTextGeneration.ts"; +import { ProviderDriverError } from "../Errors.ts"; +import { makeKiroAdapter } from "../Layers/KiroAdapter.ts"; +import { checkKiroProviderStatus, makePendingKiroProvider } from "../Layers/KiroProvider.ts"; +import { ProviderEventLoggers } from "../Layers/ProviderEventLoggers.ts"; +import { makeManagedServerProvider } from "../makeManagedServerProvider.ts"; +import { + defaultProviderContinuationIdentity, + type ProviderDriver, + type ProviderInstance, +} from "../ProviderDriver.ts"; +import type { ServerProviderDraft } from "../providerSnapshot.ts"; +import { mergeProviderInstanceEnvironment } from "../ProviderInstanceEnvironment.ts"; +import { + enrichProviderSnapshotWithVersionAdvisory, + makeStaticProviderMaintenanceResolver, + makeProviderMaintenanceCapabilities, + resolveProviderMaintenanceCapabilitiesEffect, +} from "../providerMaintenance.ts"; +import { makeKiroContinuationGroupKey, makeKiroEnvironment } from "./KiroHome.ts"; + +const decodeKiroSettings = Schema.decodeSync(KiroSettings); + +const DRIVER_KIND = ProviderDriverKind.make("kiro"); +const SNAPSHOT_REFRESH_INTERVAL = Duration.minutes(5); +const UPDATE = makeStaticProviderMaintenanceResolver( + makeProviderMaintenanceCapabilities({ + provider: DRIVER_KIND, + packageName: null, + updateExecutable: "kiro-cli", + updateArgs: ["update", "--non-interactive"], + updateLockKey: "kiro-cli", + }), +); + +export type KiroDriverEnv = + | ChildProcessSpawner.ChildProcessSpawner + | FileSystem.FileSystem + | HttpClient.HttpClient + | Path.Path + | ProviderEventLoggers + | ServerConfig; + +const withInstanceIdentity = + (input: { + readonly instanceId: ProviderInstance["instanceId"]; + readonly displayName: string | undefined; + readonly accentColor: string | undefined; + readonly continuationGroupKey: string; + }) => + (snapshot: ServerProviderDraft): ServerProvider => ({ + ...snapshot, + instanceId: input.instanceId, + driver: DRIVER_KIND, + ...(input.displayName ? { displayName: input.displayName } : {}), + ...(input.accentColor ? { accentColor: input.accentColor } : {}), + continuation: { groupKey: input.continuationGroupKey }, + }); + +export const KiroDriver: ProviderDriver = { + driverKind: DRIVER_KIND, + metadata: { + displayName: "Kiro", + supportsMultipleInstances: true, + }, + configSchema: KiroSettings, + defaultConfig: (): KiroSettings => decodeKiroSettings({}), + create: ({ instanceId, displayName, accentColor, environment, enabled, config }) => + Effect.gen(function* () { + const spawner = yield* ChildProcessSpawner.ChildProcessSpawner; + const httpClient = yield* HttpClient.HttpClient; + const eventLoggers = yield* ProviderEventLoggers; + const baseEnv = withLocalUserHome(mergeProviderInstanceEnvironment(environment)); + const effectiveConfig = { ...config, enabled } satisfies KiroSettings; + const processEnv = yield* makeKiroEnvironment(effectiveConfig, baseEnv); + const fallbackContinuationIdentity = defaultProviderContinuationIdentity({ + driverKind: DRIVER_KIND, + instanceId, + }); + const maintenanceCapabilities = yield* resolveProviderMaintenanceCapabilitiesEffect(UPDATE, { + binaryPath: effectiveConfig.binaryPath, + env: processEnv, + }); + const continuationGroupKey = yield* makeKiroContinuationGroupKey(effectiveConfig, baseEnv); + const stampIdentity = withInstanceIdentity({ + instanceId, + displayName, + accentColor, + continuationGroupKey, + }); + + const adapter = yield* makeKiroAdapter(effectiveConfig, { + instanceId, + environment: processEnv, + ...(eventLoggers.native ? { nativeEventLogger: eventLoggers.native } : {}), + }); + const textGeneration = yield* makeKiroTextGeneration(effectiveConfig, processEnv); + + const checkProvider = checkKiroProviderStatus(effectiveConfig, processEnv).pipe( + Effect.map(stampIdentity), + Effect.provideService(ChildProcessSpawner.ChildProcessSpawner, spawner), + ); + + const snapshot = yield* makeManagedServerProvider({ + maintenanceCapabilities, + getSettings: Effect.succeed(effectiveConfig), + streamSettings: Stream.never, + haveSettingsChanged: () => false, + initialSnapshot: (settings) => + makePendingKiroProvider(settings).pipe(Effect.map(stampIdentity)), + checkProvider, + enrichSnapshot: ({ snapshot, publishSnapshot }) => + enrichProviderSnapshotWithVersionAdvisory(snapshot, maintenanceCapabilities).pipe( + Effect.provideService(HttpClient.HttpClient, httpClient), + Effect.flatMap((enrichedSnapshot) => publishSnapshot(enrichedSnapshot)), + ), + refreshInterval: SNAPSHOT_REFRESH_INTERVAL, + }).pipe( + Effect.mapError( + (cause) => + new ProviderDriverError({ + driver: DRIVER_KIND, + instanceId, + detail: `Failed to build Kiro snapshot: ${cause.message ?? String(cause)}`, + cause, + }), + ), + ); + + return { + instanceId, + driverKind: DRIVER_KIND, + continuationIdentity: { + ...fallbackContinuationIdentity, + continuationKey: continuationGroupKey, + }, + displayName, + accentColor, + enabled, + snapshot, + adapter, + textGeneration, + } satisfies ProviderInstance; + }), +}; diff --git a/apps/server/src/provider/Drivers/KiroHome.test.ts b/apps/server/src/provider/Drivers/KiroHome.test.ts new file mode 100644 index 00000000000..682cac75b5c --- /dev/null +++ b/apps/server/src/provider/Drivers/KiroHome.test.ts @@ -0,0 +1,55 @@ +import * as NodeOS from "node:os"; + +import * as NodeServices from "@effect/platform-node/NodeServices"; +import { describe, expect, it } from "@effect/vitest"; +import * as Effect from "effect/Effect"; +import * as Path from "effect/Path"; + +import { + makeKiroContinuationGroupKey, + makeKiroEnvironment, + resolveKiroHomePath, +} from "./KiroHome.ts"; + +it.layer(NodeServices.layer)("KiroHome", (it) => { + describe("Kiro home resolution", () => { + it.effect("uses the process Kiro home when no override is configured", () => + Effect.gen(function* () { + const path = yield* Path.Path; + const resolved = path.resolve(NodeOS.userInfo().homedir, ".kiro"); + + expect(yield* resolveKiroHomePath({ homePath: "" })).toBe(resolved); + expect(yield* makeKiroEnvironment({ homePath: "" })).toBe(process.env); + expect(yield* makeKiroContinuationGroupKey({ homePath: "" })).toBe(`kiro:home:${resolved}`); + }), + ); + + it.effect( + "uses the OS account home when the server was launched with the Codex temp home", + () => + Effect.gen(function* () { + const path = yield* Path.Path; + const env = { HOME: "/private/tmp/t3code-home" }; + const resolved = path.resolve(NodeOS.userInfo().homedir, ".kiro"); + + expect(yield* resolveKiroHomePath({ homePath: "" }, env)).toBe(resolved); + expect(yield* resolveKiroHomePath({ homePath: "~/.kiro" }, env)).toBe(resolved); + expect(yield* makeKiroContinuationGroupKey({ homePath: "" }, env)).toBe( + `kiro:home:${resolved}`, + ); + }), + ); + + it.effect("sets KIRO_HOME and stamps continuation keys with the configured home", () => + Effect.gen(function* () { + const path = yield* Path.Path; + const homePath = "~/.kiro-work"; + const resolved = path.resolve(NodeOS.userInfo().homedir, ".kiro-work"); + + expect(yield* resolveKiroHomePath({ homePath })).toBe(resolved); + expect((yield* makeKiroEnvironment({ homePath })).KIRO_HOME).toBe(resolved); + expect(yield* makeKiroContinuationGroupKey({ homePath })).toBe(`kiro:home:${resolved}`); + }), + ); + }); +}); diff --git a/apps/server/src/provider/Drivers/KiroHome.ts b/apps/server/src/provider/Drivers/KiroHome.ts new file mode 100644 index 00000000000..0565ece9823 --- /dev/null +++ b/apps/server/src/provider/Drivers/KiroHome.ts @@ -0,0 +1,49 @@ +import * as NodeOS from "node:os"; + +import type { KiroSettings } from "@t3tools/contracts"; +import * as Effect from "effect/Effect"; +import * as Path from "effect/Path"; + +import { resolveLocalUserHome } from "../../localUserEnvironment.ts"; + +function expandKiroHomePath(value: string, baseEnv: NodeJS.ProcessEnv): string { + if (!value) return value; + const localHome = resolveLocalUserHome(baseEnv) ?? NodeOS.homedir(); + if (value === "~") return localHome; + if (value.startsWith("~/") || value.startsWith("~\\")) { + return `${localHome}/${value.slice(2)}`; + } + return value; +} + +export const resolveKiroHomePath = Effect.fn("resolveKiroHomePath")(function* ( + config: Pick, + baseEnv: NodeJS.ProcessEnv = process.env, +): Effect.fn.Return { + const path = yield* Path.Path; + const homePath = config.homePath.trim(); + return homePath.length > 0 + ? path.resolve(expandKiroHomePath(homePath, baseEnv)) + : path.resolve(resolveLocalUserHome(baseEnv) ?? NodeOS.homedir(), ".kiro"); +}); + +export const makeKiroEnvironment = Effect.fn("makeKiroEnvironment")(function* ( + config: Pick, + baseEnv: NodeJS.ProcessEnv = process.env, +): Effect.fn.Return { + const homePath = config.homePath.trim(); + if (homePath.length === 0) return baseEnv; + const resolvedHomePath = yield* resolveKiroHomePath(config, baseEnv); + return { + ...baseEnv, + KIRO_HOME: resolvedHomePath, + }; +}); + +export const makeKiroContinuationGroupKey = Effect.fn("makeKiroContinuationGroupKey")(function* ( + config: Pick, + baseEnv: NodeJS.ProcessEnv = process.env, +): Effect.fn.Return { + const resolvedHomePath = yield* resolveKiroHomePath(config, baseEnv); + return `kiro:home:${resolvedHomePath}`; +}); diff --git a/apps/server/src/provider/Layers/CursorAdapter.ts b/apps/server/src/provider/Layers/CursorAdapter.ts index efef5f0a83b..6ca679cce55 100644 --- a/apps/server/src/provider/Layers/CursorAdapter.ts +++ b/apps/server/src/provider/Layers/CursorAdapter.ts @@ -673,7 +673,7 @@ export function makeCursorAdapter( ? ({ outcome: "cancelled" } as const) : { outcome: "selected" as const, - optionId: acpPermissionOutcome(resolved), + optionId: acpPermissionOutcome(resolved, params.options), }, }; }), diff --git a/apps/server/src/provider/Layers/KiroAdapter.ts b/apps/server/src/provider/Layers/KiroAdapter.ts new file mode 100644 index 00000000000..16439da34b9 --- /dev/null +++ b/apps/server/src/provider/Layers/KiroAdapter.ts @@ -0,0 +1,47 @@ +import { type KiroSettings, ProviderDriverKind, type ProviderInstanceId } from "@t3tools/contracts"; + +import { makeKiroAcpRuntime } from "../acp/KiroAcpSupport.ts"; +import { makeStandardAcpAdapter } from "../acp/StandardAcpAdapter.ts"; +import { type EventNdjsonLogger } from "./EventNdjsonLogger.ts"; + +const PROVIDER = ProviderDriverKind.make("kiro"); +const KIRO_ACTIVE_PROMPT_MESSAGE_METHOD = "_message/send"; +const SUPPORTED_KIRO_IMAGE_MIME_TYPES = new Set([ + "image/gif", + "image/jpeg", + "image/png", + "image/webp", +]); + +export interface KiroAdapterLiveOptions { + readonly environment?: NodeJS.ProcessEnv; + readonly nativeEventLogPath?: string; + readonly nativeEventLogger?: EventNdjsonLogger; + readonly instanceId?: ProviderInstanceId; +} + +export function makeKiroAdapter(kiroSettings: KiroSettings, options?: KiroAdapterLiveOptions) { + return makeStandardAcpAdapter({ + provider: PROVIDER, + runtimeLabel: "Kiro", + ...(options?.environment ? { environment: options.environment } : {}), + ...(options?.nativeEventLogPath ? { nativeEventLogPath: options.nativeEventLogPath } : {}), + ...(options?.nativeEventLogger ? { nativeEventLogger: options.nativeEventLogger } : {}), + ...(options?.instanceId ? { instanceId: options.instanceId } : {}), + activePromptMessageMethod: KIRO_ACTIVE_PROMPT_MESSAGE_METHOD, + supportedImageMimeTypes: SUPPORTED_KIRO_IMAGE_MIME_TYPES, + stopSessionOnInterruptCancelUnsupported: true, + sendMessageWhilePromptActive: ({ runtime, sessionId, content, contentBlocks }) => + runtime.request(KIRO_ACTIVE_PROMPT_MESSAGE_METHOD, { + sessionId, + content: + contentBlocks.length === 1 && contentBlocks[0]?.type === "text" ? content : contentBlocks, + }), + makeRuntime: (input) => + makeKiroAcpRuntime({ + kiroSettings, + ...(options?.environment ? { environment: options.environment } : {}), + ...input, + }), + }); +} diff --git a/apps/server/src/provider/Layers/KiroProvider.test.ts b/apps/server/src/provider/Layers/KiroProvider.test.ts new file mode 100644 index 00000000000..85ce09eef75 --- /dev/null +++ b/apps/server/src/provider/Layers/KiroProvider.test.ts @@ -0,0 +1,64 @@ +import { describe, expect, it } from "vitest"; +import { createModelCapabilities } from "@t3tools/shared/model"; + +import { parseKiroListModelsOutput } from "./KiroProvider.ts"; + +const emptyCapabilities = createModelCapabilities({ optionDescriptors: [] }); + +describe("parseKiroListModelsOutput", () => { + it("publishes Kiro CLI model choices from chat --list-models json", () => { + expect( + parseKiroListModelsOutput({ + code: 0, + stderr: "", + stdout: JSON.stringify({ + models: [ + { + model_name: "auto", + model_id: "auto", + description: "Models chosen by task", + context_window_tokens: 1_000_000, + }, + { + model_name: "claude-opus-4.7", + model_id: "claude-opus-4.7", + description: "Experimental preview", + context_window_tokens: 1_000_000, + }, + ], + default_model: "auto", + }), + }), + ).toEqual([ + { + slug: "auto", + name: "auto", + isCustom: false, + capabilities: emptyCapabilities, + }, + { + slug: "claude-opus-4.7", + name: "claude-opus-4.7", + isCustom: false, + capabilities: emptyCapabilities, + }, + ]); + }); + + it("ignores malformed and duplicate model entries", () => { + expect( + parseKiroListModelsOutput({ + code: 0, + stderr: "", + stdout: JSON.stringify({ + models: [ + { model_id: "auto", model_name: "Auto" }, + { model_id: "auto", model_name: "Auto duplicate" }, + { model_name: "missing id" }, + null, + ], + }), + }).map((model) => model.slug), + ).toEqual(["auto"]); + }); +}); diff --git a/apps/server/src/provider/Layers/KiroProvider.ts b/apps/server/src/provider/Layers/KiroProvider.ts new file mode 100644 index 00000000000..0c32f80c59c --- /dev/null +++ b/apps/server/src/provider/Layers/KiroProvider.ts @@ -0,0 +1,589 @@ +import type { + KiroSettings, + ModelCapabilities, + ServerProviderAuth, + ServerProviderModel, + ServerProviderState, +} from "@t3tools/contracts"; +import { ProviderDriverKind } from "@t3tools/contracts"; +import { createModelCapabilities } from "@t3tools/shared/model"; +import * as Cause from "effect/Cause"; +import * as DateTime from "effect/DateTime"; +import * as Effect from "effect/Effect"; +import * as Exit from "effect/Exit"; +import * as Option from "effect/Option"; +import * as Result from "effect/Result"; +import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process"; +import type * as EffectAcpSchema from "effect-acp/schema"; + +import { + buildServerProvider, + collectStreamAsString, + isCommandMissingCause, + parseGenericCliVersion, + providerModelsFromSettings, + type CommandResult, + type ServerProviderDraft, +} from "../providerSnapshot.ts"; +import { makeKiroAcpRuntime } from "../acp/KiroAcpSupport.ts"; + +const PROVIDER = ProviderDriverKind.make("kiro"); +const KIRO_PRESENTATION = { + displayName: "Kiro", + showInteractionModeToggle: true, +} as const; +const EMPTY_CAPABILITIES: ModelCapabilities = createModelCapabilities({ + optionDescriptors: [], +}); +const KIRO_FALLBACK_MODELS: ReadonlyArray = [ + { + slug: "auto", + name: "Auto", + isCustom: false, + capabilities: EMPTY_CAPABILITIES, + }, +]; +const VERSION_TIMEOUT_MS = 4_000; +const WHOAMI_TIMEOUT_MS = 8_000; +const LIST_MODELS_TIMEOUT_MS = 8_000; +const KIRO_ACP_MODEL_DISCOVERY_TIMEOUT_MS = 15_000; + +interface KiroWhoamiJsonPayload { + readonly email?: unknown; + readonly username?: unknown; + readonly profile?: unknown; + readonly authType?: unknown; + readonly authenticationMethod?: unknown; + readonly status?: unknown; +} + +interface KiroSessionSelectOption { + readonly value: string; + readonly name: string; +} + +interface KiroListModelsJsonPayload { + readonly models?: unknown; + readonly default_model?: unknown; + readonly defaultModel?: unknown; +} + +function getKiroFallbackModels( + kiroSettings: Pick, +): ReadonlyArray { + return providerModelsFromSettings( + KIRO_FALLBACK_MODELS, + PROVIDER, + kiroSettings.customModels, + EMPTY_CAPABILITIES, + ); +} + +function stripAnsi(text: string): string { + // eslint-disable-next-line no-control-regex + return text.replace(/\x1b\[[0-9;]*[A-Za-z]|\x1b\].*?\x07/g, ""); +} + +function parseKiroWhoamiJsonPayload(raw: string): KiroWhoamiJsonPayload | undefined { + const trimmed = raw.trim(); + if (!trimmed.startsWith("{")) return undefined; + try { + const parsed = JSON.parse(trimmed) as unknown; + if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) return undefined; + return parsed as KiroWhoamiJsonPayload; + } catch { + return undefined; + } +} + +function readFirstString( + record: Record, + keys: ReadonlyArray, +): string | undefined { + for (const key of keys) { + const value = record[key]; + if (typeof value === "string" && value.trim().length > 0) { + return value.trim(); + } + } + return undefined; +} + +function parseKiroWhoamiOutput(result: CommandResult): { + readonly auth: ServerProviderAuth; + readonly status: Exclude; + readonly message?: string; +} { + const jsonPayload = parseKiroWhoamiJsonPayload(result.stdout); + if (jsonPayload) { + const record = jsonPayload as Record; + const email = readFirstString(record, ["email", "userEmail"]); + const username = readFirstString(record, ["username", "userName", "userId"]); + const type = readFirstString(record, ["authType", "authenticationMethod", "loginType"]); + const profile = readFirstString(record, ["profile", "profileName"]); + const normalizedStatus = readFirstString(record, ["status", "sessionStatus"])?.toLowerCase(); + if ( + result.code !== 0 || + normalizedStatus === "not logged in" || + normalizedStatus === "unauthenticated" + ) { + return { + status: "error", + auth: { status: "unauthenticated" }, + message: "Kiro CLI is not authenticated. Run `kiro-cli login` and try again.", + }; + } + return { + status: "ready", + auth: { + status: "authenticated", + ...(email ? { email } : {}), + ...(type ? { type } : {}), + ...(profile ? { label: profile } : username ? { label: username } : {}), + }, + }; + } + + const combined = stripAnsi(`${result.stdout}\n${result.stderr}`); + const lowerOutput = combined.toLowerCase(); + if ( + result.code !== 0 || + lowerOutput.includes("not logged in") || + lowerOutput.includes("login required") || + lowerOutput.includes("authentication required") + ) { + return { + status: "error", + auth: { status: "unauthenticated" }, + message: "Kiro CLI is not authenticated. Run `kiro-cli login` and try again.", + }; + } + + const emailMatch = /\bEmail:\s*([^\s]+@[^\s]+)/i.exec(combined); + return { + status: "ready", + auth: { + status: "authenticated", + ...(emailMatch?.[1] ? { email: emailMatch[1].trim() } : {}), + }, + }; +} + +function flattenSessionConfigSelectOptions( + configOption: EffectAcpSchema.SessionConfigOption | undefined, +): ReadonlyArray { + if (!configOption || configOption.type !== "select") return []; + return configOption.options.flatMap((entry) => + "value" in entry + ? [ + { + value: entry.value.trim(), + name: entry.name.trim(), + }, + ] + : entry.options.map((option) => ({ + value: option.value.trim(), + name: option.name.trim(), + })), + ); +} + +function findModelConfigOption( + configOptions: ReadonlyArray, +): EffectAcpSchema.SessionConfigOption | undefined { + return configOptions.find((option) => option.category === "model"); +} + +function buildKiroModelsFromModelState( + modelState: EffectAcpSchema.SessionModelState | null | undefined, +): ReadonlyArray { + if (!modelState || modelState.availableModels.length === 0) return []; + const seen = new Set(); + return modelState.availableModels.flatMap((model) => { + const slug = model.modelId.trim(); + const name = model.name.trim(); + if (!slug || seen.has(slug)) return []; + seen.add(slug); + return [ + { + slug, + name: name || slug, + isCustom: false, + capabilities: EMPTY_CAPABILITIES, + } satisfies ServerProviderModel, + ]; + }); +} + +function buildKiroModelsFromConfigOptions( + configOptions: ReadonlyArray | null | undefined, +): ReadonlyArray { + if (!configOptions || configOptions.length === 0) return []; + const modelChoices = flattenSessionConfigSelectOptions(findModelConfigOption(configOptions)); + const seen = new Set(); + return modelChoices.flatMap((modelChoice) => { + const slug = modelChoice.value.trim(); + const name = modelChoice.name.trim(); + if (!slug || seen.has(slug)) return []; + seen.add(slug); + return [ + { + slug, + name: name || slug, + isCustom: false, + capabilities: EMPTY_CAPABILITIES, + } satisfies ServerProviderModel, + ]; + }); +} + +function readKiroListModelsArray(raw: string): ReadonlyArray { + const trimmed = raw.trim(); + if (!trimmed) return []; + try { + const parsed = JSON.parse(trimmed) as unknown; + if (Array.isArray(parsed)) return parsed; + if (!parsed || typeof parsed !== "object") return []; + const payload = parsed as KiroListModelsJsonPayload; + return Array.isArray(payload.models) ? payload.models : []; + } catch { + return []; + } +} + +function readKiroListModelString( + record: Record, + keys: ReadonlyArray, +): string | undefined { + for (const key of keys) { + const value = record[key]; + if (typeof value === "string" && value.trim().length > 0) { + return value.trim(); + } + } + return undefined; +} + +export function parseKiroListModelsOutput( + result: CommandResult, +): ReadonlyArray { + if (result.code !== 0) return []; + const seen = new Set(); + return readKiroListModelsArray(result.stdout).flatMap((entry) => { + if (!entry || typeof entry !== "object" || Array.isArray(entry)) return []; + const record = entry as Record; + const slug = readKiroListModelString(record, ["model_id", "id", "slug", "value"]); + if (!slug || seen.has(slug)) return []; + seen.add(slug); + const name = readKiroListModelString(record, [ + "model_name", + "name", + "display_name", + "displayName", + ]); + return [ + { + slug, + name: name || slug, + isCustom: false, + capabilities: EMPTY_CAPABILITIES, + } satisfies ServerProviderModel, + ]; + }); +} + +function hasKiroModelCapabilities(model: Pick): boolean { + return (model.capabilities?.optionDescriptors?.length ?? 0) > 0; +} + +function mergeKiroDiscoveredModels( + primary: ReadonlyArray, + secondary: ReadonlyArray, +): ReadonlyArray { + if (primary.length === 0) return secondary; + if (secondary.length === 0) return primary; + + const secondaryBySlug = new Map(secondary.map((model) => [model.slug, model] as const)); + const seen = new Set(); + const merged = primary.map((model) => { + seen.add(model.slug); + const secondaryModel = secondaryBySlug.get(model.slug); + return secondaryModel && hasKiroModelCapabilities(secondaryModel) + ? { ...model, capabilities: secondaryModel.capabilities } + : model; + }); + + for (const model of secondary) { + if (!seen.has(model.slug)) { + merged.push(model); + } + } + + return merged; +} + +function buildDiscoveredKiroModels( + response: + | EffectAcpSchema.LoadSessionResponse + | EffectAcpSchema.NewSessionResponse + | EffectAcpSchema.ResumeSessionResponse, +): ReadonlyArray { + const modelStateModels = buildKiroModelsFromModelState(response.models); + if (modelStateModels.length > 0) return modelStateModels; + return buildKiroModelsFromConfigOptions(response.configOptions); +} + +const discoverKiroModelsViaAcp = ( + kiroSettings: KiroSettings, + environment: NodeJS.ProcessEnv = process.env, +) => + Effect.gen(function* () { + const childProcessSpawner = yield* ChildProcessSpawner.ChildProcessSpawner; + const runtime = yield* makeKiroAcpRuntime({ + kiroSettings, + environment, + childProcessSpawner, + cwd: process.cwd(), + clientInfo: { name: "t3-code-provider-probe", version: "0.0.0" }, + }); + const started = yield* runtime.start(); + return buildDiscoveredKiroModels(started.sessionSetupResult); + }).pipe(Effect.scoped); + +function buildKiroProviderSnapshot(input: { + readonly checkedAt: string; + readonly kiroSettings: KiroSettings; + readonly version: string | null; + readonly auth: ServerProviderAuth; + readonly status: Exclude; + readonly discoveredModels?: ReadonlyArray; + readonly message?: string; + readonly discoveryWarning?: string; +}): ServerProviderDraft { + const messages = [input.message, input.discoveryWarning] + .map((message) => message?.trim()) + .filter((message): message is string => Boolean(message)); + return buildServerProvider({ + driver: PROVIDER, + presentation: KIRO_PRESENTATION, + enabled: input.kiroSettings.enabled, + checkedAt: input.checkedAt, + models: providerModelsFromSettings( + input.discoveredModels && input.discoveredModels.length > 0 + ? input.discoveredModels + : KIRO_FALLBACK_MODELS, + PROVIDER, + input.kiroSettings.customModels, + EMPTY_CAPABILITIES, + ), + probe: { + installed: true, + version: input.version, + status: + input.discoveryWarning && input.status === "ready" ? ("warning" as const) : input.status, + auth: input.auth, + ...(messages.length > 0 ? { message: messages.join(" ") } : {}), + }, + }); +} + +const runKiroCommand = ( + kiroSettings: KiroSettings, + args: ReadonlyArray, + environment: NodeJS.ProcessEnv = process.env, +) => + Effect.gen(function* () { + const spawner = yield* ChildProcessSpawner.ChildProcessSpawner; + const command = ChildProcess.make(kiroSettings.binaryPath, [...args], { + env: environment, + shell: process.platform === "win32", + }); + const child = yield* spawner.spawn(command); + const [stdout, stderr, exitCode] = yield* Effect.all( + [ + collectStreamAsString(child.stdout), + collectStreamAsString(child.stderr), + child.exitCode.pipe(Effect.map(Number)), + ], + { concurrency: "unbounded" }, + ); + return { stdout, stderr, code: exitCode } satisfies CommandResult; + }).pipe(Effect.scoped); + +export const makePendingKiroProvider = ( + kiroSettings: KiroSettings, +): Effect.Effect => + Effect.gen(function* () { + const checkedAt = yield* Effect.map(DateTime.now, DateTime.formatIso); + const fallbackModels = getKiroFallbackModels(kiroSettings); + if (!kiroSettings.enabled) { + return buildServerProvider({ + driver: PROVIDER, + presentation: KIRO_PRESENTATION, + enabled: false, + checkedAt, + models: fallbackModels, + probe: { + installed: false, + version: null, + status: "warning", + auth: { status: "unknown" }, + message: "Kiro is disabled in T3 Code settings.", + }, + }); + } + return buildServerProvider({ + driver: PROVIDER, + presentation: KIRO_PRESENTATION, + enabled: true, + checkedAt, + models: fallbackModels, + probe: { + installed: true, + version: null, + status: "warning", + auth: { status: "unknown" }, + message: "Checking Kiro CLI availability...", + }, + }); + }); + +export const checkKiroProviderStatus = Effect.fn("checkKiroProviderStatus")(function* ( + kiroSettings: KiroSettings, + environment: NodeJS.ProcessEnv = process.env, +): Effect.fn.Return { + const checkedAt = DateTime.formatIso(yield* DateTime.now); + const fallbackModels = getKiroFallbackModels(kiroSettings); + if (!kiroSettings.enabled) { + return buildServerProvider({ + driver: PROVIDER, + presentation: KIRO_PRESENTATION, + enabled: false, + checkedAt, + models: fallbackModels, + probe: { + installed: false, + version: null, + status: "warning", + auth: { status: "unknown" }, + message: "Kiro is disabled in T3 Code settings.", + }, + }); + } + + const versionProbe = yield* runKiroCommand(kiroSettings, ["--version"], environment).pipe( + Effect.timeoutOption(VERSION_TIMEOUT_MS), + Effect.result, + ); + if (Result.isFailure(versionProbe)) { + const error = versionProbe.failure; + return buildServerProvider({ + driver: PROVIDER, + presentation: KIRO_PRESENTATION, + enabled: kiroSettings.enabled, + checkedAt, + models: fallbackModels, + probe: { + installed: !isCommandMissingCause(error), + version: null, + status: "error", + auth: { status: "unknown" }, + message: isCommandMissingCause(error) + ? "Kiro CLI (`kiro-cli`) is not installed or not on PATH." + : `Failed to execute Kiro CLI health check: ${error instanceof Error ? error.message : String(error)}.`, + }, + }); + } + if (Option.isNone(versionProbe.success)) { + return buildServerProvider({ + driver: PROVIDER, + presentation: KIRO_PRESENTATION, + enabled: kiroSettings.enabled, + checkedAt, + models: fallbackModels, + probe: { + installed: true, + version: null, + status: "error", + auth: { status: "unknown" }, + message: "Kiro CLI is installed but timed out while running `kiro-cli --version`.", + }, + }); + } + + const version = parseGenericCliVersion( + `${versionProbe.success.value.stdout}\n${versionProbe.success.value.stderr}`, + ); + const whoamiProbe = yield* runKiroCommand( + kiroSettings, + ["whoami", "--format", "json"], + environment, + ).pipe(Effect.timeoutOption(WHOAMI_TIMEOUT_MS), Effect.result); + const parsedAuth = + Result.isSuccess(whoamiProbe) && Option.isSome(whoamiProbe.success) + ? parseKiroWhoamiOutput(whoamiProbe.success.value) + : { + status: "warning" as const, + auth: { status: "unknown" as const }, + message: + Result.isFailure(whoamiProbe) && isCommandMissingCause(whoamiProbe.failure) + ? "Kiro CLI (`kiro-cli`) is not installed or not on PATH." + : "Could not verify Kiro CLI authentication status.", + }; + + let discoveredModels: ReadonlyArray = []; + let discoveryWarning: string | undefined; + if (parsedAuth.auth.status !== "unauthenticated") { + const listModelsExit = yield* Effect.exit( + runKiroCommand(kiroSettings, ["chat", "--list-models", "--format", "json"], environment).pipe( + Effect.timeoutOption(LIST_MODELS_TIMEOUT_MS), + ), + ); + if (Exit.isFailure(listModelsExit)) { + yield* Effect.logWarning("Kiro CLI model list failed", { + cause: Cause.pretty(listModelsExit.cause), + }); + } else if (Option.isNone(listModelsExit.value)) { + yield* Effect.logWarning("Kiro CLI model list timed out", { + timeoutMs: LIST_MODELS_TIMEOUT_MS, + }); + } else { + discoveredModels = parseKiroListModelsOutput(listModelsExit.value.value); + } + + const discoveryExit = yield* Effect.exit( + discoverKiroModelsViaAcp(kiroSettings, environment).pipe( + Effect.timeoutOption(KIRO_ACP_MODEL_DISCOVERY_TIMEOUT_MS), + ), + ); + if (Exit.isFailure(discoveryExit)) { + yield* Effect.logWarning("Kiro ACP model discovery failed", { + cause: Cause.pretty(discoveryExit.cause), + }); + if (discoveredModels.length === 0) { + discoveryWarning = "Kiro model discovery failed. Check server logs for details."; + } + } else if (Option.isNone(discoveryExit.value)) { + if (discoveredModels.length === 0) { + discoveryWarning = `Kiro model discovery timed out after ${KIRO_ACP_MODEL_DISCOVERY_TIMEOUT_MS}ms.`; + } + } else if (discoveryExit.value.value.length === 0) { + if (discoveredModels.length === 0) { + discoveryWarning = "Kiro model discovery returned no built-in models."; + } + } else { + discoveredModels = mergeKiroDiscoveredModels(discoveredModels, discoveryExit.value.value); + } + } + + return buildKiroProviderSnapshot({ + checkedAt, + kiroSettings, + version, + auth: parsedAuth.auth, + status: parsedAuth.status, + ...(parsedAuth.message ? { message: parsedAuth.message } : {}), + discoveredModels, + ...(discoveryWarning ? { discoveryWarning } : {}), + }); +}); diff --git a/apps/server/src/provider/Layers/ProviderInstanceRegistryLive.test.ts b/apps/server/src/provider/Layers/ProviderInstanceRegistryLive.test.ts index 86f99c97326..42a58dab98a 100644 --- a/apps/server/src/provider/Layers/ProviderInstanceRegistryLive.test.ts +++ b/apps/server/src/provider/Layers/ProviderInstanceRegistryLive.test.ts @@ -10,7 +10,7 @@ * * 2. **Many drivers, one registry** — the "all drivers slice" describe * block below configures one instance of every shipped driver - * (`codex`, `claudeAgent`, `cursor`, `opencode`) in a single + * (`codex`, `claudeAgent`, `cursor`, `kiro`, `opencode`) in a single * `ProviderInstanceConfigMap` and asserts the registry boots them all * without cross-contamination. This proves the driver SPI is uniform * across every provider — any driver plugs into the registry through @@ -18,9 +18,9 @@ * * Every instance in these tests is configured with `enabled: false` so the * provider-status checks short-circuit to pending/disabled snapshots - * without trying to spawn real `codex` / `claude` / `agent` / `opencode` - * binaries. That keeps the assertions focused on registry routing - * behaviour rather than the runtime details of each provider. + * without trying to spawn real provider binaries. That keeps the assertions + * focused on registry routing behaviour rather than the runtime details of + * each provider. */ import { describe, expect, it } from "@effect/vitest"; import * as NodeServices from "@effect/platform-node/NodeServices"; @@ -28,6 +28,7 @@ import { type ClaudeSettings, type CodexSettings, type CursorSettings, + type KiroSettings, type OpenCodeSettings, ProviderDriverKind, type ProviderInstanceConfigMap, @@ -41,6 +42,7 @@ import { ServerConfig } from "../../config.ts"; import { ClaudeDriver } from "../Drivers/ClaudeDriver.ts"; import { CodexDriver } from "../Drivers/CodexDriver.ts"; import { CursorDriver } from "../Drivers/CursorDriver.ts"; +import { KiroDriver } from "../Drivers/KiroDriver.ts"; import { OpenCodeDriver } from "../Drivers/OpenCodeDriver.ts"; import { OpenCodeRuntimeLive } from "../opencodeRuntime.ts"; import { NoOpProviderEventLoggers, ProviderEventLoggers } from "./ProviderEventLoggers.ts"; @@ -79,6 +81,15 @@ const makeCursorConfig = (overrides: Partial): CursorSettings => ...overrides, }); +const makeKiroConfig = (overrides: Partial): KiroSettings => ({ + enabled: false, + binaryPath: "kiro-cli", + homePath: "", + agentName: "", + customModels: [], + ...overrides, +}); + const makeOpenCodeConfig = (overrides: Partial): OpenCodeSettings => ({ enabled: false, binaryPath: "opencode", @@ -218,7 +229,7 @@ describe("ProviderInstanceRegistryLive — multi-instance codex slice", () => { }); describe("ProviderInstanceRegistryLive — all drivers slice", () => { - // All four drivers need `NodeServices` (ChildProcessSpawner + FileSystem + + // All built-in drivers need `NodeServices` (ChildProcessSpawner + FileSystem + // Path). `OpenCodeDriver.create` additionally yields `OpenCodeRuntime` // at construction time, so we wire `OpenCodeRuntimeLive` into the stack. // `OpenCodeRuntimeLive` bundles its own `NetService.layer` via @@ -244,11 +255,13 @@ describe("ProviderInstanceRegistryLive — all drivers slice", () => { const codexId = ProviderInstanceId.make("codex_default"); const claudeId = ProviderInstanceId.make("claude_default"); const cursorId = ProviderInstanceId.make("cursor_default"); + const kiroId = ProviderInstanceId.make("kiro_default"); const openCodeId = ProviderInstanceId.make("opencode_default"); const codexDriverKind = ProviderDriverKind.make("codex"); const claudeDriverKind = ProviderDriverKind.make("claudeAgent"); const cursorDriverKind = ProviderDriverKind.make("cursor"); + const kiroDriverKind = ProviderDriverKind.make("kiro"); const openCodeDriverKind = ProviderDriverKind.make("opencode"); const configMap: ProviderInstanceConfigMap = { @@ -273,6 +286,12 @@ describe("ProviderInstanceRegistryLive — all drivers slice", () => { enabled: false, config: makeCursorConfig({}), }, + [kiroId]: { + driver: kiroDriverKind, + displayName: "Kiro", + enabled: false, + config: makeKiroConfig({ homePath: "/home/julius/.kiro-work" }), + }, [openCodeId]: { driver: openCodeDriverKind, displayName: "OpenCode", @@ -282,7 +301,7 @@ describe("ProviderInstanceRegistryLive — all drivers slice", () => { }; const { registry } = yield* makeProviderInstanceRegistry({ - drivers: [CodexDriver, ClaudeDriver, CursorDriver, OpenCodeDriver], + drivers: [CodexDriver, ClaudeDriver, CursorDriver, KiroDriver, OpenCodeDriver], configMap, }); @@ -292,9 +311,9 @@ describe("ProviderInstanceRegistryLive — all drivers slice", () => { expect(unavailable).toEqual([]); const instances = yield* registry.listInstances; - expect(instances).toHaveLength(4); + expect(instances).toHaveLength(5); expect(instances.map((instance) => instance.instanceId).toSorted()).toEqual( - [codexId, claudeId, cursorId, openCodeId].toSorted(), + [codexId, claudeId, cursorId, kiroId, openCodeId].toSorted(), ); // Instance lookup by id resolves each instance to its own bundle — @@ -303,14 +322,17 @@ describe("ProviderInstanceRegistryLive — all drivers slice", () => { const codex = yield* registry.getInstance(codexId); const claude = yield* registry.getInstance(claudeId); const cursor = yield* registry.getInstance(cursorId); + const kiro = yield* registry.getInstance(kiroId); const openCode = yield* registry.getInstance(openCodeId); expect(codex?.driverKind).toBe(codexDriverKind); expect(claude?.driverKind).toBe(claudeDriverKind); expect(cursor?.driverKind).toBe(cursorDriverKind); + expect(kiro?.driverKind).toBe(kiroDriverKind); expect(openCode?.driverKind).toBe(openCodeDriverKind); expect(codex?.displayName).toBe("Codex"); expect(claude?.displayName).toBe("Claude"); expect(cursor?.displayName).toBe("Cursor"); + expect(kiro?.displayName).toBe("Kiro"); expect(openCode?.displayName).toBe("OpenCode"); // Every instance owns its own set of closures — no sharing across @@ -318,16 +340,29 @@ describe("ProviderInstanceRegistryLive — all drivers slice", () => { // distinct references even when two instances happen to share a // trait (e.g. Cursor + others all use a stub-or-real // `textGeneration`; they must still be different object values). - const adapters = [codex!.adapter, claude!.adapter, cursor!.adapter, openCode!.adapter]; + const adapters = [ + codex!.adapter, + claude!.adapter, + cursor!.adapter, + kiro!.adapter, + openCode!.adapter, + ]; expect(new Set(adapters).size).toBe(adapters.length); const textGenerations = [ codex!.textGeneration, claude!.textGeneration, cursor!.textGeneration, + kiro!.textGeneration, openCode!.textGeneration, ]; expect(new Set(textGenerations).size).toBe(textGenerations.length); - const snapshots = [codex!.snapshot, claude!.snapshot, cursor!.snapshot, openCode!.snapshot]; + const snapshots = [ + codex!.snapshot, + claude!.snapshot, + cursor!.snapshot, + kiro!.snapshot, + openCode!.snapshot, + ]; expect(new Set(snapshots).size).toBe(snapshots.length); // Snapshots identify themselves by `instanceId` + `driver` so @@ -356,6 +391,12 @@ describe("ProviderInstanceRegistryLive — all drivers slice", () => { `${cursorDriverKind}:instance:${cursorId}`, ); + const kiroSnapshot = yield* kiro!.snapshot.getSnapshot; + expect(kiroSnapshot.instanceId).toBe(kiroId); + expect(kiroSnapshot.driver).toBe(kiroDriverKind); + expect(kiroSnapshot.enabled).toBe(false); + expect(kiroSnapshot.continuation?.groupKey).toBe("kiro:home:/home/julius/.kiro-work"); + const openCodeSnapshot = yield* openCode!.snapshot.getSnapshot; expect(openCodeSnapshot.instanceId).toBe(openCodeId); expect(openCodeSnapshot.driver).toBe(openCodeDriverKind); diff --git a/apps/server/src/provider/Layers/ProviderRegistry.test.ts b/apps/server/src/provider/Layers/ProviderRegistry.test.ts index fb6eb3b443d..261cce2bb0f 100644 --- a/apps/server/src/provider/Layers/ProviderRegistry.test.ts +++ b/apps/server/src/provider/Layers/ProviderRegistry.test.ts @@ -1300,6 +1300,7 @@ it.layer(Layer.mergeAll(NodeServices.layer, ServerSettingsService.layerTest(), T "claudeAgent", "codex", "cursor", + "kiro", "opencode", ]); assert.strictEqual(cursorProvider?.enabled, false); diff --git a/apps/server/src/provider/Services/KiroAdapter.ts b/apps/server/src/provider/Services/KiroAdapter.ts new file mode 100644 index 00000000000..157ebcd6326 --- /dev/null +++ b/apps/server/src/provider/Services/KiroAdapter.ts @@ -0,0 +1,12 @@ +/** + * KiroAdapter — shape type for the Kiro CLI provider adapter. + * + * The driver model bundles one adapter per configured instance, so this + * module is a naming anchor for the per-instance Kiro adapter contract. + * + * @module KiroAdapter + */ +import type { ProviderAdapterError } from "../Errors.ts"; +import type { ProviderAdapterShape } from "./ProviderAdapter.ts"; + +export interface KiroAdapterShape extends ProviderAdapterShape {} diff --git a/apps/server/src/provider/acp/AcpAdapterSupport.test.ts b/apps/server/src/provider/acp/AcpAdapterSupport.test.ts index a7fcdc4c827..b3f8faa21fa 100644 --- a/apps/server/src/provider/acp/AcpAdapterSupport.test.ts +++ b/apps/server/src/provider/acp/AcpAdapterSupport.test.ts @@ -11,6 +11,18 @@ describe("AcpAdapterSupport", () => { expect(acpPermissionOutcome("decline")).toBe("reject-once"); }); + it("uses provider-supplied ACP permission option ids when available", () => { + const options = [ + { optionId: "allow_once", name: "Yes", kind: "allow_once" }, + { optionId: "allow_always", name: "Always", kind: "allow_always" }, + { optionId: "reject_once", name: "No", kind: "reject_once" }, + ] as const; + + expect(acpPermissionOutcome("accept", options)).toBe("allow_once"); + expect(acpPermissionOutcome("acceptForSession", options)).toBe("allow_always"); + expect(acpPermissionOutcome("decline", options)).toBe("reject_once"); + }); + it("maps ACP request errors to provider adapter request errors", () => { const error = mapAcpToAdapterError( ProviderDriverKind.make("cursor"), @@ -25,4 +37,21 @@ describe("AcpAdapterSupport", () => { expect(error._tag).toBe("ProviderAdapterRequestError"); expect(error.message).toContain("Invalid params"); }); + + it("surfaces ACP request error data when the provider reports a generic internal error", () => { + const error = mapAcpToAdapterError( + ProviderDriverKind.make("kiro"), + "thread-1" as never, + "session/prompt", + new EffectAcpErrors.AcpRequestError({ + code: -32603, + errorMessage: "Internal error", + data: "Prompt already in progress", + }), + ); + + expect(error._tag).toBe("ProviderAdapterRequestError"); + expect(error.message).toContain("Prompt already in progress"); + expect(error.message).not.toContain("Internal error"); + }); }); diff --git a/apps/server/src/provider/acp/AcpAdapterSupport.ts b/apps/server/src/provider/acp/AcpAdapterSupport.ts index cde110e6dd9..81f0094045d 100644 --- a/apps/server/src/provider/acp/AcpAdapterSupport.ts +++ b/apps/server/src/provider/acp/AcpAdapterSupport.ts @@ -5,6 +5,7 @@ import { } from "@t3tools/contracts"; import * as Schema from "effect/Schema"; import * as EffectAcpErrors from "effect-acp/errors"; +import type * as EffectAcpSchema from "effect-acp/schema"; import { ProviderAdapterRequestError, @@ -14,6 +15,36 @@ import { const isAcpProcessExitedError = Schema.is(EffectAcpErrors.AcpProcessExitedError); const isAcpRequestError = Schema.is(EffectAcpErrors.AcpRequestError); +function readAcpErrorDataMessage(data: unknown): string | undefined { + if (typeof data === "string") { + const trimmed = data.trim(); + return trimmed.length > 0 ? trimmed : undefined; + } + if (data && typeof data === "object") { + const maybeRecord = data as Record; + for (const key of ["message", "detail", "error"]) { + const value = maybeRecord[key]; + if (typeof value === "string" && value.trim()) { + return value.trim(); + } + } + try { + return JSON.stringify(data); + } catch { + return undefined; + } + } + return undefined; +} + +function formatAcpRequestErrorDetail(error: EffectAcpErrors.AcpRequestError): string { + const dataMessage = readAcpErrorDataMessage(error.data); + if (!dataMessage) { + return error.message; + } + return error.message === "Internal error" ? dataMessage : `${error.message}: ${dataMessage}`; +} + export function mapAcpToAdapterError( provider: ProviderDriverKind, threadId: ThreadId, @@ -31,7 +62,7 @@ export function mapAcpToAdapterError( return new ProviderAdapterRequestError({ provider, method, - detail: error.message, + detail: formatAcpRequestErrorDetail(error), cause: error, }); } @@ -43,13 +74,35 @@ export function mapAcpToAdapterError( }); } -export function acpPermissionOutcome(decision: ProviderApprovalDecision): string { +const FALLBACK_PERMISSION_OPTION_IDS = { + accept: "allow-once", + acceptForSession: "allow-always", + decline: "reject-once", +} as const satisfies Record, string>; + +const PERMISSION_OPTION_KINDS = { + accept: "allow_once", + acceptForSession: "allow_always", + decline: "reject_once", +} as const satisfies Record< + Exclude, + EffectAcpSchema.PermissionOption["kind"] +>; + +export function acpPermissionOutcome( + decision: ProviderApprovalDecision, + options?: ReadonlyArray, +): string { switch (decision) { - case "acceptForSession": - return "allow-always"; case "accept": - return "allow-once"; - case "decline": + case "acceptForSession": + case "decline": { + const optionKind = PERMISSION_OPTION_KINDS[decision]; + const matchingOption = options?.find( + (option) => option.kind === optionKind && option.optionId.trim().length > 0, + ); + return matchingOption?.optionId.trim() ?? FALLBACK_PERMISSION_OPTION_IDS[decision]; + } default: return "reject-once"; } diff --git a/apps/server/src/provider/acp/AcpRuntimeModel.test.ts b/apps/server/src/provider/acp/AcpRuntimeModel.test.ts index ae12d3112aa..222587f0505 100644 --- a/apps/server/src/provider/acp/AcpRuntimeModel.test.ts +++ b/apps/server/src/provider/acp/AcpRuntimeModel.test.ts @@ -283,4 +283,28 @@ describe("AcpRuntimeModel", () => { }, }); }); + + it("infers Kiro permission kinds when request payload omits tool kind", () => { + const editRequest = parsePermissionRequest({ + sessionId: "session-1", + options: [{ optionId: "allow_once", name: "Yes", kind: "allow_once" }], + toolCall: { + toolCallId: "tool-edit", + title: "Editing ComposerPrimaryActions.tsx", + }, + }); + const commandRequest = parsePermissionRequest({ + sessionId: "session-1", + options: [{ optionId: "allow_once", name: "Yes", kind: "allow_once" }], + toolCall: { + toolCallId: "tool-command", + title: 'Running: find node_modules/lucide-react -name "send.js"', + }, + }); + + expect(editRequest.kind).toBe("edit"); + expect(editRequest.toolCall?.kind).toBe("edit"); + expect(commandRequest.kind).toBe("execute"); + expect(commandRequest.toolCall?.kind).toBe("execute"); + }); }); diff --git a/apps/server/src/provider/acp/AcpRuntimeModel.ts b/apps/server/src/provider/acp/AcpRuntimeModel.ts index ffd214a5bf1..7df240ce8bd 100644 --- a/apps/server/src/provider/acp/AcpRuntimeModel.ts +++ b/apps/server/src/provider/acp/AcpRuntimeModel.ts @@ -237,8 +237,58 @@ function extractTextContentFromToolCallContent( return chunks.length > 0 ? chunks.join("\n") : undefined; } -function normalizeToolKind(kind: unknown): string | undefined { - return typeof kind === "string" && kind.trim().length > 0 ? kind.trim() : undefined; +const ACP_TOOL_KINDS = new Set([ + "read", + "edit", + "delete", + "move", + "search", + "execute", + "think", + "fetch", + "switch_mode", + "other", +]); + +function normalizeToolKind(kind: unknown): EffectAcpSchema.ToolKind | undefined { + if (typeof kind !== "string") { + return undefined; + } + const trimmed = kind.trim(); + return ACP_TOOL_KINDS.has(trimmed as EffectAcpSchema.ToolKind) + ? (trimmed as EffectAcpSchema.ToolKind) + : undefined; +} + +function inferToolKindFromTitle( + title: string | null | undefined, +): EffectAcpSchema.ToolKind | undefined { + const normalizedTitle = title?.trim().toLowerCase(); + if (!normalizedTitle) { + return undefined; + } + + if ( + normalizedTitle.startsWith("running:") || + normalizedTitle.startsWith("run:") || + normalizedTitle.startsWith("executing:") + ) { + return "execute"; + } + + if ( + /^(editing|edit|modifying|writing|creating|updating|deleting|moving|renaming)\b/.test( + normalizedTitle, + ) + ) { + return "edit"; + } + + if (/^(reading|read|opening|viewing)\b/.test(normalizedTitle)) { + return "read"; + } + + return undefined; } function canonicalItemTypeFromAcpToolKind(kind: string | undefined): ToolLifecycleItemType { @@ -379,11 +429,13 @@ export function mergeToolCallState( export function parsePermissionRequest( params: EffectAcpSchema.RequestPermissionRequest, ): AcpPermissionRequest { + const kind = + normalizeToolKind(params.toolCall.kind) ?? inferToolKindFromTitle(params.toolCall.title); const toolCall = makeToolCallState( { toolCallId: params.toolCall.toolCallId, title: params.toolCall.title, - kind: params.toolCall.kind, + kind, status: params.toolCall.status, rawInput: params.toolCall.rawInput, rawOutput: params.toolCall.rawOutput, @@ -392,14 +444,13 @@ export function parsePermissionRequest( }, { fallbackStatus: "pending" }, ); - const kind = normalizeToolKind(params.toolCall.kind) ?? "unknown"; const detail = toolCall?.command ?? toolCall?.title ?? toolCall?.detail ?? (typeof params.sessionId === "string" ? `Session ${params.sessionId}` : undefined); return { - kind, + kind: kind ?? "unknown", ...(detail ? { detail } : {}), ...(toolCall ? { toolCall } : {}), }; diff --git a/apps/server/src/provider/acp/AcpSessionRuntime.ts b/apps/server/src/provider/acp/AcpSessionRuntime.ts index 8652b2cfeaf..4134c93e625 100644 --- a/apps/server/src/provider/acp/AcpSessionRuntime.ts +++ b/apps/server/src/provider/acp/AcpSessionRuntime.ts @@ -46,7 +46,8 @@ export interface AcpSessionRuntimeOptions { readonly name: string; readonly version: string; }; - readonly authMethodId: string; + readonly authMethodId?: string; + readonly setModelStrategy?: "config-option" | "session-set-model"; readonly requestLogger?: (event: AcpSessionRequestLogEvent) => Effect.Effect; readonly protocolLogging?: { readonly logIncoming?: boolean; @@ -378,15 +379,17 @@ const makeAcpSessionRuntime = ( acp.agent.initialize(initializePayload), ); - const authenticatePayload = { - methodId: options.authMethodId, - } satisfies EffectAcpSchema.AuthenticateRequest; + if (options.authMethodId !== undefined) { + const authenticatePayload = { + methodId: options.authMethodId, + } satisfies EffectAcpSchema.AuthenticateRequest; - yield* runLoggedRequest( - "authenticate", - authenticatePayload, - acp.agent.authenticate(authenticatePayload), - ); + yield* runLoggedRequest( + "authenticate", + authenticatePayload, + acp.agent.authenticate(authenticatePayload), + ); + } let sessionId: string; let sessionSetupResult: @@ -543,7 +546,20 @@ const makeAcpSessionRuntime = ( setConfigOption, setModel: (model) => getStartedState.pipe( - Effect.flatMap((started) => setConfigOption(started.modelConfigId ?? "model", model)), + Effect.flatMap((started) => { + if (options.setModelStrategy !== "session-set-model") { + return setConfigOption(started.modelConfigId ?? "model", model); + } + const requestPayload = { + sessionId: started.sessionId, + modelId: model, + } satisfies EffectAcpSchema.SetSessionModelRequest; + return runLoggedRequest( + "session/set_model", + requestPayload, + acp.agent.setSessionModel(requestPayload), + ); + }), Effect.asVoid, ), request: (method, payload) => diff --git a/apps/server/src/provider/acp/KiroAcpSupport.test.ts b/apps/server/src/provider/acp/KiroAcpSupport.test.ts new file mode 100644 index 00000000000..5224678c2ad --- /dev/null +++ b/apps/server/src/provider/acp/KiroAcpSupport.test.ts @@ -0,0 +1,33 @@ +import { describe, expect, it } from "vitest"; + +import { buildKiroAcpSpawnInput } from "./KiroAcpSupport.ts"; + +describe("buildKiroAcpSpawnInput", () => { + it("starts Kiro ACP with the default CLI binary", () => { + expect(buildKiroAcpSpawnInput(undefined, "/repo")).toEqual({ + command: "kiro-cli", + args: ["acp"], + cwd: "/repo", + }); + }); + + it("passes a configured agent name and environment through to the ACP process", () => { + const env = { KIRO_HOME: "/tmp/kiro" }; + + expect( + buildKiroAcpSpawnInput( + { + binaryPath: "/opt/kiro/bin/kiro-cli", + agentName: "builder", + }, + "/repo", + env, + ), + ).toEqual({ + command: "/opt/kiro/bin/kiro-cli", + args: ["acp", "--agent", "builder"], + cwd: "/repo", + env, + }); + }); +}); diff --git a/apps/server/src/provider/acp/KiroAcpSupport.ts b/apps/server/src/provider/acp/KiroAcpSupport.ts new file mode 100644 index 00000000000..00b86928564 --- /dev/null +++ b/apps/server/src/provider/acp/KiroAcpSupport.ts @@ -0,0 +1,56 @@ +import { type KiroSettings } from "@t3tools/contracts"; +import * as Effect from "effect/Effect"; +import * as Layer from "effect/Layer"; +import * as Scope from "effect/Scope"; +import { ChildProcessSpawner } from "effect/unstable/process"; +import type * as EffectAcpErrors from "effect-acp/errors"; + +import { + AcpSessionRuntime, + type AcpSessionRuntimeOptions, + type AcpSessionRuntimeShape, + type AcpSpawnInput, +} from "./AcpSessionRuntime.ts"; + +type KiroAcpRuntimeSettings = Pick; + +export interface KiroAcpRuntimeInput extends Omit< + AcpSessionRuntimeOptions, + "authMethodId" | "setModelStrategy" | "spawn" +> { + readonly childProcessSpawner: ChildProcessSpawner.ChildProcessSpawner["Service"]; + readonly environment?: NodeJS.ProcessEnv; + readonly kiroSettings: KiroAcpRuntimeSettings | null | undefined; +} + +export function buildKiroAcpSpawnInput( + kiroSettings: KiroAcpRuntimeSettings | null | undefined, + cwd: string, + environment?: NodeJS.ProcessEnv, +): AcpSpawnInput { + const agentName = kiroSettings?.agentName.trim(); + return { + command: kiroSettings?.binaryPath || "kiro-cli", + args: ["acp", ...(agentName ? (["--agent", agentName] as const) : [])], + cwd, + ...(environment ? { env: environment } : {}), + }; +} + +export const makeKiroAcpRuntime = ( + input: KiroAcpRuntimeInput, +): Effect.Effect => + Effect.gen(function* () { + const acpContext = yield* Layer.build( + AcpSessionRuntime.layer({ + ...input, + spawn: buildKiroAcpSpawnInput(input.kiroSettings, input.cwd, input.environment), + setModelStrategy: "session-set-model", + }).pipe( + Layer.provide( + Layer.succeed(ChildProcessSpawner.ChildProcessSpawner, input.childProcessSpawner), + ), + ), + ); + return yield* Effect.service(AcpSessionRuntime).pipe(Effect.provide(acpContext)); + }); diff --git a/apps/server/src/provider/acp/StandardAcpAdapter.test.ts b/apps/server/src/provider/acp/StandardAcpAdapter.test.ts new file mode 100644 index 00000000000..557098eb8f5 --- /dev/null +++ b/apps/server/src/provider/acp/StandardAcpAdapter.test.ts @@ -0,0 +1,620 @@ +import * as NodeServices from "@effect/platform-node/NodeServices"; +import { assert, it } from "@effect/vitest"; +import * as Deferred from "effect/Deferred"; +import * as Effect from "effect/Effect"; +import * as Exit from "effect/Exit"; +import * as Fiber from "effect/Fiber"; +import * as FileSystem from "effect/FileSystem"; +import * as Layer from "effect/Layer"; +import * as Path from "effect/Path"; +import * as Stream from "effect/Stream"; +import { AcpRequestError } from "effect-acp/errors"; +import type * as EffectAcpSchema from "effect-acp/schema"; + +import { type ChatAttachment, ProviderDriverKind, ThreadId } from "@t3tools/contracts"; + +import { ServerConfig } from "../../config.ts"; +import { ProviderAdapterRequestError, ProviderAdapterValidationError } from "../Errors.ts"; +import type { AcpSessionRuntimeShape } from "./AcpSessionRuntime.ts"; +import { makeStandardAcpAdapter } from "./StandardAcpAdapter.ts"; + +const standardAcpAdapterTestLayer = ServerConfig.layerTest(process.cwd(), { + prefix: "t3-standard-acp-test-", +}).pipe(Layer.provideMerge(NodeServices.layer)); + +function makeFakeAcpRuntime(input: { + readonly cancelCalled: Deferred.Deferred; + readonly cancel?: Effect.Effect; + readonly prompt?: () => Effect.Effect; + readonly request?: (method: string, payload: unknown) => Effect.Effect; + readonly supportsImagePrompts?: boolean; +}): AcpSessionRuntimeShape { + const ignoreHandler = () => Effect.void; + return { + handleRequestPermission: ignoreHandler, + handleElicitation: ignoreHandler, + handleReadTextFile: ignoreHandler, + handleWriteTextFile: ignoreHandler, + handleCreateTerminal: ignoreHandler, + handleTerminalOutput: ignoreHandler, + handleTerminalWaitForExit: ignoreHandler, + handleTerminalKill: ignoreHandler, + handleTerminalRelease: ignoreHandler, + handleSessionUpdate: ignoreHandler, + handleElicitationComplete: ignoreHandler, + handleUnknownExtRequest: ignoreHandler, + handleUnknownExtNotification: ignoreHandler, + handleExtRequest: ignoreHandler, + handleExtNotification: ignoreHandler, + start: () => + Effect.succeed({ + sessionId: "fake-session", + initializeResult: { + protocolVersion: 1, + agentCapabilities: { + loadSession: true, + ...(input.supportsImagePrompts === false + ? {} + : { promptCapabilities: { image: true } }), + }, + } as EffectAcpSchema.InitializeResponse, + sessionSetupResult: { + sessionId: "fake-session", + } as EffectAcpSchema.NewSessionResponse, + modelConfigId: undefined, + }), + getEvents: () => Stream.empty, + getModeState: Effect.sync(() => undefined), + getConfigOptions: Effect.succeed([]), + prompt: input.prompt ?? (() => Effect.succeed({ stopReason: "end_turn" })), + cancel: input.cancel ?? Deferred.succeed(input.cancelCalled, undefined).pipe(Effect.asVoid), + setMode: () => Effect.succeed({} as EffectAcpSchema.SetSessionModeResponse), + setConfigOption: () => Effect.succeed({} as EffectAcpSchema.SetSessionConfigOptionResponse), + setModel: () => Effect.void, + request: input.request ?? (() => Effect.succeed({})), + notify: () => Effect.void, + } as unknown as AcpSessionRuntimeShape; +} + +it.effect("keeps interrupted ACP turns active until session/prompt resolves", () => + Effect.gen(function* () { + const provider = ProviderDriverKind.make("cursor"); + const threadId = ThreadId.make("standard-acp-cancel-awaits-prompt"); + const promptStarted = yield* Deferred.make(); + const promptResponse = yield* Deferred.make(); + const cancelCalled = yield* Deferred.make(); + const runtime = makeFakeAcpRuntime({ + cancelCalled, + prompt: () => + Deferred.succeed(promptStarted, undefined).pipe( + Effect.andThen(Deferred.await(promptResponse)), + ), + }); + + const adapter = yield* makeStandardAcpAdapter({ + provider, + runtimeLabel: "Fake ACP", + makeRuntime: () => Effect.succeed(runtime), + }); + + yield* adapter.startSession({ + threadId, + provider, + cwd: process.cwd(), + runtimeMode: "full-access", + }); + + const sendTurnFiber = yield* adapter + .sendTurn({ + threadId, + input: "cancel after provider prompt resolves", + attachments: [], + }) + .pipe(Effect.forkChild); + + yield* Effect.yieldNow; + assert.isUndefined(sendTurnFiber.pollUnsafe()); + yield* Deferred.await(promptStarted).pipe(Effect.timeout("1 second")); + yield* adapter.interruptTurn(threadId).pipe(Effect.timeout("1 second")); + yield* Deferred.await(cancelCalled).pipe(Effect.timeout("1 second")); + yield* Effect.yieldNow; + + const earlySendTurnExit = sendTurnFiber.pollUnsafe(); + assert.isUndefined(earlySendTurnExit); + + yield* Deferred.succeed(promptResponse, { stopReason: "cancelled" }); + const result = yield* Fiber.join(sendTurnFiber); + + assert.equal(result.threadId, threadId); + yield* adapter.stopSession(threadId); + }).pipe(Effect.scoped, Effect.provide(standardAcpAdapterTestLayer)), +); + +it.effect("rejects image attachments when the ACP session does not advertise image prompts", () => + Effect.gen(function* () { + const provider = ProviderDriverKind.make("cursor"); + const threadId = ThreadId.make("standard-acp-image-capability-required"); + const cancelCalled = yield* Deferred.make(); + const runtime = makeFakeAcpRuntime({ + cancelCalled, + supportsImagePrompts: false, + }); + const attachment: ChatAttachment = { + type: "image", + id: "image-capability-required", + name: "image-capability-required.png", + mimeType: "image/png", + sizeBytes: 1, + }; + + const adapter = yield* makeStandardAcpAdapter({ + provider, + runtimeLabel: "Fake ACP", + makeRuntime: () => Effect.succeed(runtime), + }); + + yield* adapter.startSession({ + threadId, + provider, + cwd: process.cwd(), + runtimeMode: "full-access", + }); + + const error = yield* adapter + .sendTurn({ + threadId, + input: "inspect", + attachments: [attachment], + }) + .pipe(Effect.flip, Effect.timeout("1 second")); + + assert.instanceOf(error, ProviderAdapterValidationError); + assert.equal(error.issue, "Fake ACP session does not support image attachments."); + yield* adapter.stopSession(threadId); + }).pipe(Effect.scoped, Effect.provide(standardAcpAdapterTestLayer)), +); + +it.effect("rejects image attachments outside a provider image MIME allowlist", () => + Effect.gen(function* () { + const provider = ProviderDriverKind.make("kiro"); + const threadId = ThreadId.make("standard-acp-image-mime-allowlist"); + const cancelCalled = yield* Deferred.make(); + const runtime = makeFakeAcpRuntime({ cancelCalled }); + const attachment: ChatAttachment = { + type: "image", + id: "image-mime-allowlist", + name: "image-mime-allowlist.svg", + mimeType: "image/svg+xml", + sizeBytes: 1, + }; + + const adapter = yield* makeStandardAcpAdapter({ + provider, + runtimeLabel: "Kiro", + supportedImageMimeTypes: new Set(["image/png"]), + makeRuntime: () => Effect.succeed(runtime), + }); + + yield* adapter.startSession({ + threadId, + provider, + cwd: process.cwd(), + runtimeMode: "full-access", + }); + + const error = yield* adapter + .sendTurn({ + threadId, + input: "inspect", + attachments: [attachment], + }) + .pipe(Effect.flip, Effect.timeout("1 second")); + + assert.instanceOf(error, ProviderAdapterRequestError); + assert.equal(error.detail, "Unsupported Kiro image attachment type 'image/svg+xml'."); + yield* adapter.stopSession(threadId); + }).pipe(Effect.scoped, Effect.provide(standardAcpAdapterTestLayer)), +); + +it.effect("forwards session/cancel when no local active prompt is registered", () => + Effect.gen(function* () { + const provider = ProviderDriverKind.make("cursor"); + const threadId = ThreadId.make("standard-acp-cancel-without-local-prompt"); + const cancelCalled = yield* Deferred.make(); + const runtime = makeFakeAcpRuntime({ cancelCalled }); + + const adapter = yield* makeStandardAcpAdapter({ + provider, + runtimeLabel: "Fake ACP", + makeRuntime: () => Effect.succeed(runtime), + }); + + yield* adapter.startSession({ + threadId, + provider, + cwd: process.cwd(), + runtimeMode: "full-access", + }); + + yield* adapter.interruptTurn(threadId).pipe(Effect.timeout("1 second")); + yield* Deferred.await(cancelCalled).pipe(Effect.timeout("1 second")); + yield* adapter.stopSession(threadId); + }).pipe(Effect.scoped, Effect.provide(standardAcpAdapterTestLayer)), +); + +it.effect("stops the ACP session on interrupt when cancel is unsupported and opted in", () => + Effect.gen(function* () { + const provider = ProviderDriverKind.make("kiro"); + const threadId = ThreadId.make("standard-acp-cancel-unsupported-stops-session"); + const cancelCalled = yield* Deferred.make(); + const runtime = makeFakeAcpRuntime({ + cancelCalled, + cancel: Deferred.succeed(cancelCalled, undefined).pipe( + Effect.andThen(Effect.fail(AcpRequestError.methodNotFound("session/cancel"))), + ), + }); + + const adapter = yield* makeStandardAcpAdapter({ + provider, + runtimeLabel: "Fake ACP", + stopSessionOnInterruptCancelUnsupported: true, + makeRuntime: () => Effect.succeed(runtime), + }); + + yield* adapter.startSession({ + threadId, + provider, + cwd: process.cwd(), + runtimeMode: "full-access", + }); + + yield* adapter.interruptTurn(threadId).pipe(Effect.timeout("1 second")); + yield* Deferred.await(cancelCalled).pipe(Effect.timeout("1 second")); + + assert.isFalse(yield* adapter.hasSession(threadId)); + }).pipe(Effect.scoped, Effect.provide(standardAcpAdapterTestLayer)), +); + +it.effect("stops the ACP session on interrupt after a successful cancel write when opted in", () => + Effect.gen(function* () { + const provider = ProviderDriverKind.make("kiro"); + const threadId = ThreadId.make("standard-acp-cancel-write-stops-session"); + const cancelCalled = yield* Deferred.make(); + const runtime = makeFakeAcpRuntime({ cancelCalled }); + + const adapter = yield* makeStandardAcpAdapter({ + provider, + runtimeLabel: "Fake ACP", + stopSessionOnInterruptCancelUnsupported: true, + makeRuntime: () => Effect.succeed(runtime), + }); + + yield* adapter.startSession({ + threadId, + provider, + cwd: process.cwd(), + runtimeMode: "full-access", + }); + + yield* adapter.interruptTurn(threadId).pipe(Effect.timeout("1 second")); + yield* Deferred.await(cancelCalled).pipe(Effect.timeout("1 second")); + + assert.isFalse(yield* adapter.hasSession(threadId)); + }).pipe(Effect.scoped, Effect.provide(standardAcpAdapterTestLayer)), +); + +it.effect("routes text sent during an active ACP prompt through the active prompt hook", () => + Effect.gen(function* () { + const provider = ProviderDriverKind.make("cursor"); + const threadId = ThreadId.make("standard-acp-active-prompt-steering"); + const promptStarted = yield* Deferred.make(); + const promptResponse = yield* Deferred.make(); + const cancelCalled = yield* Deferred.make(); + let promptCallCount = 0; + const requests: Array<{ readonly method: string; readonly payload: unknown }> = []; + const runtime = makeFakeAcpRuntime({ + cancelCalled, + prompt: () => + Effect.sync(() => { + promptCallCount += 1; + }).pipe( + Effect.andThen(Deferred.succeed(promptStarted, undefined)), + Effect.andThen(Deferred.await(promptResponse)), + ), + request: (method, payload) => + Effect.sync(() => { + requests.push({ method, payload }); + return {}; + }), + }); + + const adapter = yield* makeStandardAcpAdapter({ + provider, + runtimeLabel: "Fake ACP", + activePromptMessageMethod: "_message/send", + sendMessageWhilePromptActive: ({ runtime, sessionId, content }) => + runtime.request("_message/send", { sessionId, content }), + makeRuntime: () => Effect.succeed(runtime), + }); + + yield* adapter.startSession({ + threadId, + provider, + cwd: process.cwd(), + runtimeMode: "full-access", + }); + + const sendTurnFiber = yield* adapter + .sendTurn({ + threadId, + input: "start a long prompt", + attachments: [], + }) + .pipe(Effect.forkChild); + + yield* Effect.yieldNow; + assert.isUndefined(sendTurnFiber.pollUnsafe()); + yield* Deferred.await(promptStarted).pipe(Effect.timeout("1 second")); + assert.equal(promptCallCount, 1); + + const steeringResult = yield* adapter + .sendTurn({ + threadId, + input: "steer the active prompt", + attachments: [], + }) + .pipe(Effect.timeout("1 second")); + + assert.equal(promptCallCount, 1); + assert.deepEqual(requests, [ + { + method: "_message/send", + payload: { sessionId: "fake-session", content: "steer the active prompt" }, + }, + ]); + + yield* Deferred.succeed(promptResponse, { stopReason: "end_turn" }); + const firstResult = yield* Fiber.join(sendTurnFiber); + + assert.equal(steeringResult.turnId, firstResult.turnId); + yield* adapter.stopSession(threadId); + }).pipe(Effect.scoped, Effect.provide(standardAcpAdapterTestLayer)), +); + +it.effect( + "routes attachments sent during an active ACP prompt through the active prompt hook", + () => + Effect.gen(function* () { + const provider = ProviderDriverKind.make("cursor"); + const threadId = ThreadId.make("standard-acp-active-prompt-attachment-steering"); + const promptStarted = yield* Deferred.make(); + const promptResponse = yield* Deferred.make(); + const cancelCalled = yield* Deferred.make(); + const serverConfig = yield* ServerConfig; + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const imageBytes = Buffer.from("fake image bytes"); + const attachment: ChatAttachment = { + type: "image", + id: "active-prompt-image", + name: "active-prompt-image.png", + mimeType: "image/png", + sizeBytes: imageBytes.byteLength, + }; + yield* fileSystem.writeFile( + path.join(serverConfig.attachmentsDir, `${attachment.id}.png`), + imageBytes, + ); + + let promptCallCount = 0; + const requests: Array<{ readonly method: string; readonly payload: unknown }> = []; + const runtime = makeFakeAcpRuntime({ + cancelCalled, + prompt: () => + Effect.sync(() => { + promptCallCount += 1; + }).pipe( + Effect.andThen(Deferred.succeed(promptStarted, undefined)), + Effect.andThen(Deferred.await(promptResponse)), + ), + request: (method, payload) => + Effect.sync(() => { + requests.push({ method, payload }); + return {}; + }), + }); + + const adapter = yield* makeStandardAcpAdapter({ + provider, + runtimeLabel: "Fake ACP", + activePromptMessageMethod: "_message/send", + sendMessageWhilePromptActive: ({ runtime, sessionId, content, contentBlocks }) => + runtime.request("_message/send", { + sessionId, + content: + contentBlocks.length === 1 && contentBlocks[0]?.type === "text" + ? content + : contentBlocks, + }), + makeRuntime: () => Effect.succeed(runtime), + }); + + yield* adapter.startSession({ + threadId, + provider, + cwd: process.cwd(), + runtimeMode: "full-access", + }); + + const sendTurnFiber = yield* adapter + .sendTurn({ + threadId, + input: "start a long prompt", + attachments: [], + }) + .pipe(Effect.forkChild); + + yield* Effect.yieldNow; + assert.isUndefined(sendTurnFiber.pollUnsafe()); + yield* Deferred.await(promptStarted).pipe(Effect.timeout("1 second")); + + const steeringResult = yield* adapter + .sendTurn({ + threadId, + input: "inspect this", + attachments: [attachment], + }) + .pipe(Effect.timeout("1 second")); + + assert.equal(promptCallCount, 1); + assert.deepEqual(requests, [ + { + method: "_message/send", + payload: { + sessionId: "fake-session", + content: [ + { type: "text", text: "inspect this" }, + { + type: "image", + data: imageBytes.toString("base64"), + mimeType: "image/png", + }, + ], + }, + }, + ]); + + yield* Deferred.succeed(promptResponse, { stopReason: "end_turn" }); + const firstResult = yield* Fiber.join(sendTurnFiber); + + assert.equal(steeringResult.turnId, firstResult.turnId); + yield* adapter.stopSession(threadId); + }).pipe(Effect.scoped, Effect.provide(standardAcpAdapterTestLayer)), +); + +it.effect("starts a fresh ACP prompt after the previous prompt completes", () => + Effect.gen(function* () { + const provider = ProviderDriverKind.make("cursor"); + const threadId = ThreadId.make("standard-acp-new-prompt-after-completion"); + const cancelCalled = yield* Deferred.make(); + let promptCallCount = 0; + const requests: Array<{ readonly method: string; readonly payload: unknown }> = []; + const runtime = makeFakeAcpRuntime({ + cancelCalled, + prompt: () => + Effect.sync(() => { + promptCallCount += 1; + return { stopReason: "end_turn" as const }; + }), + request: (method, payload) => + Effect.sync(() => { + requests.push({ method, payload }); + return {}; + }), + }); + + const adapter = yield* makeStandardAcpAdapter({ + provider, + runtimeLabel: "Fake ACP", + activePromptMessageMethod: "_message/send", + sendMessageWhilePromptActive: ({ runtime, sessionId, content }) => + runtime.request("_message/send", { sessionId, content }), + makeRuntime: () => Effect.succeed(runtime), + }); + + yield* adapter.startSession({ + threadId, + provider, + cwd: process.cwd(), + runtimeMode: "full-access", + }); + + yield* adapter.sendTurn({ + threadId, + input: "first prompt", + attachments: [], + }); + const sessionsAfterFirst = yield* adapter.listSessions(); + assert.isUndefined(sessionsAfterFirst[0]?.activeTurnId); + yield* adapter.sendTurn({ + threadId, + input: "second prompt", + attachments: [], + }); + + assert.equal(promptCallCount, 2); + assert.deepEqual(requests, []); + yield* adapter.stopSession(threadId); + }).pipe(Effect.scoped, Effect.provide(standardAcpAdapterTestLayer)), +); + +it.effect("restores the previous active ACP turn after an overlapping prompt fails", () => + Effect.gen(function* () { + const provider = ProviderDriverKind.make("cursor"); + const threadId = ThreadId.make("standard-acp-overlap-failure-restores-active-turn"); + const promptStarted = yield* Deferred.make(); + const promptResponse = yield* Deferred.make(); + const cancelCalled = yield* Deferred.make(); + let promptCallCount = 0; + const runtime = makeFakeAcpRuntime({ + cancelCalled, + prompt: () => + Effect.sync(() => { + promptCallCount += 1; + return promptCallCount; + }).pipe( + Effect.flatMap((callCount) => + callCount === 1 + ? Deferred.succeed(promptStarted, undefined).pipe( + Effect.andThen(Deferred.await(promptResponse)), + ) + : Effect.fail( + AcpRequestError.internalError("Internal error", "Prompt already in progress"), + ), + ), + ), + }); + + const adapter = yield* makeStandardAcpAdapter({ + provider, + runtimeLabel: "Fake ACP", + makeRuntime: () => Effect.succeed(runtime), + }); + + yield* adapter.startSession({ + threadId, + provider, + cwd: process.cwd(), + runtimeMode: "full-access", + }); + + const sendTurnFiber = yield* adapter + .sendTurn({ + threadId, + input: "start a long prompt", + attachments: [], + }) + .pipe(Effect.forkChild); + + yield* Deferred.await(promptStarted).pipe(Effect.timeout("1 second")); + const sessionsWhileFirstPromptRuns = yield* adapter.listSessions(); + const firstActiveTurnId = sessionsWhileFirstPromptRuns[0]?.activeTurnId; + assert.isDefined(firstActiveTurnId); + + const secondExit = yield* adapter + .sendTurn({ + threadId, + input: "overlapping prompt", + attachments: [], + }) + .pipe(Effect.exit, Effect.timeout("1 second")); + + assert.isTrue(Exit.isFailure(secondExit)); + const sessionsAfterOverlapFailure = yield* adapter.listSessions(); + assert.equal(sessionsAfterOverlapFailure[0]?.activeTurnId, firstActiveTurnId); + + yield* Deferred.succeed(promptResponse, { stopReason: "end_turn" }); + yield* Fiber.join(sendTurnFiber); + yield* adapter.stopSession(threadId); + }).pipe(Effect.scoped, Effect.provide(standardAcpAdapterTestLayer)), +); diff --git a/apps/server/src/provider/acp/StandardAcpAdapter.ts b/apps/server/src/provider/acp/StandardAcpAdapter.ts new file mode 100644 index 00000000000..0b8f0d14299 --- /dev/null +++ b/apps/server/src/provider/acp/StandardAcpAdapter.ts @@ -0,0 +1,1049 @@ +import { + ApprovalRequestId, + EventId, + type ProviderApprovalDecision, + type ProviderDriverKind, + type ProviderInteractionMode, + type ProviderRuntimeEvent, + type ProviderSession, + type ProviderUserInputAnswers, + ProviderInstanceId, + RuntimeRequestId, + type RuntimeMode, + type ThreadId, + TurnId, +} from "@t3tools/contracts"; +import * as DateTime from "effect/DateTime"; +import * as Deferred from "effect/Deferred"; +import * as Effect from "effect/Effect"; +import * as Exit from "effect/Exit"; +import * as Fiber from "effect/Fiber"; +import * as FileSystem from "effect/FileSystem"; +import * as Option from "effect/Option"; +import * as Path from "effect/Path"; +import * as PubSub from "effect/PubSub"; +import * as Random from "effect/Random"; +import * as Schema from "effect/Schema"; +import * as Scope from "effect/Scope"; +import * as Semaphore from "effect/Semaphore"; +import * as Stream from "effect/Stream"; +import * as SynchronizedRef from "effect/SynchronizedRef"; +import { ChildProcessSpawner } from "effect/unstable/process"; +import * as EffectAcpErrors from "effect-acp/errors"; +import type * as EffectAcpSchema from "effect-acp/schema"; + +import { resolveAttachmentPath } from "../../attachmentStore.ts"; +import { ServerConfig } from "../../config.ts"; +import { + ProviderAdapterProcessError, + ProviderAdapterRequestError, + ProviderAdapterSessionNotFoundError, + ProviderAdapterValidationError, + type ProviderAdapterError, +} from "../Errors.ts"; +import { acpPermissionOutcome, mapAcpToAdapterError } from "./AcpAdapterSupport.ts"; +import { + makeAcpAssistantItemEvent, + makeAcpContentDeltaEvent, + makeAcpPlanUpdatedEvent, + makeAcpRequestOpenedEvent, + makeAcpRequestResolvedEvent, + makeAcpToolCallEvent, +} from "./AcpCoreRuntimeEvents.ts"; +import { makeAcpNativeLoggers } from "./AcpNativeLogging.ts"; +import { + type AcpSessionMode, + type AcpSessionModeState, + parsePermissionRequest, +} from "./AcpRuntimeModel.ts"; +import type { AcpSessionRuntimeShape } from "./AcpSessionRuntime.ts"; +import type { AcpSessionRuntimeOptions } from "./AcpSessionRuntime.ts"; +import { type EventNdjsonLogger, makeEventNdjsonLogger } from "../Layers/EventNdjsonLogger.ts"; +import type { ProviderAdapterShape } from "../Services/ProviderAdapter.ts"; + +const encodeUnknownJsonStringExit = Schema.encodeUnknownExit(Schema.UnknownFromJsonString); +const STANDARD_ACP_RESUME_VERSION = 1 as const; +const ACP_PLAN_MODE_ALIASES = ["plan", "architect"]; +const ACP_IMPLEMENT_MODE_ALIASES = ["code", "agent", "default", "chat", "implement"]; +const ACP_APPROVAL_MODE_ALIASES = ["ask"]; + +export interface StandardAcpAdapterOptions { + readonly provider: ProviderDriverKind; + readonly runtimeLabel: string; + readonly environment?: NodeJS.ProcessEnv; + readonly nativeEventLogPath?: string; + readonly nativeEventLogger?: EventNdjsonLogger; + readonly instanceId?: ProviderInstanceId; + readonly activePromptMessageMethod?: string; + readonly supportedImageMimeTypes?: ReadonlySet; + readonly stopSessionOnInterruptCancelUnsupported?: boolean; + readonly sendMessageWhilePromptActive?: (input: { + readonly runtime: AcpSessionRuntimeShape; + readonly sessionId: string; + readonly content: string; + readonly contentBlocks: ReadonlyArray; + }) => Effect.Effect; + readonly makeRuntime: ( + input: { + readonly childProcessSpawner: ChildProcessSpawner.ChildProcessSpawner["Service"]; + readonly cwd: string; + readonly resumeSessionId?: string; + readonly clientInfo: { readonly name: string; readonly version: string }; + } & Pick, + ) => Effect.Effect; +} + +interface PendingApproval { + readonly decision: Deferred.Deferred; + readonly kind: string | "unknown"; +} + +interface PendingUserInput { + readonly answers: Deferred.Deferred; +} + +interface StandardAcpSessionContext { + readonly threadId: ThreadId; + session: ProviderSession; + readonly scope: Scope.Closeable; + readonly acp: AcpSessionRuntimeShape; + readonly acpSessionId: string; + readonly supportsImagePrompts: boolean; + notificationFiber: Fiber.Fiber | undefined; + readonly pendingApprovals: Map; + readonly pendingUserInputs: Map; + readonly turns: Array<{ id: TurnId; items: Array }>; + lastPlanFingerprint: string | undefined; + activeTurnId: TurnId | undefined; + activePrompt: + | { + readonly turnId: TurnId; + } + | undefined; + stopped: boolean; +} + +function encodeJsonStringForDiagnostics(input: unknown): string | undefined { + const result = encodeUnknownJsonStringExit(input); + return Exit.isSuccess(result) ? result.value : undefined; +} + +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +function parseStandardAcpResume(raw: unknown): { sessionId: string } | undefined { + if (!isRecord(raw)) return undefined; + if (raw.schemaVersion !== STANDARD_ACP_RESUME_VERSION) return undefined; + if (raw.protocol !== "acp") return undefined; + if (typeof raw.sessionId !== "string" || !raw.sessionId.trim()) return undefined; + return { sessionId: raw.sessionId.trim() }; +} + +function supportsImagePromptContent(initializeResult: EffectAcpSchema.InitializeResponse): boolean { + return initializeResult.agentCapabilities?.promptCapabilities?.image === true; +} + +function normalizeModeSearchText(mode: AcpSessionMode): string { + return [mode.id, mode.name, mode.description] + .filter((value): value is string => typeof value === "string" && value.length > 0) + .join(" ") + .toLowerCase() + .replace(/[^a-z0-9]+/g, " ") + .trim(); +} + +function findModeByAliases( + modes: ReadonlyArray, + aliases: ReadonlyArray, +): AcpSessionMode | undefined { + const normalizedAliases = aliases.map((alias) => alias.toLowerCase()); + for (const alias of normalizedAliases) { + const exact = modes.find((mode) => { + const id = mode.id.toLowerCase(); + const name = mode.name.toLowerCase(); + return id === alias || name === alias; + }); + if (exact) return exact; + } + for (const alias of normalizedAliases) { + const partial = modes.find((mode) => normalizeModeSearchText(mode).includes(alias)); + if (partial) return partial; + } + return undefined; +} + +function isPlanMode(mode: AcpSessionMode): boolean { + return findModeByAliases([mode], ACP_PLAN_MODE_ALIASES) !== undefined; +} + +function resolveRequestedModeId(input: { + readonly interactionMode: ProviderInteractionMode | undefined; + readonly runtimeMode: RuntimeMode; + readonly modeState: AcpSessionModeState | undefined; +}): string | undefined { + const modeState = input.modeState; + if (!modeState) return undefined; + + if (input.interactionMode === "plan") { + return findModeByAliases(modeState.availableModes, ACP_PLAN_MODE_ALIASES)?.id; + } + + if (input.runtimeMode === "approval-required") { + return ( + findModeByAliases(modeState.availableModes, ACP_APPROVAL_MODE_ALIASES)?.id ?? + findModeByAliases(modeState.availableModes, ACP_IMPLEMENT_MODE_ALIASES)?.id ?? + modeState.availableModes.find((mode) => !isPlanMode(mode))?.id ?? + modeState.currentModeId + ); + } + + return ( + findModeByAliases(modeState.availableModes, ACP_IMPLEMENT_MODE_ALIASES)?.id ?? + findModeByAliases(modeState.availableModes, ACP_APPROVAL_MODE_ALIASES)?.id ?? + modeState.availableModes.find((mode) => !isPlanMode(mode))?.id ?? + modeState.currentModeId + ); +} + +function applyRequestedSessionConfiguration(input: { + readonly runtime: AcpSessionRuntimeShape; + readonly runtimeMode: RuntimeMode; + readonly interactionMode: ProviderInteractionMode | undefined; + readonly model: string | undefined; + readonly mapError: (context: { + readonly cause: EffectAcpErrors.AcpError; + readonly method: "session/set_model" | "session/set_mode"; + }) => ProviderAdapterError; +}): Effect.Effect { + return Effect.gen(function* () { + if (input.model !== undefined) { + yield* input.runtime.setModel(input.model).pipe( + Effect.mapError((cause) => + input.mapError({ + cause, + method: "session/set_model", + }), + ), + ); + } + + const requestedModeId = resolveRequestedModeId({ + interactionMode: input.interactionMode, + runtimeMode: input.runtimeMode, + modeState: yield* input.runtime.getModeState, + }); + if (!requestedModeId) return; + + yield* input.runtime.setMode(requestedModeId).pipe( + Effect.mapError((cause) => + input.mapError({ + cause, + method: "session/set_mode", + }), + ), + ); + }); +} + +function selectAutoApprovedPermissionOption( + request: EffectAcpSchema.RequestPermissionRequest, +): string | undefined { + const allowAlwaysOption = request.options.find((option) => option.kind === "allow_always"); + if (typeof allowAlwaysOption?.optionId === "string" && allowAlwaysOption.optionId.trim()) { + return allowAlwaysOption.optionId.trim(); + } + + const allowOnceOption = request.options.find((option) => option.kind === "allow_once"); + if (typeof allowOnceOption?.optionId === "string" && allowOnceOption.optionId.trim()) { + return allowOnceOption.optionId.trim(); + } + + return undefined; +} + +function settlePendingApprovalsAsCancelled( + pendingApprovals: ReadonlyMap, +): Effect.Effect { + return Effect.forEach( + Array.from(pendingApprovals.values()), + (pending) => Deferred.succeed(pending.decision, "cancel").pipe(Effect.ignore), + { discard: true }, + ); +} + +function settlePendingUserInputsAsEmptyAnswers( + pendingUserInputs: ReadonlyMap, +): Effect.Effect { + return Effect.forEach( + Array.from(pendingUserInputs.values()), + (pending) => Deferred.succeed(pending.answers, {}).pipe(Effect.ignore), + { discard: true }, + ); +} + +export function makeStandardAcpAdapter( + options: StandardAcpAdapterOptions, +): Effect.Effect< + ProviderAdapterShape, + never, + | ChildProcessSpawner.ChildProcessSpawner + | FileSystem.FileSystem + | Path.Path + | ServerConfig + | Scope.Scope +> { + return Effect.gen(function* () { + const provider = options.provider; + const boundInstanceId = options.instanceId ?? ProviderInstanceId.make(provider); + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const childProcessSpawner = yield* ChildProcessSpawner.ChildProcessSpawner; + const serverConfig = yield* Effect.service(ServerConfig); + const nativeEventLogger = + options.nativeEventLogger ?? + (options.nativeEventLogPath !== undefined + ? yield* makeEventNdjsonLogger(options.nativeEventLogPath, { + stream: "native", + }) + : undefined); + const managedNativeEventLogger = + options.nativeEventLogger === undefined ? nativeEventLogger : undefined; + + const sessions = new Map(); + const threadLocksRef = yield* SynchronizedRef.make(new Map()); + const runtimeEventPubSub = yield* PubSub.unbounded(); + + const nowIso = Effect.map(DateTime.now, DateTime.formatIso); + const nextEventId = Effect.map(Random.nextUUIDv4, (id) => EventId.make(id)); + const makeEventStamp = () => Effect.all({ eventId: nextEventId, createdAt: nowIso }); + + const offerRuntimeEvent = (event: ProviderRuntimeEvent) => + PubSub.publish(runtimeEventPubSub, event).pipe(Effect.asVoid); + + const getThreadSemaphore = (threadId: string) => + SynchronizedRef.modifyEffect(threadLocksRef, (current) => { + const existing: Option.Option = Option.fromNullishOr( + current.get(threadId), + ); + return Option.match(existing, { + onNone: () => + Semaphore.make(1).pipe( + Effect.map((semaphore) => { + const next = new Map(current); + next.set(threadId, semaphore); + return [semaphore, next] as const; + }), + ), + onSome: (semaphore) => Effect.succeed([semaphore, current] as const), + }); + }); + + const withThreadLock = (threadId: string, effect: Effect.Effect) => + Effect.flatMap(getThreadSemaphore(threadId), (semaphore) => semaphore.withPermit(effect)); + + const logNative = (threadId: ThreadId, method: string, payload: unknown) => + Effect.gen(function* () { + if (!nativeEventLogger) return; + const observedAt = yield* nowIso; + yield* nativeEventLogger.write( + { + observedAt, + event: { + id: yield* Random.nextUUIDv4, + kind: "notification", + provider, + createdAt: observedAt, + method, + threadId, + payload, + }, + }, + threadId, + ); + }); + + const emitPlanUpdate = ( + ctx: StandardAcpSessionContext, + payload: { + readonly explanation?: string | null; + readonly plan: ReadonlyArray<{ + readonly step: string; + readonly status: "pending" | "inProgress" | "completed"; + }>; + }, + rawPayload: unknown, + method: string, + ) => + Effect.gen(function* () { + const fingerprint = `${ctx.activeTurnId ?? "no-turn"}:${encodeJsonStringForDiagnostics(payload) ?? "[unserializable payload]"}`; + if (ctx.lastPlanFingerprint === fingerprint) return; + ctx.lastPlanFingerprint = fingerprint; + yield* offerRuntimeEvent( + makeAcpPlanUpdatedEvent({ + stamp: yield* makeEventStamp(), + provider, + threadId: ctx.threadId, + turnId: ctx.activeTurnId, + payload, + source: "acp.jsonrpc", + method, + rawPayload, + }), + ); + }); + + const requireSession = ( + threadId: ThreadId, + ): Effect.Effect => { + const ctx = sessions.get(threadId); + if (!ctx || ctx.stopped) { + return Effect.fail(new ProviderAdapterSessionNotFoundError({ provider, threadId })); + } + return Effect.succeed(ctx); + }; + + const buildPromptContentBlocks = ( + input: Parameters["sendTurn"]>[0], + method: string, + supportsImagePrompts: boolean, + ) => + Effect.gen(function* () { + const promptParts: Array = []; + if (input.input?.trim()) { + promptParts.push({ type: "text", text: input.input.trim() }); + } + if (input.attachments && input.attachments.length > 0) { + if (!supportsImagePrompts) { + return yield* new ProviderAdapterValidationError({ + provider, + operation: "sendTurn", + issue: `${options.runtimeLabel} session does not support image attachments.`, + }); + } + for (const attachment of input.attachments) { + const supportedImageMimeTypes = options.supportedImageMimeTypes; + if ( + supportedImageMimeTypes && + !supportedImageMimeTypes.has(attachment.mimeType.toLowerCase()) + ) { + return yield* new ProviderAdapterRequestError({ + provider, + method, + detail: `Unsupported ${options.runtimeLabel} image attachment type '${attachment.mimeType}'.`, + }); + } + const attachmentPath = resolveAttachmentPath({ + attachmentsDir: serverConfig.attachmentsDir, + attachment, + }); + if (!attachmentPath) { + return yield* new ProviderAdapterRequestError({ + provider, + method, + detail: `Invalid attachment id '${attachment.id}'.`, + }); + } + const bytes = yield* fileSystem.readFile(attachmentPath).pipe( + Effect.mapError( + (cause) => + new ProviderAdapterRequestError({ + provider, + method, + detail: cause.message, + cause, + }), + ), + ); + promptParts.push({ + type: "image", + data: Buffer.from(bytes).toString("base64"), + mimeType: attachment.mimeType, + }); + } + } + + if (promptParts.length === 0) { + return yield* new ProviderAdapterValidationError({ + provider, + operation: "sendTurn", + issue: "Turn requires non-empty text or attachments.", + }); + } + + return promptParts; + }); + + const stopSessionInternal = (ctx: StandardAcpSessionContext) => + Effect.gen(function* () { + if (ctx.stopped) return; + ctx.stopped = true; + yield* settlePendingApprovalsAsCancelled(ctx.pendingApprovals); + yield* settlePendingUserInputsAsEmptyAnswers(ctx.pendingUserInputs); + if (ctx.notificationFiber) { + yield* Fiber.interrupt(ctx.notificationFiber); + } + yield* Effect.ignore(Scope.close(ctx.scope, Exit.void)); + sessions.delete(ctx.threadId); + yield* offerRuntimeEvent({ + type: "session.exited", + ...(yield* makeEventStamp()), + provider, + threadId: ctx.threadId, + payload: { exitKind: "graceful" }, + }); + }); + + const startSession: ProviderAdapterShape["startSession"] = (input) => + withThreadLock( + input.threadId, + Effect.gen(function* () { + if (input.provider !== undefined && input.provider !== provider) { + return yield* new ProviderAdapterValidationError({ + provider, + operation: "startSession", + issue: `Expected provider '${provider}' but received '${input.provider}'.`, + }); + } + if (!input.cwd?.trim()) { + return yield* new ProviderAdapterValidationError({ + provider, + operation: "startSession", + issue: "cwd is required and must be non-empty.", + }); + } + + const cwd = path.resolve(input.cwd.trim()); + const selectedModel = + input.modelSelection?.instanceId === boundInstanceId + ? input.modelSelection.model + : undefined; + const existing = sessions.get(input.threadId); + if (existing && !existing.stopped) { + yield* stopSessionInternal(existing); + } + + const pendingApprovals = new Map(); + const pendingUserInputs = new Map(); + const sessionScope = yield* Scope.make("sequential"); + let sessionScopeTransferred = false; + yield* Effect.addFinalizer(() => + sessionScopeTransferred ? Effect.void : Scope.close(sessionScope, Exit.void), + ); + let ctx!: StandardAcpSessionContext; + + const resumeSessionId = parseStandardAcpResume(input.resumeCursor)?.sessionId; + const acpNativeLoggers = makeAcpNativeLoggers({ + nativeEventLogger, + provider, + threadId: input.threadId, + }); + const acp = yield* options + .makeRuntime({ + childProcessSpawner, + cwd, + ...(resumeSessionId ? { resumeSessionId } : {}), + clientInfo: { name: "t3-code", version: "0.0.0" }, + ...acpNativeLoggers, + }) + .pipe( + Effect.provideService(Scope.Scope, sessionScope), + Effect.mapError( + (cause) => + new ProviderAdapterProcessError({ + provider, + threadId: input.threadId, + detail: cause.message, + cause, + }), + ), + ); + + const started = yield* Effect.gen(function* () { + yield* acp.handleRequestPermission((params) => + Effect.gen(function* () { + yield* logNative(input.threadId, "session/request_permission", params); + if (input.runtimeMode === "full-access") { + const autoApprovedOptionId = selectAutoApprovedPermissionOption(params); + if (autoApprovedOptionId !== undefined) { + return { + outcome: { + outcome: "selected" as const, + optionId: autoApprovedOptionId, + }, + }; + } + } + const permissionRequest = parsePermissionRequest(params); + const requestId = ApprovalRequestId.make(crypto.randomUUID()); + const runtimeRequestId = RuntimeRequestId.make(requestId); + const decision = yield* Deferred.make(); + pendingApprovals.set(requestId, { + decision, + kind: permissionRequest.kind, + }); + yield* offerRuntimeEvent( + makeAcpRequestOpenedEvent({ + stamp: yield* makeEventStamp(), + provider, + threadId: input.threadId, + turnId: ctx?.activeTurnId, + requestId: runtimeRequestId, + permissionRequest, + detail: + permissionRequest.detail ?? + encodeJsonStringForDiagnostics(params)?.slice(0, 2000) ?? + "[unserializable params]", + args: params, + source: "acp.jsonrpc", + method: "session/request_permission", + rawPayload: params, + }), + ); + const resolved = yield* Deferred.await(decision); + pendingApprovals.delete(requestId); + yield* offerRuntimeEvent( + makeAcpRequestResolvedEvent({ + stamp: yield* makeEventStamp(), + provider, + threadId: input.threadId, + turnId: ctx?.activeTurnId, + requestId: runtimeRequestId, + permissionRequest, + decision: resolved, + }), + ); + return { + outcome: + resolved === "cancel" + ? ({ outcome: "cancelled" } as const) + : { + outcome: "selected" as const, + optionId: acpPermissionOutcome(resolved, params.options), + }, + }; + }), + ); + return yield* acp.start(); + }).pipe( + Effect.mapError((error) => + mapAcpToAdapterError(provider, input.threadId, "session/start", error), + ), + ); + + yield* applyRequestedSessionConfiguration({ + runtime: acp, + runtimeMode: input.runtimeMode, + interactionMode: undefined, + model: selectedModel, + mapError: ({ cause, method }) => + mapAcpToAdapterError(provider, input.threadId, method, cause), + }); + + const now = yield* nowIso; + const session: ProviderSession = { + provider, + providerInstanceId: boundInstanceId, + status: "ready", + runtimeMode: input.runtimeMode, + cwd, + model: selectedModel, + threadId: input.threadId, + resumeCursor: { + schemaVersion: STANDARD_ACP_RESUME_VERSION, + protocol: "acp", + sessionId: started.sessionId, + }, + createdAt: now, + updatedAt: now, + }; + + ctx = { + threadId: input.threadId, + session, + scope: sessionScope, + acp, + acpSessionId: started.sessionId, + supportsImagePrompts: supportsImagePromptContent(started.initializeResult), + notificationFiber: undefined, + pendingApprovals, + pendingUserInputs, + turns: [], + lastPlanFingerprint: undefined, + activeTurnId: undefined, + activePrompt: undefined, + stopped: false, + }; + + const nf = yield* Stream.runDrain( + Stream.mapEffect(acp.getEvents(), (event) => + Effect.gen(function* () { + switch (event._tag) { + case "ModeChanged": + return; + case "AssistantItemStarted": + yield* offerRuntimeEvent( + makeAcpAssistantItemEvent({ + stamp: yield* makeEventStamp(), + provider, + threadId: ctx.threadId, + turnId: ctx.activeTurnId, + itemId: event.itemId, + lifecycle: "item.started", + }), + ); + return; + case "AssistantItemCompleted": + yield* offerRuntimeEvent( + makeAcpAssistantItemEvent({ + stamp: yield* makeEventStamp(), + provider, + threadId: ctx.threadId, + turnId: ctx.activeTurnId, + itemId: event.itemId, + lifecycle: "item.completed", + }), + ); + return; + case "PlanUpdated": + yield* logNative(ctx.threadId, "session/update", event.rawPayload); + yield* emitPlanUpdate(ctx, event.payload, event.rawPayload, "session/update"); + return; + case "ToolCallUpdated": + yield* logNative(ctx.threadId, "session/update", event.rawPayload); + yield* offerRuntimeEvent( + makeAcpToolCallEvent({ + stamp: yield* makeEventStamp(), + provider, + threadId: ctx.threadId, + turnId: ctx.activeTurnId, + toolCall: event.toolCall, + rawPayload: event.rawPayload, + }), + ); + return; + case "ContentDelta": + yield* logNative(ctx.threadId, "session/update", event.rawPayload); + yield* offerRuntimeEvent( + makeAcpContentDeltaEvent({ + stamp: yield* makeEventStamp(), + provider, + threadId: ctx.threadId, + turnId: ctx.activeTurnId, + ...(event.itemId ? { itemId: event.itemId } : {}), + text: event.text, + rawPayload: event.rawPayload, + }), + ); + return; + } + }), + ), + ).pipe(Effect.forkChild); + + ctx.notificationFiber = nf; + sessions.set(input.threadId, ctx); + sessionScopeTransferred = true; + + yield* offerRuntimeEvent({ + type: "session.started", + ...(yield* makeEventStamp()), + provider, + threadId: input.threadId, + payload: { resume: started.initializeResult }, + }); + yield* offerRuntimeEvent({ + type: "session.state.changed", + ...(yield* makeEventStamp()), + provider, + threadId: input.threadId, + payload: { state: "ready", reason: `${options.runtimeLabel} ACP session ready` }, + }); + yield* offerRuntimeEvent({ + type: "thread.started", + ...(yield* makeEventStamp()), + provider, + threadId: input.threadId, + payload: { providerThreadId: started.sessionId }, + }); + + return session; + }).pipe(Effect.scoped), + ); + + const sendTurn: ProviderAdapterShape["sendTurn"] = (input) => + Effect.gen(function* () { + const ctx = yield* requireSession(input.threadId); + const activePromptTurnId = + ctx.activePrompt?.turnId ?? ctx.activeTurnId ?? ctx.session.activeTurnId; + if (activePromptTurnId && options.sendMessageWhilePromptActive) { + const method = options.activePromptMessageMethod ?? "session/message"; + const contentBlocks = yield* buildPromptContentBlocks( + input, + method, + ctx.supportsImagePrompts, + ); + const content = contentBlocks + .filter( + (block): block is Extract => + block.type === "text", + ) + .map((block) => block.text) + .join("\n") + .trim(); + yield* options + .sendMessageWhilePromptActive({ + runtime: ctx.acp, + sessionId: ctx.acpSessionId, + content, + contentBlocks, + }) + .pipe( + Effect.mapError((error) => + mapAcpToAdapterError(provider, input.threadId, method, error), + ), + ); + + ctx.activePrompt = { turnId: activePromptTurnId }; + ctx.activeTurnId = activePromptTurnId; + ctx.session = { + ...ctx.session, + activeTurnId: activePromptTurnId, + updatedAt: yield* nowIso, + }; + + yield* offerRuntimeEvent({ + type: "turn.started", + ...(yield* makeEventStamp()), + provider, + threadId: input.threadId, + turnId: activePromptTurnId, + payload: {}, + }); + + return { + threadId: input.threadId, + turnId: activePromptTurnId, + resumeCursor: ctx.session.resumeCursor, + }; + } + + const turnId = TurnId.make(crypto.randomUUID()); + const turnModel = + input.modelSelection?.instanceId === boundInstanceId + ? input.modelSelection.model + : undefined; + const model = turnModel ?? ctx.session.model; + yield* applyRequestedSessionConfiguration({ + runtime: ctx.acp, + runtimeMode: ctx.session.runtimeMode, + interactionMode: input.interactionMode, + model, + mapError: ({ cause, method }) => + mapAcpToAdapterError(provider, input.threadId, method, cause), + }); + + const promptParts = yield* buildPromptContentBlocks( + input, + "session/prompt", + ctx.supportsImagePrompts, + ); + + const previousActivePrompt = ctx.activePrompt; + const previousActiveTurnId = ctx.activeTurnId; + const previousSessionActiveTurnId = ctx.session.activeTurnId; + ctx.activePrompt = { turnId }; + ctx.activeTurnId = turnId; + ctx.lastPlanFingerprint = undefined; + ctx.session = { + ...ctx.session, + activeTurnId: turnId, + updatedAt: yield* nowIso, + }; + + yield* offerRuntimeEvent({ + type: "turn.started", + ...(yield* makeEventStamp()), + provider, + threadId: input.threadId, + turnId, + payload: model ? { model } : {}, + }); + + const result = yield* ctx.acp + .prompt({ + prompt: promptParts, + }) + .pipe( + Effect.mapError((error) => + mapAcpToAdapterError(provider, input.threadId, "session/prompt", error), + ), + Effect.ensuring( + Effect.sync(() => { + if (ctx.activePrompt?.turnId === turnId) { + ctx.activePrompt = previousActivePrompt; + } + if (ctx.activeTurnId === turnId) { + ctx.activeTurnId = previousActiveTurnId; + } + if (ctx.session.activeTurnId === turnId) { + const nextSession = { ...ctx.session }; + if (previousSessionActiveTurnId !== undefined) { + nextSession.activeTurnId = previousSessionActiveTurnId; + } else { + delete nextSession.activeTurnId; + } + ctx.session = nextSession; + } + }), + ), + ); + + ctx.turns.push({ id: turnId, items: [{ prompt: promptParts, result }] }); + const nextSession = { + ...ctx.session, + updatedAt: yield* nowIso, + ...(model ? { model } : {}), + }; + delete nextSession.activeTurnId; + ctx.session = nextSession; + + yield* offerRuntimeEvent({ + type: "turn.completed", + ...(yield* makeEventStamp()), + provider, + threadId: input.threadId, + turnId, + payload: { + state: result.stopReason === "cancelled" ? "cancelled" : "completed", + stopReason: result.stopReason ?? null, + }, + }); + + return { + threadId: input.threadId, + turnId, + resumeCursor: ctx.session.resumeCursor, + }; + }); + + const interruptTurn: ProviderAdapterShape["interruptTurn"] = (threadId) => + Effect.gen(function* () { + const ctx = yield* requireSession(threadId); + yield* settlePendingApprovalsAsCancelled(ctx.pendingApprovals); + yield* settlePendingUserInputsAsEmptyAnswers(ctx.pendingUserInputs); + // ACP owns prompt termination. Keep the turn active until the prompt + // call returns; still forward cancel when local prompt state is missing + // because a resumed provider may have remote work in flight. + yield* Effect.ignore(ctx.acp.cancel); + if (options.stopSessionOnInterruptCancelUnsupported) { + yield* stopSessionInternal(ctx); + } + }); + + const respondToRequest: ProviderAdapterShape["respondToRequest"] = ( + threadId, + requestId, + decision, + ) => + Effect.gen(function* () { + const ctx = yield* requireSession(threadId); + const pending = ctx.pendingApprovals.get(requestId); + if (!pending) { + return yield* new ProviderAdapterRequestError({ + provider, + method: "session/request_permission", + detail: `Unknown pending approval request: ${requestId}`, + }); + } + yield* Deferred.succeed(pending.decision, decision); + }); + + const respondToUserInput: ProviderAdapterShape["respondToUserInput"] = ( + threadId, + requestId, + answers, + ) => + Effect.gen(function* () { + const ctx = yield* requireSession(threadId); + const pending = ctx.pendingUserInputs.get(requestId); + if (!pending) { + return yield* new ProviderAdapterRequestError({ + provider, + method: "elicitation", + detail: `Unknown pending user-input request: ${requestId}`, + }); + } + yield* Deferred.succeed(pending.answers, answers); + }); + + const readThread: ProviderAdapterShape["readThread"] = (threadId) => + Effect.gen(function* () { + const ctx = yield* requireSession(threadId); + return { threadId, turns: ctx.turns }; + }); + + const rollbackThread: ProviderAdapterShape["rollbackThread"] = ( + threadId, + numTurns, + ) => + Effect.gen(function* () { + const ctx = yield* requireSession(threadId); + if (!Number.isInteger(numTurns) || numTurns < 1) { + return yield* new ProviderAdapterValidationError({ + provider, + operation: "rollbackThread", + issue: "numTurns must be an integer >= 1.", + }); + } + const nextLength = Math.max(0, ctx.turns.length - numTurns); + ctx.turns.splice(nextLength); + return { threadId, turns: ctx.turns }; + }); + + const stopSession: ProviderAdapterShape["stopSession"] = (threadId) => + withThreadLock( + threadId, + Effect.gen(function* () { + const ctx = yield* requireSession(threadId); + yield* stopSessionInternal(ctx); + }), + ); + + const listSessions: ProviderAdapterShape["listSessions"] = () => + Effect.sync(() => Array.from(sessions.values(), (c) => ({ ...c.session }))); + + const hasSession: ProviderAdapterShape["hasSession"] = (threadId) => + Effect.sync(() => { + const c = sessions.get(threadId); + return c !== undefined && !c.stopped; + }); + + const stopAll: ProviderAdapterShape["stopAll"] = () => + Effect.forEach(sessions.values(), stopSessionInternal, { discard: true }); + + yield* Effect.addFinalizer(() => + Effect.forEach(sessions.values(), stopSessionInternal, { discard: true }).pipe( + Effect.tap(() => PubSub.shutdown(runtimeEventPubSub)), + Effect.tap(() => managedNativeEventLogger?.close() ?? Effect.void), + ), + ); + + return { + provider, + capabilities: { sessionModelSwitch: "in-session" }, + startSession, + sendTurn, + interruptTurn, + readThread, + rollbackThread, + respondToRequest, + respondToUserInput, + stopSession, + listSessions, + hasSession, + stopAll, + streamEvents: Stream.fromPubSub(runtimeEventPubSub), + } satisfies ProviderAdapterShape; + }); +} diff --git a/apps/server/src/provider/builtInDrivers.ts b/apps/server/src/provider/builtInDrivers.ts index 5af56dc6b0e..4692b007a4b 100644 --- a/apps/server/src/provider/builtInDrivers.ts +++ b/apps/server/src/provider/builtInDrivers.ts @@ -23,6 +23,7 @@ import { ClaudeDriver, type ClaudeDriverEnv } from "./Drivers/ClaudeDriver.ts"; import { CodexDriver, type CodexDriverEnv } from "./Drivers/CodexDriver.ts"; import { CursorDriver, type CursorDriverEnv } from "./Drivers/CursorDriver.ts"; +import { KiroDriver, type KiroDriverEnv } from "./Drivers/KiroDriver.ts"; import { OpenCodeDriver, type OpenCodeDriverEnv } from "./Drivers/OpenCodeDriver.ts"; import type { AnyProviderDriver } from "./ProviderDriver.ts"; @@ -35,6 +36,7 @@ export type BuiltInDriversEnv = | ClaudeDriverEnv | CodexDriverEnv | CursorDriverEnv + | KiroDriverEnv | OpenCodeDriverEnv; /** @@ -46,5 +48,6 @@ export const BUILT_IN_DRIVERS: ReadonlyArray .filter((token) => token.length > 0) .toSorted(); -const assertBrowserApiCorsHeaders = (headers: Headers) => { - assert.equal(headers.get("access-control-allow-origin"), "*"); +const assertBrowserApiCorsHeaders = (headers: Headers, options?: { readonly origin?: string }) => { + assert.equal(headers.get("access-control-allow-origin"), options?.origin ?? "*"); + if (options?.origin !== undefined) { + assert.equal(headers.get("access-control-allow-credentials"), "true"); + assert.include(splitHeaderTokens(headers.get("vary")), "Origin"); + } assert.deepEqual(splitHeaderTokens(headers.get("access-control-allow-methods")), [ "GET", "OPTIONS", @@ -922,7 +926,8 @@ const assertBrowserApiCorsHeaders = (headers: Headers) => { "traceparent", ]); }; -const crossOriginClientOrigin = "http://remote-client.test:3773"; +const trustedHostedClientOrigin = "https://app.t3.codes"; +const untrustedBrowserClientOrigin = "https://evil.example"; const getWsServerUrl = ( pathname = "", @@ -1053,7 +1058,7 @@ it.layer(NodeServices.layer)("server router seam", (it) => { const response = yield* Effect.promise(() => fetch(url, { headers: { - origin: crossOriginClientOrigin, + origin: trustedHostedClientOrigin, }, }), ); @@ -1062,7 +1067,7 @@ it.layer(NodeServices.layer)("server router seam", (it) => { )) as typeof testEnvironmentDescriptor; assert.equal(response.status, 200); - assertBrowserApiCorsHeaders(response.headers); + assertBrowserApiCorsHeaders(response.headers, { origin: trustedHostedClientOrigin }); assert.deepEqual(body, testEnvironmentDescriptor); }).pipe(Effect.provide(NodeHttpServer.layerTest)), ); @@ -1194,7 +1199,7 @@ it.layer(NodeServices.layer)("server router seam", (it) => { Effect.gen(function* () { yield* buildAppUnderTest(); - const origin = crossOriginClientOrigin; + const origin = trustedHostedClientOrigin; const { response: bootstrapResponse, body: bootstrapBody } = yield* bootstrapBearerSession( defaultDesktopBootstrapToken, { @@ -1203,7 +1208,7 @@ it.layer(NodeServices.layer)("server router seam", (it) => { ); assert.equal(bootstrapResponse.status, 200); - assertBrowserApiCorsHeaders(bootstrapResponse.headers); + assertBrowserApiCorsHeaders(bootstrapResponse.headers, { origin }); assert.equal(bootstrapBody.authenticated, true); assert.equal(typeof bootstrapBody.sessionToken, "string"); @@ -1222,7 +1227,7 @@ it.layer(NodeServices.layer)("server router seam", (it) => { }; assert.equal(sessionResponse.status, 200); - assertBrowserApiCorsHeaders(sessionResponse.headers); + assertBrowserApiCorsHeaders(sessionResponse.headers, { origin }); assert.equal(sessionBody.authenticated, true); assert.equal(sessionBody.sessionMethod, "bearer-session-token"); @@ -1241,7 +1246,7 @@ it.layer(NodeServices.layer)("server router seam", (it) => { }; assert.equal(wsTokenResponse.status, 200); - assertBrowserApiCorsHeaders(wsTokenResponse.headers); + assertBrowserApiCorsHeaders(wsTokenResponse.headers, { origin }); assert.equal(typeof wsTokenBody.token, "string"); }).pipe(Effect.provide(NodeHttpServer.layerTest)), ); @@ -1257,7 +1262,7 @@ it.layer(NodeServices.layer)("server router seam", (it) => { fetch(wsTokenUrl, { method: "OPTIONS", headers: { - origin: crossOriginClientOrigin, + origin: trustedHostedClientOrigin, "access-control-request-method": "POST", "access-control-request-headers": "authorization", }, @@ -1265,7 +1270,7 @@ it.layer(NodeServices.layer)("server router seam", (it) => { ); assert.equal(response.status, 204); - assertBrowserApiCorsHeaders(response.headers); + assertBrowserApiCorsHeaders(response.headers, { origin: trustedHostedClientOrigin }); }).pipe(Effect.provide(NodeHttpServer.layerTest)), ); @@ -1278,7 +1283,30 @@ it.layer(NodeServices.layer)("server router seam", (it) => { fetch(wsTokenUrl, { method: "POST", headers: { - origin: crossOriginClientOrigin, + origin: trustedHostedClientOrigin, + }, + }), + ); + const body = (yield* Effect.promise(() => response.json())) as { + readonly error?: string; + }; + + assert.equal(response.status, 401); + assertBrowserApiCorsHeaders(response.headers, { origin: trustedHostedClientOrigin }); + assert.equal(body.error, "Authentication required."); + }).pipe(Effect.provide(NodeHttpServer.layerTest)), + ); + + it.effect("does not reflect untrusted origins on remote websocket-token auth failures", () => + Effect.gen(function* () { + yield* buildAppUnderTest(); + + const wsTokenUrl = yield* getHttpServerUrl("/api/auth/ws-token"); + const response = yield* Effect.promise(() => + fetch(wsTokenUrl, { + method: "POST", + headers: { + origin: untrustedBrowserClientOrigin, }, }), ); @@ -1288,6 +1316,8 @@ it.layer(NodeServices.layer)("server router seam", (it) => { assert.equal(response.status, 401); assertBrowserApiCorsHeaders(response.headers); + assert.isNull(response.headers.get("access-control-allow-credentials")); + assert.notInclude(splitHeaderTokens(response.headers.get("vary")), "Origin"); assert.equal(body.error, "Authentication required."); }).pipe(Effect.provide(NodeHttpServer.layerTest)), ); @@ -1881,7 +1911,8 @@ it.layer(NodeServices.layer)("server router seam", (it) => { }); assert.equal(response.status, 204); - assert.equal(response.headers["access-control-allow-origin"], "*"); + assert.equal(response.headers["access-control-allow-origin"], "http://localhost:5733"); + assert.equal(response.headers["access-control-allow-credentials"], "true"); assert.deepEqual(localTraceRecords, [ { type: "otlp-span", @@ -1947,7 +1978,9 @@ it.layer(NodeServices.layer)("server router seam", (it) => { ); assert.equal(response.status, 204); - assert.equal(response.headers.get("access-control-allow-origin"), "*"); + assert.equal(response.headers.get("access-control-allow-origin"), "http://localhost:5733"); + assert.equal(response.headers.get("access-control-allow-credentials"), "true"); + assert.include(splitHeaderTokens(response.headers.get("vary")), "Origin"); assert.deepEqual(splitHeaderTokens(response.headers.get("access-control-allow-methods")), [ "GET", "OPTIONS", diff --git a/apps/server/src/terminal/Layers/Manager.test.ts b/apps/server/src/terminal/Layers/Manager.test.ts index b81d6596592..39ee7074d49 100644 --- a/apps/server/src/terminal/Layers/Manager.test.ts +++ b/apps/server/src/terminal/Layers/Manager.test.ts @@ -1,4 +1,5 @@ import * as NodeServices from "@effect/platform-node/NodeServices"; +import * as NodeOS from "node:os"; import { assert, it } from "@effect/vitest"; import { DEFAULT_TERMINAL_ID, @@ -1025,6 +1026,7 @@ it.layer( setEnv("PORT", "5173"); setEnv("T3CODE_PORT", "3773"); setEnv("VITE_DEV_SERVER_URL", "http://localhost:5173"); + setEnv("HOME", "/private/tmp/t3code-home"); setEnv("TEST_TERMINAL_KEEP", "keep-me"); try { @@ -1037,6 +1039,7 @@ it.layer( expect(spawnInput.env.PORT).toBeUndefined(); expect(spawnInput.env.T3CODE_PORT).toBeUndefined(); expect(spawnInput.env.VITE_DEV_SERVER_URL).toBeUndefined(); + expect(spawnInput.env.HOME).toBe(NodeOS.userInfo().homedir); expect(spawnInput.env.TEST_TERMINAL_KEEP).toBe("keep-me"); } finally { restoreEnv(); diff --git a/apps/server/src/terminal/Layers/Manager.ts b/apps/server/src/terminal/Layers/Manager.ts index 9f20ebc8315..26473d01ef4 100644 --- a/apps/server/src/terminal/Layers/Manager.ts +++ b/apps/server/src/terminal/Layers/Manager.ts @@ -21,6 +21,7 @@ import * as Semaphore from "effect/Semaphore"; import * as SynchronizedRef from "effect/SynchronizedRef"; import { ServerConfig } from "../../config.ts"; +import { resolveLocalUserHome } from "../../localUserEnvironment.ts"; import { increment, terminalRestartsTotal, @@ -705,6 +706,10 @@ function createTerminalSpawnEnv( if (shouldExcludeTerminalEnvKey(key)) continue; spawnEnv[key] = value; } + const localHome = resolveLocalUserHome(baseEnv); + if (localHome) { + spawnEnv.HOME = localHome; + } if (runtimeEnv) { for (const [key, value] of Object.entries(runtimeEnv)) { spawnEnv[key] = value; diff --git a/apps/server/src/textGeneration/KiroTextGeneration.ts b/apps/server/src/textGeneration/KiroTextGeneration.ts new file mode 100644 index 00000000000..ceec34005e9 --- /dev/null +++ b/apps/server/src/textGeneration/KiroTextGeneration.ts @@ -0,0 +1,266 @@ +import * as Effect from "effect/Effect"; +import * as Option from "effect/Option"; +import * as Ref from "effect/Ref"; +import * as Schema from "effect/Schema"; +import { ChildProcessSpawner } from "effect/unstable/process"; + +import { type KiroSettings, type ModelSelection, TextGenerationError } from "@t3tools/contracts"; +import { sanitizeBranchFragment, sanitizeFeatureBranchName } from "@t3tools/shared/git"; +import { extractJsonObject } from "@t3tools/shared/schemaJson"; + +import { makeKiroAcpRuntime } from "../provider/acp/KiroAcpSupport.ts"; +import { + buildBranchNamePrompt, + buildCommitMessagePrompt, + buildPrContentPrompt, + buildThreadTitlePrompt, +} from "./TextGenerationPrompts.ts"; +import { + sanitizeCommitSubject, + sanitizePrTitle, + sanitizeThreadTitle, +} from "./TextGenerationUtils.ts"; +import { type TextGenerationShape, type ThreadTitleGenerationResult } from "./TextGeneration.ts"; + +const KIRO_TIMEOUT_MS = 180_000; + +function mapKiroAcpError( + operation: + | "generateCommitMessage" + | "generatePrContent" + | "generateBranchName" + | "generateThreadTitle", + detail: string, + cause: unknown, +): TextGenerationError { + return new TextGenerationError({ + operation, + detail, + ...(cause !== undefined ? { cause } : {}), + }); +} + +function isTextGenerationError(error: unknown): error is TextGenerationError { + return ( + typeof error === "object" && + error !== null && + "_tag" in error && + error._tag === "TextGenerationError" + ); +} + +export const makeKiroTextGeneration = Effect.fn("makeKiroTextGeneration")(function* ( + kiroSettings: KiroSettings, + environment: NodeJS.ProcessEnv = process.env, +) { + const commandSpawner = yield* ChildProcessSpawner.ChildProcessSpawner; + + const runKiroJson = ({ + operation, + cwd, + prompt, + outputSchemaJson, + modelSelection, + }: { + operation: + | "generateCommitMessage" + | "generatePrContent" + | "generateBranchName" + | "generateThreadTitle"; + cwd: string; + prompt: string; + outputSchemaJson: S; + modelSelection: ModelSelection; + }): Effect.Effect => + Effect.gen(function* () { + const outputRef = yield* Ref.make(""); + const runtime = yield* makeKiroAcpRuntime({ + kiroSettings, + environment, + childProcessSpawner: commandSpawner, + cwd, + clientInfo: { name: "t3-code-git-text", version: "0.0.0" }, + }); + + yield* runtime.handleSessionUpdate((notification) => { + const update = notification.update; + if (update.sessionUpdate !== "agent_message_chunk") { + return Effect.void; + } + const content = update.content; + if (content.type !== "text") { + return Effect.void; + } + return Ref.update(outputRef, (current) => current + content.text); + }); + + const promptResult = yield* Effect.gen(function* () { + yield* runtime.start(); + yield* runtime + .setModel(modelSelection.model) + .pipe( + Effect.mapError((cause) => + mapKiroAcpError( + operation, + "Failed to set Kiro ACP model for text generation.", + cause, + ), + ), + ); + return yield* runtime.prompt({ + prompt: [{ type: "text", text: prompt }], + }); + }).pipe( + Effect.timeoutOption(KIRO_TIMEOUT_MS), + Effect.flatMap( + Option.match({ + onNone: () => + Effect.fail( + new TextGenerationError({ + operation, + detail: "Kiro CLI request timed out.", + }), + ), + onSome: (value) => Effect.succeed(value), + }), + ), + Effect.mapError((cause) => + isTextGenerationError(cause) + ? cause + : mapKiroAcpError(operation, "Kiro ACP request failed.", cause), + ), + ); + + const rawResult = (yield* Ref.get(outputRef)).trim(); + if (!rawResult) { + return yield* new TextGenerationError({ + operation, + detail: + promptResult.stopReason === "cancelled" + ? "Kiro ACP request was cancelled." + : "Kiro CLI returned empty output.", + }); + } + + const decodeOutput = Schema.decodeEffect(Schema.fromJsonString(outputSchemaJson)); + return yield* decodeOutput(extractJsonObject(rawResult)).pipe( + Effect.catchTag("SchemaError", (cause) => + Effect.fail( + new TextGenerationError({ + operation, + detail: "Kiro CLI returned invalid structured output.", + cause, + }), + ), + ), + ); + }).pipe( + Effect.mapError((cause) => + isTextGenerationError(cause) + ? cause + : mapKiroAcpError(operation, "Kiro ACP text generation failed.", cause), + ), + Effect.scoped, + ); + + const generateCommitMessage: TextGenerationShape["generateCommitMessage"] = Effect.fn( + "KiroTextGeneration.generateCommitMessage", + )(function* (input) { + const { prompt, outputSchema } = buildCommitMessagePrompt({ + branch: input.branch, + stagedSummary: input.stagedSummary, + stagedPatch: input.stagedPatch, + includeBranch: input.includeBranch === true, + }); + + const generated = yield* runKiroJson({ + operation: "generateCommitMessage", + cwd: input.cwd, + prompt, + outputSchemaJson: outputSchema, + modelSelection: input.modelSelection, + }); + + return { + subject: sanitizeCommitSubject(generated.subject), + body: generated.body.trim(), + ...("branch" in generated && typeof generated.branch === "string" + ? { branch: sanitizeFeatureBranchName(generated.branch) } + : {}), + }; + }); + + const generatePrContent: TextGenerationShape["generatePrContent"] = Effect.fn( + "KiroTextGeneration.generatePrContent", + )(function* (input) { + const { prompt, outputSchema } = buildPrContentPrompt({ + baseBranch: input.baseBranch, + headBranch: input.headBranch, + commitSummary: input.commitSummary, + diffSummary: input.diffSummary, + diffPatch: input.diffPatch, + }); + + const generated = yield* runKiroJson({ + operation: "generatePrContent", + cwd: input.cwd, + prompt, + outputSchemaJson: outputSchema, + modelSelection: input.modelSelection, + }); + + return { + title: sanitizePrTitle(generated.title), + body: generated.body.trim(), + }; + }); + + const generateBranchName: TextGenerationShape["generateBranchName"] = Effect.fn( + "KiroTextGeneration.generateBranchName", + )(function* (input) { + const { prompt, outputSchema } = buildBranchNamePrompt({ + message: input.message, + attachments: input.attachments, + }); + + const generated = yield* runKiroJson({ + operation: "generateBranchName", + cwd: input.cwd, + prompt, + outputSchemaJson: outputSchema, + modelSelection: input.modelSelection, + }); + + return { + branch: sanitizeBranchFragment(generated.branch), + }; + }); + + const generateThreadTitle: TextGenerationShape["generateThreadTitle"] = Effect.fn( + "KiroTextGeneration.generateThreadTitle", + )(function* (input) { + const { prompt, outputSchema } = buildThreadTitlePrompt({ + message: input.message, + attachments: input.attachments, + }); + + const generated = yield* runKiroJson({ + operation: "generateThreadTitle", + cwd: input.cwd, + prompt, + outputSchemaJson: outputSchema, + modelSelection: input.modelSelection, + }); + + return { + title: sanitizeThreadTitle(generated.title), + } satisfies ThreadTitleGenerationResult; + }); + + return { + generateCommitMessage, + generatePrContent, + generateBranchName, + generateThreadTitle, + } satisfies TextGenerationShape; +}); diff --git a/apps/web/index.html b/apps/web/index.html index 88e1c8b4f23..83d3fd719aa 100644 --- a/apps/web/index.html +++ b/apps/web/index.html @@ -7,14 +7,14 @@ content="width=device-width, initial-scale=1.0, viewport-fit=cover, interactive-widget=resizes-content" /> - - + +