Address PR review: UID length guard, ec_id decoupling, validation, docs

- Add MAX_UID_LENGTH check to extract_fp_signals, matching all other
  sync paths (pixel, batch, pull) that enforce the 512-byte limit
- Decouple ec_id from PullSyncContext in RouteOutcome so first-party
  signal writes succeed even when client_ip is unavailable
- Add current_timestamp_ms() helper to ec module, replace inline
  SystemTime boilerplate in adapter
- Add 128-char max length validation for fp_signal_cookie_names
- Fix partner.rs doc comments referencing nonexistent fp_signal_enabled
  field — now correctly references fp_signal_cookie_names
- Document TTL default behavior in filter_stale_signals
- Fix Prettier formatting on two doc files

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: ChristianPavilonis

crates/trusted-server-adapter-fastly/src/main.rs

@@ -12,6 +12,7 @@ use trusted_server_core::constants::{
 };
 use trusted_server_core::ec::admin::handle_register_partner;
 use trusted_server_core::ec::batch_sync::handle_batch_sync;
+use trusted_server_core::ec::current_timestamp_ms;
 use trusted_server_core::ec::device::DeviceSignals;
 use trusted_server_core::ec::finalize::ec_finalize_response;
 use trusted_server_core::ec::fp_signals::{
@@ -111,21 +112,19 @@ fn main() -> Result<(), Error> {
     let RouteOutcome {
         response,
         pull_sync_context,
+        ec_id,
         fp_signals,
         fp_signal_configs,
     } = outcome;
 
     response.send_to_client();
 
     // Write first-party signals to KV (post-send, off critical path).
+    // Uses ec_id directly — not gated on PullSyncContext — so signals are
+    // still written when client_ip is unavailable.
     if !fp_signals.is_empty() {
-        if let Some(ref ctx) = pull_sync_context {
-            run_fp_signal_collection_after_send(
-                &settings,
-                ctx.ec_id(),
-                &fp_signals,
-                &fp_signal_configs,
-            );
+        if let Some(ref ec_id) = ec_id {
+            run_fp_signal_collection_after_send(&settings, ec_id, &fp_signals, &fp_signal_configs);
         }
     }
 
@@ -140,6 +139,10 @@ fn main() -> Result<(), Error> {
 struct RouteOutcome {
     response: Response,
     pull_sync_context: Option<PullSyncContext>,
+    /// The active EC ID, carried independently from [`PullSyncContext`] so
+    /// that FP signal writes succeed even when `client_ip` is unavailable
+    /// (which makes `PullSyncContext` `None`).
+    ec_id: Option<String>,
     fp_signals: Vec<FpSignal>,
     fp_signal_configs: Vec<FpSignalPartnerConfig>,
 }
@@ -189,6 +192,7 @@ async fn route_request(
         return Ok(RouteOutcome {
             response,
             pull_sync_context: None,
+            ec_id: None,
             fp_signals: vec![],
             fp_signal_configs: vec![],
         });
@@ -203,6 +207,7 @@ async fn route_request(
                 return Ok(RouteOutcome {
                     response,
                     pull_sync_context: None,
+                    ec_id: None,
                     fp_signals: vec![],
                     fp_signal_configs: vec![],
                 });
@@ -233,6 +238,7 @@ async fn route_request(
             return Ok(RouteOutcome {
                 response,
                 pull_sync_context: None,
+                ec_id: None,
                 fp_signals: vec![],
                 fp_signal_configs: vec![],
             });
@@ -245,6 +251,7 @@ async fn route_request(
             return Ok(RouteOutcome {
                 response,
                 pull_sync_context: None,
+                ec_id: None,
                 fp_signals: vec![],
                 fp_signal_configs: vec![],
             });
@@ -392,10 +399,7 @@ async fn route_request(
                 if configs.is_empty() {
                     (vec![], vec![])
                 } else {
-                    let now_ms = std::time::SystemTime::now()
-                        .duration_since(std::time::UNIX_EPOCH)
-                        .map(|d| d.as_millis() as u64)
-                        .unwrap_or(0);
+                    let now_ms = current_timestamp_ms();
                     let signals = ec_context
                         .cookie_jar()
                         .map(|j| extract_fp_signals(j, &configs, now_ms))
@@ -415,9 +419,12 @@ async fn route_request(
         None
     };
 
+    let ec_id = ec_context.ec_value().map(str::to_owned);
+
     Ok(RouteOutcome {
         response,
         pull_sync_context,
+        ec_id,
         fp_signals,
         fp_signal_configs,
     })

crates/trusted-server-core/src/ec/fp_signals.rs

@@ -18,7 +18,7 @@ use error_stack::{Report, ResultExt};
 use serde::{Deserialize, Serialize};
 
 use super::kv::{CasWriteOutcome, KvIdentityGraph};
-use super::kv_types::KvPartnerId;
+use super::kv_types::{KvPartnerId, MAX_UID_LENGTH};
 use super::log_id;
 
 /// Per-partner configuration for first-party signal extraction.
@@ -184,6 +184,15 @@ pub fn extract_fp_signals(
             continue;
         }
 
+        if uid.len() > MAX_UID_LENGTH {
+            log::debug!(
+                "Skipping fp_signal for partner '{}': UID exceeds {MAX_UID_LENGTH} bytes (got {})",
+                config.partner_id,
+                uid.len(),
+            );
+            continue;
+        }
+
         signals.push(FpSignal {
             partner_id: config.partner_id.clone(),
             uid,
@@ -216,6 +225,9 @@ fn filter_stale_signals<'a>(
     signals
         .iter()
         .filter(|signal| {
+            // A missing TTL means the partner config was removed between
+            // extraction and write. Default to 0 (always refresh) since
+            // the signal was legitimately extracted with a valid config.
             let ttl = ttl_by_partner
                 .get(signal.partner_id.as_str())
                 .copied()
@@ -869,6 +881,42 @@ mod tests {
         );
     }
 
+    #[test]
+    fn extract_fp_signals_skips_uid_exceeding_max_length() {
+        // Arrange — cookie value exceeds MAX_UID_LENGTH (512 bytes)
+        let long_uid = "x".repeat(MAX_UID_LENGTH + 1);
+        let jar = jar_with(&[("partner_cookie", &long_uid)]);
+        let configs = [raw_config("partner", &["partner_cookie"])];
+
+        // Act
+        let signals = extract_fp_signals(&jar, &configs, 0);
+
+        // Assert
+        assert!(
+            signals.is_empty(),
+            "should skip signal with UID exceeding MAX_UID_LENGTH"
+        );
+    }
+
+    #[test]
+    fn extract_fp_signals_accepts_uid_at_max_length() {
+        // Arrange — cookie value is exactly MAX_UID_LENGTH (512 bytes)
+        let uid = "x".repeat(MAX_UID_LENGTH);
+        let jar = jar_with(&[("partner_cookie", &uid)]);
+        let configs = [raw_config("partner", &["partner_cookie"])];
+
+        // Act
+        let signals = extract_fp_signals(&jar, &configs, 0);
+
+        // Assert
+        assert_eq!(
+            signals.len(),
+            1,
+            "should accept signal at exactly MAX_UID_LENGTH"
+        );
+        assert_eq!(signals[0].uid, uid, "should use the full UID value");
+    }
+
     #[test]
     fn extract_fp_signals_raw_config_with_expired_identity_expires_skipped() {
         // Arrange — a partner with json_path: None whose cookie value happens to be

crates/trusted-server-core/src/ec/mod.rs

@@ -495,6 +495,21 @@ pub(crate) fn current_timestamp() -> u64 {
         })
 }
 
+/// Returns the current Unix timestamp in milliseconds.
+///
+/// Uses `std::time::SystemTime` which is supported on `wasm32-wasip1`.
+/// The `as u64` narrowing from `u128` is safe for ~584 million years.
+#[must_use]
+pub fn current_timestamp_ms() -> u64 {
+    std::time::SystemTime::now()
+        .duration_since(std::time::UNIX_EPOCH)
+        .map(|d| d.as_millis() as u64)
+        .unwrap_or_else(|err| {
+            log::error!("SystemTime::now() failed, falling back to epoch 0: {err}");
+            0
+        })
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

crates/trusted-server-core/src/ec/partner.rs

@@ -8,9 +8,10 @@
 //! - `_pull_enabled` — JSON array of partner IDs with `pull_sync_enabled:
 //!   true`, enabling O(1+N) reads on the pull sync hot path instead of a
 //!   full partner scan.
-//! - `_fp_signal_enabled` — JSON array of partner IDs with
-//!   `fp_signal_enabled: true`, enabling O(1+N) reads for first-party
-//!   signal collection config without a full partner scan.
+//! - `_fp_signal_enabled` — JSON array of partner configs for partners
+//!   with at least one `fp_signal_cookie_names` entry, enabling O(1+N)
+//!   reads for first-party signal collection config without a full
+//!   partner scan.
 
 use std::{collections::HashSet, sync::OnceLock};
 
@@ -257,10 +258,17 @@ pub fn validate_fp_signal_config(record: &PartnerRecord) -> Result<(), String> {
         ));
     }
 
+    const MAX_COOKIE_NAME_LENGTH: usize = 128;
     for name in &record.fp_signal_cookie_names {
         if name.is_empty() {
             return Err("fp_signal_cookie_names entries must not be empty".to_owned());
         }
+        if name.len() > MAX_COOKIE_NAME_LENGTH {
+            return Err(format!(
+                "fp_signal_cookie_names entry must be at most {MAX_COOKIE_NAME_LENGTH} characters, got {}",
+                name.len()
+            ));
+        }
         if !name.is_ascii() {
             return Err(format!(
                 "fp_signal_cookie_names entry '{name}' must contain only ASCII characters"
@@ -323,9 +331,9 @@ pub fn hash_api_key(api_key: &str) -> String {
 ///   O(1) auth lookups during batch sync.
 /// - `_pull_enabled` stores a JSON array of partner IDs with
 ///   `pull_sync_enabled: true` for O(1+N) reads during pull sync dispatch.
-/// - `_fp_signal_enabled` stores a JSON array of partner IDs with
-///   `fp_signal_enabled: true` for O(1+N) reads during first-party signal
-///   collection.
+/// - `_fp_signal_enabled` stores a JSON array of partner configs for
+///   partners with at least one `fp_signal_cookie_names` entry for
+///   O(1+N) reads during first-party signal collection.
 pub struct PartnerStore {
     store_name: String,
 }
@@ -1314,4 +1322,16 @@ mod tests {
             "should mention 4-segment limit, got: {err}"
         );
     }
+
+    #[test]
+    fn validate_fp_signal_rejects_cookie_name_too_long() {
+        let mut record = base_fp_signal_record();
+        record.fp_signal_cookie_names = vec!["x".repeat(129)];
+        let err = validate_fp_signal_config(&record)
+            .expect_err("should reject cookie name exceeding 128 chars");
+        assert!(
+            err.contains("at most 128"),
+            "should mention 128-char limit, got: {err}"
+        );
+    }
 }

docs/superpowers/plans/2026-04-09-first-party-signal-collection.md

@@ -12,19 +12,20 @@
 
 ## File Structure
 
-| File | Responsibility |
-|------|---------------|
-| `crates/trusted-server-core/src/ec/fp_signals.rs` | **New.** Types (`FpSignalPartnerConfig`, `FpSignal`, `FpSignalError`), extraction logic, JSON path walker, batched CAS write, all unit tests. |
-| `crates/trusted-server-core/src/ec/mod.rs` | Declare `pub mod fp_signals`. Add `cookie_jar` field to `EcContext`, store it during `read_from_request_with_geo`, expose via `cookie_jar()` accessor. |
-| `crates/trusted-server-core/src/ec/partner.rs` | Add 3 FP signal fields to `PartnerRecord`. Add `FP_SIGNAL_INDEX_KEY` constant. Add `fp_signal_configs()` accessor. Add `update_fp_signal_index()` and call it from `upsert()`. Skip new index key in `list_registered()`. Add validation function. |
-| `crates/trusted-server-core/src/ec/admin.rs` | Add 3 FP signal fields to `RegisterPartnerRequest`. Wire them into `PartnerRecord` construction and validation. |
-| `crates/trusted-server-adapter-fastly/src/main.rs` | Expand `RouteOutcome` to carry `fp_signals` and `fp_signal_configs`. Extract signals pre-send. Write signals post-send via `run_fp_signal_collection_after_send()`. |
+| File                                               | Responsibility                                                                                                                                                                                                                                     |
+| -------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
+| `crates/trusted-server-core/src/ec/fp_signals.rs`  | **New.** Types (`FpSignalPartnerConfig`, `FpSignal`, `FpSignalError`), extraction logic, JSON path walker, batched CAS write, all unit tests.                                                                                                      |
+| `crates/trusted-server-core/src/ec/mod.rs`         | Declare `pub mod fp_signals`. Add `cookie_jar` field to `EcContext`, store it during `read_from_request_with_geo`, expose via `cookie_jar()` accessor.                                                                                             |
+| `crates/trusted-server-core/src/ec/partner.rs`     | Add 3 FP signal fields to `PartnerRecord`. Add `FP_SIGNAL_INDEX_KEY` constant. Add `fp_signal_configs()` accessor. Add `update_fp_signal_index()` and call it from `upsert()`. Skip new index key in `list_registered()`. Add validation function. |
+| `crates/trusted-server-core/src/ec/admin.rs`       | Add 3 FP signal fields to `RegisterPartnerRequest`. Wire them into `PartnerRecord` construction and validation.                                                                                                                                    |
+| `crates/trusted-server-adapter-fastly/src/main.rs` | Expand `RouteOutcome` to carry `fp_signals` and `fp_signal_configs`. Extract signals pre-send. Write signals post-send via `run_fp_signal_collection_after_send()`.                                                                                |
 
 ---
 
 ### Task 1: Add `FpSignalPartnerConfig` and `FpSignal` types
 
 **Files:**
+
 - Create: `crates/trusted-server-core/src/ec/fp_signals.rs`
 - Modify: `crates/trusted-server-core/src/ec/mod.rs:30-43`
 
@@ -108,6 +109,7 @@ git commit -m "Add FpSignalPartnerConfig, FpSignal, and FpSignalError types"
 ### Task 2: Implement JSON path extraction with tests
 
 **Files:**
+
 - Modify: `crates/trusted-server-core/src/ec/fp_signals.rs`
 - Test: `crates/trusted-server-core/src/ec/fp_signals.rs` (inline `#[cfg(test)]`)
 
@@ -236,6 +238,7 @@ git commit -m "Implement JSON path extraction for first-party signal cookies"
 ### Task 3: Implement `extract_fp_signals` with tests
 
 **Files:**
+
 - Modify: `crates/trusted-server-core/src/ec/fp_signals.rs`
 
 - [ ] **Step 1: Write failing tests for cookie extraction**
@@ -518,6 +521,7 @@ git commit -m "Implement first-party signal extraction from cookie jar"
 ### Task 4: Add UID2 expiry tests
 
 **Files:**
+
 - Modify: `crates/trusted-server-core/src/ec/fp_signals.rs`
 
 - [ ] **Step 1: Write UID2 expiry tests**
@@ -603,6 +607,7 @@ git commit -m "Add UID2 expiry check tests for first-party signal extraction"
 ### Task 5: Expose `CookieJar` from `EcContext`
 
 **Files:**
+
 - Modify: `crates/trusted-server-core/src/ec/mod.rs:139-161` (struct), `193-238` (read_from_request_with_geo), `407-464` (test helpers)
 
 - [ ] **Step 1: Add `cookie_jar` field to `EcContext` struct**
@@ -659,6 +664,7 @@ git commit -m "Expose parsed CookieJar from EcContext for signal extraction"
 ### Task 6: Add FP signal fields to `PartnerRecord`
 
 **Files:**
+
 - Modify: `crates/trusted-server-core/src/ec/partner.rs:62-115` (struct), `707-984` (tests)
 
 - [ ] **Step 1: Add 3 fields to `PartnerRecord`**
@@ -721,6 +727,7 @@ git commit -m "Add first-party signal fields to PartnerRecord"
 ### Task 7: Add FP signal validation
 
 **Files:**
+
 - Modify: `crates/trusted-server-core/src/ec/partner.rs`
 
 - [ ] **Step 1: Write failing validation tests**
@@ -1012,6 +1019,7 @@ git commit -m "Add first-party signal validation for PartnerRecord"
 ### Task 8: Add `_fp_signal_enabled` index to `PartnerStore`
 
 **Files:**
+
 - Modify: `crates/trusted-server-core/src/ec/partner.rs`
 
 - [ ] **Step 1: Add the index constant and skip it in `list_registered`**
@@ -1162,6 +1170,7 @@ git commit -m "Add _fp_signal_enabled index to PartnerStore"
 ### Task 9: Add FP signal fields to admin registration endpoint
 
 **Files:**
+
 - Modify: `crates/trusted-server-core/src/ec/admin.rs`
 
 - [ ] **Step 1: Add fields to `RegisterPartnerRequest`**
@@ -1261,6 +1270,7 @@ git commit -m "Add first-party signal fields to partner registration endpoint"
 ### Task 10: Implement `write_fp_signals` with tests
 
 **Files:**
+
 - Modify: `crates/trusted-server-core/src/ec/fp_signals.rs`
 
 - [ ] **Step 1: Write batched write tests**
@@ -1628,6 +1638,7 @@ git commit -m "Implement batched CAS write for first-party signals"
 ### Task 11: Wire extraction and writing into the adapter
 
 **Files:**
+
 - Modify: `crates/trusted-server-adapter-fastly/src/main.rs`
 
 - [ ] **Step 1: Update imports**