Skip to content

Commit 9035890

Browse files
authored
Merge pull request #4372 from nightt5879/nightt5879/issue-3915-skill-inline-task
fix(skills): preserve inline task text
2 parents 352e86a + 66a6edf commit 9035890

6 files changed

Lines changed: 155 additions & 8 deletions

File tree

.github/AUTHOR_MAP

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ zlh124 = zlh124 <56312993+zlh124@users.noreply.github.com>
5050
LeoLin990405 = LeoLin990405 <101193087+LeoLin990405@users.noreply.github.com>
5151
THINKER-ONLY = THINKER-ONLY <181556007+THINKER-ONLY@users.noreply.github.com>
5252
nightt5879 = nightt5879 <87569709+nightt5879@users.noreply.github.com>
53+
CCChisato = Fushimi Rio <158128433+CCChisato@users.noreply.github.com>
5354
aznikline = aznikline <27564626+aznikline@users.noreply.github.com>
5455
Aznable = aznikline <27564626+aznikline@users.noreply.github.com>
5556
anikline@gmail.com = aznikline <27564626+aznikline@users.noreply.github.com>

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
99

1010
### Fixed
1111

12+
- Preserve the trailing task in inline skill invocations: `$<skill> do X`,
13+
`/<skill> do X`, and `/skill <skill> do X` now dispatch `do X` immediately
14+
with the selected skill instructions attached, while a bare `$<skill>` still
15+
arms the next message. A skill literally named `install` remains callable
16+
through `$install` and `/install` without shadowing the existing `/skill
17+
install` management path (#3915, with fix direction from @CCChisato).
1218
- Make offline `scorecard` pricing provider-aware: `turn_end` records carry the
1319
effective route and a non-secret billing surface, runtime exports and
1420
supported aliases ingest cleanly, legacy/unknown routes remain explicitly

crates/tui/CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
99

1010
### Fixed
1111

12+
- Preserve the trailing task in inline skill invocations: `$<skill> do X`,
13+
`/<skill> do X`, and `/skill <skill> do X` now dispatch `do X` immediately
14+
with the selected skill instructions attached, while a bare `$<skill>` still
15+
arms the next message. A skill literally named `install` remains callable
16+
through `$install` and `/install` without shadowing the existing `/skill
17+
install` management path (#3915, with fix direction from @CCChisato).
1218
- Make offline `scorecard` pricing provider-aware: `turn_end` records carry the
1319
effective route and a non-secret billing surface, runtime exports and
1420
supported aliases ingest cleanly, legacy/unknown routes remain explicitly

crates/tui/src/commands/groups/skills/skills.rs

Lines changed: 116 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use crate::skills::install::{
88
self, DEFAULT_MAX_SIZE_BYTES, DEFAULT_REGISTRY_URL, InstallOutcome, InstallSource,
99
RegistryFetchResult, SkillSyncOutcome, SyncResult, UpdateResult,
1010
};
11-
use crate::tui::app::App;
11+
use crate::tui::app::{App, AppAction};
1212
use crate::tui::history::HistoryCell;
1313

1414
use crate::commands::CommandResult;
@@ -280,18 +280,17 @@ fn list_skills(app: &mut App, arg: Option<&str>) -> CommandResult {
280280
CommandResult::message(output)
281281
}
282282

283-
/// Run a specific skill — activates skill for next user message, or
284-
/// dispatches a sub-command (`install`, `update`, `uninstall`, `trust`).
285283
/// Try to run a skill by exact name (used for unified slash-command namespace, #435).
284+
/// A trailing request is dispatched immediately; a bare invocation arms the next message.
286285
/// Returns None when no skill with that name exists, so the caller can try other sources.
287286
pub(in crate::commands) fn run_skill_by_name(
288287
app: &mut App,
289288
name: &str,
290-
_arg: Option<&str>,
289+
request: Option<&str>,
291290
) -> Option<CommandResult> {
292291
let registry = discover_visible_skills(app);
293292
if registry.get(name).is_some() {
294-
Some(activate_skill(app, name))
293+
Some(activate_skill_for_request(app, name, request))
295294
} else {
296295
None
297296
}
@@ -312,6 +311,8 @@ fn run_skill(app: &mut App, name: Option<&str>) -> CommandResult {
312311
let mut iter = raw.splitn(2, char::is_whitespace);
313312
let head = iter.next().unwrap_or("").trim();
314313
let rest = iter.next().unwrap_or("").trim();
314+
let request = (!rest.is_empty()).then_some(rest);
315+
315316
match head {
316317
"install" => return install_skill(app, rest),
317318
"update" => return update_skill(app, rest),
@@ -320,7 +321,21 @@ fn run_skill(app: &mut App, name: Option<&str>) -> CommandResult {
320321
_ => {}
321322
}
322323

323-
activate_skill(app, raw)
324+
activate_skill_for_request(app, head, request)
325+
}
326+
327+
fn activate_skill_for_request(app: &mut App, name: &str, request: Option<&str>) -> CommandResult {
328+
let mut result = activate_skill(app, name);
329+
if result.is_error {
330+
return result;
331+
}
332+
333+
if let Some(request) = request {
334+
result.action = Some(AppAction::SendMessage(request.to_string()));
335+
} else if let Some(message) = result.message.as_mut() {
336+
message.push_str("\n\nType your request and the skill instructions will be applied.");
337+
}
338+
result
324339
}
325340

326341
fn activate_skill(app: &mut App, name: &str) -> CommandResult {
@@ -342,7 +357,7 @@ fn activate_skill(app: &mut App, name: &str) -> CommandResult {
342357
app.active_skill = Some(instruction);
343358

344359
CommandResult::message(format!(
345-
"Skill '{}' activated.\n\nDescription: {}\n\nType your request and the skill instructions will be applied.",
360+
"Skill '{}' activated.\n\nDescription: {}",
346361
skill.name, skill.description
347362
))
348363
} else {
@@ -747,7 +762,7 @@ impl crate::commands::traits::RegisterCommand for SkillCmd {
747762
mod tests {
748763
use super::*;
749764
use crate::config::Config;
750-
use crate::tui::app::{App, TuiOptions};
765+
use crate::tui::app::{App, AppAction, TuiOptions};
751766
use std::ffi::OsString;
752767
use tempfile::TempDir;
753768

@@ -1225,6 +1240,99 @@ mod tests {
12251240
assert!(msg.contains("/skill install"), "got: {msg}");
12261241
}
12271242

1243+
#[test]
1244+
fn inline_skill_invocations_preserve_task_text() {
1245+
let tmpdir = TempDir::new().unwrap();
1246+
let _home = IsolatedHome::new(&tmpdir);
1247+
create_skill_dir(
1248+
&tmpdir,
1249+
"test-skill",
1250+
"---\nname: test-skill\ndescription: A test skill\n---\nDo something special",
1251+
);
1252+
1253+
for input in [
1254+
"$test-skill do X",
1255+
"/test-skill do X",
1256+
"/skill test-skill do X",
1257+
] {
1258+
let mut app = create_test_app_with_tmpdir(&tmpdir);
1259+
let result = crate::commands::execute(input, &mut app);
1260+
1261+
assert!(!result.is_error, "{input} failed: {result:?}");
1262+
assert!(
1263+
matches!(result.action, Some(AppAction::SendMessage(ref task)) if task == "do X"),
1264+
"{input} did not send the trailing task: {result:?}"
1265+
);
1266+
assert!(
1267+
app.active_skill
1268+
.as_deref()
1269+
.is_some_and(|instruction| instruction.contains("Do something special")),
1270+
"{input} did not retain the skill instruction"
1271+
);
1272+
}
1273+
}
1274+
1275+
#[test]
1276+
fn bare_dollar_skill_still_arms_the_next_message() {
1277+
let tmpdir = TempDir::new().unwrap();
1278+
let _home = IsolatedHome::new(&tmpdir);
1279+
create_skill_dir(
1280+
&tmpdir,
1281+
"test-skill",
1282+
"---\nname: test-skill\ndescription: A test skill\n---\nDo something special",
1283+
);
1284+
let mut app = create_test_app_with_tmpdir(&tmpdir);
1285+
1286+
let result = crate::commands::execute("$test-skill", &mut app);
1287+
1288+
assert!(!result.is_error, "unexpected activation error: {result:?}");
1289+
assert!(result.action.is_none(), "bare invocation sent a request");
1290+
assert!(
1291+
app.active_skill
1292+
.as_deref()
1293+
.is_some_and(|instruction| instruction.contains("Do something special"))
1294+
);
1295+
}
1296+
1297+
#[test]
1298+
fn install_skill_uses_unified_namespaces_without_shadowing_management() {
1299+
let tmpdir = TempDir::new().unwrap();
1300+
let _home = IsolatedHome::new(&tmpdir);
1301+
create_skill_dir(
1302+
&tmpdir,
1303+
"install",
1304+
"---\nname: install\ndescription: Install review skill\n---\nReview installs safely",
1305+
);
1306+
1307+
for input in ["$install audit this", "/install audit this"] {
1308+
let mut app = create_test_app_with_tmpdir(&tmpdir);
1309+
let result = crate::commands::execute(input, &mut app);
1310+
1311+
assert!(!result.is_error, "{input} failed: {result:?}");
1312+
assert!(
1313+
matches!(result.action, Some(AppAction::SendMessage(ref task)) if task == "audit this"),
1314+
"{input} did not send the skill task: {result:?}"
1315+
);
1316+
assert!(
1317+
app.active_skill
1318+
.as_deref()
1319+
.is_some_and(|instruction| instruction.contains("Review installs safely")),
1320+
"{input} did not activate the installed skill"
1321+
);
1322+
}
1323+
1324+
let mut app = create_test_app_with_tmpdir(&tmpdir);
1325+
let management = crate::commands::execute("/skill install", &mut app);
1326+
assert!(management.is_error, "management command was shadowed");
1327+
assert!(
1328+
management
1329+
.message
1330+
.as_deref()
1331+
.is_some_and(|message| message.contains("/skill install <"))
1332+
);
1333+
assert!(app.active_skill.is_none());
1334+
}
1335+
12281336
#[test]
12291337
fn test_skill_subcommand_dispatch_uninstall_missing() {
12301338
let tmpdir = TempDir::new().unwrap();

crates/tui/src/tui/ui/tests.rs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8390,6 +8390,29 @@ async fn streaming_enter_queue_pushes_visible_toast() {
83908390
assert!(toast.text.contains("Queued follow-up"));
83918391
}
83928392

8393+
#[tokio::test]
8394+
async fn inline_skill_request_keeps_instruction_when_busy_queueing() {
8395+
let mut app = create_test_app();
8396+
app.is_loading = true;
8397+
app.streaming_message_index = Some(0);
8398+
app.active_skill = Some("Use the test skill".to_string());
8399+
let config = Config::default();
8400+
let engine = crate::core::engine::mock_engine_handle();
8401+
8402+
let queued = build_queued_message(&mut app, "do X".to_string());
8403+
assert!(app.active_skill.is_none(), "skill must be consumed once");
8404+
submit_or_steer_message(&mut app, &config, &engine.handle, queued)
8405+
.await
8406+
.expect("streaming skill request queues");
8407+
8408+
let queued = app.queued_messages.front().expect("queued skill request");
8409+
assert_eq!(queued.display, "do X");
8410+
assert_eq!(
8411+
queued.skill_instruction.as_deref(),
8412+
Some("Use the test skill")
8413+
);
8414+
}
8415+
83938416
#[tokio::test]
83948417
async fn numeric_plan_choice_still_queues_follow_up_when_busy() {
83958418
let mut app = create_test_app();

docs/CONTRIBUTORS.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,9 @@ and dead-code removal landed alongside Cursor dogfood evidence for v0.8.67.
4444
bindgen support for codewhale-workflow-js (PR #4349)
4545
- **[eugenicum](https://github.com/eugenicum)** — copy-paste rail-pollution
4646
report with code-aware fix direction (#4208)
47+
- **[Fushimi Rio / CCChisato](https://github.com/CCChisato)** — inline skill
48+
task pass-through fix direction and the exact-name `install` skill collision
49+
identified in #3915
4750

4851
- **[Jeffrey Luna / Mr-Moon121](https://github.com/Mr-Moon121)** — anti-polling
4952
constitution for sub-agent waiting (harvested into #4097 / PR #4229 from

0 commit comments

Comments
 (0)