fix(acp): use global session list for unstable_listSessions

[Haohao-end]

Issue for this PR

Closes #18575

Type of change

• Bug fix
• New feature
• Refactor / code improvement
• Documentation

What does this PR do?

unstable_listSessions was using the project-scoped session list path, so ACP only returned sessions for the current project.

This changes it to use the global session list path and keeps the existing response mapping and pagination behavior.

How did you verify your code works?

• Added a regression test for unstable_listSessions
• Ran:
  • bun test test/acp/list-sessions.test.ts
  • bun typecheck

Screenshots / recordings

N/A

Checklist

• I have tested my changes locally
• I have not included unrelated changes in this PR

[github-actions]

The following comment was made by an LLM, it may be inaccurate:

Potential duplicate found:

• PR fix(acp): use global session list in unstable_listSessions #18576: "fix(acp): use global session list in unstable_listSessions"
  • fix(acp): use global session list in unstable_listSessions #18576

This PR appears to be addressing the exact same issue (#18575) with nearly identical changes. Both PRs fix the same problem where ACP's unstable_listSessions needs to use the global session list instead of the project-scoped one. The title and scope are virtually identical.

You should check whether PR #18576 is already merged or if one of these PRs should be closed in favor of the other.

[atharvau]

Code Review for PR #18645: ACP Session List Fix

✅ Overall Assessment: APPROVED

This PR fixes a significant issue in the ACP agent by switching from local session filtering to the global experimental session list API. This is both a correctness fix and a performance improvement.

---

🔍 Changes Summary

File: packages/opencode/src/acp/agent.ts

• Replaces manual session filtering with sdk.experimental.session.list
• Uses proper pagination with cursor from response headers
• Removes manual sorting and slicing logic

File: packages/opencode/test/acp/list-sessions.test.ts

• Adds comprehensive test coverage for the new implementation
• Mocks SDK properly to test pagination behavior

---

✅ What's Good

1. Correct API Usage: Now uses the proper global session list instead of local directory-scoped filtering
2. Better Performance: Delegates pagination to the server instead of client-side processing
3. Proper Pagination: Uses standard cursor-based pagination from response headers
4. Excellent Test Coverage: Comprehensive test that verifies the API calls and response mapping
5. Clean Refactoring: Removes ~15 lines of manual pagination logic

---

🔧 Code Quality

• Simpler Logic: Much cleaner code without manual sorting/filtering
• Proper Error Handling: Maintains throwOnError: true behavior
• Consistent Mapping: Session response mapping remains unchanged

---

🧪 Testing

The test is well-structured:

• Good Mocking: Creates realistic SDK mock with proper response structure
• Proper Cleanup: Uses try/finally to ensure agent cleanup
• Complete Coverage: Tests both input parameters and output mapping
• Realistic Data: Uses actual timestamp values and proper data structure

---

🚀 Recommendation: APPROVE

This is an excellent fix that corrects a fundamental issue while improving code quality and adding proper test coverage. The change simplifies the implementation while making it more correct.

Rating: ⭐⭐⭐⭐⭐

[atharvau]

Code Review Summary

Overall Assessment: ✅ APPROVE with Minor Concerns

Good refactoring to use the global session API, but needs consideration of behavioral changes.

✅ Strengths:

• Better API Usage: Migrates from deprecated local session list to global experimental API
• Proper Pagination: Uses cursor-based pagination from headers
• Comprehensive Tests: Well-written test coverage validates the new behavior
• Performance: More efficient than client-side filtering/sorting

🟡 Potential Issues:

• Behavioral Change: Now returns global sessions instead of directory-scoped sessions
• API Dependency: Relies on experimental API (stability concerns)
• Parameter Ignored: params.cwd parameter is completely ignored now

Security & Performance:

• ✅ No security issues
• ✅ Better performance (server-side pagination)
• ✅ Proper error handling

Breaking Changes:

• ⚠️ API behavior change: Previously filtered by directory, now returns global sessions
• This may affect clients expecting directory-scoped results

Recommendation: Approve, but consider if the behavioral change is intentional and documented.

[Haohao-end]

Consolidating on this PR only.

PR #18743 was a duplicate follow-up and has been closed. I’ll keep any further updates for #18575 on this PR.

[Haohao-end]

Reopening this PR.

I accidentally closed it while trying to respond to the review thread.

To clarify the behavior change: this is intentional for #18575. unstable_listSessions should return global sessions in ACP mode, and the previous params.cwd-scoped behavior was the bug described in the issue.