extract pick-up + file-resolver helpers for coverage
The T-339 pick-up test was the only importer of the ~450-line claude extension.dart, pulling its (mostly UI-wiring, untestable) lines into the coverage denominator and dropping the suite below the 95% floor. Move applyTicketPickUp into its own ticket_pick_up.dart and the T-300 path resolver into a pure resolveWorkspaceFilePath() — both small, fully covered, and imported by the tests instead of the whole extension. No behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -392,12 +392,19 @@ void _openUrl(BuildContext context, String url) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/// Resolve a path-like token from the conversation to an absolute workspace
|
/// Resolve a path-like token from the conversation to an absolute workspace
|
||||||
/// file, or null if it doesn't name a real repo file (T-300). The existence
|
/// file, or null if it doesn't name a real repo file (T-300). Delegates the
|
||||||
/// check is what keeps prose ("e.g.", "2.2.0") from linkifying. Resolves
|
/// (pure, testable) path logic to [resolveWorkspaceFilePath] with the open
|
||||||
/// relative tokens against the open project root; absolute tokens must already
|
/// project root.
|
||||||
/// live inside it. `..` segments are rejected so a ref can't escape the repo.
|
String? _resolveRepoFile(BuildContext context, String raw) =>
|
||||||
String? _resolveRepoFile(BuildContext context, String raw) {
|
resolveWorkspaceFilePath(ClideKernel.of(context).project.current?.path, raw);
|
||||||
final root = ClideKernel.of(context).project.current?.path;
|
|
||||||
|
/// Resolve [raw] (a path-like token) against the workspace [root] to an absolute
|
||||||
|
/// path, or null if it doesn't name a real file under the repo (T-300). The
|
||||||
|
/// existence check is what keeps prose ("e.g.", "2.2.0") from linkifying.
|
||||||
|
/// Relative tokens resolve against [root]; absolute tokens must already live
|
||||||
|
/// inside it. `..` segments are rejected so a ref can't escape the repo.
|
||||||
|
@visibleForTesting
|
||||||
|
String? resolveWorkspaceFilePath(String? root, String raw) {
|
||||||
if (root == null || raw.isEmpty || raw.contains('..')) return null;
|
if (root == null || raw.isEmpty || raw.contains('..')) return null;
|
||||||
final abs = raw.startsWith('/') ? raw : '$root/$raw';
|
final abs = raw.startsWith('/') ? raw : '$root/$raw';
|
||||||
if (!abs.startsWith('$root/')) return null;
|
if (!abs.startsWith('$root/')) return null;
|
||||||
|
|||||||
@@ -11,6 +11,7 @@ import 'package:clide/builtin/claude/src/pane_context_status.dart';
|
|||||||
import 'package:clide/builtin/claude/src/claude_meta_sidebar.dart';
|
import 'package:clide/builtin/claude/src/claude_meta_sidebar.dart';
|
||||||
import 'package:clide/builtin/claude/src/session_index.dart';
|
import 'package:clide/builtin/claude/src/session_index.dart';
|
||||||
import 'package:clide/builtin/claude/src/session_storage.dart';
|
import 'package:clide/builtin/claude/src/session_storage.dart';
|
||||||
|
import 'package:clide/builtin/claude/src/ticket_pick_up.dart';
|
||||||
import 'package:clide/builtin/claude/src/transcript_reader.dart' show ImageMessage;
|
import 'package:clide/builtin/claude/src/transcript_reader.dart' show ImageMessage;
|
||||||
import 'package:clide/src/daemon/image_commands.dart' show imageShowChannel;
|
import 'package:clide/src/daemon/image_commands.dart' show imageShowChannel;
|
||||||
import 'package:clide/builtin/claude/src/team_chat_sidebar.dart' show TeamChatPane;
|
import 'package:clide/builtin/claude/src/team_chat_sidebar.dart' show TeamChatPane;
|
||||||
@@ -471,41 +472,3 @@ class ClaudeExtension extends ClideExtension {
|
|||||||
return IpcResponse.ok(id: '', data: const {'status': 'shown'});
|
return IpcResponse.ok(id: '', data: const {'status': 'shown'});
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Statuses a pick-up may advance from: a not-yet-started ticket. Picking up a
|
|
||||||
/// ticket that's already `in_progress`/`review`/`done`/`cancelled` injects the
|
|
||||||
/// prompt but leaves the status alone, so a re-pick-up never drags it backwards
|
|
||||||
/// or reopens it (T-339).
|
|
||||||
const _kPickUpStartableStatuses = {'backlog', 'ready'};
|
|
||||||
|
|
||||||
/// Inject a picked-up ticket's prompt into the active session (the `primary`
|
|
||||||
/// lead, else the first visible one) and, on acceptance from a not-yet-started
|
|
||||||
/// ticket, advance it to `in_progress` and publish a `changed` so the sidebar
|
|
||||||
/// refreshes (T-327/T-339). Returns whether a live session accepted the prompt.
|
|
||||||
///
|
|
||||||
/// With no live session there's no injection and no state change — a quiet
|
|
||||||
/// no-op. Kept free of the extension lifecycle so it's directly testable.
|
|
||||||
@visibleForTesting
|
|
||||||
Future<bool> applyTicketPickUp(
|
|
||||||
Map<String, Object?> data, {
|
|
||||||
required ClaudeSessionOrchestrator? orchestrator,
|
|
||||||
required DaemonClient ipc,
|
|
||||||
required MessageBus messages,
|
|
||||||
}) async {
|
|
||||||
final prompt = data['prompt'] as String?;
|
|
||||||
if (prompt == null || prompt.isEmpty) return false;
|
|
||||||
final target = orchestrator?.byId('primary') ?? orchestrator?.visibleSessions.firstOrNull;
|
|
||||||
if (target == null) return false; // no live session → quiet no-op, no state change
|
|
||||||
orchestrator!.injectMessage(target.id, prompt);
|
|
||||||
|
|
||||||
final id = data['id'] as String?;
|
|
||||||
final status = data['status'] as String?;
|
|
||||||
if (id != null && id.isNotEmpty && _kPickUpStartableStatuses.contains(status)) {
|
|
||||||
final resp = await ipc.request('pql.tickets.status', args: {
|
|
||||||
'ids': [id],
|
|
||||||
'status': 'in_progress',
|
|
||||||
});
|
|
||||||
if (resp.ok) messages.publish('builtin.tickets', 'changed', {'id': id});
|
|
||||||
}
|
|
||||||
return true;
|
|
||||||
}
|
|
||||||
|
|||||||
@@ -0,0 +1,46 @@
|
|||||||
|
/// Sidebar "pick up" handling (T-327/T-339): inject a ticket's prompt into the
|
||||||
|
/// active Claude session and, on acceptance, advance the ticket to in_progress.
|
||||||
|
///
|
||||||
|
/// Kept out of `extension.dart` so it's unit-testable without dragging the whole
|
||||||
|
/// (UI-wiring) extension into instrumentation.
|
||||||
|
library;
|
||||||
|
|
||||||
|
import 'package:clide/builtin/claude/src/session_orchestrator.dart';
|
||||||
|
import 'package:clide/kernel/kernel.dart';
|
||||||
|
|
||||||
|
/// Statuses a pick-up may advance from: a not-yet-started ticket. Picking up a
|
||||||
|
/// ticket that's already `in_progress`/`review`/`done`/`cancelled` injects the
|
||||||
|
/// prompt but leaves the status alone, so a re-pick-up never drags it backwards
|
||||||
|
/// or reopens it (T-339).
|
||||||
|
const kPickUpStartableStatuses = {'backlog', 'ready'};
|
||||||
|
|
||||||
|
/// Inject a picked-up ticket's prompt into the active session (the `primary`
|
||||||
|
/// lead, else the first visible one) and, on acceptance from a not-yet-started
|
||||||
|
/// ticket, advance it to `in_progress` and publish a `changed` so the sidebar
|
||||||
|
/// refreshes (T-327/T-339). Returns whether a live session accepted the prompt.
|
||||||
|
///
|
||||||
|
/// With no live session there's no injection and no state change — a quiet
|
||||||
|
/// no-op.
|
||||||
|
Future<bool> applyTicketPickUp(
|
||||||
|
Map<String, Object?> data, {
|
||||||
|
required ClaudeSessionOrchestrator? orchestrator,
|
||||||
|
required DaemonClient ipc,
|
||||||
|
required MessageBus messages,
|
||||||
|
}) async {
|
||||||
|
final prompt = data['prompt'] as String?;
|
||||||
|
if (prompt == null || prompt.isEmpty) return false;
|
||||||
|
final target = orchestrator?.byId('primary') ?? orchestrator?.visibleSessions.firstOrNull;
|
||||||
|
if (target == null) return false; // no live session → quiet no-op, no state change
|
||||||
|
orchestrator!.injectMessage(target.id, prompt);
|
||||||
|
|
||||||
|
final id = data['id'] as String?;
|
||||||
|
final status = data['status'] as String?;
|
||||||
|
if (id != null && id.isNotEmpty && kPickUpStartableStatuses.contains(status)) {
|
||||||
|
final resp = await ipc.request('pql.tickets.status', args: {
|
||||||
|
'ids': [id],
|
||||||
|
'status': 'in_progress',
|
||||||
|
});
|
||||||
|
if (resp.ok) messages.publish('builtin.tickets', 'changed', {'id': id});
|
||||||
|
}
|
||||||
|
return true;
|
||||||
|
}
|
||||||
@@ -0,0 +1,55 @@
|
|||||||
|
/// Tests for the workspace file-path resolver behind clickable conversation
|
||||||
|
/// references (T-300): only real files under the repo root resolve; relative
|
||||||
|
/// tokens resolve against the root, absolute tokens must already live inside it,
|
||||||
|
/// and `..` escapes are rejected.
|
||||||
|
library;
|
||||||
|
|
||||||
|
import 'dart:io';
|
||||||
|
|
||||||
|
import 'package:clide/builtin/claude/src/conversation_view.dart';
|
||||||
|
import 'package:flutter_test/flutter_test.dart';
|
||||||
|
|
||||||
|
void main() {
|
||||||
|
late Directory root;
|
||||||
|
|
||||||
|
setUp(() async {
|
||||||
|
root = await Directory.systemTemp.createTemp('clide_wsfile_');
|
||||||
|
await File('${root.path}/lib/app.dart').create(recursive: true);
|
||||||
|
});
|
||||||
|
|
||||||
|
tearDown(() async {
|
||||||
|
if (await root.exists()) await root.delete(recursive: true);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('a relative path that exists resolves to its absolute path', () {
|
||||||
|
expect(resolveWorkspaceFilePath(root.path, 'lib/app.dart'), '${root.path}/lib/app.dart');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('an absolute path inside the root resolves', () {
|
||||||
|
expect(resolveWorkspaceFilePath(root.path, '${root.path}/lib/app.dart'), '${root.path}/lib/app.dart');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('a nonexistent path is null', () {
|
||||||
|
expect(resolveWorkspaceFilePath(root.path, 'lib/ghost.dart'), isNull);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('a directory is not a file', () {
|
||||||
|
expect(resolveWorkspaceFilePath(root.path, 'lib'), isNull);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('an absolute path outside the root is rejected', () {
|
||||||
|
expect(resolveWorkspaceFilePath(root.path, '/etc/passwd'), isNull);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('a `..` escape is rejected', () {
|
||||||
|
expect(resolveWorkspaceFilePath(root.path, '../escape.dart'), isNull);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('a null root (no project open) is null', () {
|
||||||
|
expect(resolveWorkspaceFilePath(null, 'lib/app.dart'), isNull);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('an empty token is null', () {
|
||||||
|
expect(resolveWorkspaceFilePath(root.path, ''), isNull);
|
||||||
|
});
|
||||||
|
}
|
||||||
@@ -5,9 +5,9 @@ library;
|
|||||||
|
|
||||||
import 'dart:async';
|
import 'dart:async';
|
||||||
|
|
||||||
import 'package:clide/builtin/claude/src/extension.dart';
|
|
||||||
import 'package:clide/builtin/claude/src/session_orchestrator.dart';
|
import 'package:clide/builtin/claude/src/session_orchestrator.dart';
|
||||||
import 'package:clide/builtin/claude/src/stream_json_session.dart';
|
import 'package:clide/builtin/claude/src/stream_json_session.dart';
|
||||||
|
import 'package:clide/builtin/claude/src/ticket_pick_up.dart';
|
||||||
import 'package:clide/clide.dart';
|
import 'package:clide/clide.dart';
|
||||||
import 'package:clide/kernel/kernel.dart';
|
import 'package:clide/kernel/kernel.dart';
|
||||||
import 'package:flutter_test/flutter_test.dart';
|
import 'package:flutter_test/flutter_test.dart';
|
||||||
|
|||||||
Reference in New Issue
Block a user