Skip to content

Commit 6784b62

Browse files
tedkahwajiclaude
andcommitted
fix(onboarding): require a skill_id for terminal session statuses
clap derives Vec<String> as optional, so `sessions create` would post an empty skill_ids and defer the error to the backend. Mirror the API's SessionStatus.IsTerminal() rule client-side: require at least one --skill-id for completed/failed/abandoned, still allow empty for in_progress. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 0e4ac4b commit 6784b62

2 files changed

Lines changed: 50 additions & 1 deletion

File tree

src/commands/onboarding.rs

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ const SKILLS_PATH: &str = "/api/v2/onboarding/skills";
99
const SESSIONS_PATH: &str = "/api/v2/onboarding/sessions";
1010

1111
const VALID_STATUSES: &[&str] = &["in_progress", "completed", "failed", "abandoned"];
12+
// Terminal statuses require at least one skill_id; the backend allows in_progress to be empty.
13+
const TERMINAL_STATUSES: &[&str] = &["completed", "failed", "abandoned"];
1214
const VALID_INTENTS: &[&str] = &["explore", "install", "reference"];
1315

1416
/// List the available Datadog onboarding skills (`GET /api/v2/onboarding/skills`).
@@ -73,6 +75,10 @@ pub async fn sessions_create(
7375
);
7476
}
7577

78+
if TERMINAL_STATUSES.contains(&status) && skill_ids.is_empty() {
79+
anyhow::bail!("at least one --skill-id is required when status is '{status}'");
80+
}
81+
7682
let mut attributes = serde_json::json!({
7783
"skill_ids": skill_ids,
7884
"summary": summary,
@@ -301,4 +307,47 @@ mod tests {
301307
assert!(err.contains("invalid status"), "got: {err}");
302308
cleanup_env();
303309
}
310+
311+
#[tokio::test]
312+
async fn sessions_create_terminal_requires_skill_id() {
313+
let _lock = lock_env().await;
314+
let server = mockito::Server::new_async().await;
315+
let cfg = test_config(&server.url());
316+
let result =
317+
super::sessions_create(&cfg, "run-123", &[], "summary", "completed", None).await;
318+
let err = result.unwrap_err().to_string();
319+
assert!(err.contains("at least one --skill-id"), "got: {err}");
320+
cleanup_env();
321+
}
322+
323+
#[tokio::test]
324+
async fn sessions_create_in_progress_allows_empty_skill_ids() {
325+
let _lock = lock_env().await;
326+
let mut server = mockito::Server::new_async().await;
327+
let cfg = test_config(&server.url());
328+
let mock = server
329+
.mock("POST", "/api/v2/onboarding/sessions")
330+
.match_body(mockito::Matcher::PartialJson(serde_json::json!({
331+
"data": {
332+
"type": "onboarding_session",
333+
"id": "run-123",
334+
"attributes": { "status": "in_progress", "skill_ids": [] },
335+
},
336+
})))
337+
.with_status(201)
338+
.with_header("content-type", "application/vnd.api+json")
339+
.with_body(r#"{"data":{"id":"run-123"}}"#)
340+
.create_async()
341+
.await;
342+
let result =
343+
super::sessions_create(&cfg, "run-123", &[], "session started", "in_progress", None)
344+
.await;
345+
assert!(
346+
result.is_ok(),
347+
"in_progress create failed: {:?}",
348+
result.err()
349+
);
350+
mock.assert_async().await;
351+
cleanup_env();
352+
}
304353
}

src/main.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6537,7 +6537,7 @@ enum OnboardingSessionsActions {
65376537
/// Session (onboarding run) ID
65386538
#[arg(long)]
65396539
session_id: String,
6540-
/// Skill ID touched in this session (repeat for multiple)
6540+
/// Skill ID touched in this session (repeat for multiple; required unless status is in_progress)
65416541
#[arg(long = "skill-id")]
65426542
skill_ids: Vec<String>,
65436543
/// Short summary of the session outcome

0 commit comments

Comments
 (0)