\nsecurity\np0\ndocs/a.md`;
+ const findings = parseFindings(text);
+ assert.equal(findings.length, 1);
+ const cut = key => ({ ...finishedReviewer(key), findings, rawText: text, parsed: reviewerReported(text, findings) });
+ assert.equal(cut("x").parsed, false);
+ for (const results of [[cut("security"), finishedReviewer("correctness")], [finishedReviewer("security"), finishedReviewer("correctness"), cut("qa")]]) {
+ const level = mergeConfidence({ failedReviewers: unfinishedReviewers(results), ranCount: results.length }).level;
+ assert.notEqual(level, "high");
+ const run = await runMain({ results });
+ assert.equal(run.approvals.length, 0);
+ assert.equal(run.posted.includes("🟢 High"), false);
+ assert.ok(run.posted.includes("did not finish"));
+ assert.equal(commentsModule.reviewPassCount(run.posted), 1);
+ }
+ const all = await runMain({ results: [cut("security"), cut("correctness")] });
+ assert.match(all.error, /no review was produced/);
+ assert.equal(all.approvals.length, 0);
+ assert.equal(commentsModule.reviewedKey(all.posted), "");
+});
+
+test("a coordinator cut off anywhere before its block closes leaves the count unknown", async () => {
+ const closed = '';
+ const head = `${commentsModule.REVIEW_HEADING}\n\n> [!TIP]\n> ### ✅ Approved\n> Looks right.\n\n`;
+ for (let cut = 0; cut < closed.length; cut += 1) {
+ const coordinatorOutput = head + closed.slice(0, cut);
+ assert.ok(extractInlineBlock(coordinatorOutput).error, coordinatorOutput);
+ const inline = extractInlineBlock(coordinatorOutput);
+ assert.equal(mergeConfidence({ counts: severityCounts(inline.findings), findingsKnown: !inline.error, ranCount: 2 }).level, "low");
+ }
+ const run = await runMain({ coordinatorOutput: head + closed.slice(0, -5) });
+ assert.equal(run.approvals.length, 0);
+ assert.equal(run.posted.includes("🟢 High"), false);
+});
+
+const reviewedThenCancelled = (head = MAIN_HEAD, next = PAIRED_MAIN_HEAD) => {
+ const body = commentsModule.finishReviewBody(`\n${commentsModule.REVIEW_HEADING}\n\n> [!TIP]\n> ### ✅ Approved`, {
+ key: head, passes: 1, maxPasses: 0, reviewed: true, confidence: { level: "high", reasons: ["no blockers"] },
+ gateLine: "**Auto-approved** — small change, all gates passed.",
+ });
+ return commentsModule.claimedReviewBody(body, 2, next);
+};
+
+test("a skip on a reviewed head clears a cancelled run's progress without a review or a pass", async () => {
+ const body = reviewedThenCancelled();
+ assert.ok(body.includes("is being reviewed"));
+ const run = await runMain({ existing: { id: 1, body } });
+ assert.ok(run.logs.some(line => line.includes("already_reviewed")));
+ assert.equal(run.error, undefined);
+ assert.equal(run.updates, 1);
+ assert.equal(run.creates, 0);
+ assert.equal(run.claims, 0);
+ assert.equal(run.coordinatorCalls, 0);
+ assert.equal(run.approvals.length, 0);
+ assert.equal(run.posted.includes("is being reviewed"), false);
+ assert.ok(run.posted.includes("🟢 High"));
+ assert.equal(run.posted.includes("Auto-approved"), false);
+ assert.equal(commentsModule.reviewPassCount(run.posted), 2);
+ assert.equal(commentsModule.reviewedKey(run.posted), MAIN_HEAD);
+ const again = await runMain({ existing: { id: 1, body: run.posted } });
+ assert.equal(again.updates, 0);
+ assert.equal(again.creates, 0);
+});
+
+test("a skip with nothing to restore stays silent", async () => {
+ const body = commentsModule.finishReviewBody(`${commentsModule.REVIEW_HEADING}\n\n> [!TIP]\n> ### ✅ Approved`, { key: MAIN_HEAD, passes: 3, maxPasses: 0, reviewed: true, confidence: { level: "high", reasons: ["no blockers"] } });
+ const run = await runMain({ existing: { id: 1, body } });
+ assert.equal(run.updates, 0);
+ assert.equal(run.creates, 0);
+ assert.equal(run.claims, 0);
+});
+
+test("a fenced or hidden callout never decides an approval", () => {
+ const approved = "> [!TIP]\n> ### ✅ Approved";
+ const caution = "> [!CAUTION]\n> ### ⛔ Not reviewed";
+ assert.equal(isApprovedReview(`## R\n\n${approved}\n\n- note`), true);
+ assert.equal(isApprovedReview(`## R\n\n\`\`\`\n${approved}\n\`\`\`\n\n${caution}`), false);
+ assert.equal(isApprovedReview(`## R\n\n~~~\n${approved}\n~~~`), false);
+ assert.equal(isApprovedReview(`\n\n${caution}`), false);
+ assert.equal(isApprovedReview(``), false);
+ assert.equal(isApprovedReview(`${approved}\n\n ${caution}`), false);
+ assert.equal(isApprovedReview(`\n${caution}\n-->\n${approved}`), false);
+ assert.equal(isApprovedReview("> [!TIP]\n> ### ⛔ Not reviewed"), false);
+ assert.equal(isApprovedReview(`> \`\`\`\n${approved}\n> \`\`\``), false);
+ assert.equal(isApprovedReview(`> \n${approved}`), false);
+ assert.equal(isApprovedReview(`\n${approved}\n`), false);
+ assert.equal(isApprovedReview(`${approved}\n\n${approved}`), false);
+ assert.equal(isApprovedReview(`${approved}\n\n> [!tip]\n> x`), false);
+});
+
+test("the fallback cap never drops a blocker", () => {
+ const blockers = Array.from({ length: 18 }, (_, i) => xmlFinding("p1", `blocker ${i}`));
+ const kept = selectFallbackFindings([reviewer([...blockers, xmlFinding("p2", "minor")])]);
+ assert.equal(kept.filter(f => f.severity === "p1").length, 18);
+ assert.equal(kept.some(f => f.severity === "p2"), false);
+ const few = selectFallbackFindings([reviewer([xmlFinding("p1", "one"), xmlFinding("p2", "two")])]);
+ assert.deepEqual(few.map(f => f.severity), ["p1", "p2"]);
+});
+
+test("a forced re-review reaches the paired repo", () => {
+ const workflow = readFileSync(new URL("../../../.github/workflows/ai-code-review.yml", import.meta.url), "utf8");
+ assert.match(workflow, /FORCE_REVIEW: \$\{\{ inputs\.force_full \|\| 'false' \}\}/);
+ assert.match(workflow, /"\$FORCE_REVIEW" = "true" \]; then bypass\+=\(-f force_full=true\)/);
+});
+
+test("the pairing lookup only accepts a PR from the paired repo itself", () => {
+ const workflow = readFileSync(new URL("../../../.github/workflows/ai-code-review.yml", import.meta.url), "utf8");
+ assert.ok(workflow.includes('select(.head.repo.full_name == \\"$ent_repo\\")'));
+});
+
+test("renaming production code away counts as a production change", () => {
+ const rename = (from, to) => `diff --git a/${from} b/${to}\nsimilarity index 100%\nrename from ${from}\nrename to ${to}\n`;
+ assert.equal(changedCodeEvidence(rename("src/service/old.rs", "archive/old.rs")).prodChanged, true);
+ assert.equal(changedCodeEvidence(rename("archive/old.rs", "src/service/old.rs")).prodChanged, true);
+ assert.equal(changedCodeEvidence(rename("docs/a.md", "docs/b.md")).prodChanged, false);
+ assert.equal(changedCodeEvidence(rename("o2/src/service/old.rs", "o2/archive/old.rs"), "o2").prodChanged, true);
+});
+
+test("a blocker survives when every reviewer's report is cut off", async () => {
+ const blocker = "securityp1a.rs1a real blocker";
+ const cutOff = { ...silentReviewer("security"), findings: parseFindings(`\n${blocker}`) };
+ const run = await runMain({ results: [cutOff, silentReviewer("correctness")] });
+ assert.ok(run.posted.includes("a real blocker"));
+ assert.ok(run.posted.includes("the findings below are incomplete"));
+ assert.ok(run.posted.includes("Not reviewed"));
+ assert.equal(run.approvals.length, 0);
+ assert.equal(run.coordinatorCalls, 0);
+});
+
+test("a total failure with nothing parsed still says there is nothing to report", async () => {
+ const run = await runMain({ results: [silentReviewer("security"), silentReviewer("correctness")] });
+ assert.ok(run.posted.includes("there are no findings to report"));
+ assert.equal(run.posted.includes("Findings so far"), false);
+});
+
+test("a coordinator report without its findings list falls back to the reviewers' findings", () => {
+ const source = readFileSync(new URL("../run-review.mjs", import.meta.url), "utf8");
+ const start = source.indexOf("const inline = extractInlineBlock(completion.text);");
+ const guard = source.indexOf("if (inline.error) throw", start);
+ const result = source.indexOf("return {", start);
+ assert.ok(start !== -1 && guard !== -1 && guard < result);
+ const text = `${commentsModule.REVIEW_HEADING}\n\n> [!CAUTION]\n> ### ⛔ Blockers`;
+ assert.ok(extractInlineBlock(text).error);
+ const fallback = buildFallbackReview([reviewer([xmlFinding("p1", "kept blocker")])], "full", [], "");
+ assert.ok(fallback.includes("kept blocker"));
+});
+
+test("an outer findings wrapper left open is a cut-off report", () => {
+ assert.equal(reviewerReported("No problems.", []), false);
+ assert.equal(reviewerReported("No problems.", []), true);
+});
+
+test("a no-issues report quoted as an example inside failure prose is not a finished review", () => {
+ const fenced = "```xml\n\n```";
+ const table = [
+ [`I could not inspect the repository. A clean report would look like:\n\n${fenced}`, false],
+ [`Here is an example of the expected format:\n${fenced}\nI was unable to read the diff.`, false],
+ [`Example:\n${fenced}`, false],
+ ["I was unable to access the files.\n", false],
+ ["Failed to load the diff, so: ", false],
+ [`\n\n${fenced}`, false],
+ [fenced, true],
+ ["```\np3x\n```", true],
+ ["I could not find any problem.", true],
+ ["p2sThe handler cannot see `````` here.", true],
+ ];
+ for (const [text, parsed] of table) assert.equal(reviewerReported(text, parseFindings(text)), parsed, text);
+});
+
+test("a report that is only quoted, commented out or unwrapped is not a finished review", () => {
+ const finding = "p3x";
+ const table = [
+ ["Expected response format: ``.", false],
+ ["", false],
+ ["", false],
+ ["Expected: ``.", false],
+ ["Expected: ``LGTM``", false],
+ [``, false],
+ ["", false],
+ ["``", false],
+ ["LGTM", false],
+ ["\n LGTM\n \n", true],
+ ["```xml\n\n LGTM\n\n```", true],
+ [`${finding}`, true],
+ [`p3uses \`x\`see \`\``, true],
+ ];
+ for (const [text, parsed] of table) assert.equal(reviewerReported(text, parseFindings(text)), parsed, text);
+});
+
+test("a no-issues body saying the review could not be done is not a finished review", () => {
+ const wrap = body => `${body}`;
+ const unfinished = [
+ "I was unable to read the repository files, so I could not perform this review.",
+ "Unable to access the diff.",
+ "I could not open the changed files.",
+ "I couldn't inspect the code.",
+ "I was not able to complete the review.",
+ "Could not review this PR: the tool failed.",
+ "I cannot fully review these changes.",
+ "I have no access to the repository.",
+ "I don't have access to the files.",
+ "The read tool failed on every file.",
+ "Failed to load the diff.",
+ "I could not run the tests or read the sources.",
+ "I could not run the tests or access the repository.",
+ "I was unable to get the diff.",
+ "Failed to fetch the PR files.",
+ "I could not finish the review.",
+ "I couldn't perform a full review of the changes.",
+ ];
+ const finished = [
+ "I could not find any problem.",
+ "No issues found.",
+ "Unable to identify any defect in the changed lines.",
+ "I couldn't spot any correctness issue after reading every changed file.",
+ "Reviewed all files; nothing to report.",
+ "I could not find any review-worthy issue.",
+ "",
+ "Reviewed every changed file and caller; no defects found. I could not run tests in this read-only session.",
+ "Unable to run the build locally; the code reads correctly.",
+ "I couldn't execute the linters, but every changed line was reviewed.",
+ "Could not run cargo test; reviewed the code statically.",
+ "I was not able to run the app or its commands.",
+ "I failed to complete the build in this sandbox.",
+ "No problems found. I cannot open the app here.",
+ ];
+ for (const body of unfinished) assert.equal(reviewerReported(wrap(body), []), false, body);
+ for (const body of finished) assert.equal(reviewerReported(wrap(body), []), true, body);
+});
+
+test("a typographic apostrophe or odd spacing cannot hide that the review did not happen", () => {
+ const wrap = body => `${body}`;
+ const unfinished = [
+ "I couldn’t inspect the repository because file reads failed.",
+ "I couldn‘t read the diff.",
+ "I canʼt access the files.",
+ "I wasn′t able to review the changes.",
+ "I don＇t have access to the repository.",
+ "I could\nnot\tinspect the code.",
+ ];
+ for (const body of unfinished) assert.equal(reviewerReported(wrap(body), []), false, body);
+ assert.equal(reviewerReported("I couldn’t open the tool.\n", []), false);
+ assert.equal(reviewerReported(wrap("I couldn’t find any problem."), []), true);
+});
+
+test("a tilde-fenced or blockquoted report example is not a finished review", () => {
+ const table = [
+ ["Expected response format:\n~~~xml\n\n~~~\nI could not proceed.", false],
+ ["Example:\n~~~xml\n\n~~~", false],
+ ["Expected response format:\n> ", false],
+ ["Expected response format:\n> \n> LGTM\n> ", false],
+ ["~~~xml\nLGTM\n~~~", true],
+ ["Example:\n> quoted\n\nLGTM", true],
+ ];
+ for (const [text, parsed] of table) assert.equal(reviewerReported(text, parseFindings(text)), parsed, text);
+});
From 476bbfb39c6f7e02449eca30272458c7f6cb8e61 Mon Sep 17 00:00:00 2001
From: Shrinath Rao
Date: Tue, 3 Nov 2026 06:35:00 +0000
Subject: [PATCH 3/7] fix: reject a negative toolset list limit and publish
satisfiable scorer schemas (#16301)
---
src/api/management/src/models/scorers/mod.rs | 130 +++++++++++++++++-
src/api/management/src/request/ai/toolsets.rs | 7 +-
src/infra/src/table/org_ai_toolsets.rs | 34 ++++-
3 files changed, 159 insertions(+), 12 deletions(-)
diff --git a/src/api/management/src/models/scorers/mod.rs b/src/api/management/src/models/scorers/mod.rs
index 7d0f005e80..b40a8a1ac4 100644
--- a/src/api/management/src/models/scorers/mod.rs
+++ b/src/api/management/src/models/scorers/mod.rs
@@ -18,7 +18,13 @@ use infra::table::scorers::ScorerType;
use openobserve_core::llm_evaluations::scorers::schema_derivation::ExtraMetadataField;
use serde::{Deserialize, Serialize};
use serde_json::Value;
-use utoipa::ToSchema;
+use utoipa::{
+ PartialSchema, ToSchema,
+ openapi::{
+ RefOr,
+ schema::{ObjectBuilder, OneOfBuilder, Schema, Type},
+ },
+};
#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, ToSchema)]
#[serde(rename_all = "snake_case", deny_unknown_fields)]
@@ -73,13 +79,30 @@ pub struct RemoteScorerHeader {
pub value: String,
}
-#[derive(Clone, Debug, Deserialize, PartialEq, ToSchema)]
+#[derive(Clone, Debug, Deserialize, PartialEq)]
#[serde(tag = "type", rename_all = "snake_case", deny_unknown_fields)]
pub enum ScorerCreateConfig {
LlmJudge(LlmJudgeScorerRequest),
Remote(RemoteScorerRequest),
}
+// Not derived: utoipa's allOf keeps additionalProperties:false and so forbids the tag.
+impl PartialSchema for ScorerCreateConfig {
+ fn schema() -> RefOr {
+ OneOfBuilder::new()
+ .item(tagged_variant("llm_judge", LlmJudgeScorerRequest::schema()))
+ .item(tagged_variant("remote", RemoteScorerRequest::schema()))
+ .into()
+ }
+}
+
+impl ToSchema for ScorerCreateConfig {
+ fn schemas(schemas: &mut Vec<(String, RefOr)>) {
+ LlmJudgeScorerRequest::schemas(schemas);
+ RemoteScorerRequest::schemas(schemas);
+ }
+}
+
#[derive(Clone, Debug, Deserialize, PartialEq, ToSchema)]
#[serde(rename_all = "camelCase", deny_unknown_fields)]
pub struct LlmJudgeScorerRequest {
@@ -104,13 +127,36 @@ pub struct RemoteScorerRequest {
pub params: RemoteScorerParams,
}
-#[derive(Clone, Debug, Deserialize, PartialEq, ToSchema)]
+#[derive(Clone, Debug, Deserialize, PartialEq)]
#[serde(tag = "type", rename_all = "snake_case", deny_unknown_fields)]
pub enum ScorerUpdateConfig {
LlmJudge(LlmJudgeScorerUpdateRequest),
Remote(RemoteScorerUpdateRequest),
}
+// Not derived for the same reason as `ScorerCreateConfig`.
+impl PartialSchema for ScorerUpdateConfig {
+ fn schema() -> RefOr {
+ OneOfBuilder::new()
+ .item(tagged_variant(
+ "llm_judge",
+ LlmJudgeScorerUpdateRequest::schema(),
+ ))
+ .item(tagged_variant(
+ "remote",
+ RemoteScorerUpdateRequest::schema(),
+ ))
+ .into()
+ }
+}
+
+impl ToSchema for ScorerUpdateConfig {
+ fn schemas(schemas: &mut Vec<(String, RefOr)>) {
+ LlmJudgeScorerUpdateRequest::schemas(schemas);
+ RemoteScorerUpdateRequest::schemas(schemas);
+ }
+}
+
#[derive(Clone, Debug, Deserialize, PartialEq, ToSchema)]
#[serde(rename_all = "camelCase", deny_unknown_fields)]
pub struct LlmJudgeScorerUpdateRequest {
@@ -139,6 +185,22 @@ fn params_to_value(params: T) -> Value {
serde_json::to_value(params).unwrap_or_else(|_| serde_json::json!({}))
}
+/// Adds the serde `type` tag to a variant's own object schema.
+fn tagged_variant(tag: &str, variant: RefOr) -> RefOr {
+ let RefOr::T(Schema::Object(mut object)) = variant else {
+ return variant;
+ };
+ object.properties.insert(
+ "type".to_string(),
+ ObjectBuilder::new()
+ .schema_type(Type::String)
+ .enum_values(Some([tag]))
+ .into(),
+ );
+ object.required.push("type".to_string());
+ RefOr::T(Schema::Object(object))
+}
+
/// HTTP request body for creating a Scorer.
#[derive(Clone, Debug, Deserialize, ToSchema)]
#[serde(rename_all = "camelCase", deny_unknown_fields)]
@@ -452,6 +514,30 @@ impl From> for ListScorerVersionsResponseBody
mod tests {
use super::*;
+ /// Indexes of the `oneOf` variants a body satisfies on required keys, known keys and tag.
+ fn matching_variants(schema: RefOr, body: &Value) -> Vec {
+ let schema = serde_json::to_value(schema).unwrap();
+ let body = body.as_object().unwrap();
+ let variants = schema["oneOf"].as_array().unwrap();
+ (0..variants.len())
+ .filter(|&i| {
+ let variant = &variants[i];
+ let properties = variant["properties"].as_object().unwrap();
+ let closed = variant["additionalProperties"] == Value::Bool(false);
+ let required = variant["required"].as_array().unwrap();
+ let tag_ok = variant["properties"]["type"]["enum"]
+ .as_array()
+ .unwrap()
+ .contains(&body["type"]);
+ tag_ok
+ && required
+ .iter()
+ .all(|k| body.contains_key(k.as_str().unwrap()))
+ && (!closed || body.keys().all(|k| properties.contains_key(k)))
+ })
+ .collect()
+ }
+
fn sample_scorer() -> infra::table::scorers::Scorer {
infra::table::scorers::Scorer {
id: "abc".to_string(),
@@ -472,6 +558,44 @@ mod tests {
}
}
+ #[test]
+ fn scorer_config_schemas_admit_exactly_the_bodies_serde_accepts() {
+ let judge = serde_json::json!({
+ "type": "llm_judge",
+ "producesScoreConfigId": "scfg-entity-1",
+ "template": "Judge {{input}}",
+ "outputSchema": {"type": "object"},
+ "params": {"provider_id": "p1"}
+ });
+ let remote = serde_json::json!({
+ "type": "remote",
+ "template": "{{output}}",
+ "params": {"endpoint": "https://eval.example.com", "timeout_ms": null}
+ });
+ for (body, variant) in [(&judge, 0), (&remote, 1)] {
+ serde_json::from_value::(body.clone()).unwrap();
+ serde_json::from_value::(body.clone()).unwrap();
+ assert_eq!(
+ matching_variants(ScorerCreateConfig::schema(), body),
+ [variant]
+ );
+ assert_eq!(
+ matching_variants(ScorerUpdateConfig::schema(), body),
+ [variant]
+ );
+ }
+
+ let mut unknown = judge.clone();
+ unknown["bogus"] = serde_json::json!(1);
+ assert!(serde_json::from_value::(unknown.clone()).is_err());
+ assert!(matching_variants(ScorerCreateConfig::schema(), &unknown).is_empty());
+
+ let mut mistagged = judge.clone();
+ mistagged["type"] = serde_json::json!("remote");
+ assert!(serde_json::from_value::(mistagged.clone()).is_err());
+ assert!(matching_variants(ScorerCreateConfig::schema(), &mistagged).is_empty());
+ }
+
#[test]
fn test_scorer_response_body_from_entity() {
let s = sample_scorer();
diff --git a/src/api/management/src/request/ai/toolsets.rs b/src/api/management/src/request/ai/toolsets.rs
index cc45a86ea6..4c62df1254 100644
--- a/src/api/management/src/request/ai/toolsets.rs
+++ b/src/api/management/src/request/ai/toolsets.rs
@@ -161,8 +161,8 @@ pub struct ListQuery {
pub name: Option,
/// Exact filter on the toolset kind (`mcp`, `cli`, `skill`, `generic`).
pub kind: Option,
- /// Maximum number of results to return.
- pub limit: Option,
+ /// Maximum number of results to return; a negative value is rejected with 400.
+ pub limit: Option,
}
#[utoipa::path(
@@ -177,11 +177,12 @@ pub struct ListQuery {
("org_id" = String, Path, description = "Organization name"),
("name" = Option, Query, description = "Substring filter on name (case-insensitive)"),
("kind" = Option, Query, description = "Exact filter on kind: mcp | cli | skill | generic"),
- ("limit" = Option, Query, description = "Maximum number of results"),
+ ("limit" = Option, Query, description = "Maximum number of results"),
),
security(("Authorization" = [])),
responses(
(status = 200, description = "List of toolsets", content_type = "application/json", body = Object),
+ (status = 400, description = "Invalid query parameters, such as a negative limit"),
(status = 403, description = "Feature not supported", content_type = "application/json"),
),
extensions(("x-o2-ratelimit" = json!({"module": "AI", "operation": "list"})), ("x-o2-mcp" = json!({"enabled": false})))
diff --git a/src/infra/src/table/org_ai_toolsets.rs b/src/infra/src/table/org_ai_toolsets.rs
index e3b18ccfd0..c4ce8f5322 100644
--- a/src/infra/src/table/org_ai_toolsets.rs
+++ b/src/infra/src/table/org_ai_toolsets.rs
@@ -20,8 +20,8 @@
//! org's per-org DEK via [`cipher::get_dek`].
use sea_orm::{
- ActiveModelTrait, ColumnTrait, EntityTrait, Order, QueryFilter, QueryOrder, QuerySelect, Set,
- SqlErr,
+ ActiveModelTrait, ColumnTrait, ConnectionTrait, EntityTrait, Order, QueryFilter, QueryOrder,
+ QuerySelect, Set, SqlErr,
};
use svix_ksuid::{Ksuid, KsuidLike};
@@ -193,9 +193,17 @@ pub async fn get_by_name(org: &str, name: &str) -> Result, er
pub async fn list(
org: &str,
filter: ListFilter,
- limit: Option,
+ limit: Option,
+) -> Result, errors::Error> {
+ list_with(get_orm_client_ro().await, org, filter, limit).await
+}
+
+pub async fn list_with(
+ conn: &C,
+ org: &str,
+ filter: ListFilter,
+ limit: Option,
) -> Result, errors::Error> {
- let client = get_orm_client_ro().await;
let mut query = Entity::find()
.filter(Column::Org.eq(org))
.order_by(Column::CreatedAt, Order::Desc);
@@ -207,10 +215,11 @@ pub async fn list(
query = query.filter(Column::Kind.eq(kind));
}
if let Some(n) = limit {
- query = query.limit(n as u64);
+ // sqlx binds LIMIT as i64 and panics on anything larger.
+ query = query.limit(n.min(i64::MAX as u64));
}
- let rows = query.all(client).await?;
+ let rows = query.all(conn).await?;
if rows.is_empty() {
return Ok(vec![]);
@@ -280,6 +289,19 @@ mod tests {
// ListFilter
// -----------------------------------------------------------------------
+ #[tokio::test]
+ async fn list_with_an_out_of_range_limit_does_not_panic() {
+ use sea_orm::{Database, Schema};
+
+ let db = Database::connect("sqlite::memory:").await.unwrap();
+ let backend = db.get_database_backend();
+ let stmt = Schema::new(backend).create_table_from_entity(Entity);
+ db.execute(backend.build(&stmt)).await.unwrap();
+
+ let rows = list_with(&db, "org", ListFilter::default(), Some(u64::MAX)).await;
+ assert!(rows.unwrap().is_empty());
+ }
+
#[test]
fn test_list_filter_default() {
let f = ListFilter::default();
From b36c61ebd1f09cfe381785b9b03d5baa4e0133d8 Mon Sep 17 00:00:00 2001
From: Huaijin
Date: Tue, 3 Nov 2026 06:43:36 +0000
Subject: [PATCH 4/7] feat(profiles): support user-defined schema for profiles
streams (#16237)
---
src/api/search/src/profiles/mod.rs | 75 ++++++++++++++++++--
src/config/src/meta/stream.rs | 4 +-
src/core/src/ingestion/mod.rs | 87 +++++++++++++++++++++--
src/core/src/logs/mod.rs | 23 +++++--
src/core/src/metrics/ingest.rs | 106 ++++++++++++++++++++---------
src/core/src/metrics/json.rs | 8 +--
src/core/src/metrics/otlp.rs | 6 +-
src/core/src/metrics/prom.rs | 6 +-
src/core/src/profiles/mod.rs | 52 ++++++++++++--
src/core/src/traces/mod.rs | 68 +++++++++++++++++-
src/schema/src/lib.rs | 41 +++++++++++
11 files changed, 406 insertions(+), 70 deletions(-)
diff --git a/src/api/search/src/profiles/mod.rs b/src/api/search/src/profiles/mod.rs
index 11dc1e3c90..fe9646444f 100644
--- a/src/api/search/src/profiles/mod.rs
+++ b/src/api/search/src/profiles/mod.rs
@@ -21,9 +21,10 @@ pub mod meta;
pub mod series;
pub mod tag_values;
+use arrow_schema::Schema;
use axum::{Json, extract::rejection::JsonRejection, http::Method, response::Response};
use config::{
- get_config,
+ TIMESTAMP_COL_NAME, get_config,
meta::{
search::{Query, Request, RequestEncoding, Response as SearchResponse, SearchEventType},
stream::StreamType,
@@ -532,11 +533,7 @@ pub(crate) fn default_result_size() -> i64 {
pub(crate) async fn load_schema_field_names(org_id: &str, stream_name: &str) -> Vec {
match infra::schema::get(org_id, stream_name, StreamType::Profiles).await {
- Ok(schema) => schema
- .fields()
- .iter()
- .map(|f| f.name().to_string())
- .collect(),
+ Ok(schema) => queryable_field_names(&schema),
Err(e) => {
warn!("profiles schema lookup failed: {e}");
Vec::new()
@@ -544,6 +541,27 @@ pub(crate) async fn load_schema_field_names(org_id: &str, stream_name: &str) ->
}
}
+/// On a UDS stream the other columns are packed into `_all`, so they hold no rows to filter on.
+fn queryable_field_names(schema: &Schema) -> Vec {
+ let defined_schema_fields = infra::schema::unwrap_stream_settings(schema)
+ .map(|settings| settings.defined_schema_fields)
+ .unwrap_or_default();
+ let column_all = &get_config().common.column_all;
+ // `_all` outlives a cleared UDS in the schema, so it is never a filter key
+ schema
+ .fields()
+ .iter()
+ .map(|f| f.name())
+ .filter(|name| {
+ *name != column_all
+ && (defined_schema_fields.is_empty()
+ || *name == TIMESTAMP_COL_NAME
+ || defined_schema_fields.contains(name))
+ })
+ .cloned()
+ .collect()
+}
+
#[cfg(test)]
mod tests {
use super::*;
@@ -718,4 +736,49 @@ mod tests {
"{text}"
);
}
+
+ #[test]
+ fn queryable_field_names_follow_the_user_defined_schema() {
+ use arrow_schema::{DataType, Field};
+ use config::meta::stream::StreamSettings;
+
+ let fields = [
+ TIMESTAMP_COL_NAME,
+ "stack",
+ "value",
+ "service_name",
+ "k8s_pod_name",
+ "k8s_node_name",
+ "_all",
+ ]
+ .map(|name| Field::new(name, DataType::Utf8, true));
+ let settings = StreamSettings {
+ defined_schema_fields: ["k8s_pod_name", "service_name", "stack", "value"]
+ .map(str::to_string)
+ .to_vec(),
+ ..Default::default()
+ };
+ let metadata = std::collections::HashMap::from([(
+ "settings".to_string(),
+ json::to_string(&settings).unwrap(),
+ )]);
+
+ let cleared_uds = queryable_field_names(&Schema::new(fields.to_vec()));
+ assert_eq!(cleared_uds.len(), fields.len() - 1);
+ assert!(!cleared_uds.iter().any(|name| name == "_all"));
+
+ let uds = Schema::new_with_metadata(fields.to_vec(), metadata);
+ let mut names = queryable_field_names(&uds);
+ names.sort();
+ assert_eq!(
+ names,
+ [
+ TIMESTAMP_COL_NAME,
+ "k8s_pod_name",
+ "service_name",
+ "stack",
+ "value"
+ ]
+ );
+ }
}
diff --git a/src/config/src/meta/stream.rs b/src/config/src/meta/stream.rs
index 3e76fdaf03..3bcc4506c9 100644
--- a/src/config/src/meta/stream.rs
+++ b/src/config/src/meta/stream.rs
@@ -160,7 +160,7 @@ impl StreamType {
pub fn support_uds(&self) -> bool {
matches!(
*self,
- StreamType::Logs | StreamType::Metrics | StreamType::Traces
+ StreamType::Logs | StreamType::Metrics | StreamType::Traces | StreamType::Profiles
)
}
@@ -1734,7 +1734,7 @@ mod tests {
assert!(StreamType::Metrics.support_uds());
assert!(StreamType::Traces.support_uds());
assert!(!StreamType::EnrichmentTables.support_uds());
- assert!(!StreamType::Profiles.support_uds());
+ assert!(StreamType::Profiles.support_uds());
assert!(!StreamType::Filelist.support_uds());
assert!(!StreamType::Metadata.support_uds());
assert!(!StreamType::Index.support_uds());
diff --git a/src/core/src/ingestion/mod.rs b/src/core/src/ingestion/mod.rs
index 18243fb1ef..40e5a59b5f 100644
--- a/src/core/src/ingestion/mod.rs
+++ b/src/core/src/ingestion/mod.rs
@@ -819,11 +819,7 @@ pub async fn get_uds_and_original_data_streams(
);
if !stream_settings.defined_schema_fields.is_empty() {
- let mut fields =
- HashSet::<_>::from_iter(stream_settings.defined_schema_fields.iter().cloned());
- if !fields.contains(TIMESTAMP_COL_NAME) {
- fields.insert(TIMESTAMP_COL_NAME.to_string());
- }
+ let fields = uds_pack_fields(&stream_settings.defined_schema_fields);
user_defined_schema_map.insert(stream.stream_name.to_string(), Some(fields));
} else {
user_defined_schema_map.insert(stream.stream_name.to_string(), None);
@@ -831,6 +827,34 @@ pub async fn get_uds_and_original_data_streams(
}
}
+/// The fields to repack a batch with when its own schema check turned the UDS on, else None.
+pub fn newly_enabled_uds_fields(
+ had_uds: bool,
+ schema: Option<&SchemaCache>,
+) -> Option> {
+ if had_uds {
+ return None;
+ }
+ let settings = infra::schema::unwrap_stream_settings(schema?.schema())?;
+ if settings.defined_schema_fields.is_empty() {
+ return None;
+ }
+ let mut fields = uds_pack_fields(&settings.defined_schema_fields);
+ // ingest adds these after packing, so a repack must not move them into `_all`
+ fields.extend(settings.uds_internal_columns());
+ Some(fields)
+}
+
+/// Packs each record's columns outside `fields` into `_all`.
+pub fn repack_records<'a>(
+ records: impl IntoIterator- >,
+ fields: &HashSet,
+) {
+ for record in records {
+ *record = refactor_map(std::mem::take(record), fields);
+ }
+}
+
/// Calls the SnowflakeIdGenerator instance associated with this stream to generate a new i64 ID.
pub fn generate_record_id(org_id: &str, stream_name: &str, stream_type: &StreamType) -> i64 {
let key = format!("{org_id}/{stream_type}/{stream_name}");
@@ -890,6 +914,12 @@ pub fn refactor_map(
new_map
}
+fn uds_pack_fields(defined_schema_fields: &[String]) -> HashSet {
+ let mut fields = HashSet::<_>::from_iter(defined_schema_fields.iter().cloned());
+ fields.insert(TIMESTAMP_COL_NAME.to_string());
+ fields
+}
+
/// The downtime that silences a real-time firing, only when every ingested row is muted.
#[cfg(feature = "enterprise")]
async fn realtime_downtime(
@@ -1077,6 +1107,53 @@ mod tests {
use super::*;
+ #[test]
+ fn test_newly_enabled_uds_fields_only_when_the_check_turned_the_uds_on() {
+ let with_settings = |settings: &config::meta::stream::StreamSettings| {
+ SchemaCache::new(arrow_schema::Schema::empty().with_metadata(
+ std::collections::HashMap::from([(
+ "settings".to_string(),
+ json::to_string(settings).unwrap(),
+ )]),
+ ))
+ };
+ let uds = config::meta::stream::StreamSettings {
+ defined_schema_fields: vec!["foo".to_string()],
+ store_original_data: true,
+ ..Default::default()
+ };
+ let uds_schema = with_settings(&uds);
+ let no_uds_schema = with_settings(&Default::default());
+
+ let fields = newly_enabled_uds_fields(false, Some(&uds_schema)).unwrap();
+ for name in [
+ "foo",
+ TIMESTAMP_COL_NAME,
+ config::ID_COL_NAME,
+ config::ORIGINAL_DATA_COL_NAME,
+ ] {
+ assert!(fields.contains(name), "missing {name}");
+ }
+ assert!(newly_enabled_uds_fields(true, Some(&uds_schema)).is_none());
+ assert!(newly_enabled_uds_fields(false, Some(&no_uds_schema)).is_none());
+ assert!(newly_enabled_uds_fields(false, None).is_none());
+ }
+
+ #[test]
+ fn test_repack_records_moves_columns_outside_the_uds_into_all() {
+ let mut records = [json::json!({"foo": 1, "bar": "x"})
+ .as_object()
+ .unwrap()
+ .clone()];
+ let fields = HashSet::from(["foo".to_string()]);
+ repack_records(records.iter_mut(), &fields);
+ assert!(records[0].contains_key("foo") && !records[0].contains_key("bar"));
+ let all = records[0][&get_config().common.column_all]
+ .as_str()
+ .unwrap();
+ assert!(all.contains("bar"));
+ }
+
#[test]
fn test_only_the_fail_closed_refusal_is_recognised() {
let reason = config::meta::self_reporting::redaction::fail_closed_rejection(
diff --git a/src/core/src/logs/mod.rs b/src/core/src/logs/mod.rs
index 02c9792e41..7d00161405 100644
--- a/src/core/src/logs/mod.rs
+++ b/src/core/src/logs/mod.rs
@@ -50,8 +50,9 @@ use crate::{
alerts::alert::AlertExt,
common::meta::stream::SchemaRecords,
ingestion::{
- PartitionMemo, TriggerAlertData, evaluate_trigger, resolve_batch_schema,
- schema_records_to_entries, write_entries, write_entries_synced,
+ PartitionMemo, TriggerAlertData, evaluate_trigger, newly_enabled_uds_fields,
+ repack_records, resolve_batch_schema, schema_records_to_entries, write_entries,
+ write_entries_synced,
},
};
@@ -416,7 +417,7 @@ async fn write_logs(
org_id: &str,
stream_name: &str,
status: &mut IngestionStatus,
- json_data: Vec<(i64, Map)>,
+ mut json_data: Vec<(i64, Map)>,
columnar: Option,
is_derived: bool,
) -> Result {
@@ -482,7 +483,7 @@ async fn write_logs(
// End get stream alert
// start check for schema
- let (schema_evolution, infer_schema) = resolve_batch_schema(
+ let (mut schema_evolution, mut infer_schema) = resolve_batch_schema(
org_id,
stream_name,
StreamType::Logs,
@@ -492,6 +493,20 @@ async fn write_logs(
has_uds,
)
.await?;
+ // the check that turned the UDS on projected the schema, which would drop unpacked columns
+ if let Some(fields) = newly_enabled_uds_fields(has_uds, stream_schema_map.get(stream_name)) {
+ repack_records(json_data.iter_mut().map(|(_, record)| record), &fields);
+ (schema_evolution, infer_schema) = resolve_batch_schema(
+ org_id,
+ stream_name,
+ StreamType::Logs,
+ &mut stream_schema_map,
+ &json_data,
+ is_derived,
+ true,
+ )
+ .await?;
+ }
// get schema
let latest_schema = stream_schema_map
diff --git a/src/core/src/metrics/ingest.rs b/src/core/src/metrics/ingest.rs
index 1ae47bbe2c..14740e1bee 100644
--- a/src/core/src/metrics/ingest.rs
+++ b/src/core/src/metrics/ingest.rs
@@ -48,8 +48,8 @@ use crate::{
common::meta::stream::SchemaRecords,
ingestion::{
PartitionMemo, TriggerAlertData, evaluate_trigger, get_stream_partition_keys,
- get_thread_id, inferred_column_type, refactor_map, schema_records_to_entries,
- write_entries,
+ get_thread_id, inferred_column_type, newly_enabled_uds_fields, refactor_map,
+ repack_records, schema_records_to_entries, write_entries,
},
pipeline::batch_execution::ExecutablePipeline,
};
@@ -204,48 +204,38 @@ pub(super) async fn run_pipelines(
(outputs, failures)
}
-/// The stream's schema once `records` are in it, as the write path keys and partitions by it.
-pub(super) async fn resolve_batch_schema(
+/// The stream's schema once `rows` are in it, as the write path keys and partitions by it.
+pub(super) async fn resolve_batch_schema(
org_id: &str,
stream_name: &str,
stream_schema_map: &mut HashMap,
- records: &[&json::Map],
+ rows: &mut [T],
+ record: fn(&mut T) -> &mut json::Map,
min_timestamp: i64,
has_uds: bool,
) -> Result<(Arc, String)> {
- check_columns_per_record(org_id, stream_name, records)?;
- // probed once per field of every record, so it takes foldhash rather than SipHash
- let schema_fields: hashbrown::HashMap<&str, &DataType> =
- match stream_schema_map.get(stream_name) {
- Some(schema) => schema
- .schema()
- .fields()
- .iter()
- .map(|f| (f.name().as_str(), f.data_type()))
- .collect(),
- None => hashbrown::HashMap::default(),
- };
- let evolving: Vec<&json::Map> = records
- .iter()
- .copied()
- .filter(|record| record_evolves_schema(record, &schema_fields))
- .collect();
- drop(schema_fields);
- // a user-defined schema is applied by check_for_schema itself, so it cannot be skipped
- if !evolving.is_empty() || has_uds {
- let checked = if evolving.is_empty() {
- records.to_vec()
- } else {
- evolving
- };
- check_for_schema(
+ let records: Vec<_> = rows.iter_mut().map(|row| &*record(row)).collect();
+ check_metrics_schema(
+ org_id,
+ stream_name,
+ stream_schema_map,
+ &records,
+ min_timestamp,
+ has_uds,
+ )
+ .await?;
+ drop(records);
+ // the check that turned the UDS on projected the schema, which would drop unpacked columns
+ if let Some(fields) = newly_enabled_uds_fields(has_uds, stream_schema_map.get(stream_name)) {
+ repack_records(rows.iter_mut().map(record), &fields);
+ let records: Vec<_> = rows.iter_mut().map(|row| &*record(row)).collect();
+ check_metrics_schema(
org_id,
stream_name,
- StreamType::Metrics,
stream_schema_map,
- checked,
+ &records,
min_timestamp,
- false, // is_derived is false for metrics
+ true,
)
.await?;
}
@@ -504,6 +494,54 @@ pub(super) fn spawn_triggers(stream_trigger_map: HashMap,
+ records: &[&json::Map],
+ min_timestamp: i64,
+ has_uds: bool,
+) -> Result<()> {
+ check_columns_per_record(org_id, stream_name, records)?;
+ // probed once per field of every record, so it takes foldhash rather than SipHash
+ let schema_fields: hashbrown::HashMap<&str, &DataType> =
+ match stream_schema_map.get(stream_name) {
+ Some(schema) => schema
+ .schema()
+ .fields()
+ .iter()
+ .map(|f| (f.name().as_str(), f.data_type()))
+ .collect(),
+ None => hashbrown::HashMap::default(),
+ };
+ let evolving: Vec<&json::Map> = records
+ .iter()
+ .copied()
+ .filter(|record| record_evolves_schema(record, &schema_fields))
+ .collect();
+ drop(schema_fields);
+ // a user-defined schema is applied by check_for_schema itself, so it cannot be skipped
+ if !evolving.is_empty() || has_uds {
+ let checked = if evolving.is_empty() {
+ records.to_vec()
+ } else {
+ evolving
+ };
+ check_for_schema(
+ org_id,
+ stream_name,
+ StreamType::Metrics,
+ stream_schema_map,
+ checked,
+ min_timestamp,
+ false, // is_derived is false for metrics
+ )
+ .await?;
+ }
+ Ok(())
+}
+
/// `check_for_schema` applies the column limit to the batch's merged schema, not to one record.
fn check_columns_per_record(
org_id: &str,
diff --git a/src/core/src/metrics/json.rs b/src/core/src/metrics/json.rs
index 1ad55d1e1b..381e41ff3f 100644
--- a/src/core/src/metrics/json.rs
+++ b/src/core/src/metrics/json.rs
@@ -311,7 +311,7 @@ fn finish_rows(
async fn buffer_stream_rows(
org_id: &str,
stream_name: &str,
- (rows, first_type): (Vec
, Option<&'static str>),
+ (mut rows, first_type): (Vec, Option<&'static str>),
lookups: &mut StreamLookups,
partitions: &mut HashMap,
) -> Result> {
@@ -340,18 +340,16 @@ async fn buffer_stream_rows(
let has_uds = matches!(lookups.user_defined_schemas.get(stream_name), Some(Some(_)));
let min_timestamp = rows.iter().map(|(_, ts)| *ts).min().unwrap_or(0);
- let record_refs: Vec<&json::Map> =
- rows.iter().map(|(record, ..)| record).collect();
let (schema, schema_key) = ingest::resolve_batch_schema(
org_id,
stream_name,
&mut lookups.schemas,
- &record_refs,
+ &mut rows,
+ |(record, _)| record,
min_timestamp,
has_uds,
)
.await?;
- drop(record_refs);
let alerts = lookups
.alerts
diff --git a/src/core/src/metrics/otlp.rs b/src/core/src/metrics/otlp.rs
index a7b485f3ca..79ce589f82 100644
--- a/src/core/src/metrics/otlp.rs
+++ b/src/core/src/metrics/otlp.rs
@@ -642,9 +642,11 @@ pub async fn handle_otlp_request(
}
json_data
};
+ let mut json_data = json_data;
let record_refs: Vec<&json::Map> =
json_data.iter().map(|(record, _)| record).collect();
let min_timestamp = batch_min_timestamp(&record_refs, Utc::now().timestamp_micros());
+ drop(record_refs);
let has_uds = matches!(
user_defined_schema_map.get(&local_metric_name),
Some(Some(_))
@@ -653,12 +655,12 @@ pub async fn handle_otlp_request(
org_id,
&local_metric_name,
&mut metric_schema_map,
- &record_refs,
+ &mut json_data,
+ |(record, _)| record,
min_timestamp,
has_uds,
)
.await?;
- drop(record_refs);
let alerts = stream_alerts_map.get(&cache_stream_key(
org_id,
diff --git a/src/core/src/metrics/prom.rs b/src/core/src/metrics/prom.rs
index 9e0c93b80f..02f51cb3e4 100644
--- a/src/core/src/metrics/prom.rs
+++ b/src/core/src/metrics/prom.rs
@@ -663,18 +663,16 @@ pub async fn remote_write(
finish_identity_columns(&mut json_data);
let has_uds = matches!(user_defined_schema_map.get(&stream_name), Some(Some(_)));
let min_timestamp = json_data.iter().map(|(_, ts, _)| *ts).min().unwrap_or(0);
- let record_refs: Vec<&json::Map> =
- json_data.iter().map(|(record, ..)| record).collect();
let (schema, schema_key) = ingest::resolve_batch_schema(
org_id,
&stream_name,
&mut metric_schema_map,
- &record_refs,
+ &mut json_data,
+ |(record, ..)| record,
min_timestamp,
has_uds,
)
.await?;
- drop(record_refs);
let alerts =
stream_alerts_map.get(&cache_stream_key(org_id, StreamType::Metrics, &stream_name));
diff --git a/src/core/src/profiles/mod.rs b/src/core/src/profiles/mod.rs
index 26f7cd1a48..12f72bc628 100644
--- a/src/core/src/profiles/mod.rs
+++ b/src/core/src/profiles/mod.rs
@@ -29,7 +29,11 @@ use axum::{http, response::Response as HttpResponse};
use bytes::Bytes;
use config::{
TIMESTAMP_COL_NAME, get_config,
- meta::{otlp::OtlpRequestType, self_reporting::usage::UsageType, stream::StreamType},
+ meta::{
+ otlp::OtlpRequestType,
+ self_reporting::usage::UsageType,
+ stream::{StreamParams, StreamType},
+ },
metrics,
utils::{
json,
@@ -60,8 +64,9 @@ use crate::{
stream::SchemaRecords,
},
ingestion::{
- check_ingestion_allowed, get_stream_partition_keys, get_thread_id, get_write_partition_key,
- write_file,
+ check_ingestion_allowed, get_stream_partition_keys, get_thread_id,
+ get_uds_and_original_data_streams, get_write_partition_key, newly_enabled_uds_fields,
+ repack_records, write_file,
},
};
@@ -342,6 +347,22 @@ pub async fn handle_otlp_request(
#[cfg(feature = "vectorscan")]
apply_redaction(org_id, &stream_name, &mut all_records).await;
+ let mut user_defined_schema_map = HashMap::with_capacity(1);
+ get_uds_and_original_data_streams(
+ &[StreamParams::new(
+ org_id,
+ &stream_name,
+ StreamType::Profiles,
+ )],
+ &mut user_defined_schema_map,
+ &mut HashMap::new(),
+ &mut HashMap::new(),
+ )
+ .await;
+ if let Some(Some(fields)) = user_defined_schema_map.get(&stream_name) {
+ repack_records(all_records.iter_mut(), fields);
+ }
+
if !all_records.is_empty() {
buffer_records(
org_id,
@@ -396,7 +417,7 @@ pub async fn handle_otlp_request(
async fn buffer_records(
org_id: &str,
stream_name: &str,
- records: Vec>,
+ mut records: Vec>,
partition_keys: &Vec,
partition_time_level: config::meta::stream::PartitionTimeLevel,
write_buf: &mut HashMap,
@@ -435,17 +456,34 @@ async fn buffer_records(
stream_schema_map.insert(stream_name.to_string(), SchemaCache::new(schema));
}
- let record_refs: Vec<&json::Map> = records.iter().collect();
- let (_schema_evolution, _infer_schema) = check_for_schema(
+ let had_uds = stream_schema_map
+ .get(stream_name)
+ .and_then(|schema| infra::schema::unwrap_stream_settings(schema.schema()))
+ .is_some_and(|settings| !settings.defined_schema_fields.is_empty());
+ check_for_schema(
org_id,
stream_name,
StreamType::Profiles,
stream_schema_map,
- record_refs,
+ records.iter().collect(),
min_timestamp,
false,
)
.await?;
+ // the check that turned the UDS on projected the schema, which would drop unpacked columns
+ if let Some(fields) = newly_enabled_uds_fields(had_uds, stream_schema_map.get(stream_name)) {
+ repack_records(records.iter_mut(), &fields);
+ check_for_schema(
+ org_id,
+ stream_name,
+ StreamType::Profiles,
+ stream_schema_map,
+ records.iter().collect(),
+ min_timestamp,
+ false,
+ )
+ .await?;
+ }
let schema = stream_schema_map
.get(stream_name)
diff --git a/src/core/src/traces/mod.rs b/src/core/src/traces/mod.rs
index cf46a9cb5c..df1154700d 100644
--- a/src/core/src/traces/mod.rs
+++ b/src/core/src/traces/mod.rs
@@ -408,6 +408,27 @@ fn restore_derived_fields(
}
}
+/// Packs a batch into a UDS its own schema check turned on, keeping what per-span packing keeps.
+fn repack_for_new_uds(json_data: &mut [(i64, Map)], fields: &HashSet) {
+ let mut fields = fields.clone();
+ // per-span packing writes these after refactor_map, so they stay columns
+ fields.extend(
+ [
+ "gen_ai_agent_name",
+ "gen_ai_agent_id",
+ "gen_ai_agent_env",
+ "gen_ai_agent_version",
+ O2_INGEST_TS_COL_NAME,
+ ]
+ .map(str::to_string),
+ );
+ for (_, record_val) in json_data.iter_mut() {
+ let saved_fields = save_derived_fields_for_uds(record_val, &fields);
+ *record_val = crate::ingestion::refactor_map(std::mem::take(record_val), &fields);
+ restore_derived_fields(record_val, saved_fields);
+ }
+}
+
/// Derived identity columns of one OTLP span; true when DBM stamped it.
pub fn enrich_otlp_span(
span_kind: i32,
@@ -1849,7 +1870,7 @@ async fn write_traces_by_stream(
async fn write_traces(
org_id: &str,
stream_name: &str,
- json_data: Vec<(i64, json::Map)>,
+ mut json_data: Vec<(i64, json::Map)>,
) -> Result {
let cfg = get_config();
// get schema and stream settings
@@ -1908,6 +1929,23 @@ async fn write_traces(
)
.await
.map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidData, e.to_string()))?;
+ // the check that turned the UDS on projected the schema, which would drop unpacked columns
+ if let Some(fields) =
+ crate::ingestion::newly_enabled_uds_fields(has_uds, traces_schema_map.get(stream_name))
+ {
+ repack_for_new_uds(&mut json_data, &fields);
+ check_batch_schema(
+ org_id,
+ stream_name,
+ StreamType::Traces,
+ &mut traces_schema_map,
+ &json_data,
+ false,
+ true,
+ )
+ .await
+ .map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidData, e.to_string()))?;
+ }
let record_schema = traces_schema_map
.get(stream_name)
.unwrap()
@@ -3029,6 +3067,34 @@ mod tests {
assert_eq!(record_val, before);
}
+ #[test]
+ fn test_repack_for_new_uds_keeps_derived_and_agent_columns() {
+ use config::utils::json;
+
+ let record = json::json!({
+ "trace_id": "5b8efff798038103d269b633813fc60c",
+ "o2_db_system": "postgresql",
+ "infer_peer_key": "db:5432",
+ "gen_ai_agent_name": "planner",
+ "junk_attr": "packed",
+ });
+ let mut json_data = vec![(1, record.as_object().unwrap().clone())];
+ let fields = std::collections::HashSet::from(["trace_id".to_string()]);
+
+ super::repack_for_new_uds(&mut json_data, &fields);
+
+ let record = &json_data[0].1;
+ for name in [
+ "trace_id",
+ "o2_db_system",
+ "infer_peer_key",
+ "gen_ai_agent_name",
+ ] {
+ assert!(record.contains_key(name), "missing {name}");
+ }
+ assert!(!record.contains_key("junk_attr"));
+ }
+
#[test]
fn test_finalize_keeps_o2_db_fields_under_uds() {
// A user-defined schema that doesn't list the o2_db_* columns must not
diff --git a/src/schema/src/lib.rs b/src/schema/src/lib.rs
index a46aebcd66..1061024896 100644
--- a/src/schema/src/lib.rs
+++ b/src/schema/src/lib.rs
@@ -968,6 +968,20 @@ pub fn check_schema_for_defined_schema_fields(
}
}
}
+ StreamType::Profiles => {
+ fields.insert("stack".to_string());
+ fields.insert("value".to_string());
+ fields.insert("profile_type".to_string());
+ fields.insert("profile_unit".to_string());
+ fields.insert("service_name".to_string());
+ fields.insert("otel_scope_name".to_string());
+ fields.insert("service_instance_id".to_string());
+ fields.insert("host_name".to_string());
+ fields.insert("process_pid".to_string());
+ fields.insert("profile_duration_nanos".to_string());
+ fields.insert("trace_id".to_string());
+ fields.insert("span_id".to_string());
+ }
_ => {}
}
fields
@@ -1303,6 +1317,33 @@ mod tests {
assert!(result.contains("my_field"));
}
+ #[test]
+ fn test_check_schema_for_defined_schema_fields_profiles_adds_query_columns() {
+ let schema = Schema::new(vec![] as Vec);
+ let input = vec!["k8s_pod_name".to_string()];
+ let result = check_schema_for_defined_schema_fields(StreamType::Profiles, &schema, input);
+ let mut fields = result.iter().map(String::as_str).collect::>();
+ fields.sort_unstable();
+ assert_eq!(
+ fields,
+ [
+ "host_name",
+ "k8s_pod_name",
+ "otel_scope_name",
+ "process_pid",
+ "profile_duration_nanos",
+ "profile_type",
+ "profile_unit",
+ "service_instance_id",
+ "service_name",
+ "span_id",
+ "stack",
+ "trace_id",
+ "value",
+ ]
+ );
+ }
+
#[test]
fn test_check_schema_for_defined_schema_fields_traces_includes_gen_ai_fields() {
let schema = Schema::new(vec![
From e561cfcad94888d6b750ac8ffb0766967e578b18 Mon Sep 17 00:00:00 2001
From: Vamsi Karthik R
Date: Tue, 3 Nov 2026 06:54:41 +0000
Subject: [PATCH 5/7] test(oncall): run the on-call suites for real in
enterprise CI (#15987)
---
tests/api-testing/README.md | 2 +-
tests/api-testing/tests/oncall/conftest.py | 17 +-
.../tests/oncall/oncall_helpers.py | 91 ++++--
.../tests/oncall/test_alert_ack_rbac.py | 5 +-
.../tests/oncall/test_alert_mute_oncall.py | 2 +-
.../tests/oncall/test_oncall_ack_token.py | 143 ++++-----
.../tests/oncall/test_oncall_alert_binding.py | 4 +-
.../tests/oncall/test_oncall_escalation.py | 4 +-
.../tests/oncall/test_oncall_routing.py | 4 +-
.../tests/oncall/test_oncall_schedule.py | 102 ++++---
.../pages/oncallPages/oncallPagesListPage.js | 28 +-
.../oncallPages/oncallPolicyEditorPage.js | 5 -
.../oncallPages/oncallResponseDetailPage.js | 9 +
.../pages/oncallPages/oncallTeamDetailPage.js | 21 +-
.../OnCall/oncall-coverage.spec.js | 7 +-
.../OnCall/oncall-dryrun.spec.js | 8 +-
.../playwright-tests/OnCall/oncall-l0.spec.js | 168 ++++------
.../OnCall/oncall-members.spec.js | 5 +-
.../OnCall/oncall-mine.spec.js | 89 ++----
.../OnCall/oncall-onboarding.spec.js | 287 ++++++++----------
.../OnCall/oncall-page-lifecycle.spec.js | 140 ++++-----
.../OnCall/oncall-pages-list.spec.js | 148 +++++----
.../OnCall/oncall-policy.spec.js | 9 +-
.../OnCall/oncall-quickstart.spec.js | 37 +--
.../OnCall/oncall-rbac-ui.spec.js | 3 +-
.../OnCall/oncall-response-verbs.spec.js | 61 ++--
.../OnCall/oncall-routing.spec.js | 47 ++-
.../OnCall/oncall-schedule.spec.js | 16 +-
.../OnCall/oncall-snooze-lapse.spec.js | 3 +-
.../OnCall/oncall-team-health.spec.js | 33 +-
.../OnCall/oncall-teams-crud.spec.js | 184 +++++------
.../oncall-ent3160-schedule-and-pages.spec.js | 3 +-
.../playwright-tests/utils/mail-sink.js | 8 +-
.../playwright-tests/utils/oncall-org-lock.js | 119 ++++++++
.../playwright-tests/utils/oncall-seed-ext.js | 98 ++++--
.../playwright-tests/utils/oncall-seed.js | 255 +++++++---------
36 files changed, 1127 insertions(+), 1038 deletions(-)
create mode 100644 tests/ui-testing/playwright-tests/utils/oncall-org-lock.js
diff --git a/tests/api-testing/README.md b/tests/api-testing/README.md
index f285b9b9e6..bd723afe18 100644
--- a/tests/api-testing/README.md
+++ b/tests/api-testing/README.md
@@ -135,7 +135,7 @@ Fleet runs leave `fl_auto_*` orgs behind, because org delete is cloud-only. Most
## Troubleshooting
- **`KeyError: 'ZO_BASE_URL'` on collection** — `conftest.py` reads env vars at import. Export them before running pytest (even `--collect-only`).
-- **On-call ack-token tests skip** — they read the paging mail to get the acknowledgement token, which exists only in the delivered message. Start Mailpit (`MAILPIT_BASE_URL`, default `http://127.0.0.1:8025`) and point the server at it with `ZO_SMTP_ENABLED=true ZO_SMTP_HOST=127.0.0.1 ZO_SMTP_PORT=1025`. CI does not start Mailpit, so these skip there by design.
+- **On-call ack-token tests skip locally** — they read the paging mail to get the acknowledgement token, which exists only in the delivered message. Start Mailpit with `mailpit --smtp 127.0.0.1:1025 --listen 127.0.0.1:8025 --smtp-allowed-recipients '(?i)@test\.invalid$'` (the filter makes mail to the reachability tests' other domain a real refusal), point the server at it with `ZO_SMTP_ENABLED=true ZO_SMTP_HOST=127.0.0.1 ZO_SMTP_PORT=1025 ZO_SMTP_FROM_EMAIL=oncall@ci.openobserve.test`, and set `MAILPIT_BASE_URL` if it is not `http://127.0.0.1:8025`. With no mailbox they skip locally and fail when `CI` is set. The enterprise `ent_oncall` shard starts Mailpit v1.21.8 on 1025/8025 with that recipient filter and starts the server with the same SMTP settings.
- **`test_workflow.py` fails with SMTP error** — `ZO_SMTP_ENABLED=true` not set. CI does this; locally you'd need to wire an SMTP env.
- **Test_organisations.py warning about `yield`** — `test_create_organization` uses pre-pytest-4 yield-style and is silently ignored. Scheduled for rewrite in Phase 4.
diff --git a/tests/api-testing/tests/oncall/conftest.py b/tests/api-testing/tests/oncall/conftest.py
index dae806a0ea..1227ba5945 100644
--- a/tests/api-testing/tests/oncall/conftest.py
+++ b/tests/api-testing/tests/oncall/conftest.py
@@ -5,14 +5,13 @@ template and the two Alert Destinations already provisioned, and a teardown
sweep of every team, alert, ownership rule and user the module registered on it.
Everything here is **enterprise-gated**: `_require_oncall` skips the whole
-directory when on-call is not served, so an OSS build reports skips rather than
-a wall of 404s.
+directory on an OSS build, so it reports skips rather than a wall of 404s, and
+FAILS on an enterprise build with on-call off rather than skipping green.
-It decides that by PROBING `GET {org}/oncall/teams` — 200/403 served, 404 absent
-— not by reading `oncall_enabled` off `/config`. That key is what the original
-issue quoted, and a current enterprise build does not publish it on the public
-`/config`, so gating on it skipped all 104 tests against a server that was
-serving on-call perfectly well. See `oncall_helpers.oncall_enabled`.
+It decides that by PROBING `GET {org}/oncall/teams` — 200/403 served, 404 not —
+and on a 404 reading `build_type` off the public `/config`. No on-call flag is
+published there, so gating on one skipped all 104 tests against a server that
+was serving on-call perfectly well. See `oncall_helpers.oncall_enabled`.
Scope is `module`, not `function`: a page only exists once the alert scheduler
has evaluated a rule, which costs the better part of a minute, and a per-test
@@ -46,14 +45,14 @@ def oncall_available(client: OpenObserveClient, oncall_org: str) -> bool:
@pytest.fixture(scope="session", autouse=True)
def _require_oncall(oncall_available: bool) -> None:
- """Skip cleanly rather than fail when the feature is not built or not on.
+ """Skip cleanly on an OSS build; `oncall_enabled` raises on an enterprise build with on-call off.
Session-scoped on purpose: a function-scoped gate is set up AFTER the
module-scoped fixtures it is meant to protect, so an OSS build would seed
teams and wait on pages that can never arrive before ever reaching the skip.
"""
if not oncall_available:
- pytest.skip("this build does not serve the on-call routes")
+ pytest.skip("on-call is enterprise-only and this is an OSS build")
@pytest.fixture(scope="module")
diff --git a/tests/api-testing/tests/oncall/oncall_helpers.py b/tests/api-testing/tests/oncall/oncall_helpers.py
index bf37aa8d5d..10c6481932 100644
--- a/tests/api-testing/tests/oncall/oncall_helpers.py
+++ b/tests/api-testing/tests/oncall/oncall_helpers.py
@@ -12,11 +12,12 @@ the behaviour under test.
**Enterprise-gated.** Every route here exists only when `O2_ONCALL_ENABLED` is
on in an enterprise build; the router does not even register them otherwise, so
-an OSS instance answers 404 rather than 403. `oncall_enabled()` probes
-`GET {org}/oncall/teams` and skips on a 404. It does NOT read a `/config` flag:
-a current enterprise build does not publish `config.oncall_enabled` at all, and
-gating on it skipped all 104 tests against a server that was serving on-call
-perfectly well. Any status other than 200/403/404 raises rather than skips.
+the probe `GET {org}/oncall/teams` answers 404 rather than 403. `oncall_enabled()`
+skips on that 404 only when the public `/config` reports `build_type:
+opensource`; on an enterprise build the 404 means the flag is off (the default
+since o2-enterprise#2748), and it raises, because skipping there hid the whole
+suite in CI. It does NOT read an on-call flag off `/config`: none is published.
+Any status other than 200/403/404 raises rather than skips.
Environment the suite reads:
@@ -25,11 +26,15 @@ Environment the suite reads:
* `MAILPIT_BASE_URL` (default `http://127.0.0.1:8025`) — where to read the
paging mail. An acknowledgement token is minted per person and exists ONLY
inside the delivered mail: no API exposes it. So without a reachable
- mailbox the ack-token cases cannot run, and they SKIP with that reason
- rather than passing emptily. Nothing starts Mailpit in CI today, so those
- cases are a known, visible coverage gap rather than an enforced one; run
- them locally with `mailpit` on its default ports and the server pointed at
- it (`ZO_SMTP_ENABLED=true ZO_SMTP_HOST=127.0.0.1 ZO_SMTP_PORT=1025`).
+ mailbox the ack-token cases cannot run: they SKIP locally and FAIL when
+ `CI` is set (`skip_or_fail_in_ci`), because the enterprise `ent_oncall` API
+ shard starts Mailpit, so a missing mailbox there is a broken job. That
+ Mailpit accepts only `test.invalid` recipients and refuses `MAILBOX_DOMAIN`,
+ which is how §7 gets real delivery failures beside real deliveries. Locally:
+ `mailpit --smtp-allowed-recipients '(?i)@test\\.invalid$'` on its default
+ ports and the server pointed at it (`ZO_SMTP_ENABLED=true
+ ZO_SMTP_HOST=127.0.0.1 ZO_SMTP_PORT=1025 ZO_SMTP_FROM_EMAIL=`;
+ without a From address every send fails "Invalid input").
Org addressing: on a multi-tenant deployment an org is addressed by its
**identifier** (a ksuid), not by its display name. `resolve_org` maps whichever
@@ -60,8 +65,10 @@ import re
import time
import uuid
from datetime import UTC, datetime, timedelta
-from typing import Any
+from typing import Any, NoReturn
+from urllib.parse import urljoin
+import pytest
import requests
from support.client import OpenObserveClient
@@ -116,6 +123,9 @@ SINK_DEST = f"{PREFIX}_dest_sink"
DEAD_DEST = f"{PREFIX}_dest_dead"
DEAD_URL = "http://127.0.0.1:1/never-listens"
+# Not IANA-reserved, so reachability counts its failures; it has no MX or A record, and the ent_oncall CI Mailpit refuses it.
+MAILBOX_DOMAIN = "oncall-pt.openobserve.ai"
+
def uniq(prefix: str = PREFIX) -> str:
"""A unique, human-readable name. uuid4, not a timestamp: two xdist workers
@@ -149,30 +159,47 @@ def resolve_org(client: OpenObserveClient, wanted: str) -> str:
return wanted
+def build_type(client: OpenObserveClient) -> str:
+ """`opensource`, `enterprise` or `cloud`, read off the unauthenticated `/config`."""
+ url = urljoin(client.base_url, "config")
+ try:
+ resp = requests.get(url, timeout=client.timeout)
+ body = resp.json() if resp.status_code == 200 else None
+ except (requests.RequestException, ValueError) as exc:
+ raise AssertionError(f"cannot read build_type from {url}: {exc}") from exc
+ kind = body.get("build_type") if isinstance(body, dict) else None
+ if not kind:
+ raise AssertionError(
+ f"no build_type from {url}: HTTP {resp.status_code} — {resp.text[:200]}")
+ return kind
+
+
def oncall_enabled(client: OpenObserveClient, org: str) -> bool:
- """Whether this build serves the on-call routes.
+ """Whether this build serves the on-call routes; False only on an OSS build.
- Probes `GET {org}/oncall/teams` rather than reading a flag off `/config`.
- An earlier version read `config.oncall_enabled`, which is what the issue
- report quoted — but a current enterprise build does not publish that key at
- all, so the gate skipped all 104 tests against a server that was serving
- on-call perfectly well.
+ Probes `GET {org}/oncall/teams`: 200 or 403 (the route answering) means
+ served. A 404 is a skip only when `/config` says `opensource`; an enterprise
+ build answering 404 has `O2_ONCALL_ENABLED` off, and skipping there is what
+ turned every enterprise CI run of this suite into a silent green skip.
- A 200 (or a 403, which is still the route answering) means the feature is
- there; a 404 means this build does not carry it. ANYTHING ELSE RAISES, and
- deliberately so: this gate is autouse and session-scoped, so returning False
- for a 500 would skip every on-call test and report a green run against a
- broken server. A deployment that cannot answer is not a deployment without
- the feature, and the two must not collapse into the same silent skip.
+ EVERYTHING ELSE RAISES: this gate is autouse and session-scoped, so returning
+ False for a 500 would report a green run against a broken server.
"""
resp = client.get("oncall/teams", org=org)
if resp.status_code in (200, 403):
return True
- if resp.status_code == 404:
+ if resp.status_code != 404:
+ raise AssertionError(
+ f"on-call probe failed: HTTP {resp.status_code} on {org}/oncall/teams — "
+ f"{resp.text[:200]}"
+ )
+ kind = build_type(client)
+ if kind == "opensource":
return False
raise AssertionError(
- f"on-call probe failed: HTTP {resp.status_code} on {org}/oncall/teams — "
- f"{resp.text[:200]}"
+ f"on-call is off on this {kind} build: {org}/oncall/teams answered 404. "
+ "Start the server with O2_ONCALL_ENABLED=true (off by default since "
+ "o2-enterprise#2748) — this suite fails rather than skips on an enterprise build."
)
@@ -211,6 +238,13 @@ def mailpit_available() -> bool:
return False
+def skip_or_fail_in_ci(reason: str) -> NoReturn:
+ """Skip locally; fail when `CI` is set, where the ent_oncall shard provisions every dependency these cases need."""
+ if os.environ.get("CI"):
+ pytest.fail(f"{reason} (CI is set: the ent_oncall job provides this, so its absence is a broken job)")
+ pytest.skip(reason)
+
+
def ack_link_for(recipient_marker: str, *, timeout_s: int = 180) -> str | None:
"""The acknowledge URL out of the newest page email addressed to a recipient.
@@ -796,14 +830,15 @@ class OnCallClient:
def make_user(client: OpenObserveClient, org: str, role: str,
- password: str = "Complexpass#123") -> tuple[str, OpenObserveClient]:
+ password: str = "Complexpass#123", *,
+ domain: str = "test.invalid") -> tuple[str, OpenObserveClient]:
"""Create an org user with `role` and return (email, a client acting as them).
The returned client authenticates as that user, which is the only way to
assert what the API refuses them — asserting with root's credentials and a
`user_email` field in the body would test nothing.
"""
- email = f"{PREFIX}_{role}_{uuid.uuid4().hex[:8]}@test.invalid"
+ email = f"{PREFIX}_{role}_{uuid.uuid4().hex[:8]}@{domain}"
payload = {
"email": email, "password": password, "first_name": "OnCall",
"last_name": role.capitalize(), "role": role, "organization": org,
diff --git a/tests/api-testing/tests/oncall/test_alert_ack_rbac.py b/tests/api-testing/tests/oncall/test_alert_ack_rbac.py
index 1129786b6f..f3a44bf0d6 100644
--- a/tests/api-testing/tests/oncall/test_alert_ack_rbac.py
+++ b/tests/api-testing/tests/oncall/test_alert_ack_rbac.py
@@ -70,7 +70,7 @@ def editor(client: OpenObserveClient, oncall_org: str, oncall: OnCallClient) ->
@pytest.fixture(scope="module")
def member(client: OpenObserveClient, oncall_org: str, oncall: OnCallClient) -> tuple[str, OpenObserveClient]:
- return _non_admin(client, oncall_org, oncall, "user")
+ return _non_admin(client, oncall_org, oncall, "viewer")
@pytest.fixture(scope="module")
@@ -131,5 +131,6 @@ def test_an_ack_on_the_page_acks_the_alert(oncall: OnCallClient, admin):
assert oncall.acknowledge(page["id"]).status_code == 200
acks = wait_until(lambda: _acks(oncall, alert_id), timeout=30, interval=2, msg="the page ack reached the alert")
- assert acks[0]["ack_source"] == "oncall"
+ # The source records how the person acked (an API call here), not that it was a page.
+ assert acks[0]["ack_source"] == "api"
assert acks[0]["acked_by"] == oncall._c.email
diff --git a/tests/api-testing/tests/oncall/test_alert_mute_oncall.py b/tests/api-testing/tests/oncall/test_alert_mute_oncall.py
index 4cc5e1c487..5bf236de88 100644
--- a/tests/api-testing/tests/oncall/test_alert_mute_oncall.py
+++ b/tests/api-testing/tests/oncall/test_alert_mute_oncall.py
@@ -141,7 +141,7 @@ def test_until_it_clears_with_keep_firing_for_ends_on_the_run_that_ends_the_hold
def test_the_mute_rule_refuses_a_viewer_with_names_and_lets_an_editor_and_a_responder_mute(
client: OpenObserveClient, oncall_org: str, oncall: OnCallClient, admin):
- responder_email, as_responder = _non_admin(client, oncall_org, oncall, "user")
+ responder_email, as_responder = _non_admin(client, oncall_org, oncall, "viewer")
_, as_viewer = _non_admin(client, oncall_org, oncall, "viewer")
_, as_editor = _non_admin(client, oncall_org, oncall, "editor")
admin_email, _ = admin
diff --git a/tests/api-testing/tests/oncall/test_oncall_ack_token.py b/tests/api-testing/tests/oncall/test_oncall_ack_token.py
index 83691a39bd..284242afbc 100644
--- a/tests/api-testing/tests/oncall/test_oncall_ack_token.py
+++ b/tests/api-testing/tests/oncall/test_oncall_ack_token.py
@@ -26,7 +26,7 @@ why this surface carries the sharpest invariants in the feature — from
* a GET must only look — a link-prefetching mail gateway would otherwise
acknowledge a page nobody read
-## The token could not be obtained, and nothing here pretends otherwise
+## The token exists only inside the page email, so it is read from a mailbox
An acknowledgement token is minted inside the enterprise crate
(`o2_enterprise::enterprise::oncall::token`) and embedded in the outgoing
@@ -44,18 +44,18 @@ notification. Everything reachable from this repo was checked:
(the one OSS reference, src/jobs/src/job/oncall_maintenance.rs, passes None).
* No notification- or template-preview route exists.
-So 12.3, 12.4 and 12.6 are BLOCKED on a mailbox (or an enterprise mint helper)
-and are marked skip with that reason. They are written out in full rather than
-stubbed, so they run the moment `minted_ack_token` can return a real one. No
-token is fabricated and no verifier is stubbed to turn them green — a test that
-passes without exercising `ack_claims` would prove nothing about this route.
+So 12.3, 12.4 and 12.6 read the token out of a real mailbox (Mailpit), which
+the enterprise `ent_oncall` API shard starts. Without one they skip locally and
+FAIL when `CI` is set: a skip there would hide a broken job behind a green run.
+No token is fabricated and no verifier is stubbed to turn them green — a test
+that passes without exercising `ack_claims` would prove nothing about this route.
12.1, 12.2, 12.5 and 12.7 need only a malformed, expired or foreign token and
run for real. Read the honest note on each: with no signing key on this side, a
401 cannot be attributed to the *specific* check that produced it, so the
expired and foreign-org cases pin "no such token acts", not "the expiry check
fired". 12.7 is live for everything observable without a valid token (a GET
-never says "Acknowledged", never carries a Location) and its full form is skipped.
+never says "Acknowledged", never carries a Location) and its full form needs the mailbox.
Only the pages' fixed headings are asserted ("Acknowledged", "Already acknowledged
by", "This link no longer works"); the rest is markup a person reads. Status codes,
@@ -76,20 +76,13 @@ from tests.oncall.oncall_helpers import (
ack_link_for,
mailpit_available,
make_user,
+ oncall_enabled,
rotation,
shift_rule,
+ skip_or_fail_in_ci,
uniq,
)
-TOKEN_MINT_UNAVAILABLE = (
- "no acknowledgement token is obtainable: it is minted inside "
- "o2_enterprise::enterprise::oncall::token and only ever leaves the server "
- "inside the notification body. The delivery ledger "
- "(GET /oncall/responses/{id}/deliveries), the personal inbox "
- "(GET /oncall/my/deliveries) and test-page all omit it, and no "
- "notification-preview or mint endpoint exists. Needs a captured mailbox or "
- "an enterprise-side mint helper exposed to tests."
-)
# JWT-shaped so the route gets something structurally plausible to reject, but
# signed with nothing — this must never be mistaken for a real token.
@@ -97,9 +90,7 @@ UNSIGNED_SIGNATURE = "not-a-real-signature"
# Shapes a hostile or broken link can arrive in. Each must be refused, and
# refused the same way: 401 from the claims check.
-# The whole feature needs an enterprise build with `O2_ONCALL_ENABLED`; the
-# marker lets CI hold this directory separately, and the session-scoped gate
-# in conftest skips it rather than failing when the flag is off.
+# Enterprise-only: the conftest gate skips on an OSS build and fails on an enterprise one without `O2_ONCALL_ENABLED`.
pytestmark = pytest.mark.enterprise
MALFORMED_TOKENS = [
@@ -118,26 +109,14 @@ MALFORMED_TOKENS = [
# =============================================================================
@pytest.fixture(scope="module", autouse=True)
-def require_oncall(create_session, base_url, org_id):
- """Skip the module unless the build actually serves the on-call routes.
+def require_oncall(client, org_id):
+ """Skip the module only on an OSS build; `oncall_enabled` raises on anything else.
- Probes `oncall/teams` rather than reading `config.oncall_enabled`: that key
- is what the issue report quoted, but a current enterprise build does not
- publish it, and gating on it skipped every test against a server that was
- serving on-call correctly. A 404 means the routes are not registered, in
- which case `/oncall/ack` falls through to the authenticated catch-all and
- every assertion below would be measuring the auth middleware instead.
+ Without the routes `/oncall/ack` falls through to the authenticated
+ catch-all, and every assertion below would be measuring the auth middleware.
"""
- resp = create_session.get(f"{base_url}api/{org_id}/oncall/teams")
- if resp.status_code == 404:
- pytest.skip("this build does not serve the on-call routes")
- # 404 above is the only legitimate skip: the routes are genuinely absent. Any
- # OTHER unexpected status is the server being broken, and skipping on it takes
- # all 42 cases in this file with it — a 500 would have retired the whole ack
- # surface silently.
- assert resp.status_code in (200, 403), (
- f"on-call is registered but `oncall/teams` answered {resp.status_code}, so the "
- f"server is not healthy enough to measure the ack route against: {resp.text[:200]}")
+ if not oncall_enabled(client, org_id):
+ pytest.skip("on-call is enterprise-only and this is an OSS build")
@pytest.fixture
@@ -152,7 +131,31 @@ def foreign_org():
@pytest.fixture
def minted_ack_token(oncall):
- """A real token, the response it acks, and its subject.
+ """A real token, the response it acks, and its (admin) subject, alone on the team."""
+ return mint_ack_token(oncall, make_user(oncall._c, oncall.org, "admin")[0])
+
+
+@pytest.fixture
+def viewer_ack_token_with_a_teammate(oncall):
+ """A real token whose subject is a viewer, on a team that has a second member.
+
+ Neither exemption in `record_for_actor` applies: a viewer cannot work any
+ page on role alone, and a team that still has members is not open to the org.
+ """
+ try:
+ subject, _ = make_user(oncall._c, oncall.org, "viewer")
+ except AssertionError as e:
+ if "require open-fga" in str(e):
+ pytest.skip("non-admin roles need OpenFGA on")
+ raise
+ oncall.users.append(subject)
+ teammate, _ = make_user(oncall._c, oncall.org, "admin")
+ oncall.users.append(teammate)
+ return mint_ack_token(oncall, subject, teammates=[teammate])
+
+
+def mint_ack_token(oncall, email, teammates=()):
+ """Page `email` for real and return (token, response id, email).
The token is minted server-side and leaves only inside the page email, so it
is read back out of a mailbox. A TEST page will not do — those are sent with
@@ -161,30 +164,31 @@ def minted_ack_token(oncall):
on call" instead of a page, which is the trap that looks like a missing token.
"""
if not mailpit_available():
- pytest.skip(
+ skip_or_fail_in_ci(
f"no readable mailbox at {MAILPIT_BASE}; the ack token exists only "
"inside a page email. Run Mailpit and point the server's SMTP at it."
)
- email, _ = make_user(oncall._c, oncall.org, "admin")
marker = email.split("@")[0]
team_id = oncall.team_id(uniq("oncall_ack_team"))
- oncall.add_member(team_id, email)
+ for member in (email, *teammates):
+ added = oncall.add_member(team_id, member)
+ assert added.status_code == 200, f"adding {member} refused: {added.status_code} {added.text}"
resp = oncall.set_schedule(team_id, [rotation("R1", "Primary", [shift_rule("L1", [email])])])
assert resp.status_code == 200, f"schedule refused: {resp.status_code} {resp.text}"
page = oncall.open_pages_for([team_id], priority=2)[0]
link = ack_link_for(marker)
if not link:
- pytest.skip(
+ skip_or_fail_in_ci(
"a page opened but no acknowledge link reached the mailbox within the "
"wait; check the server's SMTP settings and ZO_WEB_URL"
)
token = urllib.parse.parse_qs(urllib.parse.urlparse(link).query).get("token", [None])[0]
assert token, f"ack link carried no token: {link}"
- yield token, page["id"], email
+ return token, page["id"], email
# =============================================================================
@@ -509,12 +513,10 @@ def test_the_ack_route_does_not_enumerate_orgs(base_url, org_id, foreign_org):
@pytest.mark.parametrize("token", MALFORMED_TOKENS)
def test_a_get_never_redirects(base_url, org_id, token):
"""The "Acknowledged" page is the only thing an acknowledgement emits, and a
- GET must never emit it, nor a redirect. A mail gateway prefetching the link would otherwise answer the
- page on the responder's behalf.
+ GET must never emit it, nor a redirect. A mail gateway prefetching the link
+ would otherwise answer the page on the responder's behalf.
- Partial by necessity: without a valid token this covers every shape a
- prefetcher could be handed from a *broken* link, not the real one. The full
- case is the skipped test below.
+ Covers only broken-link shapes; the valid-token case below runs whenever the mailbox is reachable.
"""
assert_did_not_act(get_ack(base_url, org_id, token))
@@ -570,7 +572,7 @@ def test_a_get_with_a_valid_token_does_not_acknowledge(create_session, base_url,
# =============================================================================
-# 12.3, 12.4, 12.6 — blocked on a token that cannot be obtained
+# 12.3, 12.4, 12.6 — a real token, read from the page email
# =============================================================================
def test_a_valid_token_acknowledges_and_links_with_the_org(create_session, base_url,
@@ -652,26 +654,15 @@ def test_a_used_link_opened_again_shows_the_state_not_the_button(create_session,
assert_did_not_act(again)
-@pytest.mark.xfail(
- reason=(
- "PRODUCT DEFECT, not a test gap: the emailed link still acknowledges after "
- "its subject is removed from the team. Measured on :5090 — remove the member "
- "(200, membership empty), POST the link, 200 and the record reads acknowledged, "
- "attributed to the removed person. The signed-in path refuses the same action "
- "with NotOnThisTeam. o2-enterprise#2481."
- ),
- strict=True,
-)
-def test_a_token_whose_subject_left_the_team_is_refused(create_session, base_url, org_id,
- minted_ack_token):
- """12.6, and invariant 3: claims resolve through `service::ack_claims`, so
- the same membership check as the authenticated route applies here.
+def test_a_token_whose_subject_left_the_team_does_not_acknowledge(
+ create_session, base_url, org_id, viewer_ack_token_with_a_teammate):
+ """12.6: a viewer removed from a team that still has members cannot take its page from their old email.
- The token is minted while the subject is on the team and presented after
- they are removed — a leaver's old page email must stop working, or §1's
- team scoping has a hole in it that needs no credentials at all.
+ `ack_claims` checks only org membership, so the link still opens; the ack
+ rule then needs edit access to the alert or being on call for it, hence the
+ viewer subject, and the teammate keeps the team from being emptied.
"""
- token, response_id, subject = minted_ack_token
+ token, response_id, subject = viewer_ack_token_with_a_teammate
record = create_session.get(f"{base_url}api/{org_id}/oncall/responses/{response_id}")
assert record.status_code == 200, record.text
@@ -682,10 +673,22 @@ def test_a_token_whose_subject_left_the_team_is_refused(create_session, base_url
params={"user_email": subject},
)
assert removed.status_code == 200, removed.text
+ roster = create_session.get(f"{base_url}api/{org_id}/oncall/teams/{team_id}/members")
+ assert roster.status_code == 200, roster.text
+ remaining = [m.get("user_email") for m in roster.json()]
+ # An emptied team is open to the whole org, which would make this case measure the wrong rule.
+ assert remaining, "the teammate must still be on the team"
+ assert subject not in remaining, f"the subject must be gone: {remaining}"
- assert_handler_refused(post_ack(base_url, org_id, token))
- assert_handler_refused(get_ack(base_url, org_id, token))
+ assert_did_not_act(get_ack(base_url, org_id, token))
+
+ leaver_post = post_ack(base_url, org_id, token)
+ assert leaver_post.status_code == 403, f"{leaver_post.status_code}: {leaver_post.text[:300]}"
+ assert leaver_post.json().get("code") == "ack_refused", leaver_post.text[:300]
+ assert_did_not_act(leaver_post)
after = create_session.get(f"{base_url}api/{org_id}/oncall/responses/{response_id}")
assert after.status_code == 200, after.text
- assert after.json()["response"].get("state") != "acknowledged"
+ body = after.json()["response"]
+ assert body.get("state") != "acknowledged", body
+ assert (body.get("acked_by") or "").lower() != subject.lower(), body
diff --git a/tests/api-testing/tests/oncall/test_oncall_alert_binding.py b/tests/api-testing/tests/oncall/test_oncall_alert_binding.py
index 0e1b8f0a5a..f0b309c7b7 100644
--- a/tests/api-testing/tests/oncall/test_oncall_alert_binding.py
+++ b/tests/api-testing/tests/oncall/test_oncall_alert_binding.py
@@ -51,9 +51,7 @@ from .oncall_helpers import (
logger = logging.getLogger(__name__)
-# The whole feature needs an enterprise build with `O2_ONCALL_ENABLED`; the
-# marker lets CI hold this directory separately, and the session-scoped gate
-# in conftest skips it rather than failing when the flag is off.
+# Enterprise-only: the conftest gate skips on an OSS build and fails on an enterprise one without `O2_ONCALL_ENABLED`.
pytestmark = pytest.mark.enterprise
# A column no semantic field group maps to a dimension, so no ownership rule
diff --git a/tests/api-testing/tests/oncall/test_oncall_escalation.py b/tests/api-testing/tests/oncall/test_oncall_escalation.py
index aed112170f..6d0b15a3be 100644
--- a/tests/api-testing/tests/oncall/test_oncall_escalation.py
+++ b/tests/api-testing/tests/oncall/test_oncall_escalation.py
@@ -52,9 +52,7 @@ from .oncall_helpers import (
logger = logging.getLogger(__name__)
-# The whole feature needs an enterprise build with `O2_ONCALL_ENABLED`; the
-# marker lets CI hold this directory separately, and the session-scoped gate
-# in conftest skips it rather than failing when the flag is off.
+# Enterprise-only: the conftest gate skips on an OSS build and fails on an enterprise one without `O2_ONCALL_ENABLED`.
pytestmark = pytest.mark.enterprise
diff --git a/tests/api-testing/tests/oncall/test_oncall_routing.py b/tests/api-testing/tests/oncall/test_oncall_routing.py
index f3560fc999..0a2f206f1a 100644
--- a/tests/api-testing/tests/oncall/test_oncall_routing.py
+++ b/tests/api-testing/tests/oncall/test_oncall_routing.py
@@ -54,9 +54,7 @@ from .oncall_helpers import (
logger = logging.getLogger(__name__)
-# The whole feature needs an enterprise build with `O2_ONCALL_ENABLED`; the
-# marker lets CI hold this directory separately, and the session-scoped gate
-# in conftest skips it rather than failing when the flag is off.
+# Enterprise-only: the conftest gate skips on an OSS build and fails on an enterprise one without `O2_ONCALL_ENABLED`.
pytestmark = pytest.mark.enterprise
# Two owning namespaces and one nobody claims, all three uniquified. The owned
diff --git a/tests/api-testing/tests/oncall/test_oncall_schedule.py b/tests/api-testing/tests/oncall/test_oncall_schedule.py
index 31d916aa55..c2600030de 100644
--- a/tests/api-testing/tests/oncall/test_oncall_schedule.py
+++ b/tests/api-testing/tests/oncall/test_oncall_schedule.py
@@ -19,12 +19,15 @@ window applies at no instant at all, which is what §8.2 refuses.
§7's two-failure threshold is deliberate, and §7.2 exists to guard it against
being tightened: one blip must not bench somebody who is holding the pager.
Producing a *recorded* delivery failure needs a transport that is configured
-and fails, which is what the helper's dead destination (a closed port on
-loopback) is for. Where the deployment cannot deliver to a member at all — no
-SMTP, no webhook — a degradation caused by failures cannot be told apart from
-one caused by there being no transport, and those tests skip with that reason
-rather than asserting something they cannot see. That is a real gap in coverage
-on an SMTP-less instance and is called out rather than papered over.
+and fails. Email alone decides `would_a_page_land`, and delivery health is kept
+per recipient and channel (a webhook page is recorded against the team's
+channel, never the person), so the failures are email ones: mail configured but
+not landing. In the enterprise `ent_oncall` CI shard that is a Mailpit that
+refuses `MAILBOX_DOMAIN` (and accepts the ack-token tests' `test.invalid`).
+Where the deployment cannot deliver to a member at all — no SMTP — or where its
+mail does land, a degradation caused by failures cannot be produced: those tests
+skip with that reason locally, and FAIL when `CI` is set, where either state
+means the shard's mail setup is broken.
"""
from __future__ import annotations
@@ -38,15 +41,17 @@ from support.client import OpenObserveClient
from support.wait import WaitTimeout
from .oncall_helpers import (
- DEAD_DEST,
+ MAILBOX_DOMAIN,
MINUTES_PER_DAY,
OnCallClient,
+ delivered_recipients,
ladder,
make_user,
micros_from_now,
rotation,
rotation_target,
shift_rule,
+ skip_or_fail_in_ci,
uniq,
user_target,
)
@@ -54,9 +59,7 @@ from .oncall_helpers import failed_recipients as failures_in
logger = logging.getLogger(__name__)
-# The whole feature needs an enterprise build with `O2_ONCALL_ENABLED`; the
-# marker lets CI hold this directory separately, and the session-scoped gate
-# in conftest skips it rather than failing when the flag is off.
+# Enterprise-only: the conftest gate skips on an OSS build and fails on an enterprise one without `O2_ONCALL_ENABLED`.
pytestmark = pytest.mark.enterprise
ALL_DAYS = [0, 1, 2, 3, 4, 5, 6]
@@ -72,7 +75,7 @@ DEGRADE_AFTER = 2
@pytest.fixture(scope="module")
def rota_member(client: OpenObserveClient, oncall: OnCallClient, oncall_org: str) -> str:
- email, _unused = make_user(client, oncall_org, "user")
+ email, _unused = make_user(client, oncall_org, "user", domain=MAILBOX_DOMAIN)
oncall.users.append(email)
return email
@@ -93,6 +96,12 @@ def member_verdict(oncall: OnCallClient, team_id: str, email: str) -> dict | Non
return None
+def skip_if_mail_lands(ledger: dict, email: str) -> None:
+ """A deployment whose mail is delivered cannot produce the failures §7 counts."""
+ if any(email.lower() in r.lower() for r in delivered_recipients(ledger)):
+ skip_or_fail_in_ci("mail to this member was delivered here, so no delivery failure can be produced")
+
+
def one_rotation(rotation_id: str, members: list[str], **kw) -> list[dict]:
"""A schedule body with a single named rotation."""
return [rotation(rotation_id, "Primary",
@@ -269,36 +278,50 @@ def test_the_escalation_preview_agrees_with_reachability(
f"preview says {recipient}, reachability says {verdict}")
-def test_a_preview_for_a_transport_that_can_deliver_nothing_says_so(
- oncall: OnCallClient, rota_team: str, rota_member: str):
+def test_a_preview_whose_ladder_can_reach_nobody_says_so(
+ client: OpenObserveClient, oncall: OnCallClient, oncall_org: str,
+ rota_team: str, rota_member: str):
"""§7.3 — the loudest finding a preview can report.
- Reachable only on a deployment with no SMTP, which is the ordinary state of
- a test instance: with the ladder set to the email channel alone, nothing
- the preview describes can reach a human, and it has to say so rather than
- drawing a ladder that looks configured.
+ When nothing the preview describes can reach a human it has to say so rather
+ than draw a ladder that looks configured. `reaches_nobody` is "no recipient's
+ email would land" (email alone decides the verdict, whatever channels the
+ ladder names), so it is produced either way a deployment can be: with no
+ SMTP, for any member; with SMTP, for a member whose address mail can never
+ reach — a reserved domain. Each branch pins its own reason, so neither can
+ pass on the other's.
"""
reach = oncall.reachability(rota_team)
assert reach.status_code == 200, reach.text
if reach.json().get("smtp_configured"):
- pytest.skip("SMTP is configured here, so the email channel can deliver")
+ member, _unused = make_user(client, oncall_org, "user")
+ oncall.users.append(member)
+ team = oncall.staffed_team([member], name=uniq("oncall_pt_nowhere"))
+ cause = "uses a domain reserved for documentation"
+ else:
+ team, member, cause = rota_team, rota_member, "no SMTP transport"
assert oncall.set_policy(
- rota_team, [ladder([user_target(rota_member)], channels=["email"])]
+ team, [ladder([user_target(member)], channels=["email"])]
).status_code == 200
- preview = oncall.escalation_preview(rota_team, priority=1)
+ preview = oncall.escalation_preview(team, priority=1)
assert preview.status_code == 200, preview.text
body = preview.json()
assert body.get("reaches_nobody") is True, body
- for rung in body.get("rungs", []):
- for recipient in rung.get("recipients", []):
- assert recipient.get("would_a_page_land") is False, recipient
+ recipients = [r for rung in body.get("rungs", []) for r in rung.get("recipients", [])]
+ assert recipients, f"the rung targets {member} and must list them: {body}"
+ for recipient in recipients:
+ assert recipient.get("would_a_page_land") is False, recipient
+ assert cause in (recipient.get("why_not") or ""), recipient
@pytest.fixture(scope="module")
def failing_team(oncall: OnCallClient, rota_member: str) -> dict:
- """A team whose only transport is a destination that cannot be reached.
+ """A team that pages its only member by email alone.
+
+ Email because it alone moves `would_a_page_land`: a webhook page is recorded
+ against the team's channel, so it can never bench a person.
Two rungs to the same person: the first fires with the record and the
second is reachable on demand via `escalate`, which is how a SECOND
@@ -309,9 +332,8 @@ def failing_team(oncall: OnCallClient, rota_member: str) -> dict:
baseline = member_verdict(oncall, team, rota_member)
assert oncall.set_policy(
team,
- [ladder([user_target(rota_member)], channels=["webhook"],
- delays=[0, 30 * 60 * 1_000_000])],
- destinations=[DEAD_DEST]).status_code == 200
+ [ladder([user_target(rota_member)], channels=["email"],
+ delays=[0, 30 * 60 * 1_000_000])]).status_code == 200
page = oncall.open_pages_for([team])[0]
return {"team": team, "page": page, "baseline": baseline}
@@ -320,25 +342,22 @@ def test_a_single_delivery_failure_does_not_bench_a_responder(
oncall: OnCallClient, failing_team: dict, rota_member: str):
"""§7.2 — the guard on the threshold. One blip is not a pattern.
- Skipped rather than asserted when the member was already unreachable before
- any page was sent: a degradation caused by two failures cannot be told
+ Skipped locally (failed in CI) rather than asserted when the member was
+ already unreachable before any page was sent: a degradation caused by two failures cannot be told
apart from one caused by there being no transport at all, and asserting
into that ambiguity would produce a test that passes for the wrong reason.
"""
if not (failing_team["baseline"] or {}).get("would_a_page_land"):
- pytest.skip("this member is unreachable before any failure; "
- "a failure-driven degradation cannot be isolated here")
+ skip_or_fail_in_ci("this member is unreachable before any failure; "
+ "a failure-driven degradation cannot be isolated here")
ledger = oncall.wait_for_deliveries(failing_team["page"]["id"])
+ skip_if_mail_lands(ledger, rota_member)
mine = [r for r in failures_in(ledger) if r and rota_member.lower() in r.lower()]
- # NOT a skip. `failing_team` routes through DEAD_DEST, a destination that
- # cannot be reached, so exactly one recorded failure is deterministic by
- # construction — nothing about a deployment changes it. Skipping here turns a
- # broken delivery ledger into a green run, which is the failure this whole
- # section exists to catch.
+ # NOT a skip: mail is configured and was not delivered, so a missing failure row is a broken ledger.
assert len(mine) == 1, (
f"expected exactly one recorded failure against {rota_member}, got {len(mine)}. "
- "The destination is unreachable by construction, so this is the delivery "
+ "Mail is configured here and was not delivered, so this is the delivery "
f"ledger not recording failures, not an environment difference: {ledger}")
verdict = member_verdict(oncall, failing_team["team"], rota_member)
@@ -356,8 +375,8 @@ def test_two_consecutive_failures_drop_a_responder_out_of_reachable(
one page.
"""
if not (failing_team["baseline"] or {}).get("would_a_page_land"):
- pytest.skip("this member is unreachable before any failure; "
- "a failure-driven degradation cannot be isolated here")
+ skip_or_fail_in_ci("this member is unreachable before any failure; "
+ "a failure-driven degradation cannot be isolated here")
resp = oncall.escalate(failing_team["page"]["id"])
assert resp.status_code == 200, resp.text
@@ -370,11 +389,12 @@ def test_two_consecutive_failures_drop_a_responder_out_of_reachable(
count=DEGRADE_AFTER)
except WaitTimeout:
ledger = oncall.deliveries(failing_team["page"]["id"]).json()
+ skip_if_mail_lands(ledger, rota_member)
mine = [r for r in failures_in(ledger) if r and rota_member.lower() in r.lower()]
- # NOT a skip, for the same reason as above: DEAD_DEST guarantees the failures.
+ # NOT a skip, for the same reason as above: nothing was delivered, so every attempt must be a failure.
assert len(mine) >= DEGRADE_AFTER, (
f"expected at least {DEGRADE_AFTER} recorded failures against {rota_member}, "
- f"got {len(mine)}. The destination is unreachable by construction, so a short "
+ f"got {len(mine)}. Mail is configured here and was not delivered, so a short "
f"count means the ledger stopped recording failures: {ledger}")
verdict = member_verdict(oncall, failing_team["team"], rota_member)
diff --git a/tests/ui-testing/pages/oncallPages/oncallPagesListPage.js b/tests/ui-testing/pages/oncallPages/oncallPagesListPage.js
index 175a9d6a74..bb15c5cab4 100644
--- a/tests/ui-testing/pages/oncallPages/oncallPagesListPage.js
+++ b/tests/ui-testing/pages/oncallPages/oncallPagesListPage.js
@@ -92,7 +92,6 @@ export class OnCallPagesListPage {
// The data-test lands on the OCheckbox LABEL; the thing that toggles is the
// inner `button[role="checkbox"]`, so that is what is clicked.
selectAll: '[data-test="o2-table-select-all"] button[role="checkbox"]',
- confirmOk: '[data-test="confirm-dialog"] [data-test="o-dialog-primary-btn"]',
whoIsOn: '[data-test="oncall-who-is-on"]',
whoIsOnPrimaryReach: '[data-test="oncall-who-is-on-primary-reach"]',
setupBanner: '[data-test="oncall-setup-banner"]',
@@ -143,7 +142,6 @@ export class OnCallPagesListPage {
getSelectAll() { return this.page.locator(this.locators.selectAll).first(); }
getBulkAck() { return this.page.locator(this.locators.bulkAck); }
getBulkResolve() { return this.page.locator(this.locators.bulkResolve); }
- getConfirmOk() { return this.page.locator(this.locators.confirmOk).first(); }
getRowByIndex(index) { return this.page.locator(this.rowByIndex(index)); }
getEscalationCell(responseId) { return this.page.locator(this.escalationCell(responseId)); }
getWhoIsOn() { return this.page.locator(this.locators.whoIsOn); }
@@ -164,6 +162,16 @@ export class OnCallPagesListPage {
return await el.getAttribute('data-state');
}
+ /** Both the checklist and the table's loading flag wait on `setupLoaded`, so either settling means the setup context is in. */
+ async waitForSetupContext() {
+ const settled = this.page.locator([
+ this.locators.setupChecklist,
+ this.locators.setupBanner,
+ `${this.locators.table} [data-test="o2-table"][data-test-loading="false"]`,
+ ].join(', ')).first();
+ await expect(settled).toBeVisible({ timeout: 30000 });
+ }
+
/** Open the checklist if it is sitting collapsed behind its banner. */
async expandSetupChecklist() {
if (await this.getSetupBanner().count()) {
@@ -511,6 +519,22 @@ export class OnCallPagesListPage {
await this.page.locator(this.rowSelect(target)).click();
}
+ /** Resolve every selected row through the bulk dialog, picking `cause` by its wire value. */
+ async bulkResolveWithCause(cause) {
+ await this.getBulkResolve().click();
+ const dialog = this.page.locator('[data-test="oncall-bulk-resolve-dialog"]');
+ await expect(dialog).toBeVisible({ timeout: 20000 });
+ const field = '[data-test="oncall-bulk-resolve-cause"]';
+ const trigger = this.page.locator(part(field, 'trigger')).first();
+ await trigger.waitFor({ state: 'visible', timeout: 20000 });
+ await trigger.click();
+ const option = this.page.locator(`${part(field, 'option')}[data-test-value="${cause}"]`).first();
+ await option.waitFor({ state: 'visible', timeout: 20000 });
+ await option.click();
+ await this.page.locator('[data-test="oncall-bulk-resolve-confirm"]').click();
+ await expect(dialog).toBeHidden({ timeout: 20000 });
+ }
+
async readBulkCount() {
const node = this.page.locator(this.locators.bulkCount);
if ((await node.count()) === 0) return null;
diff --git a/tests/ui-testing/pages/oncallPages/oncallPolicyEditorPage.js b/tests/ui-testing/pages/oncallPages/oncallPolicyEditorPage.js
index dc81ee5247..d744f36630 100644
--- a/tests/ui-testing/pages/oncallPages/oncallPolicyEditorPage.js
+++ b/tests/ui-testing/pages/oncallPages/oncallPolicyEditorPage.js
@@ -68,7 +68,6 @@ export class OnCallPolicyEditorPage {
teamChannel: '[data-test="oncall-team-channel"]',
teamChannelSelect: '[data-test="oncall-team-channel-select"]',
teamChannelSource: '[data-test="oncall-team-channel-source"]',
- teamChannelSave: '[data-test="oncall-team-channel-save"]',
teamChannelClear: '[data-test="oncall-team-channel-clear"]',
teamChannelSilent: '[data-test="oncall-team-channel-silent"]',
@@ -256,10 +255,6 @@ export class OnCallPolicyEditorPage {
await this.page.keyboard.press('Escape');
}
- async saveTeamChannel() {
- await this.page.locator(this.locators.teamChannelSave).first().click();
- }
-
// ------------------------------------------------------------------ L0
async expectL0EditorVisible() {
diff --git a/tests/ui-testing/pages/oncallPages/oncallResponseDetailPage.js b/tests/ui-testing/pages/oncallPages/oncallResponseDetailPage.js
index d6bdd61655..52c79b716a 100644
--- a/tests/ui-testing/pages/oncallPages/oncallResponseDetailPage.js
+++ b/tests/ui-testing/pages/oncallPages/oncallResponseDetailPage.js
@@ -39,6 +39,10 @@ function part(selector, suffix) {
return selector.replace(/"\]$/, `-${suffix}"]`);
}
+/** A 404 on this record's agent report is the designed "no agent ran" answer (request/oncall/mod.rs get_response_report). */
+export const isNoReportYet = (url, org, responseId) =>
+ new URL(url).pathname === `/api/${org}/oncall/responses/${responseId}/report`;
+
export class OnCallResponseDetailPage {
constructor(page) {
this.page = page;
@@ -416,6 +420,11 @@ export class OnCallResponseDetailPage {
await expect(this.page.locator(this.locators.teamLink)).toContainText(teamName, { timeout: 30000 });
}
+ async expectTeamLinkTargets(teamId) {
+ await expect(this.page.locator(this.locators.teamLink))
+ .toHaveAttribute('href', new RegExp(`/oncall/teams/${teamId}(/|\\?|$)`), { timeout: 30000 });
+ }
+
}
export default OnCallResponseDetailPage;
diff --git a/tests/ui-testing/pages/oncallPages/oncallTeamDetailPage.js b/tests/ui-testing/pages/oncallPages/oncallTeamDetailPage.js
index 07ab5a68cd..abe5d756e1 100644
--- a/tests/ui-testing/pages/oncallPages/oncallTeamDetailPage.js
+++ b/tests/ui-testing/pages/oncallPages/oncallTeamDetailPage.js
@@ -71,7 +71,7 @@ export class OnCallTeamDetailPage {
error: '[data-test="oncall-team-detail-error"]',
notAvailable: '[data-test="oncall-team-detail-not-available"]',
coverageTag: '[data-test="oncall-team-coverage"]',
- configRisks: '[data-test="oncall-team-config-risks"]',
+ configRisks: '[data-test="oncall-attention-expand"]',
coverageCard: '[data-test="oncall-team-coverage-card"]',
openSchedule: '[data-test="oncall-team-open-schedule"]',
@@ -527,6 +527,16 @@ export class OnCallTeamDetailPage {
await this.getQuickStartApply().click();
}
+ /** Accept the "replace the shift rules of …" confirm and return the preset write it issues. */
+ async confirmQuickStartReplace() {
+ const write = this.page.waitForResponse(
+ (res) => /\/oncall\/teams\/[^/]+\/schedule\/from-preset/.test(res.url()) && res.request().method() === 'POST',
+ { timeout: 30000 },
+ );
+ await this.page.locator(this.locators.confirmOk).click();
+ return await write;
+ }
+
/**
* §11.2: Save with regions unnamed and unstaffed must not be a silent no-op.
*
@@ -667,14 +677,7 @@ export class OnCallTeamDetailPage {
- /**
- * The config-risk count riding the title.
- *
- * It is a COUNT, not a list — the risk texts live on
- * `GET /teams/{id}/config-risks` and are derived on read, so a spec asserting
- * that a risk cleared reads the endpoint and asserts the tag disappeared.
- * Returns null when the tag is absent, which is what "no risks" looks like.
- */
+ /** The attention banner's finding count (config risks, with folded findings discounted), or null when no banner shows. */
async readConfigRiskCount() {
const tag = this.page.locator(this.locators.configRisks);
if ((await tag.count()) === 0) return null;
diff --git a/tests/ui-testing/playwright-tests/OnCall/oncall-coverage.spec.js b/tests/ui-testing/playwright-tests/OnCall/oncall-coverage.spec.js
index 5654f024ba..ab943f919e 100644
--- a/tests/ui-testing/playwright-tests/OnCall/oncall-coverage.spec.js
+++ b/tests/ui-testing/playwright-tests/OnCall/oncall-coverage.spec.js
@@ -478,10 +478,9 @@ test.describe('On-call covers, swaps and absences', {
const t0 = nowMicros();
const beforeA = (await holdersAt(page, a.team.id, t0)).Primary;
const beforeB = (await holdersAt(page, b.team.id, t0)).Primary;
- test.skip(
- beforeA !== beforeB,
- 'this case needs one person on call on both teams; the anchors resolved to two',
- );
+ expect(beforeA, 'the first team must have somebody on call to mark away').toBeTruthy();
+ expect(beforeB, 'the same roster half a shift past its anchor must put the same person on call on both teams')
+ .toBe(beforeA);
const created = await createAbsence(page, {
userEmail: beforeA, fromMicros: t0 - HOUR, toMicros: t0 + 2 * HOUR,
diff --git a/tests/ui-testing/playwright-tests/OnCall/oncall-dryrun.spec.js b/tests/ui-testing/playwright-tests/OnCall/oncall-dryrun.spec.js
index 571cc996df..f15c028ea7 100644
--- a/tests/ui-testing/playwright-tests/OnCall/oncall-dryrun.spec.js
+++ b/tests/ui-testing/playwright-tests/OnCall/oncall-dryrun.spec.js
@@ -7,7 +7,8 @@
* TS-13.03 a test page goes down the real path and leaves no trace
* TS-13.06 every link in every page email resolves to a real route (the W-01 guard)
*
- * ENTERPRISE-GATED (@enterprise); skips with a reason via `isOnCallAvailable()`.
+ * ENTERPRISE-GATED (@enterprise). `isOnCallAvailable()` skips on an OSS build
+ * and fails an enterprise build that has on-call switched off.
*
* WHY THESE FOUR ARE ONE FILE. They are the same claim from four directions: a
* dry run has to be TRUE and has to be FREE. A preview that lies converts a
@@ -67,6 +68,7 @@ const {
simulateRouting,
getEscalationPreview,
getCauseAnalytics,
+ requireMailSink,
waitForMail,
extractLinks,
} = require('../utils/oncall-seed-ext.js');
@@ -171,6 +173,7 @@ test.describe('On-call dry runs', {
const destination = await seedNotificationDestination(page, f.prefix.toLowerCase());
const { pages } = await firePageAndWait(page, {
alertOptions: { name: `${f.prefix}_alert`, stream, destinations: [destination] },
+ expectTeamId: f.team.id,
});
// The prediction and the firing have to agree on the team AND on the person.
@@ -269,6 +272,7 @@ test.describe('On-call dry runs', {
test('a test page reaches a real mailbox and leaves no record behind', {
tag: ['@P0'],
}, async ({ page }, testInfo) => {
+ await requireMailSink(test);
const f = await seedRoutableTeam(page, testInfo, 'test');
const recordsBefore = await listResponses(page, { teamId: f.team.id, includeResolved: true });
@@ -323,6 +327,7 @@ test.describe('On-call dry runs', {
test('every link in a page email resolves to a route that renders its own screen', {
tag: ['@P0'],
}, async ({ page }, testInfo) => {
+ await requireMailSink(test);
const f = await seedRoutableTeam(page, testInfo, 'links');
const onCall = (await whoIsOnCall(page, f.team.id)).map((slot) => slot.user_email);
@@ -341,6 +346,7 @@ test.describe('On-call dry runs', {
const pageSince = Date.now() - 2000;
const { pages } = await firePageAndWait(page, {
alertOptions: { name: `${f.prefix}_alert`, stream, destinations: [destination] },
+ expectTeamId: f.team.id,
});
const pageMail = await waitForMail({ to: onCall[0], sinceMs: pageSince, subject: /\[P\d\]/, timeout: 120000 });
expect(pageMail, 'a fired page must produce a page email').not.toBeNull();
diff --git a/tests/ui-testing/playwright-tests/OnCall/oncall-l0.spec.js b/tests/ui-testing/playwright-tests/OnCall/oncall-l0.spec.js
index 212e1d8a60..65f9b0a2f4 100644
--- a/tests/ui-testing/playwright-tests/OnCall/oncall-l0.spec.js
+++ b/tests/ui-testing/playwright-tests/OnCall/oncall-l0.spec.js
@@ -8,15 +8,11 @@
* 9.3 P4 `only` — nobody is paged, and the agent still records a verdict
* 9.4 agent unavailable — the preview must not claim `available: true`
*
- * THESE ARE EXPECTED RED UNTIL THE AGENT IS LIVE, AND THAT IS THE POINT.
- * On build `8401c636ea` `escalation-preview` reported `available: true` for all
- * three modes while nothing from the agent ever reached a page. They are
- * written against the documented contract rather than against that behaviour,
- * and tagged `@oncall-l0` so CI can hold them separately instead of letting a
- * known product gap block the suite. A green run here is news; a red one is the
- * status quo the plan already records.
+ * Where a case needs a reachable agent (a verdict on a real page), it skips
+ * unless the preview reports one; CI runs without an agent.
*
- * ENTERPRISE-GATED (@enterprise); skips with a reason via `isOnCallAvailable()`.
+ * ENTERPRISE-GATED (@enterprise). `isOnCallAvailable()` skips on an OSS build
+ * and fails an enterprise build that has on-call switched off.
*
* There is no L0 page object, and none is invented here. The lever is
* `setTeamPolicy`'s `l0` argument — absent means UNCHANGED, so it is only ever
@@ -27,14 +23,6 @@
* P1 invariant (always `parallel`) and the pages-nobody rule (P4/P5 -> `only`)
* before answering, so nothing here re-derives it from `policy.l0.mode`.
*
- * BLOCKED, and recorded rather than faked — the timing halves of 9.1-9.3 need a
- * page opened AT a chosen priority. An alert carries a top-level `priority`
- * field on the wire, but `createPagingAlert` in the seeding layer does not pass
- * one through, so a UI spec can only open a page at whatever priority the alert
- * path defaults to. What each case CAN assert is stated on the test. Closing the
- * rest needs either a priority argument on the seed helper or the API suite,
- * which owns per-priority ladder timing.
- *
* Self-cleaning, worker-scoped prefix.
*/
@@ -70,16 +58,6 @@ const workerPrefix = (testInfo) => `${PREFIX}_w${testInfo.workerIndex}`;
const gate = { checked: false, available: false, reason: '' };
-async function listOrgUsers(page) {
- const res = await page.request.get(`${baseUrl()}/api/${orgId()}/users`, {
- headers: getAuthHeaders(),
- });
- if (!res.ok()) return [];
- const body = await res.json().catch(() => ({}));
- const list = Array.isArray(body) ? body : (body?.data ?? []);
- return Array.isArray(list) ? list : [];
-}
-
/**
* A dry run of one priority's ladder, agent step included.
*
@@ -150,11 +128,7 @@ test.describe('On-call L0 / AI SRE', {
* below are the only ones those two accept.
*/
async function seedTeamWithL0(page, testInfo, { p2Mode = 'gate', rungs = [1, 2, 3, 4] } = {}) {
- // Seed our OWN member rather than borrowing listOrgUsers()[0]. The org is
- // shared across workers and every suite now sweeps the accounts it creates,
- // so the first user in the list can be another worker's fixture that is
- // deleted mid-test — which surfaces as a 400 on add-members, naming an
- // address this spec never chose.
+ // Our own member: the first org user may be another worker's fixture, deleted mid-test.
const { email: memberEmail } = await createOrgUser(page, {
email: oncallUserEmail(uniqueName(workerPrefix(testInfo)), 'l0'),
});
@@ -203,7 +177,9 @@ test.describe('On-call L0 / AI SRE', {
const { team } = await seedTeamWithL0(page, testInfo);
const preview = await escalationPreview(page, team.id, 1);
- const claimed = preview?.l0?.available === true;
+ expect(preview.l0, 'the policy configured L0, so the preview must answer whether an agent is available')
+ .toBeTruthy();
+ const claimed = preview.l0.available === true;
testLogger.info('§9.4 preview L0 block', { l0: preview?.l0 ?? null });
const stream = uniqueName(`${workerPrefix(testInfo)}_stream`);
@@ -245,30 +221,16 @@ test.describe('On-call L0 / AI SRE', {
// -------------------------------------------------------------------- 9.1
- /**
- * §9.1 — P1 is pinned `parallel`, and parallel means nothing waits.
- *
- * Holding a critical page behind a model is not a setting the product offers;
- * the server pins P1 and 400s anything else, so the preview resolving P1 to
- * anything but `parallel` is a server bug rather than a policy one. The
- * paging half is read from the record that a real firing opened: the first
- * rung must be at +0, not deferred behind a triage budget.
- *
- * Blocked half: the verdict is asserted only when the record this fired is
- * itself a P1. Without a priority on the seed's alert helper the priority is
- * the alert path's default, so it is read back rather than assumed.
- */
- test('§9.1 P1 is parallel — the first rung fires at +0 and the agent stays out of an alert-backed page', {
+ /** §9.1 — P1 is pinned `parallel` server-side, and parallel means the first rung does not wait on the agent. */
+ test('§9.1 P1 is parallel — the first rung fires at +0 without waiting on the agent', {
tag: ['@P1', '@oncall-l0'],
}, async ({ page }, testInfo) => {
test.setTimeout(900_000);
const { team } = await seedTeamWithL0(page, testInfo);
const preview = await escalationPreview(page, team.id, 1);
- expect(
- preview?.l0?.mode ?? 'parallel',
- 'P1 is pinned parallel server-side — a critical page is never held',
- ).toBe('parallel');
+ expect(preview.l0, 'the policy configured L0, so the P1 preview must carry its block').toBeTruthy();
+ expect(preview.l0.mode, 'P1 is pinned parallel server-side — a critical page is never held').toBe('parallel');
const stream = uniqueName(`${workerPrefix(testInfo)}_stream`);
await seedOnCallStream(page, stream);
@@ -281,66 +243,56 @@ test.describe('On-call L0 / AI SRE', {
},
});
const record = pages[0];
-
- const progress = await getEscalationProgress(page, record.id);
- testLogger.info('§9.1 ladder progress', { priority: record.priority, progress });
- const first = (progress?.fired ?? [])[0];
- expect(first, 'a parallel priority pages immediately, so a rung must already have fired').toBeTruthy();
- expect(
- first?.after_micros ?? 0,
- 'parallel means the page goes out at +0 while the agent runs alongside',
- ).toBe(0);
-
expect(
Number(record.priority),
'the alert names P1, so its page must open at P1 — otherwise this asserts another rung\'s behaviour',
).toBe(1);
- const detail = await getResponse(page, record.id);
- const actors = new Set((detail?.events ?? []).map((e) => e?.actor).filter(Boolean));
- testLogger.info('§9.1 actors on an alert-backed P1 page', { actors: [...actors] });
+ let progress;
+ await expect.poll(async () => {
+ progress = await getEscalationProgress(page, record.id);
+ return (progress?.fired ?? []).length;
+ }, {
+ timeout: 60000, intervals: [2000], message: 'a parallel priority pages immediately, so a rung must have fired',
+ }).toBeGreaterThan(0);
+ testLogger.info('§9.1 ladder progress', { priority: record.priority, progress });
expect(
- actors.has('o2-sre'),
- 'the agent does not participate on an alert-backed page — see the open P1 in o2-enterprise#2481',
- ).toBe(false);
+ progress.fired[0].after_micros,
+ 'parallel means the page goes out at +0 while the agent runs alongside',
+ ).toBe(0);
+ // Measured from `opened_at`, not `ladder_anchor`: a triage hold moves the anchor, so only the open time exposes it.
+ expect(
+ progress.fired[0].at - record.opened_at,
+ 'the first rung must fire inside the triage budget — a held P1 would fire only after it',
+ ).toBeLessThan(TRIAGE_BUDGET_SECONDS * 1_000_000);
});
- /**
- * §9.1b — the verdict half, which this deployment cannot answer.
- *
- * The agent only joins a page whose subject is an INCIDENT. Two things have to
- * be true and neither is here: `/config` reports `incidents_enabled: false`, so
- * no incident-backed page can open at all, and `ai_enabled: false` with no model
- * provider configured, so nothing would attach a verdict even if one did.
- *
- * `creates_incident` is NOT the obstacle — it is a field on the ordinary alert
- * (`entity/alerts.rs`), and a plain alert accepts and stores it. An earlier note
- * here blamed the composite-alert entity; that was wrong, and it made the gap
- * look structural when it is only configuration.
- *
- * To un-park: run against a deployment with `O2_INCIDENTS_ENABLED=true`,
- * `O2_AI_ENABLED=true` and a real model provider, seed the alert with
- * `createsIncident: true`, and assert the verdict as below. Measuring this on an
- * alert-backed page instead is how the documented split gets misread as a
- * product bug (o2-enterprise#2481, open P1).
- */
- test.fixme('§9.1b a parallel agent attaches its verdict on an incident-backed page. o2-enterprise#2481.', {
+ test('§9.1b a parallel agent attaches its verdict on an alert-backed P1 page', {
tag: ['@P1', '@oncall-l0'],
}, async ({ page }, testInfo) => {
+ test.setTimeout(900_000);
const { team } = await seedTeamWithL0(page, testInfo);
- const record = { id: null, team };
- let verdicts = 0;
- const deadline = Date.now() + (TRIAGE_BUDGET_SECONDS + 120) * 1000;
- while (Date.now() < deadline) {
- const detail = await getResponse(page, record.id);
- verdicts = (detail?.events ?? []).filter((e) => e?.kind === 'ai_verdict').length;
- if (verdicts > 0) break;
- await page.waitForTimeout(5000);
- }
- expect(
- verdicts,
- 'a parallel agent must still attach its verdict — running alongside is not running never',
- ).toBeGreaterThan(0);
+ const preview = await escalationPreview(page, team.id, 1);
+ test.skip(!usableL0(preview), 'needs a reachable AI SRE agent (O2_AGENT_URL); this deployment has none');
+
+ const stream = uniqueName(`${workerPrefix(testInfo)}_stream`);
+ await seedOnCallStream(page, stream);
+ const { pages } = await firePageAndWait(page, {
+ alertOptions: {
+ name: uniqueName(`${workerPrefix(testInfo)}_alert`),
+ stream,
+ teamId: team.id,
+ priority: 1,
+ },
+ });
+ await expect.poll(async () => {
+ const detail = await getResponse(page, pages[0].id);
+ return (detail?.events ?? []).filter((e) => e?.kind === 'ai_verdict').length;
+ }, {
+ timeout: (TRIAGE_BUDGET_SECONDS + 120) * 1000,
+ intervals: [5000],
+ message: 'a parallel agent must still attach its verdict — running alongside is not running never',
+ }).toBeGreaterThan(0);
});
// -------------------------------------------------------------------- 9.2
@@ -348,13 +300,13 @@ test.describe('On-call L0 / AI SRE', {
/**
* §9.2 — a gate holds the first rung for the budget, or until the verdict.
*
- * What is assertable without a P2 page: the server resolves P2 to `gate` with
- * the budget the policy set, and the screen draws the hold ONLY when an agent
- * is actually reachable — `available: false` must draw no L0 step at all,
- * because a hold rendered there reads as configured and is wrong.
+ * The server resolves P2 to `gate` with the budget the policy set, and the
+ * screen draws the hold ONLY when an agent is actually reachable —
+ * `available: false` must draw no L0 step at all, because a hold rendered
+ * there reads as configured and is wrong.
*
- * Blocked half: that the rung is genuinely deferred. It needs a page opened at
- * P2, which the seeding layer cannot do.
+ * Not asserted: that the rung is genuinely deferred. With no reachable agent a
+ * gate holds nothing, and CI runs without one.
*/
test('§9.2 P2 resolves to a gate with its budget, and the hold is drawn only when an agent exists', {
tag: ['@P1', '@oncall-l0'],
@@ -394,10 +346,10 @@ test.describe('On-call L0 / AI SRE', {
*
* Both halves of "nobody is paged" are read from the preview, which is the
* dry run of exactly that question: P4 resolves to `only`, and the ladder for
- * P4 reaches nobody. That is assertable without opening a P4 page, which is
- * why it is the one case here that is not blocked on the seed's alert helper.
+ * P4 reaches nobody.
*
- * Blocked half: that a verdict is still recorded. That needs a P4 page.
+ * Not asserted: that a verdict is still recorded. That needs a reachable
+ * agent, and CI runs without one.
*/
test('§9.3 P4 is investigate-only until the team gives it a rung', {
tag: ['@P1', '@oncall-l0'],
diff --git a/tests/ui-testing/playwright-tests/OnCall/oncall-members.spec.js b/tests/ui-testing/playwright-tests/OnCall/oncall-members.spec.js
index 4f6696c701..efb4b6ec0e 100644
--- a/tests/ui-testing/playwright-tests/OnCall/oncall-members.spec.js
+++ b/tests/ui-testing/playwright-tests/OnCall/oncall-members.spec.js
@@ -7,7 +7,8 @@
* TS-04.02 a member on no rotation is named out loud, not silently stranded
* TS-04.04 a member payload that adds nobody is refused (the W-02 guard)
*
- * ENTERPRISE-GATED (@enterprise); skips with a reason via `isOnCallAvailable()`.
+ * ENTERPRISE-GATED (@enterprise). `isOnCallAvailable()` skips on an OSS build
+ * and fails an enterprise build that has on-call switched off.
*
* WHAT IS DELIBERATELY *NOT* ASSERTED HERE:
* - The exact default ladder DELAYS. The plan quotes the book's 5/15/30/60m;
@@ -235,7 +236,7 @@ test.describe('On-call team staffing', {
* green the moment the risk is derived, and weakening it to assert the gap
* would pin the gap as correct.
*/
- test.fixme('a member on no rotation is named as a config risk rather than silently stranded — not wired: config-risks derives no such kind, verified 15 Sep on :5090. o2-enterprise#2481.', {
+ test.fixme('a member on no rotation is named as a config risk rather than silently stranded — config-risks derives no such kind. o2-enterprise#2481.', {
tag: ['@P0'],
}, async ({ page }, testInfo) => {
const prefix = uniqueName(`${workerPrefix(testInfo)}_strand`);
diff --git a/tests/ui-testing/playwright-tests/OnCall/oncall-mine.spec.js b/tests/ui-testing/playwright-tests/OnCall/oncall-mine.spec.js
index d0f16595ce..c538c76bd3 100644
--- a/tests/ui-testing/playwright-tests/OnCall/oncall-mine.spec.js
+++ b/tests/ui-testing/playwright-tests/OnCall/oncall-mine.spec.js
@@ -29,9 +29,7 @@
*
* ISOLATION IS STRUCTURAL, NOT A FILTER. The inbox takes no user parameter at
* all: the recipient is the auth header, server-side, and read markers live in
- * their own table keyed (org, user, event). The cross-user half of TS-19.03 is
- * parked — see its fixme — because every non-root identity on this deployment
- * gets 403 on everything.
+ * their own table keyed (org, user, event).
*/
const { test, expect, navigateToBase } = require('../utils/enhanced-baseFixtures.js');
@@ -45,12 +43,12 @@ const {
setTeamSchedule,
rotation,
detachPolicyFromRotations,
+ setTeamPolicy,
createOwnershipRule,
seedOnCallStream,
waitForStreamSearchable,
seedNotificationDestination,
firePageAndWait,
- triggerAlertOrLetSchedulerFire,
listResponses,
orgId,
uniqueName,
@@ -252,6 +250,7 @@ test.describe('On-call — my duty, my inbox, my history', {
await firePageAndWait(page, {
// P1: the parallel rung. A lower priority walks the ladder one slot at a time.
alertOptions: { name: `${prefix}_alert`, stream, destinations: [destination], priority: 1 },
+ expectTeamId: team.id,
});
// The inbox is keyed on the caller server-side, so the row arrives without
@@ -299,55 +298,18 @@ test.describe('On-call — my duty, my inbox, my history', {
.toBeFalsy();
});
- /**
- * The other half of TS-19.03: one person's read marker is invisible to another.
- *
- * PARKED. The isolation is structural and I read it — `list_my_deliveries`
- * takes the recipient from the auth header with the comment "the caller,
- * never a parameter", the SQL filters `Recipient.eq(user_email)`, and the
- * read join is scoped to the caller's email ("joining on the event alone
- * reads a row a colleague read") — but proving it needs a SECOND identity
- * that can call the endpoint, and on this deployment no non-root identity
- * can. A freshly created **admin** gets 403 on `/oncall/my/deliveries`,
- * `/oncall/teams` and even `/streams`; role tuples are not being written, so
- * it is the environment, not the product. Verified 16 Sep on :5090.
- */
- /**
- * TS-19.03b — read markers are per person, and that is the whole point of the
- * inbox: it answers "what have I not looked at", not "what has anyone looked at".
- *
- * PARKED on a seeding gap, not on a missing assertion. What is written below is
- * correct and runs; what is missing is a way to get one page into two inboxes.
- *
- * Measured on :5090 rather than assumed:
- * - a rotation is a SEQUENCE, so two members on one rotation yields ONE page
- * - adding members to a rotationless team leaves NOBODY on call: the mail that
- * goes out is " has nobody on call", not a page, so no delivery row is
- * written for anyone. A staffed rotation is required before any of this works
- * - with a staffed rotation the inbox is fine — one member, one delivery row
- *
- * So the gap is purely the two-recipient seed: a page has to reach two people at
- * once, and neither one rotation with two members nor an auto-staffed team does
- * that. Two staffed rotations on one parallel rung is the shape to try next.
- *
- * Asserting isolation with ONE actor proves nothing — marking a row read and
- * reading it back as the same person is true by construction, which is what the
- * original stub here did.
- */
- test.fixme('TS-19.03b one responder\'s read markers are invisible to another — not wired: no seeding shape found that puts ONE page in TWO inboxes, see the note above. o2-enterprise#2481.', {
+ test('TS-19.03b one responder\'s read markers are invisible to another', {
tag: ['@P1'],
}, async ({ page, browser }, testInfo) => {
+ test.setTimeout(420_000);
const prefix = uniqueName(`${workerPrefix(testInfo)}_iso`);
- // createOrgUsers returns ADDRESSES, not user objects.
const [mine, theirs] = await createOrgUsers(page, prefix, 2);
-
- // No schedule is written on purpose. A rotation is a SEQUENCE — only one of
- // its members is on call at a time — so two people on one rotation produces
- // one page, not two. Adding members to a rotationless team auto-staffs the
- // primary and secondary slots and builds a P1 rung that pages both in
- // parallel, which is the only default shape that reaches two inboxes at once.
const team = await createTeam(page, { name: `${prefix}_team` });
await addTeamMembers(page, team.id, [mine, theirs]);
+ // A rotation pages one person at a time; a whole-team first rung puts this one page in both inboxes.
+ await setTeamPolicy(page, team.id, {
+ rungs: [{ priority: 2, steps: [{ after_micros: 0, targets: [{ kind: 'whole_team' }] }], channels: ['email'] }],
+ });
const service = `${prefix}_svc`;
const stream = prefix.toLowerCase();
@@ -355,9 +317,11 @@ test.describe('On-call — my duty, my inbox, my history', {
await waitForStreamSearchable(page, stream, seeded.records);
await createOwnershipRule(page, { teamId: team.id, dimensions: { service } });
const destination = await seedNotificationDestination(page, stream);
- await firePageAndWait(page, {
- alertOptions: { name: `${prefix}_alert`, stream, destinations: [destination] },
+ const { pages } = await firePageAndWait(page, {
+ alertOptions: { name: `${prefix}_alert`, stream, destinations: [destination], priority: 2 },
+ expectTeamId: team.id,
});
+ const pageId = pages[0].id;
const first = await loginAs(browser, { email: mine });
const second = await loginAs(browser, { email: theirs });
@@ -365,13 +329,13 @@ test.describe('On-call — my duty, my inbox, my history', {
const rowFor = async (session) => {
let found;
await expect.poll(async () => {
- const box = await myDeliveries(session.page, { limit: 200 });
- found = (box?.deliveries ?? []).find((d) => String(d.title ?? '').includes(prefix));
+ const box = await myDeliveries(session.page, { limit: 200, asSession: true });
+ found = (box?.deliveries ?? []).find((d) => d.response_id === pageId);
return Boolean(found);
}, {
timeout: 120000,
intervals: [3000],
- message: 'both responders on the rotation must receive the page',
+ message: 'both responders on the whole-team rung must receive the page',
}).toBe(true);
return found;
};
@@ -381,27 +345,30 @@ test.describe('On-call — my duty, my inbox, my history', {
testLogger.info('TS-19.03b both inboxes hold the page', {
ours: ourRow.event_id, theirs: theirRow.event_id,
});
+ // Rows carry no recipient, so distinct ledger rows are what prove two different inboxes were read.
+ expect(theirRow.event_id, 'each responder must be reading their own inbox row').not.toBe(ourRow.event_id);
// One person reads it. Their own marker is the only thing that may move.
expect((await markDeliveriesRead(first.page, {
- eventIds: [ourRow.event_id], read: true,
+ eventIds: [ourRow.event_id], read: true, asSession: true,
})).status).toBe(200);
await expect.poll(async () => {
- const box = await myDeliveries(first.page, { limit: 200 });
+ const box = await myDeliveries(first.page, { limit: 200, asSession: true });
return (box?.deliveries ?? []).find((d) => d.event_id === ourRow.event_id)?.read;
}, {
timeout: 30000, intervals: [1000],
message: 'the reader\'s own marker must actually move',
}).toBeTruthy();
- const theirBox = await myDeliveries(second.page, { limit: 200 });
+ const theirBox = await myDeliveries(second.page, { limit: 200, asSession: true });
const stillTheirs = (theirBox.deliveries ?? []).find((d) => d.event_id === theirRow.event_id);
testLogger.info('TS-19.03b the other inbox after the read', { read: stillTheirs?.read });
+ expect(stillTheirs, 'the colleague\'s row must still be in their inbox').toBeTruthy();
expect(
- stillTheirs?.read,
+ stillTheirs.read,
'a colleague reading a page must not mark it read for me',
- ).toBeFalsy();
+ ).toBe(false);
} finally {
await first.context.close().catch(() => {});
await second.context.close().catch(() => {});
@@ -460,8 +427,10 @@ test.describe('On-call — my duty, my inbox, my history', {
const destination = await seedNotificationDestination(page, stream);
// Firing one, closed with a distinctive cause: this is the history.
+ // A 30-minute window keeps the rule firing well past the dampening wait, so the second firing is certain.
const first = await firePageAndWait(page, {
- alertOptions: { name: `${prefix}_alert`, stream, destinations: [destination] },
+ alertOptions: { name: `${prefix}_alert`, stream, destinations: [destination], periodMinutes: 30 },
+ expectTeamId: team.id,
});
const earlier = first.pages[0];
const cause = RESOLUTION_CAUSES[6]; // genuine_defect — distinctive on purpose
@@ -488,8 +457,7 @@ test.describe('On-call — my duty, my inbox, my history', {
});
await page.waitForTimeout(FLAP_DAMPENING_MS + 15_000);
- // Firing two, from the SAME alert — a second firing of one rule.
- await triggerAlertOrLetSchedulerFire(page, first.alert.id);
+ // Firing two, from the SAME alert: the scheduler re-fires it every minute once dampening ends.
await expect
.poll(async () => {
const all = await listResponses(page, { teamId: team.id, includeResolved: true });
@@ -522,6 +490,7 @@ test.describe('On-call — my duty, my inbox, my history', {
await createOwnershipRule(page, { teamId: team.id, dimensions: { service: otherService } });
const otherFire = await firePageAndWait(page, {
alertOptions: { name: `${otherPrefix}_alert`, stream: otherStream, destinations: [destination] },
+ expectTeamId: team.id,
});
const otherPriors = await getPriorCauses(page, otherFire.pages[0].id);
expect(
diff --git a/tests/ui-testing/playwright-tests/OnCall/oncall-onboarding.spec.js b/tests/ui-testing/playwright-tests/OnCall/oncall-onboarding.spec.js
index 850e6e9c98..ffb29f4f3c 100644
--- a/tests/ui-testing/playwright-tests/OnCall/oncall-onboarding.spec.js
+++ b/tests/ui-testing/playwright-tests/OnCall/oncall-onboarding.spec.js
@@ -2,10 +2,11 @@
* On-Call — Quick start actually writes a schedule (TS-06.07)
*
* Plan: docs/test_generator/test-plans/oncall-ui-test-plan.md, Tier 1 —
- * TS-06.07 applying a preset must issue a real `PUT /teams/{id}/schedule`,
- * and the stored schedule afterwards must be the preset's.
+ * TS-06.07 applying a preset must issue a real `POST /teams/{id}/schedule/from-preset`,
+ * and the primary rotation afterwards must hold the preset's shift rules.
*
- * ENTERPRISE-GATED (@enterprise); skips with a reason via `isOnCallAvailable()`.
+ * ENTERPRISE-GATED (@enterprise). `isOnCallAvailable()` skips on an OSS build
+ * and fails an enterprise build that has on-call switched off.
*
* WHY THIS IS ITS OWN FILE AND NOT AN APPEND. `oncall-quickstart.spec.js` is
* being repaired by another workstream in this same run, so it is off-limits.
@@ -16,16 +17,16 @@
* and issues NOTHING looks exactly like success: the drawer disappears, no
* error is shown, and the team's schedule is whatever it was before. So the
* assertion is not "the drawer closed" — it is that the request went out AND
- * that what came back is the preset's shape. `attemptQuickStartSave()` watches
- * the network for precisely this reason.
+ * that what came back is the preset's shape. `confirmQuickStartReplace()` waits
+ * on the write itself for precisely this reason.
*
* WHAT IS DELIBERATELY *NOT* ASSERTED HERE:
* - The preset's exact restriction minutes. Those are the catalogue's
* business decision, read from `GET /oncall/schedule-presets` at runtime;
* pinning them here would make the test a copy of the catalogue rather than
* a check on it. What is asserted is that the two staffed groups became two
- * rotations holding the people who were staffed into them, and that the
- * rotations are no longer the auto-created `source: "default"` pair.
+ * shift rules on the primary rotation holding the people staffed into them,
+ * and that the rotation is no longer the auto-created `source: "default"` one.
*
* Self-cleaning, worker-scoped prefix.
*/
@@ -42,7 +43,6 @@ const {
setTeamSchedule,
rotation,
createOwnershipRule,
- listOwnershipRules,
seedOnCallStream,
waitForStreamSearchable,
seedNotificationDestination,
@@ -51,13 +51,17 @@ const {
orgId,
uniqueName,
deleteOnCallFixturesByPrefix,
+ deleteTeamById,
} = require('../utils/oncall-seed.js');
const {
createOrgUsers,
- getCoverageGaps,
waitForMail,
+ requireMailSink,
extractLinks,
+ deleteDestination,
+ deleteAlertById,
} = require('../utils/oncall-seed-ext.js');
+const { withFreshOrg } = require('../utils/first-event-helpers.js');
const PREFIX = 'e2e_oncall_onboarding';
@@ -107,22 +111,8 @@ test.describe('On-call onboarding', {
// ------------------------------------------------------------------ TS-06.07
- /**
- * UNWIRED, and kept as a fixme with its real assertions intact.
- *
- * Reproduced on this build, 15 Sep: with the `weekday_weekend` preset chosen
- * and both required groups staffed, Apply is ENABLED (`disabled: false`),
- * clicking it issues NO request (`requestSeen: false`), and the screen says
- * nothing about why (`error: null`, `invalidFields: 0`). A Quick start that
- * validates, looks successful and writes nothing is indistinguishable from
- * one that worked, which is the entire reason this case exists.
- *
- * The body asserts the CORRECT behaviour — the apply issues
- * `PUT /teams/{id}/schedule` and the stored schedule is the preset's — so it
- * flips green the moment the handler is wired, and weakening it to assert the
- * no-op would pin the defect as correct.
- */
- test.fixme('applying a Quick start preset issues a real schedule write, not a silent no-op — not wired: Apply is enabled, issues no request and reports no error, verified 15 Sep on :5090. o2-enterprise#2481.', {
+ // Apply rewrites only the PRIMARY rotation's shift rules in place, so escalation steps that name it keep working.
+ test('applying a Quick start preset issues a real schedule write, not a silent no-op', {
tag: ['@P0'],
}, async ({ page }, testInfo) => {
const prefix = uniqueName(`${workerPrefix(testInfo)}_preset`);
@@ -130,14 +120,15 @@ test.describe('On-call onboarding', {
const team = await createTeam(page, { name: `${prefix}_team` });
await addTeamMembers(page, team.id, emails);
- // The auto-staffed starting point, so "the schedule changed" is a real
- // before/after rather than an assumption about an empty team.
const before = await getTeamSchedule(page, team.id);
testLogger.info('TS-06.07 schedule before Quick start', {
rotations: before.rotations.map((r) => ({ id: r.id, name: r.name, source: r.source })),
});
+ expect(before.rotations.length, 'a staffed team starts with an auto-created rotation for Quick start to rewrite')
+ .toBeGreaterThan(0);
expect(before.rotations.every((r) => r.source === 'default'),
'a new team starts on auto-created rotations — that is what Quick start replaces').toBe(true);
+ const [primary, ...others] = before.rotations;
await pm.oncallTeamDetailPage.goto(ORG, team.id, 'schedule');
await pm.oncallTeamDetailPage.expectAvailable();
@@ -145,167 +136,140 @@ test.describe('On-call onboarding', {
await pm.oncallTeamDetailPage.openQuickStart();
await pm.oncallTeamDetailPage.chooseQuickStartTemplate(PRESET_ID);
- // Two groups staffed with different people, so the stored rotations can be
- // told apart afterwards.
const staffing = {
weekdays: [emails[0], emails[1]],
weekend: [emails[2], emails[3]],
};
- // Only `members` is required on this preset — `name` is optional and takes
- // a sensible default — so staffing is the whole of filling it in.
for (const row of PRESET_ROWS) {
await pm.oncallTeamDetailPage.setQuickStartRegionMembers(row, staffing[row]);
}
- // The network watch is the point: a Quick start that closes the drawer
- // without issuing a PUT looks identical to one that worked.
- const attempt = await pm.oncallTeamDetailPage.attemptQuickStartSave({ settleMs: 4000 });
- testLogger.info('TS-06.07 apply attempt', attempt);
- expect(attempt.disabled, 'a fully-filled Quick start must be applicable').toBe(false);
- expect(attempt.requestSeen, 'applying a preset must issue PUT /teams/{id}/schedule — a silent no-op is the defect')
- .toBe(true);
- expect(attempt.error, 'a valid preset must not be refused').toBeNull();
+ await pm.oncallTeamDetailPage.applyQuickStart();
+ await expect(pm.oncallTeamDetailPage.getConfirmDialog(),
+ 'rewriting a stored rotation is confirmed first, naming it').toContainText(primary.name, { timeout: 20000 });
+ const write = await pm.oncallTeamDetailPage.confirmQuickStartReplace();
+ expect(write.status(), `the preset write must be accepted: ${await write.text().catch(() => '')}`).toBe(200);
- // And the stored schedule has to actually be the preset's.
await expect.poll(
- async () => (await getTeamSchedule(page, team.id)).rotations.some((r) => r.source !== 'default'),
- { timeout: 60000, intervals: [1000], message: 'the schedule never left its auto-created state' },
- ).toBe(true);
+ async () => (await getTeamSchedule(page, team.id)).rotations.find((r) => r.id === primary.id)?.source ?? 'gone',
+ { timeout: 60000, intervals: [1000], message: 'the primary rotation never left its auto-created state' },
+ ).not.toBe('default');
const after = await getTeamSchedule(page, team.id);
testLogger.info('TS-06.07 schedule after Quick start', {
rotations: after.rotations.map((r) => ({
id: r.id, name: r.name, source: r.source,
- members: r.shift_rules?.[0]?.members,
+ rules: (r.shift_rules ?? []).map((rule) => ({ name: rule.name, members: rule.members })),
})),
});
- expect(after.rotations.length, 'a two-group preset stores two rotations').toBe(PRESET_ROWS.length);
-
- // Each staffed group became a rotation holding exactly the people put in it
- // — the strongest available statement that the preset, not something else,
- // is what got written.
- const storedMembers = after.rotations.map((r) => [...(r.shift_rules?.[0]?.members ?? [])].sort());
+ const rewritten = after.rotations.find((r) => r.id === primary.id);
+ expect(rewritten, 'Quick start must rewrite the primary rotation in place, not delete or replace it').toBeTruthy();
+ expect(rewritten.name, 'the rewritten rotation keeps its name, so escalation steps still point at it')
+ .toBe(primary.name);
+ const storedMembers = (rewritten.shift_rules ?? []).map((rule) => [...(rule.members ?? [])].sort());
+ expect(storedMembers.length, 'a two-group preset writes one shift rule per group').toBe(PRESET_ROWS.length);
for (const row of PRESET_ROWS) {
expect(storedMembers, `the "${row}" group must be stored with the people staffed into it`)
.toContainEqual([...staffing[row]].sort());
}
+ // The write goes through `set_schedule`, which clears `source` on every rotation, so only the shape is compared.
+ const shape = (r) => ({ id: r.id, name: r.name, shift_rules: r.shift_rules });
+ expect(after.rotations.filter((r) => r.id !== primary.id).map(shape),
+ 'only the primary rotation is rewritten; every other rotation is left as it was').toEqual(others.map(shape));
});
- /**
- * TS-22.02 — the checklist is derived from live configuration, never
- * remembered.
- *
- * A checklist that stored "done" would keep saying done after somebody
- * deleted the destination it was about, and the first anybody would hear of
- * it is a page that announced nowhere.
- *
- * ASSERTED IN BOTH DIRECTIONS, INCLUDING ABSENCE. The obvious shape for this
- * test skips when the checklist is not drawn — and on any org that has been
- * used, it never is, so the case would be permanently skipped and nobody
- * would notice. But "not drawn" is itself derived: the component appears only
- * while at least one step is outstanding. So a fully configured org must draw
- * NOTHING, and that is just as much an assertion as a tick is. This computes
- * what the four steps should say from the live configuration and holds the
- * screen to it either way.
- */
+ // Staged in an org of its own: in the shared org other workers' setup completes all four steps, so the checklist is never drawn there.
test('TS-22.02 checklist state is derived from live configuration, including its absence', {
tag: ['@P1'],
- }, async ({ page }) => {
- // What the product computes the four steps from, read independently.
- const teams = (await listTeams(page)) ?? [];
- const rules = (await listOwnershipRules(page)) ?? [];
- const gaps = await getCoverageGaps(page);
- const hasTeam = teams.length > 0;
- const hasRouting = rules.length > 0;
- // "Would any team page somebody": fewer teams in the gap list than exist.
- const gapCount = Array.isArray(gaps) ? gaps.length : (gaps?.total ?? gaps?.teams?.length ?? 0);
- const hasStaffedRotation = hasTeam && gapCount < teams.length;
-
- await pm.oncallPagesListPage.goto(ORG);
-
- const checklist = pm.oncallPagesListPage.getSetupChecklist();
- await pm.oncallPagesListPage.expandSetupChecklist();
-
- if (!(await checklist.count())) {
- // Absence is derived too: the checklist withdraws only once nothing is
- // outstanding.
- //
- // NARROWED ON PURPOSE. I first asserted all four steps were satisfied and
- // it failed — because my re-derivation was wrong, not the product:
- // `hasRouting` in OnCallResponses.vue is
- // `rules.length > 0 || responses.some(r => !!r.team_id)`, so an org with
- // no ownership rules but routed pages legitimately has that step done.
- // Re-implementing the product's derivation only tests my copy of it. What
- // is checkable without duplicating internals is the unambiguous subset:
- // a withdrawn checklist must not be hiding an org with no teams, or one
- // where no team would page anybody.
- expect(
- hasTeam,
- 'the checklist is not drawn, so the org must at least have a team — a checklist that hides an unconfigured org is remembering, not deriving',
- ).toBe(true);
- expect(
- hasStaffedRotation,
- 'and at least one team must actually put somebody on call',
- ).toBe(true);
- testLogger.info('TS-22.02 checklist absent, and the configuration says it should be', {
- teams: teams.length, rules: rules.length, gapCount,
- });
- return;
- }
-
- await expect(checklist).toBeVisible({ timeout: 30000 });
-
- const stateOf = (key) => pm.oncallPagesListPage.readSetupStepState(key);
-
- // Each step's rendered state must follow from the configuration, not from
- // anything the component kept.
- // The snapshot above was taken BEFORE the screen rendered, and the org is
- // shared with every other worker — one of them creating a team between the
- // two reads makes the checklist right and the snapshot stale. So each step is
- // compared against a configuration re-read alongside it, bounded: the
- // assertion still requires the rendered state to follow from the
- // configuration, it just does not require the org to hold still.
- const satisfiedNow = async (key) => {
- const teamsNow = (await listTeams(page)) ?? [];
- if (key === 'team') return teamsNow.length > 0;
- if (key === 'routing') return ((await listOwnershipRules(page)) ?? []).length > 0;
- const g = await getCoverageGaps(page);
- const n = Array.isArray(g) ? g.length : (g?.total ?? g?.teams?.length ?? 0);
- return teamsNow.length > 0 && n < teamsNow.length;
+ }, async ({ page }, testInfo) => {
+ // Opening a page needs a real alert firing in the staged org.
+ test.setTimeout(480_000);
+ const list = pm.oncallPagesListPage;
+ const checklist = list.getSetupChecklist();
+ const stateOf = (key) => list.readSetupStepState(key);
+ const expectSteps = async (expected, why) => {
+ for (const [key, done] of Object.entries(expected)) {
+ // A step that is not drawn reads null, so a missing checklist cannot pass as "not done".
+ await expect.poll(async () => {
+ const state = await stateOf(key);
+ return state === null ? null : state === 'done';
+ }, {
+ timeout: 20000, intervals: [1000],
+ message: `${why}: the "${key}" step must read ${done ? 'done' : 'not done'}`,
+ }).toBe(done);
+ }
};
- for (const key of ['team', 'routing', 'rotation']) {
- expect(await stateOf(key), `the "${key}" step must render its state as data, not only as a colour`)
- .toBeTruthy();
- await expect.poll(
- async () => (await stateOf(key)) === 'done' === (await satisfiedNow(key)),
- { timeout: 20000, intervals: [1000],
- message: `the "${key}" step must read done exactly when the live configuration satisfies it` },
- ).toBe(true);
- }
+ await withFreshOrg(page, 'oncall_checklist', async (stagedOrg) => {
+ const sharedOrg = process.env.ORGNAME;
+ // Every seed helper reads its org from ORGNAME, and a worker runs one test at a time.
+ process.env.ORGNAME = stagedOrg;
+ let alertId = null;
+ try {
+ const prefix = uniqueName(`${workerPrefix(testInfo)}_chk`);
+ const reload = async () => {
+ await list.goto(stagedOrg);
+ await list.waitForSetupContext();
+ await list.expandSetupChecklist();
+ await expect(checklist, 'an unfinished setup must draw the checklist').toBeVisible({ timeout: 30000 });
+ };
- // Nothing was clicked and nothing was stored, so a reload must recompute
- // the same answer from the same configuration.
- const before = await stateOf('team');
- await pm.oncallPagesListPage.goto(ORG);
- await pm.oncallPagesListPage.expandSetupChecklist();
- await expect(checklist).toBeVisible({ timeout: 30000 });
- expect(await stateOf('team'),
- 'a reload must recompute the same state — the checklist remembers nothing of its own')
- .toBe(before);
+ await reload();
+ await expect(checklist, 'an org with nothing configured must be shown the checklist').toBeVisible({ timeout: 30000 });
+ await expectSteps({ team: false, rotation: false, routing: false, destinations: false }, 'an empty org');
+ await expect(list.getSetupCollapse(),
+ 'with no pages the checklist is the whole screen, so it offers nothing to hide it behind').toHaveCount(0);
- // "Hide for now" is a view state, and a checklist that stayed hidden would
- // hide an unfinished setup for good.
- const collapse = pm.oncallPagesListPage.getSetupCollapse();
- if (await collapse.count()) {
- await collapse.first().click();
- await pm.oncallPagesListPage.goto(ORG);
- const reappeared = (await pm.oncallPagesListPage.getSetupBanner().count())
- + (await checklist.count());
- expect(reappeared,
- 'hiding the checklist must not survive a reload while the setup is still unfinished')
- .toBeGreaterThan(0);
- }
+ const team = await createTeam(page, { name: `${prefix}_team` });
+ await reload();
+ await expectSteps({ team: true, rotation: false }, 'a team with nobody in it');
+
+ await deleteTeamById(page, team.id);
+ await expect.poll(async () => (await listTeams(page)).length, { timeout: 30000 }).toBe(0);
+ await reload();
+ await expectSteps({ team: false }, 'the only team was deleted, so a remembered tick would be stale');
+
+ const [member] = await createOrgUsers(page, prefix, 1);
+ const staffed = await createTeam(page, { name: `${prefix}_staffed` });
+ await addTeamMembers(page, staffed.id, [member]);
+ const destination = await seedNotificationDestination(page, prefix);
+ await reload();
+ await expectSteps({ team: true, rotation: true, routing: false, destinations: true },
+ 'a staffed team and a destination, but nothing routed');
+
+ const stream = `${prefix}_stream`.toLowerCase();
+ await seedOnCallStream(page, stream);
+ const fired = await firePageAndWait(page, {
+ alertOptions: { name: `${prefix}_alert`, stream, teamId: staffed.id },
+ });
+ alertId = fired.alert.id;
+
+ // A page bound straight to a team completes routing without a rule, so all four steps are done.
+ await list.goto(stagedOrg);
+ await expect(list.getRowByIndex(0), 'the fired page must be on the list').toBeVisible({ timeout: 30000 });
+ await list.waitForSetupContext();
+ await expect(checklist, 'a fully configured org must draw no checklist').toHaveCount(0);
+ await expect(list.getSetupBanner(), 'and no setup banner either').toHaveCount(0);
+
+ const deleted = await deleteDestination(page, destination, stagedOrg);
+ expect(deleted.ok(), `deleting the destination must succeed: ${deleted.status()}`).toBe(true);
+ await list.goto(stagedOrg);
+ await expect(list.getSetupBanner(),
+ 'the only destination is gone, so the setup is unfinished again and must say so over the live list')
+ .toBeVisible({ timeout: 30000 });
+ await list.expandSetupChecklist();
+ await expectSteps({ team: true, rotation: true, routing: true, destinations: false }, 'the destination was deleted');
+
+ await list.getSetupCollapse().click();
+ await expect(checklist, '"Hide for now" folds the checklist back into its banner').toHaveCount(0);
+ await list.goto(stagedOrg);
+ await expect(list.getSetupBanner(),
+ 'hiding the checklist must not survive a reload while the setup is still unfinished').toBeVisible({ timeout: 30000 });
+ } finally {
+ if (alertId) await deleteAlertById(page, alertId, stagedOrg);
+ process.env.ORGNAME = sharedOrg;
+ }
+ });
});
/**
@@ -442,12 +406,13 @@ test.describe('On-call onboarding', {
* declaration on the `h1` fixes it. Kept as a failing expectation so it goes
* green when that lands.
*/
- test.fixme('TS-16.06 the acknowledge landing page is usable at a phone viewport — not wired: the ack page h1 has no word-break, so a long record title overflows a 390px viewport by 159px and can push the Acknowledge button off screen (ack_confirm_page, request/oncall/mod.rs), verified 16 Sep on :5090. o2-enterprise#2481.', {
+ test.fixme('TS-16.06 the acknowledge landing page is usable at a phone viewport — a long title overflows it. o2-enterprise#2481.', {
tag: ['@P1'],
}, async ({ page, browser }, testInfo) => {
// A real firing has to clear ingestion, the scheduler and the ladder; the
// 3-minute default is for screens, not for this.
test.setTimeout(480_000);
+ await requireMailSink(test);
const prefix = uniqueName(`${workerPrefix(testInfo)}_ack`);
const emails = await createOrgUsers(page, prefix, 1);
const team = await createTeam(page, { name: `${prefix}_team` });
@@ -475,7 +440,7 @@ test.describe('On-call onboarding', {
const mail = await waitForMail({
to: emails[0], sinceMs: since, subject: /\[P\d\]/, timeout: 180000,
});
- test.skip(!mail, 'no mail sink is reachable on this deployment, so there is no ack link to open');
+ expect(mail, `a real firing must page ${emails[0]} by email`).not.toBeNull();
const ackLink = extractLinks(mail).find((u) => /\/oncall\/ack\?/.test(u));
expect(ackLink, 'a page email must carry an acknowledge link — that is its purpose').toBeTruthy();
diff --git a/tests/ui-testing/playwright-tests/OnCall/oncall-page-lifecycle.spec.js b/tests/ui-testing/playwright-tests/OnCall/oncall-page-lifecycle.spec.js
index 447bf98046..1887bdd769 100644
--- a/tests/ui-testing/playwright-tests/OnCall/oncall-page-lifecycle.spec.js
+++ b/tests/ui-testing/playwright-tests/OnCall/oncall-page-lifecycle.spec.js
@@ -12,7 +12,8 @@
* them, and does not silently invent one
* TS-29.07 console and network hygiene on every on-call route
*
- * ENTERPRISE-GATED (@enterprise); skips with a reason via `isOnCallAvailable()`.
+ * ENTERPRISE-GATED (@enterprise). `isOnCallAvailable()` skips on an OSS build
+ * and fails an enterprise build that has on-call switched off.
*
* FOUR SURFACES, ONE TRUTH. A page record is described in four places — the
* record itself, the human timeline, the delivery ledger and the cause
@@ -41,6 +42,7 @@
const { test, expect, navigateToBase } = require('../utils/enhanced-baseFixtures.js');
const testLogger = require('../utils/test-logger.js');
const PageManager = require('../../pages/page-manager.js');
+const { isNoReportYet } = require('../../pages/oncallPages/oncallResponseDetailPage.js');
const path = require('path');
/** The signed-in state global-setup writes; see enhanced-baseFixtures.js. */
@@ -70,6 +72,7 @@ const {
getCauseAnalytics,
eventsOfKind,
getResponseHistory,
+ requireMailSink,
waitForMail,
waitForResponseState,
RESOLUTION_CAUSES,
@@ -150,13 +153,14 @@ test.describe('On-call page lifecycle', {
test('the golden path: fire, page the right person, ack, resolve with a cause', {
tag: ['@P0'],
}, async ({ page }, testInfo) => {
+ await requireMailSink(test);
const f = await seedRoutedAlert(page, testInfo, 'golden');
// Two later rungs inside a minute, so "the ack stopped the ladder" is a
// claim about rungs that would otherwise have fired during the test.
await shortenPolicyForSpeed(page, f.team.id, 2, [0, 45, 90]);
const since = Date.now() - 2000;
- const { pages } = await firePageAndWait(page, { alertOptions: f.alertOptions });
+ const { pages } = await firePageAndWait(page, { alertOptions: f.alertOptions, expectTeamId: f.team.id });
const record = pages[0];
testLogger.info('TS-15.01 record opened', { id: record.id, team: record.team_id, priority: record.priority });
expect(record.team_id, 'the ownership rule must put the page on the team that claims the service')
@@ -247,94 +251,68 @@ test.describe('On-call page lifecycle', {
// ------------------------------------------------------------------ TS-15.03
- /**
- * UNWIRED, and kept as a fixme with its real assertions intact.
- *
- * Verified on this build, 15 Sep: an alert on a one-minute evaluation window
- * over a stream nothing is still writing to DOES notice it went healthy — the
- * record's timeline collects `recovery: the alert stopped firing`, once per
- * evaluation — and the record nevertheless stays `state: "triggered"` with no
- * `closed_at` eight minutes and seven recovery events later. The engine sees
- * the recovery and does not act on it.
- *
- * That is the worst shape this can take: the Pages list keeps showing a page
- * that needs somebody for a condition that has already cleared, so the screen
- * that exists to say "what still needs a human" fills up with things that do
- * not. The body asserts the correct behaviour — the record closes itself, no
- * acknowledgement is forged, and nothing is delivered afterwards — so it goes
- * green the moment the recovery is acted on.
- */
- test.fixme('a recovery closes the record without a human, and says nobody answered — not wired: the record collects repeated `recovery` events and stays triggered, verified 15 Sep on :5090. o2-enterprise#2481.', {
+ test('a recovery closes the record without a human, and says nobody answered', {
tag: ['@P0'],
}, async ({ page }, testInfo) => {
- test.setTimeout(15 * 60 * 1000);
- // A one-minute evaluation window over a stream nothing is still writing to:
- // the window drains on its own, which is what "the condition went healthy"
- // looks like from the engine's side.
+ test.setTimeout(10 * 60 * 1000);
+ // A one-minute window over rows ingested once drains within about two minutes: that is the recovery.
const f = await seedRoutedAlert(page, testInfo, 'recover', {
periodMinutes: 1, frequencyMinutes: 1,
});
- const { pages } = await firePageAndWait(page, { alertOptions: f.alertOptions });
+ const { pages } = await firePageAndWait(page, { alertOptions: f.alertOptions, expectTeamId: f.team.id });
const record = pages[0];
testLogger.info('TS-15.03 record opened', { id: record.id });
- // Nobody acks. The close must happen anyway.
- const closed = await waitForResponseState(page, record.id, ['resolved', 'recovered', 'closed'], {
- timeout: 10 * 60 * 1000, pollMs: 10000,
+ const closed = await waitForResponseState(page, record.id, ['resolved'], {
+ timeout: 6 * 60 * 1000, pollMs: 5000,
});
+ // The record row is written before its `state` event is appended, so the timeline can lag the state.
+ let detail;
+ await expect.poll(async () => {
+ detail = await getResponseDetail(page, record.id);
+ return eventsOfKind(detail.events, 'state').some((e) => /^resolved/.test(String(e.body)));
+ }, { timeout: 30000, intervals: [1000], message: 'the resolved state event never reached the timeline' })
+ .toBe(true);
testLogger.info('TS-15.03 record after recovery', {
- state: closed.state, closedAt: closed.closed_at, ackedBy: closed.acked_by,
+ state: closed.state, closedAt: closed.closed_at, ackedBy: closed.acked_by, cause: closed.cause,
+ events: detail.events.map((e) => [e.kind, e.actor, e.body]),
});
- expect(closed.state, 'an alert that stops firing must close its own record — nobody is coming')
- .not.toBe('triggered');
-
- // "Nobody ever answered this" has to stay countable: an un-acked recovery
- // is a different fact from one a person handled, and MTTA depends on it.
+ expect(closed.closed_at, 'a record that closed itself must carry its close time').toBeTruthy();
expect(closed.acked_by ?? null, 'a recovery must not forge an acknowledgement nobody made').toBeNull();
+ expect(eventsOfKind(detail.events, 'ack'), 'nobody answered, and the timeline must not say otherwise')
+ .toEqual([]);
+ expect(closed.cause ?? null, 'the engine knows the signal stopped, not why, so it must not invent a cause')
+ .toBeNull();
- // The timeline says WHY it closed, so a reader is not left guessing.
- const detail = await getResponseDetail(page, record.id);
- const closing = detail.events.filter((e) => /recover|resolv|clos/i.test(String(e.body ?? '')));
- testLogger.info('TS-15.03 closing events', { closing });
- expect(closing.length, 'a record that closes itself must say so on its own timeline').toBeGreaterThan(0);
- expect(closing[closing.length - 1].actor, 'an automatic close is the engine\'s act, not a person\'s')
- .not.toBe(closed.acked_by ?? 'never');
+ const recoveries = eventsOfKind(detail.events, 'recovery');
+ expect(recoveries.length, 'the timeline must say the signal recovered — once, not once per evaluation')
+ .toBe(1);
+ expect(recoveries[0].actor, 'an automatic close is the engine\'s act, not a person\'s').toBe('o2-engine');
+ const closes = eventsOfKind(detail.events, 'state').filter((e) => /^resolved/.test(String(e.body)));
+ expect(closes.length, 'the record closed once').toBe(1);
+ expect(closes[0].actor, 'the engine closed it').toBe('o2-engine');
+ expect(closes[0].body, 'and the timeline says it recovered rather than that someone fixed it')
+ .toMatch(/recovered automatically/);
+ // `o2-sre` is the AI agent, which writes verdicts on agent-enabled deployments; neither is a person.
+ const humanActors = detail.events.filter((e) => !['o2-engine', 'o2-sre'].includes(e.actor));
+ expect(humanActors, 'nobody but the engine may have acted on a page nobody answered').toEqual([]);
- // And the escalation stops with it: no delivery after the close.
const ledger = await getDeliveries(page, record.id);
const afterClose = ledger.deliveries.filter((row) => row.at > closed.closed_at);
expect(afterClose, 'nothing may be delivered for a record that has already closed').toEqual([]);
});
- /**
- * The header half of TS-15.01, UNWIRED, kept as a fixme with its real
- * assertions intact.
- *
- * Verified on this build, 15 Sep: on an open record whose
- * `oncall-response-detail-page` root, `oncall-response-stats` and
- * `oncall-response-elapsed` all render, the ENTIRE subtitle slot is absent —
- * `oncall-response-subtitle`, `oncall-response-team-link` and
- * `oncall-response-firing` each have a DOM count of 0. The component draws
- * them under `v-if="response"` inside the page header's `#subtitle` slot, and
- * that slot is not reaching the page.
- *
- * What it costs a responder: the record never says which team it belongs to,
- * never links to that team's rotation — `OnCallResponseDetail.vue` calls that
- * link "the next click a responder makes" in its own comment — and never
- * shows the firing count that distinguishes "again" from "first time". The
- * rest of the golden path works, which is why it is asserted green above and
- * this is a separate fixme rather than a failure hung on the whole case.
- */
- test.fixme('the record header names the team the page belongs to and links to it — not wired: the whole subtitle slot renders zero elements, verified 15 Sep on :5090. o2-enterprise#2481.', {
+ test('the record header names the team the page belongs to and links to it', {
tag: ['@P0'],
}, async ({ page }, testInfo) => {
const f = await seedRoutedAlert(page, testInfo, 'header');
- const { pages } = await firePageAndWait(page, { alertOptions: f.alertOptions });
+ const { pages } = await firePageAndWait(page, { alertOptions: f.alertOptions, expectTeamId: f.team.id });
await pm.oncallResponseDetailPage.goto(ORG, pages[0].id);
await pm.oncallResponseDetailPage.expectDetailVisible();
- await pm.oncallResponseDetailPage.expectTeamLinkNames(f.team.name);
await pm.oncallResponseDetailPage.expectSubtitleContains(f.team.name);
+ await pm.oncallResponseDetailPage.expectTeamLinkNames(f.team.name);
+ await pm.oncallResponseDetailPage.expectTeamLinkTargets(f.team.id);
});
// ------------------------------------------------------------------ TS-15.04
@@ -346,11 +324,14 @@ test.describe('On-call page lifecycle', {
const f = await seedRoutedAlert(page, testInfo, 'flap');
// Deliberately NOT shortened: the point is that nothing new fires, so the
// default P2 ladder's five-minute second rung stays out of the window.
- const { alert, pages } = await firePageAndWait(page, { alertOptions: f.alertOptions });
+ const { alert, pages } = await firePageAndWait(page, { alertOptions: f.alertOptions, expectTeamId: f.team.id });
const record = pages[0];
+ // The record opens before its first rung's delivery rows are written.
+ await expect.poll(
+ async () => (await getDeliveries(page, record.id))?.total ?? 0,
+ { timeout: 90000, intervals: [2000], message: 'the first rung has to have fired for "once" to mean anything' },
+ ).toBeGreaterThan(0);
const firstLedger = await getDeliveries(page, record.id);
- expect(firstLedger.total, 'the first rung has to have fired for "once" to mean anything')
- .toBeGreaterThan(0);
// The alert re-evaluates every minute over a window that still holds rows,
// so it keeps firing for the whole of this wait.
@@ -380,7 +361,7 @@ test.describe('On-call page lifecycle', {
tag: ['@P0'],
}, async ({ page }, testInfo) => {
const f = await seedRoutedAlert(page, testInfo, 'cause');
- const { pages } = await firePageAndWait(page, { alertOptions: f.alertOptions });
+ const { pages } = await firePageAndWait(page, { alertOptions: f.alertOptions, expectTeamId: f.team.id });
const record = pages[0];
await pm.oncallResponseDetailPage.goto(ORG, record.id);
@@ -421,7 +402,7 @@ test.describe('On-call page lifecycle', {
tag: ['@P0'],
}, async ({ page }, testInfo) => {
const f = await seedRoutedAlert(page, testInfo, 'hygiene');
- const { pages } = await firePageAndWait(page, { alertOptions: f.alertOptions });
+ const { pages } = await firePageAndWait(page, { alertOptions: f.alertOptions, expectTeamId: f.team.id });
const record = pages[0];
const consoleErrors = [];
@@ -442,10 +423,15 @@ test.describe('On-call page lifecycle', {
const sharedShellAsset = (message) =>
/Failed to load resource/.test(message.text())
&& (message.location()?.url ?? '').includes('/web/src/assets/');
+ const missingReport = (message) =>
+ /Failed to load resource: .* 404/.test(message.text())
+ && Boolean(message.location()?.url) && isNoReportYet(message.location().url, ORG, record.id);
page.on('console', (message) => {
const text = message.text();
- if (message.type() === 'error' && !sharedShellAsset(message)) consoleErrors.push(text);
+ if (message.type() === 'error' && !sharedShellAsset(message) && !missingReport(message)) {
+ consoleErrors.push(text);
+ }
if (/\[Vue warn\]/.test(text)) vueWarnings.push(text);
});
page.on('pageerror', (error) => consoleErrors.push(`uncaught: ${error.message}`));
@@ -454,7 +440,9 @@ test.describe('On-call page lifecycle', {
const url = response.url();
if (!/\/api\/.*\/oncall\//.test(url)) return;
apiCallsHere += 1;
- if (response.status() === 404) notFound.push(`${response.status()} ${url}`);
+ if (response.status() === 404 && !isNoReportYet(url, ORG, record.id)) {
+ notFound.push(`${response.status()} ${url}`);
+ }
// G24: the Pages list once called a count endpoint that does not exist,
// 404ing on every single load without anybody noticing.
if (/\/oncall\/responses\/count/.test(url)) countCalls.push(url);
@@ -517,7 +505,7 @@ test.describe('On-call page lifecycle', {
// 3-minute default is for screens, not for this.
test.setTimeout(600_000);
const f = await seedRoutedAlert(page, testInfo, 'deep');
- const { pages } = await firePageAndWait(page, { alertOptions: f.alertOptions });
+ const { pages } = await firePageAndWait(page, { alertOptions: f.alertOptions, expectTeamId: f.team.id });
const id = pages[0].id;
// A genuinely cold context: no history, no warmed store, no prior route —
@@ -537,7 +525,9 @@ test.describe('On-call page lifecycle', {
const failures = [];
cold.on('response', (res) => {
const url = res.url();
- if (url.includes('/oncall/') && res.status() >= 400) failures.push(`${res.status()} ${url}`);
+ if (!url.includes('/oncall/') || res.status() < 400) return;
+ if (res.status() === 404 && isNoReportYet(url, ORG, id)) return;
+ failures.push(`${res.status()} ${url}`);
});
await coldPm.oncallResponseDetailPage.goto(ORG, id);
@@ -596,7 +586,7 @@ test.describe('On-call page lifecycle', {
// 3-minute default is for screens, not for this.
test.setTimeout(600_000);
const f = await seedRoutedAlert(page, testInfo, 'ledger');
- const { pages } = await firePageAndWait(page, { alertOptions: f.alertOptions });
+ const { pages } = await firePageAndWait(page, { alertOptions: f.alertOptions, expectTeamId: f.team.id });
const id = pages[0].id;
await expect.poll(async () => (await getDeliveries(page, id))?.total, { timeout: 120000 })
@@ -654,7 +644,7 @@ test.describe('On-call page lifecycle', {
// 3-minute default is for screens, not for this.
test.setTimeout(600_000);
const f = await seedRoutedAlert(page, testInfo, 'g24');
- await firePageAndWait(page, { alertOptions: f.alertOptions });
+ await firePageAndWait(page, { alertOptions: f.alertOptions, expectTeamId: f.team.id });
// Scoped to on-call's OWN endpoints. G24 was an on-call count request that
// did not exist; widening this to every `/api/` call on the screen would
diff --git a/tests/ui-testing/playwright-tests/OnCall/oncall-pages-list.spec.js b/tests/ui-testing/playwright-tests/OnCall/oncall-pages-list.spec.js
index 45a3b6cd27..2a0251cd73 100644
--- a/tests/ui-testing/playwright-tests/OnCall/oncall-pages-list.spec.js
+++ b/tests/ui-testing/playwright-tests/OnCall/oncall-pages-list.spec.js
@@ -3,13 +3,14 @@
*
* Plan: docs/test_generator/features/oncall-test-plan.md
* §2.4 the page detail badges each responder with what the ledger says
- * §3.2 an exhausted ladder reads "Ladder finished — nobody left"
+ * §3.2 an exhausted ladder reads "Nobody answered"
* §3.3 a ladder still climbing keeps saying so — the wording is per-state
* §11.3 a group heading's count against the rows actually drawn
* §11.6 a teamless page reads "Unrouted", and the exhaust reason must not
* claim the team was deleted
*
- * ENTERPRISE-GATED (@enterprise); skips with a reason via `isOnCallAvailable()`.
+ * ENTERPRISE-GATED (@enterprise). `isOnCallAvailable()` skips on an OSS build
+ * and fails an enterprise build that has on-call switched off.
*
* SLOW BY NATURE. Every case here needs a REAL page, which means an alert that
* fires and a scheduler that turns the firing into a record — `firePageAndWait`
@@ -58,13 +59,14 @@ const {
deleteOnCallFixturesByPrefix,
} = require('../utils/oncall-seed.js');
const { createOrgUser, oncallUserEmail } = require('../utils/oncall-seed-ext.js');
+const { DEFAULT_TEAM_LOCK, acquireOrgLock } = require('../utils/oncall-org-lock.js');
const PREFIX = 'e2e_oncall_pages';
-/** Wording the plan pins, verbatim from `oncall.ladderFinished`. */
-const FINISHED_WORDING = 'Ladder finished';
-/** The blanket wording §3.2 guards against, from `oncall.escalationClimbing`. */
-const CLIMBING_WORDING = 'Escalating';
+/** Wording the plan pins, verbatim from `oncall.state_exhausted`. */
+const FINISHED_WORDING = 'Nobody answered';
+/** The climbing wording an exhausted ladder must drop, from `oncall.escalationPagingStep*`. */
+const CLIMBING_WORDING = 'Paging step';
/** `oncall.statUnrouted` — what a page nothing claimed must say. */
const UNROUTED_WORDING = 'Unrouted';
@@ -177,7 +179,16 @@ test.describe('On-call pages list', {
test('the pages list loads with its table', {
tag: ['@P0', '@smoke'],
- }, async () => {
+ }, async ({ page }, testInfo) => {
+ test.setTimeout(480_000);
+ // An org that has never paged draws the setup checklist instead of the table, and CI starts every shard fresh.
+ const { team } = await seedPageableTeam(page, testInfo);
+ const stream = uniqueName(`${workerPrefix(testInfo)}_stream`);
+ await seedOnCallStream(page, stream);
+ await firePageAndWait(page, {
+ alertOptions: { name: uniqueName(`${workerPrefix(testInfo)}_alert`), stream, teamId: team.id },
+ });
+
await pm.oncallPagesListPage.goto(ORG);
await pm.oncallPagesListPage.expectAvailable();
await pm.oncallPagesListPage.expectListVisible();
@@ -244,7 +255,7 @@ test.describe('On-call pages list', {
* situations. The honest wording is per-state, so this reads the same cell
* twice: once while the ladder is still climbing — it must not already claim
* to be finished — and once after the server reports exhaustion, where it must
- * say so in words and must NOT still read "Escalating".
+ * say so in words and must NOT still read "Paging step".
*
* BOTH READS GO THROUGH THE TEAM FILTER, and that is what makes this test
* survive a busy org rather than a freshly-wiped one. The list loads a
@@ -277,6 +288,8 @@ test.describe('On-call pages list', {
name: uniqueName(`${workerPrefix(testInfo)}_alert`),
stream,
teamId: team.id,
+ // seedPageableTeam writes a P1-only ladder; at the default P2 nothing would climb at all.
+ priority: 1,
},
});
const responseId = pages[0].id;
@@ -292,11 +305,20 @@ test.describe('On-call pages list', {
// §3.3 — while it is still climbing the cell must not claim it is over.
await openListOnThisTeam();
await expect(cell, 'the freshly opened page must be on the list').toBeVisible({ timeout: 60000 });
- const climbing = await getEscalationProgress(page, responseId);
- if (climbing && climbing.exhausted !== true) {
- await expect(cell, 'a ladder still due to fire must not read as finished')
- .not.toContainText(FINISHED_WORDING);
- }
+ // Read after the list fetched, so a ladder still climbing now was climbing when the cell was drawn.
+ // The first rung is written after the record opens, so it is polled for rather than read once.
+ let climbing;
+ await expect.poll(async () => {
+ climbing = await getEscalationProgress(page, responseId);
+ return climbing?.fired?.length ?? 0;
+ }, {
+ timeout: 60_000,
+ intervals: [2000],
+ message: 'the P1 ladder must have fired its first rung',
+ }).toBeGreaterThan(0);
+ expect(climbing?.exhausted, 'a ladder whose second rung is a minute away has not run out yet').not.toBe(true);
+ await expect(cell, 'a ladder still due to fire must not read as finished')
+ .not.toContainText(FINISHED_WORDING);
// The ladder's own cadence decides when it runs out; poll the server rather
// than the screen, so a UI that never updates fails as a UI failure.
@@ -329,52 +351,34 @@ test.describe('On-call pages list', {
* §11.6 — a page nothing routed is "Unrouted", and the reason must be true.
*
* A teamless page is still opened and must stay closable, so the row reads
- * "Unrouted" rather than going blank. The second half matters more: the
- * exhaust reason has to say nothing claimed it, not that a team was deleted —
- * the second sends somebody looking for a team that never existed.
- *
- * Requires the org to have NO default routing team. Setting one is org-wide
- * and would break every other worker's routing, so this skips rather than
- * touching it.
- *
- * PARKED ON A CONFIRMED PRODUCT DEFECT, NOT ON A TEST PROBLEM. The second
- * half genuinely fails on this build, and the assertion below is the correct
- * one — so it is left exactly as written and the test is fixme'd rather than
- * softened. Reproduced without Playwright: fire an alert with no
- * `oncall_team` into an org whose `default_team_id` is null, and
- * `GET /api/{org}/oncall/responses/{id}/escalation` answers
- * {"stopped_because": "the owning team was deleted — this page needs a new owner"}
- * on a page whose `team_id` has been null since it opened. Nothing was ever
- * deleted; no team ever claimed it. The string belongs to the enterprise
- * backend (it appears in this repo only as a fixture, in
- * web/src/components/oncall/OnCallEscalation.spec.ts:430), so the fix is a
- * paired o2-enterprise change. Everything up to that assertion — the page
- * opening teamless, the row reading "Unrouted" — passes.
+ * "Unrouted" rather than going blank, and the reason it went nowhere must say
+ * nothing claimed it rather than blame a deletion that never happened.
*/
- test.fixme('§11.6 a teamless page reads Unrouted and blames nothing on a deletion. o2-enterprise#2481.', {
+ test('§11.6 a teamless page reads Unrouted and blames nothing on a deletion', {
tag: ['@P1'],
}, async ({ page }, testInfo) => {
- test.setTimeout(480_000);
- const routing = await getRoutingConfig(page);
- test.skip(
- Boolean(routing?.default_team_id),
- 'this org nominates a catch-all team, so nothing can be unrouted — clearing it is org-wide and would break parallel workers',
- );
-
+ test.setTimeout(720_000);
const stream = uniqueName(`${workerPrefix(testInfo)}_stream`);
- // A dimension value nothing owns: the shared service names are claimed by
- // other fixtures, and a rule matching one would route this page after all.
+ // A dimension value nothing owns, so no ownership rule can claim the page.
const service = uniqueName(`${workerPrefix(testInfo)}_svc`);
await seedOnCallStream(page, stream, { services: [service] });
-
- // No `oncall_team`, so routing is left to the rules and nothing claims the page.
const destination = await seedNotificationDestination(page, workerPrefix(testInfo));
const alertName = uniqueName(`${workerPrefix(testInfo)}_alert`);
- const { pages } = await firePageAndWait(page, {
- alertOptions: { name: alertName, stream, destinations: [destination] },
- });
- const record = pages[0];
- expect(record.team_id ?? null, 'nothing must have claimed this page').toBeFalsy();
+
+ // Held until the page has opened: a default team nominated meanwhile by the routing spec would claim it.
+ const releaseLock = await acquireOrgLock(DEFAULT_TEAM_LOCK, { testInfo });
+ let record;
+ try {
+ expect((await getRoutingConfig(page))?.default_team_id ?? null,
+ 'no test may leave a default team nominated, or nothing in this org can be unrouted').toBeNull();
+ const { pages } = await firePageAndWait(page, {
+ alertOptions: { name: alertName, stream, destinations: [destination] },
+ expectTeamId: null,
+ });
+ record = pages[0];
+ } finally {
+ releaseLock();
+ }
await pm.oncallPagesListPage.goto(ORG);
await pm.oncallPagesListPage.expectListVisible();
@@ -388,18 +392,20 @@ test.describe('On-call pages list', {
'a page no rule claimed reads Unrouted — the team cell is a tag, not a link, so it carries no row-scoped data-test',
).toContainText(UNROUTED_WORDING, { timeout: 30000 });
- // The sentences the product would show for why this went nowhere.
const progress = await getEscalationProgress(page, record.id);
const detail = await getResponse(page, record.id);
+ expect(progress, 'the escalation state of an open page must be readable').toBeTruthy();
+ expect(detail, 'the page must be readable').toBeTruthy();
+ testLogger.info('§11.6 escalation state', { stoppedBecause: progress.stopped_because });
+ expect(progress.stopped_because, 'the ladder must say why it is not climbing: nothing ever routed this page')
+ .toMatch(/never routed/i);
+
const reasons = [
- progress?.stopped_because,
- detail?.response?.why_not,
- ...(detail?.events ?? [])
- .filter((e) => e?.kind === 'exhausted')
- .map((e) => e?.body),
+ progress.stopped_because,
+ detail.response?.why_not,
+ ...(detail.events ?? []).filter((e) => e?.kind === 'exhausted').map((e) => e?.body),
].filter((s) => typeof s === 'string' && s.length > 0);
testLogger.info('§11.6 exhaust reasons', { reasons });
-
for (const reason of reasons) {
expect(
reason,
@@ -437,22 +443,22 @@ test.describe('On-call pages list', {
name: uniqueName(`${workerPrefix(testInfo)}_alert`),
stream,
teamId: team.id,
+ // seedPageableTeam writes a P1-only ladder; at the default P2 nobody would be paged.
+ priority: 1,
},
});
const responseId = pages[0].id;
- // The ledger is written as the ladder runs, so an empty read a second after
- // the record opens is "not yet", not "nothing was sent". Polled with a
- // deadline rather than asserted, because an empty ledger is a legitimate
- // outcome this test then asserts the ABSENCE of a badge against.
let sends = [];
- const ledgerDeadline = Date.now() + 120_000;
- while (Date.now() < ledgerDeadline) {
+ await expect.poll(async () => {
sends = deliveryRows(await getDeliveries(page, responseId))
.filter((row) => row?.recipient === memberEmail);
- if (sends.length) break;
- await page.waitForTimeout(3000);
- }
+ return sends.length;
+ }, {
+ timeout: 120_000,
+ intervals: [3000],
+ message: 'the P1 first rung pages the rotation holder, so the ledger must record a send to them',
+ }).toBeGreaterThan(0);
testLogger.info('§2.4 ledger for the on-call responder', {
memberEmail, sends: sends.length,
});
@@ -461,14 +467,6 @@ test.describe('On-call pages list', {
await expect(pm.oncallPagesListPage.getWhoIsOn()).toBeVisible({ timeout: 30000 });
const badge = pm.oncallPagesListPage.getWhoIsOnPrimaryReach();
- if (sends.length === 0) {
- await expect(
- badge,
- 'with nothing in the ledger for this address there is nothing to badge',
- ).toHaveCount(0);
- return;
- }
-
await expect(badge, 'a responder the ladder paged must carry a reach badge')
.toBeVisible({ timeout: 30000 });
const landed = sends.some((row) => row?.delivered === true);
diff --git a/tests/ui-testing/playwright-tests/OnCall/oncall-policy.spec.js b/tests/ui-testing/playwright-tests/OnCall/oncall-policy.spec.js
index 7093181d25..ffbfed7729 100644
--- a/tests/ui-testing/playwright-tests/OnCall/oncall-policy.spec.js
+++ b/tests/ui-testing/playwright-tests/OnCall/oncall-policy.spec.js
@@ -298,17 +298,18 @@ test.describe('On-call escalation policy editor', {
* arrive from legacy data or a newer engine. Writing one needs a fixture at
* the storage layer, which is a decision, not a test.
*/
- test.fixme('TS-10.08b an unrecognised target kind renders as unrecognised rather than as a deleted rotation — not wired: POST/PUT policy 422s any kind outside rotation|user|whole_team, so the state cannot be seeded through the API, verified 16 Sep on :5090. o2-enterprise#2481.', {
+ test.fixme('TS-10.08b an unrecognised target kind renders as unrecognised rather than as a deleted rotation — the API 422s an unknown kind, so it cannot be seeded. o2-enterprise#2481.', {
tag: ['@P1'],
}, async ({ page }, testInfo) => {
const name = uniqueName(`${workerPrefix(testInfo)}_kind`);
const { team } = await stageTeam(page, name);
- await putPolicy(page, team.id, {
+ const seeded = await putPolicy(page, team.id, {
rungs: [{
priority: 2, channels: ['email'],
steps: [{ after_micros: 0, targets: [{ kind: 'martian_relay' }] }],
}],
});
+ expect(seeded.status, `the unknown-kind policy must be stored before its rendering can be read: ${seeded.text}`).toBe(200);
await pm.oncallPolicyEditorPage.gotoEscalationTab(ORG, team.id);
await pm.oncallPolicyEditorPage.openEditor();
@@ -366,6 +367,8 @@ test.describe('On-call escalation policy editor', {
// None — never set, so the policy's list applies again.
await pm.oncallPolicyEditorPage.clearTeamChannel();
+ // The channel choice is only a draft until the policy's Save, which also closes the editor.
+ await pm.oncallPolicyEditorPage.savePolicy();
await expect
.poll(async () => (await getTeamChannel(page, team.id))?.source, {
timeout: 20000,
@@ -377,6 +380,8 @@ test.describe('On-call escalation policy editor', {
expect(cleared.destinations,
'both states render an empty list — which is exactly why source has to carry the difference')
.toEqual([]);
+ await pm.oncallPolicyEditorPage.openEditor();
+ await pm.oncallPolicyEditorPage.openTab('delivery');
const policySource = await pm.oncallPolicyEditorPage.readChannelSourceText();
expect(policySource.toLowerCase(),
'and the screen must now attribute it to the policy instead')
diff --git a/tests/ui-testing/playwright-tests/OnCall/oncall-quickstart.spec.js b/tests/ui-testing/playwright-tests/OnCall/oncall-quickstart.spec.js
index 49e41da69a..6c5540fa0e 100644
--- a/tests/ui-testing/playwright-tests/OnCall/oncall-quickstart.spec.js
+++ b/tests/ui-testing/playwright-tests/OnCall/oncall-quickstart.spec.js
@@ -5,7 +5,8 @@
* "Quick start with regions unnamed and unstaffed: Save is disabled, or it
* names the missing field — never a silent no-op."
*
- * ENTERPRISE-GATED (@enterprise); skips with a reason via `isOnCallAvailable()`.
+ * ENTERPRISE-GATED (@enterprise). `isOnCallAvailable()` skips on an OSS build
+ * and fails an enterprise build that has on-call switched off.
*
* THE PLAN'S NOTE UNDER §11.2 IS STALE AGAINST MAIN. It records "still
* reproduces: Save fires no request, shows no inline error and no toast" from an
@@ -169,15 +170,10 @@ test.describe('On-call Quick start', {
const { team } = await seedEmptyScheduleTeam(page, testInfo);
await openQuickStartOn(team.id);
- const regions = pm.oncallTeamDetailPage.getPresetTab(REGIONS_PRESET);
- if (await regions.count()) {
- await pm.oncallTeamDetailPage.chooseQuickStartTemplate(REGIONS_PRESET);
- } else {
- // The catalogue is server-supplied; on a build without the regions shape
- // the landing shape still has a required people picker, which is the same
- // question asked of a different form.
- testLogger.warn('Quick start has no follow_the_sun shape — using the landing shape');
- }
+ // The catalogue is fetched after the drawer opens, so wait for the tab rather than count it.
+ await expect(pm.oncallTeamDetailPage.getPresetTab(REGIONS_PRESET),
+ 'the server catalogue ships follow_the_sun, the shape whose regions carry a required name').toBeVisible({ timeout: 20000 });
+ await pm.oncallTeamDetailPage.chooseQuickStartTemplate(REGIONS_PRESET);
const outcome = await pm.oncallTeamDetailPage.attemptQuickStartSave();
testLogger.info('§11.2 Quick start save outcome', outcome);
@@ -198,14 +194,12 @@ test.describe('On-call Quick start', {
'a Save that fires no request AND says nothing is the §11.2 defect',
).toBe(false);
- // Whatever the screen did, nothing may have been written.
- if (!outcome.requestSeen) {
- const stored = await getTeamSchedule(page, team.id);
- expect(
- (stored?.rotations ?? []).length,
- 'a refused apply must not have created rotations',
- ).toBe(0);
- }
+ expect(outcome.requestSeen, 'unnamed, unstaffed regions are invalid, so nothing may reach the server').toBe(false);
+ const stored = await getTeamSchedule(page, team.id);
+ expect(
+ (stored?.rotations ?? []).length,
+ 'a refused apply must not have created rotations',
+ ).toBe(0);
});
/**
@@ -231,11 +225,8 @@ test.describe('On-call Quick start', {
const { team, members } = await seedEmptyScheduleTeam(page, testInfo);
await openQuickStartOn(team.id);
- const regions = pm.oncallTeamDetailPage.getPresetTab(REGIONS_PRESET);
- test.skip(
- (await regions.count()) === 0,
- 'this build\'s preset catalogue has no follow_the_sun shape, which is the only one with a required name',
- );
+ await expect(pm.oncallTeamDetailPage.getPresetTab(REGIONS_PRESET),
+ 'the server catalogue ships follow_the_sun, the shape whose regions carry a required name').toBeVisible({ timeout: 20000 });
await pm.oncallTeamDetailPage.chooseQuickStartTemplate(REGIONS_PRESET);
// Row keys for a repeated group are `-`, and a group_list
diff --git a/tests/ui-testing/playwright-tests/OnCall/oncall-rbac-ui.spec.js b/tests/ui-testing/playwright-tests/OnCall/oncall-rbac-ui.spec.js
index 87047e6d8a..b45453df53 100644
--- a/tests/ui-testing/playwright-tests/OnCall/oncall-rbac-ui.spec.js
+++ b/tests/ui-testing/playwright-tests/OnCall/oncall-rbac-ui.spec.js
@@ -7,7 +7,8 @@
* through: the manual pass asserted API status codes and never asserted what
* the screen drew.
*
- * ENTERPRISE-GATED (@enterprise); skips with a reason via `isOnCallAvailable()`.
+ * ENTERPRISE-GATED (@enterprise). `isOnCallAvailable()` skips on an OSS build
+ * and fails an enterprise build that has on-call switched off.
*
* WHAT §10.5 ASKS FOR IS NOT WHAT MAIN DOES, AND THE DIVERGENCE IS DELIBERATE.
* `useOnCallPermissions` carries this in its own comment: the API has no
diff --git a/tests/ui-testing/playwright-tests/OnCall/oncall-response-verbs.spec.js b/tests/ui-testing/playwright-tests/OnCall/oncall-response-verbs.spec.js
index 128941302b..aa5de061ec 100644
--- a/tests/ui-testing/playwright-tests/OnCall/oncall-response-verbs.spec.js
+++ b/tests/ui-testing/playwright-tests/OnCall/oncall-response-verbs.spec.js
@@ -10,7 +10,8 @@
* renders as such rather than as a blank cell
* TS-02.01 a viewer can work a page end to end
*
- * ENTERPRISE-GATED (@enterprise); skips with a reason via `isOnCallAvailable()`.
+ * ENTERPRISE-GATED (@enterprise). `isOnCallAvailable()` skips on an OSS build
+ * and fails an enterprise build that has on-call switched off.
*
* SNOOZE IS NOT AN ACK, AND THAT IS THE WHOLE CASE. Naively implemented, a
* snooze either claims the record (so nobody else picks it up and nobody is
@@ -69,6 +70,7 @@ const {
promoteResponse,
eventsOfKind,
waitForMail,
+ requireMailSink,
loginAs,
RESOLUTION_CAUSES,
} = require('../utils/oncall-seed-ext.js');
@@ -220,6 +222,7 @@ test.describe('On-call response verbs', {
test('handing off to a person pages them, and is not settled until they answer', {
tag: ['@P0'],
}, async ({ page }, testInfo) => {
+ await requireMailSink(test);
const f = await seedOpenPage(page, testInfo, 'hop', { delaysSeconds: [0, 600] });
const id = f.record.id;
const onCall = (await whoIsOnCall(page, f.team.id)).map((slot) => slot.user_email);
@@ -279,6 +282,7 @@ test.describe('On-call response verbs', {
test('handing off to another team re-resolves from that team\'s own schedule', {
tag: ['@P0'],
}, async ({ page }, testInfo) => {
+ await requireMailSink(test);
const a = await seedOpenPage(page, testInfo, 'hot', { delaysSeconds: [0, 600] });
const id = a.record.id;
@@ -429,14 +433,12 @@ test.describe('On-call response verbs', {
test('a viewer can work a page end to end, with no verb hidden or disabled', {
tag: ['@P0'],
}, async ({ browser, page }, testInfo) => {
- const f = await seedOpenPage(page, testInfo, 'viewer', { delaysSeconds: [0, 600] });
- const id = f.record.id;
-
test.skip(
!(await nonAdminRolesAvailable(page)),
- 'this deployment refuses non-admin roles (OpenFGA is off in the Playwright CI lane), '
- + 'so a viewer cannot be created here; the RBAC contract itself is covered by the API suite',
+ 'non-admin roles need OpenFGA, which the OnCall Playwright lane runs without; the API suite covers the viewer contract',
);
+ const f = await seedOpenPage(page, testInfo, 'viewer', { delaysSeconds: [0, 600] });
+ const id = f.record.id;
const viewerEmail = oncallUserEmail(uniqueName(`${workerPrefix(testInfo)}_v`), 'viewer');
const viewer = await createOrgUser(page, { email: viewerEmail, role: 'viewer' });
@@ -737,40 +739,37 @@ test.describe('On-call response verbs', {
expect(record.acked_by, 'the seeded record must carry who acknowledged it').toBeTruthy();
});
- /**
- * The other half of TS-15.07: bulk resolve captures a cause.
- *
- * PARKED, because it is a real gap rather than a flaky assertion.
- *
- * Resolving ONE page forces a cause: `oncall-resolve-dialog` will not confirm
- * without one of the eight, and the whole cause-analytics surface
- * (`/oncall/analytics/causes`, the "this alert, before" card) is built on
- * them. Resolving FIFTY captures nothing — `bulkResolve` in
- * views/OnCall/OnCallResponses.vue calls
- * `oncallService.resolveResponse({ org_identifier, response_id })` with no
- * `cause` at all, and the server's `ResolveRequest.cause` is
- * `#[serde(default)] Option<..>`, so the records close with a null cause and
- * are skipped by `group_causes` forever.
- *
- * The bulk path is how a noisy morning gets cleaned up, so it is precisely
- * the path whose causes would have been most worth having. Left as a failing
- * expectation: it goes green when bulk resolve asks for a cause the way the
- * single-record dialog does.
- */
- test.fixme('TS-15.07b bulk resolve captures a cause the way resolving one page does — not wired: OnCallResponses.vue bulkResolve calls resolveResponse with no cause, and ResolveRequest.cause is Option<_> with serde(default), so every bulk-resolved record stores a null cause and is dropped from cause analytics, verified 16 Sep on :5090. o2-enterprise#2481.', {
+ /** The other half of TS-15.07: bulk resolve records the chosen cause, so a noisy morning's clean-up still feeds cause analytics. */
+ test('TS-15.07b bulk resolve captures a cause the way resolving one page does', {
tag: ['@P1'],
}, async ({ page }, testInfo) => {
+ test.setTimeout(600_000);
const f = await seedOpenPage(page, testInfo, 'bulkcause');
+ const cause = 'noisy_threshold';
+ expect(RESOLUTION_CAUSES, 'the cause this test picks must be one the product offers').toContain(cause);
await pm.oncallPagesListPage.goto(ORG);
await pm.oncallPagesListPage.filterByTeam(f.team.id);
+ await expect
+ .poll(async () => (await pm.oncallPagesListPage.readRowKeys()).length, {
+ timeout: 60000,
+ intervals: [1000],
+ message: 'the page this test fired never reached the list for its own team',
+ })
+ .toBeGreaterThan(0);
+
await pm.oncallPagesListPage.getSelectAll().click();
- await pm.oncallPagesListPage.getBulkResolve().click();
- await pm.oncallPagesListPage.getConfirmOk().click();
+ await pm.oncallPagesListPage.bulkResolveWithCause(cause);
await expect
- .poll(async () => (await getResponseRecord(page, f.record.id))?.cause, { timeout: 60000 })
- .toBeTruthy();
+ .poll(async () => (await getResponseRecord(page, f.record.id))?.state, {
+ timeout: 60000,
+ message: 'the bulk resolve never closed the selected page',
+ })
+ .toBe('resolved');
+ expect((await getResponseRecord(page, f.record.id))?.cause,
+ 'a bulk-resolved page must carry the cause picked in the dialog, or cause analytics never sees it')
+ .toBe(cause);
});
});
diff --git a/tests/ui-testing/playwright-tests/OnCall/oncall-routing.spec.js b/tests/ui-testing/playwright-tests/OnCall/oncall-routing.spec.js
index 9514061dea..15a124c528 100644
--- a/tests/ui-testing/playwright-tests/OnCall/oncall-routing.spec.js
+++ b/tests/ui-testing/playwright-tests/OnCall/oncall-routing.spec.js
@@ -5,7 +5,8 @@
* "Routing rule permanently shadowed: surfaced above the table, not only as a
* per-row pill."
*
- * ENTERPRISE-GATED (@enterprise); skips with a reason via `isOnCallAvailable()`.
+ * ENTERPRISE-GATED (@enterprise). `isOnCallAvailable()` skips on an OSS build
+ * and fails an enterprise build that has on-call switched off.
*
* SHADOWING IS THE SERVER'S VERDICT. Deciding that rule A is shadowed means
* comparing every rule against every other, including rules a team-scoped
@@ -67,6 +68,7 @@ const {
deleteTeamExpectingStatus,
} = require('../utils/oncall-seed-ext.js');
const { getAuthHeaders } = require('../utils/cloud-auth.js');
+const { DEFAULT_TEAM_LOCK, acquireOrgLock } = require('../utils/oncall-org-lock.js');
const PREFIX = 'e2e_oncall_routing';
@@ -143,7 +145,9 @@ test.describe('On-call routing rules', {
test('the routing page loads with both tabs', {
tag: ['@P0', '@smoke'],
- }, async ({ page }) => {
+ }, async ({ page }, testInfo) => {
+ // With no team the screen draws its "create a team" empty state instead of the tabs.
+ await createTeam(page, { name: uniqueName(`${workerPrefix(testInfo)}_smoke`) });
await pm.oncallRoutingPage.goto(ORG);
await pm.oncallRoutingPage.expectAvailable();
await pm.oncallRoutingPage.expectPageVisible();
@@ -384,7 +388,7 @@ test.describe('On-call routing rules', {
* endpoint validates, and weakening it to assert the 200 would pin the defect
* as correct.
*/
- test.fixme('TS-12.10 an ownership rule on a dimension outside the semantic group list is refused — W-03: POST /oncall/ownership accepts {"totally_made_up_dim":"x"} with 200, verified 15 Sep on :5090. o2-enterprise#2481.', {
+ test.fixme('TS-12.10 an ownership rule on a dimension outside the semantic group list is refused — the API stores it with 200. o2-enterprise#2481.', {
tag: ['@P0'],
}, async ({ page }, testInfo) => {
const team = await createTeam(page, { name: uniqueName(`${workerPrefix(testInfo)}_w03`) });
@@ -591,6 +595,28 @@ test.describe('On-call routing rules', {
* they are at least good citizens of the org they borrow.
*/
test.describe.serial('org default team', () => {
+ let releaseLock = null;
+ let priorDefault = null;
+ let restoreDefault = false;
+
+ test.beforeEach(async ({ page }, testInfo) => {
+ releaseLock = await acquireOrgLock(DEFAULT_TEAM_LOCK, { testInfo });
+ priorDefault = (await getRoutingConfig(page))?.default_team_id ?? null;
+ restoreDefault = true;
+ });
+
+ // Restored even when the test fails: a leaked nomination routes every other worker's unmatched page.
+ test.afterEach(async ({ page }) => {
+ try {
+ // afterEach still runs when the outer beforeEach skipped, and then there is nothing to restore.
+ if (restoreDefault) await setDefaultTeam(page, priorDefault);
+ } finally {
+ releaseLock?.();
+ releaseLock = null;
+ restoreDefault = false;
+ }
+ });
+
/**
* TS-14.01 and TS-14.02 in ONE test, deliberately.
*
@@ -701,15 +727,12 @@ test.describe('On-call routing rules', {
test('TS-14.03 a signal that only defaulted is recorded as defaulted, and a claimed one is not recorded at all', {
tag: ['@P1'],
}, async ({ page }, testInfo) => {
- // A real firing has to clear ingestion, the scheduler and the ladder; the
- // 3-minute default is for screens, not for this.
- test.setTimeout(900_000);
+ // A real firing clears ingestion, the scheduler and the ladder; added to, not set, so beforeEach's lock wait is kept.
+ test.setTimeout(testInfo.timeout - testInfo.project.timeout + 900_000);
const f = await seedPageableFixture(page, testInfo, 'owned');
await createOwnershipRule(page, { teamId: f.team.id, dimensions: { service: f.service } });
- // Nominate a default so an unmatched signal has somewhere to land,
- // remembering what was there so the org is handed back as it was found.
- const priorDefault = (await getRoutingConfig(page))?.default_team_id ?? null;
+ // Nominate a default so an unmatched signal has somewhere to land.
await setDefaultTeam(page, f.team.id);
await expect.poll(async () => (await getRoutingConfig(page))?.default_team_id,
{ timeout: 60000, intervals: [1000] }).toBe(f.team.id);
@@ -762,8 +785,6 @@ test.describe('On-call routing rules', {
pm.oncallRoutingPage.getUnroutedFilterDefault(),
'the screen must offer the defaulted-vs-unowned distinction, not just one undifferentiated queue',
).toBeVisible({ timeout: 30000 });
-
- await setDefaultTeam(page, priorDefault).catch(() => {});
});
/**
@@ -791,7 +812,6 @@ test.describe('On-call routing rules', {
}, async ({ page }, testInfo) => {
const name = uniqueName(`${workerPrefix(testInfo)}_dflt`);
const team = await createTeam(page, { name });
- const priorDefault = (await getRoutingConfig(page))?.default_team_id ?? null;
await pm.oncallRoutingPage.goto(ORG);
await pm.oncallRoutingPage.expectAvailable();
@@ -853,9 +873,6 @@ test.describe('On-call routing rules', {
// And the screen still says which team is the catch-all.
await pm.oncallRoutingPage.goto(ORG);
await pm.oncallRoutingPage.expectDefaultTeamLabelNames(name);
-
- // Leave the org as it was found: an org-wide nomination outlives this spec.
- await setDefaultTeam(page, priorDefault).catch(() => {});
});
});
});
diff --git a/tests/ui-testing/playwright-tests/OnCall/oncall-schedule.spec.js b/tests/ui-testing/playwright-tests/OnCall/oncall-schedule.spec.js
index 0c5fda4090..79001b5d04 100644
--- a/tests/ui-testing/playwright-tests/OnCall/oncall-schedule.spec.js
+++ b/tests/ui-testing/playwright-tests/OnCall/oncall-schedule.spec.js
@@ -6,9 +6,8 @@
* §8.6 a shift rule with nobody in it says so
* §11.5 timeline bars carry display names, not raw email addresses
*
- * ENTERPRISE-GATED (@enterprise). Skips with a reason via `isOnCallAvailable()`
- * rather than failing on an OSS build, where every on-call route is a 404 or a
- * 403 "Not Supported" and the product treats both as the same calm fact.
+ * ENTERPRISE-GATED (@enterprise). `isOnCallAvailable()` skips on an OSS build
+ * and fails an enterprise build that has on-call switched off.
*
* Self-cleaning, worker-scoped prefix — `afterAll` runs once per worker in
* parallel mode, so each worker sweeps only its own `_w_` names.
@@ -82,7 +81,7 @@ async function listOrgUsers(page) {
const res = await page.request.get(`${baseUrl()}/api/${orgId()}/users`, {
headers: getAuthHeaders(),
});
- if (!res.ok()) return [];
+ if (!res.ok()) throw new Error(`listing org users failed: HTTP ${res.status()} — ${(await res.text()).slice(0, 200)}`);
const body = await res.json().catch(() => ({}));
const list = Array.isArray(body) ? body : (body?.data ?? []);
return Array.isArray(list) ? list : [];
@@ -321,8 +320,7 @@ test.describe('On-call schedules and rotations', {
* address, and at a dozen members two addresses clip to the same illegible
* prefix. `nameOf()` falls back to the raw email for an account with no first
* or last name, and that fallback is CORRECT — so this test staffs the
- * rotation with an account that has a display name, and skips with a reason
- * when the org has none. Asserting "no @ on the chart" against a nameless
+ * rotation with an account it seeds with a display name. Asserting "no @ on the chart" against a nameless
* roster would be asserting the fallback is a bug.
*/
test('§11.5 timeline bars carry display names, not raw email addresses', {
@@ -336,10 +334,8 @@ test.describe('On-call schedules and rotations', {
email: oncallUserEmail(uniqueName(workerPrefix(testInfo)), 'named'),
});
const named = (await listOrgUsers(page)).find((u) => u.email === seed.email);
- test.skip(
- !named,
- 'no account in this org has a first or last name — the timeline correctly falls back to the address, so §11.5 has nothing to assert here',
- );
+ expect(named, 'the account this test just created must be on the org roster').toBeTruthy();
+ expect(displayNameOf(named), 'the seeded account carries a first and last name for the band to show').not.toBe('');
const { team, rotationId } = await seedAllDayTeam(page, testInfo, {
memberEmail: named.email,
diff --git a/tests/ui-testing/playwright-tests/OnCall/oncall-snooze-lapse.spec.js b/tests/ui-testing/playwright-tests/OnCall/oncall-snooze-lapse.spec.js
index d98d393c7d..e5a65edb8a 100644
--- a/tests/ui-testing/playwright-tests/OnCall/oncall-snooze-lapse.spec.js
+++ b/tests/ui-testing/playwright-tests/OnCall/oncall-snooze-lapse.spec.js
@@ -24,7 +24,8 @@
* while it is quiet — lives in `oncall-response-verbs.spec.js` and does run in
* CI.
*
- * ENTERPRISE-GATED (@enterprise); skips with a reason via `isOnCallAvailable()`.
+ * ENTERPRISE-GATED (@enterprise). `isOnCallAvailable()` skips on an OSS build
+ * and fails an enterprise build that has on-call switched off.
* Self-cleaning, worker-scoped prefix.
*/
diff --git a/tests/ui-testing/playwright-tests/OnCall/oncall-team-health.spec.js b/tests/ui-testing/playwright-tests/OnCall/oncall-team-health.spec.js
index 93ce431462..2c336672ba 100644
--- a/tests/ui-testing/playwright-tests/OnCall/oncall-team-health.spec.js
+++ b/tests/ui-testing/playwright-tests/OnCall/oncall-team-health.spec.js
@@ -7,7 +7,8 @@
* TS-19.01 the delivery ledger records every attempt, one row per
* (run, rung, recipient, channel)
*
- * ENTERPRISE-GATED (@enterprise); skips with a reason via `isOnCallAvailable()`.
+ * ENTERPRISE-GATED (@enterprise). `isOnCallAvailable()` skips on an OSS build
+ * and fails an enterprise build that has on-call switched off.
*
* WHY "DERIVED ON READ" IS THE WHOLE OF TS-21.02. A stored risk list would
* argue with the configuration beside it the instant somebody fixed something,
@@ -71,6 +72,8 @@ const {
getEscalationPreview,
getTeamOverview,
mailSince,
+ requireMailSink,
+ oncallUserEmail,
} = require('../utils/oncall-seed-ext.js');
const PREFIX = 'e2e_oncall_health';
@@ -337,30 +340,34 @@ test.describe('On-call team health', {
const me = process.env.ZO_ROOT_USER_EMAIL;
test.skip(
!me || !/@example\.(com|org|net)$/i.test(me),
- 'this case is about a reserved-domain address; the suite is running as a deliverable one',
+ 'needs a reserved-domain root user (CI runs as root@example.com); this run signs in with a deliverable address',
);
+ await requireMailSink(test);
+ // A deliverable member beside the reserved-domain one, so a reachability check that probed by sending would show up in the sink.
+ const { email: deliverable } = await createOrgUser(page, { email: oncallUserEmail(name, 'ok') });
const team = await createTeam(page, { name, timezone: 'UTC' });
- await addTeamMembers(page, team.id, [me]);
+ await addTeamMembers(page, team.id, [me, deliverable]);
await setTeamSchedule(page, team.id, {
timezone: 'UTC',
- rotations: [rotation({ id: 'Primary', members: [me] })],
+ // Half a shift in, so the reserved-domain member is the one holding the pager now.
+ rotations: [rotation({ id: 'Primary', members: [me, deliverable], anchorMicros: Date.now() * 1000 - DAY_MICROS / 2 })],
});
- // Half one: nothing is sent. Counted across the call rather than assumed.
- const before = await mailSince(0);
+ const since = Date.now();
const reach = await getReachability(page, team.id);
- const after = await mailSince(0);
- expect(after.length,
+ // A send is asynchronous, so give one the time it takes to reach the sink before reading it.
+ await page.waitForTimeout(5000);
+ const sent = (await mailSince(since)).filter((m) => m.to.includes(deliverable) || m.to.includes(me.toLowerCase()));
+ expect(sent.map((m) => m.subject),
'asking whether a page would land must not send one — it is a calculation, not a probe')
- .toBe(before.length);
+ .toEqual([]);
expect(reach, 'the reachability endpoint must answer').toBeTruthy();
- expect(reach.total, 'the fixture has exactly one member').toBe(1);
- expect(reach.reachable, 'and that member is on a reserved domain, so none are reachable')
- .toBe(0);
+ expect(reach.total, 'the fixture has exactly two members').toBe(2);
+ expect(reach.reachable, 'only the member on a deliverable domain is reachable').toBe(1);
expect(reach.unreachable_members, 'the unreachable member must be named, not just counted')
- .toContain(me);
+ .toEqual([me]);
const member = reach.members.find((m) => m.user_email === me);
expect(member.would_a_page_land, 'a page to a reserved domain does not land').toBe(false);
diff --git a/tests/ui-testing/playwright-tests/OnCall/oncall-teams-crud.spec.js b/tests/ui-testing/playwright-tests/OnCall/oncall-teams-crud.spec.js
index c3d0983fef..a1722280ca 100644
--- a/tests/ui-testing/playwright-tests/OnCall/oncall-teams-crud.spec.js
+++ b/tests/ui-testing/playwright-tests/OnCall/oncall-teams-crud.spec.js
@@ -4,11 +4,8 @@
* Plan: docs/test_generator/features/oncall-test-plan.md (§11.1, plus the list
* CRUD the rest of the suite depends on being able to reach).
*
- * ENTERPRISE-GATED. On-call answers 404 (feature off) or 403 "Not Supported"
- * (OSS build) on every route, and the product treats both as the same calm
- * fact. `isOnCallAvailable()` turns that into one boolean, so an OSS runner
- * SKIPS with a reason instead of failing thirty seconds into a table timeout.
- * Tagged @enterprise, never @all.
+ * ENTERPRISE-GATED (@enterprise, never @all). `isOnCallAvailable()` skips on an
+ * OSS build and fails an enterprise build that has on-call switched off.
*
* Self-cleaning: every team is named `e2e_oncall_teams_w_*` and swept
* by that prefix in afterAll. `afterAll` runs once PER WORKER in parallel mode,
@@ -43,8 +40,10 @@ const {
waitForStreamSearchable,
seedNotificationDestination,
createPagingAlert,
- triggerAlertOrLetSchedulerFire,
waitForPages,
+ listResponses,
+ getDeliveries,
+ getEscalationProgress,
orgId,
uniqueName,
deleteOnCallFixturesByPrefix,
@@ -53,6 +52,9 @@ const {
createOrgUsers,
getResponseRecord,
getResponseHistory,
+ eventsOfKind,
+ resolveResponse,
+ shortenPolicyForSpeed,
deleteTeamExpectingStatus,
} = require('../utils/oncall-seed-ext.js');
@@ -253,12 +255,7 @@ test.describe('On-call teams CRUD', { tag: ['@oncall', '@oncallTeams', '@enterpr
// -------------------------------------------------------------- P2 edge cases
- /**
- * `oncall-teams-policies-btn` NAVIGATES to /oncall/policies. It reads like a
- * filter on this screen and is not one — a spec that treated it as one would
- * pass on an empty list forever.
- */
- test.fixme('the policies button leaves the teams list for the policies screen — not wired: the button was deleted in f6ad43986f (#14502) and nothing replaced it; the oncall/policies route still registers and renders but no component links to it, so the screen is reachable only by typing the URL. o2-enterprise#2481', {
+ test.fixme('the policies button leaves the teams list for the policies screen — nothing links to /oncall/policies since #14502. o2-enterprise#2628', {
tag: ['@P2'],
}, async ({ page }) => {
await pm.oncallTeamsPage.goto(ORG);
@@ -327,6 +324,9 @@ test.describe('On-call teams CRUD', { tag: ['@oncall', '@oncallTeams', '@enterpr
await pm.oncallTeamsPage.fillTeamName(name);
await pm.oncallTeamsPage.saveDrawer();
await expect(pm.oncallTeamsPage.getFormDrawer()).toBeHidden({ timeout: 30000 });
+ await expect(page, 'create must open the new team')
+ .toHaveURL(/\/oncall\/teams\/[^/?]+/, { timeout: 30000 });
+ await pm.oncallTeamsPage.goto(ORG);
await pm.oncallTeamsPage.search(name);
const row = pm.oncallTeamsPage.getRows();
@@ -401,90 +401,50 @@ test.describe('On-call teams CRUD', { tag: ['@oncall', '@oncallTeams', '@enterpr
).toContainText(after, { timeout: 30000 });
});
- /**
- * The other half of TS-03.04: restriction windows re-evaluated in the new zone.
- *
- * PARKED — and it is a real split-brain, not a missing feature.
- *
- * The engine evaluates a rotation's restriction windows in the SCHEDULE's
- * stored `timezone`. `PUT /oncall/teams/{id}` changes the TEAM's timezone and
- * leaves the schedule's alone — verified against :5090: after moving a team
- * from UTC to Asia/Kolkata, `GET .../schedule` still answered
- * `"timezone": "UTC"` and a 09:00-17:00 weekday restriction still fired on
- * UTC hours (Wed 06:00 UTC = 11:30 IST resolved to NOBODY on call).
- *
- * Meanwhile the UI renders the whole schedule in the team's NEW zone:
- * OnCallTeamDetail.vue passes `:timezone="team?.timezone"` into every
- * schedule component, and OnCallScheduleEditor.vue only writes that zone onto
- * the schedule when somebody SAVES the schedule. So between a timezone change
- * and the next schedule save, the screen shows the roster in one zone and the
- * pager fires in another — on the one question the screen exists to answer.
- *
- * Left as a failing expectation on purpose: it goes green the day the team
- * write propagates the zone (or the UI stops claiming it has).
- */
- test.fixme('TS-03.04b changing a team\'s timezone re-evaluates its restriction windows in the new zone — not wired: PUT /oncall/teams/{id} changes the team zone but leaves the schedule at its own stored zone, so the engine keeps evaluating restrictions in the old one while the UI renders the new one, verified 16 Sep on :5090. o2-enterprise#2481.', {
+ test('TS-03.04b changing a team\'s timezone re-evaluates its restriction windows in the new zone', {
tag: ['@P1'],
}, async ({ page }, testInfo) => {
const name = uniqueName(`${workerPrefix(testInfo)}_tz`);
+ const [member] = await createOrgUsers(page, name, 1);
const team = await createTeam(page, { name, timezone: 'UTC' });
+ // Weekdays and weekends alike, so the instants below need no day-of-week arithmetic.
+ const officeHours = { days: [0, 1, 2, 3, 4, 5, 6], start_minute: 9 * 60, end_minute: 17 * 60 };
+ await addTeamMembers(page, team.id, [member]);
+ await setTeamSchedule(page, team.id, {
+ timezone: 'UTC',
+ rotations: [rotation({ id: 'Primary', members: [member], restrictions: [officeHours] })],
+ });
+
+ // Tomorrow 01:00 UTC is 10:00 in Tokyo; tomorrow 10:00 UTC is 19:00 in Tokyo.
+ const tomorrow = new Date();
+ tomorrow.setUTCDate(tomorrow.getUTCDate() + 1);
+ const at = (hourUtc) => Date.UTC(
+ tomorrow.getUTCFullYear(), tomorrow.getUTCMonth(), tomorrow.getUTCDate(), hourUtc,
+ ) * 1000;
+ const onCallAt = async (micros) => (await whoIsOnCall(page, team.id, micros)).map((slot) => slot.user_email);
+
+ expect(await onCallAt(at(1)), 'before the move, 01:00 UTC is outside UTC office hours').not.toContain(member);
+ expect(await onCallAt(at(10)), 'before the move, 10:00 UTC is inside UTC office hours').toContain(member);
await pm.oncallTeamsPage.goto(ORG);
await pm.oncallTeamsPage.openEditDrawer(team.id, { name });
- // The zone the picker can actually offer — see the note in TS-03.04. The
- // split-brain below is about the schedule not following the team, not about
- // which zone was chosen, and Kolkata is not in this runtime's ICU list.
await pm.oncallTeamsPage.selectTimezone('Asia/Tokyo');
await pm.oncallTeamsPage.saveDrawer();
+ await expect(pm.oncallTeamsPage.getFormDrawer()).toBeHidden({ timeout: 30000 });
await expect
- .poll(async () => (await getTeamSchedule(page, team.id))?.timezone, { timeout: 20000 })
+ .poll(async () => (await getTeamSchedule(page, team.id))?.timezone, {
+ timeout: 20000, message: 'the schedule must move with the team it belongs to',
+ })
.toBe('Asia/Tokyo');
+ expect(await onCallAt(at(1)), '01:00 UTC is 10:00 in Tokyo, inside the moved office hours').toContain(member);
+ expect(await onCallAt(at(10)), '10:00 UTC is 19:00 in Tokyo, outside the moved office hours')
+ .not.toContain(member);
});
- /**
- * TS-03.05 — deleting a team must never leave a live page pointing at nothing.
- *
- * Either answer is acceptable and the case accepts both: refuse the delete
- * and list what is still open, or close the records with an explicit timeline
- * entry saying the team was deleted. What is not acceptable is the third
- * outcome — the team vanishes and the open record keeps a `team_id` that
- * resolves to nothing, so the record can never be routed, escalated or
- * handed off again and nothing on screen says why.
- */
- /**
- * TS-03.05 — PARKED. A real product gap, not a test that asks too much.
- *
- * `delete_team` (o2-enterprise .../oncall/service.rs:183) refuses ONLY when the
- * team is the org's nominated default. Past that it deletes ownership rules,
- * overrides, schedules and policies — and never queries `oncall_responses` at
- * all. Its own doc comment states the intent: "Response records are kept: they
- * are history, and history has to survive a reorg."
- *
- * That reasoning holds for a RESOLVED record. It does not hold for an OPEN one:
- * the delete succeeds (200), the page stays `triggered` against a `team_id`
- * that no longer resolves, and nothing escalates it any further — the ladder,
- * the schedule and the policy it would have climbed were all just deleted. The
- * record is not history; it is a live page nobody will ever be woken for, and
- * no timeline entry says why.
- *
- * The asymmetry is the argument that this is an oversight rather than a
- * decision: taking ONE MEMBER off a team already does the check. The same file
- * carries `open_records_acknowledged_by` (service.rs:1038), which calls
- * `oncall_responses::list_open` for the team and reports the open records that
- * person can no longer close. Deleting the entire team — which strands every
- * open record, not one person's — skips it.
- *
- * The test accepts EITHER honest answer — refuse and name what is still open,
- * or allow and close the record with a timeline entry. The product does
- * neither, verified 16 Sep on :5090. The body is left intact so it goes green
- * the day one of the two is implemented.
- */
- test.fixme('TS-03.05 a team with a live page cannot be deleted into a dangling reference — not wired: delete_team (o2-enterprise oncall/service.rs:183) refuses only for the org default team and never queries oncall_responses, so DELETE /oncall/teams/{id} returns 200 while an open page stays triggered against a team, schedule and policy that no longer exist, with no timeline entry, verified 16 Sep on :5090. o2-enterprise#2481.', {
+ test('TS-03.05 deleting a team with a live page stops its ladder and leaves the page open for a new owner', {
tag: ['@P1'],
}, async ({ page }, testInfo) => {
- // A real firing has to clear ingestion, the scheduler and the ladder; the
- // 3-minute default is for screens, not for this.
test.setTimeout(600_000);
const name = uniqueName(`${workerPrefix(testInfo)}_live`);
const users = await createOrgUsers(page, name, 2);
@@ -494,6 +454,8 @@ test.describe('On-call teams CRUD', { tag: ['@oncall', '@oncallTeams', '@enterpr
timezone: 'UTC',
rotations: [rotation({ id: 'Primary', members: users })],
});
+ // Past the 60s first-delivery poll, so it is still pending at the delete but due inside the wait below.
+ await shortenPolicyForSpeed(page, team.id, 2, [0, 90]);
const stream = `${name}_stream`;
await seedOnCallStream(page, stream);
@@ -503,36 +465,48 @@ test.describe('On-call teams CRUD', { tag: ['@oncall', '@oncallTeams', '@enterpr
name: `${name}_alert`, stream, teamId: team.id, priority: 2,
destinations: [destination],
});
- await triggerAlertOrLetSchedulerFire(page, alert.id);
- const pages = await waitForPages(page, { alertId: alert.id, expected: 1 });
- expect(pages.length, 'the fixture needs a genuinely open page on this team')
- .toBeGreaterThan(0);
- const openRecord = pages[0];
+ const [openRecord] = await waitForPages(page, { alertId: alert.id, expected: 1, expectTeamId: team.id });
+ await expect.poll(async () => (await getDeliveries(page, openRecord.id))?.total ?? 0, {
+ timeout: 60000, intervals: [1000], message: 'the first rung must have paged before the team goes',
+ }).toBeGreaterThan(0);
+ const ledgerAtDelete = await getDeliveries(page, openRecord.id);
+ expect(ledgerAtDelete.deliveries.map((row) => row.rung_micros),
+ 'the second rung must still be pending when the team is deleted').toEqual(
+ ledgerAtDelete.deliveries.map(() => 0),
+ );
+
+ const deletedAtMicros = Date.now() * 1000;
const deleted = await deleteTeamExpectingStatus(page, team.id);
+ expect(deleted.status, `deleting a non-default team is allowed: ${deleted.text}`).toBe(200);
- if (deleted.status >= 400) {
- // Answer one: refused. The refusal has to name what is still open,
- // or it is a wall rather than an explanation.
- expect(String(deleted.body?.message ?? deleted.text ?? ''),
- 'a refusal must say what is still open, not just "cannot delete"')
- .toMatch(/page|open|response|firing/i);
- return;
- }
-
- // Answer two: allowed — then the record must have been closed, and the
- // timeline must say why, rather than left triggered against a gone team.
const record = await getResponseRecord(page, openRecord.id);
- expect(record, 'the record must still be readable after its team is deleted').toBeTruthy();
- expect(record.state,
- 'deleting the team must close the page it left open rather than strand it')
- .toBe('resolved');
+ testLogger.info('TS-03.05 record after the team delete', record);
+ expect(record.state, 'the page stays open: somebody still has to own it').toBe('triggered');
+ expect(record.exhausted_at, 'the ladder is stopped, and the record says when').toBeTruthy();
+ expect(record.team_id, 'the record keeps naming the team it was lost from').toBe(team.id);
- const events = await getResponseHistory(page, openRecord.id);
- const text = JSON.stringify(events);
- expect(text,
- 'and the timeline must carry an explicit entry saying the team was deleted')
- .toMatch(/team.*(delete|removed|gone)/i);
+ const exhausted = eventsOfKind(await getResponseHistory(page, openRecord.id), 'exhausted');
+ expect(exhausted.length, 'the timeline must say once why the ladder stopped').toBe(1);
+ expect(exhausted[0].actor, 'the engine stopped it, not a person').toBe('o2-engine');
+ expect(exhausted[0].body, 'and say it was the team delete').toMatch(/owning team was deleted/);
+
+ const progress = await getEscalationProgress(page, openRecord.id);
+ expect(progress.stopped_because, 'the escalation view names the same reason')
+ .toMatch(/owning team was deleted/);
+ expect(progress.next_at ?? null, 'and schedules nothing further').toBeNull();
+
+ await page.waitForTimeout(100_000);
+ const ledgerAfter = await getDeliveries(page, openRecord.id);
+ expect(ledgerAfter.deliveries.filter((row) => row.at > deletedAtMicros),
+ 'the pending rung must not fire once its team is gone').toEqual([]);
+
+ const open = await listResponses(page, { subjectType: 'alert', sourceId: alert.id });
+ expect(open.map((r) => r.id), 'the page stays on the open list until somebody closes it')
+ .toContain(openRecord.id);
+ const resolved = await resolveResponse(page, openRecord.id, { cause: 'noisy_threshold' });
+ expect(resolved.status, `a page whose team is gone can still be closed: ${resolved.text}`).toBe(200);
+ expect((await getResponseRecord(page, openRecord.id)).state).toBe('resolved');
});
});
diff --git a/tests/ui-testing/playwright-tests/RegressionSet/Alerts/oncall-ent3160-schedule-and-pages.spec.js b/tests/ui-testing/playwright-tests/RegressionSet/Alerts/oncall-ent3160-schedule-and-pages.spec.js
index 1ce659eb9d..8a15417c35 100644
--- a/tests/ui-testing/playwright-tests/RegressionSet/Alerts/oncall-ent3160-schedule-and-pages.spec.js
+++ b/tests/ui-testing/playwright-tests/RegressionSet/Alerts/oncall-ent3160-schedule-and-pages.spec.js
@@ -19,7 +19,6 @@ const {
seedOnCallStream,
seedNotificationDestination,
createPagingAlert,
- triggerAlertOrLetSchedulerFire,
waitForPages,
getRoutingConfig,
deleteTeamById,
@@ -134,10 +133,10 @@ test.describe('On-call P2 fixes (o2-enterprise#3160)', {
});
made.alerts.push(alert.id);
alerts.push({ ...spec, alert });
- await triggerAlertOrLetSchedulerFire(page, alert.id);
}
seededPages = {};
for (const { key, teamId, alert } of alerts) {
+ // The scheduler fires each new alert; a manual trigger would open a teamless page that swallows that firing.
const [record] = await waitForPages(page, { alertId: alert.id, teamId });
made.pages.push(record.id);
seededPages[key] = { name: alert.name, record };
diff --git a/tests/ui-testing/playwright-tests/utils/mail-sink.js b/tests/ui-testing/playwright-tests/utils/mail-sink.js
index de02a6c68e..06a5e326dc 100644
--- a/tests/ui-testing/playwright-tests/utils/mail-sink.js
+++ b/tests/ui-testing/playwright-tests/utils/mail-sink.js
@@ -14,8 +14,10 @@
* mailinator — public inbox read over HTTP, works against a shared env where
* you cannot run a container next to the app
*
- * Tests that need a sink call `await sink.available()` and skip cleanly when
- * false, rather than failing.
+ * Tests that need a sink check `await sink.available()` first: with no sink
+ * `list()` throws rather than report an empty mailbox. On-call specs go through
+ * `requireMailSink` (oncall-seed-ext.js), which skips locally and fails under CI,
+ * where a missing sink is a broken runner.
*/
const MODE = process.env.MAIL_SINK || 'auto';
const MAILPIT = process.env.MAILPIT_URL || 'http://localhost:8025';
@@ -76,7 +78,7 @@ async function list() {
const d = await r.json();
return (d.msgs || []).map((m) => ({ id: m.id, subject: m.subject, ts: Number(m.time) || 0 }));
}
- return [];
+ throw new Error(`cannot list mail: ${await unavailableReason()}`);
}
async function count() {
diff --git a/tests/ui-testing/playwright-tests/utils/oncall-org-lock.js b/tests/ui-testing/playwright-tests/utils/oncall-org-lock.js
new file mode 100644
index 0000000000..503a080de8
--- /dev/null
+++ b/tests/ui-testing/playwright-tests/utils/oncall-org-lock.js
@@ -0,0 +1,119 @@
+// Copyright 2026 OpenObserve Inc.
+
+// Tests that nominate the org default team and tests that need an unrouted page live in different files, so only a cross-worker lock orders them.
+
+const fs = require('node:fs');
+const os = require('node:os');
+const path = require('node:path');
+const testLogger = require('./test-logger.js');
+
+/** The lock every test touching the org's default routing team takes. */
+const DEFAULT_TEAM_LOCK = 'oncall-default-team';
+
+// Outlasts routing's whole default-team group (TS-14.01/02, TS-14.03's 15 minutes, TS-14.04), which can retake the lock back to back.
+const ORG_LOCK_WAIT_MS = 25 * 60 * 1000;
+
+// A lock dir whose pid file is empty or unreadable this long after its last write is corrupt, not being stamped.
+const UNSTAMPED_STALE_MS = 5000;
+
+const heldHere = new Set();
+
+function lockDir(name) {
+ const server = String(process.env.ZO_BASE_URL || 'local').replace(/[^a-z0-9]+/gi, '_');
+ return path.join(os.tmpdir(), `o2-e2e-lock_${server}_${name}`);
+}
+
+function readHolder(dir) {
+ let pid = 0;
+ try {
+ const stat = fs.statSync(dir);
+ try {
+ pid = Number(fs.readFileSync(path.join(dir, 'pid'), 'utf8')) || 0;
+ } catch {
+ pid = 0;
+ }
+ // Released and replaced between the two reads: the pid is not this lock's, so look again.
+ if (fs.statSync(dir).ino !== stat.ino) return null;
+ return { pid, id: `${stat.ino}-${Math.trunc(stat.mtimeMs)}`, ageMs: Date.now() - stat.mtimeMs };
+ } catch {
+ return null;
+ }
+}
+
+function isStale({ pid, ageMs }) {
+ if (!pid) return ageMs > UNSTAMPED_STALE_MS;
+ try {
+ process.kill(pid, 0);
+ return false;
+ } catch (e) {
+ return e.code !== 'EPERM';
+ }
+}
+
+function takeOver(dir, holder) {
+ // The tombstone is kept and named after the dead lock, so a second waiter's rename onto it fails instead of moving a fresh lock.
+ const tomb = `${dir}.stale-${holder.id}`;
+ try {
+ fs.renameSync(dir, tomb);
+ } catch {
+ return false;
+ }
+ for (const entry of fs.readdirSync(path.dirname(dir))) {
+ const other = path.join(path.dirname(dir), entry);
+ if (other !== tomb && other.startsWith(`${dir}.stale-`)) fs.rmSync(other, { recursive: true, force: true });
+ }
+ testLogger.info('Org lock taken over from a dead holder', { dir, deadPid: holder.pid });
+ return true;
+}
+
+/** Take the named lock, waiting for whichever worker holds it, and return its release function; pass testInfo so the wait does not eat the test's own budget. */
+async function acquireOrgLock(name, { testInfo, timeout = ORG_LOCK_WAIT_MS, pollMs = 1000 } = {}) {
+ if (heldHere.has(name)) {
+ throw new Error(`org lock "${name}" is already held by this worker (pid ${process.pid}); release it before taking it again`);
+ }
+ const dir = lockDir(name);
+ const started = Date.now();
+ testInfo?.setTimeout(testInfo.timeout + timeout);
+ // Built complete in a private dir and renamed into place, so a lock never exists without its pid.
+ const fresh = `${dir}.new-${process.pid}-${started}`;
+ fs.mkdirSync(fresh);
+ fs.writeFileSync(path.join(fresh, 'pid'), String(process.pid));
+ try {
+ for (;;) {
+ try {
+ fs.renameSync(fresh, dir);
+ break;
+ } catch (e) {
+ if (e.code !== 'ENOTEMPTY' && e.code !== 'EEXIST') throw e;
+ }
+ const holder = readHolder(dir);
+ if (!holder || (isStale(holder) && takeOver(dir, holder))) continue;
+ if (Date.now() - started > timeout) {
+ throw new Error(`waited ${timeout}ms for the org-wide lock "${name}" (${dir}); pid ${holder.pid || 'unknown'} still holds it`);
+ }
+ await new Promise((r) => setTimeout(r, pollMs));
+ }
+ } catch (e) {
+ fs.rmSync(fresh, { recursive: true, force: true });
+ throw e;
+ }
+ const waited = Date.now() - started;
+ testInfo?.setTimeout(testInfo.timeout - timeout + waited);
+ heldHere.add(name);
+ testLogger.info('Org lock taken', { name, pid: process.pid, waitedMs: waited });
+
+ let released = false;
+ return () => {
+ if (released) return;
+ released = true;
+ heldHere.delete(name);
+ if (readHolder(dir)?.pid !== process.pid) return;
+ // Renamed away before deleting, so the lock path is never an empty dir another worker's rename could replace.
+ const gone = `${dir}.released-${process.pid}-${Date.now()}`;
+ fs.renameSync(dir, gone);
+ fs.rmSync(gone, { recursive: true, force: true });
+ testLogger.info('Org lock released', { name, pid: process.pid });
+ };
+}
+
+module.exports = { DEFAULT_TEAM_LOCK, acquireOrgLock };
diff --git a/tests/ui-testing/playwright-tests/utils/oncall-seed-ext.js b/tests/ui-testing/playwright-tests/utils/oncall-seed-ext.js
index 365dfcdbd3..8aab97aecb 100644
--- a/tests/ui-testing/playwright-tests/utils/oncall-seed-ext.js
+++ b/tests/ui-testing/playwright-tests/utils/oncall-seed-ext.js
@@ -77,8 +77,9 @@ function url(path) {
* rule that should be rejected), so this deliberately does not throw on a
* non-2xx the way `oncall-seed.js`'s `must()` does.
*/
-async function call(page, method, path, data) {
- const opts = { headers: getAuthHeaders() };
+async function call(page, method, path, data, { asSession = false } = {}) {
+ // A secondary sign-in is identified by its cookie alone; root's header would make the identity a precedence question.
+ const opts = { headers: asSession ? {} : getAuthHeaders() };
if (data !== undefined) opts.data = data;
const res = await page.request[method](url(path), opts);
const text = await res.text().catch(() => '');
@@ -99,8 +100,8 @@ async function call(page, method, path, data) {
* there", so every emptiness assertion built on one passed the moment the API
* stopped answering. An error is not an empty result.
*/
-async function read(page, path) {
- const { ok, status, body, text } = await call(page, 'get', path);
+async function read(page, path, { asSession = false } = {}) {
+ const { ok, status, body, text } = await call(page, 'get', path, undefined, { asSession });
if (!ok) {
throw new Error(`GET ${path} failed: ${status} ${String(text ?? '').slice(0, 200)}`);
}
@@ -166,6 +167,9 @@ async function nonAdminRolesAvailable(page) {
email, password: SEED_USER_PASSWORD, first_name: 'RBAC', last_name: 'Probe',
role: 'viewer', organization: orgId(),
});
+ if (status >= 400 && !/already exist/i.test(text || '') && !/open-?fga/i.test(text || '')) {
+ throw new Error(`probing for non-admin roles failed: HTTP ${status} — ${String(text).slice(0, 200)}`);
+ }
_nonAdminRoles = status === 200 || /already exist/i.test(text || '');
if (_nonAdminRoles) await deleteOrgUser(page, email);
testLogger.info('Non-admin roles probe', { available: _nonAdminRoles, status });
@@ -344,7 +348,8 @@ async function waitForResponseState(page, responseId, states, { timeout = 180_00
async function simulateRouting(page, { dimensions, oncallTeam = undefined }) {
const payload = { dimensions };
if (oncallTeam !== undefined) payload.oncall_team = oncallTeam;
- const { body } = await call(page, 'post', '/oncall/routing/preview', payload);
+ const { ok, status, body, text } = await call(page, 'post', '/oncall/routing/preview', payload);
+ if (!ok) throw new Error(`routing preview failed: HTTP ${status} — ${String(text).slice(0, 200)}`);
return body;
}
@@ -465,18 +470,34 @@ async function shortenPolicyForSpeed(page, teamId, priority, delaysSeconds, { ta
const MAILPIT = process.env.MAILPIT_URL || 'http://localhost:8025';
-/**
- * One message, fully fetched.
- *
- * `mail-sink.js` indexes messages (id, subject, timestamp) and can fetch the
- * NEWEST one, but a page fan-out puts several in the sink at once and the one a
- * case cares about is rarely the newest. This adds the per-id read on the
- * Mailpit backend only — the sink that exists on this environment — rather than
- * re-implementing the module's backend selection.
- */
+/** Why the on-call mail helpers cannot read mail here, or null when Mailpit answers. */
+async function mailSinkProblem() {
+ if ((await sink.backend()) === 'mailpit') return null;
+ return `the on-call mail helpers read Mailpit only and it is not reachable on ${MAILPIT}; `
+ + await sink.unavailableReason();
+}
+
+/** Throw rather than let a missing sink read as "no mail was sent". */
+async function assertMailSink() {
+ const problem = await mailSinkProblem();
+ if (problem) throw new Error(`cannot read mail: ${problem}`);
+}
+
+/** Call first in a test that reads mail: skips locally when no sink is up, throws in CI where that is a broken runner. */
+async function requireMailSink(test) {
+ const problem = await mailSinkProblem();
+ if (!problem) return;
+ if (process.env.CI) throw new Error(`this test reads mail and the CI runner has no mail sink: ${problem}`);
+ test.skip(true, problem);
+}
+
+/** One message by id, or null if it was deleted after it was listed; a fan-out's wanted message is rarely the newest. */
async function fetchMessage(id) {
const res = await fetch(`${MAILPIT}/api/v1/message/${id}`);
- if (!res.ok) return null;
+ if (res.status === 404) return null;
+ if (!res.ok) {
+ throw new Error(`reading Mailpit message ${id} failed: HTTP ${res.status} — ${(await res.text()).slice(0, 200)}`);
+ }
const m = await res.json();
return {
id,
@@ -489,14 +510,14 @@ async function fetchMessage(id) {
}
/**
- * Wait for a message matching `to` and/or `subject`, newest first.
+ * Wait for a message matching `to` and/or `subject`, newest first; null only when none arrived in time.
*
* `sinceMs` scopes to messages that arrived after a caller-recorded instant, so
* a shared sink full of other workers' fixtures cannot satisfy the wait with
* somebody else's page.
*/
async function waitForMail({ to = null, subject = null, sinceMs = 0, timeout = 120_000, pollMs = 2000 } = {}) {
- if (!(await sink.available())) return null;
+ await assertMailSink();
const wantTo = to ? String(to).toLowerCase() : null;
const deadline = Date.now() + timeout;
@@ -507,8 +528,7 @@ async function waitForMail({ to = null, subject = null, sinceMs = 0, timeout = 1
: index;
for (const row of [...candidates].sort((a, b) => b.ts - a.ts)) {
const full = await fetchMessage(row.id);
- if (!full) continue;
- if (wantTo && !full.to.includes(wantTo)) continue;
+ if (!full || (wantTo && !full.to.includes(wantTo))) continue;
return full;
}
await new Promise((r) => setTimeout(r, pollMs));
@@ -518,7 +538,7 @@ async function waitForMail({ to = null, subject = null, sinceMs = 0, timeout = 1
/** Every message since an instant, fully fetched — for fan-out assertions. */
async function mailSince(sinceMs) {
- if (!(await sink.available())) return [];
+ await assertMailSink();
const index = (await sink.list()).filter((m) => m.ts >= sinceMs);
const out = [];
for (const row of index) {
@@ -583,19 +603,19 @@ async function putPolicy(page, teamId, { rungs, destinations = undefined, l0 = u
return await call(page, 'put', `/oncall/teams/${encodeURIComponent(teamId)}/policy`, payload);
}
-/** The caller's own inbox: `{total, unread, deliveries}`. Keyed on the auth header, never a param. */
-async function myDeliveries(page, { unreadOnly = undefined, limit = undefined } = {}) {
+/** The caller's own inbox: `{total, unread, deliveries}`. Keyed on the caller's identity, never a param. */
+async function myDeliveries(page, { unreadOnly = undefined, limit = undefined, asSession = false } = {}) {
const params = new URLSearchParams();
if (unreadOnly !== undefined) params.set('unread_only', String(unreadOnly));
if (limit !== undefined) params.set('limit', String(limit));
const query = params.toString() ? `?${params}` : '';
- return await read(page, `/oncall/my/deliveries${query}`);
+ return await read(page, `/oncall/my/deliveries${query}`, { asSession });
}
/** Mark inbox rows read or unread again. `{all: true, read: true}` marks everything. */
-async function markDeliveriesRead(page, { eventIds = [], read: isRead = true, all = false } = {}) {
+async function markDeliveriesRead(page, { eventIds = [], read: isRead = true, all = false, asSession = false } = {}) {
return await call(page, 'post', '/oncall/my/deliveries/read',
- { event_ids: eventIds, read: isRead, all });
+ { event_ids: eventIds, read: isRead, all }, { asSession });
}
/** The team overview: the summary card's own endpoint, built from four other reads. */
@@ -613,7 +633,10 @@ async function getTeamOverview(page, teamId) {
*/
async function getTeamAttention(page, teamId) {
const body = await read(page, `/oncall/teams/${encodeURIComponent(teamId)}/config-risks`);
- return body?.risks ?? [];
+ if (!Array.isArray(body?.risks)) {
+ throw new Error(`config-risks answered no risks list: ${JSON.stringify(body).slice(0, 200)}`);
+ }
+ return body.risks;
}
/** Ownership-rule hit statistics, the numbers the rules table renders per row. */
@@ -647,6 +670,26 @@ async function createOwnershipRuleExpectingStatus(page, body) {
return await call(page, 'post', '/oncall/ownership', body);
}
+/** The org is explicit because a case staged in its own org must not fall back to the shared one. */
+async function deleteDestination(page, name, org = orgId()) {
+ return await page.request.delete(
+ `${baseUrl()}/api/${org}/alerts/destinations/${encodeURIComponent(name)}`,
+ { headers: getAuthHeaders() },
+ );
+}
+
+/** Teardown only: an already-gone alert is not a failure. */
+async function deleteAlertById(page, alertId, org = orgId()) {
+ try {
+ await page.request.delete(
+ `${baseUrl()}/api/v2/${org}/alerts/${encodeURIComponent(alertId)}`,
+ { headers: getAuthHeaders() },
+ );
+ } catch (e) {
+ testLogger.debug('Alert cleanup failed (non-fatal)', { alertId, error: e.message });
+ }
+}
+
module.exports = {
SEED_MAIL_DOMAIN,
RESOLUTION_CAUSES,
@@ -686,6 +729,7 @@ module.exports = {
deleteAbsence,
deleteOwnershipRule,
shortenPolicyForSpeed,
+ requireMailSink,
waitForMail,
mailSince,
extractLinks,
@@ -700,4 +744,6 @@ module.exports = {
getOwnershipStats,
createOwnershipRuleExpectingStatus,
deleteTeamExpectingStatus,
+ deleteDestination,
+ deleteAlertById,
};
diff --git a/tests/ui-testing/playwright-tests/utils/oncall-seed.js b/tests/ui-testing/playwright-tests/utils/oncall-seed.js
index a32cbbaf87..e86928a81e 100644
--- a/tests/ui-testing/playwright-tests/utils/oncall-seed.js
+++ b/tests/ui-testing/playwright-tests/utils/oncall-seed.js
@@ -5,11 +5,7 @@
*
* Plan: docs/test_generator/features/oncall-test-plan.md
*
- * On-Call is ENTERPRISE-GATED. Every route answers 404 (feature off) or 403
- * "Not Supported" (OSS build) when it is unavailable, and the product treats
- * both as the same calm fact rather than an error — so must we.
- * `isOnCallAvailable()` turns that into one boolean a spec can skip on, instead
- * of each spec failing thirty seconds into a timeout on an OSS runner.
+ * On-Call is ENTERPRISE-GATED: `isOnCallAvailable()` skips on an OSS build and throws when an enterprise build has it off.
*
* Three environment facts this module encodes, each of which fails QUIETLY if
* you get it wrong:
@@ -53,8 +49,8 @@ const SEED_SERVICES = ['checkout', 'search', 'payments'];
const FIRST_RUNG_MICROS = 0;
const SECOND_RUNG_MICROS = 60 * MICROS;
-/** How long a fired alert may take to become a page. Generous: the scheduler owns the cadence. */
-const PAGE_WAIT_MS = 180_000;
+/** A new alert is due at once and the scheduler ticks every 10s, so its page opens in about 10-20s. */
+const PAGE_WAIT_MS = 90_000;
function baseUrl() {
const url = process.env.ZO_BASE_URL || 'http://localhost:5080';
@@ -152,30 +148,42 @@ async function must(res, what, payload) {
);
}
+/** GET one resource: null only when it does not exist (404), a throw on any other failure. */
+async function getOrNull(page, url, what) {
+ const res = await page.request.get(url, { headers: getAuthHeaders() });
+ if (res.status() === 404) return null;
+ const text = await res.text().catch(() => '');
+ if (!res.ok()) throw new Error(`${what} failed: HTTP ${res.status()} — ${text.slice(0, 200)}`);
+ return text.trim() ? JSON.parse(text) : null;
+}
+
// ------------------------------------------------------------- capability probe
+/** The server's `build_type` from the unauthenticated `/config`, or a throw when it cannot be read. */
+async function readBuildType(page) {
+ const res = await page.request.get(`${baseUrl()}/config`);
+ const text = await res.text().catch(() => '');
+ if (!res.ok()) {
+ throw new Error(`on-call probe: could not read GET /config (HTTP ${res.status()}) — ${text.slice(0, 200)}`);
+ }
+ let buildType;
+ try {
+ buildType = JSON.parse(text)?.build_type;
+ } catch {
+ buildType = undefined;
+ }
+ if (!buildType) throw new Error(`on-call probe: GET /config carries no build_type — ${text.slice(0, 200)}`);
+ return buildType;
+}
+
/**
- * Is on-call reachable in this deployment?
- *
- * Asks the teams list rather than `/config`: the list IS the capability probe
- * the UI itself uses (§G.8.1), so a spec that skips on this skips on exactly
- * what the screens react to. `/config`'s `oncall_enabled` is checked as well
- * because it distinguishes "off" from "OSS build" in the reason string, which
- * is the difference between a misconfigured runner and a correct skip.
+ * Is on-call reachable here? Skips only on an OSS build; anything else that is not a working on-call throws.
*
* @returns {Promise<{available: boolean, reason: string}>}
*/
async function isOnCallAvailable(page) {
const org = orgId();
- if (!org) return { available: false, reason: 'no org identifier — ORGNAME / cloud-config is unset' };
-
- let flag = null;
- try {
- const cfg = await page.request.get(`${baseUrl()}/config`, { headers: getAuthHeaders() });
- if (cfg.ok()) flag = (await cfg.json().catch(() => ({})))?.oncall_enabled ?? null;
- } catch {
- // A failed /config read is not itself a verdict; the teams probe below decides.
- }
+ if (!org) throw new Error('on-call probe: no org identifier — ORGNAME / cloud-config is unset');
const res = await page.request.get(`${baseUrl()}/api/${org}/oncall/teams`, {
headers: getAuthHeaders(),
@@ -183,22 +191,24 @@ async function isOnCallAvailable(page) {
if (res.ok()) return { available: true, reason: '' };
const body = await res.text().catch(() => '');
- if (isUnavailableStatus(res.status(), body)) {
+ // Every spec skips on `available: false`, so only a genuinely absent feature may return it.
+ if (!isUnavailableStatus(res.status(), body)) {
+ throw new Error(
+ `on-call probe failed: HTTP ${res.status()} on /oncall/teams — ${body.slice(0, 200)}`,
+ );
+ }
+
+ const buildType = await readBuildType(page);
+ if (buildType === 'opensource') {
return {
available: false,
- reason: flag === false
- ? 'on-call is disabled on this deployment (config.oncall_enabled=false)'
- : `on-call is not available here (HTTP ${res.status()}) — OSS build or O2_ONCALL_ENABLED unset`,
+ reason: `on-call is enterprise-only and this is an OSS build (HTTP ${res.status()} on /oncall/teams)`,
};
}
- // A THROW, not another `available: false`. Every spec calls this in
- // `beforeEach` and skips on a false, so returning one here turns any backend
- // fault into a silent pass: a 500 on this one route would skip all 106 tests
- // and report both shards green. Only 404 and 403 "Not Supported" say the
- // feature is absent; a 500, 502 or 401 says the deployment is broken, and a
- // broken deployment must fail the suite rather than excuse it.
throw new Error(
- `on-call probe failed: HTTP ${res.status()} on /oncall/teams — ${body.slice(0, 200)}`,
+ `on-call is switched off on this ${buildType} build: /oncall/teams answered HTTP ${res.status()}. ` +
+ 'Start the server with O2_ONCALL_ENABLED=true; skipping here would report the on-call suite green ' +
+ 'without running it.',
);
}
@@ -239,12 +249,10 @@ async function listTeams(page) {
}
async function getTeam(page, teamId) {
- const res = await page.request.get(
- `${baseUrl()}/api/${orgId()}/oncall/teams/${encodeURIComponent(teamId)}`,
- { headers: getAuthHeaders() },
+ return await getOrNull(
+ page, `${baseUrl()}/api/${orgId()}/oncall/teams/${encodeURIComponent(teamId)}`,
+ `Reading team ${teamId}`,
);
- if (!res.ok()) return null;
- return await res.json().catch(() => null);
}
/**
@@ -352,12 +360,10 @@ async function setTeamSchedule(page, teamId, { timezone = 'UTC', rotations }) {
}
async function getTeamSchedule(page, teamId) {
- const res = await page.request.get(
- `${baseUrl()}/api/${orgId()}/oncall/teams/${encodeURIComponent(teamId)}/schedule`,
- { headers: getAuthHeaders() },
+ return await getOrNull(
+ page, `${baseUrl()}/api/${orgId()}/oncall/teams/${encodeURIComponent(teamId)}/schedule`,
+ `Reading the schedule of team ${teamId}`,
);
- if (!res.ok()) return null;
- return await res.json().catch(() => null);
}
/** Who the engine says is on call, optionally at a future instant (micros). */
@@ -495,12 +501,10 @@ async function detachPolicyFromRotations(page, teamId) {
}
async function getTeamPolicy(page, teamId) {
- const res = await page.request.get(
- `${baseUrl()}/api/${orgId()}/oncall/teams/${encodeURIComponent(teamId)}/policy`,
- { headers: getAuthHeaders() },
+ return await getOrNull(
+ page, `${baseUrl()}/api/${orgId()}/oncall/teams/${encodeURIComponent(teamId)}/policy`,
+ `Reading the policy of team ${teamId}`,
);
- if (!res.ok()) return null;
- return await res.json().catch(() => null);
}
// ---------------------------------------------------------------- ownership
@@ -554,11 +558,10 @@ async function ownershipStats(page, { teamId = undefined, days = undefined } = {
if (teamId) params.set('team_id', teamId);
if (days !== undefined) params.set('days', String(days));
const query = params.toString() ? `?${params}` : '';
- const res = await page.request.get(
- `${baseUrl()}/api/${orgId()}/oncall/ownership/stats${query}`, { headers: getAuthHeaders() },
+ return await getOrNull(
+ page, `${baseUrl()}/api/${orgId()}/oncall/ownership/stats${query}`,
+ 'Reading ownership-rule stats',
);
- if (!res.ok()) return null;
- return await res.json().catch(() => null);
}
/** Nominate the org's catch-all team, or clear it with null. */
@@ -569,11 +572,10 @@ async function setDefaultTeam(page, teamId) {
}
async function getRoutingConfig(page) {
- const res = await page.request.get(`${baseUrl()}/api/${orgId()}/oncall/routing/config`, {
- headers: getAuthHeaders(),
- });
- if (!res.ok()) return null;
- return await res.json().catch(() => null);
+ return await getOrNull(
+ page, `${baseUrl()}/api/${orgId()}/oncall/routing/config`,
+ 'Reading the routing config',
+ );
}
// ------------------------------------------------------------------ log data
@@ -768,58 +770,6 @@ async function createPagingAlert(page, {
return { id, name };
}
-/**
- * Fire an alert now instead of waiting for its schedule.
- *
- * PATCH, not POST — the route is `PATCH /api/v2/{org}/alerts/{id}/trigger`.
- * Without this a spec waits a whole frequency window for the scheduler.
- */
-async function triggerAlert(page, alertId, { folder = 'default' } = {}) {
- const res = await api(
- page, 'patch',
- `${baseUrl()}/api/v2/${orgId()}/alerts/${encodeURIComponent(alertId)}/trigger?folder=${encodeURIComponent(folder)}`,
- );
- return await must(res, `Trigger alert ${alertId}`);
-}
-
-/**
- * Fire now if the route will, otherwise let the scheduler do it.
- *
- * THE MANUAL TRIGGER IS BROKEN FOR EXACTLY THE ALERT SHAPE ON-CALL USES, and
- * the failure is a bare `500 {"code":500,"message":""}` with nothing to read.
- * Reproduced against this build, smallest case: an alert carrying
- * `oncall_team` and `destinations: []` — which `POST /api/v2/{org}/alerts`
- * ACCEPTS with 200, and which the scheduler then fires correctly, opening its
- * page within ~15s — 500s on `PATCH .../trigger`. Add any destination to that
- * same alert and the same PATCH answers 200. So the create route, the
- * scheduler and the trigger route disagree about whether the shape is legal,
- * and only the trigger route is wrong.
- *
- * Firing is SCAFFOLDING for the UI cases here — none of them assert on the
- * trigger route — so a spec must not die on it. This swallows the 500 and
- * falls through to `waitForPages`, which still proves the page opened; a
- * scheduler that then never fires fails the spec with its own message rather
- * than this one. The swallow is deliberately narrow: any other failure is
- * re-thrown, so a trigger route that breaks generally still fails loudly.
- *
- * @returns {Promise} whether the manual trigger actually took
- */
-async function triggerAlertOrLetSchedulerFire(page, alertId, { folder = 'default' } = {}) {
- try {
- await triggerAlert(page, alertId, { folder });
- return true;
- } catch (e) {
- if (!/failed: HTTP 500/.test(String(e?.message))) throw e;
- testLogger.warn(
- 'Manual alert trigger 500ed; waiting for the scheduler instead. This is the known ' +
- 'trigger-route defect for an on-call alert with no notification destinations — the ' +
- 'alert itself saved fine and the scheduler fires it.',
- { alertId },
- );
- return false;
- }
-}
-
// -------------------------------------------------------------------- pages
/**
@@ -866,20 +816,39 @@ async function listResponses(page, {
}
async function getResponse(page, responseId) {
- const res = await page.request.get(
- `${baseUrl()}/api/${orgId()}/oncall/responses/${encodeURIComponent(responseId)}`,
- { headers: getAuthHeaders() },
+ return await getOrNull(
+ page, `${baseUrl()}/api/${orgId()}/oncall/responses/${encodeURIComponent(responseId)}`,
+ `Reading page ${responseId}`,
);
- if (!res.ok()) return null;
- return await res.json().catch(() => null);
+}
+
+/** The routing evidence on a page that landed on the wrong team, for the failure message. */
+async function describeMisroutedPage(page, record, expectTeamId) {
+ const lines = [
+ `Page ${record.id} opened on team ${record.team_id ?? 'NONE (unrouted)'}; ` +
+ `expected ${expectTeamId ?? 'NONE (unrouted)'}.`,
+ ];
+ try {
+ const detail = await getResponse(page, record.id);
+ for (const e of detail?.events ?? []) {
+ lines.push(` event ${e.kind} by ${e.actor}: ${String(e.body ?? '').slice(0, 300)}`);
+ }
+ const progress = await getEscalationProgress(page, record.id);
+ if (progress?.stopped_because) lines.push(` escalation stopped because: ${progress.stopped_because}`);
+ } catch (e) {
+ lines.push(` (could not read the record's events: ${e.message})`);
+ }
+ lines.push(
+ 'Check the ownership rule claims the dimension the alert\'s rows carry, and that nothing ' +
+ 'fired the alert by hand: a manual trigger opens a teamless page that swallows every scheduled firing.',
+ );
+ return lines.join('\n');
}
/**
- * Wait for a page to exist, matching on the alert that opened it.
+ * Wait for the pages an alert opens, matched on `source_id` because each firing is its own record.
*
- * Matching on `source_id` rather than a title: a record is keyed
- * `(subject_type, source_id, firing)` and each firing is its own record, so the
- * alert id is the only handle that survives a re-fire.
+ * `expectTeamId` (a team id, or null for an unrouted page) fails at once when any page lands elsewhere.
*
* @returns {Promise} every page opened by this alert, newest first
*/
@@ -887,8 +856,9 @@ async function waitForPages(page, {
alertId,
expected = 1,
teamId = undefined,
+ expectTeamId = undefined,
timeout = PAGE_WAIT_MS,
- pollMs = 3000,
+ pollMs = 2000,
} = {}) {
const deadline = Date.now() + timeout;
let seen = [];
@@ -897,6 +867,10 @@ async function waitForPages(page, {
seen = await listResponses(page, {
subjectType: 'alert', sourceId: alertId, teamId, includeResolved: true,
});
+ if (expectTeamId !== undefined) {
+ const stray = seen.find((r) => (r.team_id ?? null) !== expectTeamId);
+ if (stray) throw new Error(await describeMisroutedPage(page, stray, expectTeamId));
+ }
if (seen.length >= expected) {
testLogger.info('On-call page(s) observed', { alertId, count: seen.length });
return seen;
@@ -905,26 +879,27 @@ async function waitForPages(page, {
}
throw new Error(
- `Alert ${alertId} never opened ${expected} on-call page(s) within ${timeout}ms (saw ${seen.length}).\n` +
- `Causes, in the order worth checking: the alert has no oncall_team AND no ownership rule matches\n` +
- `its identity (a teamless page is still opened, so zero pages means the alert never FIRED);\n` +
- `the alert's window holds no rows (seed the stream first); or the scheduler is not running.`,
+ `Alert ${alertId} opened ${seen.length} of ${expected} on-call page(s) within ${timeout}ms.\n` +
+ 'A firing opens a page even when nothing routes it, so a shortfall means the alert never FIRED: ' +
+ 'check the stream has rows inside the alert\'s window and that the alert scheduler is running.',
);
}
/**
- * Create the alert, fire it, and wait for the page it opens.
+ * Create the alert and wait for the scheduler to fire it into a page; never fire it by hand.
*
- * The one helper a UI spec that just needs "a page on screen" should call.
+ * `expectTeamId` defaults to the alert's own `teamId`; pass the team an ownership rule should pick otherwise.
*
* @returns {Promise<{alert: {id: string, name: string}, pages: object[]}>}
*/
-async function firePageAndWait(page, { alertOptions, expected = 1, timeout = PAGE_WAIT_MS } = {}) {
+async function firePageAndWait(page, {
+ alertOptions,
+ expected = 1,
+ expectTeamId = alertOptions?.teamId,
+ timeout = PAGE_WAIT_MS,
+} = {}) {
const alert = await createPagingAlert(page, alertOptions);
- await triggerAlertOrLetSchedulerFire(page, alert.id);
- const pages = await waitForPages(page, {
- alertId: alert.id, expected, teamId: alertOptions.teamId, timeout,
- });
+ const pages = await waitForPages(page, { alertId: alert.id, expected, expectTeamId, timeout });
return { alert, pages };
}
@@ -953,22 +928,18 @@ async function sendTestPage(page, teamId, { priority = undefined } = {}) {
* `EscalationProgress`, which is the trap.
*/
async function getEscalationProgress(page, responseId) {
- const res = await page.request.get(
- `${baseUrl()}/api/${orgId()}/oncall/responses/${encodeURIComponent(responseId)}/escalation`,
- { headers: getAuthHeaders() },
+ return await getOrNull(
+ page, `${baseUrl()}/api/${orgId()}/oncall/responses/${encodeURIComponent(responseId)}/escalation`,
+ `Reading the escalation progress of page ${responseId}`,
);
- if (!res.ok()) return null;
- return await res.json().catch(() => null);
}
/** The delivery ledger — the ground truth `progress.reached` must agree with (§2.1). */
async function getDeliveries(page, responseId) {
- const res = await page.request.get(
- `${baseUrl()}/api/${orgId()}/oncall/responses/${encodeURIComponent(responseId)}/deliveries`,
- { headers: getAuthHeaders() },
+ return await getOrNull(
+ page, `${baseUrl()}/api/${orgId()}/oncall/responses/${encodeURIComponent(responseId)}/deliveries`,
+ `Reading the delivery ledger of page ${responseId}`,
);
- if (!res.ok()) return null;
- return await res.json().catch(() => null);
}
// ------------------------------------------------------------------ teardown
@@ -1136,8 +1107,6 @@ module.exports = {
waitForStreamSearchable,
seedNotificationDestination,
createPagingAlert,
- triggerAlert,
- triggerAlertOrLetSchedulerFire,
listResponses,
getResponse,
waitForPages,
From f7436e1472b3c1123fa3804815a9125cafe9a837 Mon Sep 17 00:00:00 2001
From: Loakesh Indiran <66478092+Loaki07@users.noreply.github.com>
Date: Tue, 3 Nov 2026 07:30:54 +0000
Subject: [PATCH 6/7] fix(synthetics): offer only AWS when adding a location
(#16308)
---
.../Synthetics/synthetics-settings.spec.js | 5 +++--
.../settings/SyntheticsLocationForm.spec.ts | 17 +++++++++++++++-
.../settings/SyntheticsLocationForm.vue | 20 ++++++++++++-------
3 files changed, 32 insertions(+), 10 deletions(-)
diff --git a/tests/ui-testing/playwright-tests/Synthetics/synthetics-settings.spec.js b/tests/ui-testing/playwright-tests/Synthetics/synthetics-settings.spec.js
index 19fefbfb16..0afb052721 100644
--- a/tests/ui-testing/playwright-tests/Synthetics/synthetics-settings.spec.js
+++ b/tests/ui-testing/playwright-tests/Synthetics/synthetics-settings.spec.js
@@ -73,13 +73,14 @@ test.describe('Synthetics settings — locations and tokens', { tag: ['@syntheti
test('adds a public location from Settings', { tag: ['@P1'] }, async ({ page }, testInfo) => {
const region = uniqueName('loc', testInfo);
- const id = `${PROVIDER}-${region}`;
+ // The create form offers only AWS.
+ const id = `aws-${region}`;
testLogger.info('Opening the locations settings in the meta org');
const s = pm.syntheticsSettingsPage;
await s.gotoLocations(META_ORG);
testLogger.info('Adding a public location', { locationId: id });
await s.openAddForm();
- await s.fillLocationForm({ label: `E2E ${region}`, provider: 'custom', customProvider: PROVIDER, region });
+ await s.fillLocationForm({ label: `E2E ${region}`, provider: 'aws', region });
await s.submitLocationForm();
testLogger.info('Verifying the location is listed');
await s.expectToast('Location created successfully');
diff --git a/web/src/components/settings/SyntheticsLocationForm.spec.ts b/web/src/components/settings/SyntheticsLocationForm.spec.ts
index 4335291e79..be86faf80d 100644
--- a/web/src/components/settings/SyntheticsLocationForm.spec.ts
+++ b/web/src/components/settings/SyntheticsLocationForm.spec.ts
@@ -470,9 +470,24 @@ describe("SyntheticsLocationForm", () => {
// ── Provider options ───────────────────────────────────────────────────────
- it("exposes the four provider options in the computed", () => {
+ it("offers only AWS when creating a location", () => {
wrapper = createWrapper();
const options = wrapper.vm.providerOptions;
+ expect(options.map((o: { value: string }) => o.value)).toEqual(["aws"]);
+ });
+
+ it("keeps all provider options in edit mode to label existing locations", () => {
+ wrapper = createWrapper({
+ isEdit: true,
+ data: {
+ id: "gcp-us-east1",
+ provider: "gcp",
+ region: "us-east1",
+ label: "GCP",
+ enabled: true,
+ },
+ });
+ const options = wrapper.vm.providerOptions;
expect(options).toHaveLength(4);
expect(options.map((o: { value: string }) => o.value)).toEqual([
"aws",
diff --git a/web/src/components/settings/SyntheticsLocationForm.vue b/web/src/components/settings/SyntheticsLocationForm.vue
index 7890b21447..aad2c79b6a 100644
--- a/web/src/components/settings/SyntheticsLocationForm.vue
+++ b/web/src/components/settings/SyntheticsLocationForm.vue
@@ -141,13 +141,19 @@ export default defineComponent({
const locationFormRef = ref(null);
const locationFormSchema = makeSyntheticsLocationFormSchema(t);
- // Provider options for the select dropdown.
- const providerOptions = computed(() => [
- { label: raw("AWS"), value: "aws" },
- { label: raw("GCP"), value: "gcp" },
- { label: raw("Azure"), value: "azure" },
- { label: t("synthetics.locations.providerOther"), value: "custom" },
- ]);
+ // Provider options for the select dropdown. Only AWS probes are supported,
+ // so create offers AWS alone. Edit keeps the full list so the (disabled)
+ // select still labels locations saved with another provider.
+ const providerOptions = computed(() =>
+ props.isEdit
+ ? [
+ { label: raw("AWS"), value: "aws" },
+ { label: raw("GCP"), value: "gcp" },
+ { label: raw("Azure"), value: "azure" },
+ { label: t("synthetics.locations.providerOther"), value: "custom" },
+ ]
+ : [{ label: raw("AWS"), value: "aws" }],
+ );
// Edit-prefill defaults. OForm reads `:default-values` once at mount; ODrawer
// (reka-ui `lazy`) unmounts/remounts its body on close/open so this re-seeds
From 289a52ea207740ff99417cf32f300a5afc787673 Mon Sep 17 00:00:00 2001
From: Vamsi Karthik R
Date: Tue, 3 Nov 2026 07:43:25 +0000
Subject: [PATCH 7/7] fix(logs): keep field-value filters on the exact field
name (#16304)
---
.../ci-matrix/ci_matrix_regression.json | 3 +-
.../pages/pipelinesPages/pipelinesPage.js | 23 +++++
...gs-3322-field-filter-similar-names.spec.js | 94 +++++++++++++++++++
.../NodeForm/ScheduledPipeline.spec.js | 38 ++++++++
.../logs/SearchBar.addSearchTerm.spec.ts | 57 +++++++++++
web/src/plugins/logs/SearchBar.vue | 3 +-
web/src/plugins/logs/filterUtils.spec.ts | 85 +++++++++++++++++
web/src/plugins/logs/filterUtils.ts | 6 +-
8 files changed, 305 insertions(+), 4 deletions(-)
create mode 100644 tests/ui-testing/playwright-tests/RegressionSet/Logs/logs-3322-field-filter-similar-names.spec.js
diff --git a/tests/ui-testing/ci-matrix/ci_matrix_regression.json b/tests/ui-testing/ci-matrix/ci_matrix_regression.json
index 54b9ab52a4..f20fbb7c26 100644
--- a/tests/ui-testing/ci-matrix/ci_matrix_regression.json
+++ b/tests/ui-testing/ci-matrix/ci_matrix_regression.json
@@ -14,7 +14,8 @@
"logs-table-stream-switch.spec.js",
"logs-internal-streams.spec.js",
"logs-11966-value-multiselect-quotes.spec.js",
- "logs-explain-sql-mode.spec.js"
+ "logs-explain-sql-mode.spec.js",
+ "logs-3322-field-filter-similar-names.spec.js"
],
"disabled": []
},
diff --git a/tests/ui-testing/pages/pipelinesPages/pipelinesPage.js b/tests/ui-testing/pages/pipelinesPages/pipelinesPage.js
index 915b8b60d5..d85870f862 100644
--- a/tests/ui-testing/pages/pipelinesPages/pipelinesPage.js
+++ b/tests/ui-testing/pages/pipelinesPages/pipelinesPage.js
@@ -3107,6 +3107,29 @@ export class PipelinesPage {
return (dataTest ?? '').replace(`logs-search-subfield-add-${fieldName}-`, '');
}
+ /** Replace the Associate Query SQL through the Monaco model; the field list reads the editor value. */
+ async setQuerySourceSql(query) {
+ const host = '[data-test="scheduled-pipeline-sql-editor"]';
+ await this.page.locator(host).waitFor({ state: 'visible', timeout: 15000 });
+ await this.page.evaluate(({ selector, value }) => {
+ const root = document.querySelector(selector);
+ const editor = (window.monaco?.editor?.getEditors?.() ?? []).find((ed) => root?.contains(ed.getDomNode?.()));
+ if (!editor) throw new Error('Associate Query SQL editor not found');
+ const model = editor.getModel();
+ editor.executeEdits('setQuerySourceSql', [{ range: model.getFullModelRange(), text: value }]);
+ }, { selector: host, value: query });
+ await expect.poll(() => this.getQuerySourceSql(), { timeout: 5000 }).toBe(query);
+ }
+
+ /** The Associate Query SQL exactly as the editor model holds it. */
+ async getQuerySourceSql() {
+ return await this.page.evaluate((selector) => {
+ const root = document.querySelector(selector);
+ const editor = (window.monaco?.editor?.getEditors?.() ?? []).find((ed) => root?.contains(ed.getDomNode?.()));
+ return editor ? editor.getValue() : null;
+ }, '[data-test="scheduled-pipeline-sql-editor"]');
+ }
+
/** Untick every selected value for the open field, which removes its query condition. */
async clearQueryFieldValueFilter() {
const clearBtn = this.page.locator('[data-test="field-values-panel-clear-selection-btn"]');
diff --git a/tests/ui-testing/playwright-tests/RegressionSet/Logs/logs-3322-field-filter-similar-names.spec.js b/tests/ui-testing/playwright-tests/RegressionSet/Logs/logs-3322-field-filter-similar-names.spec.js
new file mode 100644
index 0000000000..74368b5ced
--- /dev/null
+++ b/tests/ui-testing/playwright-tests/RegressionSet/Logs/logs-3322-field-filter-similar-names.spec.js
@@ -0,0 +1,94 @@
+const { test, expect, navigateToBase } = require('../../utils/enhanced-baseFixtures.js');
+const testLogger = require('../../utils/test-logger.js');
+const PageManager = require('../../../pages/page-manager.js');
+const { sendRequest, getHeaders, getIngestionUrl, waitForStreamData } = require('../../utils/data-ingestion.js');
+const { getOrgIdentifier } = require('../../utils/cloud-auth.js');
+
+// operation_name is the tail of db_operation_name, so an unbounded field match edits the wrong condition.
+const ROWS = [
+ { db_operation_name: 'SELECT', operation_name: 'GET /', name: 'row_a' },
+ { db_operation_name: 'SELECT', operation_name: 'GET /', name: 'row_b' },
+ { db_operation_name: 'SELECT', operation_name: 'POST /', name: 'row_c' },
+ { db_operation_name: 'INSERT', operation_name: 'GET /', name: 'row_d' },
+];
+
+test.describe("Field-value filter on a field whose name ends another field's name (o2-enterprise#3322)", () => {
+ let pm;
+ let stream;
+
+ test.beforeEach(async ({ page }, testInfo) => {
+ testLogger.testStart(testInfo.title, testInfo.file);
+ await navigateToBase(page);
+ pm = new PageManager(page);
+
+ stream = 'e2e_3322_' + Math.random().toString(36).slice(2, 7);
+ const orgId = getOrgIdentifier() || 'default';
+ await sendRequest(page, getIngestionUrl(orgId, stream), ROWS, getHeaders());
+ await waitForStreamData(page, stream, ROWS.length);
+ });
+
+ test.afterEach(async ({ page }) => {
+ await new PageManager(page).logsPage.deleteStream(stream).catch((e) =>
+ testLogger.warn(`Failed to delete stream ${stream}: ${e.message}`)
+ );
+ });
+
+ test("Logs SQL mode: ticking operation_name values keeps the db_operation_name condition", {
+ tag: ['@bug-3322', '@P2', '@regression', '@logsRegression']
+ }, async () => {
+ const base = `SELECT * FROM "${stream}" WHERE db_operation_name = 'SELECT'`;
+ await pm.logsPage.openLogsWithSqlQuery(stream, base);
+ await pm.logsPage.clickRefreshButton();
+ await expect.poll(async () => (await pm.logsPage.getResultRowTexts()).length, { timeout: 30000 }).toBe(3);
+
+ await pm.logsPage.clickFieldExpandButton('operation_name');
+ await pm.logsPage.fieldValueCheckbox('operation_name', 'GET /').click();
+ await expect.poll(() => pm.logsPage.getQueryEditorText(), {
+ timeout: 15000,
+ message: 'the db_operation_name condition must stay and operation_name must be added',
+ }).toBe(`${base} AND operation_name = 'GET /'`);
+
+ await pm.logsPage.clickRefreshButton();
+ await expect.poll(async () => (await pm.logsPage.getResultRowTexts()).length, { timeout: 30000 }).toBe(2);
+ const rows = (await pm.logsPage.getResultRowTexts()).join('\n');
+ expect(rows).toContain('row_a');
+ expect(rows).toContain('row_b');
+
+ await pm.logsPage.fieldValueCheckbox('operation_name', 'POST /').click();
+ await expect.poll(() => pm.logsPage.getQueryEditorText(), {
+ timeout: 15000,
+ message: 'a second value must replace only the operation_name condition',
+ }).toBe(`${base} AND (operation_name = 'GET /' OR operation_name = 'POST /')`);
+
+ await pm.logsPage.clickRefreshButton();
+ await expect.poll(async () => (await pm.logsPage.getResultRowTexts()).length, { timeout: 30000 }).toBe(3);
+ expect((await pm.logsPage.getResultRowTexts()).join('\n')).not.toContain('row_d');
+ });
+
+ test("Pipelines Query source SQL: ticking an operation_name value keeps the db_operation_name condition", {
+ tag: ['@bug-3322', '@P2', '@regression', '@logsRegression', '@pipelinesRegressionQuery']
+ }, async () => {
+ await pm.pipelinesPage.openPipelineMenu();
+ await pm.pipelinesPage.addPipeline();
+ await pm.pipelinesPage.dragStreamToTarget(pm.pipelinesPage.queryButton);
+ await pm.pipelinesPage.waitForScheduledPipelineDialog();
+ await pm.pipelinesPage.expandBuildQuerySection();
+ await pm.pipelinesPage.selectStreamType('logs');
+ await pm.pipelinesPage.selectStreamName(stream);
+ await pm.pipelinesPage.expectSqlEditorVisible();
+
+ const base = `SELECT * FROM "${stream}" WHERE db_operation_name='SELECT'`;
+ await pm.pipelinesPage.setQuerySourceSql(base);
+
+ const value = await pm.pipelinesPage.addQueryFieldValueFilter('operation_name');
+ testLogger.info(`Ticked operation_name='${value}'`);
+ expect(['GET /', 'POST /']).toContain(value);
+ await expect.poll(() => pm.pipelinesPage.getQuerySourceSql(), {
+ timeout: 15000,
+ message: 'the db_operation_name condition must stay and operation_name must be added',
+ }).toBe(`${base} AND operation_name='${value}'`);
+
+ await pm.pipelinesPage.clickBodyCorner();
+ await pm.pipelinesPage.clickCancelPipelineBtnForce();
+ });
+});
diff --git a/web/src/components/pipeline/NodeForm/ScheduledPipeline.spec.js b/web/src/components/pipeline/NodeForm/ScheduledPipeline.spec.js
index 50e58f5559..c49d374d3a 100644
--- a/web/src/components/pipeline/NodeForm/ScheduledPipeline.spec.js
+++ b/web/src/components/pipeline/NodeForm/ScheduledPipeline.spec.js
@@ -470,6 +470,44 @@ describe("ScheduledPipeline Component", () => {
"SELECT max(_timestamp), count(*) FROM \"stream1\" WHERE level='error' GROUP BY histogram(_timestamp)",
);
});
+
+ it.each([
+ ["an include", "operation_name='GET /'"],
+ ["a multi-value exclude", "(operation_name!='GET /' and operation_name!='POST /')"],
+ ])("adds %s next to a field whose name ends with the same name", async (_, filter) => {
+ const base = `SELECT * FROM "qa" WHERE db_operation_name='SELECT'`;
+ const mockEditorRef = {
+ getCursorIndex: vi.fn().mockReturnValue(-1),
+ getValue: vi.fn().mockReturnValue(base),
+ setValue: vi.fn(),
+ };
+ child.pipelineEditorRef = mockEditorRef;
+ child.tab = "sql";
+
+ await child.handleSidebarEvent("add-field", filter);
+
+ expect(mockEditorRef.setValue).toHaveBeenCalledWith(`${base} AND ${filter}`);
+ });
+
+ it("still replaces the exact field's value next to the longer field", async () => {
+ const mockEditorRef = {
+ getCursorIndex: vi.fn().mockReturnValue(-1),
+ getValue: vi
+ .fn()
+ .mockReturnValue(
+ `SELECT * FROM "qa" WHERE db_operation_name='SELECT' AND operation_name='POST /'`,
+ ),
+ setValue: vi.fn(),
+ };
+ child.pipelineEditorRef = mockEditorRef;
+ child.tab = "sql";
+
+ await child.handleSidebarEvent("add-field", "operation_name='GET /'");
+
+ expect(mockEditorRef.setValue).toHaveBeenCalledWith(
+ `SELECT * FROM "qa" WHERE db_operation_name='SELECT' AND operation_name='GET /'`,
+ );
+ });
});
describe("Function Management", () => {
diff --git a/web/src/plugins/logs/SearchBar.addSearchTerm.spec.ts b/web/src/plugins/logs/SearchBar.addSearchTerm.spec.ts
index f0a8a5f6dc..4e6d12e13c 100644
--- a/web/src/plugins/logs/SearchBar.addSearchTerm.spec.ts
+++ b/web/src/plugins/logs/SearchBar.addSearchTerm.spec.ts
@@ -112,6 +112,63 @@ describe("SearchBar — adding a filter term to the query", () => {
expect(wrapper.vm.searchObj.data.stream.addToFilter).toBe("");
});
+ describe("a field whose name ends another field's name", () => {
+ const base = `SELECT * FROM "qa" WHERE db_operation_name = 'SELECT'`;
+
+ beforeEach(() => {
+ wrapper.vm.searchObj.meta.sqlMode = true;
+ });
+
+ it("adds an include and an exclude in SQL mode without touching the longer field", async () => {
+ wrapper.vm.searchObj.data.query = base;
+ expect(await addFilterTerm("operation_name = 'GET /'")).toBe(
+ `${base} AND operation_name = 'GET /'`,
+ );
+
+ wrapper.vm.searchObj.data.query = base;
+ expect(await addFilterTerm("operation_name != 'GET /'")).toBe(
+ `${base} AND operation_name != 'GET /'`,
+ );
+ });
+
+ it("adds a multi-value group in SQL mode without touching the longer field's group", async () => {
+ const group = "(db_operation_name = 'SELECT' OR db_operation_name = 'INSERT')";
+ const added = "(operation_name = 'GET /' OR operation_name = 'POST /')";
+ wrapper.vm.searchObj.data.query = `SELECT * FROM "qa" WHERE ${group}`;
+
+ expect(await addFilterTerm(added)).toBe(`SELECT * FROM "qa" WHERE ${group} AND ${added}`);
+ });
+
+ it("still replaces the exact field's condition, plain or stream-qualified", async () => {
+ wrapper.vm.searchObj.data.query = `${base} AND operation_name = 'POST /'`;
+ expect(await addFilterTerm("operation_name = 'GET /'")).toBe(
+ `${base} AND operation_name = 'GET /'`,
+ );
+
+ wrapper.vm.searchObj.data.query = `${base} AND "qa".operation_name = 'POST /'`;
+ expect(await addFilterTerm(`"qa".operation_name = 'GET /'`)).toBe(
+ `${base} AND "qa".operation_name = 'GET /'`,
+ );
+ });
+
+ it("still replaces a quoted exact field and leaves a quoted longer field alone", async () => {
+ wrapper.vm.searchObj.data.query = `${base} AND "db_operation_name" != 'INSERT' AND "operation_name" = 'POST /' AND operation_name = 'PUT /'`;
+
+ expect(await addFilterTerm("operation_name = 'GET /'")).toBe(
+ `${base} AND "db_operation_name" != 'INSERT' AND operation_name = 'GET /' AND operation_name = 'GET /'`,
+ );
+ });
+
+ it("keeps the longer field in non-SQL mode", async () => {
+ wrapper.vm.searchObj.meta.sqlMode = false;
+ wrapper.vm.searchObj.data.query = "db_operation_name = 'SELECT'";
+
+ expect(await addFilterTerm("operation_name = 'GET /'")).toBe(
+ "db_operation_name = 'SELECT' and operation_name = 'GET /'",
+ );
+ });
+ });
+
describe("free text (item 1 AC6.9)", () => {
const stream = (name: string, fts: boolean) => ({
name,
diff --git a/web/src/plugins/logs/SearchBar.vue b/web/src/plugins/logs/SearchBar.vue
index 5d6b74e85e..fa041b13ed 100644
--- a/web/src/plugins/logs/SearchBar.vue
+++ b/web/src/plugins/logs/SearchBar.vue
@@ -1871,7 +1871,8 @@ const replaceExistingFieldCondition = (
const valPat = `(?:'(?:[^']|'')*'|null|\\d+(?:\\.\\d+)?|true|false)`;
const opPat = `(?:=|!=|is(?:\\s+not)?)`;
const fieldPat = `(?:"${esc}"|${esc})`;
- const condPat = `(?:"[^"]+"\\.)?${fieldPat}\\s*${opPat}\\s*${valPat}`;
+ // The lookbehind stops `operation_name` from matching inside `db_operation_name`.
+ const condPat = `(? {
+ it("does not see a condition on db_operation_name as one on operation_name", () => {
+ expect(hasFieldCondition("db_operation_name = 'SELECT'", "operation_name")).toBe(false);
+ expect(hasFieldCondition(`"qa".db_operation_name = 'SELECT'`, "operation_name")).toBe(false);
+ });
+
+ it("adds an include next to the longer field's condition", () => {
+ expect(applyFilterToQuery("db_operation_name = 'SELECT'", "operation_name = 'GET /'")).toBe(
+ "db_operation_name = 'SELECT' and operation_name = 'GET /'",
+ );
+ });
+
+ it("adds an exclude next to the longer field's condition", () => {
+ expect(applyFilterToQuery("db_operation_name = 'SELECT'", "operation_name != 'GET /'")).toBe(
+ "db_operation_name = 'SELECT' and operation_name != 'GET /'",
+ );
+ });
+
+ it("adds a multi-value include and exclude group next to the longer field's condition", () => {
+ const include = "(operation_name = 'GET /' OR operation_name = 'POST /')";
+ const exclude = "(operation_name != 'GET /' AND operation_name != 'POST /')";
+ expect(applyFilterToQuery("db_operation_name = 'SELECT'", include)).toBe(
+ `db_operation_name = 'SELECT' and ${include}`,
+ );
+ expect(applyFilterToQuery("db_operation_name = 'SELECT'", exclude)).toBe(
+ `db_operation_name = 'SELECT' and ${exclude}`,
+ );
+ });
+
+ it("leaves the longer field's multi-value group intact", () => {
+ const group = "(db_operation_name = 'SELECT' OR db_operation_name = 'INSERT')";
+ expect(applyFilterToQuery(group, "operation_name = 'GET /'")).toBe(
+ `${group} and operation_name = 'GET /'`,
+ );
+ });
+
+ it("still replaces the exact field's condition and keeps the longer field's", () => {
+ expect(
+ applyFilterToQuery(
+ "db_operation_name = 'SELECT' and operation_name = 'POST /'",
+ "operation_name = 'GET /'",
+ ),
+ ).toBe("db_operation_name = 'SELECT' and operation_name = 'GET /'");
+ expect(
+ applyFilterToQuery(
+ "(db_operation_name = 'SELECT' OR db_operation_name = 'INSERT') and operation_name = 'POST /'",
+ "(operation_name = 'GET /' OR operation_name = 'PUT /')",
+ ),
+ ).toBe(
+ "(db_operation_name = 'SELECT' OR db_operation_name = 'INSERT') and (operation_name = 'GET /' OR operation_name = 'PUT /')",
+ );
+ });
+
+ it("handles stream-qualified fields", () => {
+ expect(
+ applyFilterToQuery(`"qa".db_operation_name = 'SELECT'`, `"qa".operation_name = 'GET /'`),
+ ).toBe(`"qa".db_operation_name = 'SELECT' and "qa".operation_name = 'GET /'`);
+ expect(
+ applyFilterToQuery(
+ `"qa".db_operation_name = 'SELECT' and "qa".operation_name = 'POST /'`,
+ `"qa".operation_name = 'GET /'`,
+ ),
+ ).toBe(`"qa".db_operation_name = 'SELECT' and "qa".operation_name = 'GET /'`);
+ });
+
+ it("keeps null, number and boolean conditions on the longer field", () => {
+ expect(applyFilterToQuery("http_status = 500 and status is null", "status = 'ok'")).toBe(
+ "http_status = 500 and status = 'ok'",
+ );
+ expect(applyFilterToQuery("is_enabled = true and enabled = true", "enabled = false")).toBe(
+ "is_enabled = true and enabled = false",
+ );
+ });
+
+ it("replaces only the exact field when called directly", () => {
+ expect(
+ replaceExistingFieldCondition(
+ "username = 'bob' and name = 'al'",
+ "name",
+ "(name = 'al' OR name = 'eve')",
+ ),
+ ).toBe("username = 'bob' and (name = 'al' OR name = 'eve')");
+ });
+});
diff --git a/web/src/plugins/logs/filterUtils.ts b/web/src/plugins/logs/filterUtils.ts
index 1786eac433..f1c5b65385 100644
--- a/web/src/plugins/logs/filterUtils.ts
+++ b/web/src/plugins/logs/filterUtils.ts
@@ -39,7 +39,8 @@ export const hasFieldCondition = (queryStr: string, fieldName: string): boolean
const esc = fieldName.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
const valPat = `(?:'(?:[^']|'')*'|null|\\d+(?:\\.\\d+)?|true|false)`;
const opPat = `(?:=|!=|is(?:\\s+not)?)`;
- const condPat = `(?:"[^"]+"\\.)?${esc}\\s*${opPat}\\s*${valPat}`;
+ // The lookbehind stops `operation_name` from matching inside `db_operation_name`.
+ const condPat = `(?