diff --git a/.github/workflows/audio-capture-prebuilds.yml b/.github/workflows/audio-capture-prebuilds.yml index 65f4ec4184..4716ca541d 100644 --- a/.github/workflows/audio-capture-prebuilds.yml +++ b/.github/workflows/audio-capture-prebuilds.yml @@ -50,7 +50,7 @@ jobs: runner: 'windows-2022' arch: 'x64' steps: - - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + - uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 - uses: 'actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e' # v6.4.0 with: node-version: '22' diff --git a/.github/workflows/build-and-publish-image.yml b/.github/workflows/build-and-publish-image.yml index b682771c75..0c0daba602 100644 --- a/.github/workflows/build-and-publish-image.yml +++ b/.github/workflows/build-and-publish-image.yml @@ -28,7 +28,7 @@ jobs: steps: - name: 'Checkout repository' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: ref: '${{ github.ref }}' diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7b2ec4a01d..5d40f391a9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -136,7 +136,7 @@ jobs: - name: 'Checkout' id: 'checkout' if: "${{ needs.classify_pr.outputs.skip_ci != 'true' }}" - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: ref: "${{ github.event.inputs.branch_ref || (github.event_name == 'pull_request' && format('refs/pull/{0}/head', github.event.pull_request.number)) || (github.event_name == 'merge_group' && github.event.merge_group.head_sha) || github.ref }}" # Shallow: nothing here walks git history (the verify guard below checks @@ -386,7 +386,7 @@ jobs: - name: 'Checkout' id: 'checkout' if: "${{ needs.classify_pr.outputs.skip_ci != 'true' }}" - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: ref: "${{ github.event.inputs.branch_ref || (github.event_name == 'pull_request' && format('refs/pull/{0}/head', github.event.pull_request.number)) || (github.event_name == 'merge_group' && github.event.merge_group.head_sha) || github.ref }}" @@ -441,7 +441,7 @@ jobs: - name: 'Checkout' id: 'checkout' if: "${{ needs.classify_pr.outputs.skip_ci != 'true' }}" - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: ref: "${{ github.event.inputs.branch_ref || (github.event_name == 'pull_request' && format('refs/pull/{0}/head', github.event.pull_request.number)) || (github.event_name == 'merge_group' && github.event.merge_group.head_sha) || github.ref }}" @@ -510,7 +510,7 @@ jobs: - '22.x' steps: - name: 'Checkout' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 - name: 'Download coverage reports artifact' uses: 'actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c' # v8.0.1 @@ -557,7 +557,7 @@ jobs: OPENAI_MODEL: '${{ secrets.OPENAI_MODEL }}' steps: - name: 'Checkout' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: ref: "${{ github.event.inputs.branch_ref || (github.event_name == 'merge_group' && github.event.merge_group.head_sha) || github.ref }}" # Shallow, mirroring the Ubuntu gate: nothing here walks git history, diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index ded8856643..6b4c33c8fb 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -30,7 +30,7 @@ jobs: timeout-minutes: 30 steps: - name: 'Checkout' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 - name: 'Initialize CodeQL' uses: 'github/codeql-action/init@df559355d593797519d70b90fc8edd5db049e7a2' # ratchet:github/codeql-action/init@v3 diff --git a/.github/workflows/desktop-release.yml b/.github/workflows/desktop-release.yml index c730fbede1..1f8fe3f0b7 100644 --- a/.github/workflows/desktop-release.yml +++ b/.github/workflows/desktop-release.yml @@ -76,7 +76,7 @@ jobs: steps: - name: 'Check out source' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: fetch-depth: 0 @@ -234,7 +234,7 @@ jobs: steps: - name: 'Check out source' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: ref: '${{ needs.release_metadata.outputs.release_ref }}' diff --git a/.github/workflows/docs-page-action.yml b/.github/workflows/docs-page-action.yml index 2a10ba62d9..0d2352285f 100644 --- a/.github/workflows/docs-page-action.yml +++ b/.github/workflows/docs-page-action.yml @@ -24,7 +24,7 @@ jobs: runs-on: 'ubuntu-latest' steps: - name: 'Checkout' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 - name: 'Setup Pages' uses: 'actions/configure-pages@45bfe0192ca1faeb007ade9deae92b16b8254a0d' # ratchet:actions/configure-pages@v6 diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index d6d13addb0..c5afbf1d38 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -44,7 +44,7 @@ jobs: - '22.x' steps: - name: 'Checkout' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 - name: 'Set up Node.js ${{ matrix.node-version }}' uses: 'actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e' # v6.4.0 @@ -109,7 +109,7 @@ jobs: ${{ github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository }} steps: - name: 'Checkout' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 - name: 'Set up Node.js' uses: 'actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e' # v6.4.0 diff --git a/.github/workflows/gemini-scheduled-pr-triage.yml b/.github/workflows/gemini-scheduled-pr-triage.yml index bc6830c2e0..77119672cf 100644 --- a/.github/workflows/gemini-scheduled-pr-triage.yml +++ b/.github/workflows/gemini-scheduled-pr-triage.yml @@ -20,7 +20,7 @@ jobs: prs_needing_comment: '${{ steps.run_triage.outputs.prs_needing_comment }}' steps: - name: 'Checkout' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 - name: 'Run PR Triage Script' id: 'run_triage' diff --git a/.github/workflows/qwen-autofix.yml b/.github/workflows/qwen-autofix.yml index 851bbd62c6..861a537c48 100644 --- a/.github/workflows/qwen-autofix.yml +++ b/.github/workflows/qwen-autofix.yml @@ -215,7 +215,7 @@ jobs: AUTOFIX_ISSUE_EXCLUDES: 'no:assignee -linked:pr -label:autofix/skip -label:autofix/in-progress -label:status/need-information -label:status/need-retesting sort:created-desc' steps: - name: 'Checkout' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: fetch-depth: 0 persist-credentials: false @@ -997,7 +997,7 @@ jobs: WATERMARK: '${{ matrix.target.watermark }}' steps: - name: 'Checkout' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: fetch-depth: 0 persist-credentials: false @@ -1281,7 +1281,7 @@ jobs: if git rev-parse --verify "${BRANCH}" > /dev/null 2>&1; then git diff "origin/main...${BRANCH}" > "${WORKDIR}/pr.diff" || true fi - for f in feedback.md address-summary.md no-action.md failure.md pr.diff; do + for f in feedback.md address-summary.md no-action.md failure.md handoff.md pr.diff; do if [[ -f "${WORKDIR}/${f}" ]]; then echo "=============== ${f} ===============" cat "${WORKDIR}/${f}" @@ -1382,6 +1382,8 @@ jobs: OUTCOME: '${{ steps.verify.outputs.outcome }}' CONFLICT: '${{ steps.prepare.outputs.conflict }}' DRY_RUN: '${{ needs.route.outputs.dry_run }}' + GITHUB_TOKEN: '${{ secrets.CI_DEV_BOT_PAT }}' + NEWEST: '${{ steps.prepare.outputs.newest }}' run: |- SUFFIX='' [[ "${DRY_RUN}" == "true" ]] && SUFFIX=' (dry-run, nothing pushed)' @@ -1389,7 +1391,7 @@ jobs: echo "### PR #${PR} (issue #${ISSUE}) — outcome=${OUTCOME:-unknown}${SUFFIX}" echo "- Base conflict: ${CONFLICT:-unknown}" echo - for f in address-summary.md no-action.md failure.md; do + for f in address-summary.md no-action.md failure.md handoff.md; do if [[ -s "${WORKDIR}/${f}" ]]; then echo "**${f}:**" cat "${WORKDIR}/${f}" @@ -1397,3 +1399,33 @@ jobs: fi done } >> "${GITHUB_STEP_SUMMARY}" + + if [[ "${DRY_RUN}" != "true" && "${OUTCOME:-unknown}" == "failed" && -n "${NEWEST:-}" && -n "${GITHUB_TOKEN:-}" && -s "${WORKDIR}/handoff.md" ]]; then + api_error_file="$(mktemp)" + if ! bot_actor="$(GH_TOKEN="${GITHUB_TOKEN}" gh api user --jq '.login' 2>"${api_error_file}")"; then + api_error="$(tr '\r\n' ' ' < "${api_error_file}")" + rm -f "${api_error_file}" + echo "::error::Failed to verify CI_DEV_BOT_PAT identity with gh api user: ${api_error:-unknown error}." + exit 1 + fi + rm -f "${api_error_file}" + echo "CI_DEV_BOT_PAT authenticates as ${bot_actor}" + if [[ "${bot_actor}" != "${AUTOFIX_BOT}" ]]; then + echo "::error::CI_DEV_BOT_PAT authenticates as ${bot_actor}; expected ${AUTOFIX_BOT}." + exit 1 + fi + { + echo "🤖 Could not address the latest review feedback automatically." + echo + echo "The feedback was evaluated, but AutoFix failed before producing a verified commit. A human should take over this PR." + echo + echo "Run: ${GITHUB_SERVER_URL}/${GITHUB_REPOSITORY}/actions/runs/${GITHUB_RUN_ID}" + echo + echo "**Failure:**" + head -c 1500 "${WORKDIR}/failure.md" | sed 's///g' + echo + echo + echo "" + } > "${WORKDIR}/report.md" + gh pr comment "${PR}" --repo "${REPO}" --body-file "${WORKDIR}/report.md" || echo "::warning::Failed to post handoff comment on PR #${PR}" + fi diff --git a/.github/workflows/qwen-code-pr-review.yml b/.github/workflows/qwen-code-pr-review.yml index 7b7c4983eb..141772e813 100644 --- a/.github/workflows/qwen-code-pr-review.yml +++ b/.github/workflows/qwen-code-pr-review.yml @@ -387,7 +387,7 @@ jobs: # SECURITY: checkout trusted base code; /review fetches PR diff context. - name: 'Checkout base branch' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: ref: '${{ github.event.repository.default_branch }}' fetch-depth: 0 @@ -895,7 +895,7 @@ jobs: echo "pr_number=${pr_number}" >> "$GITHUB_OUTPUT" - name: 'Checkout base branch' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: ref: '${{ github.event.repository.default_branch }}' fetch-depth: 0 diff --git a/.github/workflows/qwen-pr-safety-precheck.yml b/.github/workflows/qwen-pr-safety-precheck.yml index 2bded93945..53222af08c 100644 --- a/.github/workflows/qwen-pr-safety-precheck.yml +++ b/.github/workflows/qwen-pr-safety-precheck.yml @@ -25,7 +25,7 @@ jobs: decision: '${{ steps.assess.outputs.decision }}' steps: - name: 'Checkout trusted precheck script' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: ref: '${{ github.event.repository.default_branch }}' sparse-checkout: '.github/scripts/pr-safety-precheck.mjs' diff --git a/.github/workflows/qwen-triage.yml b/.github/workflows/qwen-triage.yml index a3c59ce7ec..8329324e82 100644 --- a/.github/workflows/qwen-triage.yml +++ b/.github/workflows/qwen-triage.yml @@ -244,7 +244,7 @@ jobs: echo "stale agent state cleaned" - name: 'Checkout repo' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: token: '${{ secrets.GITHUB_TOKEN }}' @@ -442,7 +442,7 @@ jobs: - name: 'Checkout PR merge ref' if: "steps.pr.outputs.decision == 'run'" - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: # Untrusted PR code — keep the token out of .git/config. persist-credentials: false diff --git a/.github/workflows/release-sdk-python.yml b/.github/workflows/release-sdk-python.yml index 7b1e63c274..27a343515d 100644 --- a/.github/workflows/release-sdk-python.yml +++ b/.github/workflows/release-sdk-python.yml @@ -100,7 +100,7 @@ jobs: fi - name: 'Checkout' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: ref: '${{ github.event.inputs.ref || github.sha }}' fetch-depth: 0 diff --git a/.github/workflows/release-sdk.yml b/.github/workflows/release-sdk.yml index 2cf54c66ec..25564a3aab 100644 --- a/.github/workflows/release-sdk.yml +++ b/.github/workflows/release-sdk.yml @@ -64,7 +64,7 @@ jobs: steps: - name: 'Checkout' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: ref: '${{ github.event.inputs.ref || github.sha }}' fetch-depth: 0 diff --git a/.github/workflows/release-vscode-companion.yml b/.github/workflows/release-vscode-companion.yml index 5e38991711..d1fb8b01a7 100644 --- a/.github/workflows/release-vscode-companion.yml +++ b/.github/workflows/release-vscode-companion.yml @@ -60,7 +60,7 @@ jobs: steps: - name: 'Checkout' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: ref: '${{ github.event.release.tag_name || github.event.inputs.ref || github.sha }}' fetch-depth: 0 @@ -200,7 +200,7 @@ jobs: steps: - name: 'Checkout' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: ref: '${{ github.event.release.tag_name || github.event.inputs.ref || github.sha }}' fetch-depth: 0 @@ -285,7 +285,7 @@ jobs: steps: - name: 'Checkout' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: ref: '${{ github.event.release.tag_name || github.event.inputs.ref || github.sha }}' diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 8eb0687272..096b5850ed 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -57,7 +57,7 @@ jobs: steps: - name: 'Checkout' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: ref: '${{ github.event.inputs.ref || github.sha }}' fetch-depth: 0 @@ -154,7 +154,7 @@ jobs: steps: - name: 'Checkout' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: ref: '${{ github.event.inputs.ref || github.sha }}' fetch-depth: 0 @@ -212,7 +212,7 @@ jobs: steps: - name: 'Checkout' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: ref: '${{ github.event.inputs.ref || github.sha }}' fetch-depth: 0 @@ -259,7 +259,7 @@ jobs: steps: - name: 'Checkout' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: ref: '${{ github.event.inputs.ref || github.sha }}' fetch-depth: 0 @@ -347,7 +347,7 @@ jobs: steps: - name: 'Checkout' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: # Persist the bot PAT for release-branch pushes so downstream CI # workflows are triggered. diff --git a/.github/workflows/sdk-python.yml b/.github/workflows/sdk-python.yml index b3abf07e94..ce53710e7d 100644 --- a/.github/workflows/sdk-python.yml +++ b/.github/workflows/sdk-python.yml @@ -72,7 +72,7 @@ jobs: python-version: ['3.10', '3.11', '3.12'] steps: - name: 'Checkout' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 - name: 'Set up Python' uses: 'actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405' # v6.2.0 diff --git a/.github/workflows/stale.yml b/.github/workflows/stale.yml index f419cfe8db..bf3401e657 100644 --- a/.github/workflows/stale.yml +++ b/.github/workflows/stale.yml @@ -18,7 +18,7 @@ jobs: group: '${{ github.workflow }}-stale' cancel-in-progress: true steps: - - uses: 'actions/stale@b5d41d4e1d5dceea10e7104786b73624c18a190f' # v10.2.0 + - uses: 'actions/stale@eb5cf3af3ac0a1aa4c9c45633dd1ae542a27a899' # v10.3.0 with: repo-token: '${{ secrets.GITHUB_TOKEN }}' # Issues are intentionally disabled here; a separate policy will diff --git a/.github/workflows/sync-cua-driver-to-oss.yml b/.github/workflows/sync-cua-driver-to-oss.yml index 777fc57423..652c05dd3c 100644 --- a/.github/workflows/sync-cua-driver-to-oss.yml +++ b/.github/workflows/sync-cua-driver-to-oss.yml @@ -45,7 +45,7 @@ jobs: contents: 'read' steps: - name: 'Checkout' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 - name: 'Resolve cua-driver version' id: 'meta' diff --git a/.github/workflows/sync-release-to-oss.yml b/.github/workflows/sync-release-to-oss.yml index c2eee4c5fb..12dc1183c3 100644 --- a/.github/workflows/sync-release-to-oss.yml +++ b/.github/workflows/sync-release-to-oss.yml @@ -30,7 +30,7 @@ jobs: steps: - name: 'Checkout' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: ref: '${{ env.RELEASE_TAG }}' diff --git a/.github/workflows/terminal-bench.yml b/.github/workflows/terminal-bench.yml index 4897ad8aa5..e6daa65ecb 100644 --- a/.github/workflows/terminal-bench.yml +++ b/.github/workflows/terminal-bench.yml @@ -26,7 +26,7 @@ jobs: - 'swe-bench-astropy-1' steps: - name: 'Checkout' - uses: 'actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd' # v6.0.2 + uses: 'actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10' # v6.0.3 with: submodules: 'recursive' - name: 'Install uv and set the python version' diff --git a/.qwen/skills/autofix/scripts/run-agent.mjs b/.qwen/skills/autofix/scripts/run-agent.mjs index 5ea66360af..066445f111 100755 --- a/.qwen/skills/autofix/scripts/run-agent.mjs +++ b/.qwen/skills/autofix/scripts/run-agent.mjs @@ -1,7 +1,8 @@ #!/usr/bin/env node -import { spawnSync } from 'node:child_process'; +import { spawn } from 'node:child_process'; import { + createWriteStream, existsSync, mkdirSync, readFileSync, @@ -17,7 +18,7 @@ const skillPath = resolve( '..', 'SKILL.md', ); -const QWEN_TIMEOUT_MS = 50 * 60 * 1000; +const QWEN_TIMEOUT_MS = Number(process.env.QWEN_TIMEOUT_MS) || 50 * 60 * 1000; const specs = { 'assess-candidates': { inputs: ['candidates.json'], @@ -66,6 +67,87 @@ function writeFailure(workdir, message) { ); } +function writeHandoff(workdir, message) { + mkdirSync(workdir, { recursive: true }); + writeFileSync(file(workdir, 'handoff.md'), `${message}\n`); +} + +function isLoopGuardOutput(output) { + return ( + output.includes('turn_tool_call_cap') || + output.includes('Loop detection halted the run') + ); +} + +function killQwen(child, signal) { + try { + process.kill(-child.pid, signal); + } catch { + child.kill(signal); + } +} + +function runQwen(options, prompt) { + mkdirSync(options.workdir, { recursive: true }); + const log = createWriteStream(file(options.workdir, 'agent.log'), { + flags: 'w', + }); + log.on('error', () => {}); + let outputTail = ''; + let loopDetected = false; + let settled = false; + let timedOut = false; + let timer; + let killTimer; + + return new Promise((resolve) => { + const child = spawn(options.qwenBin, ['--yolo', '--prompt', prompt], { + stdio: ['inherit', 'pipe', 'pipe'], + detached: true, + }); + + const finish = (result) => { + if (settled) return; + settled = true; + clearTimeout(timer); + clearTimeout(killTimer); + const payload = { + ...result, + timedOut, + loopDetected: loopDetected || isLoopGuardOutput(outputTail), + }; + if (log.destroyed) { + resolve(payload); + } else { + log.end(() => resolve(payload)); + } + }; + + const record = (chunk, stream) => { + const text = chunk.toString('utf8'); + outputTail = (outputTail + text).slice(-20_000); + if (!loopDetected && isLoopGuardOutput(outputTail)) loopDetected = true; + log.write(chunk); + stream.write(chunk); + }; + + child.stdout.on('data', (chunk) => record(chunk, process.stdout)); + child.stderr.on('data', (chunk) => record(chunk, process.stderr)); + child.on('error', (error) => finish({ error, status: null, signal: null })); + child.on('close', (status, signal) => + finish({ error: null, status, signal }), + ); + + timer = setTimeout(() => { + timedOut = true; + killQwen(child, 'SIGTERM'); + killTimer = setTimeout(() => { + if (!settled) killQwen(child, 'SIGKILL'); + }, 10_000); + }, QWEN_TIMEOUT_MS); + }); +} + function promptFor(options, spec) { const skill = readFileSync(skillPath, 'utf8') .replace(/\r\n/g, '\n') @@ -123,22 +205,36 @@ if (missingInputs.length > 0) { ); } -const result = spawnSync(options.qwenBin, ['--yolo', '--prompt', prompt], { - stdio: 'inherit', - timeout: QWEN_TIMEOUT_MS, -}); +const result = await runQwen(options, prompt); if (result.error || result.signal || result.status !== 0) { const detail = result.error ? result.error.message - : result.signal - ? `signal ${result.signal}` - : `status ${String(result.status)}`; + : result.timedOut + ? `timeout (${QWEN_TIMEOUT_MS}ms)` + : result.signal + ? `signal ${result.signal}` + : `status ${String(result.status)}`; if (!existsSync(file(options.workdir, 'failure.md'))) { - writeFailure( - options.workdir, - `Qwen failed during ${options.mode}: ${detail}.`, - ); + if (result.loopDetected) { + writeFailure( + options.workdir, + `Qwen hit the tool-call loop guard during ${options.mode}. A human should take over this feedback batch.`, + ); + writeHandoff( + options.workdir, + 'Qwen hit the tool-call loop guard; a human should take over this feedback batch.', + ); + } else { + writeFailure( + options.workdir, + `Qwen failed during ${options.mode}: ${detail}.`, + ); + } } else { + writeHandoff( + options.workdir, + 'The agent wrote failure.md before qwen exited; a human should take over this feedback batch.', + ); console.error( `Qwen failed during ${options.mode}: ${detail}; preserving agent-written failure.md.`, ); @@ -148,6 +244,10 @@ if (result.error || result.signal || result.status !== 0) { if (existsSync(file(options.workdir, 'failure.md'))) { const content = readFileSync(file(options.workdir, 'failure.md'), 'utf8'); + writeHandoff( + options.workdir, + 'The agent wrote failure.md; a human should take over this feedback batch.', + ); console.error(`Autofix agent wrote failure.md:\n${content}`); process.exit(0); } diff --git a/packages/acp-bridge/src/bridge.test.ts b/packages/acp-bridge/src/bridge.test.ts index ee46a1082a..027f5859e3 100644 --- a/packages/acp-bridge/src/bridge.test.ts +++ b/packages/acp-bridge/src/bridge.test.ts @@ -40,7 +40,11 @@ import { WorkspaceMismatchError, } from './bridgeErrors.js'; import { MAX_WORKSPACE_PATH_LENGTH } from './workspacePaths.js'; -import { extractErrorMessage, extractErrorCode } from './bridge.js'; +import { + classifyTurnErrorKind, + extractErrorMessage, + extractErrorCode, +} from './bridge.js'; import { SERVE_STATUS_EXT_METHODS } from './status.js'; import type { ChannelFactory } from './channel.js'; import type { BridgeTelemetry } from './bridgeOptions.js'; @@ -3376,6 +3380,42 @@ describe('createAcpSessionBridge', () => { await bridge.shutdown(); }); + it('adds structured errorKind when a turn ends with terminated', async () => { + const handle = makeChannel({ + promptImpl: () => { + throw new RequestError(-32603, 'terminated'); + }, + }); + const bridge = makeBridge({ channelFactory: async () => handle.channel }); + const session = await bridge.spawnOrAttach({ workspaceCwd: WS_A }); + const abort = new AbortController(); + const iter = bridge.subscribeEvents(session.sessionId, { + signal: abort.signal, + }); + const turnError = (async () => { + for await (const event of iter) { + if (event.type === 'turn_error') return event; + } + throw new Error('turn_error was not published'); + })(); + + await expect( + bridge.sendPrompt(session.sessionId, { + sessionId: session.sessionId, + prompt: [{ type: 'text', text: 'stream may fail' }], + }), + ).rejects.toThrow('terminated'); + + const event = await turnError; + expect(event.data).toMatchObject({ + message: 'terminated', + errorKind: 'model_stream_interrupted', + }); + + abort.abort(); + await bridge.shutdown(); + }); + it('echoes user_message_chunk to ALL session subscribers (cross-client sync)', async () => { // Cross-client sync fix: a prompt sent by client A must be visible // to every SSE subscriber of the same session — not just the @@ -10287,6 +10327,24 @@ describe('extractErrorCode', () => { }); }); +describe('classifyTurnErrorKind', () => { + it('classifies bare terminated errors as model stream interruptions', () => { + expect(classifyTurnErrorKind('terminated')).toBe( + 'model_stream_interrupted', + ); + expect(classifyTurnErrorKind('Terminated')).toBe( + 'model_stream_interrupted', + ); + expect(classifyTurnErrorKind(' terminated ')).toBe( + 'model_stream_interrupted', + ); + }); + + it('leaves unrelated turn errors unclassified', () => { + expect(classifyTurnErrorKind('API rate limit exceeded')).toBeUndefined(); + }); +}); + // --------------------------------------------------------------------------- // §2.3 side-channel state layer: publish helpers + reconciliation + snapshot // --------------------------------------------------------------------------- diff --git a/packages/acp-bridge/src/bridge.ts b/packages/acp-bridge/src/bridge.ts index 70bfa8d115..decaf81c1a 100644 --- a/packages/acp-bridge/src/bridge.ts +++ b/packages/acp-bridge/src/bridge.ts @@ -912,6 +912,14 @@ export function extractErrorCode(err: unknown): string | undefined { return undefined; } +export function classifyTurnErrorKind( + message: string, +): 'model_stream_interrupted' | undefined { + return message.trim().toLowerCase() === 'terminated' + ? 'model_stream_interrupted' + : undefined; +} + function broadcastTurnError( entry: SessionEntry, sessionId: string, @@ -921,6 +929,16 @@ function broadcastTurnError( ): void { const message = extractErrorMessage(err); const code = extractErrorCode(err); + const errorKind = classifyTurnErrorKind(message); + if (errorKind) { + writeServeDebugLine( + `turn_error classified session=${JSON.stringify(sessionId)} ` + + `message=${JSON.stringify(message)} ` + + `errorKind=${JSON.stringify(errorKind)}` + + (code ? ` code=${JSON.stringify(code)}` : '') + + (promptId ? ` promptId=${JSON.stringify(promptId)}` : ''), + ); + } entry.retryAllowed = true; try { entry.events.publish({ @@ -929,6 +947,7 @@ function broadcastTurnError( sessionId, message, ...(code ? { code } : {}), + ...(errorKind ? { errorKind } : {}), ...(promptId ? { promptId } : {}), }, ...(originatorClientId ? { originatorClientId } : {}), diff --git a/packages/cli/src/acp-integration/service/filesystem.test.ts b/packages/cli/src/acp-integration/service/filesystem.test.ts index d53c512a45..ba31dc6de5 100644 --- a/packages/cli/src/acp-integration/service/filesystem.test.ts +++ b/packages/cli/src/acp-integration/service/filesystem.test.ts @@ -107,6 +107,39 @@ describe('AcpFileSystemService', () => { }); }); + it('converts core-only read params at the ACP boundary', async () => { + const mockResponse = { + content: 'slice', + _meta: { bom: false, encoding: 'utf-8' }, + }; + const client = { + readTextFile: vi.fn().mockResolvedValue(mockResponse), + } as unknown as AgentSideConnection; + const signal = new AbortController().signal; + + const svc = new AcpFileSystemService( + client, + 'session-1', + { readTextFile: true, writeTextFile: true }, + createFallback(), + ); + + await svc.readTextFile({ + path: '/some/file.txt', + line: 0, + limit: 5, + maxOutputBytes: 1024, + signal, + }); + + expect(client.readTextFile).toHaveBeenCalledWith({ + path: '/some/file.txt', + line: 1, + limit: 5, + sessionId: 'session-1', + }); + }); + it('converts RESOURCE_NOT_FOUND error to ENOENT', async () => { const resourceNotFoundError = { code: RESOURCE_NOT_FOUND_CODE, @@ -953,11 +986,20 @@ describe('AcpFileSystemService', () => { fallback, ); - const result = await svc.readTextFile({ path: '/some/file.txt' }); + const signal = new AbortController().signal; + const result = await svc.readTextFile({ + path: '/some/file.txt', + line: 0, + maxOutputBytes: 2048, + signal, + }); expect(result).toEqual(fallbackResponse); expect(fallback.readTextFile).toHaveBeenCalledWith({ path: '/some/file.txt', + line: 0, + maxOutputBytes: 2048, + signal, }); expect(client.readTextFile).not.toHaveBeenCalled(); }); diff --git a/packages/cli/src/acp-integration/service/filesystem.ts b/packages/cli/src/acp-integration/service/filesystem.ts index 95fb1d14b6..bcc010f716 100644 --- a/packages/cli/src/acp-integration/service/filesystem.ts +++ b/packages/cli/src/acp-integration/service/filesystem.ts @@ -13,6 +13,7 @@ import type { } from '@agentclientprotocol/sdk'; import { RequestError } from '@agentclientprotocol/sdk'; import type { + CoreReadTextFileRequest, FileSystemService, ReadTextFileResponse, } from '@qwen-code/qwen-code-core'; @@ -105,6 +106,29 @@ async function resolveRealPath(value: string): Promise { } } +function toAcpReadTextFileRequest( + params: CoreReadTextFileRequest, + sessionId: string, +): ReadTextFileRequest { + // `maxOutputBytes`, `signal`, and `stats` are core-local concerns that the + // current ACP schema cannot represent. Keep this boundary explicit if the + // schema grows. + const request: ReadTextFileRequest = { + path: params.path, + sessionId, + }; + if (params._meta !== undefined) { + request._meta = params._meta; + } + if (params.limit !== undefined) { + request.limit = params.limit; + } + if (params.line != null) { + request.line = params.line + 1; + } + return request; +} + export class AcpFileSystemService implements FileSystemService { constructor( private readonly connection: AgentSideConnection, @@ -115,7 +139,7 @@ export class AcpFileSystemService implements FileSystemService { ) {} async readTextFile( - params: Omit, + params: CoreReadTextFileRequest, ): Promise { if (!this.capabilities.readTextFile) { return this.fallback.readTextFile(params); @@ -123,10 +147,9 @@ export class AcpFileSystemService implements FileSystemService { let response: ReadTextFileResponse; try { - response = await this.connection.readTextFile({ - ...params, - sessionId: this.sessionId, - }); + response = await this.connection.readTextFile( + toAcpReadTextFileRequest(params, this.sessionId), + ); } catch (error) { const errorCode = getErrorCode(error); diff --git a/packages/cli/src/serve/fs/policy.ts b/packages/cli/src/serve/fs/policy.ts index 1cd7cfe5c3..0061b6cd98 100644 --- a/packages/cli/src/serve/fs/policy.ts +++ b/packages/cli/src/serve/fs/policy.ts @@ -17,12 +17,12 @@ import type { Intent, ResolvedPath } from './paths.js'; * typical source files, small enough that an SSE replay buffer * doesn't fill on a single read. * - * Files **above** this cap are refused with `file_too_large` rather - * than truncated — the underlying `readFileWithLineAndLimit` - * reads the whole file into memory before slicing lines, so soft - * truncation past the cap would still OOM the daemon. Files - * **at or below** the cap honor a tighter `opts.maxBytes` via - * post-decode truncation (`enforceReadSize`); that's where the + * Full-snapshot reads above this cap are refused with `file_too_large` + * rather than truncated. `readText` can serve explicit line windows + * from larger files, but the default read/edit contract still needs a + * bounded snapshot for hash stability, SSE buffering, and oldText + * matching. Files at or below the cap honor a tighter `opts.maxBytes` + * via post-decode truncation (`enforceReadSize`); that's where the * `meta.truncated = true` flag fires. * * `enforceReadBytesSize` (the `readBytes` gate) and `edit()` use the diff --git a/packages/cli/src/serve/fs/workspace-file-system.test.ts b/packages/cli/src/serve/fs/workspace-file-system.test.ts index 51222c23ed..a6f9fd73d0 100644 --- a/packages/cli/src/serve/fs/workspace-file-system.test.ts +++ b/packages/cli/src/serve/fs/workspace-file-system.test.ts @@ -735,6 +735,20 @@ describe('WorkspaceFileSystem - write/edit', () => { expect(after).toBe('foo=42\nbar=2\n'); }); + it('edit() preserves the tail of files larger than the default range cap', async () => { + const target = path.join(h.workspace, 'large-edit.txt'); + const tail = 'tail-marker\n'; + const content = `foo=1\n${'body\n'.repeat(6_000)}${tail}`; + await fsp.writeFile(target, content); + const r = await h.fs.resolve('large-edit.txt', 'edit'); + + const out = await h.fs.edit(r, 'foo=1', 'foo=42'); + + expect(out.writtenBytes).toBeGreaterThan(25_000); + const after = await fsp.readFile(target, 'utf-8'); + expect(after).toBe(`foo=42\n${'body\n'.repeat(6_000)}${tail}`); + }); + it('throws parse_error when oldText is not present', async () => { const target = path.join(h.workspace, 'c.txt'); await fsp.writeFile(target, 'abc'); diff --git a/packages/cli/src/ui/hooks/atCommandProcessor.test.ts b/packages/cli/src/ui/hooks/atCommandProcessor.test.ts index 4ead67f5a8..52437f6400 100644 --- a/packages/cli/src/ui/hooks/atCommandProcessor.test.ts +++ b/packages/cli/src/ui/hooks/atCommandProcessor.test.ts @@ -156,6 +156,40 @@ describe('handleAtCommand', () => { expect(result.toolDisplays![0].status).toBe(ToolCallStatus.Success); }); + it('should attach a truncated text file larger than 10MB', async () => { + const filePath = await createTestFile( + path.join(testRootDir, 'large.log'), + 'x'.repeat(11 * 1024 * 1024), + ); + + const result = await handleAtCommand({ + query: `@${filePath}`, + config: mockConfig, + onDebugMessage: mockOnDebugMessage, + messageId: 626, + signal: abortController.signal, + }); + + const processedText = Array.isArray(result.processedQuery) + ? result.processedQuery + .map((part) => + typeof part === 'string' + ? part + : 'text' in part + ? part.text + : JSON.stringify(part), + ) + .join('') + : ''; + + expect(processedText).toContain( + 'Showing lines 1-1 of at least 1 total lines', + ); + expect(processedText).toContain('... [truncated]'); + expect(result.shouldProceed).toBe(true); + expect(result.toolDisplays![0].status).toBe(ToolCallStatus.Success); + }); + it('should only allow actual temp directory paths outside the workspace', async () => { const tempParentDir = await fsPromises.mkdtemp( path.join(os.tmpdir(), 'at-command-temp-'), diff --git a/packages/core/src/services/chatRecordingService.autoTitle.test.ts b/packages/core/src/services/chatRecordingService.autoTitle.test.ts index 5061afe454..43de494d06 100644 --- a/packages/core/src/services/chatRecordingService.autoTitle.test.ts +++ b/packages/core/src/services/chatRecordingService.autoTitle.test.ts @@ -364,16 +364,14 @@ describe('ChatRecordingService - auto-title trigger', () => { expect(svc.getCurrentCustomTitle()).toBe('Auto-generated title'); expect(svc.getCurrentTitleSource()).toBe('auto'); - // finalize() was called by the constructor — drain the queued async - // write before inspecting the mock. + await svc.flush(); + expect(findCustomTitleRecord()).toBeUndefined(); + + svc.recordUserMessage([{ text: 'resume work' }]); await svc.flush(); - // The re-appended record must carry titleSource: 'auto', not 'manual'. - const finalizeRecord = vi - .mocked(jsonl.writeLine) - .mock.calls.map((c) => c[1] as ChatRecord) - .find((r) => r.type === 'system' && r.subtype === 'custom_title'); - expect(finalizeRecord?.systemPayload).toEqual({ + const reanchoredRecord = findCustomTitleRecord(); + expect(reanchoredRecord?.systemPayload).toEqual({ customTitle: 'Auto-generated title', titleSource: 'auto', }); @@ -404,12 +402,13 @@ describe('ChatRecordingService - auto-title trigger', () => { expect(svc.getCurrentCustomTitle()).toBe('User chose this'); expect(svc.getCurrentTitleSource()).toBe('manual'); await svc.flush(); + expect(findCustomTitleRecord()).toBeUndefined(); - const finalizeRecord = vi - .mocked(jsonl.writeLine) - .mock.calls.map((c) => c[1] as ChatRecord) - .find((r) => r.type === 'system' && r.subtype === 'custom_title'); - expect(finalizeRecord?.systemPayload).toEqual({ + svc.recordUserMessage([{ text: 'resume work' }]); + await svc.flush(); + + const reanchoredRecord = findCustomTitleRecord(); + expect(reanchoredRecord?.systemPayload).toEqual({ customTitle: 'User chose this', titleSource: 'manual', }); @@ -438,13 +437,14 @@ describe('ChatRecordingService - auto-title trigger', () => { // `titleSource: 'manual'` we can't actually verify. expect(svc.getCurrentTitleSource()).toBeUndefined(); await svc.flush(); + expect(findCustomTitleRecord()).toBeUndefined(); - const finalizeRecord = vi - .mocked(jsonl.writeLine) - .mock.calls.map((c) => c[1] as ChatRecord) - .find((r) => r.type === 'system' && r.subtype === 'custom_title'); + svc.recordUserMessage([{ text: 'resume work' }]); + await svc.flush(); + + const reanchoredRecord = findCustomTitleRecord(); // Payload must NOT contain a titleSource field when source is unknown. - expect(finalizeRecord?.systemPayload).toEqual({ + expect(reanchoredRecord?.systemPayload).toEqual({ customTitle: 'Legacy title', }); }); diff --git a/packages/core/src/services/chatRecordingService.customTitle.test.ts b/packages/core/src/services/chatRecordingService.customTitle.test.ts index f6a6357cd2..224d409818 100644 --- a/packages/core/src/services/chatRecordingService.customTitle.test.ts +++ b/packages/core/src/services/chatRecordingService.customTitle.test.ts @@ -135,8 +135,9 @@ describe('ChatRecordingService - recordCustomTitle', () => { }); describe('finalize', () => { - it('should re-append cached custom title to EOF', async () => { + it('should re-append cached custom title to EOF after new content', async () => { chatRecordingService.recordCustomTitle('my-feature'); + chatRecordingService.recordUserMessage([{ text: 'new work' }]); await chatRecordingService.flush(); vi.mocked(jsonl.writeLine).mockClear(); @@ -153,6 +154,17 @@ describe('ChatRecordingService - recordCustomTitle', () => { }); }); + it('should not write anything when the title is already the latest record', async () => { + chatRecordingService.recordCustomTitle('my-feature'); + await chatRecordingService.flush(); + vi.mocked(jsonl.writeLine).mockClear(); + + chatRecordingService.finalize(); + await chatRecordingService.flush(); + + expect(jsonl.writeLine).not.toHaveBeenCalled(); + }); + it('should not write anything when no custom title was set', async () => { chatRecordingService.finalize(); await chatRecordingService.flush(); @@ -160,9 +172,33 @@ describe('ChatRecordingService - recordCustomTitle', () => { expect(jsonl.writeLine).not.toHaveBeenCalled(); }); + it('should not re-append a resumed title without new content', async () => { + vi.mocked(mockConfig.getResumedSessionData).mockReturnValue({ + lastCompletedUuid: null, + } as unknown as ReturnType); + const getSessionTitleInfo = vi.fn().mockReturnValue({ + title: 'resumed-title', + source: 'manual', + }); + ( + mockConfig as unknown as { + getSessionService: () => { + getSessionTitleInfo: typeof getSessionTitleInfo; + }; + } + ).getSessionService = () => ({ getSessionTitleInfo }); + + const svc = new ChatRecordingService(mockConfig); + svc.finalize(); + await svc.flush(); + + expect(jsonl.writeLine).not.toHaveBeenCalled(); + }); + it('should re-append the latest title after multiple renames', async () => { chatRecordingService.recordCustomTitle('first-name'); chatRecordingService.recordCustomTitle('second-name'); + chatRecordingService.recordUserMessage([{ text: 'new work' }]); await chatRecordingService.flush(); vi.mocked(jsonl.writeLine).mockClear(); @@ -288,10 +324,8 @@ describe('ChatRecordingService - recordCustomTitle', () => { ).getSessionService = () => ({ getSessionTitleInfo }); const svc = new ChatRecordingService(mockConfig); - // Constructor's finalize re-appends a custom_title record on resume - // — clear it out so we can isolate the threshold-triggered re-anchor. await svc.flush(); - vi.mocked(jsonl.writeLine).mockClear(); + expect(jsonl.writeLine).not.toHaveBeenCalled(); const bulkText = 'x'.repeat(2000); for (let i = 0; i < 20; i++) { diff --git a/packages/core/src/services/chatRecordingService.ts b/packages/core/src/services/chatRecordingService.ts index 39c1c8a791..29b97dcfcd 100644 --- a/packages/core/src/services/chatRecordingService.ts +++ b/packages/core/src/services/chatRecordingService.ts @@ -564,6 +564,7 @@ export class ChatRecordingService { * returning undefined if the title is beyond both windows). */ private bytesSinceTitleAnchor = 0; + private hasNonTitleContentSinceTitleAnchor = false; constructor(config: Config) { this.config = config; @@ -577,16 +578,19 @@ export class ChatRecordingService { // resumed. Legacy records (no `titleSource` field) stay `undefined` — // treated as manual for safety without rewriting the JSONL. // - // We then re-append a custom_title record to EOF so the title stays - // within the tail window that readers scan (guarding against a crash - // before the next finalize). + // Do not re-append during construction: loading/resuming a session is a + // read operation from the user's perspective, and touching the JSONL mtime + // would make session lists treat it as fresh activity. if (config.getResumedSessionData()) { try { const sessionService = config.getSessionService(); const info = sessionService.getSessionTitleInfo(config.getSessionId()); this.currentCustomTitle = info.title; this.currentTitleSource = info.source; - this.finalize(); + if (info.title) { + // Prime the threshold so the first real content write re-anchors. + this.bytesSinceTitleAnchor = TITLE_REANCHOR_BYTES; + } } catch { // Best-effort — don't block construction } @@ -793,9 +797,11 @@ export class ChatRecordingService { private updateTitleAnchorTracking(record: ChatRecord): void { if (record.type === 'system' && record.subtype === 'custom_title') { this.bytesSinceTitleAnchor = 0; + this.hasNonTitleContentSinceTitleAnchor = false; return; } if (!this.currentCustomTitle) return; + this.hasNonTitleContentSinceTitleAnchor = true; // +1 for the trailing newline jsonl.writeLine appends. this.bytesSinceTitleAnchor += Buffer.byteLength(JSON.stringify(record), 'utf8') + 1; @@ -1319,13 +1325,10 @@ export class ChatRecordingService { } /** - * Finalizes the current session by re-appending cached metadata to EOF. - * - * Call this whenever leaving the current session — whether switching to - * another session, shutting down the process, or any other transition. - * This single entry point replaces scattered re-append calls and ensures - * the custom_title record stays within the last 64KB tail window that - * readSessionTitleFromFile() scans. + * Finalizes the current session by re-appending cached metadata to EOF, but + * only after this recorder has appended non-title content since the last + * title anchor. Pure load/resume must remain read-only so session lists do + * not treat restored sessions as newly active. * * Best-effort: errors are logged but never thrown. */ @@ -1344,6 +1347,9 @@ export class ChatRecordingService { if (!this.currentCustomTitle) { return; } + if (!this.hasNonTitleContentSinceTitleAnchor) { + return; + } try { const record: ChatRecord = { ...this.createBaseRecord('system'), diff --git a/packages/core/src/services/fileSystemService.test.ts b/packages/core/src/services/fileSystemService.test.ts index c89b40dff0..495f669662 100644 --- a/packages/core/src/services/fileSystemService.test.ts +++ b/packages/core/src/services/fileSystemService.test.ts @@ -87,7 +87,6 @@ describe('StandardFileSystemService', () => { expect(readFileWithLineAndLimit).toHaveBeenCalledWith({ path: '/test/file.txt', limit: Infinity, - line: 0, }); expect(result.content).toBe('Hello, World!'); expect(result._meta?.bom).toBe(false); @@ -116,6 +115,74 @@ describe('StandardFileSystemService', () => { expect(result._meta?.originalLineCount).toBe(100); }); + it('should preserve explicit line zero for offset reads', async () => { + vi.mocked(readFileWithLineAndLimit).mockResolvedValue({ + content: 'line 1', + bom: false, + encoding: 'utf-8', + originalLineCount: 100, + }); + + await fileSystem.readTextFile({ + path: '/test/file.txt', + line: 0, + }); + + expect(readFileWithLineAndLimit).toHaveBeenCalledWith({ + path: '/test/file.txt', + limit: Infinity, + line: 0, + }); + }); + + it('should pass maxOutputBytes and return byte-truncation metadata', async () => { + vi.mocked(readFileWithLineAndLimit).mockResolvedValue({ + content: 'partial', + bom: false, + encoding: 'utf-8', + originalLineCount: 100, + truncatedByBytes: true, + }); + + const result = await fileSystem.readTextFile({ + path: '/test/file.txt', + limit: 10, + line: 5, + maxOutputBytes: 128, + }); + + expect(readFileWithLineAndLimit).toHaveBeenCalledWith({ + path: '/test/file.txt', + limit: 10, + line: 5, + maxOutputBytes: 128, + }); + expect(result._meta?.truncatedByBytes).toBe(true); + }); + + it('should pass cached stats to readFileWithLineAndLimit', async () => { + const stats = { size: 123 } as import('node:fs').Stats; + vi.mocked(readFileWithLineAndLimit).mockResolvedValue({ + content: 'line 1', + bom: false, + encoding: 'utf-8', + originalLineCount: 1, + }); + + await fileSystem.readTextFile({ + path: '/test/file.txt', + maxOutputBytes: 128, + stats, + }); + + expect(readFileWithLineAndLimit).toHaveBeenCalledWith({ + path: '/test/file.txt', + limit: Infinity, + maxOutputBytes: 128, + stats, + }); + }); + it('should return encoding info for GBK file', async () => { vi.mocked(readFileWithLineAndLimit).mockResolvedValue({ content: '你好世界', diff --git a/packages/core/src/services/fileSystemService.ts b/packages/core/src/services/fileSystemService.ts index 4ecfd0ed7d..62189c7d36 100644 --- a/packages/core/src/services/fileSystemService.ts +++ b/packages/core/src/services/fileSystemService.ts @@ -5,6 +5,7 @@ */ import os from 'node:os'; +import type { Stats } from 'node:fs'; import * as path from 'node:path'; import { globSync } from 'glob'; import { atomicWriteFile } from '../utils/atomicFileWrite.js'; @@ -29,10 +30,26 @@ export type ReadTextFileResponse = { bom?: boolean; encoding?: string; originalLineCount?: number; + originalLineCountExact?: boolean; lineEnding?: LineEnding; + truncatedByBytes?: boolean; }; }; +export type CoreReadTextFileRequest = Omit< + ReadTextFileRequest, + 'sessionId' | 'line' +> & { + /** + * Core-local callers use 0-based line offsets. ACP protocol boundaries remain + * 1-based and convert explicitly before remote calls. + */ + line?: number | null; + maxOutputBytes?: number; + signal?: AbortSignal; + stats?: Stats; +}; + /** * Supported file encodings for new files. */ @@ -50,9 +67,7 @@ export type FileEncodingType = (typeof FileEncoding)[keyof typeof FileEncoding]; * Interface for file system operations that may be delegated to different implementations */ export interface FileSystemService { - readTextFile( - params: Omit, - ): Promise; + readTextFile(params: CoreReadTextFileRequest): Promise; writeTextFile( params: Omit, @@ -261,18 +276,32 @@ export function encodeTextFileContent( */ export class StandardFileSystemService implements FileSystemService { async readTextFile( - params: Omit, + params: CoreReadTextFileRequest, ): Promise { - const { path, limit, line } = params; - // Use encoding-aware reader that handles BOM and non-UTF-8 encodings (e.g. GBK) - const { content, bom, encoding, originalLineCount } = - await readFileWithLineAndLimit({ - path, - limit: limit ?? Number.POSITIVE_INFINITY, - line: line || 0, - }); - const lineEnding = detectLineEnding(content); - return { content, _meta: { bom, encoding, originalLineCount, lineEnding } }; + const { path, limit, line, maxOutputBytes, signal, stats } = params; + const readResult = await readFileWithLineAndLimit({ + path, + limit: limit ?? Number.POSITIVE_INFINITY, + ...(line !== undefined && line !== null ? { line } : {}), + ...(maxOutputBytes !== undefined ? { maxOutputBytes } : {}), + ...(signal !== undefined ? { signal } : {}), + ...(stats !== undefined ? { stats } : {}), + }); + const detectedLineEnding = + readResult.lineEnding ?? detectLineEnding(readResult.content); + return { + content: readResult.content, + _meta: { + bom: readResult.bom, + encoding: readResult.encoding, + originalLineCount: readResult.originalLineCount, + originalLineCountExact: readResult.originalLineCountExact, + lineEnding: detectedLineEnding, + ...(readResult.truncatedByBytes !== undefined + ? { truncatedByBytes: readResult.truncatedByBytes } + : {}), + }, + }; } async writeTextFile( diff --git a/packages/core/src/services/sessionRecap.test.ts b/packages/core/src/services/sessionRecap.test.ts new file mode 100644 index 0000000000..173d3059be --- /dev/null +++ b/packages/core/src/services/sessionRecap.test.ts @@ -0,0 +1,75 @@ +/** + * @license + * Copyright 2025 Qwen Code + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, expect, it, vi } from 'vitest'; +import type { Content } from '@google/genai'; +import type { Config } from '../config/config.js'; +import { + SYSTEM_REMINDER_CLOSE, + SYSTEM_REMINDER_OPEN, +} from '../utils/environmentContext.js'; +import { generateSessionRecap } from './sessionRecap.js'; + +const reminder = (body: string) => + `${SYSTEM_REMINDER_OPEN}\n${body}\n${SYSTEM_REMINDER_CLOSE}`; + +describe('generateSessionRecap', () => { + it('strips startup and mid-session system reminders from recap input', async () => { + const history: Content[] = [ + { + role: 'user', + parts: [ + { text: reminder('STARTUP_SKILL_LIST') }, + { text: 'fix session title pollution' }, + ], + }, + { role: 'model', parts: [{ text: 'I found the title service.' }] }, + { role: 'user', parts: [{ text: reminder('ADDED_MCP_TOOLS') }] }, + { + role: 'user', + parts: [ + { text: reminder('PLAN_MODE_REMINDER') }, + { + text: `continue with recap coverage\n${reminder('IDE_CONTEXT')}`, + }, + ], + }, + ]; + + let captured: Content[] | null = null; + const generateText = vi.fn(async (opts: { contents: Content[] }) => { + captured = opts.contents; + return { + text: 'Fixing session title pollution. Next: verify tests.', + usage: undefined, + }; + }); + const config = { + getFastModel: vi.fn(() => 'qwen-turbo'), + getModel: vi.fn(() => 'qwen-plus'), + getGeminiClient: vi.fn(() => ({ + getHistoryShallow: () => history, + })), + getBaseLlmClient: vi.fn(() => ({ generateText })), + getOutputLanguageFilePath: vi.fn(() => undefined), + } as unknown as Config; + + const result = await generateSessionRecap( + config, + new AbortController().signal, + ); + + expect(result).toBe('Fixing session title pollution. Next: verify tests.'); + expect(captured).not.toBeNull(); + const serialized = JSON.stringify(captured); + expect(serialized).not.toContain('STARTUP_SKILL_LIST'); + expect(serialized).not.toContain('ADDED_MCP_TOOLS'); + expect(serialized).not.toContain('PLAN_MODE_REMINDER'); + expect(serialized).not.toContain('IDE_CONTEXT'); + expect(serialized).toContain('fix session title pollution'); + expect(serialized).toContain('continue with recap coverage'); + }); +}); diff --git a/packages/core/src/services/sessionRecap.ts b/packages/core/src/services/sessionRecap.ts index 456dedb90e..21ea5ec637 100644 --- a/packages/core/src/services/sessionRecap.ts +++ b/packages/core/src/services/sessionRecap.ts @@ -4,9 +4,13 @@ * SPDX-License-Identifier: Apache-2.0 */ -import type { Content } from '@google/genai'; +import type { Content, Part } from '@google/genai'; import type { Config } from '../config/config.js'; import { createDebugLogger } from '../utils/debugLogger.js'; +import { + getStartupContextLength, + stripSystemReminderBlocks, +} from '../utils/environmentContext.js'; import { runSideQuery } from '../utils/sideQuery.js'; const debugLogger = createDebugLogger('SESSION_RECAP'); @@ -142,15 +146,22 @@ function extractRecap(raw: string): string { */ function filterToDialog(history: Content[]): Content[] { const out: Content[] = []; - for (const msg of history) { + for (const msg of history.slice(getStartupContextLength(history))) { if (msg.role !== 'user' && msg.role !== 'model') continue; - const textParts = (msg.parts ?? []).filter( - (part) => - typeof part?.text === 'string' && - part.text.trim() !== '' && - !part.thought && - !part.thoughtSignature, - ); + const textParts: Part[] = []; + for (const part of msg.parts ?? []) { + if ( + typeof part?.text !== 'string' || + part.text.trim() === '' || + part.thought || + part.thoughtSignature + ) { + continue; + } + const text = stripSystemReminderBlocks(part.text); + if (text.trim() === '') continue; + textParts.push({ ...part, text }); + } if (textParts.length === 0) continue; out.push({ role: msg.role, parts: textParts }); } diff --git a/packages/core/src/services/sessionTitle.test.ts b/packages/core/src/services/sessionTitle.test.ts index 2da674f754..393530d283 100644 --- a/packages/core/src/services/sessionTitle.test.ts +++ b/packages/core/src/services/sessionTitle.test.ts @@ -7,6 +7,10 @@ import { describe, expect, it, vi } from 'vitest'; import type { Content } from '@google/genai'; import type { Config } from '../config/config.js'; +import { + SYSTEM_REMINDER_CLOSE, + SYSTEM_REMINDER_OPEN, +} from '../utils/environmentContext.js'; import { sanitizeTitle, tryGenerateSessionTitle } from './sessionTitle.js'; interface MockOptions { @@ -50,6 +54,9 @@ const DIALOG_HISTORY: Content[] = [ }, ]; +const reminder = (body: string) => + `${SYSTEM_REMINDER_OPEN}\n${body}\n${SYSTEM_REMINDER_CLOSE}`; + describe('tryGenerateSessionTitle', () => { it('returns {ok:false, reason:"no_fast_model"} when fast model is absent', async () => { const { config } = makeConfig({ @@ -225,6 +232,112 @@ describe('tryGenerateSessionTitle', () => { expect(serialized).toContain('middleware stores tokens unsafely'); }); + it('skips startup context when building the title prompt', async () => { + const history: Content[] = [ + { role: 'user', parts: [{ text: reminder('STARTUP_SKILL_LIST') }] }, + { role: 'user', parts: [{ text: 'fix auto title for short prompts' }] }, + { role: 'model', parts: [{ text: 'I will inspect title generation.' }] }, + ]; + + let captured = ''; + const { config } = makeConfig({ + fastModel: 'qwen-turbo', + history, + generateJsonResult: async (opts: unknown) => { + captured = JSON.stringify((opts as { contents: Content[] }).contents); + return { title: 'Fix auto title prompts' }; + }, + }); + + await tryGenerateSessionTitle(config, new AbortController().signal); + + expect(captured).not.toContain('STARTUP_SKILL_LIST'); + expect(captured).toContain('fix auto title for short prompts'); + }); + + it('strips system-reminder parts while keeping the real user prompt', async () => { + const history: Content[] = [ + { role: 'user', parts: [{ text: 'start with titles' }] }, + { role: 'model', parts: [{ text: 'Ready.' }] }, + { + role: 'user', + parts: [ + { text: reminder('MID_SESSION_TOOL_METADATA') }, + { text: 'please add a regression test' }, + ], + }, + { role: 'model', parts: [{ text: 'I will add the test.' }] }, + ]; + + let captured = ''; + const { config } = makeConfig({ + fastModel: 'qwen-turbo', + history, + generateJsonResult: async (opts: unknown) => { + captured = JSON.stringify((opts as { contents: Content[] }).contents); + return { title: 'Add title regression test' }; + }, + }); + + await tryGenerateSessionTitle(config, new AbortController().signal); + + expect(captured).not.toContain('MID_SESSION_TOOL_METADATA'); + expect(captured).toContain('please add a regression test'); + }); + + it('drops pure system-reminder messages from the title prompt', async () => { + const history: Content[] = [ + { role: 'user', parts: [{ text: 'fix session titles' }] }, + { role: 'model', parts: [{ text: 'I found the title service.' }] }, + { role: 'user', parts: [{ text: reminder('ADDED_MCP_TOOLS') }] }, + ]; + + let captured = ''; + const { config } = makeConfig({ + fastModel: 'qwen-turbo', + history, + generateJsonResult: async (opts: unknown) => { + captured = JSON.stringify((opts as { contents: Content[] }).contents); + return { title: 'Fix session titles' }; + }, + }); + + await tryGenerateSessionTitle(config, new AbortController().signal); + + expect(captured).not.toContain('ADDED_MCP_TOOLS'); + expect(captured).toContain('fix session titles'); + }); + + it('strips IDE-merged system reminders from title prompt text', async () => { + const history: Content[] = [ + { role: 'user', parts: [{ text: 'inspect the selected file' }] }, + { role: 'model', parts: [{ text: 'I will inspect it.' }] }, + { + role: 'user', + parts: [ + { + text: `what does this function do?\n${reminder('IDE_SKILL_CONTEXT')}`, + }, + ], + }, + ]; + + let captured = ''; + const { config } = makeConfig({ + fastModel: 'qwen-turbo', + history, + generateJsonResult: async (opts: unknown) => { + captured = JSON.stringify((opts as { contents: Content[] }).contents); + return { title: 'Inspect selected function' }; + }, + }); + + await tryGenerateSessionTitle(config, new AbortController().signal); + + expect(captured).not.toContain('IDE_SKILL_CONTEXT'); + expect(captured).toContain('what does this function do?'); + }); + it('tail-slices conversations longer than 1000 characters', async () => { // A session that pivots mid-conversation — the final topic is what the // title should reflect. Feeding the head risks titling the session by diff --git a/packages/core/src/services/sessionTitle.ts b/packages/core/src/services/sessionTitle.ts index 0d2eb71554..db15bb2b50 100644 --- a/packages/core/src/services/sessionTitle.ts +++ b/packages/core/src/services/sessionTitle.ts @@ -4,9 +4,13 @@ * SPDX-License-Identifier: Apache-2.0 */ -import type { Content } from '@google/genai'; +import type { Content, Part } from '@google/genai'; import type { Config } from '../config/config.js'; import { createDebugLogger } from '../utils/debugLogger.js'; +import { + getStartupContextLength, + stripSystemReminderBlocks, +} from '../utils/environmentContext.js'; import { runSideQuery } from '../utils/sideQuery.js'; import { stripTerminalControlSequences } from '../utils/terminalSafe.js'; import { SESSION_TITLE_MAX_LENGTH } from './sessionService.js'; @@ -205,15 +209,22 @@ export function sanitizeTitle(s: string): string { */ function filterToDialog(history: Content[]): Content[] { const out: Content[] = []; - for (const msg of history) { + for (const msg of history.slice(getStartupContextLength(history))) { if (msg.role !== 'user' && msg.role !== 'model') continue; - const textParts = (msg.parts ?? []).filter( - (part) => - typeof part?.text === 'string' && - part.text.trim() !== '' && - !part.thought && - !part.thoughtSignature, - ); + const textParts: Part[] = []; + for (const part of msg.parts ?? []) { + if ( + typeof part?.text !== 'string' || + part.text.trim() === '' || + part.thought || + part.thoughtSignature + ) { + continue; + } + const text = stripSystemReminderBlocks(part.text); + if (text.trim() === '') continue; + textParts.push({ ...part, text }); + } if (textParts.length === 0) continue; out.push({ role: msg.role, parts: textParts }); } diff --git a/packages/core/src/tools/artifact/artifact-tool.test.ts b/packages/core/src/tools/artifact/artifact-tool.test.ts index 84b5ec8fad..9f68d4f151 100644 --- a/packages/core/src/tools/artifact/artifact-tool.test.ts +++ b/packages/core/src/tools/artifact/artifact-tool.test.ts @@ -134,6 +134,31 @@ describe('ArtifactTool', () => { expect(res.error?.type).toBe(ToolErrorType.FILE_NOT_FOUND); }); + it('forwards cancellation signals to source file reads', async () => { + const file = path.join(workdir, 'page.html'); + const controller = new AbortController(); + const readTextFile = vi.fn(async () => ({ content: '

x

' })); + const signalAwareTool = new ArtifactTool( + { + ...makeConfig(), + getFileSystemService: () => ({ readTextFile }), + } as unknown as Config, + { + kind: 'oss', + publish: async () => ({ id: 'x', url: 'https://h/x' }), + }, + openSpy as unknown as UrlOpener, + ); + + await signalAwareTool.build({ file_path: file }).execute(controller.signal); + + expect(readTextFile).toHaveBeenCalledWith({ + path: file, + maxOutputBytes: MAX_ARTIFACT_BYTES, + signal: controller.signal, + }); + }); + it('returns EXECUTION_FAILED when the publisher throws', async () => { const file = await writeFragment('page.html', '

x

'); const failingTool = new ArtifactTool( @@ -154,11 +179,22 @@ describe('ArtifactTool', () => { expect(openSpy).not.toHaveBeenCalled(); }); - it('enforces the size cap', async () => { + it('rejects source fragments that exceed the artifact byte budget', async () => { const big = '

' + 'a'.repeat(MAX_ARTIFACT_BYTES) + '

'; const file = await writeFragment('big.html', big); const res = await tool.build({ file_path: file }).execute(signal); expect(res.error?.type).toBe(ToolErrorType.FILE_TOO_LARGE); + expect(res.error?.message).toContain('source exceeds'); + expect(res.error?.message).toContain(`${MAX_ARTIFACT_BYTES} byte limit`); + expect(openSpy).not.toHaveBeenCalled(); + }); + + it('enforces the published artifact size cap', async () => { + const almostTooBig = 'a'.repeat(MAX_ARTIFACT_BYTES - 1); + const file = await writeFragment('wrapped-big.html', almostTooBig); + const res = await tool.build({ file_path: file }).execute(signal); + expect(res.error?.type).toBe(ToolErrorType.FILE_TOO_LARGE); + expect(res.error?.message).toContain('Artifact is too large'); expect(openSpy).not.toHaveBeenCalled(); }); diff --git a/packages/core/src/tools/artifact/artifact-tool.ts b/packages/core/src/tools/artifact/artifact-tool.ts index 5dce543c61..691a88295a 100644 --- a/packages/core/src/tools/artifact/artifact-tool.ts +++ b/packages/core/src/tools/artifact/artifact-tool.ts @@ -62,6 +62,14 @@ Set artifact.autoOpen=false in settings.json, or QWEN_ARTIFACT_NO_AUTO_OPEN=1, t const debugLogger = createDebugLogger('artifact'); +function cancelledArtifactResult(): ToolResult { + const message = 'Artifact publishing was cancelled.'; + return { + llmContent: message, + returnDisplay: message, + }; +} + class ArtifactToolInvocation extends BaseToolInvocation< ArtifactToolParams, ToolResult @@ -130,11 +138,26 @@ class ArtifactToolInvocation extends BaseToolInvocation< // Read the fragment the model wrote. let fragment: string; try { - const { content } = await this.config + const { content, _meta } = await this.config .getFileSystemService() - .readTextFile({ path: file_path }); + .readTextFile({ + path: file_path, + maxOutputBytes: MAX_ARTIFACT_BYTES, + signal, + }); + if (_meta?.truncatedByBytes === true) { + const message = `Artifact is too large (source exceeds the ${MAX_ARTIFACT_BYTES} byte limit). Trim the content or split it across multiple artifacts.`; + return { + llmContent: message, + returnDisplay: message, + error: { message, type: ToolErrorType.FILE_TOO_LARGE }, + }; + } fragment = content; } catch (err) { + if (signal.aborted || isAbortError(err)) { + return cancelledArtifactResult(); + } const notFound = isNodeError(err) && err.code === 'ENOENT'; const message = notFound ? `Artifact source file not found: ${file_path}. Write the page content to this file first.` @@ -193,11 +216,7 @@ class ArtifactToolInvocation extends BaseToolInvocation< // A user-initiated cancel (Esc / aborted signal) is not a failure — // surface it as a cancellation rather than a publish error. if (signal.aborted || isAbortError(err)) { - const message = 'Artifact publishing was cancelled.'; - return { - llmContent: message, - returnDisplay: message, - }; + return cancelledArtifactResult(); } const message = `Failed to publish artifact: ${getErrorMessage(err)}`; return { diff --git a/packages/core/src/tools/edit.test.ts b/packages/core/src/tools/edit.test.ts index 15fb31ddcb..2aed6e775a 100644 --- a/packages/core/src/tools/edit.test.ts +++ b/packages/core/src/tools/edit.test.ts @@ -1313,6 +1313,39 @@ describe('EditTool', () => { expect(fs.readFileSync(filePath, 'utf8')).toBe('X\nline b\nline c\n'); }); + it('allows editing a large text file after a ranged read', async () => { + const initialContent = [ + 'target', + 'context 1', + 'context 2', + 'context 3', + 'context 4', + 'x'.repeat(11 * 1024 * 1024), + ].join('\n'); + fs.writeFileSync(filePath, initialContent, 'utf8'); + const stats = fs.statSync(filePath); + fileReadCache.recordRead(filePath, stats, { + full: false, + cacheable: true, + }); + (mockConfig.getApprovalMode as Mock).mockReturnValueOnce( + ApprovalMode.AUTO_EDIT, + ); + + const result = await tool + .build({ + file_path: filePath, + old_string: 'target', + new_string: 'updated', + }) + .execute(abortSignal); + + expect(result.error).toBeUndefined(); + expect(fs.readFileSync(filePath, 'utf8').startsWith('updated\n')).toBe( + true, + ); + }); + it('rejects an edit when the previous read was non-cacheable (binary / pdf / image)', async () => { // ReadFile records every successful read into the cache, // including binary / PDF / image reads that produce a diff --git a/packages/core/src/tools/monitor.test.ts b/packages/core/src/tools/monitor.test.ts index 177d5f3719..48839ae826 100644 --- a/packages/core/src/tools/monitor.test.ts +++ b/packages/core/src/tools/monitor.test.ts @@ -191,8 +191,15 @@ describe('MonitorTool', () => { let monitorRegistry: MonitorRegistry; let mockChild: ReturnType; let mockIsPathWithinWorkspace: ReturnType; + let originalPager: string | undefined; + let originalGitPager: string | undefined; beforeEach(() => { + originalPager = process.env['PAGER']; + originalGitPager = process.env['GIT_PAGER']; + delete process.env['PAGER']; + delete process.env['GIT_PAGER']; + vi.clearAllMocks(); mockOsPlatform.mockReturnValue('linux'); @@ -229,6 +236,18 @@ describe('MonitorTool', () => { afterEach(() => { monitorRegistry.abortAll(); + + if (originalPager === undefined) { + delete process.env['PAGER']; + } else { + process.env['PAGER'] = originalPager; + } + + if (originalGitPager === undefined) { + delete process.env['GIT_PAGER']; + } else { + process.env['GIT_PAGER'] = originalGitPager; + } }); // Helper to access protected validateToolParamValues @@ -682,7 +701,33 @@ describe('MonitorTool', () => { const spawnOptions = mockSpawn.mock.calls[0][2]; expect(spawnOptions.env['PAGER']).toBe('cat'); - expect(spawnOptions.env['GIT_PAGER']).toBe('cat'); + expect(spawnOptions.env['GIT_PAGER']).toBeUndefined(); + }); + + it('preserves inherited git pager values for spawned processes', async () => { + process.env['GIT_PAGER'] = 'delta'; + const invocation = createInvocation({ + command: 'git log --oneline', + }); + + await invocation.execute(new AbortController().signal); + + const spawnOptions = mockSpawn.mock.calls[0][2]; + expect(spawnOptions.env['PAGER']).toBe('cat'); + expect(spawnOptions.env['GIT_PAGER']).toBe('delta'); + }); + + it('does not inject Unix pager defaults into Windows monitor env when unset', async () => { + mockOsPlatform.mockReturnValue('win32'); + const invocation = createInvocation({ + command: 'tail -f /var/log/app.log', + }); + + await invocation.execute(new AbortController().signal); + + const spawnOptions = mockSpawn.mock.calls[0][2]; + expect(spawnOptions.env['PAGER']).toBe(''); + expect(spawnOptions.env['GIT_PAGER']).toBeUndefined(); }); it('propagates explicit pager configuration to spawned processes', async () => { @@ -697,7 +742,7 @@ describe('MonitorTool', () => { const spawnOptions = mockSpawn.mock.calls[0][2]; expect(spawnOptions.env['PAGER']).toBe('more'); - expect(spawnOptions.env['GIT_PAGER']).toBe('more'); + expect(spawnOptions.env['GIT_PAGER']).toBeUndefined(); }); it('does not spawn when the turn signal is already aborted', async () => { diff --git a/packages/core/src/tools/monitor.ts b/packages/core/src/tools/monitor.ts index 0e5c1a4b5c..181ac8d942 100644 --- a/packages/core/src/tools/monitor.ts +++ b/packages/core/src/tools/monitor.ts @@ -369,7 +369,7 @@ class MonitorToolInvocation extends BaseToolInvocation< QWEN_CODE: '1', TERM: 'dumb', // no color codes for streaming ...getShellPagerEnv(this.config.getShellExecutionConfig().pager, { - includeGitPager: true, + includeGitPager: false, platform: os.platform(), }), ...getShellContextEnvVars(), diff --git a/packages/core/src/tools/read-file.test.ts b/packages/core/src/tools/read-file.test.ts index 276e1744bf..9b126433a0 100644 --- a/packages/core/src/tools/read-file.test.ts +++ b/packages/core/src/tools/read-file.test.ts @@ -423,9 +423,8 @@ describe('ReadFileTool', () => { }); }); - it('should return error for a file that is too large', async () => { + it('should read and truncate a text file larger than 10MB', async () => { const filePath = path.join(tempRootDir, 'largefile.txt'); - // 11MB of content exceeds 10MB limit const largeContent = 'x'.repeat(11 * 1024 * 1024); await fsp.writeFile(filePath, largeContent, 'utf-8'); const params: ReadFileToolParams = { file_path: filePath }; @@ -435,10 +434,28 @@ describe('ReadFileTool', () => { >; const result = await invocation.execute(abortSignal); - expect(result).toHaveProperty('error'); - expect(result.error?.type).toBe(ToolErrorType.FILE_TOO_LARGE); - expect(result.error?.message).toContain( - 'File size exceeds the 10MB limit', + expect(result.error).toBeUndefined(); + expect(result.returnDisplay).toBe( + 'Read lines 1-1 of at least 1 from largefile.txt (truncated)', + ); + expect(result.llmContent).toContain( + 'Showing lines 1-1 of at least 1 total lines', + ); + expect(result.llmContent).toContain('... [truncated]'); + }); + + it('should propagate an aborted signal before reading', async () => { + const filePath = path.join(tempRootDir, 'abort.txt'); + await fsp.writeFile(filePath, 'content', 'utf-8'); + const invocation = tool.build({ file_path: filePath }) as ToolInvocation< + ReadFileToolParams, + ToolResult + >; + const controller = new AbortController(); + controller.abort(); + + await expect(invocation.execute(controller.signal)).rejects.toThrow( + /abort/i, ); }); diff --git a/packages/core/src/tools/read-file.ts b/packages/core/src/tools/read-file.ts index eca595532a..c4ccc77b69 100644 --- a/packages/core/src/tools/read-file.ts +++ b/packages/core/src/tools/read-file.ts @@ -132,7 +132,7 @@ class ReadFileToolInvocation extends BaseToolInvocation< return 'ask'; } - async execute(): Promise { + async execute(signal: AbortSignal): Promise { const absPath = path.resolve(this.params.file_path); const projectRoot = this.config.getTargetDir(); // Auto-memory files (AGENTS.md and friends under the auto-memory @@ -206,6 +206,7 @@ class ReadFileToolInvocation extends BaseToolInvocation< offset: this.params.offset, limit: this.params.limit, pages: this.params.pages, + signal, }, ); @@ -284,7 +285,9 @@ class ReadFileToolInvocation extends BaseToolInvocation< ) { const [start, end] = result.linesShown!; const total = result.originalLineCount!; - llmContent = `Showing lines ${start}-${end} of ${total} total lines.\n\n---\n\n${result.llmContent}`; + const totalLabel = + result.originalLineCountExact === false ? `at least ${total}` : total; + llmContent = `Showing lines ${start}-${end} of ${totalLabel} total lines.\n\n---\n\n${result.llmContent}`; } else { llmContent = result.llmContent || ''; } diff --git a/packages/core/src/utils/environmentContext.test.ts b/packages/core/src/utils/environmentContext.test.ts index 27ff34d6f7..d6f6d682d3 100644 --- a/packages/core/src/utils/environmentContext.test.ts +++ b/packages/core/src/utils/environmentContext.test.ts @@ -29,6 +29,7 @@ import { getInitialChatHistory, getStartupContextLength, isSystemReminderContent, + stripSystemReminderBlocks, stripStartupContext, formatDateForContext, SYSTEM_REMINDER_OPEN, @@ -325,30 +326,17 @@ describe('getInitialChatHistory', () => { }); it('places deferred-tools reminder last so stable prefix stays cacheable on KV-caching servers', async () => { - // Pin: deferred-tools part changes when tool_search reveals a tool. - // Placing it LAST keeps the stable prefix (MCP + skills + startup) - // cacheable; only the tail recomputes on prefix-caching servers. mockToolRegistry.getDeferredToolSummary.mockReturnValue([ { name: 'web_fetch', description: 'Fetches web pages' }, ]); const [history] = await getInitialChatHistory(mockConfig as Config); - expect(history).toHaveLength(1); const parts = history[0]?.parts ?? []; - expect(parts.length).toBeGreaterThanOrEqual(1); - - // The LAST text part should be the deferred-tools reminder. const lastText = parts[parts.length - 1]?.text; - expect(lastText).toBeDefined(); expect(lastText).toContain('reachable via `tool_search`'); expect(lastText).toContain('web_fetch'); - - // The FIRST text part should NOT be the deferred-tools reminder — - // it must be the stable MCP/skills/startup prefix. - const firstText = parts[0]?.text; - expect(firstText).toBeDefined(); - expect(firstText).not.toContain('reachable via `tool_search`'); + expect(parts[0]?.text).not.toContain('reachable via `tool_search`'); }); }); @@ -399,6 +387,20 @@ describe('stripStartupContext', () => { ).toEqual([{ role: 'user', parts: [{ text: 'Hello' }] }]); }); + it('keeps a first user turn that mixes a reminder part with a prompt part', () => { + const history: Content[] = [ + { + role: 'user', + parts: [ + { text: '\nctx\n' }, + { text: 'real prompt' }, + ], + }, + ]; + + expect(stripStartupContext(history)).toEqual(history); + }); + it('should round-trip with getInitialChatHistory', async () => { const mockConfig = { getSkipStartupContext: vi.fn().mockReturnValue(false), @@ -430,6 +432,25 @@ describe('stripStartupContext', () => { }); }); +describe('stripSystemReminderBlocks', () => { + it('strips complete reminder blocks and preserves surrounding text', () => { + expect( + stripSystemReminderBlocks('axb'), + ).toBe('ab'); + expect( + stripSystemReminderBlocks( + 'axbyc', + ), + ).toBe('abc'); + }); + + it('drops a trailing unclosed reminder block', () => { + expect(stripSystemReminderBlocks('keep secret')).toBe( + 'keep ', + ); + }); +}); + describe('formatDateForContext', () => { it('should format date in en-US locale regardless of system timezone', () => { expect(formatDateForContext(new Date('2026-06-05T12:00:00Z'))).toBe( diff --git a/packages/core/src/utils/environmentContext.ts b/packages/core/src/utils/environmentContext.ts index 94b20ad2af..b24d8f3d52 100644 --- a/packages/core/src/utils/environmentContext.ts +++ b/packages/core/src/utils/environmentContext.ts @@ -593,15 +593,7 @@ export function getStartupContextLength( ): number { const firstEntry = history[0]; if (firstEntry?.role !== 'user') return 0; - const firstText = firstEntry.parts?.[0]?.text; - // Open prefix, and close tag AT THE END (not merely present). Excludes a - // prompt quoting the literal tag, and — since IDE mode merges the reminder - // into the prompt's text part — a real first turn trailing after the close. - if ( - typeof firstText === 'string' && - firstText.startsWith(SYSTEM_REMINDER_OPEN) && - firstText.trimEnd().endsWith(SYSTEM_REMINDER_CLOSE) - ) { + if (isSystemReminderContent(firstEntry)) { if (options.includeCompressed) { const compressedLength = detectCompressedPrefixLength(history, 1); if (compressedLength > 0) return 1 + compressedLength; @@ -712,6 +704,27 @@ export function isSystemReminderContent(content: Content): boolean { ); } +export function stripSystemReminderBlocks(text: string): string { + let out = ''; + let offset = 0; + + while (offset < text.length) { + const open = text.indexOf(SYSTEM_REMINDER_OPEN, offset); + if (open === -1) return out + text.slice(offset); + + const close = text.indexOf( + SYSTEM_REMINDER_CLOSE, + open + SYSTEM_REMINDER_OPEN.length, + ); + if (close === -1) return out + text.slice(offset, open); + + out += text.slice(offset, open); + offset = close + SYSTEM_REMINDER_CLOSE.length; + } + + return out; +} + /** * Strip the leading startup context reminder from a chat history. Used when * forwarding a parent session's history to a child agent that will generate diff --git a/packages/core/src/utils/fileUtils.test.ts b/packages/core/src/utils/fileUtils.test.ts index 0ff64f1b6f..c0cef12435 100644 --- a/packages/core/src/utils/fileUtils.test.ts +++ b/packages/core/src/utils/fileUtils.test.ts @@ -29,11 +29,14 @@ import { processSingleFileContent, detectBOM, decodeBufferWithEncodingInfo, + readFileWithLineAndLimit, readFileWithEncoding, readFileWithEncodingInfo, detectFileEncoding, fileExists, } from './fileUtils.js'; +import { iconvEncode } from './iconvHelper.js'; +import { LargeNonUtf8TextError } from './read-text-range.js'; import type { Config } from '../config/config.js'; import { StandardFileSystemService } from '../services/fileSystemService.js'; import { ToolErrorType } from '../tools/tool-error.js'; @@ -1993,6 +1996,149 @@ describe('fileUtils', () => { expect(result.linesShown).toEqual([1, 2]); }); + it('should preserve default full file-system reads for large text files', async () => { + const content = `head\n${'x'.repeat(11 * 1024 * 1024)}`; + actualNodeFs.writeFileSync(testTextFilePath, content); + + const result = await fsService.readTextFile({ path: testTextFilePath }); + + expect(result.content).toBe(content); + expect(result._meta?.originalLineCount).toBe(2); + expect(result._meta?.originalLineCountExact).toBe(true); + expect(result._meta?.truncatedByBytes).not.toBe(true); + }); + + it('should stream explicit offset reads for large text files', async () => { + actualNodeFs.writeFileSync( + testTextFilePath, + `skip\n${'line\n'.repeat(3 * 1024 * 1024)}`, + ); + + const result = await fsService.readTextFile({ + path: testTextFilePath, + line: 1, + }); + + expect(result.content.startsWith('line\n')).toBe(true); + expect(result.content.startsWith('skip\n')).toBe(false); + expect(result._meta?.originalLineCountExact).toBe(false); + expect(result._meta?.truncatedByBytes).toBe(true); + }); + + it('should preserve unbounded explicit line-zero reads below the large-file threshold', async () => { + const content = `head\n${'body\n'.repeat(6_000)}tail\n`; + actualNodeFs.writeFileSync(testTextFilePath, content); + + const result = await fsService.readTextFile({ + path: testTextFilePath, + line: 0, + }); + + expect(result.content).toBe(content); + expect(result._meta?.truncatedByBytes).not.toBe(true); + }); + + it('should enforce maxOutputBytes for default file-system reads below the large-file threshold', async () => { + actualNodeFs.writeFileSync(testTextFilePath, 'x'.repeat(100)); + + const result = await fsService.readTextFile({ + path: testTextFilePath, + maxOutputBytes: 10, + }); + + expect(result.content).toBe('x'.repeat(10)); + expect(result._meta?.originalLineCount).toBe(1); + expect(result._meta?.originalLineCountExact).toBe(true); + expect(result._meta?.truncatedByBytes).toBe(true); + }); + + it('should propagate large non-UTF-8 errors through bounded reads', async () => { + const gbkLine = iconvEncode('中文日志行\n', 'gbk'); + const gbkChunk = Buffer.concat( + Array.from({ length: 1024 }, () => gbkLine), + ); + const repeatCount = Math.ceil((11 * 1024 * 1024) / gbkChunk.length); + actualNodeFs.writeFileSync( + testTextFilePath, + Buffer.concat(Array.from({ length: repeatCount }, () => gbkChunk)), + ); + + await expect( + readFileWithLineAndLimit({ + path: testTextFilePath, + limit: 10, + maxOutputBytes: 10_000, + }), + ).rejects.toThrow(LargeNonUtf8TextError); + }); + + it('should propagate aborts from unbounded full reads', async () => { + actualNodeFs.writeFileSync(testTextFilePath, 'hello\nworld'); + const controller = new AbortController(); + controller.abort(); + + await expect( + readFileWithLineAndLimit({ + path: testTextFilePath, + limit: Number.POSITIVE_INFINITY, + signal: controller.signal, + }), + ).rejects.toThrow(/abort/i); + }); + + it('should propagate aborts before large unbounded full reads', async () => { + actualNodeFs.writeFileSync( + testTextFilePath, + 'x'.repeat(11 * 1024 * 1024), + ); + const controller = new AbortController(); + controller.abort(); + + await expect( + readFileWithLineAndLimit({ + path: testTextFilePath, + limit: Number.POSITIVE_INFINITY, + signal: controller.signal, + }), + ).rejects.toThrow(/abort/i); + }); + + it('should use provided stats when reading with line and byte limits', async () => { + actualNodeFs.writeFileSync(testTextFilePath, 'hello\nworld'); + const stats = actualNodeFs.statSync(testTextFilePath); + const statSpy = vi + .spyOn(fs.promises, 'stat') + .mockRejectedValueOnce(new Error('unexpected stat')); + + try { + const result = await readFileWithLineAndLimit({ + path: testTextFilePath, + limit: 1, + maxOutputBytes: 100, + stats, + }); + + expect(result.content).toBe('hello'); + expect(statSpy).not.toHaveBeenCalled(); + } finally { + statSpy.mockRestore(); + } + }); + + it('should not byte-truncate multibyte text before the character limit', async () => { + const content = '你'.repeat(1000); + actualNodeFs.writeFileSync(testTextFilePath, content, 'utf-8'); + + const result = await processSingleFileContent( + testTextFilePath, + mockConfig, + ); + + expect(result.llmContent).toBe(content); + expect(result.returnDisplay).toBe(''); + expect(result.isTruncated).toBe(false); + }); + it('should truncate long lines in text files', async () => { const longLine = 'a'.repeat(2500); actualNodeFs.writeFileSync( @@ -2073,31 +2219,152 @@ describe('fileUtils', () => { ); }); - it('should return an error if the file size exceeds 10MB', async () => { - // Create a small test file - actualNodeFs.writeFileSync(testTextFilePath, 'test content'); + it('should read large text files through bounded truncation instead of the 10MB gate', async () => { + const lines = Array.from( + { length: 65_000 }, + (_, index) => `Line ${index + 1} ${'x'.repeat(180)}`, + ); + actualNodeFs.writeFileSync(testTextFilePath, lines.join('\n')); - // Spy on fs.promises.stat to return a large file size - const statSpy = vi.spyOn(fs.promises, 'stat').mockResolvedValueOnce({ - size: 11 * 1024 * 1024, - isDirectory: () => false, - isFile: () => true, - } as fs.Stats); + const result = await processSingleFileContent( + testTextFilePath, + mockConfig, + ); - try { - const result = await processSingleFileContent( - testTextFilePath, - mockConfig, - ); + expect(result.error).toBeUndefined(); + expect(result.llmContent).toContain('Line 1'); + expect(result.returnDisplay).toContain('Read lines 1-'); + expect(result.isTruncated).toBe(true); + expect(result.originalLineCount).toBeGreaterThanOrEqual( + result.linesShown?.[1] ?? 1, + ); + expect(result.originalLineCount).toBeLessThan(65_000); + expect(result.originalLineCountExact).toBe(false); + expect(result.linesShown?.[0]).toBe(1); + }); - expect(result.error).toContain('File size exceeds the 10MB limit'); - expect(result.returnDisplay).toContain( - 'File size exceeds the 10MB limit', - ); - expect(result.llmContent).toContain('File size exceeds the 10MB limit'); - } finally { - statSpy.mockRestore(); - } + it('should stream large text files when line truncation is disabled', async () => { + actualNodeFs.writeFileSync( + testTextFilePath, + 'x'.repeat(11 * 1024 * 1024), + ); + const noLineLimitConfig = { + ...mockConfig, + getTruncateToolOutputLines: () => Number.POSITIVE_INFINITY, + } as unknown as Config; + + const result = await processSingleFileContent( + testTextFilePath, + noLineLimitConfig, + ); + + expect(result.error).toBeUndefined(); + expect(typeof result.llmContent).toBe('string'); + expect(result.llmContent).toContain('... [truncated]'); + expect(result.returnDisplay).toBe( + 'Read lines 1-1 of at least 1 from test.txt (truncated)', + ); + expect(result.isTruncated).toBe(true); + expect(result.originalLineCountExact).toBe(false); + }); + + it('should mark byte truncation metadata without character truncation', async () => { + actualNodeFs.writeFileSync(testTextFilePath, 'visible'); + const byteTruncatedConfig = { + ...mockConfig, + getTruncateToolOutputThreshold: () => Number.POSITIVE_INFINITY, + getFileSystemService: () => ({ + readTextFile: vi.fn().mockResolvedValue({ + content: 'visible', + _meta: { + originalLineCount: 1, + originalLineCountExact: false, + truncatedByBytes: true, + }, + }), + }), + } as unknown as Config; + + const result = await processSingleFileContent( + testTextFilePath, + byteTruncatedConfig, + ); + + expect(typeof result.llmContent).toBe('string'); + const llmContent = result.llmContent as string; + expect(llmContent).toBe('visible\n... [truncated]'); + expect(llmContent.match(/\.\.\. \[truncated\]/g)).toHaveLength(1); + expect(result.returnDisplay).toBe( + 'Read lines 1-1 of at least 1 from test.txt (truncated)', + ); + expect(result.isTruncated).toBe(true); + }); + + it('should use selected range as a lower bound when large file metadata is missing', async () => { + actualNodeFs.writeFileSync( + testTextFilePath, + 'x'.repeat(11 * 1024 * 1024), + ); + const missingMetadataConfig = { + ...mockConfig, + getFileSystemService: () => ({ + readTextFile: vi.fn().mockResolvedValue({ + content: 'visible\nnext', + }), + }), + } as unknown as Config; + + const result = await processSingleFileContent( + testTextFilePath, + missingMetadataConfig, + { offset: 9, limit: 2 }, + ); + + expect(result.originalLineCount).toBe(11); + expect(result.originalLineCountExact).toBe(false); + expect(result.returnDisplay).toBe( + 'Read lines 10-11 of at least 11 from test.txt', + ); + }); + + it('should preserve disabled output truncation for large text files', async () => { + const byteLength = 11 * 1024 * 1024; + actualNodeFs.writeFileSync(testTextFilePath, 'x'.repeat(byteLength)); + const noCharacterLimitConfig = { + ...mockConfig, + getTruncateToolOutputThreshold: () => Number.POSITIVE_INFINITY, + } as unknown as Config; + + const result = await processSingleFileContent( + testTextFilePath, + noCharacterLimitConfig, + ); + + expect(typeof result.llmContent).toBe('string'); + const llmContent = result.llmContent as string; + expect(llmContent).toHaveLength(byteLength); + expect(llmContent).not.toContain('... [truncated]'); + expect(result.returnDisplay).toBe(''); + expect(result.isTruncated).toBe(false); + }); + + it('should still return an error if an inline media file exceeds 10MB', async () => { + mockMimeGetType.mockReturnValue('image/png'); + actualNodeFs.writeFileSync( + testImageFilePath, + Buffer.alloc(11 * 1024 * 1024), + ); + + const result = await processSingleFileContent( + testImageFilePath, + mockConfig, + ); + + expect(result.error).toContain('File size exceeds the 10MB limit'); + expect(result.returnDisplay).toContain( + 'File size exceeds the 10MB limit', + ); + expect(result.llmContent).toContain('File size exceeds the 10MB limit'); }); it('should allow explicit page ranges above the full-PDF text-extraction size cap', async () => { diff --git a/packages/core/src/utils/fileUtils.ts b/packages/core/src/utils/fileUtils.ts index c6e03416e3..6cb62bf205 100644 --- a/packages/core/src/utils/fileUtils.ts +++ b/packages/core/src/utils/fileUtils.ts @@ -18,7 +18,7 @@ import { ToolErrorType } from '../tools/tool-error.js'; import { BINARY_EXTENSIONS } from './ignorePatterns.js'; import type { Config } from '../config/config.js'; import { createDebugLogger } from './debugLogger.js'; -import { getErrorMessage, isNodeError } from './errors.js'; +import { getErrorMessage, isAbortError, isNodeError } from './errors.js'; import type { InputModalities } from '../core/contentGenerator.js'; import { detectEncodingFromBuffer } from './systemEncoding.js'; import { @@ -35,6 +35,11 @@ import { shouldRequirePDFPageRange, } from './pdf.js'; import { readNotebookWithMetadata } from './notebook.js'; +import { readTextRange } from './read-text-range.js'; +import { + DEFAULT_RANGE_READ_BYTES, + TEXT_RANGE_FAST_PATH_MAX_SIZE, +} from './text-range-constants.js'; const debugLogger = createDebugLogger('FILE_UTILS'); @@ -273,9 +278,13 @@ function bomEncodingToName(bomEncoding: UnicodeEncoding): string { */ export async function readFileWithEncodingInfo( filePath: string, + signal?: AbortSignal, ): Promise { // Read the file once; detect BOM and decode from the single buffer. - const full = await fs.promises.readFile(filePath); + const full = await fs.promises.readFile( + filePath, + signal === undefined ? undefined : { signal }, + ); return decodeBufferWithEncodingInfo(full); } @@ -299,14 +308,42 @@ export async function readFileWithLineAndLimit(params: { path: string; limit: number; line?: number; + maxOutputBytes?: number; + signal?: AbortSignal; + stats?: import('node:fs').Stats; }): Promise<{ content: string; bom?: boolean; encoding?: string; originalLineCount: number; + originalLineCountExact?: boolean; + lineEnding?: 'crlf' | 'lf'; + truncatedByBytes?: boolean; }> { - const { path: filePath, limit, line } = params; - const { content, encoding, bom } = await readFileWithEncodingInfo(filePath); + const { path: filePath, limit, line, maxOutputBytes, signal } = params; + const stats = params.stats ?? (await fs.promises.stat(filePath)); + if ( + (line !== undefined && line > 0) || + Number.isFinite(limit) || + maxOutputBytes !== undefined + ) { + return readTextRange({ + path: filePath, + offset: line || 0, + limit, + maxOutputBytes: normalizeRangeReadByteLimit(maxOutputBytes), + stats, + ...(signal !== undefined ? { signal } : {}), + }); + } + + signal?.throwIfAborted(); + + const { content, encoding, bom } = await readFileWithEncodingInfo( + filePath, + signal, + ); + signal?.throwIfAborted(); const lines = content.split('\n'); const originalLineCount = lines.length; const startLine = line || 0; @@ -321,6 +358,7 @@ export async function readFileWithLineAndLimit(params: { bom, encoding, originalLineCount, + originalLineCountExact: true, }; } @@ -829,6 +867,7 @@ export interface ProcessedFileReadResult { error?: string; // Optional error message for the LLM if file processing failed errorType?: ToolErrorType; // Structured error type originalLineCount?: number; // For text files, the total number of lines in the original file + originalLineCountExact?: boolean; // False when a large range read stopped before EOF isTruncated?: boolean; // Indicates if displayed content was truncated linesShown?: [number, number]; // For text files [startLine, endLine] (1-based for display) /** @@ -867,6 +906,7 @@ export interface ProcessSingleFileContentOptions { * sets this after deciding the vision bridge should handle the image. */ preserveUnsupportedImage?: boolean; + signal?: AbortSignal; /** * Large full-PDF text fallback returns a tool error by default. `@`-attached * PDFs use `reference` so the model gets guidance without a failed read. @@ -945,10 +985,12 @@ export async function processSingleFileContent( limit, pages, preserveUnsupportedImage = false, + signal, largePdfBehavior = 'error', } = options; const rootDirectory = config.getTargetDir(); try { + signal?.throwIfAborted(); let stats: import('node:fs').Stats; try { // Async stat doubles as the existence check — ENOENT is handled below @@ -1101,7 +1143,7 @@ export async function processSingleFileContent( }; } } - if (fileSizeInMB > 9.9 && !willExtractPdfText) { + if (fileSizeInMB > 9.9 && !willExtractPdfText && fileType !== 'text') { return { llmContent: 'File size exceeds the 10MB limit.', returnDisplay: 'File size exceeds the 10MB limit.', @@ -1182,11 +1224,25 @@ export async function processSingleFileContent( path: filePath, limit: limit ?? config.getTruncateToolOutputLines(), line: offset, + maxOutputBytes: getRangeReadByteLimit(config), + stats, + ...(signal !== undefined ? { signal } : {}), }); - const originalLineCount = - _meta?.originalLineCount ?? (await countFileLines(filePath)); const selectedLines = content.split('\n').map((line) => line.trimEnd()); const startLine = offset || 0; + const selectedLineCount = + content.length === 0 ? 0 : selectedLines.length; + const hasOriginalLineCount = _meta?.originalLineCount !== undefined; + const originalLineCount = + _meta?.originalLineCount ?? + (stats.size >= TEXT_RANGE_FAST_PATH_MAX_SIZE + ? startLine + selectedLineCount + : await countFileLines(filePath)); + const originalLineCountExact = + _meta?.originalLineCountExact === false + ? false + : hasOriginalLineCount || + stats.size < TEXT_RANGE_FAST_PATH_MAX_SIZE; const configCharLimit = config.getTruncateToolOutputThreshold(); // Apply character limit truncation @@ -1227,17 +1283,38 @@ export async function processSingleFileContent( linesIncluded = selectedLines.length; } + if (_meta?.truncatedByBytes === true) { + const marker = '... [truncated]'; + if (!llmContent.endsWith(marker)) { + const prefix = Number.isFinite(configCharLimit) + ? llmContent.slice( + 0, + Math.max(configCharLimit - marker.length - 1, 0), + ) + : llmContent; + const separator = + prefix.length === 0 || prefix.endsWith('\n') ? '' : '\n'; + llmContent = prefix + separator + marker; + } + contentLengthTruncated = true; + } + const actualEndLine = startLine + linesIncluded; const contentRangeTruncated = - startLine > 0 || actualEndLine < originalLineCount; + startLine > 0 || + !originalLineCountExact || + actualEndLine < originalLineCount; const isTruncated = contentRangeTruncated || contentLengthTruncated; + const lineCountLabel = originalLineCountExact + ? `${originalLineCount}` + : `at least ${originalLineCount}`; // By default, return nothing to streamline the common case of a successful read_file. let returnDisplay = ''; if (isTruncated) { returnDisplay = `Read lines ${ startLine + 1 - }-${actualEndLine} of ${originalLineCount} from ${relativePathForDisplay}`; + }-${actualEndLine} of ${lineCountLabel} from ${relativePathForDisplay}`; if (contentLengthTruncated) { returnDisplay += ' (truncated)'; } @@ -1248,6 +1325,7 @@ export async function processSingleFileContent( returnDisplay, isTruncated, originalLineCount, + originalLineCountExact, linesShown: [startLine + 1, actualEndLine], stats, }; @@ -1385,6 +1463,9 @@ export async function processSingleFileContent( } } } catch (error) { + if (signal?.aborted || isAbortError(error)) { + throw error; + } const errorMessage = getErrorMessage(error); const displayPath = path .relative(rootDirectory, filePath) @@ -1398,6 +1479,28 @@ export async function processSingleFileContent( } } +function getRangeReadByteLimit(config: Config): number { + const charLimit = config.getTruncateToolOutputThreshold(); + if (charLimit === Number.POSITIVE_INFINITY) { + return Number.POSITIVE_INFINITY; + } + if (!Number.isFinite(charLimit)) { + return DEFAULT_RANGE_READ_BYTES; + } + // Leave enough byte headroom for UTF-8 text so the character budget remains + // the primary truncation control for normal source/log content. + return Math.max(DEFAULT_RANGE_READ_BYTES, Math.floor(charLimit) * 4); +} + +function normalizeRangeReadByteLimit(maxOutputBytes: number | undefined) { + if (maxOutputBytes === Number.POSITIVE_INFINITY) { + return Number.POSITIVE_INFINITY; + } + return typeof maxOutputBytes === 'number' && Number.isFinite(maxOutputBytes) + ? maxOutputBytes + : DEFAULT_RANGE_READ_BYTES; +} + export async function fileExists(filePath: string): Promise { try { await fsPromises.access(filePath, fs.constants.F_OK); diff --git a/packages/core/src/utils/pathReader.test.ts b/packages/core/src/utils/pathReader.test.ts index c6b8040d39..5b5e951acb 100644 --- a/packages/core/src/utils/pathReader.test.ts +++ b/packages/core/src/utils/pathReader.test.ts @@ -428,8 +428,7 @@ describe('readPathFromWorkspace', () => { }, ); - it('should return an error string for files exceeding the size limit', async () => { - // Mock a file slightly larger than the 10MB limit defined in fileUtils.ts + it('should truncate text files exceeding 10MB instead of failing', async () => { const largeContent = 'a'.repeat(11 * 1024 * 1024); // 11MB mock({ [CWD]: { @@ -442,7 +441,7 @@ describe('readPathFromWorkspace', () => { const config = createMockConfig(CWD, [], mockFileService); const result = await readPathFromWorkspace('large.txt', config); const textResult = result[0] as string; - // The error message comes directly from processSingleFileContent - expect(textResult).toBe('File size exceeds the 10MB limit.'); + expect(textResult).toContain('a'.repeat(100)); + expect(textResult).toContain('... [truncated]'); }); }); diff --git a/packages/core/src/utils/read-text-range.test.ts b/packages/core/src/utils/read-text-range.test.ts new file mode 100644 index 0000000000..2812045580 --- /dev/null +++ b/packages/core/src/utils/read-text-range.test.ts @@ -0,0 +1,294 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import * as fs from 'node:fs/promises'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { iconvEncode } from './iconvHelper.js'; +import { LargeNonUtf8TextError, readTextRange } from './read-text-range.js'; + +describe('readTextRange', () => { + let tempDir: string; + + beforeEach(async () => { + tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'read-text-range-')); + }); + + afterEach(async () => { + await fs.rm(tempDir, { recursive: true, force: true }); + }); + + async function writeFile( + name: string, + data: string | Buffer, + ): Promise { + const filePath = path.join(tempDir, name); + await fs.writeFile(filePath, data); + return filePath; + } + + function largeUtf8Lines(lineCount: number): string { + return Array.from( + { length: lineCount }, + (_, index) => `line-${index + 1} ${'x'.repeat(180)}`, + ).join('\n'); + } + + it('preserves split newline semantics on the fast path', async () => { + const emptyPath = await writeFile('empty.txt', ''); + await expect( + readTextRange({ + path: emptyPath, + offset: 0, + limit: 10, + maxOutputBytes: 100, + }), + ).resolves.toMatchObject({ + content: '', + originalLineCount: 1, + truncatedByBytes: false, + }); + + const trailingPath = await writeFile('trailing.txt', 'a\n'); + await expect( + readTextRange({ + path: trailingPath, + offset: 0, + limit: 10, + maxOutputBytes: 100, + }), + ).resolves.toMatchObject({ + content: 'a\n', + originalLineCount: 2, + truncatedByBytes: false, + }); + }); + + it('streams a large UTF-8 file and returns the requested range', async () => { + const filePath = await writeFile('large.log', largeUtf8Lines(65_000)); + + const result = await readTextRange({ + path: filePath, + offset: 42_000, + limit: 3, + maxOutputBytes: 10_000, + }); + + expect(result.content.split('\n')).toEqual([ + expect.stringContaining('line-42001'), + expect.stringContaining('line-42002'), + expect.stringContaining('line-42003'), + ]); + expect(result.originalLineCount).toBeGreaterThanOrEqual(42_004); + expect(result.originalLineCount).toBeLessThan(65_000); + expect(result.originalLineCountExact).toBe(false); + expect(result.encoding).toBe('utf-8'); + expect(result.bom).toBe(false); + expect(result.truncatedByBytes).toBe(false); + }); + + it('streams a large UTF-8 file from the beginning when no range is provided', async () => { + const filePath = await writeFile('large.log', largeUtf8Lines(65_000)); + + const result = await readTextRange({ + path: filePath, + maxOutputBytes: 10_000, + }); + + expect(result.content).toContain('line-1'); + expect(result.content).toContain('line-2'); + expect(result.content).not.toContain('line-65000'); + expect(result.originalLineCount).toBeGreaterThan(1); + expect(result.originalLineCountExact).toBe(false); + expect(result.truncatedByBytes).toBe(true); + }); + + it('returns empty content when a large UTF-8 range starts beyond EOF', async () => { + const filePath = await writeFile('large.log', largeUtf8Lines(65_000)); + + const result = await readTextRange({ + path: filePath, + offset: 100_000, + limit: 10, + maxOutputBytes: 10_000, + }); + + expect(result.content).toBe(''); + expect(result.originalLineCount).toBe(65_000); + expect(result.originalLineCountExact).toBe(true); + expect(result.truncatedByBytes).toBe(false); + }); + + it('preserves CRLF content and line-ending metadata for large files', async () => { + const content = Array.from( + { length: 65_000 }, + (_, index) => `line-${index + 1} ${'x'.repeat(180)}`, + ).join('\r\n'); + const filePath = await writeFile('crlf.log', content); + + const result = await readTextRange({ + path: filePath, + offset: 1, + limit: 2, + maxOutputBytes: 10_000, + }); + + expect(result.content).toContain('\r\n'); + expect(result.content.split('\n')[0]).toMatch(/\r$/); + expect(result.lineEnding).toBe('crlf'); + expect(result.originalLineCount).toBeGreaterThanOrEqual(4); + expect(result.originalLineCount).toBeLessThan(65_000); + expect(result.originalLineCountExact).toBe(false); + }); + + it('detects CRLF when the pair crosses a stream chunk boundary', async () => { + const highWaterMark = 512 * 1024; + const firstChunk = `${'a'.repeat(highWaterMark - 1)}\r`; + const body = Buffer.concat([ + Buffer.from(firstChunk), + Buffer.from('\nsecond\n'), + Buffer.alloc(11 * 1024 * 1024, 'x'), + ]); + const filePath = await writeFile('split-crlf.log', body); + + const result = await readTextRange({ + path: filePath, + offset: 0, + limit: 2, + maxOutputBytes: highWaterMark + 100, + }); + + expect(result.lineEnding).toBe('crlf'); + expect(result.content).toContain('\r\nsecond'); + }); + + it('strips UTF-8 BOM from large file content and reports BOM metadata', async () => { + const body = largeUtf8Lines(65_000); + const filePath = await writeFile( + 'bom.log', + Buffer.concat([Buffer.from([0xef, 0xbb, 0xbf]), Buffer.from(body)]), + ); + + const result = await readTextRange({ + path: filePath, + offset: 0, + limit: 1, + maxOutputBytes: 10_000, + }); + + expect(result.content.charCodeAt(0)).not.toBe(0xfeff); + expect(result.content).toContain('line-1'); + expect(result.bom).toBe(true); + expect(result.encoding).toBe('utf-8'); + expect(result.originalLineCountExact).toBe(false); + }); + + it('does not split a UTF-8 character when byte-truncating', async () => { + const filePath = await writeFile('emoji.txt', `a🙂b`); + + const result = await readTextRange({ + path: filePath, + offset: 0, + limit: 1, + maxOutputBytes: 4, + }); + + expect(result.content).toBe('a'); + expect(result.content).not.toContain('\uFFFD'); + expect(result.truncatedByBytes).toBe(true); + expect(result.originalLineCountExact).toBe(true); + }); + + it('rejects large non-UTF-8 files with a targeted error', async () => { + const gbkLine = iconvEncode('中文日志行\n', 'gbk'); + const repeatCount = Math.ceil((11 * 1024 * 1024) / gbkLine.length); + const filePath = await writeFile( + 'gbk.log', + Buffer.concat(Array.from({ length: repeatCount }, () => gbkLine)), + ); + + await expect( + readTextRange({ + path: filePath, + offset: 0, + limit: 10, + maxOutputBytes: 10_000, + }), + ).rejects.toThrow(LargeNonUtf8TextError); + }); + + it('rejects large files with invalid UTF-8 beyond the encoding sample', async () => { + const mostlyAsciiThenGbk = Buffer.concat([ + Buffer.alloc(9 * 1024, 'a'), + iconvEncode('你好', 'gbk'), + Buffer.alloc(11 * 1024 * 1024, 'b'), + ]); + const filePath = await writeFile('late-gbk.log', mostlyAsciiThenGbk); + + const promise = readTextRange({ + path: filePath, + offset: 0, + limit: 500, + maxOutputBytes: 20_000, + }); + + await expect(promise).rejects.toThrow(LargeNonUtf8TextError); + await expect(promise).rejects.toThrow(/invalid UTF-8 byte sequence/); + await expect(promise).rejects.toMatchObject({ reason: 'invalid-utf8' }); + }); + + it('bounds selected output for a large single-line file', async () => { + const filePath = await writeFile( + 'single-line.log', + 'x'.repeat(11 * 1024 * 1024), + ); + + const result = await readTextRange({ + path: filePath, + offset: 0, + limit: 1, + maxOutputBytes: 1024, + }); + + expect(result.content).toBe('x'.repeat(1024)); + expect(result.originalLineCount).toBe(1); + expect(result.originalLineCountExact).toBe(false); + expect(result.truncatedByBytes).toBe(true); + }); + + it('propagates aborts before reading large files', async () => { + const filePath = await writeFile('large.log', largeUtf8Lines(65_000)); + const controller = new AbortController(); + controller.abort(); + + await expect( + readTextRange({ + path: filePath, + offset: 0, + limit: 10, + maxOutputBytes: 10_000, + signal: controller.signal, + }), + ).rejects.toThrow(/abort/i); + }); + + it('propagates aborts while streaming large files', async () => { + const filePath = await writeFile('large.log', largeUtf8Lines(80_000)); + const controller = new AbortController(); + const promise = readTextRange({ + path: filePath, + offset: 70_000, + limit: 10, + maxOutputBytes: 10_000, + signal: controller.signal, + }); + + setTimeout(() => controller.abort(), 0); + + await expect(promise).rejects.toThrow(/abort/i); + }); +}); diff --git a/packages/core/src/utils/read-text-range.ts b/packages/core/src/utils/read-text-range.ts new file mode 100644 index 0000000000..d675372893 --- /dev/null +++ b/packages/core/src/utils/read-text-range.ts @@ -0,0 +1,281 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { createReadStream, type Stats } from 'node:fs'; +import { stat } from 'node:fs/promises'; +import { TextDecoder } from 'node:util'; +import { detectFileEncoding, readFileWithEncodingInfo } from './fileUtils.js'; +import { isUtf8CompatibleEncoding } from './iconvHelper.js'; +import { + DEFAULT_RANGE_READ_BYTES, + TEXT_RANGE_FAST_PATH_MAX_SIZE, +} from './text-range-constants.js'; + +export interface ReadTextRangeRequest { + path: string; + offset?: number; + limit?: number; + maxOutputBytes: number; + signal?: AbortSignal; + stats?: Stats; +} + +export interface ReadTextRangeResult { + content: string; + originalLineCount: number; + encoding?: string; + bom?: boolean; + lineEnding?: 'crlf' | 'lf'; + originalLineCountExact: boolean; + truncatedByBytes: boolean; +} + +export class LargeNonUtf8TextError extends Error { + constructor( + readonly encoding: string, + readonly reason?: 'invalid-utf8', + ) { + super( + reason === 'invalid-utf8' + ? 'Large text file contains invalid UTF-8 byte sequence beyond the initial encoding sample. Convert or extract a smaller UTF-8 slice and read that instead.' + : `Large non-UTF-8 text files are not supported for streaming reads (detected ${encoding}). Convert or extract a smaller UTF-8 slice and read that instead.`, + ); + this.name = 'LargeNonUtf8TextError'; + } +} + +export async function readTextRange( + request: ReadTextRangeRequest, +): Promise { + request.signal?.throwIfAborted(); + const stats = request.stats ?? (await stat(request.path)); + const maxOutputBytes = normalizeMaxBytes(request.maxOutputBytes); + + if (stats.size < TEXT_RANGE_FAST_PATH_MAX_SIZE) { + const { content, encoding, bom } = await readFileWithEncodingInfo( + request.path, + request.signal, + ); + request.signal?.throwIfAborted(); + const range = sliceDecodedContent( + content, + request.offset, + request.limit, + maxOutputBytes, + ); + return { + ...range, + encoding, + bom, + lineEnding: detectLineEndingFromContent(content), + }; + } + + return readLargeUtf8Range(request, maxOutputBytes); +} + +function normalizeMaxBytes(maxOutputBytes: number): number { + if (maxOutputBytes === Number.POSITIVE_INFINITY) { + return Number.POSITIVE_INFINITY; + } + if (!Number.isFinite(maxOutputBytes)) { + return DEFAULT_RANGE_READ_BYTES; + } + return Math.max(0, Math.floor(maxOutputBytes)); +} + +function sliceDecodedContent( + content: string, + offset: number | undefined, + limit: number | undefined, + maxOutputBytes: number, +): Pick< + ReadTextRangeResult, + | 'content' + | 'originalLineCount' + | 'originalLineCountExact' + | 'truncatedByBytes' +> { + const lines = content.split('\n'); + const originalLineCount = lines.length; + const start = Math.min(Math.max(0, offset ?? 0), originalLineCount); + const end = + limit === undefined + ? originalLineCount + : Math.min(start + Math.max(0, limit), originalLineCount); + const selected = lines.slice(start, end).join('\n'); + const truncated = truncateUtf8(selected, maxOutputBytes); + + return { + content: truncated.content, + originalLineCount, + originalLineCountExact: true, + truncatedByBytes: truncated.truncated, + }; +} + +async function readLargeUtf8Range( + request: ReadTextRangeRequest, + maxOutputBytes: number, +): Promise { + const encoding = await detectFileEncoding(request.path); + if (!isUtf8CompatibleEncoding(encoding)) { + throw new LargeNonUtf8TextError(encoding); + } + + const offset = Math.max(0, request.offset ?? 0); + const endLine = + offset + Math.max(0, request.limit ?? Number.POSITIVE_INFINITY); + let currentLine = 0; + let output = ''; + let outputBytes = 0; + let truncatedByBytes = false; + let bom = false; + let firstChunk = true; + let lineEnding: 'crlf' | 'lf' = 'lf'; + let previousChunkEndedWithCR = false; + let originalLineCountExact = true; + let stoppedEarly = false; + const decoder = new TextDecoder('utf-8', { + fatal: true, + ignoreBOM: true, + }); + + const stream = createReadStream(request.path, { + highWaterMark: 512 * 1024, + signal: request.signal, + }); + + function appendSelected(fragment: string): void { + if (fragment.length === 0 || truncatedByBytes) { + return; + } + + const available = maxOutputBytes - outputBytes; + if (available <= 0) { + truncatedByBytes = true; + return; + } + + const truncated = truncateUtf8(fragment, available); + output += truncated.content; + outputBytes += Buffer.byteLength(truncated.content, 'utf8'); + if (truncated.truncated) { + truncatedByBytes = true; + } + } + + function isSelectedLine(): boolean { + return currentLine >= offset && currentLine < endLine; + } + + function decodeUtf8Chunk( + chunk?: Buffer, + options?: TextDecodeOptions, + ): string { + try { + return decoder.decode(chunk, options); + } catch { + throw new LargeNonUtf8TextError(encoding, 'invalid-utf8'); + } + } + + try { + for await (const rawChunk of stream) { + request.signal?.throwIfAborted(); + let chunk = decodeUtf8Chunk(rawChunk as Buffer, { stream: true }); + if (firstChunk) { + firstChunk = false; + if (chunk.charCodeAt(0) === 0xfeff) { + chunk = chunk.slice(1); + bom = true; + } + } + + if ( + (previousChunkEndedWithCR && chunk.startsWith('\n')) || + chunk.includes('\r\n') + ) { + lineEnding = 'crlf'; + } + previousChunkEndedWithCR = chunk.endsWith('\r'); + + let start = 0; + let newline = chunk.indexOf('\n', start); + while (newline !== -1) { + if (isSelectedLine()) { + appendSelected(chunk.slice(start, newline)); + if (currentLine + 1 < endLine) { + appendSelected('\n'); + } + } + currentLine++; + start = newline + 1; + if (currentLine >= endLine || truncatedByBytes) { + originalLineCountExact = false; + stoppedEarly = true; + break; + } + newline = chunk.indexOf('\n', start); + } + + if (start < chunk.length && isSelectedLine()) { + appendSelected(chunk.slice(start)); + } + if (currentLine >= endLine || truncatedByBytes) { + originalLineCountExact = false; + stoppedEarly = true; + break; + } + } + } finally { + stream.destroy(); + } + + if (!stoppedEarly) { + decodeUtf8Chunk(); + } + + return { + content: output, + originalLineCount: currentLine + 1, + encoding: 'utf-8', + bom, + lineEnding, + originalLineCountExact, + truncatedByBytes, + }; +} + +function truncateUtf8( + content: string, + maxBytes: number, +): { content: string; truncated: boolean } { + const bytes = Buffer.byteLength(content, 'utf8'); + if (bytes <= maxBytes) { + return { content, truncated: false }; + } + if (maxBytes <= 0) { + return { content: '', truncated: true }; + } + + const buffer = Buffer.from(content, 'utf8'); + let end = Math.min(maxBytes, buffer.length); + // `end` is the first excluded byte. If it lands inside a multi-byte UTF-8 + // sequence, the byte at `end` is a continuation byte, so back up until the + // prefix ends before the incomplete character. + while (end > 0 && (buffer[end] & 0xc0) === 0x80) { + end--; + } + return { + content: buffer.subarray(0, end).toString('utf8'), + truncated: true, + }; +} + +function detectLineEndingFromContent(content: string): 'crlf' | 'lf' { + return content.includes('\r\n') ? 'crlf' : 'lf'; +} diff --git a/packages/core/src/utils/readManyFiles.test.ts b/packages/core/src/utils/readManyFiles.test.ts index 20d654e253..4351432c80 100644 --- a/packages/core/src/utils/readManyFiles.test.ts +++ b/packages/core/src/utils/readManyFiles.test.ts @@ -147,6 +147,22 @@ describe('readManyFiles', () => { expect(content).toContain('--- End of content ---'); }); + it('should include truncated large text files instead of reporting a size error', async () => { + const relativePath = 'large.log'; + const absolutePath = path.join(tempRootDir, relativePath); + await fs.writeFile(absolutePath, 'x'.repeat(11 * 1024 * 1024), 'utf-8'); + const mockConfig = createMockConfig(tempRootDir); + + const result = await readManyFiles(mockConfig, { paths: [relativePath] }); + + const content = contentToString(result.contentParts); + expect(content).toContain('Showing lines 1-1 of at least 1 total lines'); + expect(content).toContain('... [truncated]'); + expect(result.files).toHaveLength(1); + expect(result.files[0]!.error).toBeUndefined(); + expect(result.files[0]!.filePath).toBe(absolutePath); + }); + it('should include truncated notebooks that do not expose text line ranges', async () => { const relativePath = 'large.ipynb'; const absolutePath = path.join(tempRootDir, relativePath); @@ -276,6 +292,20 @@ describe('readManyFiles', () => { expect(content).not.toContain('Content of file1.txt'); }); + it('should propagate aborts before reading a directory', async () => { + await createTestFile('mydir', 'file1.txt'); + const mockConfig = createMockConfig(tempRootDir); + const controller = new AbortController(); + controller.abort(); + + await expect( + readManyFiles(mockConfig, { + paths: ['mydir'], + signal: controller.signal, + }), + ).rejects.toThrow(/abort/i); + }); + it('should handle directory with trailing slash', async () => { await createTestFile('mydir', 'file1.txt'); const mockConfig = createMockConfig(tempRootDir); @@ -439,6 +469,20 @@ describe('readManyFiles', () => { }); describe('per-file error surfacing', () => { + it('should propagate aborts from file reads instead of returning an error message', async () => { + const { relativePath } = await createTestFile('cancel.txt'); + const mockConfig = createMockConfig(tempRootDir); + const controller = new AbortController(); + controller.abort(); + + await expect( + readManyFiles(mockConfig, { + paths: [relativePath], + signal: controller.signal, + }), + ).rejects.toThrow(/abort/i); + }); + it('should surface processSingleFileContent errors instead of silently skipping the file', async () => { // Trigger the >10MB file-size error path in processSingleFileContent. const relativePath = 'huge.bin'; diff --git a/packages/core/src/utils/readManyFiles.ts b/packages/core/src/utils/readManyFiles.ts index 80bc835cb1..7d12b26dca 100644 --- a/packages/core/src/utils/readManyFiles.ts +++ b/packages/core/src/utils/readManyFiles.ts @@ -8,7 +8,7 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; import type { Part, PartListUnion } from '@google/genai'; import type { Config } from '../config/config.js'; -import { getErrorMessage } from './errors.js'; +import { getErrorMessage, isAbortError } from './errors.js'; import type { ProcessedFileReadResult } from './fileUtils.js'; import { isCacheableReadResult, @@ -104,7 +104,11 @@ export async function readManyFiles( config: Config, options: ReadManyFilesOptions, ): Promise { - const { paths: inputPatterns, preserveUnsupportedImageForBridge } = options; + const { + paths: inputPatterns, + preserveUnsupportedImageForBridge, + signal, + } = options; const seenFiles = new Set(); const contentParts: Part[] = []; @@ -114,6 +118,7 @@ export async function readManyFiles( const projectRoot = config.getProjectRoot(); for (const rawPattern of inputPatterns) { + signal?.throwIfAborted(); const normalizedPattern = rawPattern.replace(/\\/g, '/'); const fullPath = path.resolve(projectRoot, normalizedPattern); const stats = fs.existsSync(fullPath) ? fs.statSync(fullPath) : null; @@ -122,6 +127,7 @@ export async function readManyFiles( const { contentParts: dirParts, info } = await readDirectory( config, fullPath, + signal, ); contentParts.push(...dirParts); files.push(info); @@ -134,6 +140,7 @@ export async function readManyFiles( config, fullPath, preserveUnsupportedImageForBridge, + signal, ); if (readResult) { contentParts.push(...readResult.contentParts); @@ -142,6 +149,9 @@ export async function readManyFiles( } } } catch (error) { + if (signal?.aborted || isAbortError(error)) { + throw error; + } const errorMessage = `Error during file search: ${getErrorMessage(error)}`; return { contentParts: [errorMessage], @@ -165,11 +175,14 @@ export async function readManyFiles( async function readDirectory( config: Config, directoryPath: string, + signal?: AbortSignal, ): Promise<{ contentParts: Part[]; info: FileReadInfo }> { + signal?.throwIfAborted(); const structure = await getFolderStructure(directoryPath, { fileService: config.getFileService(), fileFilteringOptions: config.getFileFilteringOptions(), }); + signal?.throwIfAborted(); const contentParts: Part[] = [ { text: `\nContent from ${directoryPath}:\n` }, @@ -190,10 +203,12 @@ async function readFileContent( config: Config, filePath: string, preserveUnsupportedImage = false, + signal?: AbortSignal, ): Promise<{ contentParts: Part[]; info: FileReadInfo } | null> { try { const fileReadResult = await processSingleFileContent(filePath, config, { preserveUnsupportedImage, + ...(signal !== undefined ? { signal } : {}), largePdfBehavior: 'reference', }); @@ -233,7 +248,11 @@ async function readFileContent( ) { const [start, end] = fileReadResult.linesShown!; const total = fileReadResult.originalLineCount!; - fileContentForLlm = `Showing lines ${start}-${end} of ${total} total lines.\n---\n${fileReadResult.llmContent}`; + const totalLabel = + fileReadResult.originalLineCountExact === false + ? `at least ${total}` + : total; + fileContentForLlm = `Showing lines ${start}-${end} of ${totalLabel} total lines.\n---\n${fileReadResult.llmContent}`; } else { fileContentForLlm = fileReadResult.llmContent; } @@ -258,7 +277,10 @@ async function readFileContent( isDirectory: false, }, }; - } catch { + } catch (error) { + if (signal?.aborted || isAbortError(error)) { + throw error; + } return null; } } diff --git a/packages/core/src/utils/shell-pager-env.test.ts b/packages/core/src/utils/shell-pager-env.test.ts index ef10aa8475..365b9e05b2 100644 --- a/packages/core/src/utils/shell-pager-env.test.ts +++ b/packages/core/src/utils/shell-pager-env.test.ts @@ -44,6 +44,28 @@ describe('shellPagerEnv', () => { }); }); + it('omits GIT_PAGER when git pager output is not requested', () => { + expect( + getShellPagerEnv('less', { + includeGitPager: false, + platform: 'linux', + }), + ).toEqual({ + PAGER: 'less', + }); + }); + + it('uses the default pager without GIT_PAGER when git pager output is not requested', () => { + expect( + getShellPagerEnv(undefined, { + includeGitPager: false, + platform: 'linux', + }), + ).toEqual({ + PAGER: 'cat', + }); + }); + it('treats an empty pager as an explicit request to disable pager env values', () => { expect( getShellPagerEnv('', { includeGitPager: true, platform: 'linux' }), diff --git a/packages/core/src/utils/text-range-constants.ts b/packages/core/src/utils/text-range-constants.ts new file mode 100644 index 0000000000..57c2b2bc8b --- /dev/null +++ b/packages/core/src/utils/text-range-constants.ts @@ -0,0 +1,8 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +export const TEXT_RANGE_FAST_PATH_MAX_SIZE = 10 * 1024 * 1024; +export const DEFAULT_RANGE_READ_BYTES = 25_000; diff --git a/packages/sdk-typescript/src/daemon/events.ts b/packages/sdk-typescript/src/daemon/events.ts index 232a0d54a5..250ba0a48e 100644 --- a/packages/sdk-typescript/src/daemon/events.ts +++ b/packages/sdk-typescript/src/daemon/events.ts @@ -757,6 +757,7 @@ export interface DaemonTurnErrorData { sessionId: string; message: string; code?: string; + errorKind?: DaemonErrorKind | (string & {}); promptId?: string; [key: string]: unknown; } diff --git a/packages/sdk-typescript/src/daemon/types.ts b/packages/sdk-typescript/src/daemon/types.ts index fa7838f37d..fb4255f651 100644 --- a/packages/sdk-typescript/src/daemon/types.ts +++ b/packages/sdk-typescript/src/daemon/types.ts @@ -730,6 +730,8 @@ export const DAEMON_ERROR_KINDS = [ // An SSE writer's last successful flush was older than the daemon's // writer-idle deadline. 'writer_idle_timeout', + // The model response stream ended before a complete turn could be read. + 'model_stream_interrupted', ] as const; export type DaemonErrorKind = (typeof DAEMON_ERROR_KINDS)[number]; diff --git a/packages/sdk-typescript/src/daemon/ui/normalizer.ts b/packages/sdk-typescript/src/daemon/ui/normalizer.ts index d27d2b6c2c..37d697ea60 100644 --- a/packages/sdk-typescript/src/daemon/ui/normalizer.ts +++ b/packages/sdk-typescript/src/daemon/ui/normalizer.ts @@ -159,6 +159,7 @@ export function normalizeDaemonEvent( } case 'turn_error': { const code = getString(event.data, 'code'); + const errorKind = asDaemonErrorKind(getString(event.data, 'errorKind')); const promptId = getString(event.data, 'promptId'); return [ { @@ -167,6 +168,7 @@ export function normalizeDaemonEvent( source: 'turn_error', recoverable: true, ...(code ? { code } : {}), + ...(errorKind ? { errorKind } : {}), ...(promptId ? { promptId } : {}), text: getString(event.data, 'message') ?? diff --git a/packages/sdk-typescript/src/daemon/ui/transcript.ts b/packages/sdk-typescript/src/daemon/ui/transcript.ts index 2a619c1ceb..bdc8ac1a63 100644 --- a/packages/sdk-typescript/src/daemon/ui/transcript.ts +++ b/packages/sdk-typescript/src/daemon/ui/transcript.ts @@ -1016,6 +1016,9 @@ function appendStatusBlock( ...(event?.type === 'error' && event.promptId ? { promptId: event.promptId } : {}), + ...(event?.type === 'error' && event.errorKind + ? { errorKind: event.errorKind } + : {}), ...(event?.type === 'error' && event.source ? { source: event.source } : {}), diff --git a/packages/sdk-typescript/src/daemon/ui/types.ts b/packages/sdk-typescript/src/daemon/ui/types.ts index ef3b64bffb..0b7be64816 100644 --- a/packages/sdk-typescript/src/daemon/ui/types.ts +++ b/packages/sdk-typescript/src/daemon/ui/types.ts @@ -852,6 +852,7 @@ export interface DaemonStatusTranscriptBlock extends DaemonTranscriptBlockBase { text: string; code?: string; promptId?: string; + errorKind?: DaemonErrorKind; source?: string; data?: unknown; } diff --git a/packages/sdk-typescript/test/unit/daemonUi.test.ts b/packages/sdk-typescript/test/unit/daemonUi.test.ts index 10c90b9fa6..b45cca008c 100644 --- a/packages/sdk-typescript/test/unit/daemonUi.test.ts +++ b/packages/sdk-typescript/test/unit/daemonUi.test.ts @@ -1196,6 +1196,103 @@ describe('daemon UI normalizer and transcript reducer', () => { ); }); + it('preserves structured model stream interruption turn errors', () => { + expect( + normalizeDaemonEvent({ + id: 44, + v: 1, + type: 'turn_error', + data: { + sessionId: 'session-1', + message: 'terminated', + code: '-32603', + errorKind: 'model_stream_interrupted', + promptId: 'prompt-1', + }, + }), + ).toMatchObject([ + { + type: 'error', + source: 'turn_error', + recoverable: true, + code: '-32603', + errorKind: 'model_stream_interrupted', + promptId: 'prompt-1', + text: 'terminated', + }, + ]); + }); + + it('keeps structured model stream interruption on transcript blocks', () => { + const state = reduceDaemonTranscriptEvents( + createDaemonTranscriptState({ now: 100 }), + normalizeDaemonEvent({ + id: 46, + v: 1, + type: 'turn_error', + data: { + sessionId: 'session-1', + message: 'terminated', + code: '-32603', + errorKind: 'model_stream_interrupted', + promptId: 'prompt-1', + }, + }), + { now: 101 }, + ); + + expect(state.blocks).toMatchObject([ + { + kind: 'error', + source: 'turn_error', + text: 'terminated', + code: '-32603', + errorKind: 'model_stream_interrupted', + promptId: 'prompt-1', + }, + ]); + }); + + it('keeps older daemon terminated turn error text unchanged', () => { + expect( + normalizeDaemonEvent({ + id: 45, + v: 1, + type: 'turn_error', + data: { + sessionId: 'session-1', + message: 'terminated', + }, + }), + ).toMatchObject([ + { + type: 'error', + source: 'turn_error', + text: 'terminated', + }, + ]); + }); + + it('drops unrecognized errorKind from turn error events', () => { + const [event] = normalizeDaemonEvent({ + id: 47, + v: 1, + type: 'turn_error', + data: { + sessionId: 'session-1', + message: 'some error', + errorKind: 'some_future_kind', + }, + }); + + expect(event).toMatchObject({ + type: 'error', + source: 'turn_error', + text: 'some error', + }); + expect(event).not.toHaveProperty('errorKind'); + }); + it('normalizes daemon lifecycle and control events', () => { expect( normalizeDaemonEvent({ diff --git a/packages/web-shell/client/App.test.tsx b/packages/web-shell/client/App.test.tsx index 08e9cd8014..5bd3197ee2 100644 --- a/packages/web-shell/client/App.test.tsx +++ b/packages/web-shell/client/App.test.tsx @@ -808,6 +808,49 @@ describe('App session callbacks', () => { ); }); + it('allows manual retry after a model stream interrupted turn error', async () => { + const { container, rerender } = renderApp(); + await flush(); + + testState.prompt = 'recover this stream'; + await clickSubmit(container); + expect(mockSessionActions.sendPrompt).toHaveBeenCalledWith( + 'recover this stream', + expect.objectContaining({ retry: undefined }), + ); + + mockSessionActions.sendPrompt.mockClear(); + act(() => { + testState.blocks = [ + { + kind: 'error', + source: 'turn_error', + id: 'turn-error-stream-interrupted', + errorKind: 'model_stream_interrupted', + text: 'terminated', + }, + ]; + rerender(); + }); + + expect(container.querySelector('[data-testid="retry"]')).not.toBeNull(); + + await act(async () => { + container + .querySelector('[data-testid="retry"]') + ?.click(); + await Promise.resolve(); + }); + + expect(mockSessionActions.sendPrompt).toHaveBeenCalledWith( + 'recover this stream', + expect.objectContaining({ + optimisticUserMessage: false, + retry: true, + }), + ); + }); + it('gates queued submissions and only enqueues after approval', async () => { let approve: (() => void) | undefined; const onSubmitBefore = vi.fn( diff --git a/packages/web-shell/client/adapters/transcriptToMessages.test.ts b/packages/web-shell/client/adapters/transcriptToMessages.test.ts index bf935630ef..cf8cdcaf10 100644 --- a/packages/web-shell/client/adapters/transcriptToMessages.test.ts +++ b/packages/web-shell/client/adapters/transcriptToMessages.test.ts @@ -2179,6 +2179,131 @@ describe('transcriptBlocksToDaemonMessages', () => { ]); }); + it('renders model stream interruption errors from structured errorKind labels', () => { + const messages = transcriptBlocksToDaemonMessages( + [ + { + id: 'err-1', + kind: 'error' as const, + source: 'turn_error' as const, + errorKind: 'model_stream_interrupted' as const, + text: 'terminated', + data: { diagnosticId: 'abc' }, + clientReceivedAt: 1, + createdAt: 1, + updatedAt: 1, + }, + ], + { + labels: { + modelStreamInterrupted: 'Localized stream interruption.', + }, + }, + ); + + expect(messages).toEqual([ + { + id: 'err-1', + role: 'system', + content: 'Localized stream interruption.', + variant: 'error', + retryable: true, + source: 'turn_error', + data: { + diagnosticId: 'abc', + errorKind: 'model_stream_interrupted', + }, + timestamp: 1, + }, + ]); + }); + + it('upgrades older daemon terminated turn errors to localized text', () => { + const messages = transcriptBlocksToDaemonMessages( + [ + { + id: 'err-1', + kind: 'error' as const, + source: 'turn_error' as const, + text: 'terminated', + clientReceivedAt: 1, + createdAt: 1, + updatedAt: 1, + }, + ], + { + labels: { + modelStreamInterrupted: 'Localized stream interruption.', + }, + }, + ); + + expect(messages[0]).toMatchObject({ + content: 'Localized stream interruption.', + retryable: true, + source: 'turn_error', + }); + }); + + it('does not add data solely for structured errorKind labels', () => { + const messages = transcriptBlocksToDaemonMessages( + [ + { + id: 'err-1', + kind: 'error' as const, + source: 'turn_error' as const, + errorKind: 'model_stream_interrupted' as const, + text: 'terminated', + clientReceivedAt: 1, + createdAt: 1, + updatedAt: 1, + }, + ], + { + labels: { + modelStreamInterrupted: 'Localized stream interruption.', + }, + }, + ); + + expect(messages[0]).toMatchObject({ + content: 'Localized stream interruption.', + retryable: true, + source: 'turn_error', + }); + expect(messages[0]).not.toHaveProperty('data'); + }); + + it('preserves non-object error data when adding structured errorKind', () => { + const messages = transcriptBlocksToDaemonMessages( + [ + { + id: 'err-1', + kind: 'error' as const, + source: 'turn_error' as const, + errorKind: 'model_stream_interrupted' as const, + text: 'terminated', + data: 'diagnostic text', + clientReceivedAt: 1, + createdAt: 1, + updatedAt: 1, + }, + ], + { + labels: { + modelStreamInterrupted: 'Localized stream interruption.', + }, + }, + ); + + expect(messages[0]).toMatchObject({ + data: { + value: 'diagnostic text', + errorKind: 'model_stream_interrupted', + }, + }); + }); + it('converts debug blocks to system messages with info variant', () => { const messages = transcriptBlocksToDaemonMessages([ { diff --git a/packages/web-shell/client/adapters/transcriptToMessages.ts b/packages/web-shell/client/adapters/transcriptToMessages.ts index 3b09fedfca..f847bbc000 100644 --- a/packages/web-shell/client/adapters/transcriptToMessages.ts +++ b/packages/web-shell/client/adapters/transcriptToMessages.ts @@ -33,11 +33,6 @@ type DaemonPermissionTranscriptBlock = Extract< { kind: 'permission' } >; -type ExtendedDaemonStatusTranscriptBlock = DaemonStatusTranscriptBlock & { - source?: string; - data?: unknown; -}; - type ExtendedDaemonTextTranscriptBlock = DaemonTextTranscriptBlock & { meta?: { source?: unknown; @@ -50,6 +45,7 @@ interface TranscriptMessageLabels { promptCancelled?: string; branchSuccess?: (name: string) => string; midTurnInserted?: (message: string) => string; + modelStreamInterrupted?: string; } interface TranscriptMessageOptions { @@ -63,6 +59,35 @@ function isIgnoredWebShellStatus(text: string): boolean { ); } +function getErrorDisplayText( + block: DaemonStatusTranscriptBlock, + labels?: TranscriptMessageLabels, +): string { + if ( + block.errorKind === 'model_stream_interrupted' || + // Older daemons emit this turn_error before they know about errorKind. + (block.source === 'turn_error' && + block.text.trim().toLowerCase() === 'terminated') + ) { + return labels?.modelStreamInterrupted ?? block.text; + } + return block.text; +} + +function getErrorMessageData( + data: unknown, + errorKind: DaemonStatusTranscriptBlock['errorKind'], +): { data?: unknown } { + if (data === undefined) return {}; + if (!errorKind) return { data }; + return { + data: { + ...(getRecord(data) ?? { value: data }), + errorKind, + }, + }; +} + function getSessionBranchDisplayName(data: unknown): string | null { if (!data || typeof data !== 'object') return null; const branchData = data as { @@ -530,7 +555,7 @@ export function transcriptBlocksToDaemonMessages( case 'status': case 'debug': { - const statusBlock = block as ExtendedDaemonStatusTranscriptBlock; + const statusBlock = block; const branchDisplayName = statusBlock.source === 'session_branched' ? getSessionBranchDisplayName(statusBlock.data) @@ -575,16 +600,17 @@ export function transcriptBlocksToDaemonMessages( } case 'error': { - const errorBlock = block as ExtendedDaemonStatusTranscriptBlock; + const errorBlock = block; + const errorKind = errorBlock.errorKind; messages.push({ id: block.id, role: 'system', - content: errorBlock.text, + content: getErrorDisplayText(errorBlock, options.labels), variant: 'error', retryable: errorBlock.source === 'turn_error', timestamp: blockTime, ...(errorBlock.source ? { source: errorBlock.source } : {}), - ...(errorBlock.data !== undefined ? { data: errorBlock.data } : {}), + ...getErrorMessageData(errorBlock.data, errorKind), }); needsNewContentMessage = true; break; diff --git a/packages/web-shell/client/components/ChatPane.module.css b/packages/web-shell/client/components/ChatPane.module.css index 9e502a9212..b08a96e10f 100644 --- a/packages/web-shell/client/components/ChatPane.module.css +++ b/packages/web-shell/client/components/ChatPane.module.css @@ -10,10 +10,6 @@ overflow: hidden; } -.paneCurrent { - border-color: color-mix(in srgb, var(--primary) 60%, var(--border)); -} - .header { display: flex; align-items: center; @@ -52,16 +48,6 @@ color: var(--foreground); } -.currentBadge { - flex: 0 0 auto; - padding: 1px 7px; - border-radius: 999px; - font-size: 11px; - font-weight: 600; - color: var(--primary); - background: color-mix(in srgb, var(--primary) 14%, transparent); -} - .closeButton { flex: 0 0 auto; display: inline-flex; diff --git a/packages/web-shell/client/components/ChatPane.test.tsx b/packages/web-shell/client/components/ChatPane.test.tsx index 5a3e495e54..970cace7df 100644 --- a/packages/web-shell/client/components/ChatPane.test.tsx +++ b/packages/web-shell/client/components/ChatPane.test.tsx @@ -21,6 +21,7 @@ let latestOnSubmit: | undefined; let sendPromptResolve: (() => void) | undefined; let sendPromptReject: ((e: unknown) => void) | undefined; +let sendPromptAdmit: (() => void) | undefined; const sendPrompt = vi.fn(async () => ({}) as any); const submitPermission = vi.fn(async () => {}); const cancel = vi.fn(async () => {}); @@ -55,7 +56,10 @@ vi.mock('./StreamingStatus', () => ({ StreamingStatus: (props: any) => (
), })); @@ -120,10 +124,13 @@ beforeEach(() => { latestOnSubmit = undefined; sendPrompt.mockReset(); // Each sendPrompt returns a promise the test controls, so we can assert the - // draft is committed only after the prompt is accepted. + // draft is committed on admission (onAdmitted) rather than on turn completion + // (promise resolution). `sendPromptAdmit` captures the options.onAdmitted hook. + sendPromptAdmit = undefined; sendPrompt.mockImplementation( - () => + (_text?: string, options?: { onAdmitted?: () => void }) => new Promise((resolve, reject) => { + sendPromptAdmit = options?.onAdmitted; sendPromptResolve = () => resolve({}); sendPromptReject = (e) => reject(e); }), @@ -163,6 +170,13 @@ describe('ChatPane', () => { expect(container!.textContent).toContain('Refactor core'); }); + it('suppresses the rotating loading phrase in its compact status', () => { + render(); + expect(testid('pane-streaming')?.getAttribute('data-show-phrase')).toBe( + 'false', + ); + }); + it('sends a prompt to its own session on submit', () => { render(); act(() => @@ -174,16 +188,26 @@ describe('ChatPane', () => { expect((sendPrompt.mock.calls[0] as unknown[])[0]).toBe('hello there'); }); - it('commits the draft only after the prompt is accepted', async () => { + it('commits the draft on admission, without waiting for the turn to finish', async () => { render(); const commit = vi.fn(); let returned: boolean | undefined; act(() => { returned = latestOnSubmit!('hi', undefined, commit); }); - // Returns false (keep the draft) and does NOT commit before acceptance. + // Returns false (keep the draft) and does NOT commit before admission. expect(returned).toBe(false); expect(commit).not.toHaveBeenCalled(); + // The daemon admits the prompt (onAdmitted). The draft clears now, even + // though the turn promise is still pending — a long response must not strand + // the sent text in the composer until the turn ends. + await act(async () => { + sendPromptAdmit!(); + await Promise.resolve(); + }); + expect(commit).toHaveBeenCalledTimes(1); + // The turn finishing later must NOT commit again — guards against regressing + // to committing on promise resolution (turn end) instead of admission. await act(async () => { sendPromptResolve!(); await Promise.resolve(); @@ -204,6 +228,29 @@ describe('ChatPane', () => { expect(commit).not.toHaveBeenCalled(); }); + it('keeps the draft cleared and still reports the error when the turn fails after admission', async () => { + const onError = vi.fn(); + render({ onError }); + const commit = vi.fn(); + act(() => { + latestOnSubmit!('hi', undefined, commit); + }); + // Admission clears the draft. + await act(async () => { + sendPromptAdmit!(); + await Promise.resolve(); + }); + expect(commit).toHaveBeenCalledTimes(1); + // The turn then fails mid-flight: the draft stays cleared (no second commit) + // and the failure is still surfaced to onError. + await act(async () => { + sendPromptReject!(new Error('turn crashed')); + await Promise.resolve(); + }); + expect(commit).toHaveBeenCalledTimes(1); + expect(onError).toHaveBeenCalled(); + }); + it('keeps pane approvals click-only (no global keyboard shortcuts)', () => { pendingPermission = { id: 'perm-1', toolName: 'write_file', rawInput: {} }; render(); @@ -229,7 +276,11 @@ describe('ChatPane', () => { new MouseEvent('click', { bubbles: true }), ), ); - expect(submitPermission).toHaveBeenCalledWith('perm-1', 'proceed', undefined); + expect(submitPermission).toHaveBeenCalledWith( + 'perm-1', + 'proceed', + undefined, + ); }); it('routes an AskUserQuestion permission to the AskUserQuestion overlay', () => { diff --git a/packages/web-shell/client/components/ChatPane.tsx b/packages/web-shell/client/components/ChatPane.tsx index 4ae6c265a8..a640e694ce 100644 --- a/packages/web-shell/client/components/ChatPane.tsx +++ b/packages/web-shell/client/components/ChatPane.tsx @@ -30,8 +30,6 @@ const EMPTY_TOOLBAR: never[] = []; export interface ChatPaneProps { /** Header label; falls back to the session's own display name / id. */ title?: string; - /** Marks the pane bound to the window's primary (sidebar-selected) session. */ - isCurrent?: boolean; onClose?: () => void; onError?: (error: unknown, fallback: string) => void; } @@ -43,7 +41,7 @@ export interface ChatPaneProps { * state, approvals, and composer, and the browser scopes keyboard focus to the * pane the user clicks into — so there is no cross-pane approval arbitration. */ -export function ChatPane({ title, isCurrent, onClose, onError }: ChatPaneProps) { +export function ChatPane({ title, onClose, onError }: ChatPaneProps) { const { t } = useI18n(); const connection = useConnection(); const actions = useActions(); @@ -92,13 +90,19 @@ export function ChatPane({ title, isCurrent, onClose, onError }: ChatPaneProps) ): boolean => { const trimmed = text.trim(); if (!trimmed) return false; - // Keep the draft (return false) until the prompt is actually accepted: - // sendPrompt can reject (transcript still loading, session disconnected, - // or a turn already active), and committing first would silently drop the - // user's text. Commit only once it resolves. + // Keep the draft (return false) and clear it only once the daemon ADMITS + // the prompt. `onAdmitted` fires at acceptance; the sendPrompt promise + // itself resolves only when the whole (possibly long) turn finishes, so + // committing on resolution would strand the sent text in the composer for + // the entire response. If the prompt is rejected before admission + // (transcript still loading, session disconnected, or a turn already + // active) onAdmitted never fires, so the draft is preserved and the error + // is surfaced. actions - .sendPrompt(trimmed, images && images.length ? { images } : undefined) - .then(() => commitAccepted?.()) + .sendPrompt(trimmed, { + ...(images && images.length ? { images } : {}), + onAdmitted: commitAccepted, + }) .catch((error: unknown) => reportError(error, 'Failed to send prompt')); return false; }, @@ -119,7 +123,9 @@ export function ChatPane({ title, isCurrent, onClose, onError }: ChatPaneProps) const handleCancel = useCallback(() => { actions .cancel() - .catch((error: unknown) => reportError(error, 'Failed to cancel request')); + .catch((error: unknown) => + reportError(error, 'Failed to cancel request'), + ); }, [actions, reportError]); const headerLabel = @@ -127,9 +133,7 @@ export function ChatPane({ title, isCurrent, onClose, onError }: ChatPaneProps) return (
@@ -137,11 +141,6 @@ export function ChatPane({ title, isCurrent, onClose, onError }: ChatPaneProps) {headerLabel} - {isCurrent && ( - - {t('sessionsOverview.current')} - - )} {onClose && (
)} - + {/* Panes keep the composer status compact: spinner + elapsed time + + token count + cancel hint, but no rotating "witty" loading phrase. */} + ({ // Let a test force a render crash to exercise the per-pane ErrorBoundary. if (props.title === 'BOOM') throw new Error('pane exploded'); return ( -
+
{props.title} {props.onClose && ( )} + {orderedVisibleColumnIndexes.length > 0 && ( + + )} {activeFilterCount > 0 && ( {t('markdownTable.filtersActive', { count: activeFilterCount })} @@ -1670,10 +1950,14 @@ export function EnhancedTable({ - - {visibleColumnIndexes.map((columnIndex) => { + {orderedVisibleColumnIndexes.map((columnIndex) => { const header = table.headers[columnIndex]; if (!header) return null; const isSorted = sort?.columnIndex === columnIndex; @@ -1704,12 +1988,18 @@ export function EnhancedTable({ const headerAlignStyle = header.textAlign ? { textAlign: header.textAlign } : undefined; + const isFrozenColumn = frozenColumnIndex === columnIndex; + const isActiveColumn = activeColumn === columnIndex; return ( ); })} @@ -1760,7 +2087,11 @@ export function EnhancedTable({ - - {visibleColumnIndexes.map((columnIndex) => { + {orderedVisibleColumnIndexes.map((columnIndex) => { const cell = row.cells[columnIndex]; if (!cell) return null; const cellAlignStyle = cell.textAlign ? { textAlign: cell.textAlign } : undefined; + const isFrozenColumn = frozenColumnIndex === columnIndex; return (
+ {t('markdownTable.actions')} dropColumn(event, columnIndex)} + style={columnStyle(columnIndex, headerAlignStyle)} >
+
+
+ @@ -1816,13 +2148,13 @@ export function EnhancedTable({
{t('markdownTable.detailsHeader')}
- {visibleColumnIndexes.map((columnIndex) => { + {orderedVisibleColumnIndexes.map((columnIndex) => { const header = table.headers[columnIndex]; const cell = row.cells[columnIndex]; if (!header || !cell) return null; @@ -1881,7 +2213,7 @@ export function EnhancedTable({ sort={sort} style={{ left: openFilterMenu.left, top: openFilterMenu.top }} menuRef={filterMenuRef} - canHideColumn={visibleColumnIndexes.length > 1} + canHideColumn={orderedVisibleColumnIndexes.length > 1} onApply={setColumnFilter} onClose={closeFilterMenu} onHideColumn={hideColumn} diff --git a/packages/web-shell/client/components/messages/ToolGroup.test.tsx b/packages/web-shell/client/components/messages/ToolGroup.test.tsx index 03073352d6..d5544fe881 100644 --- a/packages/web-shell/client/components/messages/ToolGroup.test.tsx +++ b/packages/web-shell/client/components/messages/ToolGroup.test.tsx @@ -4,6 +4,7 @@ import { act } from 'react'; import { createRoot, type Root } from 'react-dom/client'; import type { ACPToolCall } from '../../adapters/types'; import { I18nProvider } from '../../i18n'; +import { WebShellCustomizationProvider } from '../../customization'; vi.mock('../../App', async () => { const { createContext } = await import('react'); @@ -19,7 +20,6 @@ const { extractDiff, fencedCodeBlock, formatSingleToolSummary, - formatRunningSingleToolSummary, formatToolGroupSummary, getActiveTool, getRawFileDiff, @@ -56,14 +56,17 @@ function makeTool(overrides: Partial = {}): ACPToolCall { }; } -function renderToolLine(tool: ACPToolCall): HTMLElement { +function renderToolLine( + tool: ACPToolCall, + props: Partial[0]> = {}, +): HTMLElement { const container = document.createElement('div'); document.body.appendChild(container); const root = createRoot(container); act(() => { root.render( - + , ); }); @@ -71,14 +74,19 @@ function renderToolLine(tool: ACPToolCall): HTMLElement { return container; } -function renderToolGroup(tools: ACPToolCall[]): HTMLElement { +function renderToolGroup( + tools: ACPToolCall[], + customization = {}, +): HTMLElement { const container = document.createElement('div'); document.body.appendChild(container); const root = createRoot(container); act(() => { root.render( - + + + , ); }); @@ -185,18 +193,51 @@ describe('tool group summary logic', () => { ); }); - it('formats a single tool summary from the tool itself', () => { + it('formats a single shell summary as only the semantic description', () => { + expect( + formatSingleToolSummary( + makeTool({ + toolName: 'run_shell_command', + args: { + command: 'dataworks-infra workspace list', + description: '查询用户工作空间列表', + timeout: 30000, + }, + }), + t, + ), + ).toBe('查询用户工作空间列表'); + }); + + it('falls back to command text for shell summaries without descriptions', () => { expect( formatSingleToolSummary( makeTool({ toolName: 'Shell', - args: { command: 'npm run build' }, + args: { command: 'npm run build', timeout: 30000 }, }), t, ), ).toBe('Shell npm run build'); }); + it('uses only skill names in single tool summaries', () => { + expect( + formatSingleToolSummary( + makeTool({ + toolName: 'skill', + title: + 'Skill: Use skill: "qc-helper" with args: "weather in Hangzhou next 5 days"', + args: { + skill: 'qc-helper', + args: 'weather in Hangzhou next 5 days', + }, + }), + t, + ), + ).toBe('Skill qc-helper'); + }); + it('uses action summaries for single todo and ask-user tools', () => { expect( formatSingleToolSummary(makeTool({ toolName: 'todo_write' }), t), @@ -206,20 +247,6 @@ describe('tool group summary logic', () => { ).toBe('Asked user'); }); - it('keeps running state and command details for a single active tool', () => { - expect( - formatRunningSingleToolSummary( - makeTool({ - toolName: 'Shell', - status: 'in_progress', - args: { command: 'npm run build' }, - }), - t, - '0:03', - ), - ).toBe('Running Shell npm run build 0:03'); - }); - it('truncates long single tool descriptions in the chat summary', () => { const summary = formatSingleToolSummary( makeTool({ @@ -232,6 +259,61 @@ describe('tool group summary logic', () => { expect(summary.length).toBeLessThan(140); expect(summary).toContain('...'); }); + + it('lets custom tool header extras render single-tool chat summaries', () => { + const container = renderToolGroup( + [ + makeTool({ + toolName: 'run_shell_command', + args: { + command: 'dataworks-infra workspace list', + description: '查询用户工作空间列表', + timeout: 30000, + }, + }), + ], + { + renderToolHeaderExtra: (info) => ( + + {info.kind}:{info.description} + + ), + }, + ); + + const summary = container.querySelector('button'); + expect(summary?.textContent).not.toContain('Shell'); + expect(summary?.textContent).toContain('shell:查询用户工作空间列表'); + expect(summary?.textContent).not.toContain('timeout: 30000ms'); + }); + + it('uses action descriptions for shell rows inside grouped summaries', () => { + const container = renderToolGroup([ + makeTool({ + callId: 'shell', + toolName: 'run_shell_command', + title: + 'Shell: dataworks-infra workspace list [timeout: 30000ms] (查询用户工作空间列表)', + args: { + command: 'dataworks-infra workspace list', + description: '查询用户工作空间列表', + timeout: 30000, + }, + }), + makeTool({ + callId: 'read', + toolName: 'read_file', + args: { file_path: 'README.md' }, + }), + ]); + + expect(container.textContent).toContain('Shell'); + expect(container.textContent).toContain('查询用户工作空间列表'); + expect(container.textContent).not.toContain( + 'dataworks-infra workspace list', + ); + expect(container.textContent).not.toContain('timeout: 30000ms'); + }); }); describe('tool expandability', () => { @@ -253,6 +335,32 @@ describe('tool expandability', () => { ), ).toBe(false); }); + + it('does not expand skill rows that only have the skill name', () => { + expect( + hasExpandableContent( + makeTool({ + toolName: 'skill', + title: 'Skill: Use skill: "review"', + args: { skill: 'review' }, + }), + ), + ).toBe(false); + expect( + hasExpandableContent( + makeTool({ + toolName: 'skill', + args: { skill: 'review' }, + content: [ + { + type: 'content', + content: { type: 'text', text: '# Code Review' }, + }, + ], + }), + ), + ).toBe(true); + }); }); describe('tool kind logic', () => { @@ -363,6 +471,84 @@ describe('tool row rendering', () => { expect(header.textContent).toContain(pattern); expect(header.textContent).toContain('packages/web-shell/client'); }); + + it('uses the shell tool name for expanded cards from action summaries', () => { + const container = renderToolLine( + makeTool({ + toolName: 'run_shell_command', + args: { + command: 'dataworks-infra workspace list', + description: '查询用户工作空间列表', + timeout: 30000, + }, + content: [ + { + type: 'content', + content: { type: 'text', text: 'failed\nwith details' }, + }, + ], + }), + { summaryOnly: true }, + ); + const header = container.querySelector('[role="button"]') as HTMLElement; + + expect(header.textContent).toContain('Shell'); + expect(header.textContent).toContain('查询用户工作空间列表'); + + act(() => header.click()); + + const cardTitle = container.querySelector('[class*="expandedCardTitle"]'); + expect(cardTitle?.textContent).toBe('Shell'); + }); + + it('shows complete skill content in the expanded card body', () => { + const container = renderToolLine( + makeTool({ + toolName: 'skill', + title: 'Skill: Use skill: "review" with args: "check the current diff"', + args: { + skill: 'review', + }, + content: [ + { + type: 'content', + content: { + type: 'text', + text: 'Base directory for this skill: /repo\n# Code Review', + }, + }, + ], + }), + ); + const header = container.querySelector('[role="button"]') as HTMLElement; + + expect(header.textContent).toContain('Skill'); + expect(header.textContent).toContain('review'); + expect(header.textContent).not.toContain('check the current diff'); + + act(() => header.click()); + + const output = container.querySelector('pre'); + expect(output?.textContent).toBe( + 'Base directory for this skill: /repo\n# Code Review', + ); + }); + + it('keeps running state for single todo summaries', () => { + const container = renderToolGroup([ + makeTool({ + toolName: 'todo_write', + status: 'in_progress', + args: { + todos: [{ id: '1', content: 'Check UI', status: 'in_progress' }], + }, + }), + ]); + const summary = container.querySelector('button'); + + expect(summary?.textContent).toContain('Running'); + expect(summary?.textContent).toContain('Updated task list'); + }); }); describe('tool output logic', () => { diff --git a/packages/web-shell/client/components/messages/ToolGroup.tsx b/packages/web-shell/client/components/messages/ToolGroup.tsx index 53e902f1bc..01d8c7e8f2 100644 --- a/packages/web-shell/client/components/messages/ToolGroup.tsx +++ b/packages/web-shell/client/components/messages/ToolGroup.tsx @@ -42,9 +42,12 @@ import { getAgentDisplayStatus, getAgentType, getTaskExecutionRecord, + getShellToolSemanticDescription, getToolDescription, + getToolSummaryDescription, getToolResultSummary, isAskUserQuestionToolName, + isSkillToolName, isShellToolName, toolContainsCallId, } from './toolFormatting'; @@ -77,6 +80,9 @@ export function hasExpandableContent(tool: ACPToolCall): boolean { const text = extractText(tool); return !!text && text.trim().length > 0 && text.split('\n').length > 1; } + if (isSkillToolName(name)) { + return !!getFirstToolContentText(tool); + } if (name === 'edit' || name === 'write' || name === 'editfile') { return hasEditContent(tool); } @@ -104,6 +110,7 @@ function hasDetailView(tool: ACPToolCall): boolean { name === 'read' || name === 'read_file' || name === 'readfile' || + isSkillToolName(name) || isAskUserQuestionToolName(tool.toolName) ); } @@ -477,6 +484,22 @@ function ExpandedAskUserQuestionOutput({ tool }: { tool: ACPToolCall }) { return
{text}
; } +function ExpandedSkillOutput({ tool }: { tool: ACPToolCall }) { + const content = + getFirstToolContentText(tool) || + (typeof tool.args?.args === 'string' && tool.args.args.trim() + ? tool.args.args.trim() + : (tool.title ?? '')); + + return
{content}
; +} + +function getFirstToolContentText(tool: ACPToolCall): string { + const block = tool.content?.[0]; + if (block?.type !== 'content') return ''; + return typeof block.content?.text === 'string' ? block.content.text : ''; +} + export function getToolHeaderKind(tool: ACPToolCall): ToolHeaderKind { const name = tool.toolName.toLowerCase(); if (isSubAgentToolCall(tool)) return 'agent'; @@ -575,22 +598,74 @@ export function formatSingleToolSummary( return t('toolGroup.summary.askedUser', { count: 1 }); } - const displayName = localizeToolDisplayName(tool.toolName, t); - const description = truncateText(getToolDescription(tool, workspaceCwd), 120); - return [displayName, description].filter(Boolean).join(' '); + const { displayName, description, hideDisplayName } = + getSingleToolSummaryInfo(tool, t, workspaceCwd); + return [hideDisplayName ? '' : displayName, description] + .filter(Boolean) + .join(' '); } -export function formatRunningSingleToolSummary( +function getSingleToolSummaryInfo( tool: ACPToolCall, t: ReturnType['t'], - duration?: string, workspaceCwd?: string, -): string { - return t('toolGroup.running', { - name: formatSingleToolSummary(tool, t, workspaceCwd), - count: 1, - duration: duration ?? '', - }); +): ToolHeaderExtraRenderInfo & { hideDisplayName: boolean } { + const displayName = localizeToolDisplayName(tool.toolName, t); + const description = truncateText( + getToolSummaryDescription(tool, workspaceCwd), + 120, + ); + return { + kind: getToolHeaderKind(tool), + tool, + displayName, + description, + elapsed: '', + workspaceCwd, + hideDisplayName: !!getShellToolSemanticDescription(tool), + }; +} + +function SingleToolSummary({ + tool, + runningDuration, + workspaceCwd, +}: { + tool: ACPToolCall; + runningDuration?: string; + workspaceCwd?: string; +}) { + const { t } = useI18n(); + const runningPrefix = + isActiveToolStatus(tool.status) && t('toolGroup.runningPrefix').trim(); + + if ( + isTodoWriteToolName(tool.toolName) || + isAskUserQuestionToolName(tool.toolName) + ) { + return ( + <> + {runningPrefix && {runningPrefix} } + {formatSingleToolSummary(tool, t, workspaceCwd)} + {runningDuration && {runningDuration}} + + ); + } + + const info = getSingleToolSummaryInfo(tool, t, workspaceCwd); + + return ( + <> + {runningPrefix && {runningPrefix} } + + {!info.hideDisplayName && ( + {info.displayName} + )} + + + {runningDuration && {runningDuration}} + + ); } function formatCompletedToolSummary( @@ -1046,6 +1121,12 @@ export const ToolLine = memo(function ToolLine({ workspaceCwd, }} /> +
)} {showExpanded && ( @@ -1061,8 +1142,12 @@ export const ToolLine = memo(function ToolLine({ ); } - const description = getToolDescription(tool, workspaceCwd); + const fullDescription = getToolDescription(tool, workspaceCwd); const result = getToolResultSummary(tool); + const summaryShell = summaryOnly && isShellToolName(tool.toolName); + const description = summaryShell + ? getToolSummaryDescription(tool, workspaceCwd) + : fullDescription; const displayName = localizeToolDisplayName(tool.toolName, t); const elapsed = isShellToolName(tool.toolName) || isWebFetchToolName(tool.toolName) @@ -1100,7 +1185,7 @@ export const ToolLine = memo(function ToolLine({ const useMarkdownDetail = isRead; const hideDescriptionInHeader = showDescriptionInDetail && !isShell && !isSearch && !isRead; - const expandedCardDetail = description; + const expandedCardDetail = fullDescription; const showExpandedSummaryPanel = !isTodo && expanded && !detailView && (showDescriptionInDetail || result); @@ -1158,6 +1243,14 @@ export const ToolLine = memo(function ToolLine({ workspaceCwd, }} /> + {expandable && ( + { ).toBe('cat ~/.qwen/settings.json (查看 ~/.qwen/settings.json 文件内容)'); }); + it('uses semantic shell descriptions for summaries', () => { + const shellTool = tool({ + toolName: 'run_shell_command', + title: + 'Shell: dataworks-infra workspace list [timeout: 30000ms] (查询用户工作空间列表)', + args: { + command: 'dataworks-infra workspace list', + description: '查询用户工作空间列表', + timeout: 30000, + }, + }); + + expect(getToolSummaryDescription(shellTool)).toBe('查询用户工作空间列表'); + expect(getToolDescription(shellTool)).toBe( + 'dataworks-infra workspace list [timeout: 30000ms] (查询用户工作空间列表)', + ); + }); + + it('falls back to shell commands in summaries without timeout metadata', () => { + expect( + getToolSummaryDescription( + tool({ + toolName: 'run_shell_command', + args: { + command: 'npm test', + timeout: 1000, + }, + }), + ), + ).toBe('npm test'); + }); + + it('describes skill calls from raw input', () => { + expect( + getToolDescription( + tool({ + toolName: 'skill', + args: { + skill: 'qc-helper', + args: 'weather in Hangzhou next 5 days', + }, + }), + ), + ).toBe('qc-helper'); + }); + it('summarizes read_file rawOutput by line count', () => { expect( getToolResultSummary( diff --git a/packages/web-shell/client/components/messages/toolFormatting.ts b/packages/web-shell/client/components/messages/toolFormatting.ts index 72b53ca072..8ca0d8e8e1 100644 --- a/packages/web-shell/client/components/messages/toolFormatting.ts +++ b/packages/web-shell/client/components/messages/toolFormatting.ts @@ -100,6 +100,10 @@ export function getToolDescription( tool: ACPToolCall, workspaceCwd?: string, ): string { + if (isSkillToolName(tool.toolName)) { + const skillName = getStringArg(tool.args, 'skill'); + if (skillName) return truncateText(skillName, MAX_DESCRIPTION_LENGTH); + } const fromTitle = getDescriptionFromTitle(tool, workspaceCwd); if (fromTitle) return truncateText(fromTitle, MAX_DESCRIPTION_LENGTH); const fromArgs = getDescriptionFromArgs(tool, workspaceCwd); @@ -107,6 +111,29 @@ export function getToolDescription( return ''; } +export function getToolSummaryDescription( + tool: ACPToolCall, + workspaceCwd?: string, +): string { + if (!isShellToolName(tool.toolName)) { + return getToolDescription(tool, workspaceCwd); + } + + const description = getStringArg(tool.args, 'description'); + if (description) return truncateText(description, MAX_DESCRIPTION_LENGTH); + + const fromArgs = getDescriptionFromArgs(tool, workspaceCwd, { + includeTimeout: false, + }); + if (fromArgs) return truncateText(fromArgs, MAX_DESCRIPTION_LENGTH); + return ''; +} + +export function getShellToolSemanticDescription(tool: ACPToolCall): string { + if (!isShellToolName(tool.toolName)) return ''; + return getStringArg(tool.args, 'description'); +} + export function extractText(tool: ACPToolCall): string | null { if (!tool.content) { return extractRawOutputText(tool.rawOutput); @@ -223,9 +250,11 @@ function parseGrepSummary(text: string): string | null { function getDescriptionFromArgs( tool: ACPToolCall, workspaceCwd?: string, + options: { includeTimeout?: boolean } = {}, ): string { const args = tool.args || {}; const name = tool.toolName.toLowerCase(); + const includeTimeout = options.includeTimeout ?? true; if (args.command) { let description = String(args.command); @@ -234,11 +263,12 @@ function getDescriptionFromArgs( } if (args.is_background) { description += ' [background]'; - } else if (args.timeout) { + } else if (includeTimeout && args.timeout) { description += ` [timeout: ${String(args.timeout)}ms]`; } - if (args.description) { - description += ` (${String(args.description).replace(/\n/g, ' ')})`; + const argDescription = getStringArg(args, 'description'); + if (argDescription) { + description += ` (${argDescription})`; } return truncateText(description, MAX_DESCRIPTION_LENGTH); } @@ -285,6 +315,14 @@ function getDescriptionFromArgs( return ''; } +function getStringArg( + args: Record | undefined, + key: string, +): string { + const value = args?.[key]; + return typeof value === 'string' ? value.trim().replace(/\n/g, ' ') : ''; +} + export function isShellToolName(name: string): boolean { const normalized = name.toLowerCase(); return ( @@ -295,6 +333,10 @@ export function isShellToolName(name: string): boolean { ); } +export function isSkillToolName(name: string): boolean { + return name.toLowerCase() === 'skill'; +} + export function toolContainsCallId( tool: ACPToolCall, toolCallId: string, diff --git a/packages/web-shell/client/components/messages/tools/ToolChrome.module.css b/packages/web-shell/client/components/messages/tools/ToolChrome.module.css index be33a4b9f9..0e9138ec5d 100644 --- a/packages/web-shell/client/components/messages/tools/ToolChrome.module.css +++ b/packages/web-shell/client/components/messages/tools/ToolChrome.module.css @@ -82,6 +82,16 @@ font-weight: 400; } +.chatSummaryInline { + display: inline-flex; + align-items: center; + gap: 4px; + min-width: 0; + max-width: 100%; + color: inherit; + vertical-align: bottom; +} + .chatSummaryTextActive { color: var(--muted-foreground); background-image: linear-gradient( @@ -193,12 +203,12 @@ align-items: center; gap: 8px; min-width: 0; -} - -.icon { color: var(--muted-foreground); font-size: 13px; font-weight: 400; +} + +.icon { flex-shrink: 0; white-space: nowrap; } @@ -208,15 +218,10 @@ } .lineName { - font-weight: 400; - color: var(--muted-foreground); - font-size: 13px; flex-shrink: 0; } .lineArg { - color: var(--muted-foreground); - font-size: 13px; min-width: 0; flex: 1; overflow: hidden; @@ -226,7 +231,6 @@ .lineElapsed { margin-left: auto; - color: var(--muted-foreground); font-size: 12px; flex-shrink: 0; } @@ -242,12 +246,53 @@ cursor: pointer; } -.lineExpandable:hover .lineName, -.lineExpandable:hover .lineArg, -.lineExpandable:hover .lineElapsed { +.lineExpandable .lineArg { + flex: 0 1 auto; +} + +.lineExpandable:hover, +.lineExpandable:focus-visible { color: var(--foreground); } +.lineChevronRight, +.lineChevronDown { + width: 12px; + height: 12px; + display: inline-block; + flex-shrink: 0; + opacity: 0; + transition: opacity 120ms ease; +} + +.lineChevronRight::before, +.lineChevronDown::before { + content: ''; + display: block; + width: 5px; + height: 5px; + border-right: 1.25px solid currentColor; + border-bottom: 1.25px solid currentColor; + border-radius: 0.5px; + margin: 3px 0 0 2px; +} + +.lineChevronRight::before { + transform: rotate(-45deg); +} + +.lineChevronDown::before { + margin: 2px 0 0 3px; + transform: rotate(45deg); +} + +.lineExpandable:hover .lineChevronRight, +.lineExpandable:hover .lineChevronDown, +.lineExpandable:focus-visible .lineChevronRight, +.lineExpandable:focus-visible .lineChevronDown { + opacity: 1; +} + .lineOutput { margin-left: 16px; font-size: 12px; @@ -386,6 +431,9 @@ align-items: baseline; gap: 8px; min-width: 0; + color: var(--muted-foreground); + font-size: 13px; + font-weight: 400; } .compactCount { diff --git a/packages/web-shell/client/hooks/useMessages.ts b/packages/web-shell/client/hooks/useMessages.ts index 9959ea3253..ddda0d1f14 100644 --- a/packages/web-shell/client/hooks/useMessages.ts +++ b/packages/web-shell/client/hooks/useMessages.ts @@ -14,6 +14,7 @@ export function useMessages( promptCancelled: t('request.cancelled'), branchSuccess: (name) => t('branch.success', { name }), midTurnInserted: (message) => t('midTurn.inserted', { message }), + modelStreamInterrupted: t('error.modelStreamInterrupted'), }, }), [blocks, t], diff --git a/packages/web-shell/client/i18n.tsx b/packages/web-shell/client/i18n.tsx index f231784c97..06716cb6b6 100644 --- a/packages/web-shell/client/i18n.tsx +++ b/packages/web-shell/client/i18n.tsx @@ -265,11 +265,15 @@ const EN: Messages = { }, 'markdownTable.copyTsv': 'Copy TSV', 'markdownTable.copyVisible': 'Quick copy', + 'markdownTable.freezeFirstColumn': 'Freeze first column', + 'markdownTable.unfreezeFirstColumn': 'Unfreeze first column', 'markdownTable.hideColumn': 'Hide column', 'markdownTable.showHiddenColumns': (v) => { const count = Number(v?.count ?? 0); return `Show ${count} hidden column${count === 1 ? '' : 's'}`; }, + 'markdownTable.moveColumn': (v) => `Move ${v?.column ?? ''}`, + 'markdownTable.resizeColumn': (v) => `Resize ${v?.column ?? ''}`, 'markdownTable.rowDetails': 'Details', 'markdownTable.rowDetailsAria': (v) => `View details for row ${v?.index ?? ''}`, @@ -827,6 +831,8 @@ const EN: Messages = { 'bug.submitted': 'Bug report opened in a new tab.', 'clear.blocked': 'Cannot clear while streaming — cancel first (Esc).', 'error.unknown': 'Unknown error', + 'error.modelStreamInterrupted': + 'Model response stream was interrupted. Please retry.', 'shell.command': 'Shell Command', 'compact.enabled': 'Compact mode enabled', 'compact.disabled': 'Compact mode disabled', @@ -1550,6 +1556,7 @@ const EN: Messages = { `Running ${v?.name ?? 'tool'}${v?.duration ? ` ${v.duration}` : ''}${ Number(v?.count ?? 0) > 1 ? ` · ${v?.count ?? 0} tools` : '' }`, + 'toolGroup.runningPrefix': 'Running', 'thinking.expand': 'Expand thinking', 'thinking.collapse': 'Collapse thinking', 'thinking.running': (v) => `Thinking${v?.duration ? ` ${v.duration}` : ''}`, @@ -1641,7 +1648,7 @@ const ZH: Messages = { 'toolName.todo_write': '任务清单', 'toolName.save_memory': '保存记忆', 'toolName.agent': 'Agent', - 'toolName.skill': '技能', + 'toolName.skill': '查看技能', 'toolName.enter_plan_mode': '进入计划模式', 'toolName.exit_plan_mode': '退出计划模式', 'toolName.web_fetch': '网络搜索', @@ -1907,8 +1914,12 @@ const ZH: Messages = { 'markdownTable.cellsSelected': (v) => `${v?.count ?? 0} 个单元格已选中`, 'markdownTable.copyTsv': '复制 TSV', 'markdownTable.copyVisible': '快捷复制', + 'markdownTable.freezeFirstColumn': '冻结首列', + 'markdownTable.unfreezeFirstColumn': '取消冻结首列', 'markdownTable.hideColumn': '隐藏列', 'markdownTable.showHiddenColumns': (v) => `显示 ${v?.count ?? 0} 个隐藏列`, + 'markdownTable.moveColumn': (v) => `移动 ${v?.column ?? ''}`, + 'markdownTable.resizeColumn': (v) => `调整 ${v?.column ?? ''} 列宽`, 'markdownTable.rowDetails': '详情', 'markdownTable.rowDetailsAria': (v) => `查看第 ${v?.index ?? ''} 行详情`, 'markdownTable.closeRowDetailsAria': (v) => `收起第 ${v?.index ?? ''} 行详情`, @@ -2439,6 +2450,7 @@ const ZH: Messages = { 'bug.submitted': 'Bug 报告已在新标签页中打开。', 'clear.blocked': '流式输出中无法清屏 — 先按 Esc 取消。', 'error.unknown': '未知错误', + 'error.modelStreamInterrupted': '模型响应流已中断,请重试。', 'shell.command': 'Shell 命令', 'compact.enabled': '紧凑模式已开启', 'compact.disabled': '紧凑模式已关闭', @@ -3115,6 +3127,7 @@ const ZH: Messages = { `正在执行 ${v?.name ?? '工具'}${v?.duration ? ` ${v.duration}` : ''}${ Number(v?.count ?? 0) > 1 ? ` · 共 ${v?.count ?? 0} 个工具` : '' }`, + 'toolGroup.runningPrefix': '正在执行', 'thinking.expand': '展开思考', 'thinking.collapse': '收起思考', 'thinking.running': (v) => `正在思考${v?.duration ? ` ${v.duration}` : ''}`, diff --git a/packages/webui/src/daemon/transcriptAdapter.test.ts b/packages/webui/src/daemon/transcriptAdapter.test.ts index cf0d0476ce..d87300331f 100644 --- a/packages/webui/src/daemon/transcriptAdapter.test.ts +++ b/packages/webui/src/daemon/transcriptAdapter.test.ts @@ -31,6 +31,89 @@ describe('daemonTranscriptToUnifiedMessages', () => { }); }); + it('renders model stream interruption turn errors with user-facing text', () => { + const [message] = daemonTranscriptToUnifiedMessages([ + { + id: 'error-1', + kind: 'error', + source: 'turn_error', + errorKind: 'model_stream_interrupted', + text: 'terminated', + clientReceivedAt: 1, + createdAt: 1, + updatedAt: 1, + }, + ]); + + expect(message).toMatchObject({ + type: 'tool_call', + toolCall: { + kind: 'system_error', + rawOutput: 'Model response stream was interrupted. Please retry.', + content: [ + { + content: { + text: 'Model response stream was interrupted. Please retry.', + error: 'Model response stream was interrupted. Please retry.', + }, + }, + ], + }, + }); + }); + + it('renders older daemon terminated turn errors with user-facing text', () => { + const [message] = daemonTranscriptToUnifiedMessages([ + { + id: 'error-legacy', + kind: 'error', + source: 'turn_error', + text: 'terminated', + clientReceivedAt: 1, + createdAt: 1, + updatedAt: 1, + }, + ]); + + expect(message).toMatchObject({ + type: 'tool_call', + toolCall: { + kind: 'system_error', + rawOutput: 'Model response stream was interrupted. Please retry.', + content: [ + { + content: { + text: 'Model response stream was interrupted. Please retry.', + error: 'Model response stream was interrupted. Please retry.', + }, + }, + ], + }, + }); + }); + + it('passes non-terminated turn error text through unchanged', () => { + const [message] = daemonTranscriptToUnifiedMessages([ + { + id: 'error-other', + kind: 'error', + source: 'turn_error', + text: 'context deadline exceeded', + clientReceivedAt: 1, + createdAt: 1, + updatedAt: 1, + }, + ]); + + expect(message).toMatchObject({ + type: 'tool_call', + toolCall: { + kind: 'system_error', + rawOutput: 'context deadline exceeded', + }, + }); + }); + it('maps daemon tool statuses without leaving terminal states spinning', () => { const messages = daemonTranscriptToUnifiedMessages([ createToolBlock('cancelled-tool', 'cancelled'), diff --git a/packages/webui/src/daemon/transcriptAdapter.ts b/packages/webui/src/daemon/transcriptAdapter.ts index 8492f23bbf..9fe3135840 100644 --- a/packages/webui/src/daemon/transcriptAdapter.ts +++ b/packages/webui/src/daemon/transcriptAdapter.ts @@ -10,6 +10,7 @@ import { isDaemonUiSensitiveKey, sanitizeDaemonTerminalText, type DaemonTranscriptBlock, + type DaemonStatusTranscriptBlock, type DaemonToolTranscriptBlock, } from '@qwen-code/sdk/daemon'; import type { UnifiedMessage } from '../adapters/types.js'; @@ -43,6 +44,9 @@ export interface DaemonTranscriptAdapterOptions { enrichToolDetailsWithPreview?: boolean; } +const MODEL_STREAM_INTERRUPTED_MESSAGE = + 'Model response stream was interrupted. Please retry.'; + export function daemonTranscriptToUnifiedMessages( blocks: readonly DaemonTranscriptBlock[], options: DaemonTranscriptAdapterOptions = {}, @@ -145,7 +149,8 @@ export function daemonTranscriptToUnifiedMessages( isLast, }, ]; - case 'error': + case 'error': { + const text = getErrorDisplayText(block); return [ { id: block.id, @@ -156,14 +161,14 @@ export function daemonTranscriptToUnifiedMessages( kind: 'system_error', title: 'System error', status: 'failed', - rawOutput: sanitizeDisplayText(block.text), + rawOutput: sanitizeDisplayText(text), content: [ { type: 'content', content: { type: 'error', - text: sanitizeDisplayText(block.text), - error: sanitizeDisplayText(block.text), + text: sanitizeDisplayText(text), + error: sanitizeDisplayText(text), }, }, ], @@ -172,6 +177,7 @@ export function daemonTranscriptToUnifiedMessages( isLast, }, ]; + } case 'status': return [ { @@ -196,6 +202,17 @@ export function daemonTranscriptToUnifiedMessages( }); } +function getErrorDisplayText(block: DaemonStatusTranscriptBlock): string { + if ( + block.errorKind === 'model_stream_interrupted' || + (block.source === 'turn_error' && + block.text.trim().toLowerCase() === 'terminated') + ) { + return MODEL_STREAM_INTERRUPTED_MESSAGE; + } + return block.text; +} + function daemonToolBlockToToolCallData( block: DaemonToolTranscriptBlock, enrichDetails: boolean = false, diff --git a/scripts/tests/qwen-autofix-workflow.test.js b/scripts/tests/qwen-autofix-workflow.test.js index efdec99a33..907633d769 100644 --- a/scripts/tests/qwen-autofix-workflow.test.js +++ b/scripts/tests/qwen-autofix-workflow.test.js @@ -7,6 +7,7 @@ import { execFileSync, spawnSync } from 'node:child_process'; import { chmodSync, + existsSync, mkdtempSync, readFileSync, rmSync, @@ -895,6 +896,109 @@ describe('qwen-autofix workflow', () => { ); }); + it('posts a human-handoff marker when review addressing reaches a terminal handoff', () => { + expect(reviewAddressReportStep).toContain( + "GITHUB_TOKEN: '${{ secrets.CI_DEV_BOT_PAT }}'", + ); + expect(reviewAddressReportStep).toContain( + "NEWEST: '${{ steps.prepare.outputs.newest }}'", + ); + expect(reviewAddressReportStep).toContain('"${DRY_RUN}" != "true"'); + expect(reviewAddressReportStep).toContain('-s "${WORKDIR}/handoff.md"'); + expect(reviewAddressReportStep).toContain( + '', + ); + expect(reviewAddressReportStep).toContain( + 'Could not address the latest review feedback automatically', + ); + expect(reviewAddressReportStep).toContain('gh pr comment "${PR}"'); + expect(reviewAddressReportStep).toContain( + 'GH_TOKEN="${GITHUB_TOKEN}" gh api user --jq \'.login\'', + ); + expect(reviewAddressReportStep).toContain( + 'CI_DEV_BOT_PAT authenticates as ${bot_actor}', + ); + expect(reviewAddressReportStep).toContain( + '::warning::Failed to post handoff comment on PR #${PR}', + ); + expect(reviewAddressReportStep).toContain('human should take over'); + expect(reviewAddressReportStep).toContain("sed 's///g'"); + }); + + it('writes agent output to a log and marks loop guard failures for handoff', () => { + withRunnerDir((dir) => { + writeFileSync(join(dir, 'feedback.md'), 'feedback\n'); + const stub = writeQwenStub(dir, [ + "process.stderr.write('turn_tool_call_cap: too many tool calls\\n');", + 'process.exit(1);', + ]); + + const result = runAddressReview(dir, stub); + + expect(result.status).not.toBe(0); + expect(readFileSync(join(dir, 'agent.log'), 'utf8')).toContain( + 'turn_tool_call_cap', + ); + expect(readFileSync(join(dir, 'failure.md'), 'utf8')).toContain( + 'Qwen hit the tool-call loop guard', + ); + expect(readFileSync(join(dir, 'handoff.md'), 'utf8')).toContain( + 'human should take over', + ); + }); + }); + + it('handles agent log stream errors without crashing immediately', () => { + expect(readFileSync(autofixRunnerScriptPath, 'utf8')).toContain( + "log.on('error', () => {});", + ); + expect(readFileSync(autofixRunnerScriptPath, 'utf8')).toContain( + 'if (log.destroyed)', + ); + }); + + it('detects loop guard output before it falls out of the log tail', () => { + withRunnerDir((dir) => { + writeFileSync(join(dir, 'feedback.md'), 'feedback\n'); + const stub = writeQwenStub(dir, [ + "process.stderr.write('Loop detection halted the run\\n');", + "process.stdout.write('x'.repeat(21_000));", + 'process.exit(1);', + ]); + + const result = runAddressReview(dir, stub); + + expect(result.status).not.toBe(0); + expect(readFileSync(join(dir, 'failure.md'), 'utf8')).toContain( + 'Qwen hit the tool-call loop guard', + ); + expect(readFileSync(join(dir, 'handoff.md'), 'utf8')).toContain( + 'human should take over', + ); + }); + }); + + it('does not mark generic qwen subprocess failures for handoff', () => { + withRunnerDir((dir) => { + writeFileSync(join(dir, 'feedback.md'), 'feedback\n'); + const stub = writeQwenStub(dir, [ + "process.stderr.write('temporary upstream error\\n');", + 'process.exit(1);', + ]); + + const result = runAddressReview(dir, stub); + + expect(result.status).not.toBe(0); + expect(readFileSync(join(dir, 'agent.log'), 'utf8')).toContain( + 'temporary upstream error', + ); + expect(readFileSync(join(dir, 'failure.md'), 'utf8')).toContain( + 'Qwen failed during address-review', + ); + expect(existsSync(join(dir, 'handoff.md'))).toBe(false); + }); + }); + it('preserves agent-written failure details when the qwen subprocess fails', () => { withRunnerDir((dir) => { writeFileSync(join(dir, 'candidates.json'), '[]\n'); @@ -909,14 +1013,60 @@ describe('qwen-autofix workflow', () => { expect(readFileSync(join(dir, 'failure.md'), 'utf8')).toContain( 'agent detail', ); + expect(readFileSync(join(dir, 'handoff.md'), 'utf8')).toContain( + 'human should take over', + ); }); }); it('bounds qwen subprocess runtime', () => { const runner = readFileSync(autofixRunnerScriptPath, 'utf8'); - expect(runner).toContain('const QWEN_TIMEOUT_MS = 50 * 60 * 1000'); - expect(runner).toContain('timeout: QWEN_TIMEOUT_MS'); + expect(runner).toContain('50 * 60 * 1000'); + expect(runner).toContain('setTimeout(() =>'); + expect(runner).toContain("killQwen(child, 'SIGKILL')"); + expect(runner).toContain('}, QWEN_TIMEOUT_MS)'); + }); + + it('kills qwen subprocess descendants on timeout', () => { + withRunnerDir((dir) => { + writeFileSync(join(dir, 'feedback.md'), 'feedback\n'); + const stub = writeQwenStub(dir, [ + "import { spawn } from 'node:child_process';", + "spawn(process.execPath, ['-e', 'setTimeout(() => {}, 3000)'], {", + " stdio: ['ignore', 'inherit', 'inherit'],", + '});', + 'setTimeout(() => process.exit(0), 3000);', + ]); + + const result = spawnSync( + process.execPath, + [ + autofixRunnerScriptPath, + '--mode', + 'address-review', + '--pr', + '5678', + '--issue', + '1234', + '--workdir', + dir, + '--qwen-bin', + stub, + ], + { + encoding: 'utf8', + env: { ...process.env, QWEN_TIMEOUT_MS: '100' }, + timeout: 2000, + }, + ); + + expect(result.error).toBeUndefined(); + expect(result.status).not.toBe(0); + expect(readFileSync(join(dir, 'failure.md'), 'utf8')).toContain( + 'timeout (100ms)', + ); + }); }); it('reports external qwen subprocess signals without calling them timeouts', () => { @@ -976,6 +1126,9 @@ describe('qwen-autofix workflow', () => { expect(readFileSync(join(dir, 'failure.md'), 'utf8')).toContain( 'cannot proceed', ); + expect(readFileSync(join(dir, 'handoff.md'), 'utf8')).toContain( + 'human should take over', + ); }); });