From aac38d698ff75ac4c8658addab48ef5a83617619 Mon Sep 17 00:00:00 2001 From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Date: Wed, 9 Sep 2026 16:22:17 -0400 Subject: [PATCH 001/261] fix(push): isolate deployment and validate candidates before activation (#19771) * fix(push): isolate deployment and validate candidates before activation * test(push): classify dedicated rollout outside shared SQL lock census * test(push): verify independent deployment identity and lock --- .github/workflows/cloud-push-deploy.yml | 276 +++++++++++++++--- .../scripts/cloud-sql-rollout-lock-census.mjs | 3 +- ...ay-production-identity-boundaries.test.mjs | 12 +- 3 files changed, 250 insertions(+), 41 deletions(-) diff --git a/.github/workflows/cloud-push-deploy.yml b/.github/workflows/cloud-push-deploy.yml index ea19a589bb25..6014681372d4 100644 --- a/.github/workflows/cloud-push-deploy.yml +++ b/.github/workflows/cloud-push-deploy.yml @@ -16,10 +16,9 @@ permissions: contents: read id-token: write -# The gateway applies its own schema at startup against the shared Cloud SQL instance, so a -# deploy is a connection-budget rollout and belongs in the same serialized group as the relay. +# Serialize push traffic changes independently of Relay and the shared database. concurrency: - group: production-cloud-sql-rollout + group: production-push-rollout cancel-in-progress: false defaults: @@ -75,8 +74,8 @@ jobs: - uses: google-github-actions/auth@v2 with: - workload_identity_provider: ${{ vars.PRODUCTION_GCP_RELAY_DEPLOY_WORKLOAD_IDENTITY_PROVIDER }} - service_account: ${{ vars.PRODUCTION_GCP_RELAY_DEPLOY_SERVICE_ACCOUNT }} + workload_identity_provider: ${{ vars.PRODUCTION_GCP_PUSH_DEPLOY_WORKLOAD_IDENTITY_PROVIDER }} + service_account: ${{ vars.PRODUCTION_GCP_PUSH_DEPLOY_SERVICE_ACCOUNT }} - uses: google-github-actions/setup-gcloud@v2 @@ -85,31 +84,41 @@ jobs: - name: Configure Docker auth run: gcloud auth configure-docker "${GCP_REGION}-docker.pkg.dev" --quiet - # Why: the build runs before the lease. Artifact Registry is not the Cloud SQL instance, - # and a multi-minute image build inside the lease blocks every relay deploy and rehome for - # its duration. The lease below covers exactly the connection-budget window: deploy, probe, - # shift. + # Building an image does not need the deployment lease. - name: Build and publish the immutable gateway image shell: bash run: | set -euo pipefail image_tag="${GCP_REGION}-docker.pkg.dev/${GCP_PROJECT_ID}/${REPOSITORY_ID}/${IMAGE_NAME}:sha-${SOURCE_SHA}" - docker build -f "${RUNNER_TEMP}/push-source/cloud/apps/push/Dockerfile" \ + docker buildx build --push --platform linux/amd64 --provenance=false --metadata-file "${RUNNER_TEMP}/push-image.json" \ + -f "${RUNNER_TEMP}/push-source/cloud/apps/push/Dockerfile" \ -t "${image_tag}" "${RUNNER_TEMP}/push-source/cloud" - docker push "${image_tag}" - digest="$(gcloud artifacts docker images describe "${image_tag}" \ - --format='value(image_summary.digest)')" + digest="$(jq -er '."containerimage.digest"' "${RUNNER_TEMP}/push-image.json")" [[ "${digest}" =~ ^sha256:[a-f0-9]{64}$ ]] echo "IMAGE=${GCP_REGION}-docker.pkg.dev/${GCP_PROJECT_ID}/${REPOSITORY_ID}/${IMAGE_NAME}@${digest}" \ >> "${GITHUB_ENV}" echo "IMAGE_DIGEST=${digest}" >> "${GITHUB_ENV}" + # Refuse older images before they ever boot against production. + - name: Require image support for inert validation + shell: bash + run: | + set -euo pipefail + docker run --rm --network none --entrypoint node "${IMAGE}" --input-type=module -e ' + import { loadPushConfig } from "./apps/push/dist/config.js"; + const env = { ORCA_PUSH_PUBLIC_URL: "https://push.onorca.dev", ORCA_PUSH_MODE: "validation" }; + if (loadPushConfig(env).mode !== "validation") throw new Error("validation_mode_unsupported"); + let rejected = false; + try { loadPushConfig({ ...env, ORCA_PUSH_MODE: "invalid" }); } catch { rejected = true; } + if (!rejected) throw new Error("validation_mode_not_fail_closed"); + ' + # Held across the deploy, not just a separate schema step: the gateway opens its pool and # applies its schema while the new revision starts, so the revision is the schema step. - uses: ./.github/actions/cloud-sql-rollout-lease with: bucket: onorca-cloud-terraform-state - object: terraform/state/cloud-sql-rollout/production.lock + object: terraform/state/push-rollout/production.lock # Why: the candidate inherits the serving revision's scaling. A serving revision that has # drifted below the floor would hand the candidate a cold start on every notification, and @@ -124,6 +133,12 @@ jobs: | jq -r '[.status.traffic[] | select((.percent // 0) > 0)] | if length == 1 and .[0].percent == 100 then .[0].revisionName else empty end')" test -n "${serving}" + revisions="$(gcloud run revisions list --service "${SERVICE_NAME}" \ + --project "${GCP_PROJECT_ID}" --region "${GCP_REGION}" --format='value(metadata.name)')" + if test "${revisions}" != "${serving}"; then + echo 'Retire leftover revisions under the rollout lease before deploying; three pools are the limit.' >&2 + exit 1 + fi floor="$(gcloud run revisions describe "${serving}" \ --project "${GCP_PROJECT_ID}" --region "${GCP_REGION}" \ --format="value(metadata.annotations['autoscaling.knative.dev/minScale'])")" @@ -140,16 +155,29 @@ jobs: test "${ceiling}" = "${PUSH_MAX_INSTANCES}" echo "serving revision ${serving} holds ${floor} minimum and ${ceiling} maximum instances" echo "ROLLBACK_REVISION=${serving}" >> "${GITHUB_ENV}" + gcloud run revisions describe "${serving}" \ + --project "${GCP_PROJECT_ID}" --region "${GCP_REGION}" --format=json \ + > "${RUNNER_TEMP}/push-rollback-revision.json" + image="$(jq -er '.status.imageDigest' "${RUNNER_TEMP}/push-rollback-revision.json")" + [[ "${image}" =~ @sha256:[a-f0-9]{64}$ ]] + echo "ROLLBACK_IMAGE=${image}" >> "${GITHUB_ENV}" + jq -e 'all(.spec.containers[0].env[]?; .name != "ORCA_PUSH_MODE" or .value == "active")' \ + "${RUNNER_TEMP}/push-rollback-revision.json" > /dev/null - # No traffic and a per-revision tag: the candidate boots, applies schema, and is probed on - # its own URL while every phone and desktop still reaches the previous revision. + # Validation has no schema writes, HTTP mutations, worker, or pruners; tags alone do not + # isolate background consumers from production. - name: Deploy the candidate revision with no traffic shell: bash run: | set -euo pipefail tag="c${GITHUB_RUN_ID}-${GITHUB_RUN_ATTEMPT}" - echo "CANDIDATE_TAG=${tag}" >> "${GITHUB_ENV}" - echo "CANDIDATE_REVISION=${SERVICE_NAME}-${tag}" >> "${GITHUB_ENV}" + { + echo "VALIDATION_DEPLOY_ATTEMPTED=true" + echo "VALIDATION_REVISION=${SERVICE_NAME}-${tag}" + echo "VALIDATION_TAG=${tag}" + echo "CANDIDATE_TAG=${tag}" + echo "CANDIDATE_REVISION=${SERVICE_NAME}-${tag}" + } >> "${GITHUB_ENV}" gcloud run deploy "${SERVICE_NAME}" \ --project "${GCP_PROJECT_ID}" \ --region "${GCP_REGION}" \ @@ -157,6 +185,7 @@ jobs: --tag "${tag}" \ --revision-suffix "${tag}" \ --no-traffic \ + --update-env-vars ORCA_PUSH_MODE=validation \ --quiet candidate="$(gcloud run services describe "${SERVICE_NAME}" \ --project "${GCP_PROJECT_ID}" --region "${GCP_REGION}" --format=json \ @@ -168,8 +197,7 @@ jobs: # A tagged revision is directly addressable and sits outside the service-wide cap, so the # candidate and the serving revision each draw up to the ceiling during the probe window. - # The lease is taken for exactly that doubling; a candidate that inherited a wider ceiling - # would exceed it, so the inherited scaling is asserted here too. + # Successor creation later requires three revision pools; assert the inherited ceiling. - name: Require the candidate to serve the exact image and inherited scaling shell: bash run: | @@ -195,7 +223,7 @@ jobs: if test "${code}" = 200; then jq -e . < "${RUNNER_TEMP}/push-ready.json" > /dev/null curl --fail --silent --show-error --max-time 10 "${CANDIDATE_URL}/health" \ - | jq -e '.ok == true and .deliveryProtocol == 2' > /dev/null + | jq -e '.ok == true and .deliveryProtocol == 2 and .mode == "validation"' > /dev/null echo "candidate ${CANDIDATE_REVISION} is ready after ${attempt} attempt(s)" exit 0 fi @@ -242,6 +270,53 @@ jobs: fi test "${status}" = INVALID_ARGUMENT + # Cloud Run requires a successor before the latest revision can be deleted. + - name: Retire inert validation and activate the verified image + shell: bash + run: | + set -euo pipefail + tag="a${GITHUB_RUN_ID}-${GITHUB_RUN_ATTEMPT}" + { + echo "CANDIDATE_TAG=${tag}" + echo "CANDIDATE_REVISION=${SERVICE_NAME}-${tag}" + echo "ACTIVATION_ATTEMPTED=true" + } >> "${GITHUB_ENV}" + # This is the production-effect boundary: schema, pruners and workers start here. + gcloud run deploy "${SERVICE_NAME}" \ + --project "${GCP_PROJECT_ID}" --region "${GCP_REGION}" \ + --image "${IMAGE}" --tag "${tag}" --revision-suffix "${tag}" \ + --remove-env-vars ORCA_PUSH_MODE --no-traffic --quiet + gcloud run services update-traffic "${SERVICE_NAME}" \ + --project "${GCP_PROJECT_ID}" --region "${GCP_REGION}" \ + --remove-tags "${VALIDATION_TAG}" --quiet + gcloud run revisions delete "${VALIDATION_REVISION}" \ + --project "${GCP_PROJECT_ID}" --region "${GCP_REGION}" --quiet + echo "VALIDATION_RETIRED=true" >> "${GITHUB_ENV}" + revision="$(gcloud run revisions describe "${SERVICE_NAME}-${tag}" \ + --project "${GCP_PROJECT_ID}" --region "${GCP_REGION}" --format=json)" + jq -e --arg image "${IMAGE}" --arg account "${PUSH_RUNTIME_SERVICE_ACCOUNT}" \ + --arg ceiling "${PUSH_MAX_INSTANCES}" --arg floor "${PUSH_MIN_INSTANCES}" \ + --slurpfile prior "${RUNNER_TEMP}/push-rollback-revision.json" ' + def shape: del(.containers[0].image) | + .containers[0].env = ((.containers[0].env // []) | + map(select(.name != "ORCA_PUSH_MODE")) | sort_by(.name)); + .spec.containers[0].image == $image and .spec.serviceAccountName == $account and + all(.spec.containers[0].env[]?; .name != "ORCA_PUSH_MODE") and + (.spec | shape) == ($prior[0].spec | shape) and + .metadata.annotations["autoscaling.knative.dev/maxScale"] == $ceiling and + (.metadata.annotations["autoscaling.knative.dev/minScale"] | tonumber) >= ($floor | tonumber)' \ + <<< "${revision}" > /dev/null + candidate="$(gcloud run services describe "${SERVICE_NAME}" \ + --project "${GCP_PROJECT_ID}" --region "${GCP_REGION}" --format=json \ + | jq -er --arg tag "${tag}" '[.status.traffic[] | select(.tag == $tag)] + | if length == 1 then .[0] else error("active candidate is not unique") end')" + test "$(jq -r '.revisionName' <<< "${candidate}")" = "${SERVICE_NAME}-${tag}" + url="$(jq -er '.url' <<< "${candidate}")" + [[ "${url}" =~ ^https://[^/]+$ ]] + curl --fail --silent --show-error --max-time 10 "${url}/ready" | jq -e '.ok == true' + curl --fail --silent --show-error --max-time 10 "${url}/health" \ + | jq -e '.ok == true and .deliveryProtocol == 2 and .mode == "active"' + - name: Shift all traffic to the verified candidate shell: bash run: | @@ -275,9 +350,14 @@ jobs: echo "Revision: \`${CANDIDATE_REVISION}\`" echo echo "Image: \`${IMAGE_DIGEST}\`" + echo "Known-good image: \`${ROLLBACK_IMAGE}\`" echo - echo "Rollback: \`gcloud run services update-traffic ${SERVICE_NAME}" \ - "--region ${GCP_REGION} --to-revisions ${ROLLBACK_REVISION}=100\`" + echo "Recovery: deploy \`${ROLLBACK_IMAGE}\` as a new revision with" \ + "\`--remove-env-vars ORCA_PUSH_MODE --no-traffic --tag --revision-suffix \`." + echo 'Verify its exact digest, configuration, readiness and active mode, then promote and check the public origin.' + echo 'Only then remove obsolete tags and delete rejected/previous revisions; never delete the latest revision.' + echo 'The previous revision is retired after public checks; retain this immutable image for recovery under the rollout lease.' + echo "Activation attempted: ${ACTIVATION_ATTEMPTED:-false}; traffic rollback cannot undo schema or deliveries." } >> "${GITHUB_STEP_SUMMARY}" - name: Verify the public origin after the shift @@ -289,7 +369,8 @@ jobs: "${PUSH_ORIGIN}/ready" || true)" if test "${code}" = 200; then curl --fail --silent --show-error --max-time 10 "${PUSH_ORIGIN}/health" \ - | jq -e '.ok == true and .deliveryProtocol == 2' > /dev/null + | jq -e '.ok == true and .deliveryProtocol == 2 and .mode == "active"' > /dev/null + echo "ROLLOUT_VERIFIED=true" >> "${GITHUB_ENV}" echo "${PUSH_ORIGIN} is ready after ${attempt} attempt(s)" exit 0 fi @@ -303,7 +384,7 @@ jobs: # is not a failure to deploy, it is a live gateway that has to go back, so the traffic move # is undone here rather than left to whoever reads the run. - name: Roll traffic back to the previous revision - if: ${{ (failure() || cancelled()) && env.TRAFFIC_SHIFT_ATTEMPTED == 'true' }} + if: ${{ (failure() || cancelled()) && env.TRAFFIC_SHIFT_ATTEMPTED == 'true' && env.ROLLOUT_VERIFIED != 'true' }} shell: bash run: | set -euo pipefail @@ -324,19 +405,107 @@ jobs: echo '### Push gateway rolled back' echo echo "Traffic returned to \`${ROLLBACK_REVISION}\`; the candidate" \ - "\`${CANDIDATE_REVISION}\` no longer serves." + "\`${CANDIDATE_REVISION}\` no longer serves HTTP; deletion below must stop its workers." } >> "${GITHUB_STEP_SUMMARY}" - # Why: a candidate that never took traffic is a revision holding a warm floor and a Cloud - # SQL pool for nothing. Its tag comes off first, because Cloud Run refuses to delete a - # revision a traffic target still names, and clearing CANDIDATE_TAG makes the always() tag - # step below a no-op rather than a second failure. + # Deleting a revision does not restore the service template that Terraform reconciles. + - name: Restore the known-good service template + if: ${{ (failure() || cancelled()) && env.VALIDATION_DEPLOY_ATTEMPTED == 'true' && env.ROLLOUT_VERIFIED != 'true' }} + shell: bash + run: | + set -euo pipefail + test "${TRAFFIC_SHIFT_ATTEMPTED:-false}" != true || test "${TRAFFIC_ROLLED_BACK:-false}" = true + test -n "${ROLLBACK_IMAGE}" + # A partial activation may leave validation plus active; free one slot before recovery. + latest="$(gcloud run services describe "${SERVICE_NAME}" \ + --project "${GCP_PROJECT_ID}" --region "${GCP_REGION}" \ + --format='value(status.latestCreatedRevisionName)')" + test -n "${latest}" + if test "${VALIDATION_RETIRED:-false}" != true && test "${latest}" != "${VALIDATION_REVISION}"; then + existing="$(gcloud run revisions list --service "${SERVICE_NAME}" \ + --project "${GCP_PROJECT_ID}" --region "${GCP_REGION}" \ + --filter "metadata.name=${VALIDATION_REVISION}" --format='value(metadata.name)')" + if test -n "${existing}"; then + test "${existing}" = "${VALIDATION_REVISION}" + gcloud run services update-traffic "${SERVICE_NAME}" \ + --project "${GCP_PROJECT_ID}" --region "${GCP_REGION}" \ + --remove-tags "${VALIDATION_TAG}" --quiet + gcloud run revisions delete "${VALIDATION_REVISION}" \ + --project "${GCP_PROJECT_ID}" --region "${GCP_REGION}" --quiet + fi + fi + tag="r${GITHUB_RUN_ID}-${GITHUB_RUN_ATTEMPT}" + echo "RECOVERY_TAG=${tag}" >> "${GITHUB_ENV}" + echo "TEMPLATE_RECOVERY_REVISION=${SERVICE_NAME}-${tag}" >> "${GITHUB_ENV}" + echo "Template recovery attempted: ${SERVICE_NAME}-${tag}, image ${ROLLBACK_IMAGE}." \ + >> "${GITHUB_STEP_SUMMARY}" + # Known-good schema/workers can run here even though HTTP stays on the old revision. + gcloud run deploy "${SERVICE_NAME}" \ + --project "${GCP_PROJECT_ID}" --region "${GCP_REGION}" \ + --image "${ROLLBACK_IMAGE}" --revision-suffix "${tag}" --tag "${tag}" \ + --remove-env-vars ORCA_PUSH_MODE --no-traffic --quiet + gcloud run services describe "${SERVICE_NAME}" \ + --project "${GCP_PROJECT_ID}" --region "${GCP_REGION}" --format=json \ + > "${RUNNER_TEMP}/push-recovered-service.json" + gcloud run revisions describe "${SERVICE_NAME}-${tag}" \ + --project "${GCP_PROJECT_ID}" --region "${GCP_REGION}" --format=json \ + > "${RUNNER_TEMP}/push-recovery-revision.json" + jq -e --arg image "${ROLLBACK_IMAGE}" --arg serving "${ROLLBACK_REVISION}" \ + --arg floor "${PUSH_MIN_INSTANCES}" --arg ceiling "${PUSH_MAX_INSTANCES}" \ + --slurpfile prior "${RUNNER_TEMP}/push-rollback-revision.json" \ + --slurpfile recovered "${RUNNER_TEMP}/push-recovery-revision.json" ' + def shape: del(.containers[0].image) | + .containers[0].env = ((.containers[0].env // []) | + map(select(.name != "ORCA_PUSH_MODE")) | sort_by(.name)); + .spec.template.spec.containers[0].image == $image and + all(.spec.template.spec.containers[0].env[]?; .name != "ORCA_PUSH_MODE") and + $recovered[0].spec.containers[0].image == $image and + all($recovered[0].spec.containers[0].env[]?; .name != "ORCA_PUSH_MODE") and + ($recovered[0].spec | shape) == ($prior[0].spec | shape) and + .spec.template.metadata.annotations["autoscaling.knative.dev/maxScale"] == $ceiling and + (.spec.template.metadata.annotations["autoscaling.knative.dev/minScale"] | tonumber) >= ($floor | tonumber) and + ([.status.traffic[] | select((.percent // 0) > 0)] | + length == 1 and .[0].revisionName == $serving and .[0].percent == 100) + ' "${RUNNER_TEMP}/push-recovered-service.json" > /dev/null + echo "TEMPLATE_RESTORED=true" >> "${GITHUB_ENV}" + echo 'Known-good image and normal mode restored; previous revision still serves HTTP.' \ + >> "${GITHUB_STEP_SUMMARY}" + + - name: Promote and verify the known-good recovery revision + if: ${{ always() && env.TEMPLATE_RESTORED == 'true' }} + shell: bash + run: | + set -euo pipefail + url="$(jq -er --arg tag "${RECOVERY_TAG}" \ + '.status.traffic[] | select(.tag == $tag) | .url' "${RUNNER_TEMP}/push-recovered-service.json")" + [[ "${url}" =~ ^https://[^/]+$ ]] + curl --fail --silent --show-error --max-time 10 "${url}/ready" | jq -e '.ok == true' + curl --fail --silent --show-error --max-time 10 "${url}/health" \ + | jq -e '.ok == true and .deliveryProtocol == 2 and .mode == "active"' + gcloud run services update-traffic "${SERVICE_NAME}" \ + --project "${GCP_PROJECT_ID}" --region "${GCP_REGION}" \ + --to-revisions "${TEMPLATE_RECOVERY_REVISION}=100" --quiet + serving="$(gcloud run services describe "${SERVICE_NAME}" \ + --project "${GCP_PROJECT_ID}" --region "${GCP_REGION}" --format=json \ + | jq -er '[.status.traffic[] | select((.percent // 0) > 0)] | + if length == 1 and .[0].percent == 100 then .[0].revisionName else empty end')" + test "${serving}" = "${TEMPLATE_RECOVERY_REVISION}" + curl --fail --silent --show-error --max-time 10 "${PUSH_ORIGIN}/ready" | jq -e '.ok == true' + curl --fail --silent --show-error --max-time 10 "${PUSH_ORIGIN}/health" \ + | jq -e '.ok == true and .deliveryProtocol == 2 and .mode == "active"' + echo "RECOVERY_VERIFIED=true" >> "${GITHUB_ENV}" + echo "Recovery revision ${TEMPLATE_RECOVERY_REVISION} now serves the known-good image." \ + >> "${GITHUB_STEP_SUMMARY}" + - name: Delete the rejected candidate revision - if: ${{ (failure() || cancelled()) && (env.TRAFFIC_SHIFT_ATTEMPTED != 'true' || env.TRAFFIC_ROLLED_BACK == 'true') }} + if: ${{ always() && env.RECOVERY_VERIFIED == 'true' }} shell: bash run: | set -euo pipefail - test -n "${CANDIDATE_REVISION:-}" || exit 0 + if test -z "${CANDIDATE_REVISION:-}"; then + echo "CANDIDATE_DELETED=true" >> "${GITHUB_ENV}" + exit 0 + fi if test -n "${CANDIDATE_TAG:-}"; then gcloud run services update-traffic "${SERVICE_NAME}" \ --project "${GCP_PROJECT_ID}" \ @@ -345,11 +514,44 @@ jobs: --quiet echo "CANDIDATE_TAG=" >> "${GITHUB_ENV}" fi - gcloud run revisions delete "${CANDIDATE_REVISION}" \ - --project "${GCP_PROJECT_ID}" \ - --region "${GCP_REGION}" \ - --quiet - echo "deleted the candidate revision ${CANDIDATE_REVISION}" + existing="$(gcloud run revisions list --service "${SERVICE_NAME}" \ + --project "${GCP_PROJECT_ID}" --region "${GCP_REGION}" \ + --filter "metadata.name=${CANDIDATE_REVISION}" --format='value(metadata.name)')" + if test -n "${existing}"; then + test "${existing}" = "${CANDIDATE_REVISION}" + gcloud run revisions delete "${CANDIDATE_REVISION}" \ + --project "${GCP_PROJECT_ID}" --region "${GCP_REGION}" --quiet + fi + echo "CANDIDATE_DELETED=true" >> "${GITHUB_ENV}" + echo "candidate revision ${CANDIDATE_REVISION} is absent" + + # Public checks commit the serving revision; cleanup failures must not roll it back. + - name: Retire previous consumers after public checks + if: ${{ always() && (env.ROLLOUT_VERIFIED == 'true' || (env.RECOVERY_VERIFIED == 'true' && env.CANDIDATE_DELETED == 'true')) }} + shell: bash + run: | + set -euo pipefail + serving="${CANDIDATE_REVISION}" + if test "${RECOVERY_VERIFIED:-false}" = true; then + serving="${TEMPLATE_RECOVERY_REVISION}" + fi + gcloud run services update-traffic "${SERVICE_NAME}" \ + --project "${GCP_PROJECT_ID}" --region "${GCP_REGION}" --clear-tags --quiet + revisions="$(gcloud run revisions list --service "${SERVICE_NAME}" \ + --project "${GCP_PROJECT_ID}" --region "${GCP_REGION}" --format='value(metadata.name)')" + while IFS= read -r revision; do + test -n "${revision}" || continue + test "${revision}" != "${serving}" || continue + case "${revision}" in + "${ROLLBACK_REVISION}"|"${VALIDATION_REVISION}"|"${CANDIDATE_REVISION}") ;; + *) echo "Unexpected revision ${revision}; manual retirement required." >&2; exit 1 ;; + esac + gcloud run revisions delete "${revision}" \ + --project "${GCP_PROJECT_ID}" --region "${GCP_REGION}" --quiet + done <<< "${revisions}" + echo "CANDIDATE_TAG=" >> "${GITHUB_ENV}" + echo 'Obsolete revision resources retired; verify actual SQL session drain operationally.' \ + >> "${GITHUB_STEP_SUMMARY}" - name: Drop the candidate traffic tag if: always() diff --git a/cloud/dev/scripts/cloud-sql-rollout-lock-census.mjs b/cloud/dev/scripts/cloud-sql-rollout-lock-census.mjs index 2f7157d823c8..bbfe72ec2b4a 100644 --- a/cloud/dev/scripts/cloud-sql-rollout-lock-census.mjs +++ b/cloud/dev/scripts/cloud-sql-rollout-lock-census.mjs @@ -283,8 +283,6 @@ export const LEASED_WORKFLOWS = named([ 'operate-relay-production-rehome.yml', production({ leaseFiles: ['operate-relay-production-rehome-job.yml'] }) ], - // The gateway applies its schema at startup, so its deploy revision is the schema step. - ['push-deploy.yml', production()], ['deploy-relay-asia-topology.yml', eitherEnvironment()], ['operate-relay-asia-admission.yml', eitherEnvironment()], ['deploy-relay-staging.yml', staging()], @@ -300,6 +298,7 @@ export const LEASED_WORKFLOWS = named([ ]) export const NOT_A_CLOUD_SQL_CANDIDATE = named([ + ['push-deploy.yml', 'Push uses dedicated SQL and its own production-push-rollout group and durable push-rollout lease.'], [ 'monitor-relay-production.yml', 'Read-only. Its identity holds monitoring, logging, Cloud SQL and compute viewer roles only, and it runs `gcloud sql instances describe`, never a mutation. It consumes no connection budget, so the durable lease would only let monitoring block a rollout and a rollout block monitoring.' diff --git a/cloud/dev/scripts/relay-production-identity-boundaries.test.mjs b/cloud/dev/scripts/relay-production-identity-boundaries.test.mjs index f97e742215bf..3845cb900619 100644 --- a/cloud/dev/scripts/relay-production-identity-boundaries.test.mjs +++ b/cloud/dev/scripts/relay-production-identity-boundaries.test.mjs @@ -32,8 +32,7 @@ test('no workflow names the retired generic production deploy identity', async ( 'deploy-relay-production.yml', 'operate-relay-asia-admission.yml', 'operate-relay-production-rehome-job.yml', - 'publish-relay-production.yml', - 'push-deploy.yml' + 'publish-relay-production.yml' ].map((name) => relayWorkflowFile(name)).sort()) }) @@ -168,3 +167,12 @@ test('fence broker pins the production-proven Terraform planner', async () => { const dockerfile = await source('apps/relay-fence-broker/Dockerfile') assert.match(dockerfile, /FROM hashicorp\/terraform:1\.15\.8 AS terraform/) }) + + test('push uses its dedicated identity and rollout lease', () => { + const workflow = readRelayWorkflow('push-deploy.yml') + assert.match(workflow, /PRODUCTION_GCP_PUSH_DEPLOY_WORKLOAD_IDENTITY_PROVIDER/) + assert.match(workflow, /PRODUCTION_GCP_PUSH_DEPLOY_SERVICE_ACCOUNT/) + assert.doesNotMatch(workflow, /PRODUCTION_GCP_RELAY_DEPLOY_/) + assert.match(workflow, /group: production-push-rollout/) + assert.match(workflow, /object: terraform\/state\/push-rollout\/production.lock/) +}) From 2f828e44621071c1d2d12a28631bf5a4d97a9b7b Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 9 Sep 2026 21:38:35 -0700 Subject: [PATCH 002/261] fix(native-chat): show Claude working from the send, not the provider echo (#19822) * fix(native-chat): show Claude working from the send, not the provider echo A structured session read as working only once a turnLifecycle row existed. Codex writes that row ~150ms after the send; Claude cannot write it until the SDK echoes the user message back, measured at a 3.4s median and 18s at p90, so the chat and every session list read idle for the whole wait. The journalled submission is the host's own evidence a turn is owed, so the shared projection reads it too. `unknown` still counts -- the ack budget elapsing answers delivery, not whether work is owed -- while a recovered `unknown` does not, which needed the existing row flag carried onto the projected submission. Claude's activity line now stays the generic fallback. Its only turn-wide frame carries a bare token, and its task_* prose describes a spawned task rather than this turn; compaction is kept because it explains an otherwise silent wait. * Fix structured chat pending-work lifecycle and mobile cancellation --------- Co-authored-by: Merge Sim --- .../src/session/MobileNativeChatOverlay.tsx | 1 + .../src/session/MobileNativeChatView.test.ts | 13 +++ mobile/src/session/MobileNativeChatView.tsx | 8 +- .../mobile-native-chat-controller-contract.ts | 1 + .../use-mobile-native-chat-controller.test.ts | 35 +++++++- .../use-mobile-native-chat-controller.ts | 3 + .../use-mobile-structured-agent-session.ts | 10 ++- .../journal-crash-boundary.test.ts | 27 ++++++ .../journal-pending-submission-recovery.ts | 14 ++- .../agent-session-journal/journal-reducer.ts | 6 ++ .../agent-session-journal/journal-store.ts | 7 +- .../provider-frame-activity.test.ts | 42 ++++----- .../provider-frame-activity.ts | 49 ++++------- .../provider-turn-activity-routing.test.ts | 5 +- .../structured-agent-session-host-handoff.ts | 7 ++ ...red-agent-session-send-idempotency.test.ts | 34 +++++++ ...ructured-agent-session-status-feed.test.ts | 57 +++++++++++- .../structured-agent-session-status-feed.ts | 7 +- ...red-agent-session-surface-lifetime.test.ts | 6 ++ .../structured-agent-session-turns.ts | 12 ++- ...ured-agent-session-unexpected-exit.test.ts | 11 ++- ...tructured-agent-session-unexpected-exit.ts | 13 +++ .../NativeChatStructuredSession.tsx | 4 +- .../use-structured-agent-session.test.tsx | 66 +++++++++++++- .../use-structured-agent-session.ts | 11 ++- src/shared/agent-session-journal-schemas.ts | 3 +- src/shared/agent-session-journal-types.ts | 4 + ...tructured-agent-session-projection.test.ts | 88 ++++++++++++++++++- .../structured-agent-session-projection.ts | 51 +++++++++-- 29 files changed, 504 insertions(+), 91 deletions(-) diff --git a/mobile/src/session/MobileNativeChatOverlay.tsx b/mobile/src/session/MobileNativeChatOverlay.tsx index 357a089466e2..dcb353e54f89 100644 --- a/mobile/src/session/MobileNativeChatOverlay.tsx +++ b/mobile/src/session/MobileNativeChatOverlay.tsx @@ -71,6 +71,7 @@ export function MobileNativeChatOverlay({ error={session.error} agent={controller.nativeChatAgent} agentWorking={controller.nativeChatAgentWorking} + canStop={controller.nativeChatCanStop} structuredActivityUi={controller.nativeChatStructured} streaming={streaming} onStop={controller.handleNativeChatStop} diff --git a/mobile/src/session/MobileNativeChatView.test.ts b/mobile/src/session/MobileNativeChatView.test.ts index d101c3f0ef65..63bfb7154454 100644 --- a/mobile/src/session/MobileNativeChatView.test.ts +++ b/mobile/src/session/MobileNativeChatView.test.ts @@ -74,6 +74,7 @@ type Overrides = { pending?: Parameters[0]['pending'] structuredActivityUi?: boolean agentWorking?: boolean + canStop?: boolean sendSurfaceId?: string } @@ -118,6 +119,18 @@ describe('MobileNativeChatView', () => { } /** Ids of the rows the list is currently rendering. */ + it('keeps Stop hidden during a structured dispatch until a provider turn can be cancelled', async () => { + const props = { structuredActivityUi: true, agentWorking: true, canStop: false } + await render(props) + const stops = () => + renderer!.root.findAll((node) => node.props.accessibilityLabel === 'Stop the agent') + expect(stops()).toHaveLength(0) + await update({ ...props, canStop: true }) + expect(stops()).toHaveLength(1) + await update({ agentWorking: true }) + expect(stops()).toHaveLength(1) + }) + function listIds(): string[] { const list = renderer!.root.find((node) => node.type === 'FlatList') return (list.props.data as { id: string }[]).map((row) => row.id) diff --git a/mobile/src/session/MobileNativeChatView.tsx b/mobile/src/session/MobileNativeChatView.tsx index 70a67787de00..f58e2ad05b0f 100644 --- a/mobile/src/session/MobileNativeChatView.tsx +++ b/mobile/src/session/MobileNativeChatView.tsx @@ -49,10 +49,11 @@ type Props = { /** Resolved agent for this chat; names the empty-state copy (desktop parity). */ agent?: string | null agentWorking?: boolean + canStop?: boolean /** Structured lane: per-turn "Working for N" status plus live tool progress, * replacing the bridge lane's static three-dot working row (desktop parity). */ structuredActivityUi?: boolean - /** Interrupt the agent mid-turn (shown as a Stop button on the working bar). */ + /** Interrupt a provider turn. */ onStop?: () => void /** Live partial assistant text to show as an in-progress bubble, already gated * by the overlay against the transcript catching up. */ @@ -129,6 +130,7 @@ export function MobileNativeChatView({ error, agent, agentWorking, + canStop = agentWorking, structuredActivityUi = false, onStop, streaming, @@ -396,8 +398,6 @@ export function MobileNativeChatView({ question={question} onAnswerQuestion={onAnswerQuestion} /> - {/* Chrome row above the composer: the working indicator and the global - tool-calls expand/collapse toggle on the left, Stop in the far corner. */} {agentWorking && !structuredActivityUi ? : null} @@ -414,7 +414,7 @@ export function MobileNativeChatView({ {toolsExpanded ? 'Collapse' : 'Tools'} - {agentWorking ? ( + {canStop ? ( [styles.stopButton, pressed && styles.pressed]} onPress={onStop} diff --git a/mobile/src/session/mobile-native-chat-controller-contract.ts b/mobile/src/session/mobile-native-chat-controller-contract.ts index 53187e0d6dbc..0c6ffd6a764c 100644 --- a/mobile/src/session/mobile-native-chat-controller-contract.ts +++ b/mobile/src/session/mobile-native-chat-controller-contract.ts @@ -28,6 +28,7 @@ export type MobileNativeChatController = { /** Structured lane: drives the per-turn status row and live tool progress. */ nativeChatStructured: boolean nativeChatAgentWorking: boolean + nativeChatCanStop: boolean nativeChatStreamingText?: string /** Agent mid-turn, regardless of whether chat is the visible view. */ nativeChatStreamLive: boolean diff --git a/mobile/src/session/use-mobile-native-chat-controller.test.ts b/mobile/src/session/use-mobile-native-chat-controller.test.ts index 83f8075d9141..c90033e24043 100644 --- a/mobile/src/session/use-mobile-native-chat-controller.test.ts +++ b/mobile/src/session/use-mobile-native-chat-controller.test.ts @@ -55,6 +55,7 @@ const structuredQuestion = { allowOther: true, optionTokens: ['choice-a', 'choice-b'] } +const structuredActivity = { isWorking: false, turnId: null as string | null } const structuredSessionState = { messages: [] as unknown[], status: 'ready', @@ -86,8 +87,7 @@ vi.mock('./use-mobile-native-chat-session', () => ({ vi.mock('./use-mobile-structured-agent-session', () => ({ useMobileStructuredAgentSession: () => ({ session: structuredSessionState, - isWorking: false, - turnId: null, + ...structuredActivity, sendWithOutcome: structuredSendWithOutcome, cancel: structuredCancel, permission: structuredPermission, @@ -341,6 +341,37 @@ describe('useMobileNativeChatController handleNativeChatSend', () => { expect(clientStub.sendRequest).not.toHaveBeenCalled() }) + it('separates structured working status from provider cancellation availability', async () => { + const props = { + tab: { + type: 'agent-session', + id: 'agent-tab-1', + title: 'Chat', + sessionId: 'session-structured', + agent: 'codex', + isActive: true + }, + activeHandle: null, + inputLeaseReady: false + } + structuredActivity.isWorking = true + try { + await act(async () => { + renderer?.update(createElement(Harness, props)) + }) + expect(controller?.nativeChatAgentWorking).toBe(true) + expect(controller?.nativeChatCanStop).toBe(false) + structuredActivity.turnId = 'provider-turn' + await act(async () => { + renderer?.update(createElement(Harness, props)) + }) + expect(controller?.nativeChatCanStop).toBe(true) + } finally { + structuredActivity.isWorking = false + structuredActivity.turnId = null + } + }) + it('exposes structured prompt cards and session options on structured tabs', async () => { await act(async () => { renderer?.update( diff --git a/mobile/src/session/use-mobile-native-chat-controller.ts b/mobile/src/session/use-mobile-native-chat-controller.ts index 729cec302c13..5195edf21245 100644 --- a/mobile/src/session/use-mobile-native-chat-controller.ts +++ b/mobile/src/session/use-mobile-native-chat-controller.ts @@ -299,6 +299,9 @@ export function useMobileNativeChatController(args: { /** Structured lane: drives the per-turn status row and live tool progress. */ nativeChatStructured: activeChatStructured, nativeChatAgentWorking, + nativeChatCanStop: activeChatStructured + ? structuredNativeChat.turnId !== null + : nativeChatAgentWorking, nativeChatStreamingText, nativeChatStreamLive, nativeChatStreamScopeKey: streamScopeKey, diff --git a/mobile/src/session/use-mobile-structured-agent-session.ts b/mobile/src/session/use-mobile-structured-agent-session.ts index 271f9143671d..86d3f2c8e44f 100644 --- a/mobile/src/session/use-mobile-structured-agent-session.ts +++ b/mobile/src/session/use-mobile-structured-agent-session.ts @@ -11,7 +11,10 @@ import { import { encodeNativeChatTranscriptIdentity } from '../../../src/shared/native-chat-transcript-retention' import type { MobileNativeChatSendOutcome } from './mobile-native-chat-send' import { projectStructuredAgentSessionMessages } from '../../../src/shared/structured-agent-session-message-projection' -import { activeStructuredAgentSessionTurnId } from '../../../src/shared/structured-agent-session-projection' +import { + activeStructuredAgentSessionTurnId, + hasUnansweredStructuredAgentSessionDispatch +} from '../../../src/shared/structured-agent-session-projection' import { pendingStructuredApproval, pendingStructuredQuestion, @@ -291,7 +294,10 @@ export function useMobileStructuredAgentSession(args: { loadingEarlier: loadingOlder, loadEarlier }, - isWorking: activeStructuredAgentSessionTurnId(state.items) !== null, + // A dispatch the provider has not answered yet is already work — see the desktop hook. + isWorking: + activeStructuredAgentSessionTurnId(state.items) !== null || + hasUnansweredStructuredAgentSessionDispatch(state.submissions, state.fence), turnId: activeStructuredAgentSessionTurnId(state.items), sendWithOutcome, cancel, diff --git a/src/main/native-chat/agent-session-journal/journal-crash-boundary.test.ts b/src/main/native-chat/agent-session-journal/journal-crash-boundary.test.ts index a7f54a14a4c3..85cda398c1d6 100644 --- a/src/main/native-chat/agent-session-journal/journal-crash-boundary.test.ts +++ b/src/main/native-chat/agent-session-journal/journal-crash-boundary.test.ts @@ -15,6 +15,7 @@ import type { AgentJournalMessageItem, AgentSessionJournalIdentity } from '../../../shared/agent-session-journal-types' +import { hasUnansweredStructuredAgentSessionDispatch } from '../../../shared/structured-agent-session-projection' import { digestPayload } from './journal-payload-bounds' import { reconcileSubmissions, @@ -110,6 +111,8 @@ describe('crash between provider accept and journal commit', () => { expect(restarted.pendingSubmissions().map((entry) => entry.clientMessageId)).toEqual(['cm_1']) await restarted.markPendingSubmissionsUnknown(2) expect(restarted.submissions()[0]?.dispatchState).toBe('unknown') + // Marks the send as outlived by its writer, so no reader reports it as still working. + expect(restarted.submissions()[0]?.recovered).toBe(true) const [outcome] = reconcileSubmissions({ submissions: restarted.submissions(), @@ -139,6 +142,30 @@ describe('crash between provider accept and journal commit', () => { expect(restarted.receiptFor('cm_1')?.providerItemId).toBe(agentJournalItemKey(outcome.identity)) }) + it('retires an ack timeout on restart without changing its delivery verdict', async () => { + const journal = await open() + await journal.appendSubmission({ + clientMessageId: 'cm_timeout', + payloadFingerprint: digestPayload('slow'), + body: userMessage('slow'), + fence: 1 + }) + await journal.resolveDispatch({ + clientMessageId: 'cm_timeout', + state: 'unknown', + reason: 'ack timeout', + fence: 1 + }) + expect(hasUnansweredStructuredAgentSessionDispatch(journal.submissions())).toBe(true) + const restarted = await open() + await restarted.markPendingSubmissionsUnknown(2) + expect(restarted.submissions()[0]?.dispatchState).toBe('unknown') + expect(hasUnansweredStructuredAgentSessionDispatch(restarted.submissions())).toBe(false) + const cursor = restarted.cursor() + await restarted.markPendingSubmissionsUnknown(2) + expect(restarted.cursor()).toEqual(cursor) + }) + it('reports a rejected submission as never delivered, and never re-sends it', async () => { const journal = await open() await journal.appendSubmission({ diff --git a/src/main/native-chat/agent-session-journal/journal-pending-submission-recovery.ts b/src/main/native-chat/agent-session-journal/journal-pending-submission-recovery.ts index 76bc00394f2a..e09dd109d508 100644 --- a/src/main/native-chat/agent-session-journal/journal-pending-submission-recovery.ts +++ b/src/main/native-chat/agent-session-journal/journal-pending-submission-recovery.ts @@ -2,14 +2,22 @@ import type { AgentSessionJournal } from './journal-store' export async function markJournalPendingSubmissionsUnknown( journal: AgentSessionJournal, - fence: number + fence: number, + reason = 'host_restarted_before_acknowledgement' ): Promise { - const pending = journal.pendingSubmissions().map((entry) => entry.clientMessageId) + const pending = journal + .submissions() + .filter( + (entry) => + entry.dispatchState === 'pending' || + (entry.dispatchState === 'unknown' && entry.recovered !== true) + ) + .map((entry) => entry.clientMessageId) for (const clientMessageId of pending) { await journal.resolveDispatch({ clientMessageId, state: 'unknown', - reason: 'host_restarted_before_acknowledgement', + reason, fence, recovered: true }) diff --git a/src/main/native-chat/agent-session-journal/journal-reducer.ts b/src/main/native-chat/agent-session-journal/journal-reducer.ts index 41625792aa00..e01e7d6158f7 100644 --- a/src/main/native-chat/agent-session-journal/journal-reducer.ts +++ b/src/main/native-chat/agent-session-journal/journal-reducer.ts @@ -256,10 +256,16 @@ function applyDispatch( if (submission.dispatchState === 'rejected' || submission.dispatchState === 'accepted') { return } + submission.fence = row.fence submission.dispatchState = row.state submission.providerItemId = row.providerItemId submission.reason = row.reason submission.resolvedAt = row.ts + if (row.recovered) { + submission.recovered = row.recovered + } else { + delete submission.recovered + } if (row.state !== 'accepted' || !row.providerItemId) { return } diff --git a/src/main/native-chat/agent-session-journal/journal-store.ts b/src/main/native-chat/agent-session-journal/journal-store.ts index 3c16800099bc..812e2bca8344 100644 --- a/src/main/native-chat/agent-session-journal/journal-store.ts +++ b/src/main/native-chat/agent-session-journal/journal-store.ts @@ -245,10 +245,9 @@ export class AgentSessionJournal { })) } - /** On restart every `pending` submission becomes `unknown` before the session - * accepts a writer. Orca never re-sends on the user's behalf. */ - async markPendingSubmissionsUnknown(fence: number): Promise { - return markJournalPendingSubmissionsUnknown(this, fence) + /** Retire unanswered sends after their execution owner ended, without assuming delivery. */ + async markPendingSubmissionsUnknown(fence: number, reason?: string): Promise { + return markJournalPendingSubmissionsUnknown(this, fence, reason) } /** The escape hatch for corruption, an unreconcilable prefix, a forked handle, diff --git a/src/main/native-chat/agent-session-wire/provider-frame-activity.test.ts b/src/main/native-chat/agent-session-wire/provider-frame-activity.test.ts index 405048732827..ba2eba580626 100644 --- a/src/main/native-chat/agent-session-wire/provider-frame-activity.test.ts +++ b/src/main/native-chat/agent-session-wire/provider-frame-activity.test.ts @@ -38,36 +38,24 @@ describe('provider frame activity', () => { } }) - it('uses Claude descriptions and safe semantic status without exposing tool labels', () => { - expect( - claudeProviderFrameActivity('message:system:task_started', { - description: 'Trace the activity channel' - }) - ).toBe('Working on: Trace the activity channel') - expect( - claudeProviderFrameActivity('message:system:task_progress', { - description: 'Reading tests', - summary: 'Checking remote compatibility' - }) - ).toBe('Checking remote compatibility') - expect( - claudeProviderFrameActivity('message:system:task_updated', { - patch: { description: 'Validating the renderer' } - }) - ).toBe('Validating the renderer') + it('leaves the Claude line on the generic fallback, since Claude never narrates its turn', () => { + // Prose on these frames belongs to a spawned task, not to this turn. + for (const [kind, payload] of [ + ['message:system:task_started', { description: 'Trace the activity channel' }], + ['message:system:task_progress', { summary: 'Checking remote compatibility' }], + ['message:system:task_updated', { patch: { description: 'Validating the renderer' } }], + ['message:system:control_request_progress', { status: 'api_retry' }], + ['message:tool_progress', { tool_name: 'ReadSecretFile' }] + ] as const) { + expect(claudeProviderFrameActivity(kind, payload)).toBeNull() + } + // `requesting` holds for nearly the whole turn and says no more than the fallback. + expect(claudeProviderFrameActivity('message:system:status', { status: 'requesting' })).toBeNull() expect(claudeProviderFrameActivity('message:system:status', { status: 'compacting' })).toBe( 'Compacting the conversation' ) - expect( - claudeProviderFrameActivity('message:system:control_request_progress', { - status: 'api_retry' - }) - ).toBe('Retrying a side question') - expect( - claudeProviderFrameActivity('message:tool_progress', { - tool_name: 'ReadSecretFile' - }) - ).toBeNull() + // An unmodeled frame still declines to answer, so it cannot clear live copy. + expect(claudeProviderFrameActivity('message:system:unknown_frame', {})).toBeUndefined() }) it('falls through on protocol noise and bounds long copy', () => { diff --git a/src/main/native-chat/agent-session-wire/provider-frame-activity.ts b/src/main/native-chat/agent-session-wire/provider-frame-activity.ts index 336170a4cfe5..3cd6c4abaf0e 100644 --- a/src/main/native-chat/agent-session-wire/provider-frame-activity.ts +++ b/src/main/native-chat/agent-session-wire/provider-frame-activity.ts @@ -100,40 +100,29 @@ export function codexProviderFrameActivity( return itemType ? (CODEX_ITEM_ACTIVITY[itemType] ?? null) : null } +/** + * Claude does not narrate its own turn, so the activity line stays the generic fallback. + * + * Codex names each item it starts, which is what makes its line worth reading. Claude's only + * turn-wide frame is `system/status`, whose payload is a bare token — every sentence Orca ever + * put on this line for it was Orca's own wording for `requesting`, which is true for nearly the + * whole turn and says no more than the fallback does. Its `task_*` frames do carry prose, but + * they are keyed by task id and subagent type: they describe a spawned task, not this turn, and + * the background-tasks strip already owns that. Compaction is the one exception kept — a real, + * rare state that explains an otherwise unexplained wait, and the Codex map reports it too. + */ export function claudeProviderFrameActivity(kind: string, payload: unknown): ActivityText { const source = record(payload) - if (kind === 'message:system:task_started') { - if (source?.ambient === true || source?.skip_transcript === true) { - return null - } - const description = providerActivityText(stringField(source, 'description')) - return description ? providerActivityText(`Working on: ${description}`) : null - } - if (kind === 'message:system:task_progress') { - return providerActivityText( - stringField(source, 'summary') ?? stringField(source, 'description') - ) - } - if (kind === 'message:system:task_updated') { - return providerActivityText(stringField(record(source?.patch), 'description')) - } if (kind === 'message:system:status') { - const status = stringField(source, 'status') - return status === 'compacting' - ? 'Compacting the conversation' - : status === 'requesting' - ? 'Requesting a response' - : null - } - if (kind === 'message:system:control_request_progress') { - const status = stringField(source, 'status') - return status === 'started' - ? 'Exploring a side question' - : status === 'api_retry' - ? 'Retrying a side question' - : null + return stringField(source, 'status') === 'compacting' ? 'Compacting the conversation' : null } - if (kind === 'message:tool_progress') { + if ( + kind === 'message:system:task_started' || + kind === 'message:system:task_progress' || + kind === 'message:system:task_updated' || + kind === 'message:system:control_request_progress' || + kind === 'message:tool_progress' + ) { return null } return undefined diff --git a/src/main/native-chat/agent-session-wire/provider-turn-activity-routing.test.ts b/src/main/native-chat/agent-session-wire/provider-turn-activity-routing.test.ts index a66ac567a4d7..eea8398f7eb9 100644 --- a/src/main/native-chat/agent-session-wire/provider-turn-activity-routing.test.ts +++ b/src/main/native-chat/agent-session-wire/provider-turn-activity-routing.test.ts @@ -248,10 +248,11 @@ describe('provider turn activity routing', () => { }) ) expect(state.rows).toHaveLength(turnRows) + // Only compaction reaches the line; task and side-question prose is not this turn's work. expect(state.activities.slice(-3)).toEqual([ - { turnId: TURN_ID, text: 'Checking the renderer state' }, + null, { turnId: TURN_ID, text: 'Compacting the conversation' }, - { turnId: TURN_ID, text: 'Exploring a side question' } + null ]) translator.handle(claudeMessage({ type: 'tool_progress', tool_name: 'SecretReader' })) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-host-handoff.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-host-handoff.ts index 3495409f127f..113940ff0f49 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-host-handoff.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-host-handoff.ts @@ -86,6 +86,13 @@ export function createStructuredAgentSessionHostHandoff( host.publishStatus?.(sessionId) try { await host.flush(sessionId) + const session = host.session(sessionId) + await session.journal.markPendingSubmissionsUnknown( + session.fence, + 'provider_exited_before_acknowledgement' + ) + host.subscribers.publish(sessionId, session.journal) + host.publishStatus?.(sessionId) host.eventSink(sessionId).unbind() return { state: 'stopped' } } catch (error) { diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-send-idempotency.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-send-idempotency.test.ts index 581743633c0b..be9386815ddd 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-send-idempotency.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-send-idempotency.test.ts @@ -3,6 +3,7 @@ import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { AgentJournalMessageItem } from '../../../shared/agent-session-journal-types' +import { hasUnansweredStructuredAgentSessionDispatch } from '../../../shared/structured-agent-session-projection' import { structuredAgentSessionPayloadFingerprint } from '../../../shared/structured-agent-session-mutation' import { createTrackedJournalOpener } from '../agent-session-journal/journal-store-test-open' import type { AgentSessionJournal } from '../agent-session-journal/journal-store' @@ -34,6 +35,39 @@ afterEach(async () => { }) describe('structured send idempotency', () => { + it('publishes a recovered retry as working before waiting for its provider', async () => { + const body: AgentJournalMessageItem = { + kind: 'message', + role: 'user', + blocks: [{ type: 'text', text: 'retry' }] + } + const input = { clientMessageId: 'retry-id', payloadFingerprint: 'fingerprint', body } + await journal.appendSubmission({ ...input, fence: 1 }) + await journal.markPendingSubmissionsUnknown(2) + const originalItem = journal.snapshot().items[0] + const publish = vi.fn() + const dispatch = vi.fn(async () => { + expect(publish).toHaveBeenCalledOnce() + expect(hasUnansweredStructuredAgentSessionDispatch(journal.submissions(), 2)).toBe(true) + return { state: 'unknown' as const, reason: 'ack timeout' } + }) + await performSend( + { + sessionId: 'session-1', + journal, + fence: 2, + adapter: { dispatch } as unknown as StructuredAgentSessionAdapter, + persistOptions: async () => undefined, + resolvedBy: 'caller', + publish, + now: () => 1 + }, + { ...input, retryUnknown: true } + ) + expect(hasUnansweredStructuredAgentSessionDispatch(journal.submissions(), 2)).toBe(true) + expect(journal.snapshot().items).toEqual([originalItem]) + }) + it('does not redispatch one send id reused across caller ledgers', async () => { const body: AgentJournalMessageItem = { kind: 'message', diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-status-feed.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-status-feed.test.ts index 1e3b9bb25a34..7bd537424301 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-status-feed.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-status-feed.test.ts @@ -56,9 +56,11 @@ async function openJournal(sessionId = SESSION, now?: () => number) { function indexed(session: { journal: Awaited> hasProviderChild?: boolean + fence?: number }) { return { journal: session.journal, + fence: session.fence ?? 1, ...(session.hasProviderChild !== undefined ? { hasProviderChild: session.hasProviderChild } : {}), @@ -69,7 +71,7 @@ function indexed(session: { function feedFor( sessions: Map< string, - { journal: Awaited>; hasProviderChild?: boolean } + { journal: Awaited>; hasProviderChild?: boolean; fence?: number } >, record: Partial | null = null, onStatusChanged?: StructuredAgentSessionStatusFeedDeps['onStatusChanged'] @@ -153,6 +155,59 @@ describe('StructuredAgentSessionStatusFeed', () => { ]) }) + it('stops projecting an old-host unknown submission after the owner fence advances', async () => { + const journal = await openJournal() + const session = { journal, fence: 1 } + const { feed, events } = feedFor(new Map([[SESSION, session]])) + await journal.appendSubmission({ + clientMessageId: 'old-host', + payloadFingerprint: 'fp', + body: { kind: 'message', role: 'user', blocks: [{ type: 'text', text: 'slow' }] }, + fence: 1 + }) + await journal.resolveDispatch({ + clientMessageId: 'old-host', + state: 'unknown', + reason: 'ack timeout', + fence: 1 + }) + feed.publish(SESSION) + expect(events.at(-1)).toMatchObject({ session: { status: 'working' } }) + session.fence = 2 + feed.publish(SESSION) + expect(events.at(-1)).toMatchObject({ session: { status: 'idle' } }) + }) + + it('publishes working from the pending submission, before the provider replays the turn', async () => { + const journal = await openJournal() + const { feed, events } = feedFor(new Map([[SESSION, { journal }]])) + events.length = 0 + await journal.appendSubmission({ + clientMessageId: 'client-1', + payloadFingerprint: 'fingerprint-1', + body: { kind: 'message', role: 'user', blocks: [{ type: 'text', text: 'write a poem' }] }, + fence: 1 + }) + + feed.publish(SESSION) + expect(events.at(-1)).toEqual({ + type: 'status', + session: expect.objectContaining({ status: 'working' }) + }) + + await journal.resolveDispatch({ + clientMessageId: 'client-1', + state: 'accepted', + providerIdentity: USER_IDENTITY, + fence: 1 + }) + feed.publish(SESSION) + expect(events.at(-1)).toEqual({ + type: 'status', + session: expect.objectContaining({ status: 'idle' }) + }) + }) + it('publishes working, then idle once the running marker is tombstoned, and never a repeat', async () => { const journal = await openJournal() const { feed, events } = feedFor(new Map([[SESSION, { journal }]])) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-status-feed.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-status-feed.ts index 61d9649587e0..3c4ce4b97a63 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-status-feed.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-status-feed.ts @@ -31,6 +31,7 @@ type StatusFeedSession = { journal: AgentSessionJournal params: { location: { workspaceId: string }; provider: AgentSessionRecord['provider'] } hasProviderChild?: boolean + fence?: number } export type StructuredAgentSessionStatusFeedDeps = { @@ -153,7 +154,9 @@ export class StructuredAgentSessionStatusFeed { journal: AgentSessionJournal ): AgentSessionStatusSummary { // An unreadable journal projects as "no turn": the chat itself shows the reset. - const items = journal.isReadOnly ? [] : journal.snapshot().items + const snapshot = journal.isReadOnly ? null : journal.snapshot() + const items = snapshot?.items ?? [] + const submissions = snapshot?.submissions ?? [] const record = this.deps.getRecord(sessionId) const providerSession = structuredAgentSessionProviderSessionMetadata(record) // The journal has no model: the record's acknowledged options are where an owner @@ -164,7 +167,7 @@ export class StructuredAgentSessionStatusFeed { workspaceId: session.params.location.workspaceId, agent: session.params.provider, ...(session.hasProviderChild ? { hostExecutionOwned: true as const } : {}), - ...projectStructuredAgentSessionStatusSummary(items), + ...projectStructuredAgentSessionStatusSummary(items, submissions, session.fence), ...(record?.rewind?.phase === 'prepared' || record?.rewind?.phase === 'provider-succeeded' ? { rewindBlockedReason: 'outcome-unknown' as const } : {}), diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-surface-lifetime.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-surface-lifetime.test.ts index 230e39cdaef2..f4e02f4491bd 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-surface-lifetime.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-surface-lifetime.test.ts @@ -8,6 +8,7 @@ import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterEach, beforeEach, describe, expect, it, vi, type Mock } from 'vitest' import type { AgentSessionOwnerProbe } from '../../../shared/agent-session-lease-adjudication' +import { hasUnansweredStructuredAgentSessionDispatch } from '../../../shared/structured-agent-session-projection' import { computeAgentSessionPayloadFingerprint } from '../../../shared/agent-session-mutation-envelope' import type { AgentSessionMutationEnvelope, @@ -334,6 +335,11 @@ describe('an unexpected provider exit', () => { acquisitionGeneration: 'generation-1' }) + const recoveredHistory = host.history({ sessionId: SESSION, direction: 'tail' }) + expect( + recoveredHistory.ok && + hasUnansweredStructuredAgentSessionDispatch(recoveredHistory.page.submissions) + ).toBe(false) expect(acquire).toHaveBeenCalledTimes(2) expect(dispatch).toHaveBeenCalledOnce() expect(store.getRecord(SESSION)?.lease).toMatchObject({ diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-turns.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-turns.ts index 3bd98a61735c..ba83b7e54ac6 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-turns.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-turns.ts @@ -97,6 +97,15 @@ export async function performSend( if (!(input.retryUnknown && existing?.dispatchState === 'unknown')) { await ctx.journal.appendSubmission({ ...input, fence: ctx.fence }) ctx.publish() + } else { + // Retry resumes work without moving or duplicating the original message. + await ctx.journal.resolveDispatch({ + clientMessageId: input.clientMessageId, + state: 'unknown', + reason: 'dispatch_retry_in_progress', + fence: ctx.fence + }) + ctx.publish() } const outcome = await dispatchSafely(ctx, input.clientMessageId, input.body) @@ -124,8 +133,7 @@ export async function performSend( clientMessageId: input.clientMessageId, state: 'unknown', reason: 'dispatch_result_persistence_failed', - fence: ctx.fence, - recovered: true + fence: ctx.fence }) } catch { // Nothing further to record; the pending row is settled on the next attach. diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-unexpected-exit.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-unexpected-exit.test.ts index 084ffc7547dd..2c2b9428a430 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-unexpected-exit.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-unexpected-exit.test.ts @@ -49,7 +49,11 @@ describe('provider-exit recovery tickets', () => { hasProviderChild: true, fence: 7, acquisitionGeneration: GENERATION, - journal: { snapshot: () => ({ items: [] }), appendLifecycleBatch } + journal: { + snapshot: () => ({ items: [] }), + appendLifecycleBatch, + markPendingSubmissionsUnknown: vi.fn(async () => []) + } } as unknown as StructuredAgentSessionHostSession const store = { getRecord: () => ({ @@ -89,6 +93,10 @@ describe('provider-exit recovery tickets', () => { expect(result).toMatchObject({ settlementRetryRequired: false, releasedFence: 8 }) expect(appendLifecycleBatch).toHaveBeenCalledOnce() + expect(session.journal.markPendingSubmissionsUnknown).toHaveBeenCalledWith( + 7, + 'provider_exited_before_acknowledgement' + ) expect(session.hasProviderChild).toBe(false) }) @@ -98,6 +106,7 @@ describe('provider-exit recovery tickets', () => { fence: 7, acquisitionGeneration: GENERATION, journal: { + markPendingSubmissionsUnknown: vi.fn(async () => []), snapshot: () => ({ items: [] }), appendLifecycleBatch: vi.fn(async () => { throw new Error('journal still unavailable') diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-unexpected-exit.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-unexpected-exit.ts index ddc4af9f2d34..43ba7dbbb2d7 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-unexpected-exit.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-unexpected-exit.ts @@ -81,6 +81,15 @@ export async function settleUnexpectedStructuredAgentSessionExit( settlementRetryRequired = true context.onBarrierError?.(unexpectedEvent.sessionId, error) } + try { + await session.journal.markPendingSubmissionsUnknown( + session.fence, + 'provider_exited_before_acknowledgement' + ) + } catch (error) { + settlementRetryRequired = true + context.onBarrierError?.(unexpectedEvent.sessionId, error) + } if (unexpectedEvent.settlementRetryRequired || settlementRetryRequired) { const retried = await retryUnexpectedExitSettlement({ context, @@ -170,6 +179,10 @@ export async function retryUnexpectedExitSettlement(input: { stableSettlementId: string }): Promise { try { + await input.session.journal.markPendingSubmissionsUnknown( + input.session.fence, + 'provider_exited_before_acknowledgement' + ) const mutations = unexpectedExitFallbackMutations( input.event, input.session, diff --git a/src/renderer/src/components/native-chat/NativeChatStructuredSession.tsx b/src/renderer/src/components/native-chat/NativeChatStructuredSession.tsx index da70c4b34466..a2b70db55b0e 100644 --- a/src/renderer/src/components/native-chat/NativeChatStructuredSession.tsx +++ b/src/renderer/src/components/native-chat/NativeChatStructuredSession.tsx @@ -352,7 +352,9 @@ export function NativeChatStructuredSession( targetPtyId={null} agent={props.agent} canSend={!prompt} - isWorking={controller.isWorking} + // Stop, not status: only a provider-minted turn can be interrupted, so the button + // must not flip while a dispatch is still unanswered. + isWorking={controller.turnId !== null} onStop={() => { if (controller.turnId) { void controller.cancel(controller.turnId) diff --git a/src/renderer/src/components/native-chat/use-structured-agent-session.test.tsx b/src/renderer/src/components/native-chat/use-structured-agent-session.test.tsx index abf185cb9136..5d8b291a957e 100644 --- a/src/renderer/src/components/native-chat/use-structured-agent-session.test.tsx +++ b/src/renderer/src/components/native-chat/use-structured-agent-session.test.tsx @@ -10,6 +10,7 @@ const mocks = vi.hoisted(() => ({ })) let fence = 3 let sessionCommands: { name: string; kind: 'command' | 'skill' }[] | undefined +let submissions: AgentJournalSubmission[] = [] vi.mock('@/runtime/structured-agent-session-client', () => ({ callStructuredAgentSession: mocks.call @@ -25,7 +26,7 @@ vi.mock('./use-structured-agent-session-read', () => ({ fence, commands: sessionCommands, items: [], - submissions: [], + submissions, status: 'ready', error: null, hasOlder: false, @@ -47,6 +48,7 @@ vi.mock('./use-structured-agent-session-outbox', () => ({ }) })) +import type { AgentJournalSubmission } from '../../../../shared/agent-session-journal-types' import { applyNativeChatSessionOptionSettingsMutation, resolveStructuredLaunchSeedOptions @@ -95,10 +97,72 @@ const OPTIONS = { current: { model: 'gpt-live', effort: 'medium' } } +describe('useStructuredAgentSession working state', () => { + beforeEach(() => { + vi.clearAllMocks() + fence = 3 + submissions = [] + mocks.call.mockResolvedValue(null) + }) + + it('reports work from an unanswered dispatch, and keeps the turn id provider-minted', () => { + submissions = [ + { + clientMessageId: 'client-1', + fence: 3, + payloadFingerprint: 'fingerprint-1', + dispatchState: 'pending', + providerItemId: null, + reason: null, + submittedAt: 1, + resolvedAt: null + } + ] + const { result } = renderHook(() => + useStructuredAgentSession({ + sessionId: 'session-1', + agent: 'codex', + target: LOCAL_TARGET, + isVisible: true + }) + ) + + expect(result.current.isWorking).toBe(true) + // Only the provider can mint a cancellable turn, so Stop stays unavailable here. + expect(result.current.turnId).toBeNull() + }) + + it('reports no work once the dispatch resolves and no turn is running', () => { + submissions = [ + { + clientMessageId: 'client-1', + fence: 3, + payloadFingerprint: 'fingerprint-1', + dispatchState: 'accepted', + providerItemId: 'codex:thread-1:turn-1', + reason: null, + submittedAt: 1, + resolvedAt: 2 + } + ] + const { result } = renderHook(() => + useStructuredAgentSession({ + sessionId: 'session-1', + agent: 'codex', + target: LOCAL_TARGET, + isVisible: true + }) + ) + + expect(result.current.isWorking).toBe(false) + }) +}) + describe('useStructuredAgentSession options', () => { beforeEach(() => { vi.clearAllMocks() fence = 3 + submissions = [] mocks.operationId .mockReset() .mockReturnValueOnce('operation-1') diff --git a/src/renderer/src/components/native-chat/use-structured-agent-session.ts b/src/renderer/src/components/native-chat/use-structured-agent-session.ts index 3befefa5edde..864b7e1f6089 100644 --- a/src/renderer/src/components/native-chat/use-structured-agent-session.ts +++ b/src/renderer/src/components/native-chat/use-structured-agent-session.ts @@ -22,7 +22,10 @@ import { structuredAgentSessionOptionPicks, structuredAgentSessionOptionSnapshot } from '../../../../shared/structured-agent-session-options' -import { activeStructuredAgentSessionTurnId } from '../../../../shared/structured-agent-session-projection' +import { + activeStructuredAgentSessionTurnId, + hasUnansweredStructuredAgentSessionDispatch +} from '../../../../shared/structured-agent-session-projection' import type { RuntimeClientTarget } from '@/runtime/runtime-rpc-client' import { callStructuredAgentSession } from '@/runtime/structured-agent-session-client' import { useStructuredAgentSessionHold } from './use-structured-agent-session-hold' @@ -80,6 +83,10 @@ export function useStructuredAgentSession(args: { // Refresh options each turn to confirm which model the provider actually selected. const turnId = activeStructuredAgentSessionTurnId(state.items) + // A dispatch the provider has not answered is already work; Claude's running row trails the + // send by seconds, and only a provider-minted turn is cancellable, so the two stay separate. + const isWorking = + turnId !== null || hasUnansweredStructuredAgentSessionDispatch(state.submissions, state.fence) const turnActivity = useMemo( () => selectStructuredAgentTurnActivity(state.items, turnId, state.activity), [state.activity, state.items, turnId] @@ -210,7 +217,7 @@ export function useStructuredAgentSession(args: { send: (...input: Parameters) => !commandPending.current && outboxController.send(...input), retry: outboxController.retry, - isWorking: turnId !== null, + isWorking, turnActivity, ...backgroundTasksView, turnId, diff --git a/src/shared/agent-session-journal-schemas.ts b/src/shared/agent-session-journal-schemas.ts index 89c2d9f7293a..bab70dd20443 100644 --- a/src/shared/agent-session-journal-schemas.ts +++ b/src/shared/agent-session-journal-schemas.ts @@ -186,7 +186,8 @@ export const AgentJournalSubmissionSchema = z.object({ providerItemId: z.string().nullable(), reason: z.string().nullable(), submittedAt: z.number(), - resolvedAt: z.number().nullable() + resolvedAt: z.number().nullable(), + recovered: z.literal(true).optional() }) export function isAdmissibleAgentJournalItemBody(value: unknown): value is AgentJournalItemBody { diff --git a/src/shared/agent-session-journal-types.ts b/src/shared/agent-session-journal-types.ts index c0074e015f59..c017a94b36e6 100644 --- a/src/shared/agent-session-journal-types.ts +++ b/src/shared/agent-session-journal-types.ts @@ -193,6 +193,7 @@ export type AgentJournalDispatchState = (typeof AGENT_JOURNAL_DISPATCH_STATES)[n * the turn reads as delivery unconfirmed, never as sent and never as failed. */ export type AgentJournalSubmission = { clientMessageId: string + /** Execution fence of the latest dispatch attempt or recovery. */ fence: number payloadFingerprint: string dispatchState: AgentJournalDispatchState @@ -202,6 +203,9 @@ export type AgentJournalSubmission = { reason: string | null submittedAt: number resolvedAt: number | null + /** Set when crash reconciliation resolved the dispatch, not the provider. A live + * `unknown` is a send still outstanding; a recovered one outlived its writer. */ + recovered?: true } /** Durable answer to "did my send land?", keyed by client message id. Only an diff --git a/src/shared/structured-agent-session-projection.test.ts b/src/shared/structured-agent-session-projection.test.ts index 77980fa86cc3..76cf6088cba0 100644 --- a/src/shared/structured-agent-session-projection.test.ts +++ b/src/shared/structured-agent-session-projection.test.ts @@ -1,10 +1,11 @@ import { describe, expect, it } from 'vitest' import { AGENT_STATUS_MAX_FIELD_LENGTH } from './agent-status-field-normalization' -import type { AgentJournalRenderItem } from './agent-session-journal-types' +import type { AgentJournalRenderItem, AgentJournalSubmission } from './agent-session-journal-types' import { parsePaneKey } from './stable-pane-id' import { activeStructuredAgentSessionTurnId, hasPersistedStructuredAgentSessionTurn, + hasUnansweredStructuredAgentSessionDispatch, projectStructuredItemToNativeChat, projectStructuredAgentSessionStatus, projectStructuredAgentSessionStatusSummary, @@ -19,6 +20,22 @@ function item( return { itemId, sequence, revision: 1, observedAt: sequence, body } } +function submission( + clientMessageId: string, + dispatchState: AgentJournalSubmission['dispatchState'] +): AgentJournalSubmission { + return { + clientMessageId, + fence: 1, + payloadFingerprint: clientMessageId, + dispatchState, + providerItemId: null, + reason: null, + submittedAt: 1, + resolvedAt: dispatchState === 'pending' ? null : 2 + } +} + describe('structured agent session status projection', () => { it('reuses immutable item projections and refreshes revisions and resolved prompts', () => { const original = item('diff', 1, { @@ -139,6 +156,75 @@ describe('structured agent session status projection', () => { }) }) + it('reads a session as working while a dispatch is unanswered, before any lifecycle row', () => { + const asked = item('asked', 1, { + kind: 'message', + role: 'user', + blocks: [{ type: 'text', text: 'go' }] + }) + const pending = [submission('m1', 'pending')] + + expect(hasUnansweredStructuredAgentSessionDispatch(pending)).toBe(true) + expect(projectStructuredAgentSessionStatus([asked], pending)).toBe('working') + // The first send has no journalled message until the provider replays it. + expect(projectStructuredAgentSessionStatusSummary([], pending)).toEqual({ + status: 'working', + latestPrompt: '' + }) + expect(projectStructuredAgentSessionStatusSummary([asked], pending)).toEqual({ + status: 'working', + latestPrompt: 'go' + }) + }) + + it('does not resurrect old-host unknown work after its execution fence advances', () => { + const oldHostSubmission = { ...submission('m1', 'unknown'), fence: 2 } + expect(hasUnansweredStructuredAgentSessionDispatch([oldHostSubmission], 2)).toBe(true) + expect(hasUnansweredStructuredAgentSessionDispatch([oldHostSubmission], 3)).toBe(false) + }) + + it('recognizes recovery from an older host without the optional marker', () => { + expect( + hasUnansweredStructuredAgentSessionDispatch([ + { ...submission('m1', 'unknown'), reason: 'host_restarted_before_acknowledgement' } + ]) + ).toBe(false) + }) + + it('stops reading a resolved dispatch as work, and lets a pending prompt outrank it', () => { + const asked = item('asked', 1, { + kind: 'message', + role: 'user', + blocks: [{ type: 'text', text: 'go' }] + }) + const prompt = item('prompt', 2, { + kind: 'approval', + title: 'Run command?', + detail: null, + options: [{ id: 'yes', label: 'Allow' }], + resolution: { state: 'pending', selectedOptionId: null, resolvedBy: null, resolvedAt: null } + }) + + for (const state of ['accepted', 'rejected'] as const) { + expect(hasUnansweredStructuredAgentSessionDispatch([submission('m1', state)])).toBe(false) + expect(projectStructuredAgentSessionStatus([asked], [submission('m1', state)])).toBe('idle') + } + // The ack budget elapsing is a delivery answer, not an answer about the turn. + expect(hasUnansweredStructuredAgentSessionDispatch([submission('m1', 'unknown')])).toBe(true) + expect( + hasUnansweredStructuredAgentSessionDispatch([ + { ...submission('m1', 'unknown'), recovered: true } + ]) + ).toBe(false) + expect( + projectStructuredAgentSessionStatus([asked, prompt], [submission('m1', 'pending')]) + ).toBe('attention') + expect(projectStructuredAgentSessionStatusSummary([], [])).toEqual({ + status: null, + latestPrompt: '' + }) + }) + it('carries the running tool and the newest assistant prose the sidebar row shows', () => { const ask = item('ask', 1, { kind: 'message', diff --git a/src/shared/structured-agent-session-projection.ts b/src/shared/structured-agent-session-projection.ts index 1301f766e2b0..a48537d05b7a 100644 --- a/src/shared/structured-agent-session-projection.ts +++ b/src/shared/structured-agent-session-projection.ts @@ -5,6 +5,7 @@ import { } from './agent-status-field-normalization' import type { AgentJournalRenderItem, + AgentJournalSubmission, AgentJournalToolCallItem } from './agent-session-journal-types' import { @@ -175,6 +176,34 @@ export function hasPersistedStructuredAgentSessionTurn( ) } +/** + * A send the host has journaled that the provider has neither opened a turn for nor refused. + * + * Codex declares `turn/started` within ~150ms, but Claude's running row can only be written once + * the SDK echoes the user message back — a 3.4s median and 18s at p90 on real journals. Waiting + * on that echo to call a session working leaves the whole gap reading idle in the chat and in + * every session list, so the send itself is the evidence. + * + * `unknown` still counts: it only means the ack budget elapsed, which happens on 30% of Claude + * sends whose turn then arrives anyway, and delivery confidence is a separate question from + * whether work is owed. A recovered `unknown` does not — that one outlived the host generation + * that sent it, so there is nothing still running to report. + */ +export function hasUnansweredStructuredAgentSessionDispatch( + submissions: readonly AgentJournalSubmission[], + currentFence?: number | null +): boolean { + return submissions.some( + (submission) => + (currentFence == null || submission.fence >= currentFence) && + (submission.dispatchState === 'pending' || + (submission.dispatchState === 'unknown' && + submission.recovered !== true && + // Older hosts publish the recovery reason but omit the optional marker. + submission.reason !== 'host_restarted_before_acknowledgement')) + ) +} + export type StructuredAgentSessionProjectedStatus = 'working' | 'attention' | 'idle' export function structuredAgentSessionTabId(sessionId: string): string { @@ -182,7 +211,9 @@ export function structuredAgentSessionTabId(sessionId: string): string { } export function projectStructuredAgentSessionStatus( - items: readonly AgentJournalRenderItem[] + items: readonly AgentJournalRenderItem[], + submissions: readonly AgentJournalSubmission[] = [], + currentFence?: number | null ): StructuredAgentSessionProjectedStatus { if ( items.some( @@ -193,7 +224,10 @@ export function projectStructuredAgentSessionStatus( ) { return 'attention' } - return activeStructuredAgentSessionTurnId(items) ? 'working' : 'idle' + return activeStructuredAgentSessionTurnId(items) || + hasUnansweredStructuredAgentSessionDispatch(submissions, currentFence) + ? 'working' + : 'idle' } function messageProse(blocks: readonly NativeChatBlock[]): string { @@ -270,12 +304,19 @@ export type StructuredAgentSessionStatusProjection = { * the 8 KB body): a streamed reply re-projects on every journal checkpoint, so the frame * has to stay small even though the row only ever renders one line of it. */ export function projectStructuredAgentSessionStatusSummary( - items: readonly AgentJournalRenderItem[] + items: readonly AgentJournalRenderItem[], + submissions: readonly AgentJournalSubmission[] = [], + currentFence?: number | null ): StructuredAgentSessionStatusProjection { - if (!hasPersistedStructuredAgentSessionTurn(items)) { + // A first send has no journalled message until the provider replays it, so the pending + // dispatch is also what makes a brand-new session listable at all. + if ( + !hasPersistedStructuredAgentSessionTurn(items) && + !hasUnansweredStructuredAgentSessionDispatch(submissions, currentFence) + ) { return { status: null, latestPrompt: '' } } - const status = projectStructuredAgentSessionStatus(items) + const status = projectStructuredAgentSessionStatus(items, submissions, currentFence) const activeToolCall = status === 'working' ? activeStructuredAgentSessionToolCall(items) : null const toolName = activeToolCall ? normalizeOptionalField(activeToolCall.name, AGENT_STATUS_TOOL_NAME_MAX_LENGTH) From 4b4acf26a4775cfdde4258566683e41a6c276f5b Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 9 Sep 2026 21:50:31 -0700 Subject: [PATCH 003/261] fix(mobile): enable patch-free iOS text selection in native chat (#19769) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(mobile): make every native-chat text node selectable Long-press selection worked on some chat text and not others. Markdown paragraphs — the default block for agent prose — were the one block type left out when headings, quotes, code, lists and table cells gained `selectable`, and tool result output, diff rows, the unloadable-image placeholder, permission/question bodies and the send-error banner never had it at all. Selection is now set on every content Text in the chat surface, on the outermost block Text so nested inline spans inherit it. Labels inside a Pressable (option rows, tool-line headers, buttons) are deliberately left alone: selection there would swallow the tap they exist for. Extracting MobileNativeChatEmptyState keeps the view under its max-lines cap and matches desktop, where NativeChatEmptyState is already its own component. Tests render each surface and assert selection on the block that carries the prose; both files were ablated against the unfixed source (4/10 and 3/5 red) so they pin the defect rather than the current behavior. * fix(mobile): support native text range selection on iOS * fix(mobile): remove persistent assistant message controls * fix(mobile): scope patch-free text selection to chat Use the stock react-native-uitextview dependency behind an iOS adapter and opt assistant Markdown into range selection only in native chat. Preserve the existing React Native Text behavior elsewhere and remove the persistent assistant controls. --------- Co-authored-by: Merge Sim --- mobile/package.json | 1 + mobile/pnpm-lock.yaml | 14 ++ mobile/src/components/MobileMarkdown.tsx | 155 ++++++++++---- .../components/MobileSelectableText.ios.tsx | 38 ++++ .../src/components/MobileSelectableText.tsx | 1 + .../mobile-markdown-selectable.test.tsx | 110 ++++++++++ .../mobile-selectable-text-ios.test.tsx | 198 ++++++++++++++++++ .../session/MobileNativeChatMessage.test.ts | 15 ++ .../src/session/MobileNativeChatMessage.tsx | 107 ++-------- .../src/session/MobileNativeChatToolRun.tsx | 3 - mobile/src/session/MobileNativeChatView.tsx | 27 +-- .../mobile-native-chat-message-styles.ts | 21 -- .../mobile-native-chat-message-text.test.ts | 28 +-- .../mobile-native-chat-message-text.ts | 12 -- 14 files changed, 503 insertions(+), 227 deletions(-) create mode 100644 mobile/src/components/MobileSelectableText.ios.tsx create mode 100644 mobile/src/components/MobileSelectableText.tsx create mode 100644 mobile/src/components/mobile-markdown-selectable.test.tsx create mode 100644 mobile/src/components/mobile-selectable-text-ios.test.tsx diff --git a/mobile/package.json b/mobile/package.json index 159028c978dc..13f2f98acf55 100644 --- a/mobile/package.json +++ b/mobile/package.json @@ -57,6 +57,7 @@ "react-native-safe-area-context": "^5.7.0", "react-native-screens": "^4.24.0", "react-native-svg": "^15.15.4", + "react-native-uitextview": "2.2.0", "react-native-web": "^0.21.2", "react-native-webview": "13.16.2", "react-native-worklets": "^0.8.3", diff --git a/mobile/pnpm-lock.yaml b/mobile/pnpm-lock.yaml index d44e612bb737..7cebee89eec0 100644 --- a/mobile/pnpm-lock.yaml +++ b/mobile/pnpm-lock.yaml @@ -136,6 +136,9 @@ importers: react-native-svg: specifier: ^15.15.4 version: 15.15.4(react-native@0.83.10(patch_hash=44876634a8efbb0f2c3f66cd4332be170ec821d1cbfc0264ac80680983e8513d)(@babel/core@7.29.7)(@react-native/metro-config@0.85.2(@babel/core@7.29.7))(@types/react@19.2.14)(react@19.2.8))(react@19.2.8) + react-native-uitextview: + specifier: 2.2.0 + version: 2.2.0(react-native@0.83.10(patch_hash=44876634a8efbb0f2c3f66cd4332be170ec821d1cbfc0264ac80680983e8513d)(@babel/core@7.29.7)(@react-native/metro-config@0.85.2(@babel/core@7.29.7))(@types/react@19.2.14)(react@19.2.8))(react@19.2.8) react-native-web: specifier: ^0.21.2 version: 0.21.2(react-dom@19.2.8(react@19.2.8))(react@19.2.8) @@ -6158,6 +6161,12 @@ packages: react: '*' react-native: '*' + react-native-uitextview@2.2.0: + resolution: {integrity: sha512-Vbv3cTAuyfkYrfsR2YKFsOd9OYgfys+IX5yvvYY7Wd6TrOd6FSGrX93xtQHjeLXV1ds6fDJcFJsuLi3S4Ypr8A==} + peerDependencies: + react: '*' + react-native: '*' + react-native-web@0.21.2: resolution: {integrity: sha512-SO2t9/17zM4iEnFvlu2DA9jqNbzNhoUP+AItkoCOyFmDMOhUnBBznBDCYN92fGdfAkfQlWzPoez6+zLxFNsZEg==} peerDependencies: @@ -14609,6 +14618,11 @@ snapshots: react-native: 0.83.10(patch_hash=44876634a8efbb0f2c3f66cd4332be170ec821d1cbfc0264ac80680983e8513d)(@babel/core@7.29.7)(@react-native/metro-config@0.85.2(@babel/core@7.29.7))(@types/react@19.2.14)(react@19.2.8) warn-once: 0.1.1 + react-native-uitextview@2.2.0(react-native@0.83.10(patch_hash=44876634a8efbb0f2c3f66cd4332be170ec821d1cbfc0264ac80680983e8513d)(@babel/core@7.29.7)(@react-native/metro-config@0.85.2(@babel/core@7.29.7))(@types/react@19.2.14)(react@19.2.8))(react@19.2.8): + dependencies: + react: 19.2.8 + react-native: 0.83.10(patch_hash=44876634a8efbb0f2c3f66cd4332be170ec821d1cbfc0264ac80680983e8513d)(@babel/core@7.29.7)(@react-native/metro-config@0.85.2(@babel/core@7.29.7))(@types/react@19.2.14)(react@19.2.8) + react-native-web@0.21.2(react-dom@19.2.8(react@19.2.8))(react@19.2.8): dependencies: '@babel/runtime': 7.29.2 diff --git a/mobile/src/components/MobileMarkdown.tsx b/mobile/src/components/MobileMarkdown.tsx index 2f5b52cfe18b..7ecfab8c393f 100644 --- a/mobile/src/components/MobileMarkdown.tsx +++ b/mobile/src/components/MobileMarkdown.tsx @@ -1,5 +1,22 @@ -import { Fragment, memo, useMemo, type ReactNode } from 'react' -import { Linking, Pressable, ScrollView, Text, View } from 'react-native' +import { MobileSelectableText } from './MobileSelectableText' +import { + Fragment, + createElement, + createContext, + memo, + useContext, + useMemo, + type ComponentType, + type ReactNode +} from 'react' +import { + Linking, + Pressable, + ScrollView, + Text as NativeText, + View, + type TextProps +} from 'react-native' import { normalizeMobileMarkdownPreviewHtml } from './mobile-markdown-preview-html' import { styles } from './mobile-markdown-styles' import { @@ -19,6 +36,8 @@ import { MermaidDiagram } from './pr-sidebar/MermaidDiagram' type Props = { content?: string fallback?: string + /** Enables iOS range selection for native-chat transcript prose. */ + rangeSelectable?: boolean /** Multiplier for prose font size (paragraphs, lists, quotes). Defaults to 1; * the chat view passes >1 so agent prose reads larger than the compact base. */ textScale?: number @@ -33,6 +52,12 @@ const MAX_TABLE_ROWS = 40 const MAX_TABLE_COLUMNS = 8 /** Prose base size — passed to MermaidDiagram fallback mono text. */ const MERMAID_BASE = 13 +const MarkdownTextContext = createContext>(NativeText) + +function MarkdownText(props: TextProps): React.JSX.Element { + const TextComponent = useContext(MarkdownTextContext) + return createElement(TextComponent, props) +} // Web/mail hrefs open the system handler; file-target hrefs (file: URIs and // scheme-less paths — the entire desktop file-link contract) go to onOpenFile. @@ -64,13 +89,13 @@ function renderTextRun( return segments.map((segment, segmentIndex) => { if (segment.type === 'file') { return ( - onOpenFile(segment.path)} > {segment.value} - + ) } return {segment.value} @@ -105,22 +130,34 @@ function renderInline(text: string, onOpenFile?: (pathText: string) => void): Re const link = token.match(/^\[([^\]]+)\]\(([^)]+)\)$/) if (image) { parts.push( - openMarkdownHref(image[2]!, onOpenFile)}> + openMarkdownHref(image[2]!, onOpenFile)} + > {image[1] || 'image'} - + ) } else if (link) { parts.push( - openMarkdownHref(link[2]!, onOpenFile)}> + openMarkdownHref(link[2]!, onOpenFile)} + > {link[1]} - + ) } else if (/^https?:\/\//i.test(token)) { const { url, trailing } = trimAutolinkTrailingPunctuation(token) parts.push( - openMarkdownHref(url, onOpenFile)}> + openMarkdownHref(url, onOpenFile)} + > {url} - + ) if (trailing) { parts.push({trailing}) @@ -129,38 +166,38 @@ function renderInline(text: string, onOpenFile?: (pathText: string) => void): Re const code = token.slice(1, -1) if (onOpenFile && isFilePathCodeSpan(code)) { parts.push( - onOpenFile(normalizeFilePath(code.trim()))} > {code} - + ) } else { parts.push( - + {code} - + ) } } else if (token.startsWith('~~')) { parts.push( - + {renderTextRun(token.slice(2, -2), `${key}i`, onOpenFile)} - + ) } else if (token.startsWith('**') || token.startsWith('__')) { parts.push( - + {renderTextRun(token.slice(2, -2), `${key}i`, onOpenFile)} - + ) } else { parts.push( - + {renderTextRun(token.slice(1, -1), `${key}i`, onOpenFile)} - + ) } } @@ -171,7 +208,13 @@ function renderInline(text: string, onOpenFile?: (pathText: string) => void): Re return parts } -function MobileMarkdownInner({ content, fallback = '', textScale = 1, onOpenFile }: Props) { +function MobileMarkdownContent({ + content, + fallback = '', + rangeSelectable = false, + textScale = 1, + onOpenFile +}: Props) { const text = content?.trim() ?? '' const previewText = useMemo(() => normalizeMobileMarkdownPreviewHtml(text), [text]) const blocks = useMemo(() => parseMobileMarkdown(previewText), [previewText]) @@ -181,30 +224,35 @@ function MobileMarkdownInner({ content, fallback = '', textScale = 1, onOpenFile const proseScale = scaled(13) const listScale = scaled(14) if (!text) { - return fallback ? {fallback} : null + return fallback ? ( + + {fallback} + + ) : null } const mermaidSourceOccurrences = new Map() + // Native-chat range selection is set on each block; nested inline spans inherit it. return ( {blocks.map((block, index) => { if (block.type === 'heading') { return ( - {renderInline(block.text, onOpenFile)} - + ) } if (block.type === 'quote') { return ( - + {renderInline(block.text, onOpenFile)} - + ) } @@ -225,10 +273,12 @@ function MobileMarkdownInner({ content, fallback = '', textScale = 1, onOpenFile } return ( - {block.language ? {block.language} : null} - + {block.language ? ( + {block.language} + ) : null} + {block.text} - + ) } @@ -239,10 +289,10 @@ function MobileMarkdownInner({ content, fallback = '', textScale = 1, onOpenFile style={styles.imageFrame} onPress={() => openMarkdownHref(block.url, onOpenFile)} > - {block.alt || 'Open image'} - + {block.alt || 'Open image'} + {block.url} - + ) } @@ -256,26 +306,30 @@ function MobileMarkdownInner({ content, fallback = '', textScale = 1, onOpenFile {visibleHeaders.map((header, cellIndex) => ( - + {renderInline(header, onOpenFile)} - + ))} {visibleRows.map((row, rowIndex) => ( {visibleHeaders.map((_, cellIndex) => ( - + {renderInline(row[cellIndex] ?? '', onOpenFile)} - + ))} ))} {hiddenRows > 0 || hiddenColumns > 0 ? ( - + {hiddenRows > 0 ? `${hiddenRows} more rows` : ''} {hiddenRows > 0 && hiddenColumns > 0 ? ' · ' : ''} {hiddenColumns > 0 ? `${hiddenColumns} more columns` : ''} - + ) : null} @@ -286,7 +340,7 @@ function MobileMarkdownInner({ content, fallback = '', textScale = 1, onOpenFile {block.items.map((item, itemIndex) => ( - + {item.checked == null ? block.ordered ? `${itemIndex + 1}.` @@ -294,10 +348,10 @@ function MobileMarkdownInner({ content, fallback = '', textScale = 1, onOpenFile : item.checked ? '[x]' : '[ ]'} - - + + {renderInline(item.text, onOpenFile)} - + ))} @@ -307,18 +361,31 @@ function MobileMarkdownInner({ content, fallback = '', textScale = 1, onOpenFile return } return ( - + {block.text.split('\n').map((line, lineIndex) => ( {lineIndex > 0 ? '\n' : null} {renderInline(line, onOpenFile)} ))} - + ) })} ) } +function MobileMarkdownInner(props: Props): React.JSX.Element | null { + const TextComponent = props.rangeSelectable ? MobileSelectableText : NativeText + return ( + + + + ) +} + export const MobileMarkdown = memo(MobileMarkdownInner) diff --git a/mobile/src/components/MobileSelectableText.ios.tsx b/mobile/src/components/MobileSelectableText.ios.tsx new file mode 100644 index 000000000000..74cb686e26e5 --- /dev/null +++ b/mobile/src/components/MobileSelectableText.ios.tsx @@ -0,0 +1,38 @@ +import { Children, Fragment, isValidElement, type ReactNode } from 'react' +import { StyleSheet, Text, UIManager, type TextProps } from 'react-native' +import { UITextView } from 'react-native-uitextview' + +// Older development clients can load this bundle before rebuilding their native views. +const hasRangeSelection = UIManager.hasViewManagerConfig('RNUITextView') + +function flattenFragments(children: ReactNode): ReactNode[] { + return ( + Children.map(children, (child) => + isValidElement<{ children?: ReactNode }>(child) && child.type === Fragment + ? flattenFragments(child.props.children) + : child + ) ?? [] + ) +} + +export function MobileSelectableText({ children, style, ...props }: TextProps): React.JSX.Element { + if (!hasRangeSelection) { + return ( + + {children} + + ) + } + + // The native span adapter otherwise maps numeric bold to semibold. + const textStyle = StyleSheet.flatten(style) + const nativeStyle = + textStyle?.fontWeight === '700' || textStyle?.fontWeight === 700 + ? { ...textStyle, fontWeight: 'bold' as const } + : style + return ( + + {flattenFragments(children)} + + ) +} diff --git a/mobile/src/components/MobileSelectableText.tsx b/mobile/src/components/MobileSelectableText.tsx new file mode 100644 index 000000000000..ddfffc37ee9f --- /dev/null +++ b/mobile/src/components/MobileSelectableText.tsx @@ -0,0 +1 @@ +export { Text as MobileSelectableText } from 'react-native' diff --git a/mobile/src/components/mobile-markdown-selectable.test.tsx b/mobile/src/components/mobile-markdown-selectable.test.tsx new file mode 100644 index 000000000000..2b42fc707155 --- /dev/null +++ b/mobile/src/components/mobile-markdown-selectable.test.tsx @@ -0,0 +1,110 @@ +import { createElement } from 'react' +import { act, create, type ReactTestRenderer } from 'react-test-renderer' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { MobileMarkdown } from './MobileMarkdown' + +vi.mock('react-native', () => ({ + Linking: { openURL: () => Promise.resolve() }, + Pressable: 'Pressable', + ScrollView: 'ScrollView', + StyleSheet: { create: (styles: unknown) => styles, hairlineWidth: 1 }, + Text: 'Text', + View: 'View' +})) + +vi.mock('./pr-sidebar/MermaidDiagram', () => ({ MermaidDiagram: 'MermaidDiagram' })) + +type TestNode = { + type: string + props: Record + children: (TestNode | string)[] | null +} + +/** Every Text that is not nested inside another Text, paired with the full prose + * it renders. Nested inline spans inherit selection, so only these carry it. */ +function outermostTextNodes( + node: TestNode | string, + insideText = false +): { text: string; selectable: boolean }[] { + if (typeof node === 'string') { + return [] + } + const children = node.children ?? [] + if (node.type === 'Text' && !insideText) { + return [{ text: flattenText(node), selectable: node.props.selectable === true }] + } + return children.flatMap((child) => outermostTextNodes(child, insideText || node.type === 'Text')) +} + +function flattenText(node: TestNode | string): string { + if (typeof node === 'string') { + return node + } + return (node.children ?? []).map(flattenText).join('') +} + +function renderMarkdown(props: Parameters[0]): TestNode { + let renderer: ReactTestRenderer | null = null + act(() => { + renderer = create(createElement(MobileMarkdown, { rangeSelectable: true, ...props })) + }) + const tree = renderer!.toJSON() as unknown as TestNode + act(() => renderer!.unmount()) + return tree +} + +function selectableFor(tree: TestNode, needle: string): boolean { + const match = outermostTextNodes(tree).find((entry) => entry.text.includes(needle)) + if (!match) { + throw new Error(`no Text rendered "${needle}"`) + } + return match.selectable +} + +describe('MobileMarkdown selection', () => { + afterEach(() => vi.clearAllMocks()) + + // Paragraphs are the default block for agent prose, and were the one block + // type left non-selectable when the others gained it. + it.each([ + ['paragraph', 'Paragraph prose here.'], + ['heading', 'Heading prose'], + ['quote', 'Quote prose'], + ['code', 'const code = 1'], + ['list item', 'List item prose'], + ['table header', 'Head A'], + ['table cell', 'Cell A'] + ])('makes %s prose selectable', (_label, needle) => { + const content = [ + '# Heading prose', + '', + 'Paragraph prose here.', + '', + '> Quote prose', + '', + '```ts', + 'const code = 1', + '```', + '', + '- List item prose', + '', + '| Head A | Head B |', + '| --- | --- |', + '| Cell A | Cell B |' + ].join('\n') + expect(selectableFor(renderMarkdown({ content }), needle)).toBe(true) + }) + + it('makes the empty-content fallback selectable', () => { + const tree = renderMarkdown({ content: '', fallback: 'Fallback prose' }) + expect(selectableFor(tree, 'Fallback prose')).toBe(true) + }) + + it('keeps inline spans inside their selectable block rather than splitting it', () => { + const tree = renderMarkdown({ content: 'Prose with `code` and **bold** inline.' }) + const blocks = outermostTextNodes(tree) + expect(blocks).toHaveLength(1) + expect(blocks[0]!.selectable).toBe(true) + expect(blocks[0]!.text).toBe('Prose with code and bold inline.') + }) +}) diff --git a/mobile/src/components/mobile-selectable-text-ios.test.tsx b/mobile/src/components/mobile-selectable-text-ios.test.tsx new file mode 100644 index 000000000000..3e9ee710be4e --- /dev/null +++ b/mobile/src/components/mobile-selectable-text-ios.test.tsx @@ -0,0 +1,198 @@ +import { createElement, Fragment } from 'react' +import { act, create, type ReactTestRenderer } from 'react-test-renderer' +import { afterEach, describe, expect, it, vi } from 'vitest' + +const native = vi.hoisted(() => ({ available: true })) +vi.mock('react-native', () => ({ + Platform: { OS: 'ios' }, + UIManager: { hasViewManagerConfig: () => native.available }, + Linking: { openURL: vi.fn() }, + Text: 'Text', + View: 'View', + ScrollView: 'ScrollView', + Pressable: 'Pressable', + StyleSheet: { + create: (styles: unknown) => styles, + flatten: (style: unknown): object => + Array.isArray(style) + ? Object.assign({}, ...style.flat(Infinity).filter(Boolean)) + : (style ?? {}), + hairlineWidth: 1 + } +})) +vi.mock('react-native/Libraries/Utilities/codegenNativeComponent', () => ({ + default: (name: string) => name +})) +// Exercise the dependency's real span conversion without a native runtime. +vi.mock('react-native-uitextview', () => import('react-native-uitextview/src/Text')) +vi.mock('./MobileSelectableText', () => import('./MobileSelectableText.ios')) +vi.mock('./pr-sidebar/MermaidDiagram', () => ({ MermaidDiagram: 'MermaidDiagram' })) + +let renderer: ReactTestRenderer | undefined +afterEach(() => { + act(() => renderer?.unmount()) + renderer = undefined + native.available = true + vi.resetModules() + vi.restoreAllMocks() +}) + +function render(element: React.ReactElement): ReactTestRenderer { + act(() => { + renderer = create(element) + }) + return renderer! +} + +function nodes(tree: ReactTestRenderer, name: string) { + return tree.root.findAll((node) => node.type === name) +} + +describe('iOS selectable text boundary', () => { + it('preserves line-scoped keys when repeated inline spans update or disappear', async () => { + const errors = vi.spyOn(console, 'error').mockImplementation(() => {}) + const { MobileMarkdown } = await import('./MobileMarkdown') + const onOpenFile = vi.fn() + const line = '**same** [file](src/main.ts)' + const tree = render( + createElement(MobileMarkdown, { + content: `${line}\n${line}`, + rangeSelectable: true, + onOpenFile + }) + ) + expect( + nodes(tree, 'RNUITextViewChild') + .map((node) => node.props.text) + .join('') + ).toBe('same file\nsame file') + act(() => + tree.update( + createElement(MobileMarkdown, { + content: `${line}\n**changed** [file](src/main.ts)`, + rangeSelectable: true, + onOpenFile + }) + ) + ) + expect( + nodes(tree, 'RNUITextViewChild') + .map((node) => node.props.text) + .join('') + ).toBe('same file\nchanged file') + act(() => + tree.update( + createElement(MobileMarkdown, { content: line, rangeSelectable: true, onOpenFile }) + ) + ) + const spans = nodes(tree, 'RNUITextViewChild') + expect(spans.map((node) => node.props.text).join('')).toBe('same file') + act(() => spans.find((node) => node.props.text === 'file')!.props.onPress()) + expect(onOpenFile).toHaveBeenCalledExactlyOnceWith('src/main.ts') + expect(errors.mock.calls.filter((args) => String(args[0]).includes('same key'))).toEqual([]) + }) + + it.each([ + ['500', 'medium'], + ['600', 'semibold'], + ['700', 'bold'] + ] as const)('preserves font weight %s', async (fontWeight, expected) => { + const { MobileSelectableText: Text } = await import('./MobileSelectableText.ios') + const tree = render(createElement(Text, { selectable: true, style: { fontWeight } }, 'Weight')) + expect(nodes(tree, 'RNUITextViewChild')[0]!.props.style.fontWeight).toBe(expected) + }) + + it('keeps fragments, arrays, newlines and nested styles in one native root', async () => { + const { MobileSelectableText: Text } = await import('./MobileSelectableText.ios') + const tree = render( + createElement( + Text, + { selectable: true, style: { fontSize: 18 } }, + createElement(Fragment, null, 'Before ', ['one', '\n']), + createElement(Text, { style: { fontWeight: '700' } }, 'bold'), + createElement(Text, { style: { color: 'blue' } }, 'nested'), + ' after' + ) + ) + expect(nodes(tree, 'RNUITextView')).toHaveLength(1) + expect(nodes(tree, 'Text')).toHaveLength(0) + const spans = nodes(tree, 'RNUITextViewChild') + expect(spans.map((node) => node.props.text).join('')).toBe('Before one\nboldnested after') + expect(spans.find((node) => node.props.text === 'bold')?.props.style).toMatchObject({ + fontSize: 18, + fontWeight: 'bold' + }) + expect(spans.find((node) => node.props.text === 'nested')?.props.style).toMatchObject({ + fontSize: 18, + color: 'blue' + }) + }) + + it('preserves Markdown text, inline styles and file-link callbacks', async () => { + const { MobileMarkdown } = await import('./MobileMarkdown') + const onOpenFile = vi.fn() + const tree = render( + createElement(MobileMarkdown, { + content: 'Hello 😀 [src/main.ts](src/main.ts) and `code`.\nNext line.', + rangeSelectable: true, + onOpenFile + }) + ) + const spans = nodes(tree, 'RNUITextViewChild') + expect(spans.map((node) => node.props.text).join('')).toBe( + 'Hello 😀 src/main.ts and code.\nNext line.' + ) + const link = spans.find((node) => node.props.text === 'src/main.ts')! + expect(link.props.style.color).toBeDefined() + act(() => link.props.onPress()) + expect(onOpenFile).toHaveBeenCalledExactlyOnceWith('src/main.ts') + expect(nodes(tree, 'RNUITextView')).toHaveLength(1) + }) + + it('keeps ordinary button labels on React Native Text', async () => { + const { MobileSelectableText: Text } = await import('./MobileSelectableText.ios') + const tree = render(createElement(Text, null, 'Submit')) + expect(nodes(tree, 'RNUITextView')).toHaveLength(0) + expect(nodes(tree, 'Text')).toHaveLength(1) + }) + + it('uses native range selection only when Markdown opts in', async () => { + const { MobileMarkdown } = await import('./MobileMarkdown') + const tree = render(createElement(MobileMarkdown, { content: 'Transcript prose' })) + expect(nodes(tree, 'RNUITextView')).toHaveLength(0) + expect( + nodes(tree, 'Text').find((node) => node.children.includes('Transcript prose'))?.props + .selectable + ).toBe(false) + act(() => + tree.update( + createElement(MobileMarkdown, { content: 'Transcript prose', rangeSelectable: true }) + ) + ) + expect(nodes(tree, 'RNUITextView')).toHaveLength(1) + }) + + it('keeps code-language labels on styled React Native Text', async () => { + const { MobileMarkdown } = await import('./MobileMarkdown') + const tree = render( + createElement(MobileMarkdown, { + content: '```ts\nconst value = 1\n```', + rangeSelectable: true + }) + ) + const label = nodes(tree, 'Text').find((node) => node.children.join('') === 'ts')! + expect(label.props.style.textTransform).toBe('uppercase') + expect(nodes(tree, 'RNUITextView')).toHaveLength(1) + }) + + it('falls back for older clients without the native view', async () => { + native.available = false + const { MobileSelectableText: Text } = await import('./MobileSelectableText.ios') + const tree = render( + createElement(Text, { selectable: true }, 'Old client ', createElement(Text, null, 'inline')) + ) + expect(nodes(tree, 'RNUITextView')).toHaveLength(0) + expect(nodes(tree, 'Text')).toHaveLength(2) + expect(nodes(tree, 'Text')[0]!.props.selectable).toBe(true) + }) +}) diff --git a/mobile/src/session/MobileNativeChatMessage.test.ts b/mobile/src/session/MobileNativeChatMessage.test.ts index 677b76d240c4..e09d1631a8dc 100644 --- a/mobile/src/session/MobileNativeChatMessage.test.ts +++ b/mobile/src/session/MobileNativeChatMessage.test.ts @@ -106,6 +106,21 @@ describe('MobileNativeChatMessage', () => { expect(texts.some((text) => text.includes('/tmp/host.png'))).toBe(true) }) + it('makes user message text selectable', () => { + const tree = render(userMessage([{ type: 'text', text: 'Prompt I typed' }])) + const text = tree.root + .findAllByType('Text' as never) + .find((node) => String(node.children.join('')) === 'Prompt I typed') + expect(text?.props.selectable).toBe(true) + }) + + it('routes assistant prose through selectable Markdown', () => { + const tree = render(toolMessage([{ type: 'text', text: 'Agent reply prose' }])) + const markdown = tree.root.findByType('MobileMarkdown' as never) + expect(markdown.props.content).toBe('Agent reply prose') + expect(markdown.props.rangeSelectable).toBe(true) + }) + it('labels a tool row with the target path instead of raw input JSON', () => { const tree = render( toolMessage([{ type: 'tool-call', name: 'Read', input: { file_path: 'src/index.ts' } }]), diff --git a/mobile/src/session/MobileNativeChatMessage.tsx b/mobile/src/session/MobileNativeChatMessage.tsx index cc6c086b1f3b..5d013688249a 100644 --- a/mobile/src/session/MobileNativeChatMessage.tsx +++ b/mobile/src/session/MobileNativeChatMessage.tsx @@ -1,7 +1,6 @@ -import { memo, useEffect, useRef, useState } from 'react' -import { Image, Pressable, Text, View } from 'react-native' -import * as Clipboard from 'expo-clipboard' -import { ArrowUp, Copy } from 'lucide-react-native' +import { MobileSelectableText as Text } from '../components/MobileSelectableText' +import { memo } from 'react' +import { Image, Text as NativeText, View } from 'react-native' import { splitNativeChatBlocks } from '../../../src/shared/native-chat-tool-fold' import { selectActiveToolCall } from '../../../src/shared/native-chat-tool-activity' import { isImageRefBlock, isTextBlock } from '../../../src/shared/native-chat-types' @@ -10,10 +9,8 @@ import { MobileMarkdown } from '../components/MobileMarkdown' import { MobileNativeChatTurnStatus } from './MobileNativeChatTurnStatus' import { ToolRun } from './MobileNativeChatToolRun' import type { NativeChatTurnStatus } from './use-mobile-native-chat-turn-status' -import { colors } from '../theme/mobile-theme' import { isRenderableImageUri } from './mobile-native-chat-image-preview' import { styles, TEXT_SIZE } from './mobile-native-chat-message-styles' -import { nativeChatMessageText } from './mobile-native-chat-message-text' function Prose({ block, @@ -37,7 +34,12 @@ function Prose({ ) } return ( - + ) } if (isImageRefBlock(block)) { @@ -55,53 +57,18 @@ function Prose({ ) } return ( - + 🖼 {block.alt ?? block.path ?? block.url ?? 'image'} - + ) } return null } -/** Subtle top-right controls for an agent message: copy its prose, or scroll so - * this message's top aligns to the top of the viewport. */ -function AgentControls({ - onCopy, - onScrollToTop -}: { - onCopy: () => void - onScrollToTop?: () => void -}): React.JSX.Element { - return ( - - [styles.controlButton, pressed && styles.controlPressed]} - onPress={onCopy} - hitSlop={8} - accessibilityLabel="Copy message" - > - - - {onScrollToTop ? ( - [styles.controlButton, pressed && styles.controlPressed]} - onPress={onScrollToTop} - hitSlop={8} - accessibilityLabel="Scroll this message to top" - > - - - ) : null} - - ) -} - function MobileNativeChatMessageImpl({ message, toolsExpanded = false, fontScale = 1, - messageIndex, - onScrollToMessage, onOpenFile, turnStatus, turnExpanded, @@ -114,10 +81,6 @@ function MobileNativeChatMessageImpl({ toolsExpanded?: boolean /** Multiplies all chat text sizes for pinch-to-zoom (1 = no change). */ fontScale?: number - /** This message's index in the list, paired with onScrollToMessage. */ - messageIndex?: number - /** Ask the list to align this message's top to the top of the viewport. */ - onScrollToMessage?: (index: number) => void onOpenFile?: (relativePath: string) => void /** This turn's status row, rendered under a user message (desktop parity). */ turnStatus?: NativeChatTurnStatus | null @@ -134,18 +97,6 @@ function MobileNativeChatMessageImpl({ }): React.JSX.Element { const isUser = message.role === 'user' const isReasoning = message.role === 'reasoning' - const isAgent = !isUser - // Briefly tint the bubble to confirm a copy landed. - const [copied, setCopied] = useState(false) - const copyTimer = useRef | null>(null) - useEffect( - () => () => { - if (copyTimer.current) { - clearTimeout(copyTimer.current) - } - }, - [] - ) // Separate the agent's words from its tool activity: prose renders first, the // tool calls fold into a collapsible run beneath. The user's own messages get // an inverted (filled accent) bubble so they stand apart from agent prose. @@ -165,42 +116,11 @@ function MobileNativeChatMessageImpl({ !toolsExpanded const showToolRun = tools.length > 0 && !settledToolsHidden - const handleCopy = (): void => { - const text = nativeChatMessageText(message.blocks) - if (!text) { - return - } - void Clipboard.setStringAsync(text) - setCopied(true) - if (copyTimer.current) { - clearTimeout(copyTimer.current) - } - copyTimer.current = setTimeout(() => setCopied(false), 700) - } - - // Copy + scroll-to-top, shown inline with the first tool call (or after the - // prose when there are no tools). - const controls = isAgent ? ( - onScrollToMessage(messageIndex) - : undefined - } - /> - ) : null - return ( <> {prose.map((block, index) => ( - ) : controls ? ( - {controls} ) : null} diff --git a/mobile/src/session/MobileNativeChatToolRun.tsx b/mobile/src/session/MobileNativeChatToolRun.tsx index ccc732dab88a..4dad714957ca 100644 --- a/mobile/src/session/MobileNativeChatToolRun.tsx +++ b/mobile/src/session/MobileNativeChatToolRun.tsx @@ -172,7 +172,6 @@ export function ToolRun({ defaultExpanded, expandChildren, activeCall, - trailing, onOpenFile }: { blocks: NativeChatBlock[] @@ -181,7 +180,6 @@ export function ToolRun({ expandChildren: boolean /** The still-running call, when the turn is live (desktop parity). */ activeCall: ReturnType - trailing?: React.ReactNode onOpenFile?: (relativePath: string) => void }): React.JSX.Element { const [open, setOpen] = useState(defaultExpanded) @@ -230,7 +228,6 @@ export function ToolRun({ )} - {trailing} {open ? ( diff --git a/mobile/src/session/MobileNativeChatView.tsx b/mobile/src/session/MobileNativeChatView.tsx index f58e2ad05b0f..b197faa97919 100644 --- a/mobile/src/session/MobileNativeChatView.tsx +++ b/mobile/src/session/MobileNativeChatView.tsx @@ -253,11 +253,6 @@ export function MobileNativeChatView({ [hasMore, loadingEarlier, onLoadEarlier] ) - // Align a single message's top to the top of the viewport. - const onScrollToMessage = useCallback((index: number) => { - listRef.current?.scrollToIndex({ index, viewPosition: 0, animated: true }) - }, []) - // Per-turn "Thinking / Working for N / Worked for N" rows. The structured lane // owns them; the bridge lane keeps its three-dot indicator. const turns = useMobileNativeChatTurnDisclosure({ @@ -273,15 +268,13 @@ export function MobileNativeChatView({ message={item} toolsExpanded={toolsExpanded} fontScale={fontScale} - messageIndex={index} - onScrollToMessage={onScrollToMessage} onOpenFile={onOpenFile} structuredActivityUi={structuredActivityUi} onToggleTurn={turns.onToggleTurn} {...turns.resolveRow(index, item)} /> ), - [toolsExpanded, fontScale, onScrollToMessage, onOpenFile, structuredActivityUi, turns] + [toolsExpanded, fontScale, onOpenFile, structuredActivityUi, turns] ) const emptyState = mobileNativeChatEmptyState(status, agent ?? null, error) @@ -325,21 +318,6 @@ export function MobileNativeChatView({ listRef.current?.scrollToEnd({ animated: false }) } }} - // scrollToIndex can fail before an off-screen row is measured — - // fall back to an estimated offset, then retry once it's laid out. - onScrollToIndexFailed={(info) => { - listRef.current?.scrollToOffset({ - offset: info.averageItemLength * info.index, - animated: true - }) - setTimeout(() => { - listRef.current?.scrollToIndex({ - index: info.index, - viewPosition: 0, - animated: true - }) - }, 120) - }} ListHeaderComponent={ hasMore ? ( - {/* Jump-to-latest control. The scroll-to-top affordance now lives - per-message (the up-arrow in each agent message's controls). */} + {/* Jump-to-latest control. */} {!atBottom ? ( { - it('joins text blocks and skips non-text blocks', () => { - const blocks: NativeChatBlock[] = [ - { type: 'text', text: 'Hello' }, - { type: 'tool-call', name: 'Read', input: {} }, - { type: 'text', text: 'World' } - ] - expect(nativeChatMessageText(blocks)).toBe('Hello\n\nWorld') - }) - - it('returns an empty string when there is no prose', () => { - const blocks: NativeChatBlock[] = [{ type: 'tool-call', name: 'Read', input: {} }] - expect(nativeChatMessageText(blocks)).toBe('') - }) - - it('trims surrounding whitespace', () => { - expect(nativeChatMessageText([{ type: 'text', text: ' hi ' }])).toBe('hi') - }) -}) +import { clampFontScale, FONT_SCALE_MAX, FONT_SCALE_MIN } from './mobile-native-chat-message-text' describe('clampFontScale', () => { it('clamps below the minimum', () => { diff --git a/mobile/src/session/mobile-native-chat-message-text.ts b/mobile/src/session/mobile-native-chat-message-text.ts index a94104835a9d..84d2f8f1cc9f 100644 --- a/mobile/src/session/mobile-native-chat-message-text.ts +++ b/mobile/src/session/mobile-native-chat-message-text.ts @@ -1,15 +1,3 @@ -import { isTextBlock, type NativeChatBlock } from '../../../src/shared/native-chat-types' - -/** Concatenate a message's text blocks into a single copyable string. Tool - * calls/results and image refs are skipped — Copy is for the agent's prose. */ -export function nativeChatMessageText(blocks: readonly NativeChatBlock[]): string { - return blocks - .filter(isTextBlock) - .map((b) => b.text) - .join('\n\n') - .trim() -} - /** Pinch-to-zoom font bounds. Default 1 means no visible change until pinched. */ export const FONT_SCALE_MIN = 0.8 export const FONT_SCALE_MAX = 1.8 From a067cccd38e38487772741ebf9d0ea4a8fed8644 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Wed, 9 Sep 2026 22:06:31 -0700 Subject: [PATCH 004/261] perf(terminal): resume OSC terminator search past the carried frame (#19839) A single unterminated OSC 9999 marker split across many PTY chunks re-scanned the whole accumulation for a terminator on every chunk, so work grew with the square of the frame length. Carry how much of `pending` already failed the search and resume one character before it, which is enough for an `ESC \\` straddling the chunk boundary. --- ...status-osc-split-frame-scan-budget.test.ts | 75 +++++++++++++++++++ src/shared/agent-status-osc.ts | 11 ++- 2 files changed, 85 insertions(+), 1 deletion(-) create mode 100644 src/shared/agent-status-osc-split-frame-scan-budget.test.ts diff --git a/src/shared/agent-status-osc-split-frame-scan-budget.test.ts b/src/shared/agent-status-osc-split-frame-scan-budget.test.ts new file mode 100644 index 000000000000..fda2e9b8f3ec --- /dev/null +++ b/src/shared/agent-status-osc-split-frame-scan-budget.test.ts @@ -0,0 +1,75 @@ +import { describe, expect, it, vi } from 'vitest' +import { createAgentStatusOscProcessor } from './agent-status-osc' + +/** Total characters swept by terminator/prefix searches across every chunk of a feed. */ +function feedWithScanBudget(chunks: string[]) { + let searchedChars = 0 + const indexOf = String.prototype.indexOf + const spy = vi.spyOn(String.prototype, 'indexOf').mockImplementation(function ( + this: string, + search, + from = 0 + ) { + const found = indexOf.call(this, search, from) + searchedChars += (found === -1 ? this.length : found + String(search).length) - Number(from) + return found + }) + try { + const process = createAgentStatusOscProcessor() + const results = chunks.map((chunk) => process(chunk)) + return { results, searchedChars } + } finally { + spy.mockRestore() + } +} + +describe('OSC 9999 split-frame scan budget', () => { + it('keeps per-chunk work flat as the split frame accumulates', () => { + // One unterminated marker whose payload arrives one character at a time. + const feedOf = (chunkCount: number): string[] => [ + '\x1b]9999;{"state":"working","prompt":"', + ...Array(chunkCount).fill('x') + ] + + const small = feedWithScanBudget(feedOf(2000)) + const large = feedWithScanBudget(feedOf(4000)) + + expect(small.results.every((result) => result.payloads.length === 0)).toBe(true) + // Re-scanning the accumulation would quadruple the budget when the feed doubles. + expect(large.searchedChars).toBeLessThan(small.searchedChars * 3) + }) + + it.each(['\x07', '\x1b\\'])( + 'matches whole-string parsing when split at every offset with terminator %j', + (terminator) => { + const stream = `head\x1b]9999;{"state":"working","prompt":"p"}${terminator}tail` + const whole = createAgentStatusOscProcessor()(stream) + + for (let split = 1; split < stream.length; split += 1) { + const process = createAgentStatusOscProcessor() + const first = process(stream.slice(0, split)) + const second = process(stream.slice(split)) + expect({ + cleanData: first.cleanData + second.cleanData, + payloads: [...first.payloads, ...second.payloads] + }).toEqual({ cleanData: whole.cleanData, payloads: whole.payloads }) + } + } + ) + + it('finds a string terminator straddling the resume boundary', () => { + const process = createAgentStatusOscProcessor() + // The ESC lands as the last character of the carried frame; the backslash arrives next. + expect(process('\x1b]9999;{"state":"working"}\x1b').payloads).toEqual([]) + expect(process('\\rest').payloads).toMatchObject([{ state: 'working' }]) + }) + + it('still parses a payload that completes many chunks later', () => { + const process = createAgentStatusOscProcessor() + process('\x1b]9999;{"state":"wor') + for (const chunk of ['k', 'i', 'n', 'g']) { + expect(process(chunk).payloads).toEqual([]) + } + expect(process('"}\x07done').payloads).toMatchObject([{ state: 'working' }]) + }) +}) diff --git a/src/shared/agent-status-osc.ts b/src/shared/agent-status-osc.ts index adb0c0eee47a..3b13d2de5bc3 100644 --- a/src/shared/agent-status-osc.ts +++ b/src/shared/agent-status-osc.ts @@ -57,6 +57,9 @@ function findAgentStatusTerminator( export function createAgentStatusOscProcessor(): (data: string) => ProcessedAgentStatusChunk { const MAX_PENDING = 64 * 1024 let pending = '' + // How much of `pending` already failed a terminator search, so a frame split across + // many chunks re-scans only the new bytes instead of the whole accumulation. + let pendingSearched = 0 return (data: string): ProcessedAgentStatusChunk => { // Ordinary terminal output is by far the common case. Keep it on the @@ -76,7 +79,9 @@ export function createAgentStatusOscProcessor(): (data: string) => ProcessedAgen } const combined = pending + data + const resumeFrom = pendingSearched pending = '' + pendingSearched = 0 const payloads: ParsedAgentStatusPayload[] = [] let lastPayloadCleanOffset: number | null = null @@ -100,12 +105,16 @@ export function createAgentStatusOscProcessor(): (data: string) => ProcessedAgen cleanData += combined.slice(cursor, start) const payloadStart = start + OSC_AGENT_STATUS_PREFIX.length - const terminator = findAgentStatusTerminator(combined, payloadStart, nextTerminator) + // Minus one so a `\x1b\\` straddling the previous chunk boundary is still found. + const searchFrom = + start === 0 && resumeFrom > 0 ? Math.max(payloadStart, resumeFrom - 1) : payloadStart + const terminator = findAgentStatusTerminator(combined, searchFrom, nextTerminator) if (terminator === null) { const candidate = combined.slice(start) // Own the frame so it stops pinning the consumed chunk it was sliced from. pending = candidate.length > MAX_PENDING ? '' : ownRetainedString(candidate) + pendingSearched = pending.length break } From 0b60b0dcb1215b7005a7320e6fbe45304f3abbfd Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Wed, 9 Sep 2026 22:12:52 -0700 Subject: [PATCH 005/261] perf(native-chat): bound retained items on the structured session path (#19841) `mergeSubmissions` caps submissions at 256, but `mergeItems` had no equivalent bound, so `state.items` grew for the whole life of a long structured session while the live path caps itself to its read window. Head-trim `items` to a retained-item limit when a live batch merges, and set `hasOlder` so anything trimmed is still reachable by paging. Paging older raises the limit to what the page produced, so a live batch slides the widened window instead of collapsing it back to the cap -- the same shape as the live path's growing `limitRef`. --- ...tured-agent-session-item-retention.test.ts | 152 ++++++++++++++++++ .../structured-agent-session-reducer.ts | 33 +++- 2 files changed, 179 insertions(+), 6 deletions(-) create mode 100644 src/shared/structured-agent-session-item-retention.test.ts diff --git a/src/shared/structured-agent-session-item-retention.test.ts b/src/shared/structured-agent-session-item-retention.test.ts new file mode 100644 index 000000000000..fe65b8825965 --- /dev/null +++ b/src/shared/structured-agent-session-item-retention.test.ts @@ -0,0 +1,152 @@ +import { describe, expect, it } from 'vitest' +import type { AgentJournalRenderItem } from './agent-session-journal-types' +import type { AgentSessionHistoryPage } from './agent-session-wire' +import { + EMPTY_STRUCTURED_AGENT_SESSION, + oldestStructuredAgentSessionCursor, + reduceStructuredAgentSession, + type StructuredAgentSessionState +} from './structured-agent-session-reducer' + +const CAP = 1024 + +function item(sequence: number): AgentJournalRenderItem { + return { + itemId: `item-${sequence}`, + revision: 1, + sequence, + observedAt: sequence, + body: { kind: 'message', role: 'assistant', blocks: [{ type: 'text', text: `t-${sequence}` }] } + } +} + +function page(items: AgentJournalRenderItem[], hasOlder: boolean): AgentSessionHistoryPage { + const oldest = items[0]?.sequence ?? 0 + const newest = items.at(-1)?.sequence ?? 0 + return { + sessionId: 'session-a', + epoch: 'epoch-a', + direction: 'tail', + items, + removedItemIds: [], + submissions: [], + window: { + oldest: { epoch: 'epoch-a', sequence: oldest }, + newest: { epoch: 'epoch-a', sequence: newest }, + nextCursor: { epoch: 'epoch-a', sequence: oldest } + }, + liveCursor: { epoch: 'epoch-a', sequence: newest }, + hasOlder, + hasNewer: false + } +} + +function hydrate(items: AgentJournalRenderItem[], hasOlder = false): StructuredAgentSessionState { + return reduceStructuredAgentSession(EMPTY_STRUCTURED_AGENT_SESSION, { + type: 'event', + event: { type: 'snapshot', sessionId: 'session-a', fence: 1, page: page(items, hasOlder) } + }) +} + +function streamItems( + state: StructuredAgentSessionState, + sequences: number[] +): StructuredAgentSessionState { + return sequences.reduce( + (current, sequence) => + reduceStructuredAgentSession(current, { + type: 'event', + event: { + type: 'batch', + sessionId: 'session-a', + batch: { + cursor: { epoch: 'epoch-a', sequence }, + items: [item(sequence)], + removedItemIds: [], + submissions: [] + } + } + }), + state + ) +} + +describe('structured agent session item retention', () => { + it('bounds retained items on a long live session', () => { + const streamed = streamItems( + hydrate([item(0)]), + Array.from({ length: CAP + 500 }, (_, index) => index + 1) + ) + + expect(streamed.items).toHaveLength(CAP) + expect(streamed.items.at(-1)?.sequence).toBe(CAP + 500) + expect(streamed.items[0]?.sequence).toBe(501) + }) + + it('offers paging for items the cap dropped', () => { + const streamed = streamItems( + hydrate([item(0)]), + Array.from({ length: CAP + 10 }, (_, index) => index + 1) + ) + + expect(streamed.hasOlder).toBe(true) + expect(oldestStructuredAgentSessionCursor(streamed)).toEqual({ + epoch: 'epoch-a', + sequence: streamed.items[0]?.sequence + }) + }) + + it('leaves a session under the cap untouched', () => { + const hydrated = hydrate([item(0)]) + const streamed = streamItems( + hydrated, + Array.from({ length: 200 }, (_, index) => index + 1) + ) + + expect(streamed.items).toHaveLength(201) + expect(streamed.hasOlder).toBe(false) + }) + + it('widens the retained window when older items are paged in', () => { + const streamed = streamItems( + hydrate([item(1_000)], true), + Array.from({ length: CAP + 10 }, (_, index) => index + 1_001) + ) + const older = reduceStructuredAgentSession(streamed, { + type: 'older-page', + requestedEpoch: 'epoch-a', + page: page( + Array.from({ length: 300 }, (_, index) => item(index + 700)), + true + ) + }) + expect(older.items).toHaveLength(CAP + 300) + + // A live batch slides the widened window by one instead of collapsing it back to the cap. + const afterLive = streamItems(older, [3_000]) + + expect(afterLive.items).toHaveLength(CAP + 300) + expect(afterLive.items[0]?.sequence).toBe(701) + expect(afterLive.items.some((entry) => entry.sequence === 800)).toBe(true) + }) + + it('keeps item identity stable when a batch carries no journal change', () => { + const hydrated = hydrate([item(0)]) + const unchanged = reduceStructuredAgentSession(hydrated, { + type: 'event', + event: { + type: 'batch', + sessionId: 'session-a', + fence: 2, + batch: { + cursor: { epoch: 'epoch-a', sequence: 0 }, + items: [], + removedItemIds: [], + submissions: [] + } + } + }) + + expect(unchanged.items).toBe(hydrated.items) + }) +}) diff --git a/src/shared/structured-agent-session-reducer.ts b/src/shared/structured-agent-session-reducer.ts index 10897c3f61fd..6397ca0c8c41 100644 --- a/src/shared/structured-agent-session-reducer.ts +++ b/src/shared/structured-agent-session-reducer.ts @@ -18,6 +18,8 @@ export type StructuredAgentSessionState = { fence: number | null items: AgentJournalRenderItem[] submissions: AgentJournalSubmission[] + /** Head-trim floor for `items`; paging back raises it so a live batch cannot undo the page. */ + retainedItemLimit: number hasOlder: boolean status: 'idle' | 'loading' | 'ready' | 'error' error?: string @@ -35,19 +37,23 @@ export type StructuredAgentSessionAction = | { type: 'tail-page'; page: AgentSessionHistoryPage } | { type: 'older-page'; requestedEpoch: string; page: AgentSessionHistoryPage } +const MAX_RETAINED_SUBMISSIONS = 256 +// Well above the renderer's initial read window (300) plus a page, so only genuinely +// long live sessions trim; anything trimmed is still reachable by paging older. +const MAX_RETAINED_ITEMS = 1024 + export const EMPTY_STRUCTURED_AGENT_SESSION: StructuredAgentSessionState = { epoch: null, cursor: null, fence: null, items: [], submissions: [], + retainedItemLimit: MAX_RETAINED_ITEMS, hasOlder: false, status: 'idle', handoff: null } -const MAX_RETAINED_SUBMISSIONS = 256 - function backgroundTaskStatesEqual( left: AgentSessionBackgroundTaskState | null | undefined, right: AgentSessionBackgroundTaskState | null | undefined @@ -91,6 +97,7 @@ function replacePage( fence, items: [...page.items].sort((left, right) => left.sequence - right.sequence), submissions: page.submissions, + retainedItemLimit: Math.max(MAX_RETAINED_ITEMS, page.items.length), hasOlder: page.hasOlder, status: 'ready', handoff: handoff ?? null, @@ -121,6 +128,13 @@ function mergeItems( return [...byId.values()].sort((left, right) => left.sequence - right.sequence) } +function trimRetainedItems( + items: AgentJournalRenderItem[], + limit: number +): AgentJournalRenderItem[] { + return items.length <= limit ? items : items.slice(items.length - limit) +} + function mergeSubmissions( current: readonly AgentJournalSubmission[], incoming: readonly AgentJournalSubmission[] @@ -186,6 +200,7 @@ export function reduceStructuredAgentSession( submissions: sameEpoch ? mergeSubmissions(state.submissions, action.page.submissions) : action.page.submissions, + retainedItemLimit: Math.max(MAX_RETAINED_ITEMS, action.page.items.length), hasOlder: action.page.hasOlder, status: 'ready', handoff: state.handoff, @@ -202,9 +217,11 @@ export function reduceStructuredAgentSession( if (state.epoch !== action.requestedEpoch || action.page.epoch !== action.requestedEpoch) { return state } + const paged = mergeItems(state.items, action.page.items, action.page.removedItemIds) return { ...state, - items: mergeItems(state.items, action.page.items, action.page.removedItemIds), + items: paged, + retainedItemLimit: Math.max(state.retainedItemLimit, paged.length), submissions: mergeSubmissions(state.submissions, action.page.submissions), hasOlder: action.page.hasOlder } @@ -246,13 +263,17 @@ export function reduceStructuredAgentSession( ) { return state } + const merged = journalUnchanged + ? state.items + : mergeItems(state.items, event.batch.items, event.batch.removedItemIds) + const items = trimRetainedItems(merged, state.retainedItemLimit) return { ...state, cursor: event.batch.cursor, fence: event.fence ?? state.fence, - items: journalUnchanged - ? state.items - : mergeItems(state.items, event.batch.items, event.batch.removedItemIds), + items, + // A trim leaves older items behind the cursor, so paging must stay offered. + hasOlder: items.length < merged.length ? true : state.hasOlder, submissions: journalUnchanged ? state.submissions : mergeSubmissions(state.submissions, event.batch.submissions), From 34790ce0843affb73bb6340c9189b5b77e04a06b Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Wed, 9 Sep 2026 22:14:56 -0700 Subject: [PATCH 006/261] perf(terminal): share one ESC dispatch table between the two scanners (#19842) The partial-tail state machine and the preview normalizer's per-sequence parser each carried their own copy of the byte-after-ESC table, so the DCS/SOS/PM/APC set was stated twice and could drift. Move it to `terminal-escape-introducer.ts` and have both read it. The normalizer now classifies from `charCodeAt` instead of `value[i]`, which drops the one-char string it minted on every escape it parsed -- the same allocation the ground scan already avoids. No behaviour change: `parseAnsiControlSequence` matches its previous implementation on every 2- and 3-byte sequence after ESC and on 20k random control-dense streams (differential check, not committed). --- .../runtime/terminal-ansi-normalization.ts | 14 +++--- src/shared/terminal-escape-introducer.test.ts | 46 +++++++++++++++++++ src/shared/terminal-escape-introducer.ts | 32 +++++++++++++ src/shared/terminal-partial-escape-tail.ts | 31 ++++++------- 4 files changed, 99 insertions(+), 24 deletions(-) create mode 100644 src/shared/terminal-escape-introducer.test.ts create mode 100644 src/shared/terminal-escape-introducer.ts diff --git a/src/main/runtime/terminal-ansi-normalization.ts b/src/main/runtime/terminal-ansi-normalization.ts index df76da3c35e0..e1bbe33a79d9 100644 --- a/src/main/runtime/terminal-ansi-normalization.ts +++ b/src/main/runtime/terminal-ansi-normalization.ts @@ -1,4 +1,5 @@ import { MAX_TAIL_PENDING_ANSI_CHARS } from './terminal-tail-limits' +import { classifyTerminalEscapeIntroducer } from '../../shared/terminal-escape-introducer' import { ownRetainedString } from '../../shared/own-retained-string' export function parseAnsiControlSequence( @@ -11,8 +12,9 @@ export function parseAnsiControlSequence( endIndex: number } | null { - const introducer = value[escapeIndex + 1] - if (introducer === '[') { + // charCodeAt, not value[i]: indexing mints a one-char string on every escape. + const introducer = classifyTerminalEscapeIntroducer(value.charCodeAt(escapeIndex + 1)) + if (introducer === 'csi') { for (let index = escapeIndex + 2; index < value.length; index += 1) { const code = value.charCodeAt(index) if (code < 0x40 || code > 0x7e) { @@ -30,7 +32,7 @@ export function parseAnsiControlSequence( } return null } - if (introducer === ']') { + if (introducer === 'osc') { for (let index = escapeIndex + 2; index < value.length; index += 1) { if (value[index] === '\u0007') { return { kind: 'other', endIndex: index } @@ -41,7 +43,7 @@ export function parseAnsiControlSequence( } return null } - if (isStTerminatedStringControlIntroducer(introducer)) { + if (introducer === 'string') { for (let index = escapeIndex + 2; index < value.length; index += 1) { if (value[index] === '\u001b' && value[index + 1] === '\\') { return { kind: 'other', endIndex: index + 1 } @@ -52,10 +54,6 @@ export function parseAnsiControlSequence( return { kind: 'other', endIndex: escapeIndex + 1 } } -function isStTerminatedStringControlIntroducer(introducer: string | undefined): boolean { - return introducer === 'P' || introducer === 'X' || introducer === '^' || introducer === '_' -} - export function hasCanonicalNumericCsiParams(params: string): boolean { return /^[0-9;]*$/.test(params) } diff --git a/src/shared/terminal-escape-introducer.test.ts b/src/shared/terminal-escape-introducer.test.ts new file mode 100644 index 000000000000..2fa3bfc662ef --- /dev/null +++ b/src/shared/terminal-escape-introducer.test.ts @@ -0,0 +1,46 @@ +import { describe, expect, it } from 'vitest' +import { + classifyTerminalEscapeIntroducer, + type TerminalEscapeIntroducer +} from './terminal-escape-introducer' + +/** Independent restatement of the VT500 dispatch table, written from the ranges rather + * than the implementation, so a reordered branch in the real one shows up here. */ +function expected(code: number): TerminalEscapeIntroducer { + if ('[' === String.fromCharCode(code)) { + return 'csi' + } + if (']' === String.fromCharCode(code)) { + return 'osc' + } + if ('PX^_'.includes(String.fromCharCode(code))) { + return 'string' + } + if (String.fromCharCode(code) >= ' ' && String.fromCharCode(code) <= '/') { + return 'intermediate' + } + if (code < 0x20 || code === 0x7f) { + return 'execute' + } + return 'final' +} + +describe('terminal escape introducer', () => { + it('classifies every single-byte introducer the way the VT500 table does', () => { + for (let code = 0; code <= 0xff; code += 1) { + expect([code, classifyTerminalEscapeIntroducer(code)]).toEqual([code, expected(code)]) + } + }) + + it('opens ST-terminated strings for DCS, SOS, PM and APC only', () => { + const strings = Array.from({ length: 0x100 }, (_, code) => code).filter( + (code) => classifyTerminalEscapeIntroducer(code) === 'string' + ) + expect(strings.map((code) => String.fromCharCode(code))).toEqual(['P', 'X', '^', '_']) + }) + + it('reads a missing byte (ESC at end of input) as a completed two-byte sequence', () => { + // `charCodeAt` past the end is NaN; callers rely on that not becoming csi/osc/string. + expect(classifyTerminalEscapeIntroducer(''.charCodeAt(0))).toBe('final') + }) +}) diff --git a/src/shared/terminal-escape-introducer.ts b/src/shared/terminal-escape-introducer.ts new file mode 100644 index 000000000000..850300c91a14 --- /dev/null +++ b/src/shared/terminal-escape-introducer.ts @@ -0,0 +1,32 @@ +// The byte after ESC decides which VT500 sequence just opened. Two scanners need that +// decision -- the partial-tail state machine (`terminal-partial-escape-tail.ts`) and the +// preview normalizer's per-sequence parser (`terminal-ansi-normalization.ts`) -- and they +// must agree on the DCS/SOS/PM/APC set, so the table lives here rather than in each. + +export type TerminalEscapeIntroducer = + | 'csi' // [ + | 'osc' // ] + | 'string' // P / X / ^ / _ open DCS / SOS / PM / APC -- ST-terminated + | 'intermediate' // 0x20-0x2f, more bytes to come before the final + | 'execute' // C0 executes / DEL is ignored; the ESC is still pending + | 'final' // completes a two-byte sequence (ESC 7, ESC 8, ESC c, ...) + +/** Classifies the code unit after ESC. `NaN` (ESC at end of input) reads as `final`. */ +export function classifyTerminalEscapeIntroducer(code: number): TerminalEscapeIntroducer { + if (code === 0x5b) { + return 'csi' + } + if (code === 0x5d) { + return 'osc' + } + if (code === 0x50 || code === 0x58 || code === 0x5e || code === 0x5f) { + return 'string' + } + if (code >= 0x20 && code <= 0x2f) { + return 'intermediate' + } + if (code < 0x20 || code === 0x7f) { + return 'execute' + } + return 'final' +} diff --git a/src/shared/terminal-partial-escape-tail.ts b/src/shared/terminal-partial-escape-tail.ts index 1a5e2dfc6131..53cfd3c17b4c 100644 --- a/src/shared/terminal-partial-escape-tail.ts +++ b/src/shared/terminal-partial-escape-tail.ts @@ -6,6 +6,8 @@ // partial sequence at the ingest boundary lets snapshot producers append it // after the serialized screen so the continuation completes exactly as live. +import { classifyTerminalEscapeIntroducer } from './terminal-escape-introducer' + // Mirrors the VT500 parser states that can span a chunk boundary. C0 controls // (except ESC/CAN/SUB) execute mid-sequence without aborting it, matching // xterm's state machine. @@ -32,23 +34,20 @@ export const MAX_PARTIAL_ESCAPE_TAIL_LENGTH = 4096 /** ESC-state transition shared by the fresh-ESC and abort-reprocess paths. */ function stateAfterEscByte(code: number): ScanState { - if (code === 0x5b) { - return 'csi' // [ - } - if (code === 0x5d) { - return 'osc' // ] - } - // P / X / ^ / _ open DCS / SOS / PM / APC — ST-terminated strings. - if (code === 0x50 || code === 0x58 || code === 0x5e || code === 0x5f) { - return 'string' - } - if (code >= 0x20 && code <= 0x2f) { - return 'escIntermediate' - } - if (code < 0x20 || code === 0x7f) { - return 'esc' // C0 executes / DEL is ignored mid-sequence; ESC via callers + switch (classifyTerminalEscapeIntroducer(code)) { + case 'csi': + return 'csi' + case 'osc': + return 'osc' + case 'string': + return 'string' + case 'intermediate': + return 'escIntermediate' + case 'execute': + return 'esc' // the ESC is still pending; a further ESC arrives via the callers + case 'final': + return 'ground' } - return 'ground' // final byte — two-byte sequence (ESC 7, ESC 8, ESC c, …) } /** Returns the trailing incomplete escape sequence of `stream` ('' when the From dda103d2cf69dbdd730d0a7179bb3021b6467501 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 9 Sep 2026 23:21:57 -0700 Subject: [PATCH 007/261] fix(native-chat): one `/` picker for every agent, anywhere in the prompt (#19832) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(native-chat): one `/` picker for every agent, anywhere in the prompt The composer only opened its picker when `/` was the first character of the draft, so a skill named mid-sentence ("validate it with /electron") offered nothing. Codex was worse: its `/` menu listed commands only, and skills lived on a separate `$` trigger, so the prompt box behaved differently per agent. `/` is now the whole composer grammar. It opens one grouped commands+skills menu for every agent with a known grammar, both at the start of the draft and mid-prompt after whitespace. The `$` trigger is gone. Per-agent invocation is preserved where it belongs — in what a pick writes. Each row carries its own token, so choosing a skill in Codex inserts `$electron` while Claude inserts `/electron`, and the text that reaches the agent stays the text that agent actually invokes. Only a draft-leading command is dispatchable; picking one mid-sentence completes the token instead of sending the command on its own and discarding the draft. Name collisions now key on whether both kinds share a sigil, so a Codex `/review` command and a `$review` skill stay separate rows. * test(native-chat): model dismissal inputs on the live `/` grammar The trigger-key swap cases still used `$:4` keys. editReplacesTriggerToken is sigil-agnostic so they passed, but they modelled an input the composer can no longer produce. * test(native-chat): pin inline picker dispatch and discovery reuse --------- Co-authored-by: Merge Sim --- .../NativeChatAutocompleteMenus.test.tsx | 3 + .../NativeChatAutocompleteMenus.tsx | 12 +- .../native-chat/NativeChatComposerField.tsx | 13 +- .../native-chat-composer-state.test.ts | 201 ++++++++++++++---- .../native-chat/native-chat-composer-state.ts | 86 ++++---- .../native-chat/native-chat-picker-items.ts | 54 +++-- .../use-native-chat-composer-catalog.test.tsx | 1 + .../use-native-chat-composer-keydown.test.tsx | 44 +++- .../use-native-chat-composer-keydown.ts | 6 +- ...tive-chat-picker-command-dispatch.test.tsx | 1 + .../use-native-chat-picker-state.ts | 20 +- .../use-native-chat-skills.react.test.tsx | 24 +++ src/shared/native-chat-agent-profiles.test.ts | 9 +- src/shared/native-chat-agent-profiles.ts | 5 - 14 files changed, 342 insertions(+), 137 deletions(-) diff --git a/src/renderer/src/components/native-chat/NativeChatAutocompleteMenus.test.tsx b/src/renderer/src/components/native-chat/NativeChatAutocompleteMenus.test.tsx index d5dac1efab57..88506c51838a 100644 --- a/src/renderer/src/components/native-chat/NativeChatAutocompleteMenus.test.tsx +++ b/src/renderer/src/components/native-chat/NativeChatAutocompleteMenus.test.tsx @@ -13,6 +13,7 @@ function autocomplete( query: '', triggerKey: '/:0', prefix: '/', + dispatchable: true, grouped: true, commandsEnabled: true, skillsEnabled: true, @@ -21,6 +22,7 @@ function autocomplete( kind: 'command', id: 'command:clear', name: 'clear', + token: '/clear', description: 'Clear history', skillCollision: false }, @@ -28,6 +30,7 @@ function autocomplete( kind: 'skill', id: 'skill:browser', name: 'browser', + token: '/browser', description: 'Use a browser', sources: [{ sourceKind: 'repo', skillFilePath: '/repo/browser/SKILL.md' }] } diff --git a/src/renderer/src/components/native-chat/NativeChatAutocompleteMenus.tsx b/src/renderer/src/components/native-chat/NativeChatAutocompleteMenus.tsx index 8357931fb499..7f1cd943c0f2 100644 --- a/src/renderer/src/components/native-chat/NativeChatAutocompleteMenus.tsx +++ b/src/renderer/src/components/native-chat/NativeChatAutocompleteMenus.tsx @@ -12,7 +12,7 @@ export const NativeChatPickerMenu = memo(function NativeChatPickerMenu({ onChoose, onRetry }: { - autocomplete: Extract + autocomplete: Extract activeIndex: number listboxId: string onChoose: (item: NativeChatPickerItem) => void @@ -54,7 +54,6 @@ export const NativeChatPickerMenu = memo(function NativeChatPickerMenu({ + autocomplete: Extract ): string { - if (autocomplete.mode === 'skill' || !autocomplete.commandsEnabled) { + if (!autocomplete.commandsEnabled) { return translate('components.native-chat.composer.noSkills', 'No matching skills') } if (autocomplete.skillsEnabled) { @@ -174,7 +172,6 @@ function PickerStatus({ children }: { children: React.ReactNode }): React.JSX.El function PickerOption({ item, - prefix, index, activeIndex, listboxId, @@ -182,7 +179,6 @@ function PickerOption({ onChoose }: { item: NativeChatPickerItem - prefix: '/' | '$' index: number activeIndex: number listboxId: string @@ -213,7 +209,7 @@ function PickerOption({ ) : null} - {prefix + item.name} + {item.token} {item.description ? ( {item.description} ) : null} diff --git a/src/renderer/src/components/native-chat/NativeChatComposerField.tsx b/src/renderer/src/components/native-chat/NativeChatComposerField.tsx index 6b474a2f2677..f51bf4c5400e 100644 --- a/src/renderer/src/components/native-chat/NativeChatComposerField.tsx +++ b/src/renderer/src/components/native-chat/NativeChatComposerField.tsx @@ -166,7 +166,7 @@ export function NativeChatComposerField({ {/* Extra bottom padding keeps the input box off the window rim. */}
- {autocomplete.mode === 'slash' || autocomplete.mode === 'skill' ? ( + {autocomplete.mode === 'slash' ? ( 0 + autocomplete.mode === 'slash' && autocomplete.items.length > 0 ? `${pickerListboxId}-option-${Math.min(activeSuggestion, autocomplete.items.length - 1)}` : undefined } diff --git a/src/renderer/src/components/native-chat/native-chat-composer-state.test.ts b/src/renderer/src/components/native-chat/native-chat-composer-state.test.ts index 483d29f1ff21..9d116d373fba 100644 --- a/src/renderer/src/components/native-chat/native-chat-composer-state.test.ts +++ b/src/renderer/src/components/native-chat/native-chat-composer-state.test.ts @@ -9,6 +9,7 @@ import { editReplacesTriggerToken, EMPTY_HISTORY, filterSlashCommands, + isSkillPickerTriggered, isSlashCommandDraft, pushHistory, recallNext, @@ -96,28 +97,30 @@ describe('deriveComposerAutocomplete — mention', () => { }) }) -describe('deriveComposerAutocomplete — skill', () => { +describe('deriveComposerAutocomplete — one grammar for every agent', () => { const skills = [ skill({ name: 'typescript' }), skill({ name: 'react-useeffect', directoryPath: '/repo/.agents/skills/react-useeffect' }) ] + const codex = getNativeChatAgentProfile('codex') - it('enters skill mode with the query after `$`', () => { - const result = deriveComposerAutocomplete('use $type', 9, COMMANDS, skills) - expect(result.mode).toBe('skill') - if (result.mode !== 'skill') { + it('offers Codex skills under `/`, tokenised as the form Codex invokes', () => { + const result = deriveComposerAutocomplete('use /type', 9, COMMANDS, skills, codex) + expect(result.mode).toBe('slash') + if (result.mode !== 'slash') { return } expect(result.query).toBe('type') - expect(result.items.map((entry) => entry.name)).toEqual(['typescript']) + expect(result.items.map((entry) => entry.token)).toEqual(['$typescript']) }) - it('fires at the start of input too', () => { - expect(deriveComposerAutocomplete('$react', 6, COMMANDS, skills).mode).toBe('skill') + it('no longer treats `$` as a composer trigger', () => { + expect(deriveComposerAutocomplete('use $type', 9, COMMANDS, skills, codex).mode).toBe('none') + expect(deriveComposerAutocomplete('$react', 6, COMMANDS, skills, codex).mode).toBe('none') }) it('does not fire inside shell-style text', () => { - expect(deriveComposerAutocomplete('price$tag', 9, COMMANDS, skills).mode).toBe('none') + expect(deriveComposerAutocomplete('price$tag', 9, COMMANDS, skills, codex).mode).toBe('none') }) }) @@ -194,33 +197,153 @@ describe('apply suggestions', () => { expect(result.caret).toBe('open @src/app.ts '.length) }) - it('applyPickerSuggestion replaces the active $token at the caret', () => { - const result = applyPickerSuggestion( - 'use $typ now', - 8, - { kind: 'skill', id: 'skill:typescript', name: 'typescript', description: null, sources: [] }, - '$' - ) + it('applyPickerSuggestion swaps the typed /token for the agent-native token', () => { + const result = applyPickerSuggestion('use /typ now', 8, { + kind: 'skill', + id: 'skill:typescript', + name: 'typescript', + token: '$typescript', + description: null, + sources: [] + }) expect(result.draft).toBe('use $typescript now') expect(result.caret).toBe('use $typescript '.length) + expect(result.insertedToken).toBe('$typescript') }) }) describe('native skill and command picker', () => { - it('keeps Codex commands under slash and skills under dollar', () => { - const profile = getNativeChatAgentProfile('codex') - const slash = deriveComposerAutocomplete('/', 1, COMMANDS, [skill({})], profile) + it('puts Codex commands and skills in one `/` menu, each with its own token', () => { + const slash = deriveComposerAutocomplete( + '/', + 1, + COMMANDS, + [skill({ name: 'browser' })], + getNativeChatAgentProfile('codex') + ) expect(slash.mode).toBe('slash') - if (slash.mode === 'slash') { - expect(slash.items.every((item) => item.kind === 'command')).toBe(true) + if (slash.mode !== 'slash') { + return + } + expect(slash.grouped).toBe(true) + expect(slash.items.filter((item) => item.kind === 'command').map((item) => item.token)).toEqual( + ['/clear', '/compact', '/help'] + ) + expect(slash.items.filter((item) => item.kind === 'skill').map((item) => item.token)).toEqual([ + '$browser' + ]) + }) + + it('keeps a Codex command and a same-named skill as separate rows', () => { + const result = deriveComposerAutocomplete( + '/clear', + 6, + COMMANDS, + [skill({ name: 'clear' })], + getNativeChatAgentProfile('codex') + ) + expect(result.mode).toBe('slash') + if (result.mode !== 'slash') { + return + } + expect(result.items.map((item) => item.token)).toEqual(['/clear', '$clear']) + expect(result.items.find((item) => item.kind === 'command')?.skillCollision).toBe(false) + }) + + it('offers the same commands and skills for a `/` typed mid-prompt as for a leading one', () => { + const args = [ + COMMANDS, + [skill({ name: 'electron' })], + getNativeChatAgentProfile('claude') + ] as const + const leading = deriveComposerAutocomplete('/', 1, ...args) + const midPrompt = deriveComposerAutocomplete('validate it with /', 18, ...args) + expect(midPrompt.mode).toBe('slash') + if (midPrompt.mode !== 'slash' || leading.mode !== 'slash') { + return } - const dollar = deriveComposerAutocomplete('$', 1, COMMANDS, [skill({})], profile) - expect(dollar.mode).toBe('skill') - if (dollar.mode === 'skill') { - expect(dollar.items.every((item) => item.kind === 'skill')).toBe(true) + expect(midPrompt.items).toEqual(leading.items) + expect(midPrompt.items.map((item) => item.kind)).toContain('command') + expect(midPrompt.items.map((item) => item.kind)).toContain('skill') + expect(midPrompt.grouped).toBe(leading.grouped) + }) + + it('filters the mid-prompt `/` menu by the typed token', () => { + const result = deriveComposerAutocomplete( + 'validate it with /elec', + 22, + COMMANDS, + [skill({ name: 'electron' })], + getNativeChatAgentProfile('claude') + ) + expect(result.mode).toBe('slash') + if (result.mode === 'slash') { + expect(result.prefix).toBe('/') + expect(result.items.map((item) => item.name)).toEqual(['electron']) } }) + it('marks only a draft-leading `/command` dispatchable', () => { + const profile = getNativeChatAgentProfile('claude') + const leading = deriveComposerAutocomplete('/comp', 5, COMMANDS, [], profile) + const midPrompt = deriveComposerAutocomplete('then /comp', 10, COMMANDS, [], profile) + expect(leading.mode === 'slash' && leading.dispatchable).toBe(true) + expect(midPrompt.mode === 'slash' && midPrompt.dispatchable).toBe(false) + }) + + it('leaves a mid-prompt path alone', () => { + expect( + deriveComposerAutocomplete( + 'open /Users/me/notes', + 20, + COMMANDS, + [skill({ name: 'electron' })], + getNativeChatAgentProfile('claude') + ).mode + ).toBe('none') + }) + + it('opens the mid-prompt `/` menu for Codex too, tokenised for Codex', () => { + const result = deriveComposerAutocomplete( + 'validate it with /elec', + 22, + COMMANDS, + [skill({ name: 'electron' })], + getNativeChatAgentProfile('codex') + ) + expect(result.mode).toBe('slash') + if (result.mode !== 'slash') { + return + } + expect(result.dispatchable).toBe(false) + expect(result.items.map((item) => item.token)).toEqual(['$electron']) + }) + + it.each(['claude', 'codex'] as const)( + 'loads the skill catalog for both `/` trigger positions on %s', + (agent) => { + const profile = getNativeChatAgentProfile(agent) + expect(isSkillPickerTriggered('/elec', profile)).toBe(true) + expect(isSkillPickerTriggered('validate it with /elec', profile)).toBe(true) + expect(isSkillPickerTriggered('open /Users/me', profile)).toBe(false) + // Without a catalog fetch the menu would sit on a permanent loading row. + expect(isSkillPickerTriggered('use $elec', profile)).toBe(false) + } + ) + + it('applyPickerSuggestion replaces a mid-prompt /token at the caret', () => { + const result = applyPickerSuggestion('validate it with /elec now', 22, { + kind: 'skill', + id: 'skill:electron', + name: 'electron', + token: '/electron', + description: null, + sources: [] + }) + expect(result.draft).toBe('validate it with /electron now') + expect(result.caret).toBe('validate it with /electron '.length) + }) + it('groups Claude commands and skills under slash', () => { const result = deriveComposerAutocomplete( '/', @@ -327,7 +450,7 @@ describe('native skill and command picker', () => { '$' ) expect(items.map((item) => item.name)).toEqual([longName]) - const applied = applyPickerSuggestion('$sk', 3, items[0], '$') + const applied = applyPickerSuggestion('/sk', 3, items[0]) expect(applied.draft).toBe(`$${longName} `) }) @@ -364,12 +487,14 @@ describe('native skill and command picker', () => { }) it('replaces only the active slash token and preserves text after the caret', () => { - const result = applyPickerSuggestion( - '/bro trailing', - 4, - { kind: 'skill', id: 'skill:browser', name: 'browser', description: null, sources: [] }, - '/' - ) + const result = applyPickerSuggestion('/bro trailing', 4, { + kind: 'skill', + id: 'skill:browser', + name: 'browser', + token: '/browser', + description: null, + sources: [] + }) expect(result.draft).toBe('/browser trailing') expect(result.caret).toBe('/browser '.length) }) @@ -396,29 +521,29 @@ describe('native skill and command picker', () => { it('treats a one-edit token swap as a new trigger occurrence', () => { expect(editReplacesTriggerToken('/foo', '/bar', '/:0')).toBe(true) - expect(editReplacesTriggerToken('use $foo', 'use $bar', '$:4')).toBe(true) + expect(editReplacesTriggerToken('use /foo', 'use /bar', '/:4')).toBe(true) }) it('keeps suppression while typing or deleting inside the dismissed token', () => { expect(editReplacesTriggerToken('/foo', '/food', '/:0')).toBe(false) expect(editReplacesTriggerToken('/food', '/foo', '/:0')).toBe(false) - expect(editReplacesTriggerToken('use $foo now', 'ran $foo now', '$:4')).toBe(false) + expect(editReplacesTriggerToken('use /foo now', 'ran /foo now', '/:4')).toBe(false) }) it('suppresses only the dismissed trigger occurrence', () => { const profile = getNativeChatAgentProfile('codex') - expect(deriveComposerAutocomplete('use $bro', 8, COMMANDS, [skill({})], profile).mode).toBe( - 'skill' + expect(deriveComposerAutocomplete('use /bro', 8, COMMANDS, [skill({})], profile).mode).toBe( + 'slash' ) expect( deriveComposerAutocomplete( - 'use $bro', + 'use /bro', 8, COMMANDS, [skill({})], profile, { status: 'ready', skills: [skill({})] }, - '$:4' + '/:4' ).mode ).toBe('none') }) diff --git a/src/renderer/src/components/native-chat/native-chat-composer-state.ts b/src/renderer/src/components/native-chat/native-chat-composer-state.ts index 3535a90c04be..b76622ed2559 100644 --- a/src/renderer/src/components/native-chat/native-chat-composer-state.ts +++ b/src/renderer/src/components/native-chat/native-chat-composer-state.ts @@ -9,6 +9,8 @@ import { } from '../../../../shared/native-chat-slash-commands' import { buildNativeChatPickerItems, + LEADING_SLASH_TRIGGER, + MID_PROMPT_SLASH_TRIGGER, type NativeChatPickerItem, type NativeChatSkillDiscoverySnapshot } from './native-chat-picker-items' @@ -28,7 +30,9 @@ type PickerAutocomplete = { query: string items: NativeChatPickerItem[] triggerKey: string - prefix: '/' | '$' + prefix: '/' + /** Only a draft-leading `/command` reaches the agent as a command. */ + dispatchable: boolean grouped: boolean commandsEnabled: boolean skillsEnabled: boolean @@ -40,10 +44,20 @@ export type ComposerAutocomplete = | { mode: 'none' } | ({ mode: 'slash' } & PickerAutocomplete) | { mode: 'mention'; query: string } - | ({ mode: 'skill' } & PickerAutocomplete) const EMPTY_DISCOVERY: NativeChatSkillDiscoverySnapshot = { status: 'ready', skills: [] } +/** Whether the caret sits in a token that needs the skill catalog loaded. */ +export function isSkillPickerTriggered( + before: string, + profile: NativeChatAgentProfile | null +): boolean { + if (!profile) { + return false + } + return LEADING_SLASH_TRIGGER.test(before) || MID_PROMPT_SLASH_TRIGGER.test(before) +} + export function deriveComposerAutocomplete( draft: string, caret: number, @@ -55,9 +69,11 @@ export function deriveComposerAutocomplete( sessionSkillNames?: readonly string[] ): ComposerAutocomplete { const before = draft.slice(0, caret) - if (before.startsWith('/') && !/\s/.test(before)) { + const leadingMatch = before.match(LEADING_SLASH_TRIGGER) + if (leadingMatch) { return deriveSlashAutocomplete( - before, + leadingMatch[1], + 0, agentCommands, profile, discovery, @@ -69,70 +85,64 @@ export function deriveComposerAutocomplete( if (mentionMatch) { return { mode: 'mention', query: mentionMatch[1] } } - const skillMatch = - profile?.skillPrefix === '$' || (!profile && skills.length > 0) - ? before.match(/(?:^|\s)\$(\S*)$/) - : null - if (!skillMatch) { + // Why: `/` is the whole composer grammar, so a mid-prompt token opens the same + // menu a leading one does — it just cannot dispatch. + const midPromptMatch = profile ? before.match(MID_PROMPT_SLASH_TRIGGER) : null + if (!midPromptMatch) { return { mode: 'none' } } - const triggerKey = `$:${before.length - skillMatch[1].length - 1}` - if (dismissedTriggerKey === triggerKey) { - return { mode: 'none' } - } - const query = skillMatch[1] - return { - mode: 'skill', + const query = midPromptMatch[1] + return deriveSlashAutocomplete( query, - triggerKey, - prefix: '$', - grouped: false, - commandsEnabled: false, - skillsEnabled: true, - items: buildNativeChatPickerItems([], discovery.skills, query, '$', sessionSkillNames), - skillStatus: discovery.status === 'idle' ? 'loading' : discovery.status, - ...(discovery.errorKind ? { skillErrorKind: discovery.errorKind } : {}) - } + before.length - query.length - 1, + agentCommands, + profile, + discovery, + dismissedTriggerKey, + sessionSkillNames + ) } function deriveSlashAutocomplete( - before: string, + query: string, + triggerPosition: number, agentCommands: readonly SlashCommandSuggestion[], profile: NativeChatAgentProfile | null, discovery: NativeChatSkillDiscoverySnapshot, dismissedTriggerKey: string | null, sessionSkillNames: readonly string[] | undefined ): ComposerAutocomplete { - const triggerKey = '/:0' + const triggerKey = `/:${triggerPosition}` if (dismissedTriggerKey === triggerKey) { return { mode: 'none' } } - const query = before.slice(1) - const hasSlashSkills = profile?.skillPrefix === '/' - // Why: the caller owns catalog policy (e.g. Grok ships skills-only until a - // verified catalog lands); this derivation must not re-gate per agent. + // Every agent with a known grammar offers skills here; only the token a pick + // inserts differs. The caller owns catalog policy (e.g. Grok ships skills-only + // until a verified catalog lands), so this derivation must not re-gate per agent. + const skillsEnabled = profile !== null const items = buildNativeChatPickerItems( agentCommands, - hasSlashSkills ? discovery.skills : [], + skillsEnabled ? discovery.skills : [], query, - '/', - hasSlashSkills ? sessionSkillNames : [] + profile?.skillPrefix ?? '/', + skillsEnabled ? sessionSkillNames : [] ) return { mode: 'slash', query, triggerKey, prefix: '/', - grouped: profile?.groupedSlash === true, + dispatchable: triggerPosition === 0, + grouped: skillsEnabled, commandsEnabled: agentCommands.length > 0, - skillsEnabled: hasSlashSkills, + skillsEnabled, items, - skillStatus: hasSlashSkills + skillStatus: skillsEnabled ? discovery.status === 'idle' ? 'loading' : discovery.status : 'ready', - ...(hasSlashSkills && discovery.errorKind ? { skillErrorKind: discovery.errorKind } : {}) + ...(skillsEnabled && discovery.errorKind ? { skillErrorKind: discovery.errorKind } : {}) } } diff --git a/src/renderer/src/components/native-chat/native-chat-picker-items.ts b/src/renderer/src/components/native-chat/native-chat-picker-items.ts index 427e3b744976..1a21a2461c60 100644 --- a/src/renderer/src/components/native-chat/native-chat-picker-items.ts +++ b/src/renderer/src/components/native-chat/native-chat-picker-items.ts @@ -18,6 +18,8 @@ export type NativeChatPickerItem = kind: 'command' id: string name: string + /** Exactly what a pick inserts — the form the agent invokes. */ + token: string description?: string skillCollision: boolean } @@ -25,6 +27,7 @@ export type NativeChatPickerItem = kind: 'skill' id: string name: string + token: string description: string | null sources: { sourceKind: SkillSourceKind; skillFilePath: string }[] } @@ -47,16 +50,24 @@ export function buildNativeChatPickerItems( commands: readonly SlashCommandSuggestion[], skills: readonly DiscoveredSkill[], query: string, - prefix: '/' | '$', + skillSigil: '/' | '$', sessionSkillNames?: readonly string[] ): NativeChatPickerItem[] { + // A name can only collide when both kinds invoke through the same sigil; + // where skills carry their own, `/review` and `$review` are distinct entries. + const sharedSigil = skillSigil === '/' const unclassifiedNames = new Set( commands.filter((command) => command.kindUnspecified).map((command) => command.name) ) - const mergedSkills = mergeNativeChatSkills(skills, sessionSkillNames, unclassifiedNames) + const mergedSkills = mergeNativeChatSkills( + skills, + sessionSkillNames, + unclassifiedNames, + skillSigil + ) const skillNames = new Set(mergedSkills.map((skill) => skill.name)) const resolvedCommands = commands.filter( - (command) => !(command.kindUnspecified && skillNames.has(command.name)) + (command) => !(sharedSigil && command.kindUnspecified && skillNames.has(command.name)) ) const commandNames = new Set(resolvedCommands.map((command) => command.name)) const commandItems = rankItems( @@ -67,8 +78,9 @@ export function buildNativeChatPickerItems( // it is inserted verbatim; only untrusted skill text gets sanitized. id: `command:${command.name}`, name: command.name, + token: `/${command.name}`, description: command.description ? sanitizePickerText(command.description, 240) : undefined, - skillCollision: prefix === '/' && skillNames.has(command.name) + skillCollision: sharedSigil && skillNames.has(command.name) }, stableOrder: index })), @@ -76,7 +88,7 @@ export function buildNativeChatPickerItems( ) const skillItems = rankItems( mergedSkills - .filter((skill) => !(prefix === '/' && commandNames.has(skill.name))) + .filter((skill) => !(sharedSigil && commandNames.has(skill.name))) .map((item, index) => ({ item, stableOrder: index })), query ) @@ -89,7 +101,8 @@ export function buildNativeChatPickerItems( function mergeNativeChatSkills( skills: readonly DiscoveredSkill[], sessionSkillNames: readonly string[] | undefined, - unclassifiedNames: ReadonlySet + unclassifiedNames: ReadonlySet, + skillSigil: '/' | '$' ): Extract[] { const exactPaths = new Map() for (const skill of skills) { @@ -106,7 +119,10 @@ function mergeNativeChatSkills( byName.set(safeName, [...(byName.get(safeName) ?? []), { ...skill, name: safeName }]) } const discovered = new Map( - [...byName.entries()].map(([name, namedSkills]) => [name, pickerSkill(name, namedSkills)]) + [...byName.entries()].map(([name, namedSkills]) => [ + name, + pickerSkill(name, namedSkills, skillSigil) + ]) ) // Why: when the running session reports its own skills, that report is the // authority on which ones exist — a disk scan cannot see what the session @@ -121,19 +137,21 @@ function mergeNativeChatSkills( ] : [...discovered.keys()] return [...new Set(names)] - .map((name) => discovered.get(name) ?? pickerSkill(name, [])) + .map((name) => discovered.get(name) ?? pickerSkill(name, [], skillSigil)) .sort(comparePickerSkills) } function pickerSkill( name: string, - namedSkills: readonly DiscoveredSkill[] + namedSkills: readonly DiscoveredSkill[], + skillSigil: '/' | '$' ): Extract { const sorted = [...namedSkills].sort(compareDiscoveredSkills) return { kind: 'skill' as const, id: `skill:${name}`, name, + token: `${skillSigil}${name}`, description: sorted[0]?.description ? sanitizePickerText(sorted[0].description, 240) : null, sources: sorted.map((skill) => ({ sourceKind: skill.sourceKind, @@ -245,21 +263,27 @@ function comparePickerSkills( ) } +// `/` is the composer's only trigger, for every agent. A draft-leading slash is +// the one that can dispatch; elsewhere the token starts after whitespace and its +// query stops at the next `/` so file paths stay prose. +export const LEADING_SLASH_TRIGGER = /^\/(\S*)$/ +export const MID_PROMPT_SLASH_TRIGGER = /\s\/([^\s/]*)$/ + +/** Replaces the typed `/token` with the item's own token, which for a skill is + * the agent-native form even though every agent is typed the same way. */ export function applyPickerSuggestion( draft: string, caret: number, - item: NativeChatPickerItem, - prefix: '/' | '$' + item: NativeChatPickerItem ): { draft: string; caret: number; insertedToken: string } { const before = draft.slice(0, caret) const after = draft.slice(caret) - const match = prefix === '/' ? before.match(/^\/(\S*)$/) : before.match(/(^|\s)\$(\S*)$/) + const match = before.match(LEADING_SLASH_TRIGGER) ?? before.match(MID_PROMPT_SLASH_TRIGGER) if (!match) { return { draft, caret, insertedToken: '' } } const query = match.at(-1) ?? '' const tokenStart = before.length - query.length - 1 - const insertedToken = `${prefix}${item.name}` - const nextBefore = `${before.slice(0, tokenStart)}${insertedToken} ` - return { draft: nextBefore + after, caret: nextBefore.length, insertedToken } + const nextBefore = `${before.slice(0, tokenStart)}${item.token} ` + return { draft: nextBefore + after, caret: nextBefore.length, insertedToken: item.token } } diff --git a/src/renderer/src/components/native-chat/use-native-chat-composer-catalog.test.tsx b/src/renderer/src/components/native-chat/use-native-chat-composer-catalog.test.tsx index 1c68ac91ffaf..9e7b3abc5b2d 100644 --- a/src/renderer/src/components/native-chat/use-native-chat-composer-catalog.test.tsx +++ b/src/renderer/src/components/native-chat/use-native-chat-composer-catalog.test.tsx @@ -103,6 +103,7 @@ it('Enter completes a known pre-init skill while still dispatching a built-in co items, triggerKey: '/', prefix: '/', + dispatchable: true, grouped: true, commandsEnabled: true, skillsEnabled: true, diff --git a/src/renderer/src/components/native-chat/use-native-chat-composer-keydown.test.tsx b/src/renderer/src/components/native-chat/use-native-chat-composer-keydown.test.tsx index ed3814e84761..bf39562169e9 100644 --- a/src/renderer/src/components/native-chat/use-native-chat-composer-keydown.test.tsx +++ b/src/renderer/src/components/native-chat/use-native-chat-composer-keydown.test.tsx @@ -2,13 +2,20 @@ import { renderHook } from '@testing-library/react' import { describe, expect, it, vi } from 'vitest' -import { EMPTY_HISTORY, type ComposerAutocomplete } from './native-chat-composer-state' +import { + applyPickerSuggestion, + deriveComposerAutocomplete, + EMPTY_HISTORY, + type ComposerAutocomplete +} from './native-chat-composer-state' +import { getNativeChatAgentProfile } from '../../../../shared/native-chat-agent-profiles' import { useNativeChatComposerKeyDown } from './use-native-chat-composer-keydown' const COMMAND = { kind: 'command' as const, id: 'command:clear', name: 'clear', + token: '/clear', description: 'Clear history', skillCollision: false } @@ -20,6 +27,7 @@ function picker(items = [COMMAND]): Extract composing, ...callbacks @@ -81,6 +89,36 @@ describe('useNativeChatComposerKeyDown', () => { expect(callbacks.send).toHaveBeenCalledOnce() }) + it.each(['claude', 'openclaude', 'codex', 'grok'] as const)( + 'completes mid-prompt command Enter without dispatching or losing prose for %s', + (agent) => { + const draft = 'Explain /cle before continuing' + const caret = 'Explain /cle'.length + const autocomplete = deriveComposerAutocomplete( + draft, + caret, + [COMMAND], + [], + getNativeChatAgentProfile(agent) + ) + expect(autocomplete.mode).toBe('slash') + const { handler, callbacks } = setup(autocomplete, false, draft) + const event = keyEvent('Enter') + handler(event as never) + + expect(event.preventDefault).toHaveBeenCalledOnce() + expect(callbacks.dispatchPickerCommand).not.toHaveBeenCalled() + expect(callbacks.send).not.toHaveBeenCalled() + expect(callbacks.completePickerItem).toHaveBeenCalledOnce() + const [item] = callbacks.completePickerItem.mock.calls[0] + expect(applyPickerSuggestion(draft, caret, item)).toEqual({ + draft: 'Explain /clear before continuing', + caret: 'Explain /clear '.length, + insertedToken: '/clear' + }) + } + ) + it('dismisses Escape without interrupting the agent', () => { const { handler, callbacks } = setup() handler(keyEvent('Escape') as never) diff --git a/src/renderer/src/components/native-chat/use-native-chat-composer-keydown.ts b/src/renderer/src/components/native-chat/use-native-chat-composer-keydown.ts index 2fb712f98257..c85ad949aad2 100644 --- a/src/renderer/src/components/native-chat/use-native-chat-composer-keydown.ts +++ b/src/renderer/src/components/native-chat/use-native-chat-composer-keydown.ts @@ -51,7 +51,7 @@ export function useNativeChatComposerKeyDown({ return } - if (autocomplete.mode === 'slash' || autocomplete.mode === 'skill') { + if (autocomplete.mode === 'slash') { const items = autocomplete.items if (event.key === 'ArrowDown' && items.length > 0) { event.preventDefault() @@ -66,7 +66,9 @@ export function useNativeChatComposerKeyDown({ if ((event.key === 'Enter' || event.key === 'Tab') && items.length > 0) { event.preventDefault() const item = items[activeSuggestion] ?? items[0] - if (event.key === 'Enter' && item.kind === 'command') { + // A mid-prompt command is part of the sentence being written, so Enter + // completes the token instead of sending the command on its own. + if (event.key === 'Enter' && item.kind === 'command' && autocomplete.dispatchable) { dispatchPickerCommand(item) } else { completePickerItem(item) diff --git a/src/renderer/src/components/native-chat/use-native-chat-picker-command-dispatch.test.tsx b/src/renderer/src/components/native-chat/use-native-chat-picker-command-dispatch.test.tsx index 90c495a2bc71..7584dcc15a4b 100644 --- a/src/renderer/src/components/native-chat/use-native-chat-picker-command-dispatch.test.tsx +++ b/src/renderer/src/components/native-chat/use-native-chat-picker-command-dispatch.test.tsx @@ -23,6 +23,7 @@ const COMMAND = { kind: 'command' as const, id: 'command:status', name: 'status', + token: '/status', description: 'Show status', skillCollision: false } diff --git a/src/renderer/src/components/native-chat/use-native-chat-picker-state.ts b/src/renderer/src/components/native-chat/use-native-chat-picker-state.ts index d96060342e57..9d4054b433ee 100644 --- a/src/renderer/src/components/native-chat/use-native-chat-picker-state.ts +++ b/src/renderer/src/components/native-chat/use-native-chat-picker-state.ts @@ -18,6 +18,7 @@ import { classifyNativeChatSend, deriveComposerAutocomplete, editReplacesTriggerToken, + isSkillPickerTriggered, type ComposerAutocomplete, type NativeChatPickerItem, type NativeChatSendClassification @@ -68,13 +69,7 @@ export function useNativeChatPickerState(args: { setActiveSuggestion } = args const profile = useMemo(() => getNativeChatAgentProfile(agent), [agent]) - const beforeCaret = draft.slice(0, caret) - const skillPickerTriggered = - profile?.skillPrefix === '$' - ? /(?:^|\s)\$\S*$/.test(beforeCaret) - : profile?.skillPrefix === '/' - ? beforeCaret.startsWith('/') && !/\s/.test(beforeCaret) - : false + const skillPickerTriggered = isSkillPickerTriggered(draft.slice(0, caret), profile) const discovery = useNativeChatSkills(agent, terminalTabId, skillPickerTriggered) const listboxId = `native-chat-picker-${useId().replaceAll(':', '')}` const dismissalContext = `${draftScopeKey}:${agent}` @@ -114,7 +109,7 @@ export function useNativeChatPickerState(args: { }, [dismissalContext]) useEffect(() => { - if (autocomplete.mode !== 'slash' && autocomplete.mode !== 'skill') { + if (autocomplete.mode !== 'slash') { lastOpenKeyRef.current = null return } @@ -127,10 +122,10 @@ export function useNativeChatPickerState(args: { const completeItem = useCallback( (item: NativeChatPickerItem) => { - if (autocomplete.mode !== 'slash' && autocomplete.mode !== 'skill') { + if (autocomplete.mode !== 'slash') { return } - const result = applyPickerSuggestion(draft, caret, item, autocomplete.prefix) + const result = applyPickerSuggestion(draft, caret, item) if (item.kind === 'skill' && textareaRef.current?.insertSkill) { const from = result.caret - result.insertedToken.length - 1 textareaRef.current.insertSkill(from, caret, result.insertedToken) @@ -174,10 +169,7 @@ export function useNativeChatPickerState(args: { null, sessionSkillNames ) - if ( - (next.mode !== 'slash' && next.mode !== 'skill') || - next.triggerKey !== dismissed.triggerKey - ) { + if (next.mode !== 'slash' || next.triggerKey !== dismissed.triggerKey) { setDismissed(null) } }, diff --git a/src/renderer/src/components/native-chat/use-native-chat-skills.react.test.tsx b/src/renderer/src/components/native-chat/use-native-chat-skills.react.test.tsx index f360394cf37f..e05cbb7a870c 100644 --- a/src/renderer/src/components/native-chat/use-native-chat-skills.react.test.tsx +++ b/src/renderer/src/components/native-chat/use-native-chat-skills.react.test.tsx @@ -3,6 +3,8 @@ import { act, cleanup, render, waitFor } from '@testing-library/react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { NativeChatSkillDiscovery } from './use-native-chat-skills' +import { getNativeChatAgentProfile } from '../../../../shared/native-chat-agent-profiles' +import { isSkillPickerTriggered } from './native-chat-composer-state' const mocks = vi.hoisted(() => ({ callRuntimeRpc: vi.fn(), @@ -56,6 +58,10 @@ function Probe({ enabled }: { enabled: boolean }): null { return null } +function DraftProbe({ draft }: { draft: string }): React.JSX.Element { + return +} + describe('useNativeChatSkills', () => { beforeEach(() => { mocks.state = stateForHost('local') @@ -132,6 +138,24 @@ describe('useNativeChatSkills', () => { ) }) + it('reuses one discovery while typing and reopening leading and mid-prompt slash tokens', async () => { + const view = render() + expect(mocks.callRuntimeRpc).not.toHaveBeenCalled() + + view.rerender() + await waitFor(() => expect(mocks.snapshots.at(-1)?.status).toBe('ready')) + for (const draft of ['Explain /b', 'Explain /br', 'Explain /bro']) { + view.rerender() + expect(mocks.snapshots.at(-1)?.skills.map((skill) => skill.name)).toEqual(['browser']) + } + view.rerender() + expect(mocks.snapshots.at(-1)?.status).toBe('idle') + view.rerender() + await waitFor(() => expect(mocks.snapshots.at(-1)?.status).toBe('ready')) + view.rerender() + expect(mocks.callRuntimeRpc).toHaveBeenCalledTimes(1) + }) + it('surfaces discovery failure instead of remaining loading', async () => { mocks.callRuntimeRpc.mockRejectedValueOnce(new Error('scan failed')) render() diff --git a/src/shared/native-chat-agent-profiles.test.ts b/src/shared/native-chat-agent-profiles.test.ts index 27b7e07d33bd..4d64e283c016 100644 --- a/src/shared/native-chat-agent-profiles.test.ts +++ b/src/shared/native-chat-agent-profiles.test.ts @@ -2,24 +2,23 @@ import { describe, expect, it } from 'vitest' import { getNativeChatAgentProfile } from './native-chat-agent-profiles' describe('native chat agent picker profiles', () => { - it('keeps Codex dollar skills separate from slash commands', () => { + // The composer types the same `/` for every agent; skillPrefix is only the + // form a picked skill is written as. + it('keeps Codex skills invocable as dollar tokens', () => { expect(getNativeChatAgentProfile('codex')).toMatchObject({ skillPrefix: '$', - groupedSlash: false, skillSourceOwner: 'codex' }) }) - it('groups Claude-family and Grok skills under slash', () => { + it('writes Claude-family and Grok skills as slash tokens', () => { expect(getNativeChatAgentProfile('claude')).toMatchObject({ skillPrefix: '/', - groupedSlash: true, skillSourceOwner: 'claude' }) expect(getNativeChatAgentProfile('openclaude')).toMatchObject({ skillSourceOwner: 'claude' }) expect(getNativeChatAgentProfile('grok')).toMatchObject({ skillPrefix: '/', - groupedSlash: true, skillSourceOwner: 'grok' }) }) diff --git a/src/shared/native-chat-agent-profiles.ts b/src/shared/native-chat-agent-profiles.ts index ae52b48efef8..6f86313d17df 100644 --- a/src/shared/native-chat-agent-profiles.ts +++ b/src/shared/native-chat-agent-profiles.ts @@ -3,7 +3,6 @@ import { getAgentSlashCommands, type SlashCommandSuggestion } from './native-cha export type NativeChatAgentProfile = { skillPrefix: '$' | '/' - groupedSlash: boolean /** OpenClaude reads Claude-owned roots, so this can differ from the agent. */ skillSourceOwner: AgentType } @@ -11,22 +10,18 @@ export type NativeChatAgentProfile = { const NATIVE_CHAT_AGENT_PROFILES: Partial> = { codex: { skillPrefix: '$', - groupedSlash: false, skillSourceOwner: 'codex' }, claude: { skillPrefix: '/', - groupedSlash: true, skillSourceOwner: 'claude' }, openclaude: { skillPrefix: '/', - groupedSlash: true, skillSourceOwner: 'claude' }, grok: { skillPrefix: '/', - groupedSlash: true, skillSourceOwner: 'grok' } } From 4e0aa7a4735621f72f7bbb5b0ba46585eb3e4066 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 10 Sep 2026 06:40:42 +0000 Subject: [PATCH 008/261] Update README downloads badge --- docs/assets/readme-downloads.svg | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/assets/readme-downloads.svg b/docs/assets/readme-downloads.svg index 39008eaa963f..3ea69247a269 100644 --- a/docs/assets/readme-downloads.svg +++ b/docs/assets/readme-downloads.svg @@ -1,5 +1,5 @@ - - downloads: 45m + + downloads: 46m @@ -15,7 +15,7 @@ downloads downloads - 45m - 45m + 46m + 46m From 2bf298d1dc1a41c03a270e89eca08834c36f13e0 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 9 Sep 2026 23:47:47 -0700 Subject: [PATCH 009/261] feat(native-chat): the background-tasks strip says what is running (#19311) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(native-chat): name, group, and state the background-tasks strip The strip above the composer described five different kinds of background work as "Monitoring background tasks", with identical flat-dot rows. Now: - Wire: additive optional `name`, `state`, `startedAt` on AgentSessionBackgroundTask, plus `settledTasks` on the state object so terminal siblings of a live fan-out stay visible without changing what old clients render (they keep exactly the live `tasks` list). - Reducer equality learns the new fields, so a publish whose only change is a task's state is no longer judged equal and dropped. - Header counts by kind and lists states within a kind; past three kind segments (or on a narrow strip, measured by its own border-box against the live root font size) it falls back to an honest total, never a partial enumeration, and the strip stays expandable whenever the header is lossy. - Rows group by kind (Agents / Shell / Monitors / Workflows / Tasks), stable-sorted first-seen-then-id, each with a kind icon, its own state dot, a resolved name (description -> name -> kind label), and elapsed. - Claude producer: task frames now carry name (agent_type/subagent_type), a run state mapped from patch status, and first-seen startedAt. Terminal statuses settle a task (completed->done, failed->blocked, killed/stopped->idle) instead of deleting it; settled tasks render only beside still-live work and flush when the last live task ends, so the strip exits exactly when it does today. An unreadable patch leaves a task open, never settled. - Turn gating moves off the strip: the tracker no longer zeroes its roster during a foreground turn, and the client renders the strip whenever it has contents while the idle-only flag now gates just the animated monitoring indicator and conversation commands. * feat(sidebar): indent native-chat subagents under their session row buildSubagentChildRows() has always rendered indented children from parentEntry.subagents, and the structured-session status bridge has always published an AgentStatusEntry for native chat — it just never populated subagents. Connect them: - Wire: additive optional `backgroundTasks` on AgentSessionStatusSummary (live tasks only), projected by the host status feed from the provider's backgroundTaskState hook and republished on task edges via the background-task channel, with the shared task equality suppressing no-op re-projections. - Bridge: maps agent-kind tasks onto the sidebar's own AgentSubagentState (working/waiting/blocked, terminal -> idle) — kinds stay distinct, so a backgrounded shell never lands in a subagent count — and extends its pre-write equality with the existing agentSubagentsEqual. - parentIsFresh for a bridge entry means "the host feed reported a change inside the sidebar's ordinary evidence window": every publish restamps evidenceObservedAt, and a dead feed stops restamping, so children decay to idle on lost contact instead of pinning 'working'. * fix(native-chat): settle tasks the aggregate roster evicted first; carry usage Real-agent QA showed settledTasks never rendered. A frame capture from the SDK (probe against claude 2.1.261) explains it: when a backgrounded child finishes, the producer emits `background_tasks_changed` FIRST — with the task already absent — and only then `task_updated`/`task_notification` with the outcome, in the same tick. The tracker's settle path looked the task up in the live roster the aggregate had just evicted, so retention lost the race 100% of the time. Fix: aggregate eviction of a live backgrounded task now parks its details in a bounded recently-removed map (new claude-settled-background-tasks.ts, which also owns the settled roster), and the trailing terminal edge consumes it. A removal whose outcome frame never arrives still vanishes — nothing is guessed into a finished state. A second terminal edge for the same task re-derives the settled state and can add final usage. The captured sequence is replayed verbatim as a tracker test, including the kill-at-exit tail proving the strip still exits with the last live task. The same capture disproved the PR's earlier claim that Claude task frames carry no usage: task_progress and task_notification both carry usage.total_tokens. Additive optional `totalTokens` on the wire task, covered by the shared equality; the tracker takes usage (never the transient "Running " description) from task_progress, and rows render the mock's "18.1k · 2m" meta — settled rows keep final usage with no still-growing clock. * chore(i18n): sync runtime-required catalog for backgroundTasks.runningList * fix(native-chat): preserve background task lifecycle and bound update work * fix(native-chat): transfer resumed background tasks to one live owner * fix(native-chat): bring structured session host under the line cap and restore subscribe fixture * fix(native-chat): complete journal stubs and stop notifying on feed teardown The status feed's projection cache calls journal.cursor(); the rename test's stubs are cast through unknown, so the missing method only surfaced at runtime. Teardown runs only once nothing is activated, so there is no mounted reader to notify - clearing confirmed sessions is what prevents a stale live on reactivation. * feat(native-chat): lead each strip header count with its kind icon The header carried one aggregate state dot, so a fan-out of agents and a monitor looked alike. Each count segment now leads with its own kind glyph; a collapsed total spans kinds and takes none. Monitor is the heartbeat AgentStateDot already draws for monitoring, so the strip and the agent sidebar speak one vocabulary. * feat(native-chat): give the strip's monitor heartbeat the sidebar amber The glyph matched AgentStateDot but the colour did not, so a monitor in the strip did not read as the monitor in the agent sidebar. One shared tone helper now serves the header segment and the expanded row, so they cannot diverge. Monitoring is a state the app already colours; the other four kinds are plain markers and stay neutral. A running turn still dims the whole set. * fix(native-chat): draw the strip header separator in a visible tone The separator used `text-border`, a divider-line token that is 7% white in dark mode - an order of magnitude fainter than the counts on either side, so the dot between them read as absent. main.css already records that token as too faint for a visible mark. * fix(native-chat): give the worktree-ps journal stub a cursor The status feed's projection cache calls journal.cursor(); this stub is cast through unknown, so the missing method only surfaced at runtime. Its journal never changes, so a real one would hold the cursor steady. * refactor(native-chat): split the sidebar subagent rows out of this PR The strip stands alone: the sidebar mapping, its observation plumbing and the AgentStatusEntry.subagents wiring move to a stacked follow-up. No wire field here is sidebar-only - the strip's rows read name, state, elapsed and tokens. * perf(native-chat): keep task usage out of the session status summary A `task_progress` frame ticks a background task's `totalTokens`, which failed the status feed's equality check and re-broadcast a full summary to every `agentSession.subscribeStatus` subscriber — paired-web and SSH/relay clients included — for a number no session list renders. The projection now drops usage; tokens keep flowing on the background-task channel the strip reads. * fix(native-chat): correct token unit rounding and drop the unused dot state `formatBackgroundTaskTokens` rounded before choosing the unit, so 999_950 rendered as "1000k" instead of "1m"; pick the unit from the rounded value. `backgroundTasksDotState` has no caller on this branch or the stacked sidebar PR, and its multi-kind branch would report 'monitoring' over an attention state. Delete it rather than leave it to be wired up. * fix(i18n): drop the orphaned backgroundTasks.runningList key The strip rewrite removed its only call site, and an unreferenced key gets promoted into the eagerly parsed boot catalog. Delete it from en.json and regenerate en-runtime-required.json. * fix(native-chat): show the reason on every attention row The row guarded the reason line on 'waiting', so an 'unverifiable' child ("no contact") and a 'blocked' one ("failed") rendered bare while the collapsed header named exactly those reasons. `backgroundTaskStateReason` already returns null for the non-attention states, so the guard was only lossy — the SSH boundary requires the unverifiable verdict stay legible. Also keys the header segments off their kind discriminant instead of the translated display text. * fix(native-chat): make the strip header agree with its own count The headline counts live AND settled rows, but the state breakdown omitted 'done', so one working agent beside four settled ones read "5 agents — 1 working": the count said five, the breakdown accounted for one. Done now appears in the muted detail (never as an emphasised segment) so the two agree. The single-command header also drew an elapsed clock on a settled task, which the row already refuses as a lie about finished work. * perf(native-chat): memoize the background-task roster grouping The 1 Hz elapsed tick re-rendered the strip, and the render body regrouped, re-sorted and re-translated every task each time only `now` had changed. The header still derives from `now` on purpose. * test(native-chat): cover settled rows and the mid-turn mounted strip Neither headline behaviour had component coverage: every strip render passed `settledTasks={[]}`, and the `showBackgroundTasks` seam was never set true, so the strip staying mounted through a running turn was exercised nowhere. Adds a settled-beside-live row test (final usage kept, no clock, no stop) and a mid-turn mount test (strip present, turn owns the voice). The background-task tests share one session-element helper so the file stays under its line cap. * refactor(claude): keep MAX_TASK_ID_LENGTH module-private Nothing outside claude-background-task-frames.ts references it; the export was residue from this PR's split. * test(native-chat): give the mid-turn strip test a real turn main now gates the composer's stop button on a provider-minted turnId rather than the send-time working signal, so a test claiming a running turn has to supply one. The controller mock hardcoded turnId null. --------- Co-authored-by: Merge Sim --- .../first-work-branch-rename.test.ts | 7 +- .../claude/claude-background-task-frames.ts | 106 ++++++ .../claude-background-task-resume.test.ts | 92 +++++ .../claude-background-task-tracker.test.ts | 336 +++++++++++++++--- .../claude/claude-background-task-tracker.ts | 251 +++++++------ .../claude/claude-settled-background-tasks.ts | 127 +++++++ .../claude-structured-session-close.test.ts | 8 +- .../provider-frame-activity.test.ts | 4 +- ...d-agent-session-background-task-channel.ts | 6 +- .../structured-agent-session-host.ts | 13 +- ...ructured-agent-session-status-feed.test.ts | 152 +++++++- .../structured-agent-session-status-feed.ts | 86 ++++- .../local-file-sink-memory.test.ts | 6 +- .../worktree-ps-structured-host.spec.ts | 2 + ...ructured-agent-session-rpc.test-fixture.ts | 1 + .../NativeChatBackgroundTasksStatus.test.tsx | 239 ++++++++++++- .../NativeChatBackgroundTasksStatus.tsx | 265 ++++++++++---- .../NativeChatStructuredSession.test.tsx | 120 ++++--- .../NativeChatStructuredSession.tsx | 11 +- .../background-task-header-content.ts | 192 ++++++++++ .../background-task-roster.test.ts | 245 +++++++++++++ .../native-chat/background-task-roster.ts | 181 ++++++++++ ...tructured-session-background-tasks-view.ts | 32 +- .../use-structured-agent-session.ts | 11 +- src/renderer/src/i18n/locales/en.json | 30 +- src/shared/agent-session-wire.ts | 58 +++ .../structured-agent-session-reducer.test.ts | 71 ++++ .../structured-agent-session-reducer.ts | 29 +- 28 files changed, 2331 insertions(+), 350 deletions(-) create mode 100644 src/main/claude/claude-background-task-frames.ts create mode 100644 src/main/claude/claude-background-task-resume.test.ts create mode 100644 src/main/claude/claude-settled-background-tasks.ts create mode 100644 src/renderer/src/components/native-chat/background-task-header-content.ts create mode 100644 src/renderer/src/components/native-chat/background-task-roster.test.ts create mode 100644 src/renderer/src/components/native-chat/background-task-roster.ts diff --git a/src/main/agent-hooks/first-work-branch-rename.test.ts b/src/main/agent-hooks/first-work-branch-rename.test.ts index 9cd4d5440418..8ecd4997aa0f 100644 --- a/src/main/agent-hooks/first-work-branch-rename.test.ts +++ b/src/main/agent-hooks/first-work-branch-rename.test.ts @@ -100,10 +100,14 @@ describe('maybeAutoRenameBranchOnFirstWork', () => { isPendingFirstAgentMessageRename: () => true }) const items: AgentJournalRenderItem[] = [] + // A real journal's sequence only ever advances, so the feed's projection + // cache must miss on every publish here: this test is about the rename. + let sequence = 0 const journal = { snapshot: () => ({ items }), lastActivityAt: () => 1, - isReadOnly: false + isReadOnly: false, + cursor: () => ({ epoch: 1, sequence: (sequence += 1) }) } as unknown as AgentSessionJournal const pending: Promise[] = [] const observe = vi.fn((summary, options) => { @@ -175,6 +179,7 @@ describe('maybeAutoRenameBranchOnFirstWork', () => { const journal = { isReadOnly: false, lastActivityAt: () => 1, + cursor: () => ({ epoch: 1, sequence: 1 }), snapshot: () => ({ items: [ { body: { kind: 'message', role: 'user', blocks: [{ type: 'text', text: 'Fix auth' }] } }, diff --git a/src/main/claude/claude-background-task-frames.ts b/src/main/claude/claude-background-task-frames.ts new file mode 100644 index 000000000000..d954128d91b1 --- /dev/null +++ b/src/main/claude/claude-background-task-frames.ts @@ -0,0 +1,106 @@ +// Field readers for the Claude SDK's background-task lifecycle frames +// (task_started / task_updated / task_notification / background_tasks_changed). +// Pure and bounded: every reader rejects absent, non-string, or oversized +// values so a malformed frame degrades to "field unknown", never to a throw. + +import type { + AgentSessionBackgroundTask, + AgentSessionBackgroundTaskRunState +} from '../../shared/agent-session-wire' + +const MAX_TASK_ID_LENGTH = 512 +const MAX_TASK_TEXT_LENGTH = 512 + +export type ClaudeBackgroundTaskKind = AgentSessionBackgroundTask['kind'] + +export function record(value: unknown): Record | null { + return typeof value === 'object' && value !== null ? (value as Record) : null +} + +/** The bound every task id shares, wherever it enters. An id the roster stores + * becomes a durable entry key, so a provisional one takes the same bound the + * announced path applies — an over-long id is rejected, never truncated. */ +export function isBoundedClaudeTaskId(value: string): boolean { + return value.length > 0 && value.length <= MAX_TASK_ID_LENGTH +} + +export function taskId(message: Record): string | null { + const value = message.task_id + return typeof value === 'string' && isBoundedClaudeTaskId(value) ? value : null +} + +function boundedTaskText(value: unknown): string | undefined { + if (typeof value !== 'string') { + return undefined + } + const trimmed = value.trim().replace(/\s+/g, ' ') + return trimmed.length > 0 ? trimmed.slice(0, MAX_TASK_TEXT_LENGTH) : undefined +} + +export function taskDescription(value: unknown): string | undefined { + return boundedTaskText(value) +} + +/** The provider-reported identity for a task. Subagent frames have carried the + * type under both `agent_type` and `subagent_type` across SDK versions. */ +export function taskName(frame: Record): string | undefined { + return ( + boundedTaskText(frame.name) ?? + boundedTaskText(frame.agent_type) ?? + boundedTaskText(frame.subagent_type) + ) +} + +export function classifyClaudeBackgroundTaskKind(taskType: unknown): ClaudeBackgroundTaskKind { + switch (taskType) { + case 'local_agent': + return 'agent' + case 'local_workflow': + return 'workflow' + case 'local_bash': + return 'command' + case 'monitor': + return 'monitor' + default: + return 'unknown' + } +} + +/** Cumulative token usage from a task_progress / task_notification frame. */ +export function taskUsageTotalTokens(frame: Record): number | undefined { + const usage = record(frame.usage) + const total = usage?.total_tokens + return typeof total === 'number' && Number.isFinite(total) && total >= 0 + ? Math.floor(total) + : undefined +} + +/** Settled state for a terminal status. Null for anything else — an unreadable + * status never settles a task by itself. */ +export function terminalClaudeTaskRunState( + status: unknown +): AgentSessionBackgroundTaskRunState | null { + switch (status) { + case 'completed': + return 'done' + case 'failed': + return 'blocked' + case 'killed': + case 'stopped': + return 'idle' + default: + return null + } +} + +/** Live state for a non-terminal status. Null leaves the tracked state alone. */ +export function liveClaudeTaskRunState(status: unknown): AgentSessionBackgroundTaskRunState | null { + switch (status) { + case 'pending': + case 'running': + case 'paused': + return 'working' + default: + return null + } +} diff --git a/src/main/claude/claude-background-task-resume.test.ts b/src/main/claude/claude-background-task-resume.test.ts new file mode 100644 index 000000000000..c043d4853049 --- /dev/null +++ b/src/main/claude/claude-background-task-resume.test.ts @@ -0,0 +1,92 @@ +import { describe, expect, it } from 'vitest' +import { ClaudeBackgroundTaskTracker } from './claude-background-task-tracker' + +const agent = { + task_id: 'a962f88aa82feb1c1', + task_type: 'local_agent', + description: 'Long proof writer' +} +const shell = { task_id: 'bcl6x3ixf', task_type: 'local_bash', description: 'sleep 150' } +const sibling = { task_id: 'sibling', task_type: 'local_agent' } + +function system(subtype: string, fields: Record) { + return { type: 'system', subtype, ...fields } +} + +describe('Claude background task pause/resume ownership', () => { + it('moves a retained child back to live ownership across eviction, outcome, and auto-resume', () => { + let now = 100 + const tracker = new ClaudeBackgroundTaskTracker(() => now) + const roster = (tasks: unknown[]) => + tracker.observe(system('background_tasks_changed', { tasks })) + roster([agent, sibling, shell]) + tracker.observe( + system('task_progress', { task_id: agent.task_id, usage: { total_tokens: 18000 } }) + ) + tracker.observe({ type: 'result' }) + expect(tracker.state?.tasks).toHaveLength(3) + roster([sibling, shell]) + expect(tracker.state?.settledTasks).toBeUndefined() + tracker.observe( + system('task_updated', { task_id: agent.task_id, patch: { status: 'completed' } }) + ) + tracker.observe( + system('task_notification', { + task_id: agent.task_id, + status: 'completed', + usage: { total_tokens: 19003 } + }) + ) + expect(tracker.state?.settledTasks).toEqual([ + expect.objectContaining({ + id: agent.task_id, + state: 'done', + startedAt: 100, + totalTokens: 19003 + }) + ]) + now = 150000 + roster([agent, sibling, shell]) + expect(tracker.state?.settledTasks).toBeUndefined() + expect(tracker.state?.tasks).toEqual([ + expect.objectContaining({ + id: agent.task_id, + state: 'working', + startedAt: 100, + totalTokens: 19003 + }), + expect.objectContaining({ id: sibling.task_id }), + expect.objectContaining({ id: shell.task_id }) + ]) + expect(tracker.stoppableTaskIds).toEqual([agent.task_id, sibling.task_id, shell.task_id]) + roster([sibling, shell]) + tracker.observe( + system('task_notification', { + task_id: agent.task_id, + status: 'completed', + usage: { total_tokens: 21000 } + }) + ) + expect(tracker.state?.settledTasks).toEqual([ + expect.objectContaining({ id: agent.task_id, startedAt: 100, totalTokens: 21000 }) + ]) + roster([]) + expect(tracker.state).toBeNull() + }) + + it('reconciles an edge-only resume without keeping its earlier settled copy', () => { + const tracker = new ClaudeBackgroundTaskTracker(() => 100) + for (const task of [agent, sibling]) { + tracker.observe(system('task_started', { ...task, is_backgrounded: true })) + } + tracker.observe(system('task_notification', { task_id: agent.task_id, status: 'completed' })) + tracker.observe( + system('task_updated', { + task_id: agent.task_id, + patch: { status: 'running', is_backgrounded: true } + }) + ) + expect(tracker.state?.tasks).toHaveLength(2) + expect(tracker.state?.settledTasks).toBeUndefined() + }) +}) diff --git a/src/main/claude/claude-background-task-tracker.test.ts b/src/main/claude/claude-background-task-tracker.test.ts index d8f316d7dcd8..df89a64a968d 100644 --- a/src/main/claude/claude-background-task-tracker.test.ts +++ b/src/main/claude/claude-background-task-tracker.test.ts @@ -16,6 +16,11 @@ function aggregate(tasks: unknown[]): Record { return system('background_tasks_changed', { tasks }) } +function trackerAt(times: number[]): ClaudeBackgroundTaskTracker { + let index = 0 + return new ClaudeBackgroundTaskTracker(() => times[Math.min(index++, times.length - 1)]) +} + describe('ClaudeBackgroundTaskTracker', () => { it('classifies SDK task types without inferring them from descriptions', () => { expect(classifyClaudeBackgroundTaskKind('local_agent')).toBe('agent') @@ -25,27 +30,30 @@ describe('ClaudeBackgroundTaskTracker', () => { expect(classifyClaudeBackgroundTaskKind('future_task')).toBe('unknown') }) - it('waits for the foreground turn to settle before monitoring a background task', () => { - const tracker = new ClaudeBackgroundTaskTracker() + it('publishes a backgrounded task while the foreground turn is still running', () => { + const tracker = trackerAt([100]) tracker.observe({ type: 'user' }, true) - tracker.observe( - system('task_started', { - task_id: 'task-1', - task_type: 'local_agent', - is_backgrounded: true - }) - ) - expect(tracker.state).toBeNull() - - expect(tracker.observe(result())).toBe(true) + expect( + tracker.observe( + system('task_started', { + task_id: 'task-1', + task_type: 'local_agent', + is_backgrounded: true + }) + ) + ).toBe(true) expect(tracker.state).toEqual({ state: 'monitoring', - tasks: [{ id: 'task-1', kind: 'agent' }] + tasks: [{ id: 'task-1', kind: 'agent', state: 'working', startedAt: 100 }] }) + + // The turn settling changes nothing the strip renders. + expect(tracker.observe(result())).toBe(false) + expect(tracker.state?.tasks).toHaveLength(1) }) it('uses an explicit background update for a foreground task and ignores progress alone', () => { - const tracker = new ClaudeBackgroundTaskTracker() + const tracker = trackerAt([100]) tracker.observe({ type: 'user' }, true) tracker.observe( system('task_started', { @@ -63,12 +71,12 @@ describe('ClaudeBackgroundTaskTracker', () => { tracker.observe(system('task_updated', { task_id: 'task-1', patch: { is_backgrounded: true } })) expect(tracker.state).toEqual({ state: 'monitoring', - tasks: [{ id: 'task-1', kind: 'command' }] + tasks: [{ id: 'task-1', kind: 'command', state: 'working', startedAt: 100 }] }) }) it('publishes bounded display details when a running task description changes', () => { - const tracker = new ClaudeBackgroundTaskTracker() + const tracker = trackerAt([100]) expect( tracker.observe( system('task_started', { @@ -81,7 +89,15 @@ describe('ClaudeBackgroundTaskTracker', () => { ).toBe(true) expect(tracker.state).toEqual({ state: 'monitoring', - tasks: [{ id: 'task-1', kind: 'command', description: 'run the build' }] + tasks: [ + { + id: 'task-1', + kind: 'command', + description: 'run the build', + state: 'working', + startedAt: 100 + } + ] }) expect( @@ -103,8 +119,193 @@ describe('ClaudeBackgroundTaskTracker', () => { ).toBe(false) }) + it('carries provider-reported names and re-derives classification per transition', () => { + const tracker = trackerAt([100]) + tracker.observe( + system('task_started', { + task_id: 'task-1', + task_type: 'future_task', + is_backgrounded: true + }) + ) + expect(tracker.state?.tasks?.[0]).toMatchObject({ kind: 'unknown' }) + + expect( + tracker.observe( + system('task_updated', { + task_id: 'task-1', + patch: { task_type: 'local_agent', agent_type: 'deep_review' } + }) + ) + ).toBe(true) + expect(tracker.state?.tasks?.[0]).toMatchObject({ + kind: 'agent', + name: 'deep_review', + state: 'working' + }) + }) + + it('retains settled siblings beside live work and exits with the last live task', () => { + const tracker = trackerAt([100, 200]) + tracker.observe( + system('task_started', { task_id: 'task-a', task_type: 'local_agent', is_backgrounded: true }) + ) + tracker.observe( + system('task_started', { task_id: 'task-b', task_type: 'local_agent', is_backgrounded: true }) + ) + + expect( + tracker.observe(system('task_updated', { task_id: 'task-a', patch: { status: 'completed' } })) + ).toBe(true) + expect(tracker.state).toEqual({ + state: 'monitoring', + tasks: [{ id: 'task-b', kind: 'agent', state: 'working', startedAt: 200 }], + settledTasks: [{ id: 'task-a', kind: 'agent', state: 'done', startedAt: 100 }] + }) + expect(tracker.stoppableTaskIds).toEqual(['task-b']) + + expect( + tracker.observe(system('task_updated', { task_id: 'task-b', patch: { status: 'killed' } })) + ).toBe(true) + expect(tracker.state).toBeNull() + }) + + it('settles a sibling from the captured producer order: aggregate eviction, then the outcome', () => { + // Verbatim sequence from a real SDK capture (2026-09-07): the aggregate + // roster arrives FIRST, already missing the finished task, and the + // terminal edges trail in the same tick. + const tracker = trackerAt([100, 200]) + tracker.observe( + system('task_started', { + task_id: 'bh4zn8der', + tool_use_id: 'toolu_01M', + description: 'Sleep for 5 seconds', + is_backgrounded: true, + task_type: 'local_bash' + }) + ) + tracker.observe( + aggregate([ + { task_id: 'bh4zn8der', task_type: 'local_bash', description: 'Sleep for 5 seconds' }, + { task_id: 'bprosaiim', task_type: 'local_bash', description: 'Sleep for 25 seconds' } + ]) + ) + + // The settling child is evicted by the aggregate before any outcome frame. + tracker.observe( + aggregate([ + { task_id: 'bprosaiim', task_type: 'local_bash', description: 'Sleep for 25 seconds' } + ]) + ) + tracker.observe( + system('task_updated', { + task_id: 'bh4zn8der', + patch: { status: 'completed', end_time: 1788804376515 } + }) + ) + expect( + tracker.observe( + system('task_notification', { + task_id: 'bh4zn8der', + tool_use_id: 'toolu_01M', + status: 'completed', + summary: 'Background command "Sleep for 5 seconds" completed (exit code 0)', + usage: { total_tokens: 18130, tool_uses: 1, duration_ms: 10772 } + }) + ) + ).toBe(true) + expect(tracker.state).toEqual({ + state: 'monitoring', + tasks: [ + { + id: 'bprosaiim', + kind: 'command', + description: 'Sleep for 25 seconds', + state: 'working', + startedAt: 200 + } + ], + settledTasks: [ + { + id: 'bh4zn8der', + kind: 'command', + description: 'Sleep for 5 seconds', + state: 'done', + startedAt: 100, + totalTokens: 18130 + } + ] + }) + + // Last task killed, same captured order: the strip exits. + tracker.observe(aggregate([])) + tracker.observe(system('task_updated', { task_id: 'bprosaiim', patch: { status: 'killed' } })) + tracker.observe(system('task_notification', { task_id: 'bprosaiim', status: 'stopped' })) + expect(tracker.state).toBeNull() + }) + + it('carries task_progress usage into a live row without clobbering its name', () => { + const tracker = trackerAt([100]) + tracker.observe( + system('task_started', { + task_id: 'agent-1', + task_type: 'local_agent', + subagent_type: 'general-purpose', + description: 'Sleep 6 seconds test', + is_backgrounded: true + }) + ) + expect( + tracker.observe( + system('task_progress', { + task_id: 'agent-1', + description: 'Running Sleep for 6 seconds', + subagent_type: 'general-purpose', + usage: { total_tokens: 14866, tool_uses: 1, duration_ms: 2818 }, + last_tool_name: 'Bash' + }) + ) + ).toBe(true) + expect(tracker.state?.tasks?.[0]).toEqual({ + id: 'agent-1', + kind: 'agent', + // Progress descriptions are transient activity, never the task's name. + description: 'Sleep 6 seconds test', + name: 'general-purpose', + state: 'working', + startedAt: 100, + totalTokens: 14866 + }) + }) + + it('maps terminal statuses onto settled states', () => { + const tracker = trackerAt([100, 200]) + tracker.observe( + system('task_started', { task_id: 'live', task_type: 'local_agent', is_backgrounded: true }) + ) + tracker.observe( + system('task_started', { task_id: 'failed', task_type: 'local_agent', is_backgrounded: true }) + ) + tracker.observe(system('task_notification', { task_id: 'failed', status: 'failed' })) + expect(tracker.state?.settledTasks).toEqual([ + { id: 'failed', kind: 'agent', state: 'blocked', startedAt: 200 } + ]) + }) + + it('leaves a task open when a patch cannot be read', () => { + const tracker = trackerAt([100]) + tracker.observe( + system('task_started', { task_id: 'task-1', task_type: 'local_agent', is_backgrounded: true }) + ) + expect(tracker.observe(system('task_updated', { task_id: 'task-1', patch: 'garbage' }))).toBe( + false + ) + expect(tracker.state?.tasks).toHaveLength(1) + expect(tracker.state?.settledTasks).toBeUndefined() + }) + it('replaces its roster from aggregate lifecycle frames and preserves stoppable provider ids', () => { - const tracker = new ClaudeBackgroundTaskTracker() + const tracker = trackerAt([100]) expect( tracker.observe( aggregate([ @@ -117,8 +318,8 @@ describe('ClaudeBackgroundTaskTracker', () => { expect(tracker.state).toEqual({ state: 'monitoring', tasks: [ - { id: 'task-agent', kind: 'agent', description: 'agent' }, - { id: 'task-bash', kind: 'command', description: 'bash' } + { id: 'task-agent', kind: 'agent', description: 'agent', state: 'working', startedAt: 100 }, + { id: 'task-bash', kind: 'command', description: 'bash', state: 'working', startedAt: 100 } ] }) @@ -128,18 +329,31 @@ describe('ClaudeBackgroundTaskTracker', () => { ) ).toBe(true) expect(tracker.stoppableTaskIds).toEqual(['task-next']) - expect(tracker.state).toEqual({ - state: 'monitoring', - tasks: [{ id: 'task-next', kind: 'workflow', description: 'workflow' }] - }) expect(tracker.observe(aggregate([]))).toBe(true) expect(tracker.stoppableTaskIds).toEqual([]) expect(tracker.state).toBeNull() }) + it('preserves first-seen timestamps across aggregate roster replacement', () => { + const tracker = trackerAt([100, 200]) + tracker.observe( + system('task_started', { task_id: 'task-1', task_type: 'local_agent', is_backgrounded: true }) + ) + tracker.observe( + aggregate([ + { task_id: 'task-1', task_type: 'local_agent' }, + { task_id: 'task-2', task_type: 'local_bash' } + ]) + ) + expect(tracker.state?.tasks).toEqual([ + { id: 'task-1', kind: 'agent', state: 'working', startedAt: 100 }, + { id: 'task-2', kind: 'command', state: 'working', startedAt: 200 } + ]) + }) + it('excludes ambient aggregate tasks', () => { - const tracker = new ClaudeBackgroundTaskTracker() + const tracker = trackerAt([100]) tracker.observe( aggregate([ { task_id: 'ambient', task_type: 'monitor', description: 'watcher', ambient: true }, @@ -151,7 +365,7 @@ describe('ClaudeBackgroundTaskTracker', () => { }) it('does not let late edge frames revive tasks cleared by an aggregate roster', () => { - const tracker = new ClaudeBackgroundTaskTracker() + const tracker = trackerAt([100]) tracker.observe( aggregate([{ task_id: 'task-late', task_type: 'local_agent', description: 'agent' }]) ) @@ -173,7 +387,7 @@ describe('ClaudeBackgroundTaskTracker', () => { }) it('lets an authoritative aggregate roster replace earlier terminal-edge evidence', () => { - const tracker = new ClaudeBackgroundTaskTracker() + const tracker = trackerAt([100]) tracker.observe(system('task_notification', { task_id: 'task-live', status: 'completed' })) tracker.observe( @@ -183,12 +397,28 @@ describe('ClaudeBackgroundTaskTracker', () => { expect(tracker.stoppableTaskIds).toEqual(['task-live']) expect(tracker.state).toEqual({ state: 'monitoring', - tasks: [{ id: 'task-live', kind: 'agent', description: 'agent' }] + tasks: [ + { id: 'task-live', kind: 'agent', description: 'agent', state: 'working', startedAt: 100 } + ] }) }) + it('retracts a settled copy when an authoritative roster reports the task live again', () => { + const tracker = trackerAt([100, 200, 300]) + const tasks = [ + { task_id: 'agent', task_type: 'local_agent', description: 'Review sample' }, + { task_id: 'shell', task_type: 'local_bash' } + ] + tracker.observe(aggregate(tasks)) + tracker.observe(system('task_notification', { task_id: 'agent', status: 'completed' })) + expect(tracker.state?.settledTasks).toHaveLength(1) + tracker.observe(aggregate(tasks)) + expect(tracker.state?.tasks?.map((task) => task.id)).toEqual(['agent', 'shell']) + expect(tracker.state?.settledTasks).toBeUndefined() + }) + it('keeps terminal edges authoritative on either side of aggregate replacement', () => { - const terminalFirst = new ClaudeBackgroundTaskTracker() + const terminalFirst = trackerAt([100]) terminalFirst.observe( system('task_notification', { task_id: 'task-first', status: 'completed' }) ) @@ -202,7 +432,7 @@ describe('ClaudeBackgroundTaskTracker', () => { ) expect(terminalFirst.state).toBeNull() - const terminalLast = new ClaudeBackgroundTaskTracker() + const terminalLast = trackerAt([100]) terminalLast.observe( aggregate([{ task_id: 'task-last', task_type: 'local_agent', description: 'agent' }]) ) @@ -218,7 +448,7 @@ describe('ClaudeBackgroundTaskTracker', () => { }) it('keeps terminal evidence authoritative across duplicates and out-of-order starts', () => { - const tracker = new ClaudeBackgroundTaskTracker() + const tracker = trackerAt([100]) const terminal = system('task_notification', { task_id: 'task-late', status: 'completed' }) tracker.observe(terminal) tracker.observe(terminal) @@ -239,7 +469,7 @@ describe('ClaudeBackgroundTaskTracker', () => { ) expect(tracker.state).toEqual({ state: 'monitoring', - tasks: [{ id: 'task-live', kind: 'monitor' }] + tasks: [{ id: 'task-live', kind: 'monitor', state: 'monitoring', startedAt: 100 }] }) expect( tracker.observe(system('task_updated', { task_id: 'task-live', patch: { status: 'killed' } })) @@ -249,17 +479,24 @@ describe('ClaudeBackgroundTaskTracker', () => { it('recognizes task types that are registered only as background work', () => { for (const taskType of ['local_workflow', 'monitor']) { - const tracker = new ClaudeBackgroundTaskTracker() + const tracker = trackerAt([100]) tracker.observe(system('task_started', { task_id: taskType, task_type: taskType })) expect(tracker.state).toEqual({ state: 'monitoring', - tasks: [{ id: taskType, kind: taskType === 'local_workflow' ? 'workflow' : 'monitor' }] + tasks: [ + { + id: taskType, + kind: taskType === 'local_workflow' ? 'workflow' : 'monitor', + state: taskType === 'local_workflow' ? 'working' : 'monitoring', + startedAt: 100 + } + ] }) } }) it('admits unknown background updates conservatively and bounds edge-only fallback ids', () => { - const tracker = new ClaudeBackgroundTaskTracker() + const tracker = trackerAt([100]) tracker.observe( system('task_updated', { task_id: 'unknown', patch: { is_backgrounded: true } }) ) @@ -278,7 +515,7 @@ describe('ClaudeBackgroundTaskTracker', () => { }) it('bounds aggregate rosters and resets to the edge-only fallback on clear', () => { - const tracker = new ClaudeBackgroundTaskTracker() + const tracker = trackerAt([100]) tracker.observe( aggregate( Array.from({ length: 400 }, (_, index) => ({ @@ -301,23 +538,30 @@ describe('ClaudeBackgroundTaskTracker', () => { expect(tracker.stoppableTaskIds).toEqual(['edge-after-reset']) }) - it('gates aggregate monitoring behind foreground turn completion', () => { - const tracker = new ClaudeBackgroundTaskTracker() + it('publishes an aggregate roster observed mid-turn', () => { + const tracker = trackerAt([100]) tracker.observe({ type: 'user' }, true) - tracker.observe( - aggregate([{ task_id: 'task-live', task_type: 'local_bash', description: 'command' }]) - ) - expect(tracker.state).toBeNull() - - expect(tracker.observe(result())).toBe(true) + expect( + tracker.observe( + aggregate([{ task_id: 'task-live', task_type: 'local_bash', description: 'command' }]) + ) + ).toBe(true) expect(tracker.state).toEqual({ state: 'monitoring', - tasks: [{ id: 'task-live', kind: 'command', description: 'command' }] + tasks: [ + { + id: 'task-live', + kind: 'command', + description: 'command', + state: 'working', + startedAt: 100 + } + ] }) }) it('ignores ambient SDK tasks and clears all liveness when the session ends', () => { - const tracker = new ClaudeBackgroundTaskTracker() + const tracker = trackerAt([100]) tracker.observe( system('task_started', { task_id: 'ambient', diff --git a/src/main/claude/claude-background-task-tracker.ts b/src/main/claude/claude-background-task-tracker.ts index de24b9a4fba8..68c7f4f11857 100644 --- a/src/main/claude/claude-background-task-tracker.ts +++ b/src/main/claude/claude-background-task-tracker.ts @@ -1,78 +1,54 @@ import type { AgentSessionBackgroundTask, + AgentSessionBackgroundTaskRunState, AgentSessionBackgroundTaskState } from '../../shared/agent-session-wire' +import { + classifyClaudeBackgroundTaskKind, + liveClaudeTaskRunState, + record, + taskDescription, + taskId, + taskName, + taskUsageTotalTokens, + terminalClaudeTaskRunState +} from './claude-background-task-frames' +import { + ClaudeSettledBackgroundTasks, + claudeBackgroundTaskDetail, + type TrackedClaudeBackgroundTask +} from './claude-settled-background-tasks' -const MAX_TRACKED_TASKS = 256 -const MAX_TASK_ID_LENGTH = 512 -const MAX_TASK_DESCRIPTION_LENGTH = 512 -const TERMINAL_TASK_STATES = new Set(['completed', 'failed', 'killed', 'stopped']) - -export type ClaudeBackgroundTaskKind = AgentSessionBackgroundTask['kind'] - -type TrackedTask = { - backgrounded: boolean - kind: ClaudeBackgroundTaskKind - description?: string -} - -function record(value: unknown): Record | null { - return typeof value === 'object' && value !== null ? (value as Record) : null -} - -/** The bound every task id shares, wherever it enters. An id the roster stores - * becomes a durable entry key, so a provisional one takes the same bound the - * announced path applies — an over-long id is rejected, never truncated. */ -export function isBoundedClaudeTaskId(value: string): boolean { - return value.length > 0 && value.length <= MAX_TASK_ID_LENGTH -} +// `claude-subagent-*` reads this channel through these names; the readers themselves +// live in the frames module so both consumers share one definition. +export { + classifyClaudeBackgroundTaskKind, + isBoundedClaudeTaskId, + taskDescription as claudeTaskDescription, + taskId as claudeTaskId +} from './claude-background-task-frames' +export type { ClaudeBackgroundTaskKind } from './claude-background-task-frames' -/** The task's canonical, resume-stable id. Shared with the subagent roster so - * both readers of this channel agree on what identifies a task. */ -export function claudeTaskId(message: Record): string | null { - const value = message.task_id - return typeof value === 'string' && isBoundedClaudeTaskId(value) ? value : null -} - -/** A task's human label, collapsed and bounded. */ -export function claudeTaskDescription(value: unknown): string | undefined { - if (typeof value !== 'string') { - return undefined - } - const trimmed = value.trim().replace(/\s+/g, ' ') - return trimmed.length > 0 ? trimmed.slice(0, MAX_TASK_DESCRIPTION_LENGTH) : undefined -} - -export function classifyClaudeBackgroundTaskKind(taskType: unknown): ClaudeBackgroundTaskKind { - switch (taskType) { - case 'local_agent': - return 'agent' - case 'local_workflow': - return 'workflow' - case 'local_bash': - return 'command' - case 'monitor': - return 'monitor' - default: - return 'unknown' - } -} +const MAX_TRACKED_TASKS = 256 export class ClaudeBackgroundTaskTracker { - private readonly tasks = new Map() + private readonly tasks = new Map() + private readonly retention = new ClaudeSettledBackgroundTasks() private readonly terminalTaskIds = new Set() private aggregateRosterObserved = false - private foregroundTurnActive = false private monitoring = false private publishedTasksFingerprint = '' + constructor(private readonly now: () => number = () => Date.now()) {} + get state(): AgentSessionBackgroundTaskState | null { if (!this.monitoring) { return null } return { state: 'monitoring', - tasks: this.backgroundTaskDetails() + tasks: this.backgroundTaskDetails(), + ...(this.retention.hasSettled ? { settledTasks: this.retention.settledDetails() } : {}) } } @@ -87,16 +63,14 @@ export class ClaudeBackgroundTaskTracker { } observe(message: Record, startsTurn = false): boolean { - if (startsTurn) { - this.foregroundTurnActive = true - } - if (message.type === 'result') { - this.foregroundTurnActive = false - } else if (message.type === 'system') { + // Background work publishes through a foreground turn: the strip stays + // honest mid-fan-out and the client alone decides when the idle-only + // monitoring label may speak. + if (message.type === 'system') { if (!this.observeSystemFrame(message) && !startsTurn) { return false } - } else if (!startsTurn) { + } else if (!startsTurn && message.type !== 'result') { return false } return this.refreshMonitoring() @@ -104,47 +78,51 @@ export class ClaudeBackgroundTaskTracker { clear(): boolean { this.tasks.clear() + this.retention.clear() this.terminalTaskIds.clear() this.aggregateRosterObserved = false - this.foregroundTurnActive = false return this.refreshMonitoring() } + private settle( + id: string, + state: AgentSessionBackgroundTaskRunState, + outcome: { totalTokens?: number } = {} + ): void { + this.retention.settle(id, state, outcome, this.tasks.get(id)) + this.finish(id) + } + private observeSystemFrame(message: Record): boolean { if (message.subtype === 'background_tasks_changed') { this.replaceAggregateRoster(message.tasks) return true } - const id = claudeTaskId(message) + const id = taskId(message) if (!id) { return false } if (message.subtype === 'task_notification') { - this.finish(id) + // The notification is affirmative terminal evidence even when its status + // field is unreadable — matching the liveness semantics this edge always had. + this.settle(id, terminalClaudeTaskRunState(message.status) ?? 'done', { + totalTokens: taskUsageTotalTokens(message) + }) return true } - if (message.subtype === 'task_updated') { - const patch = record(message.patch) - if (!patch) { - return false - } - if (TERMINAL_TASK_STATES.has(String(patch.status))) { - this.finish(id) - return true - } + if (message.subtype === 'task_progress') { + // Progress `description` is the current activity ("Running "), not + // the task's name — only usage (and a missing identity) may update. const existing = this.tasks.get(id) - if ( - (patch.is_backgrounded === true || claudeTaskDescription(patch.description)) && - (!this.aggregateRosterObserved || existing) - ) { - this.upsert(id, { - backgrounded: patch.is_backgrounded === true || existing?.backgrounded === true, - kind: existing?.kind ?? 'unknown', - description: claudeTaskDescription(patch.description) ?? existing?.description - }) - return true + const totalTokens = taskUsageTotalTokens(message) + if (!existing?.backgrounded || totalTokens === undefined) { + return false } - return false + this.tasks.set(id, { ...existing, totalTokens, name: existing.name ?? taskName(message) }) + return true + } + if (message.subtype === 'task_updated') { + return this.observeTaskUpdated(id, message) } if (message.subtype !== 'task_started' || this.terminalTaskIds.has(id)) { return false @@ -160,15 +138,55 @@ export class ClaudeBackgroundTaskTracker { this.upsert(id, { backgrounded: message.is_backgrounded === true || kind === 'workflow' || kind === 'monitor', kind, - description: claudeTaskDescription(message.description) + description: taskDescription(message.description), + name: taskName(message), + state: liveClaudeTaskRunState(message.status) ?? undefined, + startedAt: this.now() }) return true } + private observeTaskUpdated(id: string, message: Record): boolean { + const patch = record(message.patch) + if (!patch) { + return false + } + const settledState = terminalClaudeTaskRunState(patch.status) + if (settledState) { + this.settle(id, settledState) + return true + } + const existing = this.tasks.get(id) + // Classification is re-derived per transition: a later frame that reveals a + // real type moves the task between buckets instead of pinning first-seen. + const patchKind = + 'task_type' in patch ? classifyClaudeBackgroundTaskKind(patch.task_type) : undefined + const liveState = liveClaudeTaskRunState(patch.status) + const hasContent = + patch.is_backgrounded === true || + taskDescription(patch.description) !== undefined || + taskName(patch) !== undefined || + liveState !== null || + (patchKind !== undefined && patchKind !== 'unknown') + if (hasContent && (!this.aggregateRosterObserved || existing)) { + this.upsert(id, { + backgrounded: patch.is_backgrounded === true || existing?.backgrounded === true, + kind: patchKind ?? existing?.kind ?? 'unknown', + description: taskDescription(patch.description), + name: taskName(patch), + state: liveState ?? undefined, + startedAt: this.now() + }) + return true + } + return false + } + private replaceAggregateRoster(value: unknown): void { if (!Array.isArray(value)) { return } + const prior = new Map(this.tasks) this.aggregateRosterObserved = true this.tasks.clear() this.terminalTaskIds.clear() @@ -180,29 +198,33 @@ export class ClaudeBackgroundTaskTracker { if (!task || task.ambient === true) { continue } - const id = claudeTaskId(task) + const id = taskId(task) if (!id) { continue } + // An authoritative live roster supersedes an earlier terminal edge. + const retained = this.retention.resume(id) + const existing = prior.get(id) ?? retained + const kind = classifyClaudeBackgroundTaskKind(task.task_type) this.tasks.set(id, { backgrounded: true, - kind: classifyClaudeBackgroundTaskKind(task.task_type), - description: claudeTaskDescription(task.description) + kind: kind !== 'unknown' ? kind : (existing?.kind ?? 'unknown'), + description: taskDescription(task.description) ?? existing?.description, + name: taskName(task) ?? existing?.name, + state: liveClaudeTaskRunState(task.status) ?? existing?.state, + startedAt: existing?.startedAt ?? this.now(), + totalTokens: existing?.totalTokens }) } + for (const [id, task] of prior) { + if (task.backgrounded && !this.tasks.has(id)) { + this.retention.rememberRemoved(id, task) + } + } } - private upsert(id: string, task: TrackedTask): void { - const existing = this.tasks.get(id) - if (existing) { - this.tasks.set(id, { - backgrounded: existing.backgrounded || task.backgrounded, - kind: existing.kind === 'unknown' ? task.kind : existing.kind, - description: task.description ?? existing.description - }) - return - } - if (this.tasks.size >= MAX_TRACKED_TASKS) { + private upsert(id: string, task: TrackedClaudeBackgroundTask): void { + if (!this.tasks.has(id) && this.tasks.size >= MAX_TRACKED_TASKS) { let foregroundId: string | undefined for (const [candidateId, candidate] of this.tasks) { if (!candidate.backgrounded) { @@ -215,6 +237,20 @@ export class ClaudeBackgroundTaskTracker { } this.tasks.delete(foregroundId) } + const existing = this.tasks.get(id) ?? this.retention.resume(id) + this.terminalTaskIds.delete(id) + if (existing) { + this.tasks.set(id, { + backgrounded: existing.backgrounded || task.backgrounded, + kind: task.kind !== 'unknown' ? task.kind : existing.kind, + description: task.description ?? existing.description, + name: task.name ?? existing.name, + state: task.state ?? existing.state, + startedAt: existing.startedAt, + totalTokens: existing.totalTokens + }) + return + } this.tasks.set(id, task) } @@ -231,9 +267,12 @@ export class ClaudeBackgroundTaskTracker { } private refreshMonitoring(): boolean { - const details = this.foregroundTurnActive ? [] : this.backgroundTaskDetails() + const details = this.backgroundTaskDetails() + if (details.length === 0 && this.retention.hasSettled) { + this.retention.flushSettled() + } const next = details.length > 0 - const fingerprint = next ? JSON.stringify(details) : '' + const fingerprint = next ? JSON.stringify([details, this.retention.settledDetails()]) : '' if (next === this.monitoring && fingerprint === this.publishedTasksFingerprint) { return false } @@ -248,11 +287,7 @@ export class ClaudeBackgroundTaskTracker { if (!task.backgrounded) { continue } - details.push({ - id, - kind: task.kind, - ...(task.description ? { description: task.description } : {}) - }) + details.push(claudeBackgroundTaskDetail(id, task)) } return details } diff --git a/src/main/claude/claude-settled-background-tasks.ts b/src/main/claude/claude-settled-background-tasks.ts new file mode 100644 index 000000000000..91e975d07bb8 --- /dev/null +++ b/src/main/claude/claude-settled-background-tasks.ts @@ -0,0 +1,127 @@ +// Retention state for background tasks that have reached a terminal edge. +// +// The real producer settles a task in two steps inside one tick: +// `background_tasks_changed` arrives FIRST with the task already absent, then +// `task_updated` / `task_notification` carry the outcome. So the terminal edge +// must be able to settle a task the live roster no longer holds — that is what +// `rememberRemoved` preserves. A removal whose outcome frame never arrives +// simply vanishes: removed tasks are never rendered and never guessed into a +// finished state. + +import type { + AgentSessionBackgroundTask, + AgentSessionBackgroundTaskRunState +} from '../../shared/agent-session-wire' + +const MAX_RETAINED_TASKS = 256 + +export type TrackedClaudeBackgroundTask = { + backgrounded: boolean + kind: AgentSessionBackgroundTask['kind'] + description?: string + name?: string + state?: AgentSessionBackgroundTaskRunState + /** First-observed epoch ms; preserved across updates and roster replacement + * so clients can render elapsed and keep a stable first-seen sort. */ + startedAt: number + totalTokens?: number +} + +export function claudeBackgroundTaskDetail( + id: string, + task: TrackedClaudeBackgroundTask +): AgentSessionBackgroundTask { + return { + id, + kind: task.kind, + ...(task.description ? { description: task.description } : {}), + ...(task.name ? { name: task.name } : {}), + state: task.state ?? (task.kind === 'monitor' ? 'monitoring' : 'working'), + startedAt: task.startedAt, + ...(task.totalTokens !== undefined ? { totalTokens: task.totalTokens } : {}) + } +} + +function setBounded(map: Map, key: K, value: V): void { + map.delete(key) + map.set(key, value) + if (map.size > MAX_RETAINED_TASKS) { + const oldest = map.keys().next() + if (!oldest.done) { + map.delete(oldest.value) + } + } +} + +export class ClaudeSettledBackgroundTasks { + private readonly settled = new Map() + private readonly recentlyRemoved = new Map() + + /** An aggregate roster evicted a still-live backgrounded task; hold its + * details so the outcome frame trailing in the same tick can settle it. */ + rememberRemoved(id: string, task: TrackedClaudeBackgroundTask): void { + setBounded(this.recentlyRemoved, id, task) + } + + /** Terminal edge for `id`. `liveSource` is the live roster's entry when it + * still has one; otherwise the recently-removed copy is consumed. A second + * edge (updated, then notification) re-derives the settled state and can + * add the final usage the first edge lacked. */ + settle( + id: string, + state: AgentSessionBackgroundTaskRunState, + outcome: { totalTokens?: number }, + liveSource: TrackedClaudeBackgroundTask | undefined + ): void { + const source = liveSource ?? this.recentlyRemoved.get(id) + const already = this.settled.get(id) + if (source?.backgrounded) { + setBounded(this.settled, id, { + ...claudeBackgroundTaskDetail(id, { + ...source, + totalTokens: outcome.totalTokens ?? source.totalTokens + }), + state + }) + } else if (already) { + this.settled.set(id, { + ...already, + state, + ...(outcome.totalTokens !== undefined ? { totalTokens: outcome.totalTokens } : {}) + }) + } + this.recentlyRemoved.delete(id) + } + + /** Positive live evidence transfers identity back to the tracker, never the old outcome. */ + resume(id: string): TrackedClaudeBackgroundTask | undefined { + const settled = this.settled.get(id) + const removed = this.recentlyRemoved.get(id) + this.settled.delete(id) + this.recentlyRemoved.delete(id) + const source = settled ?? removed + if (!source || source.startedAt === undefined) { + return undefined + } + return { ...source, backgrounded: true, state: undefined, startedAt: source.startedAt } + } + + get hasSettled(): boolean { + return this.settled.size > 0 + } + + settledDetails(): AgentSessionBackgroundTask[] { + return [...this.settled.values()] + } + + /** Settled context only makes sense beside live work; the strip exits at the + * same instant it always has — when the last live task ends. */ + flushSettled(): void { + this.settled.clear() + } + + clear(): void { + this.settled.clear() + this.recentlyRemoved.clear() + } +} diff --git a/src/main/claude/claude-structured-session-close.test.ts b/src/main/claude/claude-structured-session-close.test.ts index 670c0daaf8cd..0de5049a71ce 100644 --- a/src/main/claude/claude-structured-session-close.test.ts +++ b/src/main/claude/claude-structured-session-close.test.ts @@ -55,7 +55,9 @@ describe('Claude published session close lifecycle', () => { expect(backgroundStates).toEqual([ { state: 'monitoring', - tasks: [{ id: 'background-1', kind: 'agent' }], + tasks: [ + { id: 'background-1', kind: 'agent', state: 'working', startedAt: expect.any(Number) } + ], supportsTaskStop: true } ]) @@ -74,7 +76,9 @@ describe('Claude published session close lifecycle', () => { expect(backgroundStates).toEqual([ { state: 'monitoring', - tasks: [{ id: 'background-1', kind: 'agent' }], + tasks: [ + { id: 'background-1', kind: 'agent', state: 'working', startedAt: expect.any(Number) } + ], supportsTaskStop: true }, null diff --git a/src/main/native-chat/agent-session-wire/provider-frame-activity.test.ts b/src/main/native-chat/agent-session-wire/provider-frame-activity.test.ts index ba2eba580626..c8860f227ccb 100644 --- a/src/main/native-chat/agent-session-wire/provider-frame-activity.test.ts +++ b/src/main/native-chat/agent-session-wire/provider-frame-activity.test.ts @@ -50,7 +50,9 @@ describe('provider frame activity', () => { expect(claudeProviderFrameActivity(kind, payload)).toBeNull() } // `requesting` holds for nearly the whole turn and says no more than the fallback. - expect(claudeProviderFrameActivity('message:system:status', { status: 'requesting' })).toBeNull() + expect( + claudeProviderFrameActivity('message:system:status', { status: 'requesting' }) + ).toBeNull() expect(claudeProviderFrameActivity('message:system:status', { status: 'compacting' })).toBe( 'Compacting the conversation' ) diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-background-task-channel.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-background-task-channel.ts index 4592dea26e99..6dd9db80e1df 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-background-task-channel.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-background-task-channel.ts @@ -21,7 +21,10 @@ export class StructuredAgentSessionBackgroundTaskChannel { private readonly requireSession: (sessionId: string) => StructuredAgentSessionHostSession, private readonly handoffStatus: ( sessionId: string - ) => Parameters[0]['handoff'] + ) => Parameters[0]['handoff'], + /** Task edges change the status summary too; the feed's equality check + * keeps a no-op re-projection from reaching subscribers. */ + private readonly onPublished: (sessionId: string) => void ) {} history(request: AgentSessionHistoryRequest): AgentSessionHistoryResult { @@ -53,6 +56,7 @@ export class StructuredAgentSessionBackgroundTaskChannel { const state = publishedState !== undefined ? publishedState : this.state(sessionId) if (session && state !== undefined) { this.subscribers.backgroundTasks(sessionId, state, session.fence) + this.onPublished(sessionId) } } diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-host.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-host.ts index 2df2fdf54124..358ca952b57c 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-host.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-host.ts @@ -53,7 +53,8 @@ import type { StructuredAgentSessionHostSession, StructuredAgentSessionReveal } from './structured-agent-session-host-types' -import { StructuredAgentSessionStatusFeed } from './structured-agent-session-status-feed' +import { createStructuredAgentSessionHostStatusFeed } from './structured-agent-session-status-feed' +import type { StructuredAgentSessionStatusSubscriber } from './structured-agent-session-status-feed' import { StructuredAgentSessionEventRecovery } from './structured-agent-session-event-recovery' import { StructuredAgentSessionBackgroundTaskChannel } from './structured-agent-session-background-task-channel' export type { StructuredAgentSessionHostDeps } from './structured-agent-session-host-types' @@ -64,11 +65,10 @@ export class StructuredAgentSessionHost { this ) private readonly sessions = new Map() - private readonly statusFeed = new StructuredAgentSessionStatusFeed({ + private readonly statusFeed = createStructuredAgentSessionHostStatusFeed({ sessions: this.sessions, - getRecord: (sessionId) => this.deps.store.getRecord(sessionId), now: () => this.now(), - onStatusChanged: (summary, options) => this.deps.onSessionStatusChanged?.(summary, options) + deps: () => this.deps }) private readonly subscribers = new AgentSessionSubscribers({ readCommands: (sessionId) => this.deps.adapter.readCommands?.(sessionId), @@ -91,7 +91,8 @@ export class StructuredAgentSessionHost { this.sessions, this.subscribers, (sessionId) => this.requireSession(sessionId), - (sessionId) => this.handoffs.status(sessionId) + (sessionId) => this.handoffs.status(sessionId), + (sessionId) => this.statusFeed.publish(sessionId) ) this.runtimeState = new StructuredAgentSessionHostRuntimeState( deps, @@ -345,7 +346,7 @@ export class StructuredAgentSessionHost { unsubscribe = (sessionId: string, id: string): void => this.subscribers.close(sessionId, id) /** Every session's projected status for session lists; unlike `subscribe`, retains nothing. */ - subscribeStatus: StructuredAgentSessionStatusFeed['subscribe'] = (subscriber) => + subscribeStatus = (subscriber: StructuredAgentSessionStatusSubscriber): (() => void) => this.statusFeed.subscribe(subscriber) private requireSession(sessionId: string): StructuredAgentSessionHostSession { diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-status-feed.test.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-status-feed.test.ts index 7bd537424301..5fbd58016d05 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-status-feed.test.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-status-feed.test.ts @@ -1,9 +1,12 @@ import { mkdtemp, rm } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' -import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { AgentSessionRecord } from '../../../shared/agent-session-record' -import type { AgentSessionStatusEvent } from '../../../shared/agent-session-wire' +import type { + AgentSessionBackgroundTask, + AgentSessionStatusEvent +} from '../../../shared/agent-session-wire' import { createClaudeJournalTranslator } from '../../claude/claude-structured-journal-translation' import { publishCodexTurnLifecycle } from '../../codex/codex-structured-journal-translation-turns' import { createDeferredStructuredAgentSessionEventSink } from './structured-agent-session-event-sink' @@ -74,11 +77,13 @@ function feedFor( { journal: Awaited>; hasProviderChild?: boolean; fence?: number } >, record: Partial | null = null, - onStatusChanged?: StructuredAgentSessionStatusFeedDeps['onStatusChanged'] + onStatusChanged?: StructuredAgentSessionStatusFeedDeps['onStatusChanged'], + readBackgroundTasks?: StructuredAgentSessionStatusFeedDeps['readBackgroundTasks'] ) { let now = 1_000 const feed = new StructuredAgentSessionStatusFeed({ ...(onStatusChanged ? { onStatusChanged } : {}), + ...(readBackgroundTasks ? { readBackgroundTasks } : {}), sessions: { get: (sessionId: string) => { const session = sessions.get(sessionId) @@ -621,6 +626,147 @@ describe('StructuredAgentSessionStatusFeed', () => { session: expect.objectContaining({ status: 'idle', latestPrompt: 'hello' }) }) }) + + it('reuses the journal projection across task progress and invalidates on journal changes', async () => { + const journal = await openJournal() + await journal.appendItem( + USER_IDENTITY, + { kind: 'message', role: 'user', blocks: [{ type: 'text', text: 'fan out' }] }, + { fence: 1 } + ) + await journal.appendItem( + TURN_IDENTITY, + { kind: 'status', text: 'Working', turnLifecycle: { turnId: 'turn-1', state: 'running' } }, + { fence: 1 } + ) + const snapshot = vi.spyOn(journal, 'snapshot') + let taskState: 'working' | 'waiting' = 'working' + const { feed, events } = feedFor(new Map([[SESSION, { journal }]]), null, undefined, () => ({ + state: 'monitoring', + tasks: [{ id: 'child', kind: 'agent', state: taskState }] + })) + for (let tick = 1; tick <= 100; tick++) { + taskState = tick % 2 === 1 ? 'waiting' : 'working' + feed.publish(SESSION) + } + expect(events).toHaveLength(101) + expect(snapshot).toHaveBeenCalledTimes(1) + expect(events.at(-1)).toMatchObject({ + type: 'status', + session: { status: 'working', backgroundTasks: [{ state: 'working' }] } + }) + await journal.appendTombstone(TURN_IDENTITY, { fence: 1 }) + feed.publish(SESSION) + expect(snapshot).toHaveBeenCalledTimes(2) + expect(events.at(-1)).toMatchObject({ type: 'status', session: { status: 'idle' } }) + }) + + it('invalidates cached status on unreadability and keeps record metadata live', async () => { + const journal = await openJournal() + await journal.appendItem( + USER_IDENTITY, + { kind: 'message', role: 'user', blocks: [{ type: 'text', text: 'hello' }] }, + { fence: 1 } + ) + const record = { options: { model: 'first-model' }, providerHandleChain: [] } + const { feed, events } = feedFor(new Map([[SESSION, { journal }]]), record) + record.options.model = 'second-model' + feed.publish(SESSION) + expect(events.at(-1)).toMatchObject({ + type: 'status', + session: { status: 'idle', model: 'second-model' } + }) + const readOnly = vi.spyOn(journal, 'isReadOnly', 'get').mockReturnValue(true) + feed.publish(SESSION) + expect(events.at(-1)).toMatchObject({ type: 'status', session: { status: null } }) + readOnly.mockRestore() + feed.publish(SESSION) + expect(events.at(-1)).toMatchObject({ type: 'status', session: { status: 'idle' } }) + }) + + it('projects live background tasks and republishes a task-only state change', async () => { + const journal = await openJournal() + let tasks = [ + { id: 'task-1', kind: 'agent' as const, name: 'deep_review', state: 'working' as const } + ] + const { feed, events } = feedFor(new Map([[SESSION, { journal }]]), null, undefined, () => ({ + state: 'monitoring', + tasks + })) + await journal.appendItem( + USER_IDENTITY, + { kind: 'message', role: 'user', blocks: [{ type: 'text', text: 'fan out' }] }, + { fence: 1 } + ) + feed.publish(SESSION, journal) + expect(events.at(-1)).toEqual({ + type: 'status', + session: expect.objectContaining({ + backgroundTasks: [{ id: 'task-1', kind: 'agent', name: 'deep_review', state: 'working' }] + }) + }) + + // No journal change: only the task state moved. + tasks = [{ id: 'task-1', kind: 'agent', name: 'deep_review', state: 'waiting' as never }] + const before = events.length + feed.publish(SESSION, journal) + expect(events).toHaveLength(before + 1) + expect(events.at(-1)).toEqual({ + type: 'status', + session: expect.objectContaining({ + backgroundTasks: [expect.objectContaining({ state: 'waiting' })] + }) + }) + + // An identical projection is suppressed. + feed.publish(SESSION, journal) + expect(events).toHaveLength(before + 1) + }) + + it('omits task usage so a progress tick never re-broadcasts the summary', async () => { + const journal = await openJournal() + let tasks: AgentSessionBackgroundTask[] = [ + { id: 'task-1', kind: 'agent', name: 'deep_review', state: 'working', totalTokens: 10 } + ] + const { feed, events } = feedFor(new Map([[SESSION, { journal }]]), null, undefined, () => ({ + state: 'monitoring', + tasks + })) + await journal.appendItem( + USER_IDENTITY, + { kind: 'message', role: 'user', blocks: [{ type: 'text', text: 'fan out' }] }, + { fence: 1 } + ) + feed.publish(SESSION, journal) + const before = events.length + + // A `task_progress` frame moves only usage, which no status-summary reader renders; + // re-broadcasting the whole summary per frame would cost every remote subscriber. + tasks = [ + { id: 'task-1', kind: 'agent', name: 'deep_review', state: 'working', totalTokens: 4_200 } + ] + feed.publish(SESSION, journal) + expect(events).toHaveLength(before) + expect(events.at(-1)).toEqual({ + type: 'status', + session: expect.objectContaining({ + backgroundTasks: [{ id: 'task-1', kind: 'agent', name: 'deep_review', state: 'working' }] + }) + }) + + // A state change on the same task still reaches subscribers. + tasks = [ + { id: 'task-1', kind: 'agent', name: 'deep_review', state: 'waiting', totalTokens: 4_200 } + ] + feed.publish(SESSION, journal) + expect(events).toHaveLength(before + 1) + expect(events.at(-1)).toEqual({ + type: 'status', + session: expect.objectContaining({ + backgroundTasks: [expect.objectContaining({ state: 'waiting' })] + }) + }) + }) }) /** diff --git a/src/main/native-chat/agent-session-wire/structured-agent-session-status-feed.ts b/src/main/native-chat/agent-session-wire/structured-agent-session-status-feed.ts index 3c4ce4b97a63..bddb9ae93ce7 100644 --- a/src/main/native-chat/agent-session-wire/structured-agent-session-status-feed.ts +++ b/src/main/native-chat/agent-session-wire/structured-agent-session-status-feed.ts @@ -14,9 +14,11 @@ import { agentProviderSessionsEqual } from '../../../shared/agent-session-resume import type { AgentSessionRecord } from '../../../shared/agent-session-record' import { normalizeOptionalField } from '../../../shared/agent-status-field-normalization' import { AGENT_MODEL_MAX_LENGTH } from '../../../shared/agent-status-types' -import type { - AgentSessionStatusEvent, - AgentSessionStatusSummary +import { + agentSessionBackgroundTasksEqual, + type AgentSessionBackgroundTaskState, + type AgentSessionStatusEvent, + type AgentSessionStatusSummary } from '../../../shared/agent-session-wire' import { projectStructuredAgentSessionStatusSummary } from '../../../shared/structured-agent-session-projection' import type { AgentSessionJournal } from '../agent-session-journal/journal-store' @@ -41,6 +43,9 @@ export type StructuredAgentSessionStatusFeedDeps = { /** Every projection change, whether or not anyone is subscribed. `replay` marks a re-projection * of state the host already knew (restore, an arriving subscriber) rather than a journal edge. */ onStatusChanged?: (summary: AgentSessionStatusSummary, options: { replay: boolean }) => void + /** Live provider-owned background tasks for the summary, so session lists can + * render subagent children. Optional: a provider without the hook projects none. */ + readBackgroundTasks?: (sessionId: string) => AgentSessionBackgroundTaskState | null | undefined } function summariesEqual(a: AgentSessionStatusSummary, b: AgentSessionStatusSummary): boolean { @@ -57,13 +62,50 @@ function summariesEqual(a: AgentSessionStatusSummary, b: AgentSessionStatusSumma a.toolName === b.toolName && a.toolInput === b.toolInput && a.lastAssistantMessage === b.lastAssistantMessage && + agentSessionBackgroundTasksEqual(a.backgroundTasks, b.backgroundTasks) && agentProviderSessionsEqual(undefined, a.providerSession, b.providerSession) ) } +/** Wire the host's own deps into a feed; keeps the host at one call site. + * `deps` is a thunk because the host builds the feed in a field initializer, + * before its constructor parameters are assigned. */ +export function createStructuredAgentSessionHostStatusFeed(args: { + sessions: StructuredAgentSessionStatusFeedDeps['sessions'] + now: () => number + deps: () => { + store: { getRecord: (sessionId: string) => AgentSessionRecord | null } + adapter: { + backgroundTaskState?: ( + sessionId: string + ) => AgentSessionBackgroundTaskState | null | undefined + } + onSessionStatusChanged?: StructuredAgentSessionStatusFeedDeps['onStatusChanged'] + } +}): StructuredAgentSessionStatusFeed { + return new StructuredAgentSessionStatusFeed({ + sessions: args.sessions, + getRecord: (sessionId) => args.deps().store.getRecord(sessionId), + now: args.now, + onStatusChanged: (summary, options) => args.deps().onSessionStatusChanged?.(summary, options), + readBackgroundTasks: (sessionId) => args.deps().adapter.backgroundTaskState?.(sessionId) + }) +} + export class StructuredAgentSessionStatusFeed { private readonly subscribers = new Map() private readonly published = new Map() + // Task progress must not sort and scan an unchanged conversation. Journal identity owns cleanup. + private readonly journalProjections = new WeakMap< + AgentSessionJournal, + { + epoch: string + sequence: number + readOnly: boolean + fence: number | undefined + summary: ReturnType + } + >() constructor(private readonly deps: StructuredAgentSessionStatusFeedDeps) {} @@ -154,24 +196,54 @@ export class StructuredAgentSessionStatusFeed { journal: AgentSessionJournal ): AgentSessionStatusSummary { // An unreadable journal projects as "no turn": the chat itself shows the reset. - const snapshot = journal.isReadOnly ? null : journal.snapshot() - const items = snapshot?.items ?? [] - const submissions = snapshot?.submissions ?? [] + const cursor = journal.cursor() + const readOnly = journal.isReadOnly + const fence = session.fence + let projection = this.journalProjections.get(journal) + if ( + !projection || + projection.epoch !== cursor.epoch || + projection.sequence !== cursor.sequence || + projection.readOnly !== readOnly || + projection.fence !== fence + ) { + // A journalled submission bumps `lastSequence`, so the send-time working + // signal reaches the cache; the lease fence does not, hence the extra key. + const snapshot = readOnly ? null : journal.snapshot() + projection = { + ...cursor, + readOnly, + fence, + summary: projectStructuredAgentSessionStatusSummary( + snapshot?.items ?? [], + snapshot?.submissions ?? [], + fence + ) + } + this.journalProjections.set(journal, projection) + } const record = this.deps.getRecord(sessionId) const providerSession = structuredAgentSessionProviderSessionMetadata(record) // The journal has no model: the record's acknowledged options are where an owner // handoff or a mid-session switch lands, so the row follows whichever is in force. const model = normalizeOptionalField(record?.options?.model, AGENT_MODEL_MAX_LENGTH) + // Usage is dropped here on purpose: a `task_progress` tick would otherwise fail the + // equality check and re-broadcast a full summary to every remote subscriber for a + // number no session list renders. Tokens stay live on the background-task channel. + const backgroundTasks = this.deps + .readBackgroundTasks?.(sessionId) + ?.tasks?.map(({ totalTokens: _totalTokens, ...task }) => task) return { sessionId, workspaceId: session.params.location.workspaceId, agent: session.params.provider, ...(session.hasProviderChild ? { hostExecutionOwned: true as const } : {}), - ...projectStructuredAgentSessionStatusSummary(items, submissions, session.fence), + ...projection.summary, ...(record?.rewind?.phase === 'prepared' || record?.rewind?.phase === 'provider-succeeded' ? { rewindBlockedReason: 'outcome-unknown' as const } : {}), ...(model ? { model } : {}), + ...(backgroundTasks && backgroundTasks.length > 0 ? { backgroundTasks } : {}), ...(providerSession ? { providerSession } : {}), updatedAt: journal.lastActivityAt() || this.deps.now() } diff --git a/src/main/observability/local-file-sink-memory.test.ts b/src/main/observability/local-file-sink-memory.test.ts index fef5b8d00dd5..2c6644694b97 100644 --- a/src/main/observability/local-file-sink-memory.test.ts +++ b/src/main/observability/local-file-sink-memory.test.ts @@ -2,11 +2,7 @@ import { mkdtempSync, readFileSync, rmSync, statSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { - createLocalFileSink, - DROPPED_RECORD_TYPE, - type LocalFileSink -} from './local-file-sink' +import { createLocalFileSink, DROPPED_RECORD_TYPE, type LocalFileSink } from './local-file-sink' function parseLine(raw: string): Record { return JSON.parse(raw) as Record diff --git a/src/main/runtime/orca-runtime-tests/worktree-ps-structured-host.spec.ts b/src/main/runtime/orca-runtime-tests/worktree-ps-structured-host.spec.ts index 58e7de1b6746..5779c65eddac 100644 --- a/src/main/runtime/orca-runtime-tests/worktree-ps-structured-host.spec.ts +++ b/src/main/runtime/orca-runtime-tests/worktree-ps-structured-host.spec.ts @@ -53,6 +53,8 @@ function journalWith(prompt: string): AgentSessionJournal { return { isReadOnly: false, lastActivityAt: () => OBSERVED_AT, + // This journal never changes, so a real one would hold its cursor steady. + cursor: () => ({ epoch: 1, sequence: 1 }), snapshot: () => ({ items: runningTurn(prompt) }) } as unknown as AgentSessionJournal } diff --git a/src/main/runtime/rpc/methods/structured-agent-session-rpc.test-fixture.ts b/src/main/runtime/rpc/methods/structured-agent-session-rpc.test-fixture.ts index a702bda5afc8..f9f60ded887d 100644 --- a/src/main/runtime/rpc/methods/structured-agent-session-rpc.test-fixture.ts +++ b/src/main/runtime/rpc/methods/structured-agent-session-rpc.test-fixture.ts @@ -97,6 +97,7 @@ function statusFeed(): StructuredAgentSessionStatusFeed { { journal: { isReadOnly: false, + cursor: () => ({ epoch: 'epoch-status', sequence: 2 }), lastActivityAt: () => 2, snapshot: () => ({ items: STATUS_ITEMS }) } as unknown as AgentSessionJournal, diff --git a/src/renderer/src/components/native-chat/NativeChatBackgroundTasksStatus.test.tsx b/src/renderer/src/components/native-chat/NativeChatBackgroundTasksStatus.test.tsx index e137220dd667..96be91820a37 100644 --- a/src/renderer/src/components/native-chat/NativeChatBackgroundTasksStatus.test.tsx +++ b/src/renderer/src/components/native-chat/NativeChatBackgroundTasksStatus.test.tsx @@ -2,12 +2,16 @@ import '@testing-library/jest-dom/vitest' -import { cleanup, fireEvent, render, screen } from '@testing-library/react' +import { act, cleanup, fireEvent, render, screen, within } from '@testing-library/react' +import { Profiler } from 'react' import { afterEach, describe, expect, it, vi } from 'vitest' import type { AgentSessionBackgroundTask } from '../../../../shared/agent-session-wire' import { NativeChatBackgroundTasksStatus } from './NativeChatBackgroundTasksStatus' -afterEach(cleanup) +afterEach(() => { + cleanup() + vi.useRealTimers() +}) const TASKS: AgentSessionBackgroundTask[] = [ { id: 'codex-agent:child-1', kind: 'agent', description: 'count_a' }, @@ -20,7 +24,10 @@ function renderStrip(props: { supportsTaskStop: boolean; supportsStopAll: boolea const onStop = vi.fn() render( { expect(screen.getByText('sleep 90')).toBeInTheDocument() }) }) + +describe('background-tasks strip header', () => { + function renderHeader(tasks: AgentSessionBackgroundTask[]): HTMLElement { + render( + {}} + /> + ) + return screen.getByRole('button', { expanded: false }) + } + + it('leads each kind segment with that kind icon and keeps the counts in the accessible name', () => { + const header = renderHeader([ + { id: 'a1', kind: 'agent' }, + { id: 'a2', kind: 'agent' }, + { id: 'a3', kind: 'agent' }, + { id: 'm1', kind: 'monitor' } + ]) + expect(header).toHaveAttribute('aria-label', '3 agents · 1 monitor') + expect(header.querySelector('.lucide-bot')).toBeInTheDocument() + // Heartbeat, the same glyph the agent sidebar shows for monitoring. + expect(header.querySelector('.lucide-activity')).toBeInTheDocument() + // Two kind icons and the chevron: the aggregate state dot is gone. + expect(header.querySelectorAll('svg')).toHaveLength(3) + for (const icon of header.querySelectorAll('svg')) { + expect(icon).toHaveAttribute('aria-hidden', 'true') + } + }) + + it('gives the monitor heartbeat the sidebar amber and leaves other kinds neutral', () => { + const header = renderHeader([ + { id: 'a1', kind: 'agent' }, + { id: 'm1', kind: 'monitor' } + ]) + // Same glyph AND same colour as AgentStateDot/StatusIndicator, or a monitor + // here does not read as the monitor there. + expect(header.querySelector('.lucide-activity')?.classList).toContain('text-yellow-500') + expect(header.querySelector('.lucide-bot')?.classList).toContain('text-muted-foreground') + expect(header.querySelector('.lucide-bot')?.classList).not.toContain('text-yellow-500') + }) + + it('dims the monitor amber while a turn owns the voice', () => { + render( + {}} + /> + ) + const header = screen.getByRole('button', { expanded: false }) + expect(header.querySelector('.lucide-activity')?.classList).toContain('text-yellow-500/40') + }) + + it('carries the monitor amber on the expanded row too', () => { + const header = renderHeader([ + { id: 'm1', kind: 'monitor', description: 'watcher' }, + { id: 'c1', kind: 'command', description: 'sleep 90' } + ]) + fireEvent.click(header) + // Each kind group is its own labelled list, so scope to the monitor one. + const monitors = screen.getByRole('list', { name: 'Monitors' }) + expect(monitors.querySelector('.lucide-activity')?.classList).toContain('text-yellow-500') + const shell = screen.getByRole('list', { name: 'Shell' }) + expect(shell.querySelector('.lucide-square-terminal')?.classList).toContain( + 'text-muted-foreground' + ) + }) + + it('draws the segment separator in a visible text tone, not the divider token', () => { + const header = renderHeader([ + { id: 'a1', kind: 'agent' }, + { id: 'c1', kind: 'command' } + ]) + const separators = [...header.querySelectorAll('span')].filter( + (element) => element.textContent === ' · ' + ) + expect(separators).toHaveLength(1) + // `--border` is a divider line (7% white in dark), an order of magnitude + // fainter than the counts it sits between. + expect(separators[0].classList).not.toContain('text-border') + expect(separators[0].classList).toContain('text-muted-foreground') + // One space either side; the icon's own margin is the icon-to-label gap. + expect(header.textContent).toBe('1 agent · 1 shell') + }) + + it('carries no icon on a collapsed total, which spans kinds', () => { + const header = renderHeader([ + { id: 'a1', kind: 'agent' }, + { id: 'c1', kind: 'command' }, + { id: 'm1', kind: 'monitor' }, + { id: 'w1', kind: 'workflow' } + ]) + expect(header).toHaveAttribute('aria-label', '4 background tasks') + expect(header.querySelectorAll('svg')).toHaveLength(1) + }) +}) + +describe('settled rows beside their live siblings', () => { + // Retention is the PR's headline: a finished child stays visible, keeps the + // usage it ended on, and stops claiming a clock or a stop control. + it('keeps a settled row with its final usage, no clock and no stop', () => { + render( + {}} + /> + ) + fireEvent.click(screen.getByRole('button', { expanded: false })) + const agents = screen.getByRole('list', { name: 'Agents' }) + const rows = within(agents).getAllByRole('listitem') + expect(rows).toHaveLength(2) + // First seen first: the settled sibling started earlier. + expect(rows[0].textContent).toBe('settled child18.1k') + expect(rows[1].textContent).toMatch(/^live child4\.1k · .+Stop$/) + expect(within(rows[1]).getByRole('button', { name: 'Stop live child' })).toBeInTheDocument() + expect(within(rows[0]).queryByRole('button')).toBeNull() + }) +}) + +describe('background-task row reasons', () => { + function expandedRows(tasks: AgentSessionBackgroundTask[]): HTMLElement[] { + render( + {}} + /> + ) + fireEvent.click(screen.getByRole('button', { expanded: false })) + return screen.getAllByRole('listitem') + } + + // `unverifiable` is the SSH verdict for "no contact"; a row that hides it reads + // like a working child. `blocked` is the same class of loss. + it('names the reason on every attention state, not only on waiting', () => { + const rows = expandedRows([ + { id: 'a1', kind: 'agent', description: 'ssh child', state: 'unverifiable' }, + { id: 'a2', kind: 'agent', description: 'flaky child', state: 'blocked' }, + { id: 'a3', kind: 'agent', description: 'approval child', state: 'waiting' }, + { id: 'a4', kind: 'agent', description: 'busy child', state: 'working' } + ]) + expect(rows).toHaveLength(4) + expect(rows[0].textContent).toContain('ssh child · no contact') + expect(rows[1].textContent).toContain('flaky child · failed') + expect(rows[2].textContent).toContain('approval child · needs approval') + // A running row has nothing to explain. + expect(rows[3].textContent).not.toContain('·') + }) +}) + +it('stops elapsed renders in a hidden pane and catches up on reveal', () => { + vi.useFakeTimers() + vi.setSystemTime(100_000) + const committed = vi.fn() + const view = (isVisible: boolean) => ( + + {}} + /> + + ) + const { rerender, unmount } = render(view(true)) + committed.mockClear() + act(() => vi.advanceTimersByTime(1_000)) + expect(committed).toHaveBeenCalled() + rerender(view(false)) + committed.mockClear() + act(() => vi.advanceTimersByTime(10_000)) + expect(committed).not.toHaveBeenCalled() + rerender(view(true)) + committed.mockClear() + act(() => vi.advanceTimersByTime(1_000)) + expect(committed).toHaveBeenCalled() + unmount() + expect(vi.getTimerCount()).toBe(0) +}) diff --git a/src/renderer/src/components/native-chat/NativeChatBackgroundTasksStatus.tsx b/src/renderer/src/components/native-chat/NativeChatBackgroundTasksStatus.tsx index c52b73842a9a..21b8d73f2581 100644 --- a/src/renderer/src/components/native-chat/NativeChatBackgroundTasksStatus.tsx +++ b/src/renderer/src/components/native-chat/NativeChatBackgroundTasksStatus.tsx @@ -1,62 +1,207 @@ -import { useId, useState } from 'react' -import { ChevronDown } from 'lucide-react' +import { useEffect, useId, useMemo, useRef, useState } from 'react' +import { Activity, Bot, ChevronDown, CircleHelp, SquareTerminal, Workflow } from 'lucide-react' import type { AgentSessionBackgroundTask } from '../../../../shared/agent-session-wire' import { AgentStateDot } from '@/components/AgentStateDot' import { Button } from '@/components/ui/button' +import { useNow } from '@/hooks/use-now' import { translate } from '@/i18n/i18n' +import { backgroundTasksHeaderContent } from './background-task-header-content' +import { + backgroundTaskElapsedLabel, + backgroundTaskGroupLabel, + backgroundTaskStateReason, + buildBackgroundTaskGroups, + formatBackgroundTaskTokens, + type BackgroundRosterTask +} from './background-task-roster' -function backgroundTaskLabel(task: AgentSessionBackgroundTask): string { - if (task.description) { - return task.description - } - switch (task.kind) { - case 'agent': - return translate('components.native-chat.backgroundTasks.agent', 'Background agent') - case 'workflow': - return translate('components.native-chat.backgroundTasks.workflow', 'Background workflow') - case 'command': - return translate('components.native-chat.backgroundTasks.command', 'Background command') - case 'monitor': - return translate('components.native-chat.backgroundTasks.monitor', 'Background monitor') - case 'unknown': - return translate('components.native-chat.backgroundTasks.task', 'Background task') - } +/** Below this strip width (border-box, live root font size) the header drops + * its per-kind breakdown for an honest total. A narrow split pane on a wide + * monitor must behave like a narrow window, so no viewport media query. */ +const NARROW_STRIP_REM = 24 + +function rootFontSizePx(): number { + const parsed = Number.parseFloat(getComputedStyle(document.documentElement).fontSize) + return Number.isFinite(parsed) && parsed > 0 ? parsed : 16 +} + +/** Observe the strip's own border-box width; the viewport is only the + * pre-measurement stand-in before the first observer callback. */ +function useNarrowStrip(ref: React.RefObject): boolean { + const [narrow, setNarrow] = useState(() => window.innerWidth < NARROW_STRIP_REM * 16) + useEffect(() => { + const element = ref.current + if (!element || typeof ResizeObserver === 'undefined') { + return + } + const observer = new ResizeObserver((observerEntries) => { + const width = + observerEntries[0]?.borderBoxSize?.[0]?.inlineSize ?? element.getBoundingClientRect().width + setNarrow(width < NARROW_STRIP_REM * rootFontSizePx()) + }) + observer.observe(element, { box: 'border-box' }) + return () => observer.disconnect() + }, [ref]) + return narrow +} + +const KIND_ICONS = { + agent: Bot, + command: SquareTerminal, + monitor: Activity, + workflow: Workflow, + unknown: CircleHelp +} as const + +/** Monitoring is a STATE the app colours the same on every surface — the agent + * sidebar and `AgentStateDot` both draw an amber heartbeat — so the strip must + * match it or the two stop reading as the same thing. The other four are plain + * kind markers and stay neutral. `dimmed` is the running-turn treatment. */ +function kindIconTone(kind: AgentSessionBackgroundTask['kind'], dimmed: boolean): string { + const tone = kind === 'monitor' ? 'text-yellow-500' : 'text-muted-foreground' + return dimmed ? `${tone}/40` : tone +} + +function BackgroundTaskRow(props: { + entry: BackgroundRosterTask + now: number + supportsTaskStop: boolean + stopping: boolean + onStop: (taskId: string) => void +}): React.JSX.Element { + const { entry, now } = props + const Icon = KIND_ICONS[entry.task.kind] + // Every attention state states its reason on the row, the same ones the collapsed + // header names; `unverifiable` ("no contact") must never be silently dropped. + const reason = backgroundTaskStateReason(entry.state) + // Settled rows keep their final usage but no elapsed — a still-growing clock + // on finished work would lie. + const meta = [ + entry.task.totalTokens !== undefined + ? formatBackgroundTaskTokens(entry.task.totalTokens) + : null, + entry.settled ? null : backgroundTaskElapsedLabel(entry.task, now) + ] + .filter((part): part is string => part !== null) + .join(' · ') + return ( +
  • +
  • + ) } export function NativeChatBackgroundTasksStatus(props: { tasks: readonly AgentSessionBackgroundTask[] + settledTasks: readonly AgentSessionBackgroundTask[] supportsTaskStop: boolean /** False when the provider exposes no honest stop at all; the fallback * control is hidden rather than offering a button that cannot act. */ supportsStopAll: boolean stoppingTaskIds: ReadonlySet stoppingAll: boolean + /** True while the session is idle: only then may the strip speak as the + * animated monitoring indicator. A running turn owns the voice. */ + indicatorActive: boolean + isVisible: boolean onStop: (taskId?: string) => void }): React.JSX.Element { const [expanded, setExpanded] = useState(false) const taskListId = useId() + const stripRef = useRef(null) + const narrow = useNarrowStrip(stripRef) + // The 1 Hz elapsed tick must not re-group, re-sort and re-translate the whole roster. + const groups = useMemo( + () => buildBackgroundTaskGroups(props.tasks, props.settledTasks), + [props.tasks, props.settledTasks] + ) + const singleLiveCommand = + groups.length === 1 && groups[0].kind === 'command' && groups[0].tasks.length === 1 + const hasElapsed = groups.some((group) => + group.tasks.some((entry) => !entry.settled && (entry.task.startedAt ?? 0) > 0) + ) + const now = useNow(1_000, props.isVisible && hasElapsed && (expanded || singleLiveCommand)) + const header = backgroundTasksHeaderContent(groups, { narrow, now }) + const headerText = `${header.segments.map((segment) => segment.text).join(' · ')}${header.detail ? `${header.segments.length > 0 ? ' — ' : ''}${header.detail}` : ''}` return (
    -
    +
    - ) : null} - - ) - })} - + ))} + +
    + )) ) : (

    {translate( @@ -119,7 +250,7 @@ export function NativeChatBackgroundTasksStatus(props: {

    )} {!props.supportsTaskStop && props.supportsStopAll ? ( -
    0 ? 'mt-2 border-t border-border pt-2' : 'mt-2'}> +
    0 ? 'mt-2 border-t border-border pt-2' : 'mt-2'}>