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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion fleet/PROTOCOL.md
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,10 @@ Amazon Bedrock whether the text may be shown publicly, read together with every
in the sky (so words cannot combine into something unacceptable). The model id and region
come from the coordinator's configuration (`--text-check-model`, `--text-check-region`).
Only an explicit allow commits the word; a refusal, an error, a timeout (5 s) or a
coordinator started without a text check (`--text-check none`) refuses it. A word checked
coordinator started without a text check (`--text-check none`) refuses it. An error's
reason, in the receipt, its `check` and `actions`, is one of two fixed sentences, "words are
turned off right now" (`--text-check none`) or "couldn't check that word, try again"
(anything else); what went wrong is in the coordinator's log. A word checked
against a sky that changed meanwhile is refused (send it again). A check that cannot run
marks the coordinator degraded (`failures`) until one runs again. The scene keeps the
latest 16 words. A refused word's text appears nowhere: not in the scene, not in
Expand Down
6 changes: 3 additions & 3 deletions fleet/core/src/apply.rs
Original file line number Diff line number Diff line change
Expand Up @@ -675,9 +675,9 @@ fn intent_effect(
word_rate(state, c.client, at)?;
match check {
None => return Err("the text check did not run".into()),
Some(k) if k.verdict == "error" => {
return Err(format!("the text check failed: {}", k.reason))
}
// The coordinator words an error's reason for the viewer ("words are
// turned off right now"); the detail is in its log.
Some(k) if k.verdict == "error" => return Err(k.reason.clone()),
Some(k) if !k.allowed() => {
return Err(format!("the text check refused it: {}", k.reason))
}
Expand Down
6 changes: 3 additions & 3 deletions fleet/core/tests/vectors/07_words.json
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@
},
"at": 9000,
"check": {
"reason": "text check unavailable",
"reason": "couldn't check that word, try again",
"sky": [
"hello"
],
Expand Down Expand Up @@ -185,7 +185,7 @@
{
"after": 5,
"checks": {
"/receipts/w4/reason": "the text check failed: text check unavailable"
"/receipts/w4/reason": "couldn't check that word, try again"
}
},
{
Expand Down Expand Up @@ -255,7 +255,7 @@
"id": "w4",
"name": "word",
"outcome": "refused",
"summary": "the text check failed: text check unavailable",
"summary": "couldn't check that word, try again",
"viewer": "v2"
},
{
Expand Down
2 changes: 1 addition & 1 deletion fleet/web/core/test/fixtures/coordinator.json

Large diffs are not rendered by default.

68 changes: 60 additions & 8 deletions src/fleet/coord/text_check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,26 @@ use std::time::Duration;
/// How long a word may wait for its check.
pub const CHECK_TIMEOUT: Duration = Duration::from_secs(5);

/// What a viewer reads when no checker is configured (`--text-check none`).
pub const WORDS_OFF: &str = "words are turned off right now";

/// What a viewer reads when the check could not answer (an error or a timeout). The
/// reason itself goes to the coordinator log only: it can name the model, the region or
/// an AWS error, and the receipt and the actions list are public.
pub const CHECK_FAILED: &str = "couldn't check that word, try again";

/// The error `NoCheck` answers with, so the verdict can tell "turned off" from "failed".
#[derive(Debug)]
pub struct Disabled;

impl std::fmt::Display for Disabled {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.write_str("no text check is configured")
}
}

impl std::error::Error for Disabled {}

#[derive(Debug, Clone, PartialEq, Eq)]
pub enum Verdict {
Allow,
Expand Down Expand Up @@ -46,19 +66,30 @@ async fn check_inner(checker: &dyn TextChecker, sky: &[String], text: &str) -> T
reason,
sky: vec![],
},
Ok(Err(e)) if e.downcast_ref::<Disabled>().is_some() => TextCheck {
verdict: "error".into(),
reason: WORDS_OFF.into(),
sky: vec![],
},
Ok(Err(e)) => {
tracing::warn!(error = %format!("{e:#}"), "text check failed");
TextCheck {
verdict: "error".into(),
reason: format!("{e:#}"),
reason: CHECK_FAILED.into(),
sky: vec![],
}
}
Err(_) => {
tracing::warn!(
timeout_s = CHECK_TIMEOUT.as_secs(),
"text check failed: no answer in time"
);
TextCheck {
verdict: "error".into(),
reason: CHECK_FAILED.into(),
sky: vec![],
}
}
Err(_) => TextCheck {
verdict: "error".into(),
reason: format!("no answer within {} s", CHECK_TIMEOUT.as_secs()),
sky: vec![],
},
}
}

Expand All @@ -68,7 +99,7 @@ pub struct NoCheck;
#[async_trait]
impl TextChecker for NoCheck {
async fn check(&self, _sky: &[String], _text: &str) -> Result<Verdict> {
bail!("no text check is configured")
Err(Disabled.into())
}
}

Expand Down Expand Up @@ -289,6 +320,27 @@ mod tests {
};
let v = check_text(&slow, &[], "hi").await;
assert_eq!(v.verdict, "error");
assert!(v.reason.contains("no answer"), "{v:?}");
assert_eq!(v.reason, CHECK_FAILED, "{v:?}");
}

/// The reason travels to the viewer (receipt, actions list, coordinator status), so it
/// is one of two fixed sentences and never the checker's own error.
#[tokio::test]
async fn viewers_read_a_fixed_reason_not_the_checker_error() {
let off = check_text(&NoCheck, &[], "Hi!").await;
assert_eq!(
(off.verdict.as_str(), off.reason.as_str()),
("error", WORDS_OFF)
);
let down = FakeCheck {
refuse: vec![],
fail: true,
delay: Duration::ZERO,
};
let failed = check_text(&down, &[], "Hi!").await;
assert_eq!(
(failed.verdict.as_str(), failed.reason.as_str()),
("error", CHECK_FAILED)
);
}
}
20 changes: 18 additions & 2 deletions tests/test_fleet_coord.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

use fcvm::fleet::aggregate::codec::Raw;
use fcvm::fleet::aggregate::frame::Frame;
use fcvm::fleet::coord::text_check::{FakeCheck, TextChecker};
use fcvm::fleet::coord::text_check::{FakeCheck, NoCheck, TextChecker, CHECK_FAILED, WORDS_OFF};
use fcvm::fleet::coord::turnstile::{self, Verifier};
use fcvm::fleet::coord::{agents, ec2, live, poster, start, store, Handle};
use fcvm::fleet::lines::{spawn_writer, LineReader};
Expand Down Expand Up @@ -521,7 +521,23 @@ async fn a_failing_text_check_degrades_the_coordinator() {
.unwrap()
.clone();
assert_eq!(f["component"], "text_check");
assert!(f["reason"].as_str().unwrap().contains("down"), "{f}");
// The receipt, its check and the failures list are public: they carry a fixed
// sentence, and the checker's own error ("fake text check is down") stays in the log.
assert_eq!(f["reason"], CHECK_FAILED, "{f}");
assert_eq!(r["reason"], CHECK_FAILED, "{r}");
assert_eq!(r["check"]["reason"], CHECK_FAILED, "{r}");
}

/// A coordinator started with `--text-check none` tells the viewer words are off, not
/// that a check failed.
#[tokio::test]
async fn without_a_text_check_viewers_read_that_words_are_off() {
let (url, _h) = coordinator(Arc::new(NoCheck)).await;
let mut c = Viewer::connect(&url).await;
c.intent("w", "word", json!({"text": "Hi!"})).await;
let r = c.receipt("w").await;
assert_eq!(r["outcome"], "refused");
assert_eq!(r["reason"], WORDS_OFF, "{r}");
}

/// Limits are per client address: `CF-Connecting-IP` when the coordinator trusts it (it
Expand Down
Loading