Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions docs/openapi/chimely.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,11 @@ paths:
tags:
- admin
summary: Replay dead letter
description: Requeue one failed job for another attempt.
description: |-
Requeue one failed job for another attempt.

Without an environment filter the replay reaches across environments and
`replayed` reports every row moved.
operationId: adminReplayDeadLetter
parameters:
- name: job_id
Expand All @@ -139,6 +143,12 @@ paths:
required: true
schema:
type: string
- name: environment
in: query
description: Environment slug. Pins the replay to that environment.
required: false
schema:
type: string
responses:
'200':
description: Replayed.
Expand Down Expand Up @@ -167,7 +177,7 @@ paths:
code: forbidden
message: your role does not permit this action
'404':
description: No such parked job.
description: No such parked job, or no such environment.
content:
application/json:
schema:
Expand Down
10 changes: 8 additions & 2 deletions packages/client/src/generated/api.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ export interface paths {
/**
* Replay dead letter
* @description Requeue one failed job for another attempt.
*
* Without an environment filter the replay reaches across environments and
* `replayed` reports every row moved.
*/
post: operations["adminReplayDeadLetter"];
delete?: never;
Expand Down Expand Up @@ -1292,7 +1295,10 @@ export interface operations {
};
adminReplayDeadLetter: {
parameters: {
query?: never;
query?: {
/** @description Environment slug. Pins the replay to that environment. */
environment?: string;
};
header?: never;
path: {
/** @description Job TypeID (job_…). */
Expand Down Expand Up @@ -1345,7 +1351,7 @@ export interface operations {
"application/json": components["schemas"]["Error"];
};
};
/** @description No such parked job. */
/** @description No such parked job, or no such environment. */
404: {
headers: {
[name: string]: unknown;
Expand Down
41 changes: 33 additions & 8 deletions server/src/api/admin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1215,6 +1215,13 @@ pub struct AdminReplayResult {
pub replayed: i64,
}

#[derive(Deserialize, IntoParams)]
#[into_params(parameter_in = Query)]
pub struct AdminReplayFilter {
/// Environment slug. Pins the replay to that environment.
pub environment: Option<String>,
}

#[utoipa::path(
get,
path = "/admin/api/dlq",
Expand Down Expand Up @@ -1265,8 +1272,14 @@ pub async fn list_dlq(
tag = "admin",
operation_id = "adminReplayDeadLetter",
summary = "Replay dead letter",
description = "Requeue one failed job for another attempt.",
params(("job_id" = String, Path, description = "Job TypeID (job_…).")),
description = r#"Requeue one failed job for another attempt.

Without an environment filter the replay reaches across environments and
`replayed` reports every row moved."#,
params(
("job_id" = String, Path, description = "Job TypeID (job_…)."),
AdminReplayFilter
),
responses(
(status = 200, description = "Replayed.", body = AdminReplayResult),
(
Expand All @@ -1283,7 +1296,7 @@ pub async fn list_dlq(
),
(
status = 404,
description = "No such parked job.",
description = "No such parked job, or no such environment.",
body = crate::api::contract::Error,
example = json!({"error": {"code": "not_found", "message": "no such parked job"}}),
),
Comment on lines 1296 to 1302

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 404 example only covers one of two distinct cases

The utoipa example on the 404 response hard-codes "no such parked job", but this status is also returned when the environment slug is unknown (with "no such environment"). An API consumer looking at the generated docs has no way to distinguish the two. Adding a second named example (utoipa supports examples with keys) would let callers tell apart a bad slug from a missing job.

Fix in Claude Code

Expand All @@ -1294,18 +1307,30 @@ pub async fn replay_dead_letter(
auth: AdminAuth,
State(state): State<AppState>,
Path(job_id): Path<String>,
ApiQuery(filter): ApiQuery<AdminReplayFilter>,
) -> Result<Json<AdminReplayResult>, ApiError> {
auth.require(Capability::DlqReplay)?;
let id = ids::parse_typeid(ids::JOB, &job_id)
.ok_or_else(|| ApiError::not_found("no such parked job"))?;
// Cross-environment admin path: job ids are globally unique UUIDv7s.
let replayed = dlq::replay(&state.pool, id, None)
let environment = match &filter.environment {
Some(slug) => Some(
dlq::environment_by_slug(&state.pool, slug)
.await
.map_err(ApiError::from)?
.ok_or_else(|| ApiError::not_found("no such environment"))?,
),
None => None,
};
// The same job id can be parked in more than one environment under the
// (environment_id, id) key. An unfiltered replay moves every copy, so the
// response carries the real count.
let replayed = dlq::replay(&state.pool, id, environment)
.await
.map_err(ApiError::from)?;
if !replayed {
.map_err(ApiError::from)? as i64;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Lossy u64 → i64 cast

rows_affected() returns u64, and the bare as i64 cast silently wraps to a negative value if the count ever exceeds i64::MAX. In DLQ replay that number is bounded by the number of environments a single job can be parked in, so it is practically zero risk — but a prefer-safe path would be u64::try_into().unwrap_or(i64::MAX) or simply changing AdminReplayResult.replayed to u64 (the OpenAPI schema renders it as an integer either way).

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Claude Code

if replayed == 0 {
return Err(ApiError::not_found("no such parked job"));
}
Ok(Json(AdminReplayResult { replayed: 1 }))
Ok(Json(AdminReplayResult { replayed }))
}

#[utoipa::path(
Expand Down
13 changes: 7 additions & 6 deletions server/src/dlq.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,11 @@ pub async fn list(pool: &PgPool) -> anyhow::Result<Vec<DeadLetter>> {
}

/// Replay one parked job by id, optionally pinned to one environment.
/// environment_id is part of every key. An unscoped id match would reach
/// across environments. Returns false if no such dead letter.
pub async fn replay(pool: &PgPool, id: Uuid, environment: Option<Uuid>) -> anyhow::Result<bool> {
let moved = replay_where(pool, Some(id), environment).await?;
Ok(moved > 0)
/// environment_id is part of every key. An unscoped id match reaches across
/// environments, so the moved-row count is returned. Zero means no such
/// dead letter.
pub async fn replay(pool: &PgPool, id: Uuid, environment: Option<Uuid>) -> anyhow::Result<u64> {
replay_where(pool, Some(id), environment).await
}

/// Replay every parked job, optionally only one environment's.
Expand Down Expand Up @@ -83,7 +83,8 @@ async fn replay_where(
Ok(moved)
}

/// Resolve an environment slug for the CLI's `--env` flag.
/// Resolve an environment slug for the CLI's `--env` flag and the admin
/// replay filter.
pub async fn environment_by_slug(pool: &PgPool, slug: &str) -> anyhow::Result<Option<Uuid>> {
Ok(
sqlx::query_scalar!("SELECT id FROM environments WHERE slug = $1", slug)
Expand Down
2 changes: 1 addition & 1 deletion server/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ async fn dlq_command(args: Vec<String>) -> anyhow::Result<()> {
let id = ids::parse_typeid(ids::JOB, id)
.or_else(|| id.parse().ok())
.context("expected a job_… TypeID or a raw UUID")?;
if dlq::replay(&pool, id, environment).await? {
if dlq::replay(&pool, id, environment).await? > 0 {
println!("replayed {}", ids::typeid(ids::JOB, id));
} else {
eprintln!("no such dead letter");
Expand Down
87 changes: 86 additions & 1 deletion server/tests/redteam_dlq_env_scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

mod support;

use serde_json::json;
use uuid::Uuid;

async fn park_dead_letter_with_id(pool: &sqlx::PgPool, env: Uuid, id: Uuid) {
Expand Down Expand Up @@ -73,7 +74,7 @@ async fn replay_by_id_replays_only_the_scoped_environment() {
let replayed = chimely::dlq::replay(&app.pool, shared, Some(app.env.id))
.await
.expect("replay env A's copy");
assert!(replayed, "env A's copy was replayed");
assert_eq!(replayed, 1, "only env A's copy was replayed");

assert_eq!(
dead_letters_in(&app.pool, app.env.id).await,
Expand All @@ -86,3 +87,87 @@ async fn replay_by_id_replays_only_the_scoped_environment() {
"env B's same-id dead letter must survive an env-A-scoped replay"
);
}

#[tokio::test]
async fn admin_replay_honors_the_environment_query_param() {
let app = support::spawn().await;
let env_b = app.create_environment(true).await;

let shared = chimely::ids::new_uuid();
park_dead_letter_with_id(&app.pool, app.env.id, shared).await;
park_dead_letter_with_id(&app.pool, env_b.id, shared).await;

let job = chimely::ids::typeid(chimely::ids::JOB, shared);
let res = app
.admin_post(
&format!("/admin/api/dlq/{job}/replay?environment={}", app.env.slug),
json!({}),
)
.send()
.await
.expect("replay request");
assert_eq!(res.status(), 200);
let body: serde_json::Value = res.json().await.expect("replay body");
assert_eq!(body["replayed"], json!(1));

assert_eq!(
dead_letters_in(&app.pool, app.env.id).await,
0,
"env A's copy moved back to jobs"
);
assert_eq!(
dead_letters_in(&app.pool, env_b.id).await,
1,
"env B's same-id dead letter must survive an env-scoped HTTP replay"
);
}

#[tokio::test]
async fn admin_replay_reports_the_actual_moved_count_when_unscoped() {
let app = support::spawn().await;
let env_b = app.create_environment(true).await;

// Without an environment filter the admin path keeps its documented
// cross-environment reach. The response must report every row it moved.
let shared = chimely::ids::new_uuid();
park_dead_letter_with_id(&app.pool, app.env.id, shared).await;
park_dead_letter_with_id(&app.pool, env_b.id, shared).await;

let job = chimely::ids::typeid(chimely::ids::JOB, shared);
let res = app
.admin_post(&format!("/admin/api/dlq/{job}/replay"), json!({}))
.send()
.await
.expect("replay request");
assert_eq!(res.status(), 200);
let body: serde_json::Value = res.json().await.expect("replay body");
assert_eq!(body["replayed"], json!(2), "both moved rows are counted");

assert_eq!(dead_letters_in(&app.pool, app.env.id).await, 0);
assert_eq!(dead_letters_in(&app.pool, env_b.id).await, 0);
}

#[tokio::test]
async fn admin_replay_404s_for_an_unknown_environment_slug() {
let app = support::spawn().await;

let id = chimely::ids::new_uuid();
park_dead_letter_with_id(&app.pool, app.env.id, id).await;

let job = chimely::ids::typeid(chimely::ids::JOB, id);
let res = app
.admin_post(
&format!("/admin/api/dlq/{job}/replay?environment=no-such-env"),
json!({}),
)
.send()
.await
.expect("replay request");
assert_eq!(res.status(), 404);

assert_eq!(
dead_letters_in(&app.pool, app.env.id).await,
1,
"a rejected replay must not move the parked job"
);
}