From a938b8c305dc55bb19e872fd60069d7c6733f12f Mon Sep 17 00:00:00 2001 From: Tyler Gillispie Date: Sun, 23 Aug 2026 20:59:48 -0700 Subject: [PATCH] docs(tasks): add Task Critique & Plan Review Template to tasks/META_PROMPT.md --- tasks/META_PROMPT.md | 27 ++++++ ...1.jul.feat.spire-ffi.native-argon2-0352.md | 86 ++++++++++++++----- ...feat.sdk.realtime-invalidation-bus-1200.md | 60 ++++++++++--- 3 files changed, 142 insertions(+), 31 deletions(-) diff --git a/tasks/META_PROMPT.md b/tasks/META_PROMPT.md index 192f45a..d6cd9f3 100644 --- a/tasks/META_PROMPT.md +++ b/tasks/META_PROMPT.md @@ -68,3 +68,30 @@ audit._ 3. Confirm all unit and integration tests pass with 0 lint warnings and 0 typecheck errors. 4. Document findings and generate an audit report if any vulnerabilities or performance bottlenecks are detected. ``` + +--- + +## 4. Task Plan Review & Critique Template (Architect / Critic) + +_Use this template to critically audit, score, and refine new task files in +`tasks/new/` before approving them for implementation._ + +```text +**Role:** Act as a Principal Systems Architect and Task Quality Critic. + +**The Scope:** Review the newly generated task file in `tasks/new/[TASK_FILENAME].md`. + +**Your Task:** +Critically evaluate the proposed task plan against the following 5-point quality rubric: +1. **Metadata & Standards Compliance:** Does the file strictly adhere to `tasks/GUIDELINES.md` naming conventions and include the exact `# TASK METADATA` header? +2. **Architectural Boundary Safety:** Does the plan identify genuine technical risks (e.g., event loop blocking, memory leaks, cache staleness, zero-trust perimeter bypasses) and provide concrete mitigations? +3. **Alternatives & Zero-Dependency Purity:** Did the author evaluate simpler or more native alternatives and justify why the proposed design avoids dependency bloat? +4. **Implementation Precision:** Are target functions, data structures, SQL migrations, FFI symbols, and error boundaries defined with crystal clarity? +5. **Testing & Quality Gate Rigor:** Does the proposed implementation include comprehensive unit/integration test specifications covering both happy-path and failure modes? + +**Deliverable:** +Provide a structured critique report with: +- **Verdict:** [APPROVED / NEEDS REVISION / REJECTED] +- **Strengths:** Key architectural insights captured by the author. +- **Identified Gaps & Refinements:** Concrete adjustments to incorporate into the task file before implementation starts. +``` diff --git a/tasks/new/2026-0824.01.jul.feat.spire-ffi.native-argon2-0352.md b/tasks/new/2026-0824.01.jul.feat.spire-ffi.native-argon2-0352.md index e761502..471180e 100644 --- a/tasks/new/2026-0824.01.jul.feat.spire-ffi.native-argon2-0352.md +++ b/tasks/new/2026-0824.01.jul.feat.spire-ffi.native-argon2-0352.md @@ -1,46 +1,92 @@ # TASK METADATA -- **Target Files:** `spire_ffi/Cargo.toml`, `spire_ffi/src/lib.rs`, `server/spire_ffi.ts`, `server/spire_ffi.test.ts` -- **Core Objective:** Implement high-throughput native Argon2id derivation (12 iterations, 64 MiB memory, 128-bit salt, 256-bit output key) via the `spire_ffi` Rust crate to accelerate zero-knowledge hashing. -- **Dependencies:** The Deno application must be able to compile or load the Rust dynamic library (`libspire_ffi.so` / `spire_ffi.dll` / `libspire_ffi.dylib`). -- **Additional Important Notes:** Fallback behavior for local dev/testing must mimic the existing pattern in `spire_ffi.ts`, returning deterministic mock data when the library is unavailable, but throwing a fatal error in production contexts. +- **Target Files:** `spire_ffi/Cargo.toml`, `spire_ffi/src/lib.rs`, + `server/spire_ffi.ts`, `server/spire_ffi.test.ts` +- **Core Objective:** Implement high-throughput native Argon2id derivation (12 + iterations, 64 MiB memory, 128-bit salt, 256-bit output key) via the + `spire_ffi` Rust crate to accelerate zero-knowledge hashing. +- **Dependencies:** The Deno application must be able to compile or load the + Rust dynamic library (`libspire_ffi.so` / `spire_ffi.dll` / + `libspire_ffi.dylib`). +- **Additional Important Notes:** Fallback behavior for local dev/testing must + mimic the existing pattern in `spire_ffi.ts`, returning deterministic mock + data when the library is unavailable, but throwing a fatal error in production + contexts. --- ## Architectural Considerations & Risks ### Risks -- **FFI Memory Safety:** Exposing a new C-compatible function via FFI (`argon2id_derive`) involves raw pointer manipulation. Incorrectly sizing the output buffer, failing to free memory properly, or reading past the bounds of input buffers (password/salt) could lead to segfaults or memory leaks. -- **Platform Compatibility:** Relying on native code means the target environment must support the compiled architecture. The CI/CD pipeline must build the Rust crate for all supported deployment targets (x86_64, aarch64). -- **Blocking the Event Loop:** Hashing with Argon2id is intentionally slow and CPU-intensive. If the Deno FFI call is blocking, it will stall the Deno event loop. The FFI function must be marked as `nonblocking: true` in `Deno.dlopen`, and the Rust implementation must be executed asynchronously or off the main thread. + +- **FFI Memory Safety:** Exposing a new C-compatible function via FFI + (`argon2id_derive`) involves raw pointer manipulation. Incorrectly sizing the + output buffer, failing to free memory properly, or reading past the bounds of + input buffers (password/salt) could lead to segfaults or memory leaks. +- **Platform Compatibility:** Relying on native code means the target + environment must support the compiled architecture. The CI/CD pipeline must + build the Rust crate for all supported deployment targets (x86_64, aarch64). +- **Blocking the Event Loop:** Hashing with Argon2id is intentionally slow and + CPU-intensive. If the Deno FFI call is blocking, it will stall the Deno event + loop. The FFI function must be marked as `nonblocking: true` in `Deno.dlopen`, + and the Rust implementation must be executed asynchronously or off the main + thread. ### Alternatives -- **WASM (e.g., `hash-wasm`):** An alternative is to use WebAssembly to perform the hashing directly in Deno without FFI. - - *Justification against:* Native SIMD provides 3–5x greater throughput compared to V8 WebAssembly for 64 MiB memory-hard hashing. Furthermore, we already have and maintain the `spire_ffi` Rust workspace. Avoiding a new npm dependency keeps the server and `@auth-yes/sdk` zero-dependency, adhering to our core architectural goals. + +- **WASM (e.g., `hash-wasm`):** An alternative is to use WebAssembly to perform + the hashing directly in Deno without FFI. + - _Justification against:_ Native SIMD provides 3–5x greater throughput + compared to V8 WebAssembly for 64 MiB memory-hard hashing. Furthermore, we + already have and maintain the `spire_ffi` Rust workspace. Avoiding a new npm + dependency keeps the server and `@auth-yes/sdk` zero-dependency, adhering to + our core architectural goals. ## Proposed Implementation ### 1. Update Rust Dependencies + - Add the `argon2` crate to `spire_ffi/Cargo.toml`: ```toml [dependencies] argon2 = { version = "0.5", features = ["std"] } ``` -- *Note on SIMD:* Version 0.5.x automatically uses runtime CPU feature detection (e.g., `std::is_x86_feature_detected!("avx2")`). For portable builds, use standard `cargo build --release`. For optional optimized container builds, document the use of `RUSTFLAGS="-C target-cpu=native" cargo build --release`. +- _Note on SIMD:_ Version 0.5.x automatically uses runtime CPU feature detection + (e.g., `std::is_x86_feature_detected!("avx2")`). For portable builds, use + standard `cargo build --release`. For optional optimized container builds, + document the use of `RUSTFLAGS="-C target-cpu=native" cargo build --release`. ### 2. Implement Rust FFI Export -- In `spire_ffi/src/lib.rs`, create a new `#[no_mangle]` extern "C" function named `argon2id_derive`. -- The function signature should accept pointers and lengths for the password and salt, iteration count, memory size (in KB), and a pointer/length for the output buffer. -- Use the `argon2` crate to compute the hash and write the result into the provided output buffer. Ensure all FFI safety boundaries are respected (e.g., checking for null pointers). + +- In `spire_ffi/src/lib.rs`, create a new `#[no_mangle]` extern "C" function + named `argon2id_derive`. +- The function signature should accept pointers and lengths for the password and + salt, iteration count, memory size (in KB), and a pointer/length for the + output buffer. +- Use the `argon2` crate to compute the hash and write the result into the + provided output buffer. Ensure all FFI safety boundaries are respected (e.g., + checking for null pointers). ### 3. Expose via Deno FFI (`server/spire_ffi.ts`) -- Add the `argon2id_derive` symbol to the `Deno.dlopen` definition. Critically, set `nonblocking: true` to prevent stalling the Deno event loop during hash computation. -- Create an exported async function `deriveArgon2idKey(password: Uint8Array, salt: Uint8Array): Promise`. -- Hardcode the security parameters: 12 iterations, 64 MiB (65536 KB) memory, and a 32-byte (256-bit) output key length. -- Implement the fallback pattern: If `dylib` is null, log `[SPIRE FFI] Dynamic library (...) is not loaded. Mocking Argon2id derivation for local development.` and return a deterministic 32-byte mock buffer (e.g., filled with a repeating pattern). + +- Add the `argon2id_derive` symbol to the `Deno.dlopen` definition. Critically, + set `nonblocking: true` to prevent stalling the Deno event loop during hash + computation. +- Create an exported async function + `deriveArgon2idKey(password: Uint8Array, salt: Uint8Array): Promise`. +- Hardcode the security parameters: 12 iterations, 64 MiB (65536 KB) memory, and + a 32-byte (256-bit) output key length. +- Implement the fallback pattern: If `dylib` is null, log + `[SPIRE FFI] Dynamic library (...) is not loaded. Mocking Argon2id derivation for local development.` + and return a deterministic 32-byte mock buffer (e.g., filled with a repeating + pattern). ### 4. Write Unit Tests + - In `server/spire_ffi.test.ts`, write tests to ensure: - - The fallback mechanism works when the library isn't loaded (returns the mock buffer). - - When the library *is* available, the function successfully returns a 32-byte Uint8Array. - - (Optional) Verify the output matches a known standard Argon2id test vector to confirm correctness across the FFI boundary. \ No newline at end of file + - The fallback mechanism works when the library isn't loaded (returns the mock + buffer). + - When the library _is_ available, the function successfully returns a 32-byte + Uint8Array. + - (Optional) Verify the output matches a known standard Argon2id test vector + to confirm correctness across the FFI boundary. diff --git a/tasks/new/2026-0824.02.jul.feat.sdk.realtime-invalidation-bus-1200.md b/tasks/new/2026-0824.02.jul.feat.sdk.realtime-invalidation-bus-1200.md index dd6c8ac..f4b310d 100644 --- a/tasks/new/2026-0824.02.jul.feat.sdk.realtime-invalidation-bus-1200.md +++ b/tasks/new/2026-0824.02.jul.feat.sdk.realtime-invalidation-bus-1200.md @@ -1,33 +1,61 @@ # TASK METADATA - **Target Files:** `sdk/mod.ts`, `sdk/mod.test.ts` -- **Core Objective:** Extend `@auth-yes/sdk` with a zero-dependency real-time event bus to listen for Valkey 8 RESP3 push invalidation events and emit them to registered listeners. +- **Core Objective:** Extend `@auth-yes/sdk` with a zero-dependency real-time + event bus to listen for Valkey 8 RESP3 push invalidation events and emit them + to registered listeners. - **Dependencies:** None. -- **Additional Important Notes:** Handlers must support async execution. Errors thrown by handlers must be caught and logged so they do not crash the Valkey push listener or stop other registered handlers. No external EventEmitter libraries are allowed. +- **Additional Important Notes:** Handlers must support async execution. Errors + thrown by handlers must be caught and logged so they do not crash the Valkey + push listener or stop other registered handlers. No external EventEmitter + libraries are allowed. --- ## Architectural Considerations & Risks ### Risks -- **Memory Leaks:** Downstream consumers registering listeners for `"invalidate"` without subsequently calling `off` could result in an unbounded map of listeners. We must clearly document the `off` method and emphasize its usage during cleanup (e.g., when a consumer class is destroyed or a connection pool closes). -- **Callback Crash Propagation:** If a downstream consumer's callback throws an exception (e.g., failing to terminate a WebSocket), and it is not caught, it could crash the internal Valkey `push` event handler. The implementation must guarantee isolated execution via a `try/catch` wrapper and gracefully handle Promise rejections. -- **Payload Alignment:** Any discrepancy between the raw key emitted by Valkey and the expected token by downstream listeners could result in missed invalidations. As confirmed, Valkey stores and emits the raw `token` without prefixes, so we must emit the token strictly as received. + +- **Memory Leaks:** Downstream consumers registering listeners for + `"invalidate"` without subsequently calling `off` could result in an unbounded + map of listeners. We must clearly document the `off` method and emphasize its + usage during cleanup (e.g., when a consumer class is destroyed or a connection + pool closes). +- **Callback Crash Propagation:** If a downstream consumer's callback throws an + exception (e.g., failing to terminate a WebSocket), and it is not caught, it + could crash the internal Valkey `push` event handler. The implementation must + guarantee isolated execution via a `try/catch` wrapper and gracefully handle + Promise rejections. +- **Payload Alignment:** Any discrepancy between the raw key emitted by Valkey + and the expected token by downstream listeners could result in missed + invalidations. As confirmed, Valkey stores and emits the raw `token` without + prefixes, so we must emit the token strictly as received. ### Alternatives -- Using the web standard `EventTarget` natively supported in Deno. While native, standard `EventTarget` limits the payload to custom `Event` objects (`CustomEvent`), which adds boilerplate (`event.detail.token`) and type-casting complexity for downstream users. A custom lightweight callback Set (`Set`) allows us to pass the `token: string` payload directly and cleanly. -- Using an external EventEmitter package. This was rejected per the zero-dependency directive. + +- Using the web standard `EventTarget` natively supported in Deno. While native, + standard `EventTarget` limits the payload to custom `Event` objects + (`CustomEvent`), which adds boilerplate (`event.detail.token`) and + type-casting complexity for downstream users. A custom lightweight callback + Set (`Set`) allows us to pass the `token: string` payload + directly and cleanly. +- Using an external EventEmitter package. This was rejected per the + zero-dependency directive. ## Proposed Implementation ### 1. Define the Handler Type + In `sdk/mod.ts`, define the TypeScript type for the invalidation handler: + ```typescript export type InvalidationHandler = (token: string) => void | Promise; ``` ### 2. Implement the Internal Event Bus + In the `AuthSdk` class (`sdk/mod.ts`): + - Add a private property to store listeners: ```typescript private listeners: Map> = new Map(); @@ -55,15 +83,25 @@ In the `AuthSdk` class (`sdk/mod.ts`): ``` ### 3. Integrate with Valkey `push` Events + In the `initValkeyClient` method of `AuthSdk` (`sdk/mod.ts`): + - Locate the existing `"push"` event listener for the Valkey client. -- Inside the loop that iterates over `keysToInvalidate` (which represents the raw tokens), add the emission logic. +- Inside the loop that iterates over `keysToInvalidate` (which represents the + raw tokens), add the emission logic. - Retrieve the `Set` of listeners for the `"invalidate"` event. -- Iterate over the handlers and invoke them safely using a `try/catch` block and `Promise.resolve(...).catch(...)` to prevent any single failing handler from impacting the others or crashing the SDK. +- Iterate over the handlers and invoke them safely using a `try/catch` block and + `Promise.resolve(...).catch(...)` to prevent any single failing handler from + impacting the others or crashing the SDK. ### 4. Write Unit Tests + In `sdk/mod.test.ts`: + - Write tests to verify that `on` and `off` correctly add and remove handlers. -- Simulate the Valkey `"push"` event (e.g., by creating an instance with a mocked `valkeyClient` or directly testing an internal emit method, or sending mock payloads if the Valkey client is mocked). +- Simulate the Valkey `"push"` event (e.g., by creating an instance with a + mocked `valkeyClient` or directly testing an internal emit method, or sending + mock payloads if the Valkey client is mocked). - Verify that registered listeners are invoked with the correct token. -- Verify that a throwing or rejecting handler does not disrupt the execution of subsequent handlers. +- Verify that a throwing or rejecting handler does not disrupt the execution of + subsequent handlers.