565 ai readiness #566
565 ai readiness #566
Conversation
WalkthroughAdds many documentation and skill files; implements a built-in "code" destination and associated types; threads scoped loggers and richer Destination.Context through the collector; introduces AWS Lambda and Fetch server sources with schemas/utilities and tests; refactors the CLI bundler from template-based to programmatic generation; updates demos, tests, dev tooling, and logging integration across runner/serve/docker. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client as Client
participant Collector as Collector
participant Destination as Destination (Code or Impl)
participant Logger as Logger (scoped)
Client->>Collector: push(event)
Collector->>Logger: logger.scope(destination.type) -> scopedLogger
Collector->>Destination: init? (context { config, env, collector, logger: scopedLogger })
Destination-->>Collector: init complete
Collector->>Logger: scopedLogger.debug("push" / "push batch")
Collector->>Destination: push(event|batch, context { ..., logger: scopedLogger })
Destination-->>Collector: result / error
Collector->>Logger: scopedLogger.debug("push done" / "push batch done")
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Demo flows
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/collector/src/destination.ts (1)
105-125: Skipped events are not actually re-queued (concatresult ignored)
destination.queue.concat(skippedEvents);does not mutatedestination.queue; it returns a new array that is discarded, so events filtered out by consent are never re-added to the destination queue despite the comment.Consider fixing this while you’re in this module:
- // Add skipped events back to the queue - destination.queue.concat(skippedEvents); + // Add skipped events back to the queue + destination.queue = (destination.queue || []).concat(skippedEvents);This issue appears pre-existing but can lead to silently dropped, consent-denied events that you may intend to preserve for later processing.
🧹 Nitpick comments (11)
turbo.json (1)
4-4: Concurrency reduction impacts overall build/test throughputSetting
"concurrency": "1"effectively serializes Turbo tasks. This is fine if motivated by CI/devcontainer resource limits, but it will noticeably slow builds/tests. If you expect higher local parallelism, consider documenting this choice or overriding via env/CLI for power users.packages/config/jest/index.mjs (1)
100-104: Jest parallelism and shutdown behavior changes look goodReducing
maxWorkersto2and explicitly documenting thatforceExitis disabled aligns with best practices (let Jest surface open-handle issues instead of forcing exit) and helps with devcontainer memory limits. If test runtimes become long on more powerful machines, you could later exposemaxWorkersvia an env var so local runs can opt into higher parallelism.skills/understanding-flow/SKILL.md (1)
21-29: Add a language to the ASCII flow diagram fence to satisfy markdownlintThe fenced block showing the Source → Collector → Destinations diagram lacks a language, which triggers MD040. You can keep the rendering identical and appease tooling by labeling it, e.g.:
-``` +```text Sources → Collector → Destinations (Data Capture) (Processing) (Delivery) ...</blockquote></details> <details> <summary>skills/create-destination/SKILL.md (1)</summary><blockquote> `25-33`: **Label non-code fences (`text`) to satisfy markdownlint MD040** A few fenced blocks are prose/ASCII (process steps, mapping trace, directory tree) and currently have no language, which trips MD040: - Process overview (Phase 2) block around line 25 - Mapping trace example around line 270 - Directory structure tree around line 294 You can keep them exactly as-is visually and fix lint by labeling them as text: ```diff -``` +```text 1. Research → Find SDK, understand vendor API ...Repeat similarly for the other two blocks. Also applies to: 270-277, 294-315 </blockquote></details> <details> <summary>skills/understanding-mapping/SKILL.md (1)</summary><blockquote> `38-55`: **Tighten mapping examples (duplicate key & minor TS syntax nit)** Two small doc nits that would make these examples more copy/paste-friendly: 1. **Duplicate `product` key in the mapping example** In the `const mapping = { ... }` block, `product` is declared twice; in JS/TS the latter wins, so the `view`/`add` rules would be shadowed. Consider merging into a single `product` object: ```typescript const mapping = { product: { // Exact matches view: { name: 'view_item' }, add: { name: 'add_to_cart' }, // Wildcard: any action '*': { name: 'product_interaction' }, }, // Wildcard: any entity '*': { click: { name: 'generic_click' }, }, };
Array loop example trailing
;
The array loop example ends the object with];, which is invalid TS if copied directly. Dropping the semicolon keeps it valid:{ loop: [ 'nested', { map: { /* ... */ } }, ]
- ];
- }
These are purely documentation cleanups; the conceptual mapping guidance itself looks solid.
Also applies to: 135-145
skills/create-source/SKILL.md (2)
509-513: Avoidas anyin example tests to reflect “no any” guideline.The test example for invalid input casts with
as any. Since AGENT explicitly calls out “Noanyin production code”, it would be clearer if the example also avoided it—for instance by widening theInputtype for test-only cases, or constructing an explicitly typed invalid object. That keeps the skill aligned with the repository’s TypeScript guidance.
375-382: Either wiremappingintotransformInputor clarify that the example is name‑mapping only.The earlier section defines a
Mapping.Rulesconfiguration and importsgetMappingValue, but thetransformInputexample doesn’t useconfig.mappingorgetMappingValueand just forwardsinput.propertiesintodata. To reduce confusion for readers:
- Either show a minimal example of applying
config.mappingwithgetMappingValuewhen buildingdata, or- Remove the unused
getMappingValueimport and add a short note that this transform focuses only on event naming and basic field passthrough.Right now it reads as if mapping should be applied, but the sample implementation never does.
Also applies to: 399-427, 268-307
packages/server/destinations/meta/src/push.ts (1)
13-16: Logger integration and endpoint refactor look solid.The updated
PushFnsignature, extractedendpointvariable, and scopedlogger.debug/logger.throwusage align with the destination patterns elsewhere and improve observability without changing behavior. Usingenv?.sendServer || sendServerkeeps it testable.If you want richer debugging later, you could extend the response log to include status/body when available, but that’s not required for this change.
Also applies to: 72-92
packages/server/destinations/aws/src/firehose/__tests__/firehose.test.ts (1)
2-4: Tests correctly adopt logger/context pattern; minor setup cleanup possible.Using
import type { Collector }and passinglogger: createMockLogger()into bothinitandpushmatches the new destination push context and keeps types clean.You can optionally tidy the test setup by:
- Removing the unused top-level
mockCollectordeclaration (Line 18) and relying on the local one in thepushtest, and- Dropping the unused
callsarray if it’s no longer inspected.These are non-functional, just to reduce noise in the spec.
Also applies to: 20-28, 82-91
skills/using-logger/SKILL.md (1)
1-334: Update “Needs Improvement” list and keep logger interface in sync with core typesThe overall guidance here (scoped logger, avoiding duplicate init/push logs, focusing on external calls/validation) matches the patterns in AGENT.md and the new destination changes in this PR. Based on learnings, this is the right direction.
Two adjustments to keep it from drifting:
- Outdated “Needs Improvement” entries
This PR adds structured logging to both Meta and AWS Firehose destinations, so the list under “Needs Improvement” becomes stale immediately:
packages/server/destinations/meta/src/push.tspackages/server/destinations/aws/src/firehose/push.tsConsider trimming that section to only entries that truly still lack logging, e.g.:
-**Needs Improvement:** - -- [packages/server/sources/fetch/src/index.ts](../../packages/server/sources/fetch/src/index.ts) - - Missing all logging -- [packages/server/destinations/meta/src/push.ts](../../packages/server/destinations/meta/src/push.ts) - - Missing push logging -- [packages/server/destinations/aws/src/firehose/push.ts](../../packages/server/destinations/aws/src/firehose/push.ts) - - Missing push logging +**Needs Improvement:** + +- [packages/server/sources/fetch/src/index.ts](../../packages/server/sources/fetch/src/index.ts) - + Missing all logging
- Keep the
Logger.Instanceinterface snippet authoritativeTo avoid future doc drift, please double‑check that the documented
Logger.Instanceshape (methods, argument types, presence ofthrowandscope) exactly matchespackages/core/src/types/logger.ts. If the core type evolves (e.g., new methods), this snippet should be updated in lockstep so it remains a reliable reference.packages/server/destinations/aws/src/firehose/push.ts (1)
4-9: Consider awaitingpushFirehoseto propagate Firehose errors correctly
pushis declared as anasyncPushFn, but the call topushFirehose([{ event }], firehose, context)is not awaited. That means the destination push can resolve before the Firehose client I/O finishes, and any error frompushFirehosewould surface as an unhandled Promise rather than flowing back through the collector.Given other destinations typically await their outbound API calls, I’d recommend:
-export const push: PushFn = async function (event, context) { - const { firehose } = context.config.settings || {}; - - if (firehose) pushFirehose([{ event }], firehose, context); - - return; -}; +export const push: PushFn = async function (event, context) { + const { firehose } = context.config.settings || {}; + + if (firehose) { + await pushFirehose([{ event }], firehose, context); + } +};This keeps the
PushFnreturn type asPromise<void>but ensures Firehose errors are correctly surfaced and that the lifecycle is consistent with other destinations. Please double‑check that this matches how the collector expects destinationpushto behave.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (40)
.claude/skills/create-destination/SKILL.md(1 hunks).claude/skills/create-source/SKILL.md(1 hunks).claude/skills/debugging/SKILL.md(1 hunks).claude/skills/mapping-configuration/SKILL.md(1 hunks).claude/skills/testing-strategy/SKILL.md(1 hunks).claude/skills/understanding-destinations/SKILL.md(1 hunks).claude/skills/understanding-development/SKILL.md(1 hunks).claude/skills/understanding-events/SKILL.md(1 hunks).claude/skills/understanding-flow/SKILL.md(1 hunks).claude/skills/understanding-mapping/SKILL.md(1 hunks).claude/skills/understanding-sources/SKILL.md(1 hunks).claude/skills/using-logger/SKILL.md(1 hunks).devcontainer/devcontainer.json(2 hunks).gitignore(1 hunks)AGENT.md(1 hunks)packages/collector/src/__tests__/destination.test.ts(2 hunks)packages/collector/src/destination.ts(3 hunks)packages/config/jest/index.mjs(1 hunks)packages/core/src/dev.ts(1 hunks)packages/server/destinations/aws/src/firehose/__tests__/firehose.test.ts(1 hunks)packages/server/destinations/aws/src/firehose/lib/firehose.ts(4 hunks)packages/server/destinations/aws/src/firehose/push.ts(1 hunks)packages/server/destinations/datamanager/src/index.ts(1 hunks)packages/server/destinations/datamanager/src/push.ts(0 hunks)packages/server/destinations/gcp/src/bigquery/push.ts(2 hunks)packages/server/destinations/meta/src/push.ts(2 hunks)skills/README.md(1 hunks)skills/create-destination/SKILL.md(1 hunks)skills/create-source/SKILL.md(1 hunks)skills/debugging/SKILL.md(1 hunks)skills/mapping-configuration/SKILL.md(1 hunks)skills/testing-strategy/SKILL.md(1 hunks)skills/understanding-destinations/SKILL.md(1 hunks)skills/understanding-development/SKILL.md(1 hunks)skills/understanding-events/SKILL.md(1 hunks)skills/understanding-flow/SKILL.md(1 hunks)skills/understanding-mapping/SKILL.md(1 hunks)skills/understanding-sources/SKILL.md(1 hunks)skills/using-logger/SKILL.md(1 hunks)turbo.json(1 hunks)
💤 Files with no reviewable changes (1)
- packages/server/destinations/datamanager/src/push.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENT.md)
**/*.{ts,tsx}: NEVER useanytype - always use proper TypeScript types from @walkeros/core or package-specific types
Implement event structure with consistent properties: name (ENTITY ACTION format), data (entity-specific), context (state/environment), globals (global properties), user (identification), nested (related entities), consent, and system-generated fields (id, timestamp, entity, action)
Use core functions from @walkeros/core for common operations: getEvent(), createEvent() for event creation, getMappingEvent(), getMappingValue() for transformations, isString(), isObject(), isDefined() for type checking, assign(), clone() for object operations, tryCatch(), tryCatchAsync() for error handling
Useimport typefor type-only imports to ensure proper TypeScript compilation and tree-shaking
Avoid inline change comments like '// Fixed for version X' or '// Changed due to Y' - Git tracks history, use migrations and documentation for breaking changes instead
Use Root-level Turborepo commands for parallel operations across packages, navigate to individual packages for focused development, and test changes across all affected packages due to core package dependencies
Files:
packages/server/destinations/meta/src/push.tspackages/collector/src/__tests__/destination.test.tspackages/server/destinations/aws/src/firehose/push.tspackages/server/destinations/gcp/src/bigquery/push.tspackages/core/src/dev.tspackages/server/destinations/aws/src/firehose/__tests__/firehose.test.tspackages/server/destinations/datamanager/src/index.tspackages/server/destinations/aws/src/firehose/lib/firehose.tspackages/collector/src/destination.ts
packages/*/src/**/*.ts
📄 CodeRabbit inference engine (AGENT.md)
packages/*/src/**/*.ts: Use Higher-Order Functions (HOF) and smart abstractions to avoid redundancy and keep implementations lean and performant
Implement Destination interface with standard structure: type, init(), push(), pushBatch() methods and config object with optional settings, mapping, data, consent, policy, queue, and dryRun properties
Implement event mapping with flexible patterns: exact entity-action matches, wildcard patterns using '*', conditional mappings with functions, and value mapping strategies including string key extraction, static values, custom functions, object transformation, array processing, and consent-based mapping
Files:
packages/collector/src/__tests__/destination.test.tspackages/core/src/dev.tspackages/collector/src/destination.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENT.md)
**/*.test.{ts,tsx}: Write component-level integration tests with mocked external APIs and boundaries: test destinations by mocking external APIs (gtag, fbq, etc.), test collector with mocked destinations and sources, test sources with mocked collector interface
Mock external APIs in destination tests (gtag, fbq, etc.) and verify they are called correctly with the expected data - focus on verifying external API calls, not return values
Before documenting code patterns, create functional, executable tests to validate them - use /workspaces/walkerOS/apps/quickstart as the source of truth for working examples
Files:
packages/collector/src/__tests__/destination.test.tspackages/server/destinations/aws/src/firehose/__tests__/firehose.test.ts
🧠 Learnings (13)
📓 Common learnings
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T16:24:48.032Z
Learning: Refer to AGENT.md for all relevant instructions and guidelines when working with the walkerOS codebase
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to **/*.test.{ts,tsx} : Before documenting code patterns, create functional, executable tests to validate them - use /workspaces/walkerOS/apps/quickstart as the source of truth for working examples
📚 Learning: 2025-11-25T16:24:48.032Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T16:24:48.032Z
Learning: Refer to AGENT.md for all relevant instructions and guidelines when working with the walkerOS codebase
Applied to files:
skills/README.md.claude/skills/understanding-development/SKILL.mdskills/debugging/SKILL.mdAGENT.md.claude/skills/create-source/SKILL.mdskills/using-logger/SKILL.md.claude/skills/testing-strategy/SKILL.mdskills/testing-strategy/SKILL.mdpackages/server/destinations/aws/src/firehose/__tests__/firehose.test.ts.claude/skills/understanding-mapping/SKILL.mdskills/understanding-development/SKILL.mdskills/create-destination/SKILL.md
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Use the Source → Collector → Destination(s) architecture pattern for event processing: Sources capture events from various sources (browser DOM, dataLayer, server), Collector handles central event processing with consent management, Destinations transform and send events to third-party tools
Applied to files:
skills/understanding-sources/SKILL.mdskills/understanding-flow/SKILL.md.claude/skills/understanding-flow/SKILL.mdskills/debugging/SKILL.mdAGENT.mdskills/using-logger/SKILL.mdskills/understanding-destinations/SKILL.mdskills/understanding-mapping/SKILL.md.claude/skills/understanding-sources/SKILL.mdskills/create-source/SKILL.mdpackages/collector/src/destination.tsskills/create-destination/SKILL.md
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: All walkerOS components communicate via push functions: sources use source.push(), collector uses collector.push(), destinations use destination.push(), and ELB uses elb() as the alias for collector.push()
Applied to files:
skills/understanding-sources/SKILL.mdskills/understanding-flow/SKILL.mdpackages/server/destinations/meta/src/push.tspackages/collector/src/__tests__/destination.test.tsskills/using-logger/SKILL.mdpackages/server/destinations/aws/src/firehose/push.tspackages/server/destinations/gcp/src/bigquery/push.tspackages/server/destinations/aws/src/firehose/lib/firehose.ts.claude/skills/understanding-sources/SKILL.mdpackages/collector/src/destination.ts
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to packages/*/src/**/*.ts : Implement Destination interface with standard structure: type, init(), push(), pushBatch() methods and config object with optional settings, mapping, data, consent, policy, queue, and dryRun properties
Applied to files:
skills/understanding-flow/SKILL.mdpackages/server/destinations/meta/src/push.tsAGENT.mdpackages/collector/src/__tests__/destination.test.tsskills/using-logger/SKILL.mdpackages/server/destinations/aws/src/firehose/push.tspackages/server/destinations/gcp/src/bigquery/push.tsskills/understanding-destinations/SKILL.mdskills/understanding-mapping/SKILL.md.claude/skills/understanding-destinations/SKILL.mdskills/testing-strategy/SKILL.mdpackages/server/destinations/aws/src/firehose/__tests__/firehose.test.tspackages/server/destinations/datamanager/src/index.tspackages/server/destinations/aws/src/firehose/lib/firehose.tsskills/create-source/SKILL.mdpackages/collector/src/destination.tsskills/create-destination/SKILL.md
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to packages/*/src/**/*.ts : Implement event mapping with flexible patterns: exact entity-action matches, wildcard patterns using '*', conditional mappings with functions, and value mapping strategies including string key extraction, static values, custom functions, object transformation, array processing, and consent-based mapping
Applied to files:
.claude/skills/mapping-configuration/SKILL.mdAGENT.mdskills/understanding-events/SKILL.mdskills/understanding-mapping/SKILL.mdskills/mapping-configuration/SKILL.mdskills/create-source/SKILL.md.claude/skills/understanding-mapping/SKILL.md
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to **/*.test.{ts,tsx} : Before documenting code patterns, create functional, executable tests to validate them - use /workspaces/walkerOS/apps/quickstart as the source of truth for working examples
Applied to files:
.claude/skills/understanding-development/SKILL.mdAGENT.mdpackages/collector/src/__tests__/destination.test.tsskills/using-logger/SKILL.md.claude/skills/testing-strategy/SKILL.mdskills/testing-strategy/SKILL.mdpackages/server/destinations/aws/src/firehose/__tests__/firehose.test.tsskills/create-source/SKILL.mdskills/understanding-development/SKILL.mdskills/create-destination/SKILL.md
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to **/*.test.{ts,tsx} : Write component-level integration tests with mocked external APIs and boundaries: test destinations by mocking external APIs (gtag, fbq, etc.), test collector with mocked destinations and sources, test sources with mocked collector interface
Applied to files:
AGENT.mdpackages/collector/src/__tests__/destination.test.tsskills/using-logger/SKILL.mdskills/understanding-destinations/SKILL.mdskills/testing-strategy/SKILL.mdpackages/server/destinations/aws/src/firehose/__tests__/firehose.test.ts
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to **/*.test.{ts,tsx} : Mock external APIs in destination tests (gtag, fbq, etc.) and verify they are called correctly with the expected data - focus on verifying external API calls, not return values
Applied to files:
packages/collector/src/__tests__/destination.test.tsskills/testing-strategy/SKILL.mdpackages/server/destinations/aws/src/firehose/__tests__/firehose.test.ts
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to **/*.{ts,tsx} : Implement event structure with consistent properties: name (ENTITY ACTION format), data (entity-specific), context (state/environment), globals (global properties), user (identification), nested (related entities), consent, and system-generated fields (id, timestamp, entity, action)
Applied to files:
skills/understanding-events/SKILL.md.claude/skills/understanding-events/SKILL.md
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Always use space-separated 'ENTITY ACTION' format for event names (e.g., 'page view', 'product add', 'button click'). Event names are parsed as: const [entity, action] = event.split(' ')
Applied to files:
skills/understanding-events/SKILL.md
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to **/*.{ts,tsx} : Use core functions from walkeros/core for common operations: getEvent(), createEvent() for event creation, getMappingEvent(), getMappingValue() for transformations, isString(), isObject(), isDefined() for type checking, assign(), clone() for object operations, tryCatch(), tryCatchAsync() for error handling
Applied to files:
skills/understanding-events/SKILL.mdskills/understanding-mapping/SKILL.mdskills/mapping-configuration/SKILL.mdskills/testing-strategy/SKILL.mdpackages/server/destinations/aws/src/firehose/__tests__/firehose.test.tspackages/server/destinations/aws/src/firehose/lib/firehose.tsskills/understanding-development/SKILL.md
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to **/*.{ts,tsx} : Use Root-level Turborepo commands for parallel operations across packages, navigate to individual packages for focused development, and test changes across all affected packages due to core package dependencies
Applied to files:
skills/understanding-development/SKILL.md
🧬 Code graph analysis (3)
packages/collector/src/__tests__/destination.test.ts (2)
packages/core/src/mockLogger.ts (1)
createMockLogger(41-65)packages/collector/src/destination.ts (1)
pushToDestinations(63-202)
packages/server/destinations/aws/src/firehose/push.ts (4)
packages/server/destinations/aws/src/firehose/index.ts (1)
push(23-25)packages/core/src/types/collector.ts (1)
PushFn(93-98)packages/core/src/types/destination.ts (1)
PushFn(141-144)packages/server/destinations/aws/src/firehose/types/index.ts (1)
PushFn(38-38)
packages/collector/src/destination.ts (3)
packages/core/src/useHooks.ts (1)
useHooks(12-39)packages/core/src/types/destination.ts (1)
Instance(59-69)packages/core/src/types/logger.ts (1)
Instance(70-98)
🪛 markdownlint-cli2 (0.18.1)
skills/README.md
32-32: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
skills/understanding-flow/SKILL.md
21-21: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
AGENT.md
42-42: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
skills/create-source/SKILL.md
32-32: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
314-314: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
342-342: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
skills/understanding-development/SKILL.md
44-44: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
skills/create-destination/SKILL.md
25-25: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
270-270: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
294-294: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build and test
- GitHub Check: AWS Amplify Console Web Preview
🔇 Additional comments (30)
.devcontainer/devcontainer.json (2)
10-13: CLAUDE_CONFIG_DIR path is correctly configured.The
.claude/data/directory is intentionally gitignored to exclude runtime data from version control while tracking source files (settings and skills in.claude/). The directory structure exists and has appropriate permissions; thedata/subdirectory will be created by Claude Code on first use or is available at container runtime. No action needed.
44-44: No action required. The@anthropic-ai/claude-codepackage is publicly available on npm as Anthropic's official Claude Code CLI tool and is compatible with this project. The project's Node.js engine requirement (≥20.0) exceeds the package requirement (≥18.0), and no dependency conflicts exist since the package is installed globally..gitignore (1)
95-97: Ignoring Claude Code data is appropriateIgnoring
.claude/data/keeps transient conversations/cache out of git, which is the right default for tool-generated artifacts..claude/skills/understanding-development/SKILL.md (1)
1-16: Skill pointer pattern is clear and consistentYAML metadata plus a short pointer to
@skills/understanding-development/SKILL.mdmatches the intended Claude-discovery pattern while keeping content tool‑agnostic. No issues.packages/core/src/dev.ts (1)
1-2: Reordered exports are harmless and slightly more ergonomicReordering to
export { z, zodToSchema, type JSONSchema }preserves the public surface while makingzthe first named export, which is intuitive for consumers. No type or runtime impact.packages/server/destinations/datamanager/src/index.ts (1)
17-37: Auth client logging via shared logger is appropriateUsing
logger.debug('Auth client created')aftercreateAuthClientintegrates this destination with the shared logging pattern and avoids console usage, without altering control flow. Looks good..claude/skills/using-logger/SKILL.md (1)
1-16: Logger skill metadata matches the new logging conventionsThis skill metadata correctly describes when/how to use the walkerOS logger and points to the canonical
skills/using-logger/SKILL.md. It fits the overall AI‑readiness/documentation pattern in this PR..claude/skills/understanding-flow/SKILL.md (1)
1-16: Metadata reference file—LGTM.Proper YAML frontmatter and clear reference to primary content location. Consistent with the skill discovery pattern.
skills/understanding-development/SKILL.md (1)
1-41: Comprehensive and well-structured onboarding guide.Clear overview of XP principles, commands, and core utilities aligned with walkerOS architecture. Proper emphasis on strict conventions (no
any, TDD-first, DRY)..claude/skills/understanding-mapping/SKILL.md (1)
1-16: Metadata reference—consistent pattern.YAML frontmatter and reference pointer properly formatted and aligned with other skill metadata files in the PR.
skills/understanding-destinations/SKILL.md (1)
1-125: Well-crafted destination reference guide.Clear interface documentation, env pattern explanation with working examples, and proper testing guidance. Code blocks are properly marked with language identifiers and examples align with walkerOS patterns.
.claude/skills/understanding-destinations/SKILL.md (1)
1-16: Metadata reference—consistent with pattern.YAML frontmatter and discoverability reference properly formatted.
.claude/skills/testing-strategy/SKILL.md (1)
1-16: Metadata reference—follows established pattern.Properly formatted YAML frontmatter and clear reference pointer.
.claude/skills/debugging/SKILL.md (1)
1-16: Metadata reference—consistent pattern maintained.YAML frontmatter and reference pointer properly structured.
skills/debugging/SKILL.md (2)
1-125: Exceptional debugging guide—practical and comprehensive.Clear diagnosis table, four distinct debugging strategies with examples, and six common issues with specific fixes. All TypeScript examples properly marked. Debugging checklist provides actionable verification sequence. Excellent alignment with walkerOS architecture patterns (env pattern, consent, mapping, consent).
225-237: Debugging checklist—excellent verification pattern.The 8-step checklist (lines 225-237) provides a systematic approach to event troubleshooting. Well-ordered progression from console errors → event flow → mapping → data → consent → SDK → network → vendor.
packages/collector/src/__tests__/destination.test.ts (2)
44-55: Logger mocking setup correctly models scoped loggingUsing
createMockLoggerplus a dedicatedscopedMockLoggerreturned fromlogger.scopegives you a clean way to assert on scoped debug calls while still matching the core logger interface. This is a reasonable pattern for these collector-level tests and aligns with the new destination logging behavior.
123-163: Lifecycle logging tests accurately verify init/push behaviorThe
logs init lifecycleandlogs push lifecycletests nicely validate the new logging contract: they assertlogger.scope('unknown')is used and that the scoped logger recordsinit/init doneandpush/push donewith the expectedeventpayload. This provides good regression protection for the logging behavior introduced indestinationInit/destinationPush.packages/collector/src/destination.ts (1)
218-247: Scoped debug logging around init, push, and batch looks solidThe added
destLogger/batchLoggerdebug calls arounddestinationInit, per-eventdestinationPush, andpushBatch(including counts and lifecycle “done” markers) give clear observability without changing existing control flow or consent behavior. This matches the logger scoping pattern used elsewhere and will make destination issues much easier to diagnose.Also applies to: 273-284, 296-322, 330-340
.claude/skills/create-destination/SKILL.md (1)
1-15: Metadata shim correctly forwards to the canonical skillThe frontmatter and pointer to
@skills/create-destination/SKILL.mdare clear and keep content tool-agnostic while enabling discovery. No changes needed.packages/server/destinations/gcp/src/bigquery/push.ts (1)
7-14: Logger integration in BigQuery push improves error handling and observabilityWiring
loggerinto the push context and usinglogger.throwfor missingclient/datasetId/tableIdplus the debug logs aroundinsert()give you clear, structured visibility into configuration problems and API calls, without changing the existing row-shaping behavior. This aligns well with the broader destination logging pattern in the collector.Also applies to: 30-39
skills/testing-strategy/SKILL.md (1)
20-227: Testing strategy doc aligns well with env + dev examples patternThis skill document clearly encodes the AGENT/testing guidance: env-based mocking instead of Jest module mocks, tests wired to
devexamples, and explicit rules around integration/unit/contract tests. ThemockEnvanddev.tsstructures shown are consistent with existing patterns and should be a good reference for future contributors..claude/skills/create-source/SKILL.md (1)
1-15: Claude skill locator looks consistent and minimal.Front matter and the pointer to
skills/create-source/SKILL.mdmatch the documented pattern for Claude skill references; nothing to change here..claude/skills/understanding-events/SKILL.md (1)
1-16: Understanding-events Claude reference is correctly wired.The file follows the standard pattern (front matter + pointer to
skills/understanding-events/SKILL.md) and looks good as a discovery aid..claude/skills/understanding-sources/SKILL.md (1)
1-16: Understanding-sources Claude reference matches the intended pattern.Front matter and the redirect to
skills/understanding-sources/SKILL.mdare consistent with the rest of the skills setup..claude/skills/mapping-configuration/SKILL.md (1)
1-15: Mapping-configuration Claude reference is clean and consistent.The metadata and redirect to
skills/mapping-configuration/SKILL.mdfollow the same convention as the other skills; nothing to adjust.skills/understanding-sources/SKILL.md (1)
1-115: Sources skill aligns well with Source → Collector → Destination modelThis doc clearly describes sources as capture-only, push-based components and points to the canonical
source.tstypes and concrete paths; it matches the Source → Collector → Destination architecture and push() usage described in AGENT.md. Based on learnings, this looks consistent and ready to ship.skills/mapping-configuration/SKILL.md (1)
1-280: Mapping recipes are consistent with the core mapping DSLThe patterns here (string paths,
{ value },{ fn },{ map },{ loop }, consent-gated fields, wildcard entity/action) match the flexible mapping strategies described in AGENT.md and implemented inpackages/core/src/mapping.ts. Referencingapps/quickstart/src/mappings/as validated examples is also in line with the “tests first, docs second” guidance. Based on learnings, this looks solid.skills/understanding-events/SKILL.md (1)
1-177: Event model doc matches the canonical walkerOS event contractThis skill file cleanly codifies the
"entity action"requirement, the full event property set, and the stateless/vendor‑agnostic design, exactly as described in AGENT.md and the coreevent.ts/schema files. Theelb('page view', …)examples also match the documented collector API. Based on learnings, this is accurate and ready to merge.packages/server/destinations/aws/src/firehose/lib/firehose.ts (1)
1-78: Firehose helper’s PushContext + logger integration looks correct
pushFirehosecleanly switches toDestination.PushContext, derivesenvvia the existingisAWSEnvironmentguard, and adds before/after debug logs around theclient.send(...)call, in line with the “external API call” pattern from the logger skill. The existing behavior of constructing the client fromenv.AWS.FirehoseClientor falling back to a dynamic SDK import is preserved, so this change reads as a safe, backwards‑compatible logging upgrade.
| ``` | ||
| 1. Research → Understand input format, find SDK/types | ||
| 2. Examples → Create input examples in dev entry FIRST | ||
| 3. Mapping → Define input → walkerOS event transformation | ||
| 4. Scaffold → Copy template and configure | ||
| 5. Implement → Build using examples as test fixtures | ||
| 6. Test → Verify against example variations | ||
| 7. Document → Write README | ||
| ``` |
There was a problem hiding this comment.
Add explicit languages to fenced code blocks to satisfy markdownlint (MD040).
The code fences for the process overview, mapping trace, and directory structure currently use bare ``````, which triggers MD040. Consider annotating them as text (or another appropriate language), e.g.:
-```
+```text
1. Research → Understand input format, find SDK/types
...
-```
+```Apply the same pattern for the mapping trace and the packages/server/sources/[name]/ tree block so lint passes cleanly.
Also applies to: 314-323, 342-363
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
32-32: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In skills/create-source/SKILL.md around lines 32-40 (and also update the other
mentioned ranges 314-323 and 342-363), the fenced code blocks are bare and
trigger markdownlint MD040; edit each triple-backtick block to include an
explicit language token (e.g., text) immediately after the opening backticks so
they read ```text instead of ```; apply the same change to the process overview,
mapping trace, and the directory tree block (packages/server/sources/[name]/) so
all fenced blocks pass MD040.
| test('handles minimal input', () => { | ||
| const result = transformInput(examples.inputs.minimalInput, config); | ||
|
|
||
| // Should handle gracefully (may return undefined or minimal event) | ||
| expect(result).toBeDefined(); | ||
| }); |
There was a problem hiding this comment.
Align minimalInput example with transformInput behavior and event naming rules.
minimalInput uses event: 'ping' and the test asserts expect(result).toBeDefined(), but the example transformInput splits on a space and returns undefined when it can’t derive both entity and action. Given AGENT’s "entity action" naming non‑negotiable, either:
- Update the minimal example to a valid
"entity action"(e.g.'system ping') and keep thetoBeDefinedassertion, or - Keep
'ping'but change the test/comment to assert that the function returnsundefinedwhile handling input without throwing.
Right now, the documented expectation and the sample implementation don’t match.
Also applies to: 160-163, 375-392
🤖 Prompt for AI Agents
In skills/create-source/SKILL.md around lines 502-507 (also review lines 160-163
and 375-392), the test/example uses minimalInput with event: 'ping' but
transformInput requires "entity action" and returns undefined when it can't
derive both; update the examples/tests so they align: either change
minimalInput.event to a valid two‑word name like 'system ping' and keep
expect(result).toBeDefined(), or keep 'ping' and change the test/comment to
assert that transformInput returns undefined (e.g.,
expect(result).toBeUndefined()) and note that this is acceptable behavior; apply
the chosen change consistently to the referenced line ranges.
| # walkerOS Skills | ||
|
|
||
| Skills are the source of truth for AI assistants working with this repository. | ||
| Tool-agnostic and accessible to Claude, Cursor, Copilot, and other AI tools. | ||
|
|
||
| ## Available Skills | ||
|
|
||
| ### Concept Skills (Understanding) | ||
|
|
||
| | Skill | Description | | ||
| | ------------------------------------------------------------------- | --------------------------------------------------------- | | ||
| | [understanding-development](./understanding-development/SKILL.md) | Build, test, lint, XP principles, folder structure | | ||
| | [understanding-flow](./understanding-flow/SKILL.md) | Architecture, composability, Source→Collector→Destination | | ||
| | [understanding-events](./understanding-events/SKILL.md) | Event model, entity-action naming, properties | | ||
| | [understanding-mapping](./understanding-mapping/SKILL.md) | Event transformation, data/map/loop/condition | | ||
| | [understanding-destinations](./understanding-destinations/SKILL.md) | Destination interface, env pattern, configuration | | ||
| | [understanding-sources](./understanding-sources/SKILL.md) | Source interface, capture patterns | | ||
| | [using-logger](./using-logger/SKILL.md) | Logger access, DRY principles, API logging patterns | | ||
|
|
||
| ### Task Skills | ||
|
|
||
| | Skill | Description | | ||
| | --------------------------------------------------------- | --------------------------------------------- | | ||
| | [testing-strategy](./testing-strategy/SKILL.md) | Testing patterns, env mocking, dev examples | | ||
| | [create-destination](./create-destination/SKILL.md) | Step-by-step destination creation | | ||
| | [create-source](./create-source/SKILL.md) | Step-by-step source creation | | ||
| | [mapping-configuration](./mapping-configuration/SKILL.md) | Mapping recipes for GA4, Meta, custom APIs | | ||
| | [debugging](./debugging/SKILL.md) | Troubleshooting event flow and mapping issues | | ||
|
|
||
| ## Architecture | ||
|
|
||
| ``` | ||
| skills/ <- Primary content (tool-agnostic) | ||
| .claude/skills/ <- Claude Code references (auto-discovery) | ||
| ``` | ||
|
|
||
| ## Usage | ||
|
|
||
| ### Claude Code | ||
|
|
||
| Skills in `.claude/skills/` are auto-discovered and reference primary files | ||
| here. | ||
|
|
||
| ### Other AI Tools | ||
|
|
||
| Reference skills in this directory directly. | ||
|
|
||
| ## Adding New Skills | ||
|
|
||
| 1. Create `skills/[skill-name]/SKILL.md` with full content | ||
| 2. Create `.claude/skills/[skill-name]/SKILL.md` reference | ||
| 3. Update this README | ||
| 4. Update AGENT.md navigation if needed |
There was a problem hiding this comment.
Great high‑level skills index; fix fenced code block language
The Skills README is well structured and clearly explains concepts vs. tasks and the .claude/skills indirection.
Markdownlint’s MD040 on the directory layout block can be fixed by specifying a language for the fence. For example:
-```
+```text
skills/ <- Primary content (tool-agnostic)
.claude/skills/ <- Claude Code references (auto-discovery)
-```
+```🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
32-32: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In skills/README.md around lines 1 to 53, the fenced code block under
"Architecture" lacks a language identifier which triggers markdownlint MD040;
update the opening fence from ``` to ```text (keeping the same block content and
the closing ```), so the block is explicitly marked as plain text and the linter
warning is resolved.
| ``` | ||
| packages/ | ||
| ├── core/ # Platform-agnostic types, utilities, schemas | ||
| ├── collector/ # Central event processing engine | ||
| ├── config/ # Shared config (eslint, jest, tsconfig, tsup) | ||
| ├── web/ | ||
| │ ├── core/ # Web-specific utilities | ||
| │ ├── sources/ # browser, dataLayer | ||
| │ └── destinations/ # gtag, meta, api, piwikpro, plausible | ||
| └── server/ | ||
| ├── core/ # Server-specific utilities | ||
| ├── sources/ # gcp | ||
| └── destinations/ # aws, gcp, meta | ||
| apps/ | ||
| ├── walkerjs/ # Ready-to-use browser bundle | ||
| ├── quickstart/ # Code examples (source of truth for patterns) | ||
| └── demos/ # Demo applications | ||
| ``` |
There was a problem hiding this comment.
Add language identifier to fenced code block.
The code block at line 44 defining folder structure is missing a language identifier. Since it shows a text-based tree structure, mark it as text or plaintext for proper rendering.
Apply this diff:
## Folder Structure
-```
+```text
packages/
├── core/ # Platform-agnostic types, utilities, schemas🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
44-44: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In skills/understanding-development/SKILL.md around lines 44 to 62, the fenced
code block showing the folder tree is missing a language identifier; update the
opening fence from ``` to ```text (or ```plaintext) so the block is explicitly
marked as plain text for proper rendering — replace the opening triple backticks
with ```text and keep the rest of the block unchanged.
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/cli/src/core/docker.ts (2)
165-174: Callingprocess.exit()inside a Promise breaks caller expectations.The function signature returns
Promise<void>, but on failure it callsprocess.exit()instead of rejecting. This:
- Prevents callers from catching errors or performing cleanup
- Makes the function untestable for failure cases
- Breaks composability - any wrapper/retry logic is bypassed
If the intent is to avoid duplicate error messages, reject with a marker error that the CLI entry point can handle silently.
proc.on('exit', (code) => { if (code === 0) { resolve(); } else { - // Docker already logged the error via stdio inherit - // Just exit with same code - no duplicate message - process.exit(code || 1); + // Docker already logged the error via stdio inherit + // Reject with exit code for caller to handle + const error = new Error(`Docker exited with code ${code}`); + (error as any).exitCode = code || 1; + (error as any).silent = true; // Signal to CLI not to log again + reject(error); } });
324-332: Same issue:process.exit()inside Promise executor.This has the same problem as
executeInDocker. Apply the same fix to reject with an error object containing the exit code and a silent flag.proc.on('exit', (code) => { if (code === 0) { resolve(); } else { - // Docker already logged the error via stdio inherit - // Just exit with same code - no duplicate message - process.exit(code || 1); + const error = new Error(`Docker exited with code ${code}`); + (error as any).exitCode = code || 1; + (error as any).silent = true; + reject(error); } });packages/collector/src/destination.ts (1)
129-130: Bug:.concat()result is discarded—skipped events are lost.
Array.prototype.concat()returns a new array and does not mutate the original. The skipped events are never actually added back to the queue.Apply this fix:
// Add skipped events back to the queue - destination.queue.concat(skippedEvents); + destination.queue.push(...skippedEvents);
🧹 Nitpick comments (4)
apps/demos/flows/browser-to-bigquery.json (1)
1-104: Well-structured flow configuration with appropriate demo defaults.The flow configuration correctly defines web and server flows with proper source and destination wiring. Variable references are consistent (COLLECT_URL on line 32, GCP_PROJECT_ID on line 85), and the credentials path (line 90) aligns with the setup instructions in README.md.
Note: The hardcoded GCP_PROJECT_ID and relative credentials path are appropriate for this demo context, but would need parameterization for production use.
Consider externalizing the GCP_PROJECT_ID from the demo flow file into environment variables or a separate .env file for better security and reusability across environments:
"projectId": "${GCP_PROJECT_ID}",This would allow developers to inject their own project IDs without modifying the flow configuration directly.
packages/web/destinations/gtag/src/index.ts (1)
116-119: Consider adding runtime validation for type assertions.The type assertions on lines 118-119 assume
context.dataconforms toWalkerOS.Consentandcontext.config?.settingsconforms toPartial<Settings>, but there's no runtime validation. If the data doesn't match these types, accessing properties later (e.g., in theObject.entries(consent)loop on line 161) could cause runtime errors.Consider adding validation or using type guards to ensure the data structure matches expectations before proceeding.
Example validation approach:
if (type !== 'consent' || !context.data) return; const consent = context.data as WalkerOS.Consent; +if (!consent || typeof consent !== 'object') { + context.logger?.log('Invalid consent data structure'); + return; +} + const settings = (context.config?.settings || {}) as Partial<Settings>;packages/collector/src/destination-code.ts (1)
20-32: Consider returning a value to indicate success/failure.The
pushmethod returnsundefinedin all cases. Other destinations may return a result to indicate success. While the current implementation handles errors gracefully via logging, returning a boolean or result object could help callers understand if the push succeeded.push(event, context) { const { mapping, config, logger } = context; const pushCode = (mapping as CodeMapping | undefined)?.push ?? (config.settings as Settings | undefined)?.push; - if (!pushCode) return; + if (!pushCode) return true; // No code to execute, consider successful try { const fn = new Function('event', 'context', pushCode); fn(event, context); + return true; } catch (e) { logger.error('Code destination push error:', e); + return false; } },packages/collector/src/types/code.ts (1)
3-8: Settings interface allows arbitrary code strings.The
Settingsinterface defines string fields for executable code. This design enables the code destination but inherently carries security implications. Consider adding JSDoc comments to warn developers that these strings will be executed as JavaScript.+/** + * Settings for the code destination. + * WARNING: These strings are executed as JavaScript code. + * Only use with trusted, validated input. + */ export interface Settings { + /** JavaScript code to execute during initialization */ init?: string; + /** JavaScript code to execute on lifecycle events */ on?: string; + /** JavaScript code to execute on push */ push?: string; + /** JavaScript code to execute on batch push */ pushBatch?: string; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (31)
.changeset/huge-pens-lay.md(1 hunks).devcontainer/devcontainer.json(2 hunks)apps/demos/flows/.gitignore(1 hunks)apps/demos/flows/README.md(1 hunks)apps/demos/flows/browser-to-bigquery.json(1 hunks)apps/demos/flows/index.html(1 hunks)apps/demos/flows/shared/credentials/.gitkeep(1 hunks)packages/cli/src/commands/run/execution.ts(3 hunks)packages/cli/src/core/docker.ts(2 hunks)packages/cli/src/index.ts(2 hunks)packages/collector/src/__tests__/destination-code.test.ts(1 hunks)packages/collector/src/__tests__/destination.test.ts(9 hunks)packages/collector/src/destination-code.ts(1 hunks)packages/collector/src/destination.ts(7 hunks)packages/collector/src/index.ts(1 hunks)packages/collector/src/on.ts(2 hunks)packages/collector/src/types/code.ts(1 hunks)packages/collector/src/types/index.ts(1 hunks)packages/core/src/types/destination.ts(1 hunks)packages/core/src/types/on.ts(2 hunks)packages/docker/package.json(1 hunks)packages/docker/src/index.ts(5 hunks)packages/docker/src/services/runner.ts(5 hunks)packages/docker/src/services/serve.ts(5 hunks)packages/docker/src/version.ts(1 hunks)packages/web/destinations/gtag/src/__tests__/index.test.ts(9 hunks)packages/web/destinations/gtag/src/index.ts(2 hunks)website/docs/collector/index.mdx(1 hunks)website/docs/destinations/code.mdx(1 hunks)website/docs/destinations/index.mdx(1 hunks)website/docs/getting-started/flow.mdx(1 hunks)
✅ Files skipped from review due to trivial changes (5)
- apps/demos/flows/.gitignore
- .changeset/huge-pens-lay.md
- website/docs/destinations/code.mdx
- apps/demos/flows/shared/credentials/.gitkeep
- packages/collector/src/tests/destination-code.test.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENT.md)
**/*.{ts,tsx}: NEVER useanytype - always use proper TypeScript types from @walkeros/core or package-specific types
Implement event structure with consistent properties: name (ENTITY ACTION format), data (entity-specific), context (state/environment), globals (global properties), user (identification), nested (related entities), consent, and system-generated fields (id, timestamp, entity, action)
Use core functions from @walkeros/core for common operations: getEvent(), createEvent() for event creation, getMappingEvent(), getMappingValue() for transformations, isString(), isObject(), isDefined() for type checking, assign(), clone() for object operations, tryCatch(), tryCatchAsync() for error handling
Useimport typefor type-only imports to ensure proper TypeScript compilation and tree-shaking
Avoid inline change comments like '// Fixed for version X' or '// Changed due to Y' - Git tracks history, use migrations and documentation for breaking changes instead
Use Root-level Turborepo commands for parallel operations across packages, navigate to individual packages for focused development, and test changes across all affected packages due to core package dependencies
Files:
packages/collector/src/index.tspackages/collector/src/types/index.tspackages/cli/src/index.tspackages/web/destinations/gtag/src/index.tspackages/docker/src/version.tspackages/docker/src/services/serve.tspackages/cli/src/commands/run/execution.tspackages/collector/src/destination-code.tspackages/core/src/types/on.tspackages/docker/src/services/runner.tspackages/collector/src/on.tspackages/core/src/types/destination.tspackages/collector/src/types/code.tspackages/collector/src/destination.tspackages/cli/src/core/docker.tspackages/collector/src/__tests__/destination.test.tspackages/web/destinations/gtag/src/__tests__/index.test.tspackages/docker/src/index.ts
packages/*/src/**/*.ts
📄 CodeRabbit inference engine (AGENT.md)
packages/*/src/**/*.ts: Use Higher-Order Functions (HOF) and smart abstractions to avoid redundancy and keep implementations lean and performant
Implement Destination interface with standard structure: type, init(), push(), pushBatch() methods and config object with optional settings, mapping, data, consent, policy, queue, and dryRun properties
Implement event mapping with flexible patterns: exact entity-action matches, wildcard patterns using '*', conditional mappings with functions, and value mapping strategies including string key extraction, static values, custom functions, object transformation, array processing, and consent-based mapping
Files:
packages/collector/src/index.tspackages/collector/src/types/index.tspackages/cli/src/index.tspackages/docker/src/version.tspackages/docker/src/services/serve.tspackages/cli/src/commands/run/execution.tspackages/collector/src/destination-code.tspackages/core/src/types/on.tspackages/docker/src/services/runner.tspackages/collector/src/on.tspackages/core/src/types/destination.tspackages/collector/src/types/code.tspackages/collector/src/destination.tspackages/cli/src/core/docker.tspackages/collector/src/__tests__/destination.test.tspackages/docker/src/index.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENT.md)
**/*.test.{ts,tsx}: Write component-level integration tests with mocked external APIs and boundaries: test destinations by mocking external APIs (gtag, fbq, etc.), test collector with mocked destinations and sources, test sources with mocked collector interface
Mock external APIs in destination tests (gtag, fbq, etc.) and verify they are called correctly with the expected data - focus on verifying external API calls, not return values
Before documenting code patterns, create functional, executable tests to validate them - use /workspaces/walkerOS/apps/quickstart as the source of truth for working examples
Files:
packages/collector/src/__tests__/destination.test.tspackages/web/destinations/gtag/src/__tests__/index.test.ts
🧠 Learnings (10)
📓 Common learnings
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to **/*.{ts,tsx} : Use core functions from walkeros/core for common operations: getEvent(), createEvent() for event creation, getMappingEvent(), getMappingValue() for transformations, isString(), isObject(), isDefined() for type checking, assign(), clone() for object operations, tryCatch(), tryCatchAsync() for error handling
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to packages/*/src/**/*.ts : Implement Destination interface with standard structure: type, init(), push(), pushBatch() methods and config object with optional settings, mapping, data, consent, policy, queue, and dryRun properties
Applied to files:
packages/collector/src/index.tspackages/web/destinations/gtag/src/index.tswebsite/docs/collector/index.mdxwebsite/docs/destinations/index.mdxpackages/collector/src/destination-code.tspackages/core/src/types/on.tspackages/collector/src/on.tspackages/core/src/types/destination.tspackages/collector/src/types/code.tswebsite/docs/getting-started/flow.mdxpackages/collector/src/destination.tspackages/collector/src/__tests__/destination.test.tspackages/web/destinations/gtag/src/__tests__/index.test.ts
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Consent is optional and configurable at multiple levels: destination-level, event mapping level, value mapping level, and policy level - without explicit consent requirements, events process normally
Applied to files:
packages/web/destinations/gtag/src/index.ts
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to **/*.{ts,tsx} : Use core functions from walkeros/core for common operations: getEvent(), createEvent() for event creation, getMappingEvent(), getMappingValue() for transformations, isString(), isObject(), isDefined() for type checking, assign(), clone() for object operations, tryCatch(), tryCatchAsync() for error handling
Applied to files:
packages/docker/src/version.tspackages/core/src/types/on.tspackages/collector/src/on.tspackages/collector/src/types/code.tspackages/docker/src/index.ts
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Use the Source → Collector → Destination(s) architecture pattern for event processing: Sources capture events from various sources (browser DOM, dataLayer, server), Collector handles central event processing with consent management, Destinations transform and send events to third-party tools
Applied to files:
apps/demos/flows/index.htmlwebsite/docs/collector/index.mdxwebsite/docs/destinations/index.mdxpackages/collector/src/on.tswebsite/docs/getting-started/flow.mdxpackages/collector/src/destination.tspackages/web/destinations/gtag/src/__tests__/index.test.ts
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: All walkerOS components communicate via push functions: sources use source.push(), collector uses collector.push(), destinations use destination.push(), and ELB uses elb() as the alias for collector.push()
Applied to files:
packages/core/src/types/on.tspackages/collector/src/on.tspackages/collector/src/types/code.tspackages/collector/src/destination.tspackages/collector/src/__tests__/destination.test.ts
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to packages/*/src/**/*.ts : Implement event mapping with flexible patterns: exact entity-action matches, wildcard patterns using '*', conditional mappings with functions, and value mapping strategies including string key extraction, static values, custom functions, object transformation, array processing, and consent-based mapping
Applied to files:
packages/collector/src/on.ts
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to **/*.test.{ts,tsx} : Write component-level integration tests with mocked external APIs and boundaries: test destinations by mocking external APIs (gtag, fbq, etc.), test collector with mocked destinations and sources, test sources with mocked collector interface
Applied to files:
packages/collector/src/__tests__/destination.test.tspackages/web/destinations/gtag/src/__tests__/index.test.ts
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to **/*.test.{ts,tsx} : Mock external APIs in destination tests (gtag, fbq, etc.) and verify they are called correctly with the expected data - focus on verifying external API calls, not return values
Applied to files:
packages/collector/src/__tests__/destination.test.tspackages/web/destinations/gtag/src/__tests__/index.test.ts
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to **/*.test.{ts,tsx} : Before documenting code patterns, create functional, executable tests to validate them - use /workspaces/walkerOS/apps/quickstart as the source of truth for working examples
Applied to files:
packages/collector/src/__tests__/destination.test.tspackages/web/destinations/gtag/src/__tests__/index.test.ts
🧬 Code graph analysis (9)
packages/collector/src/types/index.ts (1)
packages/core/src/types/destination.ts (1)
Code(97-97)
packages/web/destinations/gtag/src/index.ts (2)
packages/web/destinations/gtag/src/types/index.ts (1)
Settings(62-71)packages/web/core/src/environment.ts (1)
getEnv(11-17)
packages/docker/src/version.ts (1)
packages/docker/src/index.ts (1)
VERSION(10-10)
packages/docker/src/services/serve.ts (3)
packages/docker/src/index.ts (3)
runServeMode(116-116)ServeConfig(116-116)VERSION(10-10)packages/cli/src/core/logger.ts (1)
Logger(11-20)packages/docker/src/version.ts (1)
VERSION(7-8)
packages/cli/src/commands/run/execution.ts (3)
packages/core/src/logger.ts (1)
Level(213-213)packages/docker/src/services/runner.ts (1)
runFlow(24-97)packages/docker/src/services/serve.ts (1)
runServeMode(19-94)
packages/collector/src/destination-code.ts (3)
packages/core/src/types/destination.ts (2)
Instance(59-69)Settings(49-49)packages/collector/src/types/code.ts (2)
Settings(3-8)CodeMapping(10-13)packages/cli/demos/programmatic-example.js (1)
event(54-60)
packages/core/src/types/on.ts (4)
packages/collector/src/types/code.ts (3)
OnFn(23-23)Types(15-15)Context(17-17)packages/core/src/types/destination.ts (3)
TypesGeneric(39-44)Types(29-34)Context(113-119)packages/core/src/types/source.ts (2)
TypesGeneric(52-58)Types(35-47)packages/core/src/types/walkeros.ts (1)
PromiseOrValue(106-106)
packages/docker/src/services/runner.ts (1)
packages/cli/src/core/logger.ts (1)
Logger(11-20)
packages/docker/src/index.ts (4)
packages/core/src/logger.ts (1)
Level(213-213)packages/docker/src/version.ts (1)
VERSION(7-8)packages/docker/src/services/runner.ts (1)
runFlow(24-97)packages/docker/src/services/serve.ts (1)
runServeMode(19-94)
🪛 markdownlint-cli2 (0.18.1)
apps/demos/flows/README.md
32-32: Bare URL used
(MD034, no-bare-urls)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: AWS Amplify Console Web Preview
🔇 Additional comments (45)
website/docs/getting-started/flow.mdx (2)
237-256: Documentation update is clear and well-positioned.The addition effectively introduces the built-in Code Destination with a concrete example demonstrating
code: trueconfiguration and string-based push handlers. The placement logically extends the Destinations section.
242-254: Minor inconsistency: Event property access differs across examples.This example accesses
event.nameandevent.data, while the collector documentation (line 81 of collector/index.mdx) uses justevent. Verify whether this is intentional to demonstrate different use patterns or if it should be standardized across all examples for consistency.website/docs/destinations/index.mdx (1)
41-47: Well-structured categorization of destinations by scope.The new "Built-in Destinations" section logically precedes platform-specific destinations and introduces a clear taxonomy. Confirm that the linked documentation (
/docs/destinations/code) exists in the PR and is properly documented.Verify that
website/docs/destinations/code.mdxexists in this PR and contains comprehensive documentation of the Code Destination including:
- Configuration options (settings, mapping, data, consent, policy)
- String-based push handler examples
- any init() or on() context handlers
- Error handling behavior
website/docs/collector/index.mdx (2)
76-85: Update successfully demonstrates simplified setup using built-in Code Destination.The migration from an imported destination to
code: truewith string-based push handler is clear and aligns with the feature goal. The inline comment aids maintainability.
78-82: Verify consistent documentation of event object structure in Code Destination guide.This example accesses the full
eventobject, while the flow.mdx example (line 248) accessesevent.nameandevent.data. Ensure the Code Destination documentation (code.mdx) clarifies what properties are available on the event object and whether these examples reflect different valid patterns or if one should be standardized..devcontainer/devcontainer.json (3)
20-20: Clarify intent: both ChatGPT and Claude Code extensions.The configuration adds both
openai.chatgptand installs@anthropic-ai/claude-codeglobally. Given the PR focus on Claude/skill metadata and AI readiness, verify this is intentional and not a duplicate/competing tooling setup. If the intent is Claude-only, remove the ChatGPT extension.
42-43: Resource constraints are reasonable.The memory (8g with 10g swap) and CPU (4) limits are appropriate for development and should provide sufficient headroom for builds and testing without constraining performance excessively.
46-46: LGTM: postCreateCommand changes.The removal of
@openai/codexand retention of@anthropic-ai/claude-codealigns with the Claude Code focus. The build step is properly retained.packages/web/destinations/gtag/src/__tests__/index.test.ts (1)
429-435: LGTM! Consistent refactoring to structured context.All consent handler test calls have been consistently updated to pass a full context object (collector, config, data, env, logger) instead of just consent data directly. This aligns with the broader architectural change to provide richer runtime context to destination handlers.
The tests maintain their coverage and correctly verify gtag consent behavior with the new API structure.
Also applies to: 466-472, 513-519, 539-545, 574-580, 627-633, 659-665, 692-698, 728-734
apps/demos/flows/index.html (1)
1-28: Well-structured HTML demo UI with sound event architecture.The HTML correctly implements the demo UI with properly-formed event payloads and correct script loading order. The event flow description aligns with the architecture pattern. No issues identified.
packages/web/destinations/gtag/src/index.ts (2)
126-127: LGTM - consistent use of context environment.The change to use
context.envinstead of instance-level environment aligns with the refactoring goal of accessing properties through the context parameter. The use ofgetEnvfrom@walkeros/web-coreis appropriate.
161-171: Clean iteration pattern.The refactored consent mapping logic using
Object.entriesis cleaner and more direct. The handling of both array and single string gtag parameters is appropriate.packages/docker/package.json (1)
30-34: LGTM!Adding
@walkeros/coreas a runtime dependency is appropriate since the docker package now usesLoggertypes and utilities from core at runtime (in runner.ts and serve.ts).packages/cli/src/index.ts (1)
63-71: LGTM!The startup banner with version information is well-implemented. Correctly skips for
--silentand--jsonto avoid interfering with machine-readable output.packages/docker/src/version.ts (1)
1-8: LGTM!Clean implementation of build-time version injection. The
typeofcheck correctly handles cases where__VERSION__isn't defined (e.g., during development), falling back to a sensible default.packages/cli/src/commands/run/execution.ts (1)
1-7: LGTM!Good use of
createLoggerfrom@walkeros/coreas recommended in the coding guidelines. The scoped loggers (logger.scope('runner')andlogger.scope('serve')) provide clear context for log output.packages/docker/src/services/serve.ts (3)
19-22: LGTM!Good use of
import typefor theLoggertype import, following the coding guidelines. The function signature update with the logger parameter enables consistent structured logging throughout the serve mode.
52-61: Good observability enhancement.Including
VERSIONin the health response is excellent for debugging and monitoring. The comprehensive health payload (version, timestamp, mode, file, url) provides valuable runtime introspection.
84-92: Clean shutdown and error handling.The graceful shutdown pattern with signal handlers is well-implemented. Error handling correctly extracts the message and logs before exiting, which is appropriate for a top-level server entry point.
packages/docker/src/index.ts (7)
3-4: LGTM!Correct use of
import typefor type-only imports from @walkeros/core, following TypeScript best practices for tree-shaking.
7-10: LGTM!Centralizing VERSION in a dedicated module and re-exporting for external consumers is good practice for maintainability.
12-14: LGTM!Environment-based log level configuration is appropriate for runtime containers, providing users control over verbosity.
99-112: LGTM!Process error handlers correctly extract error messages and conditionally log stack traces at debug level. The implementation is consistent with the main error handler.
115-116: LGTM!Public API exports align with the updated function signatures and correctly use type-only re-exports for configuration types.
57-60: No action needed — logger.throw() correctly returns never.The logger.throw() method is properly typed to return
neverand always throws an exception. The return statement on line 60 is correct TypeScript idiom for type narrowing and will never be reached, as indicated by the comment. This pattern is appropriate and the code follows best practices.
89-93: No issue found - logger.debug() supports object parameters.The
LogFntype signature acceptscontext?: unknown | Erroras the second parameter, and thenormalizeParams()function explicitly handles object parameters by spreading them into the normalized context. The pattern used at lines 92, 101, and 109 is valid and fully supported.Likely an incorrect or invalid review comment.
packages/docker/src/services/runner.ts (4)
22-28: LGTM!Function signature correctly updated to accept logger parameter with proper JSDoc documentation. The signature aligns with the calling code in index.ts.
29-29: LGTM!Appropriate use of logger.info for status messages with clear, informative output.
Also applies to: 60-63
67-67: LGTM!Logger usage with appropriate levels: info for normal lifecycle events and error for failures.
Also applies to: 74-74, 78-78, 91-91
77-78: LGTM!Error handling correctly extracts messages from Error objects and other types, with conditional stack trace logging at debug level. The implementation is consistent with the main error handler in index.ts.
Also applies to: 90-94
packages/core/src/types/destination.ts (1)
97-103: Clean type abstraction for code-based destinations.The
Code<T>union type provides a flexible way to specify destinations—either as a fullInstance<T>or astruefor the built-in code destination. This maintains type safety while enabling the shorthand pattern used inresolveCode()within the collector package.packages/collector/src/destination.ts (3)
16-18: Clean resolver pattern for code-based destinations.The
resolveCodehelper provides a straightforward way to normalize theCode<T>union type, enabling the shorthandcode: truesyntax while maintaining a consistent internal representation.
235-236: Debug logging additions are consistent and well-structured.The logging follows a consistent pattern: scoped by destination type, using
debuglevel, and including relevant context (event count for batches, event name for single pushes). This aids observability without cluttering production logs.Also applies to: 251-252, 317-319, 327-328, 336-337, 344-345
410-428: LGTM!The
mergeEnvironmentsexport is well-implemented with proper handling of edge cases. The precedence logic (config env overrides destination env) is clearly documented.packages/collector/src/index.ts (1)
9-9: LGTM!The new
destination-codeexport follows the established pattern and is logically grouped with related destination exports.packages/collector/src/types/index.ts (1)
2-2: Namespace export for Code types.The
Codenamespace export groups the code-related types cleanly. Note thatDestination.Code(from@walkeros/core) is a type alias (Instance<T> | true), while thisCodenamespace contains the implementation types (Settings, OnFn, etc.)—the distinction is clear but worth being aware of when importing.packages/core/src/types/on.ts (1)
75-84: OnFn signature provides type-safe lifecycle event handling with complete context.The updated signature correctly implements
(type: Types, context: Destination.Context<T>), enabling destinations to handle lifecycle events (consent, session, ready, run) with full access to collector state, configuration, environment, and logger. Generic typing withT extends Destination.TypesGenericensures type safety. Tests confirm proper integration with context containingdata,collector,config,env, andlogger. Sinceonis optional in the Destination interface, implementations vary by destination need.packages/collector/src/on.ts (1)
81-95: Well-structured destination context with scoped logging.The implementation correctly builds a rich
Destination.Contextwith proper logger scoping for observability. The chainscope(destType).scope('on').scope(type)provides excellent traceability for debugging destination-specific on-event issues.packages/collector/src/__tests__/destination.test.ts (3)
44-47: Proper mock logger setup with scope chaining.The mock logger setup correctly simulates the scope chaining behavior, allowing tests to verify that lifecycle methods properly scope their loggers.
123-163: Good lifecycle logging tests.These tests verify that init and push operations properly log their lifecycle events with the scoped logger. The assertions confirm both the scope call with destination type and the debug messages for start/end of each lifecycle.
205-214: Updated context expectations align with on.ts changes.The test expectations now correctly verify the full structured context object passed to destination
onmethods, includingdata,collector,config,env, andloggerfields. This aligns with the changes inon.ts.packages/collector/src/types/code.ts (2)
1-1: Correct use ofimport typefor type-only imports.Following the coding guidelines, the file correctly uses
import typefor type-only imports, ensuring proper TypeScript compilation and tree-shaking.
10-28: Well-structured type definitions following core patterns.The type aliases and function signatures are well-defined, properly leveraging the generic
Destination.Typespattern from@walkeros/core. This enables type-safe usage of the code destination throughout the codebase.packages/collector/src/destination-code.ts (2)
48-58: Theonmethod signature is intentional and correctly implements the coreOn.OnFntype, which explicitly specifies baseDestination.Contextrather than specialized contexts. This design is appropriate becauseon()serves as a generic event handler for multiple event types (consent, session, ready, run) rather than a push-specific operation. The pattern is consistent across all destination implementations.
4-18: The code destination is an intentional feature, not a security vulnerability.This destination is explicitly designed for developers to execute custom code within the collector. Code strings come from application configuration (not untrusted input), and developers opt-in by setting
code: true. The use ofnew Function()is appropriate here—this is similar to accepting plugins or custom handlers. Whilenew Function()does execute arbitrary code, the threat model assumes trusted, developer-controlled configuration sources, not external or user-supplied input.Likely an incorrect or invalid review comment.
|
|
||
| // Persist Claude Code data across container rebuilds (uses existing .claude folder in repo) | ||
| "containerEnv": { | ||
| "CLAUDE_CONFIG_DIR": "/workspaces/walkerOS/.claude/data" | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -t d "\.claude" --max-depth 3Repository: elbwalker/walkerOS
Length of output: 44
🏁 Script executed:
# Check the entire devcontainer.json file to see postCreateCommand and other build steps
cat -n .devcontainer/devcontainer.jsonRepository: elbwalker/walkerOS
Length of output: 2149
🏁 Script executed:
# Search for any references to .claude or claude initialization in build/setup scripts
rg -r "\.claude" --type json --type sh --type bashRepository: elbwalker/walkerOS
Length of output: 458
🏁 Script executed:
cat -n .gitignore | grep -A 2 -B 2 "claude" || echo "No 'claude' references in .gitignore"Repository: elbwalker/walkerOS
Length of output: 164
🏁 Script executed:
# Search for any documentation or setup scripts that might initialize .claude
find . -type f \( -name "*.md" -o -name "setup*" -o -name "init*" \) | head -20 | xargs grep -l "claude" 2>/dev/null || echo "No setup/init files reference claude"Repository: elbwalker/walkerOS
Length of output: 73
🏁 Script executed:
# Check if .claude is in .gitignore at all
cat .gitignore 2>/dev/null | head -50Repository: elbwalker/walkerOS
Length of output: 879
🏁 Script executed:
# Check CLAUDE.md for any setup guidance
head -50 CLAUDE.mdRepository: elbwalker/walkerOS
Length of output: 485
Create .claude/data directory during container initialization.
The CLAUDE_CONFIG_DIR environment variable is set to /workspaces/walkerOS/.claude/data, but the postCreateCommand does not create this directory. While the devcontainer comment references "existing .claude folder in repo," .claude/data/ is in .gitignore and does not exist. Add directory creation to postCreateCommand to ensure Claude Code can initialize its configuration:
"postCreateCommand": "mkdir -p .claude/data && npm install && npm install -g @anthropic-ai/claude-code && npm run build"
🤖 Prompt for AI Agents
In .devcontainer/devcontainer.json around lines 9 to 13, the CLAUDE_CONFIG_DIR
points to /workspaces/walkerOS/.claude/data but the postCreateCommand does not
create that directory, so add directory creation to ensure the path exists
before installing and building; update postCreateCommand to run a mkdir -p
.claude/data (or equivalent cross-platform creation) first, then run the
existing npm install, globally install @anthropic-ai/claude-code, and npm run
build in the same command sequence.
| npx serve . -p 3000 | ||
| ``` | ||
|
|
||
| 5. Open http://localhost:3000 and click buttons |
There was a problem hiding this comment.
Fix bare URL per markdownlint MD034 rule.
The bare URL on line 32 should be wrapped in angle brackets or formatted as a markdown link for compliance.
Apply this diff:
-5. Open http://localhost:3000 and click buttons
+5. Open <http://localhost:3000> and click buttons📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 5. Open http://localhost:3000 and click buttons | |
| 5. Open <http://localhost:3000> and click buttons |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
32-32: Bare URL used
(MD034, no-bare-urls)
🤖 Prompt for AI Agents
In apps/demos/flows/README.md around line 32, the bare URL
"http://localhost:3000" violates markdownlint MD034; update the line to wrap the
URL in angle brackets or convert it to a markdown link (for example:
<http://localhost:3000> or [http://localhost:3000](http://localhost:3000)) so
the README complies with MD034.
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (4)
packages/server/sources/aws/examples/basic-handler.ts (1)
16-22: Consider a type alias for the handler signature.The explicit function type is verbose. A type alias would improve readability and reusability.
+// Handler type for reuse +type LambdaHandler = ( + event: APIGatewayProxyEvent, + context: Context, +) => Promise<APIGatewayProxyResult>; + // Handler singleton - reused across warm invocations -let handler: - | (( - event: APIGatewayProxyEvent, - context: Context, - ) => Promise<APIGatewayProxyResult>) - | null = null; +let handler: LambdaHandler | null = null;packages/server/sources/aws/src/lambda/types.ts (1)
23-25: Convert empty interface to type alias per static analysis.An empty interface is equivalent to
{}. Use a type alias for clarity, or add a comment explaining this is a placeholder for future mapping properties.-export interface Mapping { - // Custom source event mapping properties -} +// Placeholder for custom source event mapping properties +export type Mapping = Record<string, never>;packages/server/sources/aws/src/lambda/__tests__/index.test.ts (1)
871-888: Consider testing the specific error message for invalid timeout.The test verifies that an invalid timeout throws, but doesn't assert on the error message. This could help catch regressions in validation messaging.
await expect( sourceLambda( { settings: { timeout: 999999999, // Exceeds 900000ms Lambda limit }, }, { push: mockPush as never, command: mockCommand as never, elb: jest.fn() as never, logger: mockLogger, }, ), - ).rejects.toThrow(); + ).rejects.toThrow(/timeout/i);packages/server/sources/aws/src/lambda/utils.ts (1)
16-54: Consider extracting header normalization to reduce duplication.The header normalization logic (lines 18-23 and 32-37) is duplicated between the v1 and v2 branches.
+function normalizeHeaders(headers: Record<string, string | undefined> | undefined): Record<string, string> { + const normalized: Record<string, string> = {}; + if (headers) { + Object.entries(headers).forEach(([key, value]) => { + if (value) normalized[key.toLowerCase()] = value; + }); + } + return normalized; +} + export function parseEvent(event: LambdaEvent): ParsedRequest { if (isAPIGatewayV2(event)) { - const headers: Record<string, string> = {}; - if (event.headers) { - Object.entries(event.headers).forEach(([key, value]) => { - if (value) headers[key.toLowerCase()] = value; - }); - } + const headers = normalizeHeaders(event.headers); return { // ... }; } else { - const headers: Record<string, string> = {}; - if (event.headers) { - Object.entries(event.headers).forEach(([key, value]) => { - if (value) headers[key.toLowerCase()] = value; - }); - } + const headers = normalizeHeaders(event.headers); // ... } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
packages/server/sources/aws/CHANGELOG.md(1 hunks)packages/server/sources/aws/README.md(1 hunks)packages/server/sources/aws/examples/README.md(1 hunks)packages/server/sources/aws/examples/basic-handler.ts(1 hunks)packages/server/sources/aws/examples/cdk-stack.ts(1 hunks)packages/server/sources/aws/examples/sam-template.yaml(1 hunks)packages/server/sources/aws/examples/serverless.yml(1 hunks)packages/server/sources/aws/jest.config.mjs(1 hunks)packages/server/sources/aws/package.json(1 hunks)packages/server/sources/aws/src/dev.ts(1 hunks)packages/server/sources/aws/src/index.ts(1 hunks)packages/server/sources/aws/src/lambda/__tests__/index.test.ts(1 hunks)packages/server/sources/aws/src/lambda/examples/env.ts(1 hunks)packages/server/sources/aws/src/lambda/examples/events.ts(1 hunks)packages/server/sources/aws/src/lambda/examples/index.ts(1 hunks)packages/server/sources/aws/src/lambda/examples/inputs.ts(1 hunks)packages/server/sources/aws/src/lambda/examples/outputs.ts(1 hunks)packages/server/sources/aws/src/lambda/index.ts(1 hunks)packages/server/sources/aws/src/lambda/push.ts(1 hunks)packages/server/sources/aws/src/lambda/schemas/index.ts(1 hunks)packages/server/sources/aws/src/lambda/schemas/primitives.ts(1 hunks)packages/server/sources/aws/src/lambda/schemas/settings.ts(1 hunks)packages/server/sources/aws/src/lambda/types.ts(1 hunks)packages/server/sources/aws/src/lambda/utils.ts(1 hunks)packages/server/sources/aws/tsconfig.json(1 hunks)packages/server/sources/aws/tsup.config.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- packages/server/sources/aws/CHANGELOG.md
- packages/server/sources/aws/examples/README.md
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENT.md)
**/*.{ts,tsx}: NEVER useanytype - always use proper TypeScript types from @walkeros/core or package-specific types
Implement event structure with consistent properties: name (ENTITY ACTION format), data (entity-specific), context (state/environment), globals (global properties), user (identification), nested (related entities), consent, and system-generated fields (id, timestamp, entity, action)
Use core functions from @walkeros/core for common operations: getEvent(), createEvent() for event creation, getMappingEvent(), getMappingValue() for transformations, isString(), isObject(), isDefined() for type checking, assign(), clone() for object operations, tryCatch(), tryCatchAsync() for error handling
Useimport typefor type-only imports to ensure proper TypeScript compilation and tree-shaking
Avoid inline change comments like '// Fixed for version X' or '// Changed due to Y' - Git tracks history, use migrations and documentation for breaking changes instead
Use Root-level Turborepo commands for parallel operations across packages, navigate to individual packages for focused development, and test changes across all affected packages due to core package dependencies
Files:
packages/server/sources/aws/src/lambda/examples/index.tspackages/server/sources/aws/src/lambda/types.tspackages/server/sources/aws/src/lambda/schemas/settings.tspackages/server/sources/aws/src/index.tspackages/server/sources/aws/tsup.config.tspackages/server/sources/aws/src/lambda/push.tspackages/server/sources/aws/src/lambda/examples/events.tspackages/server/sources/aws/src/lambda/examples/outputs.tspackages/server/sources/aws/src/lambda/utils.tspackages/server/sources/aws/src/lambda/schemas/primitives.tspackages/server/sources/aws/src/lambda/examples/inputs.tspackages/server/sources/aws/examples/cdk-stack.tspackages/server/sources/aws/src/lambda/examples/env.tspackages/server/sources/aws/src/dev.tspackages/server/sources/aws/src/lambda/__tests__/index.test.tspackages/server/sources/aws/src/lambda/index.tspackages/server/sources/aws/src/lambda/schemas/index.tspackages/server/sources/aws/examples/basic-handler.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENT.md)
**/*.test.{ts,tsx}: Write component-level integration tests with mocked external APIs and boundaries: test destinations by mocking external APIs (gtag, fbq, etc.), test collector with mocked destinations and sources, test sources with mocked collector interface
Mock external APIs in destination tests (gtag, fbq, etc.) and verify they are called correctly with the expected data - focus on verifying external API calls, not return values
Before documenting code patterns, create functional, executable tests to validate them - use /workspaces/walkerOS/apps/quickstart as the source of truth for working examples
Files:
packages/server/sources/aws/src/lambda/__tests__/index.test.ts
🧠 Learnings (15)
📓 Common learnings
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to **/*.{ts,tsx} : Use core functions from walkeros/core for common operations: getEvent(), createEvent() for event creation, getMappingEvent(), getMappingValue() for transformations, isString(), isObject(), isDefined() for type checking, assign(), clone() for object operations, tryCatch(), tryCatchAsync() for error handling
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to packages/*/src/**/*.ts : Use Higher-Order Functions (HOF) and smart abstractions to avoid redundancy and keep implementations lean and performant
Applied to files:
packages/server/sources/aws/src/lambda/examples/index.ts
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to packages/*/src/**/*.ts : Implement Destination interface with standard structure: type, init(), push(), pushBatch() methods and config object with optional settings, mapping, data, consent, policy, queue, and dryRun properties
Applied to files:
packages/server/sources/aws/src/lambda/types.tspackages/server/sources/aws/src/lambda/push.tspackages/server/sources/aws/tsconfig.json
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to **/*.{ts,tsx} : Use core functions from walkeros/core for common operations: getEvent(), createEvent() for event creation, getMappingEvent(), getMappingValue() for transformations, isString(), isObject(), isDefined() for type checking, assign(), clone() for object operations, tryCatch(), tryCatchAsync() for error handling
Applied to files:
packages/server/sources/aws/src/lambda/types.tspackages/server/sources/aws/src/lambda/push.tspackages/server/sources/aws/src/lambda/examples/events.tspackages/server/sources/aws/src/lambda/utils.tspackages/server/sources/aws/src/lambda/schemas/primitives.tspackages/server/sources/aws/src/lambda/examples/inputs.tspackages/server/sources/aws/tsconfig.json
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: All walkerOS components communicate via push functions: sources use source.push(), collector uses collector.push(), destinations use destination.push(), and ELB uses elb() as the alias for collector.push()
Applied to files:
packages/server/sources/aws/src/lambda/types.tspackages/server/sources/aws/src/lambda/push.tspackages/server/sources/aws/examples/sam-template.yamlpackages/server/sources/aws/examples/cdk-stack.tspackages/server/sources/aws/src/lambda/examples/env.tspackages/server/sources/aws/README.md
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to packages/*/tsconfig.json : Configure TypeScript with strict settings to enforce type safety and prevent use of `any` types
Applied to files:
packages/server/sources/aws/tsup.config.tspackages/server/sources/aws/tsconfig.json
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to **/*.{ts,tsx} : Use `import type` for type-only imports to ensure proper TypeScript compilation and tree-shaking
Applied to files:
packages/server/sources/aws/tsup.config.ts
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to **/*.{ts,tsx} : Implement event structure with consistent properties: name (ENTITY ACTION format), data (entity-specific), context (state/environment), globals (global properties), user (identification), nested (related entities), consent, and system-generated fields (id, timestamp, entity, action)
Applied to files:
packages/server/sources/aws/src/lambda/examples/events.ts
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to packages/*/src/**/*.ts : Implement event mapping with flexible patterns: exact entity-action matches, wildcard patterns using '*', conditional mappings with functions, and value mapping strategies including string key extraction, static values, custom functions, object transformation, array processing, and consent-based mapping
Applied to files:
packages/server/sources/aws/src/lambda/examples/events.tspackages/server/sources/aws/src/lambda/examples/inputs.tspackages/server/sources/aws/tsconfig.json
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to **/*.test.{ts,tsx} : Before documenting code patterns, create functional, executable tests to validate them - use /workspaces/walkerOS/apps/quickstart as the source of truth for working examples
Applied to files:
packages/server/sources/aws/src/lambda/examples/events.tspackages/server/sources/aws/src/lambda/examples/outputs.tspackages/server/sources/aws/src/lambda/__tests__/index.test.ts
📚 Learning: 2025-11-25T16:24:48.032Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T16:24:48.032Z
Learning: Refer to AGENT.md for all relevant instructions and guidelines when working with the walkerOS codebase
Applied to files:
packages/server/sources/aws/examples/sam-template.yamlpackages/server/sources/aws/examples/cdk-stack.ts
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to **/*.test.{ts,tsx} : Mock external APIs in destination tests (gtag, fbq, etc.) and verify they are called correctly with the expected data - focus on verifying external API calls, not return values
Applied to files:
packages/server/sources/aws/src/lambda/examples/outputs.tspackages/server/sources/aws/src/lambda/examples/env.tspackages/server/sources/aws/src/lambda/__tests__/index.test.ts
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to **/*.test.{ts,tsx} : Write component-level integration tests with mocked external APIs and boundaries: test destinations by mocking external APIs (gtag, fbq, etc.), test collector with mocked destinations and sources, test sources with mocked collector interface
Applied to files:
packages/server/sources/aws/src/lambda/examples/outputs.tspackages/server/sources/aws/src/lambda/examples/env.tspackages/server/sources/aws/src/lambda/__tests__/index.test.ts
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to **/*.{ts,tsx} : Use Root-level Turborepo commands for parallel operations across packages, navigate to individual packages for focused development, and test changes across all affected packages due to core package dependencies
Applied to files:
packages/server/sources/aws/tsconfig.json
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to **/*.{ts,tsx} : NEVER use `any` type - always use proper TypeScript types from walkeros/core or package-specific types
Applied to files:
packages/server/sources/aws/tsconfig.json
🧬 Code graph analysis (7)
packages/server/sources/aws/src/lambda/examples/index.ts (1)
packages/cli/demos/programmatic-example.js (1)
events(109-113)
packages/server/sources/aws/src/lambda/schemas/settings.ts (3)
packages/server/sources/aws/src/lambda/schemas/index.ts (2)
SettingsSchema(8-8)Settings(8-8)packages/server/sources/aws/src/lambda/schemas/primitives.ts (1)
CorsOptionsSchema(33-53)packages/server/sources/aws/src/lambda/types.ts (1)
Settings(17-17)
packages/server/sources/aws/src/lambda/push.ts (5)
packages/server/sources/aws/src/lambda/types.ts (1)
EventRequest(57-64)packages/server/sources/aws/src/lambda/examples/env.ts (2)
push(41-52)push(42-44)packages/web/core/src/types/collector.ts (1)
Collector(24-43)packages/cli/src/core/logger.ts (1)
Logger(11-20)packages/core/src/types/walkeros.ts (1)
OrderedProperties(87-89)
packages/server/sources/aws/src/lambda/schemas/primitives.ts (1)
packages/server/sources/aws/src/lambda/types.ts (1)
CorsOptions(18-18)
packages/server/sources/aws/src/lambda/examples/env.ts (3)
packages/core/src/types/walkeros.ts (1)
Elb(4-4)packages/cli/src/core/logger.ts (1)
Logger(11-20)packages/server/sources/aws/src/lambda/types.ts (1)
Env(33-36)
packages/server/sources/aws/src/lambda/schemas/index.ts (1)
packages/server/sources/aws/src/lambda/schemas/settings.ts (1)
SettingsSchema(7-34)
packages/server/sources/aws/examples/basic-handler.ts (2)
packages/collector/src/flow.ts (1)
startFlow(7-59)packages/server/sources/aws/src/lambda/index.ts (1)
sourceLambda(22-170)
🪛 Biome (2.1.2)
packages/server/sources/aws/src/lambda/types.ts
[error] 23-25: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
🪛 markdownlint-cli2 (0.18.1)
packages/server/sources/aws/README.md
183-183: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
266-266: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build and test
🔇 Additional comments (30)
packages/server/sources/aws/README.md (1)
1-299: Comprehensive and well-structured documentation.The README provides thorough coverage of the AWS Lambda source including installation, multiple deployment patterns, configuration options, request/response formats, production considerations, and security guidance. The examples correctly demonstrate the walkerOS pattern of using
startFlow()with sources/destinations and the singleton handler pattern for Lambda warm invocations. The documentation aligns well with walkerOS architecture as reflected in the retrieved learnings.packages/server/sources/aws/jest.config.mjs (1)
1-3: Re-exporting shared Jest config via ESM looks correctImporting the shared Jest config and re‑exporting it as the default keeps test config DRY and consistent across packages. Please just confirm that
@walkeros/config/jestexposes a compatible default export and that this package’s Jest setup is configured to loadjest.config.mjsas an ES module.packages/server/sources/aws/tsconfig.json (1)
1-8: LGTM!The TypeScript configuration properly extends the base Node.js config and defines appropriate source/exclusion paths for the AWS Lambda source package.
packages/server/sources/aws/examples/basic-handler.ts (2)
28-52: LGTM!The singleton initialization pattern is correctly implemented for optimal Lambda warm-start performance. The guard clause prevents re-initialization, and the empty destinations object provides a clear extension point.
54-67: LGTM!The main function correctly delegates to the singleton handler, and the dual export pattern (
mainandhandler) enables both testing and Lambda runtime compatibility.packages/server/sources/aws/src/dev.ts (1)
1-1: LGTM!The namespace export provides a clean entry point for lambda development utilities, aligning with the
./devexport defined in package.json.packages/server/sources/aws/src/lambda/examples/index.ts (1)
1-4: LGTM!Clean barrel file with proper namespace re-exports. This pattern enables convenient imports like
import { env, inputs } from '.../examples'.packages/server/sources/aws/src/index.ts (1)
1-2: LGTM!Clean package entry point that provides both namespace exports and a named alias for the default export, offering flexibility for consumers.
packages/server/sources/aws/src/lambda/schemas/settings.ts (1)
1-36: LGTM!Well-structured Zod schema with:
- Sensible defaults and constraints (timeout max matches AWS Lambda's 15-minute limit)
- Flexible CORS configuration (boolean or detailed options)
- Good use of
.describe()for self-documenting schemas- Proper type inference with
z.inferpackages/server/sources/aws/src/lambda/schemas/index.ts (1)
1-11: LGTM!Clean schema barrel file with proper organization:
- Re-exports primitives and settings
- Uses
typekeyword for type-only export ofSettings- Generates JSON Schema via
zodToSchemafor documentation toolingpackages/server/sources/aws/src/lambda/examples/env.ts (1)
1-52: LGTM!Well-structured mock environment:
- Proper
import typeusage per coding guidelinescreateMockElbFnreturns correctly structuredPushResultnoopLoggerproperly implementsLogger.Instanceincluding the throwing behavior- Getters on
push/command/elbensure fresh mocks per access, which is good for test isolationpackages/server/sources/aws/src/lambda/push.ts (1)
21-28: LGTM! Good structured error handling.The error handling follows best practices with structured logging context and proper
instanceof Errorcheck for the error message extraction.packages/server/sources/aws/src/lambda/schemas/primitives.ts (2)
23-27: LGTM! Clear schema design with good documentation.The
CorsOriginschema clearly documents the allowed values. Whilez.literal('*')is technically redundant withz.string(), it explicitly communicates the wildcard intent which aids readability.
33-53: LGTM! Well-structured CORS options schema.The schema uses appropriate Zod validators (
.int(),.positive()) with descriptive labels for each field. All fields being optional aligns with typical CORS configuration patterns.packages/server/sources/aws/src/lambda/types.ts (1)
1-82: LGTM! Comprehensive type definitions for Lambda source.The type structure follows walkerOS patterns well:
- Proper separation of Lambda-specific types from core types
- Good use of Zod inference for schema-derived types
- Clear convenience aliases for Config types
packages/server/sources/aws/src/lambda/examples/events.ts (1)
9-25: LGTM! Well-structured example events.The example events correctly follow the walkerOS "entity action" naming convention and provide representative test data. Using
Partial<WalkerOS.Event>appropriately indicates these are incomplete payloads for testing.packages/server/sources/aws/src/lambda/__tests__/index.test.ts (3)
114-134: LGTM! Good test setup with proper mocking.The test setup correctly mocks the external push/command interfaces and uses
createMockLoggerfrom core. This follows the coding guidelines for testing sources with mocked collector interfaces.
374-481: LGTM! Comprehensive error handling tests.Good coverage of error scenarios including:
- Missing request body
- Invalid event format
- Processing errors
- Unsupported HTTP methods
Each test verifies both status codes and response body structure.
731-819: LGTM! Proper logging behavior verification.Tests correctly verify:
- No logging on successful requests (delegated to collector)
- Structured error logging with context on failures
- Request ID propagation in error logs
packages/server/sources/aws/src/lambda/utils.ts (3)
10-14: LGTM! Clean type guard implementation.The
isAPIGatewayV2type guard correctly checks for the version property, enabling proper type narrowing in consuming code.
64-74: LGTM! Robust body parsing with base64 support.The function safely handles:
- Non-string inputs (passthrough)
- Base64 decoding when flagged
- JSON parse failures (returns original string)
146-166: LGTM! Well-implemented pixel tracking response.The 1x1 transparent GIF is correctly base64-encoded with appropriate cache-control headers to prevent caching of tracking pixels.
packages/server/sources/aws/src/lambda/index.ts (7)
1-20: LGTM! Clean imports and exports following TypeScript best practices.The use of
import typefor type-only imports ensures proper tree-shaking, and the namespace re-exports provide a clean API surface.
22-34: LGTM! Proper configuration validation with schema parsing.Settings validation happens synchronously before the Lambda handler is created, ensuring invalid configurations are caught early at initialization time rather than during request processing.
35-62: LGTM! Health check and CORS preflight handling are correctly implemented.The health check provides useful diagnostic information, and the OPTIONS handler properly supports CORS preflight requests.
83-136: LGTM! POST event handling with proper validation and error responses.The body validation correctly rejects both falsy values and non-objects, and the
isEventRequesttype guard ensures only valid event requests are processed.
138-163: LGTM! Comprehensive error handling with structured logging.The fallback for unsupported methods and the catch block properly handle edge cases. The structured logging with context (error, requestId, method) and optional chaining for potentially undefined values demonstrate defensive programming.
165-172: LGTM! Clean return structure with default export.The returned
LambdaSourceobject provides a well-structured API, and the default export enables flexible import patterns for consumers.
64-81: Fix null handling in pixel tracking validation.The type check
typeof parsedData === 'object'will incorrectly pass fornullbecause in JavaScript,typeof null === 'object'is true. This could result inenvPushbeing called withnull.Apply this diff to explicitly exclude null:
if (parsed.queryString) { const parsedData = requestToData(parsed.queryString); - if (parsedData && typeof parsedData === 'object') { + if (parsedData && typeof parsedData === 'object' && parsedData !== null) { await envPush(parsedData); } }Alternatively, use a more robust check:
if (parsed.queryString) { const parsedData = requestToData(parsed.queryString); - if (parsedData && typeof parsedData === 'object') { + if (parsedData && isObject(parsedData)) { await envPush(parsedData); } }Based on coding guidelines, prefer using the
isObject()helper from@walkeros/corefor type checking.Likely an incorrect or invalid review comment.
packages/server/sources/aws/src/lambda/examples/inputs.ts (1)
1-144: LGTM! Comprehensive example events for testing and documentation.The examples provide excellent coverage across multiple dimensions:
- Both API Gateway v1 (REST API) and v2 (HTTP API) formats
- Various HTTP methods (POST, GET)
- Normal operation, health checks, and error scenarios
- Edge cases like malformed JSON and missing required fields
The use of spread operators to derive related examples (like
healthCheckEvent) is a clean pattern that maintains consistency while avoiding duplication.
| // API Gateway | ||
| const api = new apigateway.RestApi(this, 'WalkerAPI', { | ||
| restApiName: 'walkerOS Collector', | ||
| description: 'walkerOS event collection API', | ||
| defaultCorsPreflightOptions: { | ||
| allowOrigins: apigateway.Cors.ALL_ORIGINS, | ||
| allowMethods: apigateway.Cors.ALL_METHODS, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Restrict CORS configuration for production deployments.
The API Gateway allows all origins and methods, which is overly permissive. For production, restrict to specific domains and necessary HTTP methods.
Apply this diff:
// API Gateway
const api = new apigateway.RestApi(this, 'WalkerAPI', {
restApiName: 'walkerOS Collector',
description: 'walkerOS event collection API',
defaultCorsPreflightOptions: {
- allowOrigins: apigateway.Cors.ALL_ORIGINS,
- allowMethods: apigateway.Cors.ALL_METHODS,
+ // Production: Replace with specific domains
+ allowOrigins: ['https://yourdomain.com'],
+ allowMethods: ['GET', 'POST', 'OPTIONS'],
},
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // API Gateway | |
| const api = new apigateway.RestApi(this, 'WalkerAPI', { | |
| restApiName: 'walkerOS Collector', | |
| description: 'walkerOS event collection API', | |
| defaultCorsPreflightOptions: { | |
| allowOrigins: apigateway.Cors.ALL_ORIGINS, | |
| allowMethods: apigateway.Cors.ALL_METHODS, | |
| }, | |
| }); | |
| // API Gateway | |
| const api = new apigateway.RestApi(this, 'WalkerAPI', { | |
| restApiName: 'walkerOS Collector', | |
| description: 'walkerOS event collection API', | |
| defaultCorsPreflightOptions: { | |
| // Production: Replace with specific domains | |
| allowOrigins: ['https://yourdomain.com'], | |
| allowMethods: ['GET', 'POST', 'OPTIONS'], | |
| }, | |
| }); |
🤖 Prompt for AI Agents
In packages/server/sources/aws/examples/cdk-stack.ts around lines 25 to 33, the
API Gateway CORS is currently set to allow all origins and methods; update it to
read allowed origins and allowed methods from configuration or environment
(e.g., process.env.ALLOWED_ORIGINS as a comma-separated list and
process.env.ALLOWED_METHODS), then pass a specific array of origins to
defaultCorsPreflightOptions.allowOrigins and a specific array of apigateway.Cors
methods (e.g., GET, POST, OPTIONS) to allowMethods; include a safe development
fallback (e.g., localhost) when env vars are unset and ensure you do not use
Cors.ALL_ORIGINS or Cors.ALL_METHODS in production.
| // Function URL (simpler, lower cost than API Gateway) | ||
| const fnUrl = collectorFn.addFunctionUrl({ | ||
| authType: lambda.FunctionUrlAuthType.NONE, | ||
| cors: { | ||
| allowedOrigins: ['*'], | ||
| allowedMethods: [lambda.HttpMethod.GET, lambda.HttpMethod.POST], | ||
| allowedHeaders: ['Content-Type', 'Authorization'], | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Add authentication and restrict CORS origins.
The Function URL has no authentication (authType: NONE) and allows all origins ('*'), making it publicly accessible without protection. This is simpler than API Gateway but sacrifices security controls.
Consider:
const fnUrl = collectorFn.addFunctionUrl({
- authType: lambda.FunctionUrlAuthType.NONE,
+ // Production: Use AWS_IAM for authenticated access
+ authType: lambda.FunctionUrlAuthType.AWS_IAM,
cors: {
- allowedOrigins: ['*'],
+ // Production: Specify allowed domains
+ allowedOrigins: ['https://yourdomain.com'],
allowedMethods: [lambda.HttpMethod.GET, lambda.HttpMethod.POST],
allowedHeaders: ['Content-Type', 'Authorization'],
},
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Function URL (simpler, lower cost than API Gateway) | |
| const fnUrl = collectorFn.addFunctionUrl({ | |
| authType: lambda.FunctionUrlAuthType.NONE, | |
| cors: { | |
| allowedOrigins: ['*'], | |
| allowedMethods: [lambda.HttpMethod.GET, lambda.HttpMethod.POST], | |
| allowedHeaders: ['Content-Type', 'Authorization'], | |
| }, | |
| }); | |
| // Function URL (simpler, lower cost than API Gateway) | |
| const fnUrl = collectorFn.addFunctionUrl({ | |
| // Production: Use AWS_IAM for authenticated access | |
| authType: lambda.FunctionUrlAuthType.AWS_IAM, | |
| cors: { | |
| // Production: Specify allowed domains | |
| allowedOrigins: ['https://yourdomain.com'], | |
| allowedMethods: [lambda.HttpMethod.GET, lambda.HttpMethod.POST], | |
| allowedHeaders: ['Content-Type', 'Authorization'], | |
| }, | |
| }); |
| Resources: | ||
| WalkerOSFunction: | ||
| Type: AWS::Serverless::Function | ||
| Properties: | ||
| CodeUri: ./dist | ||
| Handler: index.handler | ||
| Events: | ||
| # POST endpoint for event collection | ||
| CollectPost: | ||
| Type: Api | ||
| Properties: | ||
| Path: /collect | ||
| Method: POST | ||
|
|
||
| # GET endpoint for pixel tracking | ||
| CollectGet: | ||
| Type: Api | ||
| Properties: | ||
| Path: /collect | ||
| Method: GET | ||
|
|
||
| # Health check endpoint | ||
| Health: | ||
| Type: Api | ||
| Properties: | ||
| Path: /health | ||
| Method: GET |
There was a problem hiding this comment.
Consider adding API Gateway throttling and authentication.
The template deploys publicly accessible endpoints without authentication or rate limiting. While this is an example, production deployments should consider:
- API Gateway usage plans with throttling limits
- API keys or IAM authorization
- WAF rules for DDoS protection
This prevents abuse, cost runaway, and data injection.
Add configuration comments in the template to guide users:
Resources:
WalkerOSFunction:
Type: AWS::Serverless::Function
Properties:
CodeUri: ./dist
Handler: index.handler
+ # Production: Consider adding Reserved Concurrent Executions to limit costs
+ # ReservedConcurrentExecutions: 10
Events:
# POST endpoint for event collection
+ # Production: Add API Gateway usage plan with throttling
CollectPost:
Type: Api
Properties:
Path: /collect
Method: POST
+ # Auth: AWS_IAMCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/server/sources/aws/examples/sam-template.yaml around lines 16 to 42,
the API endpoints are deployed publicly without throttling or auth; update the
template to include commented guidance and example resources: add an API Gateway
Stage/UsagePlan + Throttle settings (rate/burst) and show how to attach a
UsagePlan and ApiKey to limit requests, add an Auth block for the Api events (or
a reference to an Authorizer/IAM auth) and mark sensitive endpoints as
ApiKeyRequired/Use Cognito/IAM authorizer, and include a WebAclAssociation
resource example (or a note) to attach an AWS WAF web ACL for basic DDoS/SQL/XSS
protections; keep these as commented examples so users can opt-in for production
deployments.
| functions: | ||
| collector: | ||
| handler: dist/index.handler | ||
| events: | ||
| # HTTP API (v2) - recommended for lower cost | ||
| - httpApi: | ||
| path: /collect | ||
| method: post | ||
| - httpApi: | ||
| path: /collect | ||
| method: get | ||
| - httpApi: | ||
| path: /health | ||
| method: get | ||
|
|
||
| # CloudWatch Logs | ||
| logRetentionInDays: 7 | ||
|
|
||
| # Reserved concurrency to prevent runaway costs | ||
| # reservedConcurrency: 10 |
There was a problem hiding this comment.
Add authorization and consider enabling cost controls.
The HTTP API events lack authorization, allowing public access without authentication. Additionally, the commented reservedConcurrency should be considered for production to prevent cost runaway.
Apply this diff:
functions:
collector:
handler: dist/index.handler
events:
# HTTP API (v2) - recommended for lower cost
- httpApi:
path: /collect
method: post
+ # Production: Add authorization
+ # authorizer:
+ # type: aws_iam
- httpApi:
path: /collect
method: get
+ # authorizer:
+ # type: aws_iam
- httpApi:
path: /health
method: get
# CloudWatch Logs
logRetentionInDays: 7
# Reserved concurrency to prevent runaway costs
- # reservedConcurrency: 10
+ # Production: Uncomment to limit concurrent executions
+ reservedConcurrency: 10📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| functions: | |
| collector: | |
| handler: dist/index.handler | |
| events: | |
| # HTTP API (v2) - recommended for lower cost | |
| - httpApi: | |
| path: /collect | |
| method: post | |
| - httpApi: | |
| path: /collect | |
| method: get | |
| - httpApi: | |
| path: /health | |
| method: get | |
| # CloudWatch Logs | |
| logRetentionInDays: 7 | |
| # Reserved concurrency to prevent runaway costs | |
| # reservedConcurrency: 10 | |
| functions: | |
| collector: | |
| handler: dist/index.handler | |
| events: | |
| # HTTP API (v2) - recommended for lower cost | |
| - httpApi: | |
| path: /collect | |
| method: post | |
| # Production: Add authorization | |
| # authorizer: | |
| # type: aws_iam | |
| - httpApi: | |
| path: /collect | |
| method: get | |
| # authorizer: | |
| # type: aws_iam | |
| - httpApi: | |
| path: /health | |
| method: get | |
| # CloudWatch Logs | |
| logRetentionInDays: 7 | |
| # Reserved concurrency to prevent runaway costs | |
| # Production: Uncomment to limit concurrent executions | |
| reservedConcurrency: 10 |
🤖 Prompt for AI Agents
In packages/server/sources/aws/examples/serverless.yml around lines 22 to 41,
the HTTP API events are exposed without any authorization and
reservedConcurrency is only commented out; update each httpApi event for the
collector function to require an authorizer (e.g., reference an existing
JWT/Cognito/Custom authorizer by name or add a new authorizer block and
reference it under each httpApi event) and enable reservedConcurrency for
production (uncomment and set a sensible default like 5–10 or wire it to a
config variable), ensuring the authorizer is defined elsewhere in the template
and adding a short comment that reservedConcurrency should be tuned per
environment.
| "dependencies": { | ||
| "@walkeros/core": "0.4.2" | ||
| }, | ||
| "peerDependencies": { | ||
| "@types/aws-lambda": "^8.10.0" | ||
| }, | ||
| "devDependencies": { | ||
| "@types/aws-lambda": "^8.10.145" | ||
| }, |
There was a problem hiding this comment.
Clarify @types/aws-lambda dependency strategy.
The package lists @types/aws-lambda in both peerDependencies and devDependencies, which is unusual. Typically, type packages are only in devDependencies unless the package exports types that consumers need to reference directly.
If consumers don't need the AWS Lambda types directly (they only use your package's types), remove from peerDependencies:
"dependencies": {
"@walkeros/core": "0.4.2"
},
- "peerDependencies": {
- "@types/aws-lambda": "^8.10.0"
- },
"devDependencies": {
"@types/aws-lambda": "^8.10.145"
},If consumers do need to reference AWS Lambda types (e.g., your public API exports APIGatewayProxyEvent), keep the peer dependency but document this requirement in the README.
🤖 Prompt for AI Agents
In packages/server/sources/aws/package.json around lines 22 to 30, the
@types/aws-lambda type package is listed in both peerDependencies and
devDependencies which is redundant; inspect the package's public type exports—if
your public API does not directly expose AWS Lambda types, remove
@types/aws-lambda from peerDependencies and keep it only in devDependencies; if
your public API does export AWS Lambda types (e.g., APIGatewayProxyEvent) then
keep @types/aws-lambda in peerDependencies and remove it from devDependencies,
and add a short note to the package README stating that consumers must install
@types/aws-lambda (including a suggested version range).
| body: expect.stringContaining('"success":true'), | ||
| }; |
There was a problem hiding this comment.
Jest matchers used in non-test module may cause runtime errors.
This file uses expect.stringContaining and expect.objectContaining, which are Jest runtime objects. Since this file is in the examples/ directory (not __tests__/), it may be imported outside a Jest test context where expect is undefined, causing a ReferenceError.
Additionally, the type Partial<APIGatewayProxyResult> is misleading since body should be string, not a Jest AsymmetricMatcher.
Consider one of these approaches:
- Move to
__tests__/directory if these are only used in tests. - Use plain values and perform assertions in test files:
- body: expect.stringContaining('"success":true'),
+ body: JSON.stringify({ success: true }),- Export a helper function that creates matchers when called from tests:
// In test files:
import { successResponse } from './outputs';
expect(result).toMatchObject({
...successResponse,
body: expect.stringContaining('"success":true'),
});Also applies to: 21-21, 27-29, 36-36
| const result = await push({ | ||
| name: eventReq.event, | ||
| data: (eventReq.data || {}) as WalkerOS.Properties, | ||
| context: eventReq.context as WalkerOS.OrderedProperties | undefined, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the push.ts file and examine its imports and type usage
find . -type f -name "push.ts" -path "*aws*lambda*" | head -5Repository: elbwalker/walkerOS
Length of output: 110
🏁 Script executed:
# Find EventRequest type definition
rg -t ts "type EventRequest|interface EventRequest" --max-count=5Repository: elbwalker/walkerOS
Length of output: 305
🏁 Script executed:
# Find OrderedProperties type definition
rg -t ts "type OrderedProperties|interface OrderedProperties" --max-count=5Repository: elbwalker/walkerOS
Length of output: 134
🏁 Script executed:
# Examine the push.ts file to see full context
find . -type f -name "push.ts" -path "*aws*lambda*" -exec wc -l {} \;Repository: elbwalker/walkerOS
Length of output: 113
🏁 Script executed:
# Read the push.ts file in full
cat -n ./packages/server/sources/aws/src/lambda/push.tsRepository: elbwalker/walkerOS
Length of output: 1289
🏁 Script executed:
# Read EventRequest type from AWS lambda types
cat -n ./packages/server/sources/aws/src/lambda/types.tsRepository: elbwalker/walkerOS
Length of output: 2741
🏁 Script executed:
# Read OrderedProperties type definition
cat -n ./packages/core/src/types/walkeros.ts | head -100Repository: elbwalker/walkerOS
Length of output: 2993
🏁 Script executed:
# Find push function definition to see what it expects for context parameter
rg -t ts "export.*function push|const push.*=" -A 10 | head -50Repository: elbwalker/walkerOS
Length of output: 3415
🏁 Script executed:
# Check Express EventRequest to see if context handling is different
cat -n ./packages/server/sources/express/src/types.tsRepository: elbwalker/walkerOS
Length of output: 2259
🏁 Script executed:
# Check GCP EventRequest to see if context handling is different
cat -n ./packages/server/sources/gcp/src/cloudfunction/types.tsRepository: elbwalker/walkerOS
Length of output: 2525
🏁 Script executed:
# Check how Collector.PushFn is defined to see what it expects
rg -t ts "type PushFn|interface PushFn" -A 5 packages/coreRepository: elbwalker/walkerOS
Length of output: 856
🏁 Script executed:
# Search for EventRequest parsing/validation to see if there's transformation
rg -t ts "EventRequest" packages/server/sources/aws/src/lambda/ -B 2 -A 2 | grep -E "(parse|transform|validate|EventRequest)" | head -20Repository: elbwalker/walkerOS
Length of output: 1941
🏁 Script executed:
# Check if there's any validation schema for EventRequest
find ./packages/server/sources/aws/src/lambda -name "*.ts" -exec grep -l "schema\|validate\|zod" {} \;Repository: elbwalker/walkerOS
Length of output: 393
🏁 Script executed:
# Look at how the HTTP body is parsed into EventRequest in the lambda handler
rg -t ts "parseEvent\|parseBody\|JSON.parse" packages/server/sources/aws/src/lambda/ -B 2 -A 5 | head -40Repository: elbwalker/walkerOS
Length of output: 44
🏁 Script executed:
# Read the isEventRequest function and surrounding context
cat -n ./packages/server/sources/aws/src/lambda/utils.tsRepository: elbwalker/walkerOS
Length of output: 5660
🏁 Script executed:
# Check the schemas for any EventRequest validation
cat -n ./packages/server/sources/aws/src/lambda/schemas/index.tsRepository: elbwalker/walkerOS
Length of output: 507
🏁 Script executed:
# Look at test cases to see what format is expected for EventRequest
cat -n ./packages/server/sources/aws/src/lambda/__tests__/index.test.ts | head -100Repository: elbwalker/walkerOS
Length of output: 3464
🏁 Script executed:
# Check if there's any context transformation or validation elsewhere in the code
rg -t ts "context.*\[" packages/server/sources/aws/src/lambda/ -B 2 -A 2Repository: elbwalker/walkerOS
Length of output: 44
🏁 Script executed:
# Check how push function actually uses context to understand if the bug manifests
rg -t ts "function push|const push" packages/core/src -A 20 | grep -A 20 "context" | head -30Repository: elbwalker/walkerOS
Length of output: 44
Unsafe type cast: context from AnyObject to OrderedProperties without validation.
EventRequest.context is typed as WalkerOS.AnyObject but cast to WalkerOS.OrderedProperties without any transformation. The isEventRequest guard only validates that event is a string—it does not enforce that context matches the required OrderedProperties structure { [key: string]: [Property, number] | undefined }.
A client could send { event: "test", context: { foo: "bar" } } and it would pass validation, then fail at runtime if the collector expects context values to be tuples with counts.
Either validate and transform context to the correct format before casting, or update EventRequest.context type to WalkerOS.OrderedProperties | undefined if the input is guaranteed to be in that format.
🤖 Prompt for AI Agents
In packages/server/sources/aws/src/lambda/push.ts around line 14, the code
unsafely casts eventReq.context (WalkerOS.AnyObject) to
WalkerOS.OrderedProperties without validation; either validate/transform context
at runtime to the required shape or change the EventRequest.context type to
allow OrderedProperties | undefined. Fix by adding a runtime guard that iterates
eventReq.context keys, verifies each value is a two-element tuple [Property,
number] (and transforms or rejects entries that are invalid), then assign the
resulting safe object to the context variable (or set it to undefined) before
passing it on; alternatively update the EventRequest.context TypeScript type to
WalkerOS.OrderedProperties | undefined if you can guarantee callers supply the
correct shape and remove the unsafe cast.
| if (corsOptions.origin) { | ||
| const origin = Array.isArray(corsOptions.origin) | ||
| ? corsOptions.origin.join(', ') | ||
| : corsOptions.origin; | ||
| headers['Access-Control-Allow-Origin'] = origin; | ||
| } |
There was a problem hiding this comment.
CORS multi-origin handling may not work as expected.
When corsOptions.origin is an array, joining with comma produces "origin1, origin2" which is not valid for Access-Control-Allow-Origin. The spec only allows a single origin or *.
For multiple allowed origins, the standard pattern is to check the request's Origin header against the allowlist and return that specific origin if it matches.
- if (corsOptions.origin) {
- const origin = Array.isArray(corsOptions.origin)
- ? corsOptions.origin.join(', ')
- : corsOptions.origin;
- headers['Access-Control-Allow-Origin'] = origin;
- }
+ if (corsOptions.origin) {
+ // For array origins, you'd need to check against request Origin header
+ // and return the matching one. For now, take the first or use '*'.
+ const origin = Array.isArray(corsOptions.origin)
+ ? corsOptions.origin[0] // Consider passing request origin for dynamic matching
+ : corsOptions.origin;
+ headers['Access-Control-Allow-Origin'] = origin;
+ }A proper solution would pass the request's Origin header to getCorsHeaders and match against the allowlist.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (corsOptions.origin) { | |
| const origin = Array.isArray(corsOptions.origin) | |
| ? corsOptions.origin.join(', ') | |
| : corsOptions.origin; | |
| headers['Access-Control-Allow-Origin'] = origin; | |
| } | |
| if (corsOptions.origin) { | |
| // For array origins, you'd need to check against request Origin header | |
| // and return the matching one. For now, take the first or use '*'. | |
| const origin = Array.isArray(corsOptions.origin) | |
| ? corsOptions.origin[0] // Consider passing request origin for dynamic matching | |
| : corsOptions.origin; | |
| headers['Access-Control-Allow-Origin'] = origin; | |
| } |
🤖 Prompt for AI Agents
In packages/server/sources/aws/src/lambda/utils.ts around lines 100 to 105, the
current code joins an array of allowed origins into a comma string and sets
Access-Control-Allow-Origin, which is invalid; instead change getCorsHeaders to
accept the incoming request's Origin header (or accept an origin parameter), and
when corsOptions.origin is an array validate the request Origin against that
allowlist and set Access-Control-Allow-Origin to the matching origin only (or
omit the header/fall back to safe default) rather than joining the list; keep
existing behavior for string or '*' origins.
| const responseHeaders: Record<string, string> = { | ||
| 'Content-Type': | ||
| typeof body === 'object' ? 'application/json' : 'text/plain', | ||
| ...headers, | ||
| }; |
There was a problem hiding this comment.
Potential issue: typeof null === 'object' in JavaScript.
The check typeof body === 'object' returns true for null, which would set Content-Type: application/json and call JSON.stringify(null) (producing "null"). This may not be the intended behavior.
const responseHeaders: Record<string, string> = {
'Content-Type':
- typeof body === 'object' ? 'application/json' : 'text/plain',
+ typeof body === 'object' && body !== null ? 'application/json' : 'text/plain',
...headers,
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const responseHeaders: Record<string, string> = { | |
| 'Content-Type': | |
| typeof body === 'object' ? 'application/json' : 'text/plain', | |
| ...headers, | |
| }; | |
| const responseHeaders: Record<string, string> = { | |
| 'Content-Type': | |
| typeof body === 'object' && body !== null ? 'application/json' : 'text/plain', | |
| ...headers, | |
| }; |
🤖 Prompt for AI Agents
In packages/server/sources/aws/src/lambda/utils.ts around lines 128 to 132, the
Content-Type check uses typeof body === 'object' which is true for null; change
the condition to explicitly exclude null (e.g. body !== null && typeof body ===
'object') so null is not treated as JSON, update the header selection
accordingly, and ensure any later JSON.stringify usage uses the same null-aware
check.
| export default defineConfig([ | ||
| buildModules({ | ||
| terserOptions: {}, // Don't mangle here | ||
| }), |
There was a problem hiding this comment.
Clarify terser configuration to match intent.
The comment indicates mangling should be disabled, but an empty terserOptions object typically enables terser with default settings (which includes mangling). To preserve function/variable names for better Lambda debugging and stack traces, explicitly disable mangling.
Apply this diff:
export default defineConfig([
buildModules({
- terserOptions: {}, // Don't mangle here
+ terserOptions: {
+ mangle: false, // Preserve names for Lambda debugging
+ },
}),
]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export default defineConfig([ | |
| buildModules({ | |
| terserOptions: {}, // Don't mangle here | |
| }), | |
| export default defineConfig([ | |
| buildModules({ | |
| terserOptions: { | |
| mangle: false, // Preserve names for Lambda debugging | |
| }, | |
| }), |
🤖 Prompt for AI Agents
In packages/server/sources/aws/tsup.config.ts around lines 3 to 6, the config
currently passes an empty terserOptions which enables Terser defaults (including
mangling) despite the comment; update terserOptions to explicitly disable
mangling and preserve names (e.g. set mangle: false and keep_fnames: true (and
optionally keep_classnames: true)) so function/variable/class names are
preserved for Lambda debugging and stack traces.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
packages/core/src/flow.ts (1)
98-111: Harden identifier generation for edge cases.As flagged in a previous review, this function can generate invalid JavaScript identifiers for unscoped packages starting with digits (e.g.,
"123-utils"→"123Utils"). While scoped packages like"@123/utils"are safe due to the leading underscore, unscoped numeric-leading names are not.Apply the previously suggested fix to guard against invalid identifiers:
export function packageNameToVariable(packageName: string): string { const hasScope = packageName.startsWith('@'); const normalized = packageName - .replace('@', '') + .replace(/^@/, '') .replace(/[/-]/g, '_') .split('_') .filter((part) => part.length > 0) .map((part, i) => i === 0 ? part : part.charAt(0).toUpperCase() + part.slice(1), ) .join(''); - return hasScope ? '_' + normalized : normalized; + const base = hasScope ? `_${normalized}` : normalized; + return /^[A-Za-z_$]/.test(base) ? base : `_${base}`; }packages/cli/src/commands/bundle/bundler.ts (1)
628-671: Add validation for requiredstartFlowimport.The refactored
createEntryPointis cleaner and more maintainable, but the issue from the previous review remains:generatePlatformWrapperunconditionally callsstartFlow(config)without validating that@walkeros/collectoris present with thestartFlowimport.Add validation after line 647:
const importsCode = importStatements.join('\n'); const hasFlow = destinationPackages.size > 0 || sourcePackages.size > 0; + // Validate required startFlow import when flow exists + if (hasFlow) { + const collectorPkg = buildOptions.packages['@walkeros/collector']; + if (!collectorPkg || !collectorPkg.imports?.includes('startFlow')) { + throw new Error( + 'Flow configuration requires @walkeros/collector with startFlow import. ' + + 'Add "@walkeros/collector": { "imports": ["startFlow"] } to packages config.' + ); + } + } + // If no sources/destinations, just return user code with imports (no flow wrapper) if (!hasFlow) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/cli/src/commands/bundle/bundler.ts(5 hunks)packages/core/src/__tests__/flow.test.ts(5 hunks)packages/core/src/flow.ts(1 hunks)packages/core/src/schemas/flow.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENT.md)
**/*.{ts,tsx}: NEVER useanytype - always use proper TypeScript types from @walkeros/core or package-specific types
Implement event structure with consistent properties: name (ENTITY ACTION format), data (entity-specific), context (state/environment), globals (global properties), user (identification), nested (related entities), consent, and system-generated fields (id, timestamp, entity, action)
Use core functions from @walkeros/core for common operations: getEvent(), createEvent() for event creation, getMappingEvent(), getMappingValue() for transformations, isString(), isObject(), isDefined() for type checking, assign(), clone() for object operations, tryCatch(), tryCatchAsync() for error handling
Useimport typefor type-only imports to ensure proper TypeScript compilation and tree-shaking
Avoid inline change comments like '// Fixed for version X' or '// Changed due to Y' - Git tracks history, use migrations and documentation for breaking changes instead
Use Root-level Turborepo commands for parallel operations across packages, navigate to individual packages for focused development, and test changes across all affected packages due to core package dependencies
Files:
packages/core/src/schemas/flow.tspackages/core/src/flow.tspackages/core/src/__tests__/flow.test.tspackages/cli/src/commands/bundle/bundler.ts
packages/*/src/**/*.ts
📄 CodeRabbit inference engine (AGENT.md)
packages/*/src/**/*.ts: Use Higher-Order Functions (HOF) and smart abstractions to avoid redundancy and keep implementations lean and performant
Implement Destination interface with standard structure: type, init(), push(), pushBatch() methods and config object with optional settings, mapping, data, consent, policy, queue, and dryRun properties
Implement event mapping with flexible patterns: exact entity-action matches, wildcard patterns using '*', conditional mappings with functions, and value mapping strategies including string key extraction, static values, custom functions, object transformation, array processing, and consent-based mapping
Files:
packages/core/src/schemas/flow.tspackages/core/src/flow.tspackages/core/src/__tests__/flow.test.tspackages/cli/src/commands/bundle/bundler.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENT.md)
**/*.test.{ts,tsx}: Write component-level integration tests with mocked external APIs and boundaries: test destinations by mocking external APIs (gtag, fbq, etc.), test collector with mocked destinations and sources, test sources with mocked collector interface
Mock external APIs in destination tests (gtag, fbq, etc.) and verify they are called correctly with the expected data - focus on verifying external API calls, not return values
Before documenting code patterns, create functional, executable tests to validate them - use /workspaces/walkerOS/apps/quickstart as the source of truth for working examples
Files:
packages/core/src/__tests__/flow.test.ts
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to packages/*/src/**/*.ts : Implement Destination interface with standard structure: type, init(), push(), pushBatch() methods and config object with optional settings, mapping, data, consent, policy, queue, and dryRun properties
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to **/*.test.{ts,tsx} : Before documenting code patterns, create functional, executable tests to validate them - use /workspaces/walkerOS/apps/quickstart as the source of truth for working examples
Applied to files:
packages/core/src/__tests__/flow.test.ts
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to **/*.test.{ts,tsx} : Mock external APIs in destination tests (gtag, fbq, etc.) and verify they are called correctly with the expected data - focus on verifying external API calls, not return values
Applied to files:
packages/core/src/__tests__/flow.test.ts
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to **/*.test.{ts,tsx} : Write component-level integration tests with mocked external APIs and boundaries: test destinations by mocking external APIs (gtag, fbq, etc.), test collector with mocked destinations and sources, test sources with mocked collector interface
Applied to files:
packages/core/src/__tests__/flow.test.ts
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to **/*.{ts,tsx} : Use Root-level Turborepo commands for parallel operations across packages, navigate to individual packages for focused development, and test changes across all affected packages due to core package dependencies
Applied to files:
packages/cli/src/commands/bundle/bundler.ts
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to packages/*/src/**/*.ts : Implement Destination interface with standard structure: type, init(), push(), pushBatch() methods and config object with optional settings, mapping, data, consent, policy, queue, and dryRun properties
Applied to files:
packages/cli/src/commands/bundle/bundler.ts
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to **/*.{ts,tsx} : Use core functions from walkeros/core for common operations: getEvent(), createEvent() for event creation, getMappingEvent(), getMappingValue() for transformations, isString(), isObject(), isDefined() for type checking, assign(), clone() for object operations, tryCatch(), tryCatchAsync() for error handling
Applied to files:
packages/cli/src/commands/bundle/bundler.ts
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to **/*.{ts,tsx} : Use `import type` for type-only imports to ensure proper TypeScript compilation and tree-shaking
Applied to files:
packages/cli/src/commands/bundle/bundler.ts
🧬 Code graph analysis (2)
packages/core/src/flow.ts (1)
packages/core/src/types/flow.ts (1)
Packages(37-44)
packages/core/src/__tests__/flow.test.ts (1)
packages/core/src/flow.ts (2)
packageNameToVariable(98-111)getFlowConfig(192-295)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build and test
- GitHub Check: AWS Amplify Console Web Preview
- GitHub Check: Build and test
🔇 Additional comments (7)
packages/core/src/schemas/flow.ts (1)
115-120: LGTM! Schema additions are well-structured.The optional
codefield additions to bothSourceReferenceSchemaandDestinationReferenceSchemaare well-documented and appropriately optional. The descriptions clearly explain the purpose and default behavior.Also applies to: 160-165
packages/core/src/flow.ts (1)
113-132: LGTM! Code resolution logic is sound.The updated
resolveCodeFromPackagecorrectly prioritizes explicit code and falls back to auto-generated names from package names. The logic is clear and handles the undefined cases appropriately.packages/core/src/__tests__/flow.test.ts (1)
1301-1396: LGTM! Comprehensive test coverage for code resolution.The new tests thoroughly cover the
resolveCodeFromPackagebehavior including:
- Auto-generation when code is not provided
- Explicit code preservation
- Named exports handling
- Multiple destinations sharing the same package
The tests are functional and executable, aligning with the codebase testing patterns.
packages/cli/src/commands/bundle/bundler.ts (4)
433-458: LGTM! Source package detection is well-implemented.The
detectSourcePackagesfunction correctly mirrors thedetectDestinationPackagespattern, extracting package names from sources with explicit 'package' fields.
547-616: LGTM! Import generation logic handles all cases correctly.The updated
generateImportStatementsproperly handles three distinct import scenarios:
- Explicit imports from package config (utilities like
startFlow)- Explicit code imports from destinations/sources
- Auto-generated default imports for packages used by destinations/sources
The logic correctly skips unused packages, maintaining optimal bundle size.
717-795: LGTM! Config serialization is clean and maintainable.The
buildConfigObjectandprocessConfigValuefunctions provide a clean programmatic approach to config serialization. The logic correctly distinguishes between explicit and auto-generated codes, and the JSON serialization is appropriate for config values.
797-841: LGTM! Platform wrappers are well-structured.The
generatePlatformWrapperfunction cleanly separates browser and server concerns:
- Browser: IIFE with optional window assignments for
collectorandelb- Server: Default export function with context parameter for extensibility
Both wrappers correctly inject user code and call
startFlow, though validation for thestartFlowimport should be added upstream (as noted in thecreateEntryPointcomment).
| // Only treat as explicit if code doesn't match auto-generated pattern | ||
| // Auto-generated code starts with '_' (from packageNameToVariable) | ||
| const isAutoGenerated = destConfig.code.startsWith('_'); | ||
| if (!isAutoGenerated) { | ||
| if (!explicitCodeImports.has(destConfig.package)) { | ||
| explicitCodeImports.set(destConfig.package, new Set()); | ||
| } | ||
| explicitCodeImports.get(destConfig.package)!.add(destConfig.code); | ||
| } |
There was a problem hiding this comment.
Fragile auto-generated code detection.
The logic to distinguish explicit from auto-generated codes assumes all auto-generated identifiers start with _ (lines 486, 514). However, packageNameToVariable only prefixes scoped packages (e.g., @walkeros/foo → _walkerosFoo), leaving unscoped packages unchanged (e.g., lodash → lodash).
This means auto-generated codes for unscoped packages will be incorrectly treated as explicit imports, potentially generating named imports instead of default imports.
Consider one of these approaches:
- Track auto-generated codes explicitly: Pass a set of auto-generated codes from
resolveCodeFromPackageto avoid inference - Normalize all auto-generated codes: Always prefix with
_inpackageNameToVariable, even for unscoped packages - Document the limitation: If unscoped packages are not expected in walkerOS flows, document this assumption
Example fix (option 2):
export function packageNameToVariable(packageName: string): string {
- const hasScope = packageName.startsWith('@');
const normalized = packageName
.replace(/^@/, '')
.replace(/[/-]/g, '_')
// ... rest of normalization
- return hasScope ? `_${normalized}` : normalized;
+ // Always prefix to ensure consistent detection of auto-generated codes
+ return `_${normalized}`;
}Also applies to: 512-520
🤖 Prompt for AI Agents
In packages/cli/src/commands/bundle/bundler.ts around lines 484-492 (and also
update usage at ~512-520), the code incorrectly infers auto-generated
identifiers by checking for a leading '_' which fails for unscoped packages;
update the flow so auto-generated codes are tracked explicitly instead of
inferred: change resolveCodeFromPackage (or its caller) to return (or populate)
a Set of autoGeneratedCodes alongside the resolved code, then replace the
startsWith('_') checks with membership checks against that Set (use the Set when
deciding whether to add to explicitCodeImports), and propagate this Set to any
other places that currently rely on the '_' heuristic.
| describe('packageNameToVariable', () => { | ||
| test('converts scoped package names to valid variable names', () => { | ||
| expect(packageNameToVariable('@walkeros/server-destination-api')).toBe( | ||
| '_walkerosServerDestinationApi', | ||
| ); | ||
| }); | ||
|
|
||
| test('converts unscoped package names', () => { | ||
| expect(packageNameToVariable('lodash')).toBe('lodash'); | ||
| }); | ||
|
|
||
| test('handles multiple hyphens and slashes', () => { | ||
| expect(packageNameToVariable('@custom/my-helper')).toBe('_customMyHelper'); | ||
| expect(packageNameToVariable('@scope/package-name-test')).toBe( | ||
| '_scopePackageNameTest', | ||
| ); | ||
| }); | ||
|
|
||
| test('handles package names with numbers', () => { | ||
| expect(packageNameToVariable('@walkeros/web-destination-gtag')).toBe( | ||
| '_walkerosWebDestinationGtag', | ||
| ); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add test for numeric-leading unscoped package names.
The test suite covers scoped packages, unscoped packages, hyphens, and numbers, but doesn't test the edge case of unscoped packages starting with digits (e.g., "123-utils"), which would currently generate invalid JavaScript identifiers like "123Utils".
Add this test case to verify the fix suggested for packageNameToVariable:
test('handles package names with numbers', () => {
expect(packageNameToVariable('@walkeros/web-destination-gtag')).toBe(
'_walkerosWebDestinationGtag',
);
});
+
+ test('handles unscoped packages starting with digits', () => {
+ expect(packageNameToVariable('123-utils')).toBe('_123Utils');
+ expect(packageNameToVariable('9to5-scheduler')).toBe('_9to5Scheduler');
+ });
});🤖 Prompt for AI Agents
In packages/core/src/__tests__/flow.test.ts around lines 1272 to 1294, the suite
lacks a test for unscoped package names that start with digits; add a test case
inside the existing describe('packageNameToVariable') block that calls
packageNameToVariable('123-utils') (and/or similar) and asserts the result is a
valid identifier (e.g. '_123Utils'), thereby verifying the implementation
prefixes numeric-leading unscoped names with an underscore to produce a valid JS
variable name.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
website/docs/guides/session.mdx (1)
122-122: Consider using "whether" for better formality.Based on learnings from static analysis tools. When presenting alternatives with "or not", "whether" is more formal than "if" in technical documentation.
Apply this diff:
-is used to decide if storage access is allowed or not. If set, it registers an +is used to decide whether storage access is allowed or not. If set, it registers an
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
website/docs/guides/session.mdx(1 hunks)website/sidebars.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENT.md)
**/*.{ts,tsx}: NEVER useanytype - always use proper TypeScript types from @walkeros/core or package-specific types
Implement event structure with consistent properties: name (ENTITY ACTION format), data (entity-specific), context (state/environment), globals (global properties), user (identification), nested (related entities), consent, and system-generated fields (id, timestamp, entity, action)
Use core functions from @walkeros/core for common operations: getEvent(), createEvent() for event creation, getMappingEvent(), getMappingValue() for transformations, isString(), isObject(), isDefined() for type checking, assign(), clone() for object operations, tryCatch(), tryCatchAsync() for error handling
Useimport typefor type-only imports to ensure proper TypeScript compilation and tree-shaking
Avoid inline change comments like '// Fixed for version X' or '// Changed due to Y' - Git tracks history, use migrations and documentation for breaking changes instead
Use Root-level Turborepo commands for parallel operations across packages, navigate to individual packages for focused development, and test changes across all affected packages due to core package dependencies
Files:
website/sidebars.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to packages/*/src/**/*.ts : Implement Destination interface with standard structure: type, init(), push(), pushBatch() methods and config object with optional settings, mapping, data, consent, policy, queue, and dryRun properties
🪛 LanguageTool
website/docs/guides/session.mdx
[style] ~122-~122: In contexts where ‘if’ is followed by ‘or’, using ‘whether’ may be more appropriate (and formal).
Context: ...tecting a new session is used to decide if storage access is allowed or not. If se...
(IF_WHETHER)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build and test
- GitHub Check: AWS Amplify Console Web Preview
🔇 Additional comments (4)
website/sidebars.ts (1)
127-127: LGTM! Documentation properly linked.The new session guide is correctly added to the sidebar navigation and follows the established pattern for guide entries.
website/docs/guides/session.mdx (3)
1-408: Excellent documentation structure and content.The session guide is comprehensive, well-organized, and includes:
- Clear explanations of session detection modes
- Detailed configuration tables
- Practical code examples
- Visual flowcharts for detection processes
- Appropriate warnings and notes for consent/storage
The documentation effectively guides users through session management features.
123-124: Cross-reference link is valid.The documentation reference to
/docs/collector/commands#oncorrectly points to the "## on" heading inwebsite/docs/collector/commands.mdx(line 85). No issues found.
11-14: The PackageLink pathpackages/web/core/src/sessionis valid and exists in the repository.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/server/destinations/datamanager/src/schemas/settings.ts (1)
92-109: Replacez.any()withz.unknown()to avoid TypeScriptanytype inference in theSettingstype.The three fields (
userId,clientId,sessionAttributes) usez.any(), which infers as TypeScriptanywhenSettingsis created viaz.infer<typeof SettingsSchema>. This violates the no-anyguideline. Switch toz.unknown()instead—it maintains identical runtime flexibility while keeping the type fully typed. TheuserDatafield in the same file already follows this pattern withz.record(z.string(), z.unknown()).userId: z - .any() + .unknown() .describe( "Guided helper: First-party user ID for all events (like 'user.id')", ) .optional(), clientId: z - .any() + .unknown() .describe( "Guided helper: GA4 client ID for all events (like 'user.device')", ) .optional(), sessionAttributes: z - .any() + .unknown() .describe( "Guided helper: Privacy-safe attribution for all events (like 'context.sessionAttributes')", ) .optional(),
🧹 Nitpick comments (3)
packages/server/destinations/datamanager/src/schemas/settings.ts (3)
8-18: CredentialsSchema shape looks good; just be careful with secret handling.The schema for
client_emailandprivate_keylooks correct and matches typical service account JSON. Ensure any code that logsSettingsobjects or validation errors consistently redactscredentials.private_key(ideally the wholecredentialsblock) so these secrets never end up in logs or telemetry.
19-28: Clarify / enforce the relationship betweencredentialsandkeyFilename.Both fields are optional, so configs that omit both (or set both) are accepted by the schema. If you expect “exactly one” explicit auth mechanism when ADC isn’t relied on, consider adding a refinement on
SettingsSchemato enforce that invariant, or at least document explicitly that omitting both falls back to environment-based credentials.
29-34: Consider encoding the documented default forscopesin the schema.The description says this defaults to a Data Manager scope, but
scopesis currently just optional without a.default(...). If there is a single canonical default, you could wire it in here (possibly via a shared constant), or otherwise adjust the description to point to where the default is applied at runtime to avoid drift.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
packages/cli/README.md(3 hunks)packages/collector/README.md(1 hunks)packages/server/destinations/datamanager/src/schemas/settings.ts(1 hunks)skills/README.md(1 hunks)skills/create-destination/SKILL.md(1 hunks)skills/create-source/SKILL.md(1 hunks)skills/writing-documentation/SKILL.md(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/collector/README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- skills/create-source/SKILL.md
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENT.md)
**/*.{ts,tsx}: NEVER useanytype - always use proper TypeScript types from @walkeros/core or package-specific types
Implement event structure with consistent properties: name (ENTITY ACTION format), data (entity-specific), context (state/environment), globals (global properties), user (identification), nested (related entities), consent, and system-generated fields (id, timestamp, entity, action)
Use core functions from @walkeros/core for common operations: getEvent(), createEvent() for event creation, getMappingEvent(), getMappingValue() for transformations, isString(), isObject(), isDefined() for type checking, assign(), clone() for object operations, tryCatch(), tryCatchAsync() for error handling
Useimport typefor type-only imports to ensure proper TypeScript compilation and tree-shaking
Avoid inline change comments like '// Fixed for version X' or '// Changed due to Y' - Git tracks history, use migrations and documentation for breaking changes instead
Use Root-level Turborepo commands for parallel operations across packages, navigate to individual packages for focused development, and test changes across all affected packages due to core package dependencies
Files:
packages/server/destinations/datamanager/src/schemas/settings.ts
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to packages/*/src/**/*.ts : Implement Destination interface with standard structure: type, init(), push(), pushBatch() methods and config object with optional settings, mapping, data, consent, policy, queue, and dryRun properties
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: All walkerOS components communicate via push functions: sources use source.push(), collector uses collector.push(), destinations use destination.push(), and ELB uses elb() as the alias for collector.push()
Applied to files:
packages/cli/README.md
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to packages/*/src/**/*.ts : Implement Destination interface with standard structure: type, init(), push(), pushBatch() methods and config object with optional settings, mapping, data, consent, policy, queue, and dryRun properties
Applied to files:
packages/cli/README.mdskills/create-destination/SKILL.md
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to packages/*/src/**/*.ts : Implement event mapping with flexible patterns: exact entity-action matches, wildcard patterns using '*', conditional mappings with functions, and value mapping strategies including string key extraction, static values, custom functions, object transformation, array processing, and consent-based mapping
Applied to files:
packages/cli/README.md
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to **/*.{ts,tsx} : Use Root-level Turborepo commands for parallel operations across packages, navigate to individual packages for focused development, and test changes across all affected packages due to core package dependencies
Applied to files:
packages/cli/README.md
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to **/*.test.{ts,tsx} : Before documenting code patterns, create functional, executable tests to validate them - use /workspaces/walkerOS/apps/quickstart as the source of truth for working examples
Applied to files:
skills/writing-documentation/SKILL.mdskills/create-destination/SKILL.md
📚 Learning: 2025-11-25T16:24:48.032Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T16:24:48.032Z
Learning: Refer to AGENT.md for all relevant instructions and guidelines when working with the walkerOS codebase
Applied to files:
skills/writing-documentation/SKILL.mdskills/README.mdskills/create-destination/SKILL.md
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Use the Source → Collector → Destination(s) architecture pattern for event processing: Sources capture events from various sources (browser DOM, dataLayer, server), Collector handles central event processing with consent management, Destinations transform and send events to third-party tools
Applied to files:
skills/create-destination/SKILL.md
🪛 LanguageTool
skills/writing-documentation/SKILL.md
[uncategorized] ~132-~132: Did you mean the formatting language “Markdown” (= proper noun)?
Context: ... Use PropertyTable instead of hardcoded markdown tables --- ## Quality Checklist ### ...
(MARKDOWN_NNP)
🪛 markdownlint-cli2 (0.18.1)
skills/writing-documentation/SKILL.md
27-27: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
272-272: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
296-296: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
skills/README.md
33-33: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
skills/create-destination/SKILL.md
53-53: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
335-335: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
363-363: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build and test
- GitHub Check: AWS Amplify Console Web Preview
- GitHub Check: Build and test
🔇 Additional comments (4)
packages/cli/README.md (3)
89-130: Push command documentation is clear and well-structured.The new
pushcommand section provides good coverage: command signature, all relevant options, multiple event input format examples, and a helpful comparison table. The guidance to usesimulatefirst thenpushfor verification sets appropriate expectations for users.
220-244: Flow configuration example correctly demonstrates package-based pattern.The updated example properly shows:
- Empty package entries for destinations/sources (relying on default exports)
- Explicit
packagefields in source and destination configs- Utility library (
@walkeros/collector) retainsimportsas expectedAligns well with the patterns explained in the "Package Configuration Patterns" section below.
249-317: Package Configuration Patterns section provides clear guidance on import resolution.The three-pattern approach (default exports, named exports, utility imports) is well-explained with concrete JSON examples and generated import statements. The key points section clarifies the decision rules effectively, making it straightforward for users to know when to use
codevsimportsvs omitting them. Examples are consistent with the flow configuration shown above.skills/create-destination/SKILL.md (1)
50-57: Add language specifiers to fenced code blocks.Multiple fenced code blocks lack language identifiers, triggering markdownlint MD040. These are valid warnings since specifying the language improves parsing and readability.
Apply these fixes:
- Lines 50–57 (npm commands): Change opening fence to
```bash- Line 335 (code block): Change opening fence to
```typescriptor appropriate language- Line 363 (code block): Change opening fence to
```typescriptor appropriate languageAlso applies to: 335-335, 363-363
⛔ Skipped due to learnings
Learnt from: CR Repo: elbwalker/walkerOS PR: 0 File: AGENT.md:0-0 Timestamp: 2025-11-25T16:25:10.439Z Learning: Applies to **/*.{ts,tsx} : Avoid inline change comments like '// Fixed for version X' or '// Changed due to Y' - Git tracks history, use migrations and documentation for breaking changes instead
| ``` | ||
| skills/ <- Primary content (tool-agnostic) | ||
| .claude/skills/ <- Claude Code references (auto-discovery) | ||
| ``` |
There was a problem hiding this comment.
Add language specifier to architecture diagram code block.
The fenced code block at lines 33–36 lacks a language identifier. Change the opening fence from ``` to ```text to explicitly mark it as plain text and resolve the markdownlint MD040 warning (as noted in the prior review).
## Architecture
-```
+```text
skills/ <- Primary content (tool-agnostic)
.claude/skills/ <- Claude Code references (auto-discovery)
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.18.1)</summary>
33-33: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
In skills/README.md around lines 33 to 36, the fenced code block lacks a
language specifier causing markdownlint MD040; update the opening fence to
include the language tag "text" (i.e., change totext) so the block is
explicitly plain text and the linter warning is resolved.
</details>
<!-- fingerprinting:phantom:triton:mongoose -->
<!-- This is an auto-generated comment by CodeRabbit -->
|
|
||
| ## Documentation Types | ||
|
|
||
| ### Where Content Belongs |
There was a problem hiding this comment.
Add language specifiers to fenced code blocks.
Three fenced code blocks lack language identifiers, triggering markdownlint MD040:
- Line 27: Change
```to```text(hierarchy diagram) - Line 272: Change
```to```mdxor```jsx(MDX template code) - Line 296: Change
```to```textor appropriate language (placeholder content)
Also applies to: 272-272, 296-296
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
27-27: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In skills/writing-documentation/SKILL.md around lines 27, 272 and 296, several
fenced code blocks are missing language specifiers which triggers markdownlint
MD040; update the opening triple-backtick on line 27 to ```text for the
hierarchy diagram, update the triple-backtick on line 272 to an appropriate
MDX/JSX specifier such as ```mdx or ```jsx for the template example, and update
the triple-backtick on line 296 to ```text or another suitable language
identifier for the placeholder content so each fenced block includes a language
tag.
|
|
||
| - Link to source files instead of copying type definitions | ||
| - Reference `apps/quickstart/` examples instead of writing from scratch | ||
| - Use PropertyTable instead of hardcoded markdown tables |
There was a problem hiding this comment.
Fix capitalization: "Markdown" is a proper noun.
Line 132: Change "markdown tables" to "Markdown tables" to match style guide conventions.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~132-~132: Did you mean the formatting language “Markdown” (= proper noun)?
Context: ... Use PropertyTable instead of hardcoded markdown tables --- ## Quality Checklist ### ...
(MARKDOWN_NNP)
🤖 Prompt for AI Agents
In skills/writing-documentation/SKILL.md around line 132, the phrase uses
lowercase "markdown tables"; update the capitalization to "Markdown tables" to
match the style guide. Replace the text fragment "markdown tables" with
"Markdown tables" preserving surrounding punctuation and spacing.
There was a problem hiding this comment.
Actionable comments posted: 11
♻️ Duplicate comments (2)
packages/server/sources/aws/README.md (2)
183-185: Add language specification to fenced code block.The GET request example lacks a language identifier. This should be marked as plain text for proper syntax highlighting.
Apply this diff:
**GET - Pixel Tracking:** -``` +```text GET /collect?event=page%20view&data[title]=Home&data[path]=/ -``` +```
266-270: Add language specification to fenced code block.The CloudWatch Insights query block lacks a language identifier for proper syntax highlighting.
Apply this diff:
queries: -``` +```sql fields @timestamp, level, message, requestId, error | filter level = "error" | sort @timestamp desc -``` +```
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
packages/collector/README.md(2 hunks)packages/core/README.md(3 hunks)packages/server/core/README.md(3 hunks)packages/server/destinations/aws/README.md(1 hunks)packages/server/destinations/datamanager/README.md(2 hunks)packages/server/destinations/gcp/README.md(2 hunks)packages/server/destinations/meta/README.md(1 hunks)packages/server/sources/aws/README.md(1 hunks)packages/web/core/README.md(2 hunks)packages/web/destinations/api/README.md(1 hunks)packages/web/destinations/meta/README.md(1 hunks)packages/web/destinations/piwikpro/README.md(1 hunks)packages/web/destinations/plausible/README.md(2 hunks)packages/web/sources/browser/README.md(2 hunks)packages/web/sources/dataLayer/README.md(1 hunks)website/docs/sources/index.mdx(1 hunks)website/docs/sources/server/_category_.json(1 hunks)website/docs/sources/server/aws.mdx(1 hunks)website/docs/sources/server/express.mdx(1 hunks)website/docs/sources/server/fetch.mdx(1 hunks)website/docs/sources/server/gcp.mdx(1 hunks)website/docs/sources/server/index.mdx(1 hunks)
✅ Files skipped from review due to trivial changes (5)
- website/docs/sources/server/aws.mdx
- packages/server/destinations/datamanager/README.md
- packages/web/destinations/api/README.md
- website/docs/sources/server/category.json
- website/docs/sources/server/gcp.mdx
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to packages/*/src/**/*.ts : Implement Destination interface with standard structure: type, init(), push(), pushBatch() methods and config object with optional settings, mapping, data, consent, policy, queue, and dryRun properties
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Use the Source → Collector → Destination(s) architecture pattern for event processing: Sources capture events from various sources (browser DOM, dataLayer, server), Collector handles central event processing with consent management, Destinations transform and send events to third-party tools
Applied to files:
website/docs/sources/index.mdxwebsite/docs/sources/server/fetch.mdxpackages/collector/README.mdwebsite/docs/sources/server/index.mdxwebsite/docs/sources/server/express.mdx
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to packages/*/src/**/*.ts : Implement event mapping with flexible patterns: exact entity-action matches, wildcard patterns using '*', conditional mappings with functions, and value mapping strategies including string key extraction, static values, custom functions, object transformation, array processing, and consent-based mapping
Applied to files:
packages/core/README.md
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to **/*.{ts,tsx} : Use core functions from walkeros/core for common operations: getEvent(), createEvent() for event creation, getMappingEvent(), getMappingValue() for transformations, isString(), isObject(), isDefined() for type checking, assign(), clone() for object operations, tryCatch(), tryCatchAsync() for error handling
Applied to files:
packages/core/README.mdpackages/server/core/README.md
📚 Learning: 2025-11-25T16:24:48.032Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T16:24:48.032Z
Learning: Refer to AGENT.md for all relevant instructions and guidelines when working with the walkerOS codebase
Applied to files:
packages/core/README.mdpackages/server/core/README.mdpackages/web/core/README.md
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: All walkerOS components communicate via push functions: sources use source.push(), collector uses collector.push(), destinations use destination.push(), and ELB uses elb() as the alias for collector.push()
Applied to files:
website/docs/sources/server/index.mdxpackages/server/sources/aws/README.mdpackages/web/sources/browser/README.md
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to packages/*/src/**/*.ts : Implement Destination interface with standard structure: type, init(), push(), pushBatch() methods and config object with optional settings, mapping, data, consent, policy, queue, and dryRun properties
Applied to files:
packages/server/destinations/gcp/README.md
🪛 markdownlint-cli2 (0.18.1)
packages/web/destinations/piwikpro/README.md
3-3: Images should have alternate text (alt text)
(MD045, no-alt-text)
packages/core/README.md
3-3: Images should have alternate text (alt text)
(MD045, no-alt-text)
packages/web/destinations/meta/README.md
3-3: Images should have alternate text (alt text)
(MD045, no-alt-text)
packages/server/core/README.md
3-3: Images should have alternate text (alt text)
(MD045, no-alt-text)
packages/server/destinations/aws/README.md
3-3: Images should have alternate text (alt text)
(MD045, no-alt-text)
packages/server/destinations/gcp/README.md
3-3: Images should have alternate text (alt text)
(MD045, no-alt-text)
packages/server/destinations/meta/README.md
3-3: Images should have alternate text (alt text)
(MD045, no-alt-text)
packages/server/sources/aws/README.md
183-183: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
266-266: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
packages/web/core/README.md
3-3: Images should have alternate text (alt text)
(MD045, no-alt-text)
packages/web/destinations/plausible/README.md
3-3: Images should have alternate text (alt text)
(MD045, no-alt-text)
packages/web/sources/browser/README.md
3-3: Images should have alternate text (alt text)
(MD045, no-alt-text)
packages/web/sources/dataLayer/README.md
3-3: Images should have alternate text (alt text)
(MD045, no-alt-text)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build and test
- GitHub Check: AWS Amplify Console Web Preview
- GitHub Check: Build and test
🔇 Additional comments (8)
packages/web/destinations/plausible/README.md (1)
40-40: Branding and documentation updates look good.The domain branding updates (line 40) and configuration table reformatting (lines 47–49) are clean and properly aligned with the walkerOS branding refresh.
Also applies to: 47-49
packages/web/sources/browser/README.md (1)
2-3: Domain branding updates are correct and consistent.The changes from elbwalker.com to walkeros.io are straightforward and align with the PR objectives. URLs are properly formatted and the documentation link structure is maintained.
Also applies to: 118-119
website/docs/sources/index.mdx (1)
44-53: Consistent addition to sources overview.The new Server Sources section integrates seamlessly with the existing Browser and dataLayer sections, maintaining format consistency and aligning with the Source → Collector → Destination architecture pattern. The bullet-point summary accurately reflects the available server platforms documented elsewhere in the PR.
website/docs/sources/server/express.mdx (1)
1-110: Comprehensive Express source documentation.The documentation provides clear guidance for both standalone and app-embedded deployment modes. Configuration and HTTP methods tables serve as useful quick references. Code examples are well-annotated and follow the established walkerOS configuration pattern.
website/docs/sources/server/index.mdx (1)
1-37: Well-structured hub page for server sources.The page effectively introduces server sources with clear differentiation across platforms and use cases. All referenced documentation pages exist and are correctly linked:
./express→ website/docs/sources/server/express.mdx ✓./fetch→ website/docs/sources/server/fetch.mdx ✓./aws→ website/docs/sources/server/aws.mdx ✓./gcp→ website/docs/sources/server/gcp.mdx ✓The table and guidance sections will help users select the appropriate source for their environment.
website/docs/sources/server/fetch.mdx (1)
1-143: Excellent Fetch source documentation for edge/serverless deployments.Platform-specific examples clearly demonstrate the Web Standard Fetch API integration across diverse runtimes. Configuration options are well-documented with sensible defaults. Usage examples correctly show both single event and batch processing patterns. The batch event payload structure (
{ batch: [...] }) is properly implemented and validated against the EventSchema for each event in the array.packages/collector/README.md (2)
111-116: Logger configuration documentation added.The advanced setup now documents the new unified logger pattern with
levelandhandlerproperties. This aligns with the PR objectives to introduce scoped loggers and richer context through the collector. The example is clear and demonstrates the expected API.
122-128: Configuration table updated to document logger option.The table addition for the
loggerconfiguration option with typeobjectand description is consistent with the advanced setup example above. The documentation is clear and follows the existing table format.
| <a href="https://www.walkeros.io"> | ||
| <img title="elbwalker" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/> |
There was a problem hiding this comment.
Add alt text to the logo image for accessibility.
The <img> tag on line 3 is missing an alt attribute. Per WCAG guidelines, all images should have descriptive alt text.
<a href="https://www.walkeros.io">
- <img title="elbwalker" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/>
+ <img title="elbwalker" alt="walkerOS logo" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/>
</a>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <a href="https://www.walkeros.io"> | |
| <img title="elbwalker" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/> | |
| <a href="https://www.walkeros.io"> | |
| <img title="elbwalker" alt="walkerOS logo" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/> | |
| </a> |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
3-3: Images should have alternate text (alt text)
(MD045, no-alt-text)
🤖 Prompt for AI Agents
In packages/core/README.md around lines 2 to 3, the <img> tag for the logo is
missing an alt attribute; add a descriptive alt attribute (e.g., alt="Elbwalker
logo" or another concise descriptive phrase) to the img element to satisfy
accessibility/WCAG requirements and provide meaningful alternative text for
screen readers.
| <a href="https://www.walkeros.io"> | ||
| <img title="elbwalker" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/> |
There was a problem hiding this comment.
Add alt text to the logo image for accessibility.
The <img> tag on line 3 is missing an alt attribute. Per WCAG guidelines, all images should have descriptive alt text.
<a href="https://www.walkeros.io">
- <img title="elbwalker" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/>
+ <img title="elbwalker" alt="walkerOS logo" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/>
</a>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <a href="https://www.walkeros.io"> | |
| <img title="elbwalker" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/> | |
| <a href="https://www.walkeros.io"> | |
| <img title="elbwalker" alt="walkerOS logo" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/> | |
| </a> |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
3-3: Images should have alternate text (alt text)
(MD045, no-alt-text)
🤖 Prompt for AI Agents
In packages/server/core/README.md around lines 2 to 3, the <img> tag for the
logo is missing an alt attribute; add a concise, descriptive alt attribute
(e.g., alt="Elbwalker logo" or alt="Elbwalker company logo") to the <img>
element to satisfy WCAG accessibility requirements and improve screen reader
support.
| <a href="https://www.walkeros.io"> | ||
| <img title="elbwalker" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/> |
There was a problem hiding this comment.
Add alt text to the logo image for accessibility.
The <img> tag on line 3 is missing an alt attribute. Per WCAG guidelines, all images should have descriptive alt text.
<a href="https://www.walkeros.io">
- <img title="elbwalker" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/>
+ <img title="elbwalker" alt="walkerOS logo" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/>
</a>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <a href="https://www.walkeros.io"> | |
| <img title="elbwalker" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/> | |
| <a href="https://www.walkeros.io"> | |
| <img title="elbwalker" alt="walkerOS logo" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/> | |
| </a> |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
3-3: Images should have alternate text (alt text)
(MD045, no-alt-text)
🤖 Prompt for AI Agents
In packages/server/destinations/aws/README.md around lines 2 to 3, the <img> tag
is missing an alt attribute; add a descriptive alt attribute (e.g.,
alt="elbwalker logo" or alt="ELBWalker project logo") to the image tag to
satisfy accessibility/WCAG requirements and ensure the alt text succinctly
describes the image's purpose.
| <a href="https://www.walkeros.io"> | ||
| <img title="elbwalker" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/> |
There was a problem hiding this comment.
Add alt text to the logo image for accessibility.
The <img> tag on line 3 is missing an alt attribute. Per WCAG guidelines, all images should have descriptive alt text.
<a href="https://www.walkeros.io">
- <img title="elbwalker" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/>
+ <img title="elbwalker" alt="walkerOS logo" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/>
</a>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <a href="https://www.walkeros.io"> | |
| <img title="elbwalker" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/> | |
| <a href="https://www.walkeros.io"> | |
| <img title="elbwalker" alt="walkerOS logo" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/> |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
3-3: Images should have alternate text (alt text)
(MD045, no-alt-text)
🤖 Prompt for AI Agents
In packages/server/destinations/gcp/README.md around lines 2 to 3, the <img> tag
for the elbwalker logo is missing an alt attribute; add a concise, descriptive
alt attribute (e.g., alt="elbwalker logo" or alt="Elbwalker logo") to the tag to
satisfy accessibility/WCAG requirements and improve screen reader support.
| <a href="https://www.walkeros.io"> | ||
| <img title="elbwalker" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/> |
There was a problem hiding this comment.
Add missing alt text to the image.
The image is missing an alt attribute, which is required for accessibility and SEO.
Apply this diff to add alt text:
<a href="https://www.walkeros.io">
- <img title="elbwalker" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/>
+ <img title="elbwalker" alt="elbwalker logo" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/>
</a>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <a href="https://www.walkeros.io"> | |
| <img title="elbwalker" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/> | |
| <a href="https://www.walkeros.io"> | |
| <img title="elbwalker" alt="elbwalker logo" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/> | |
| </a> |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
3-3: Images should have alternate text (alt text)
(MD045, no-alt-text)
🤖 Prompt for AI Agents
In packages/server/destinations/meta/README.md around lines 2-3, the <img> tag
is missing an alt attribute; add a concise, descriptive alt text (for example:
alt="elbwalker logo") to the image element to satisfy accessibility and SEO
requirements, updating the tag to include that attribute.
| <a href="https://www.walkeros.io"> | ||
| <img title="elbwalker" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/> |
There was a problem hiding this comment.
Add alt text to the logo image for accessibility.
The <img> tag on line 3 is missing an alt attribute. Per WCAG guidelines, all images should have descriptive alt text.
<a href="https://www.walkeros.io">
- <img title="elbwalker" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/>
+ <img title="elbwalker" alt="walkerOS logo" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/>
</a>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <a href="https://www.walkeros.io"> | |
| <img title="elbwalker" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/> | |
| <a href="https://www.walkeros.io"> | |
| <img title="elbwalker" alt="walkerOS logo" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/> | |
| </a> |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
3-3: Images should have alternate text (alt text)
(MD045, no-alt-text)
🤖 Prompt for AI Agents
In packages/web/destinations/meta/README.md around lines 2 to 3, the <img> tag
for the logo is missing an alt attribute which reduces accessibility; add a
concise, descriptive alt attribute (e.g., alt="Elbwalker logo" or alt="elbwalker
logo") to the img tag so screen readers have meaningful text, ensuring it
follows WCAG guidance and reflects the image purpose.
| <a href="https://www.walkeros.io"> | ||
| <img title="elbwalker" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/> |
There was a problem hiding this comment.
Add alt text to the image element for accessibility.
The image on line 3 is missing an alt attribute. This is important for accessibility (screen readers) and as a fallback when the image fails to load.
Apply this diff to add descriptive alt text:
<a href="https://www.walkeros.io">
- <img title="elbwalker" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/>
+ <img title="elbwalker" alt="walkerOS logo" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/>
</a>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <a href="https://www.walkeros.io"> | |
| <img title="elbwalker" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/> | |
| <a href="https://www.walkeros.io"> | |
| <img title="elbwalker" alt="walkerOS logo" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/> | |
| </a> |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
3-3: Images should have alternate text (alt text)
(MD045, no-alt-text)
🤖 Prompt for AI Agents
In packages/web/destinations/piwikpro/README.md around lines 2 to 3, the <img>
tag is missing an alt attribute; add a concise, descriptive alt text (e.g.,
alt="elbwalker logo") to the image element to improve accessibility and provide
a fallback when the image fails to load.
| <a href="https://www.walkeros.io"> | ||
| <img title="elbwalker" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/> |
There was a problem hiding this comment.
Add alt text to the logo image.
The image is missing alt text, which impacts accessibility and SEO. Add a descriptive alt attribute to the image tag.
Apply this diff to add alt text:
<p align="left">
- <a href="https://www.walkeros.io">
- <img title="elbwalker" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/>
+ <a href="https://www.walkeros.io">
+ <img title="elbwalker" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px" alt="elbwalker logo"/>
</a>
</p>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <a href="https://www.walkeros.io"> | |
| <img title="elbwalker" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/> | |
| <a href="https://www.walkeros.io"> | |
| <img title="elbwalker" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px" alt="elbwalker logo"/> |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
3-3: Images should have alternate text (alt text)
(MD045, no-alt-text)
🤖 Prompt for AI Agents
In packages/web/destinations/plausible/README.md around lines 2 to 3, the img
tag for the elbwalker logo is missing an alt attribute; add a descriptive alt
text (for example "elbwalker logo" or "Elbwalker logo") to the <img> tag to
improve accessibility and SEO, ensuring the alt value succinctly describes the
image content.
| <a href="https://elbwalker.com"> | ||
| <img title="elbwalker" src='https://www.elbwalker.com/img/elbwalker_logo.png' width="256px"/> | ||
| <a href="https://www.walkeros.io"> | ||
| <img title="elbwalker" src='https://www.walkeros.io/img/elbwalker_logo.png' width="256px"/> |
There was a problem hiding this comment.
Add alt text to the image for accessibility.
The image element is missing an alt attribute, which is required for markdown linting compliance and accessibility.
Apply this diff to add alt text:
- <img title="elbwalker" src='https://www.walkeros.io/img/elbwalker_logo.png' width="256px"/>
+ <img title="elbwalker" alt="walkerOS logo" src='https://www.walkeros.io/img/elbwalker_logo.png' width="256px"/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <img title="elbwalker" src='https://www.walkeros.io/img/elbwalker_logo.png' width="256px"/> | |
| <img title="elbwalker" alt="walkerOS logo" src='https://www.walkeros.io/img/elbwalker_logo.png' width="256px"/> |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
3-3: Images should have alternate text (alt text)
(MD045, no-alt-text)
🤖 Prompt for AI Agents
In packages/web/sources/browser/README.md around line 3, the img tag is missing
an alt attribute which fails accessibility and markdown linting; add a concise,
descriptive alt value (for example: alt="elbwalker logo") to the image element
while keeping the existing title, src, and width attributes unchanged so the
file passes linting and is accessible.
| <a href="https://www.walkeros.io"> | ||
| <img title="elbwalker" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/> |
There was a problem hiding this comment.
Add alt text to the logo image for accessibility.
The <img> tag on line 3 is missing an alt attribute. Per WCAG guidelines, all images should have descriptive alt text.
<a href="https://www.walkeros.io">
- <img title="elbwalker" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/>
+ <img title="elbwalker" alt="walkerOS logo" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/>
</a>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <a href="https://www.walkeros.io"> | |
| <img title="elbwalker" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/> | |
| <a href="https://www.walkeros.io"> | |
| <img title="elbwalker" alt="walkerOS logo" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/> | |
| </a> |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
3-3: Images should have alternate text (alt text)
(MD045, no-alt-text)
🤖 Prompt for AI Agents
In packages/web/sources/dataLayer/README.md around lines 2-3, the <img> tag for
the elbwalker logo is missing an alt attribute; add a concise, descriptive alt
attribute (e.g., alt="Elbwalker logo" or a more specific description) to the tag
to satisfy WCAG accessibility requirements and aid screen readers.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (4)
packages/web/destinations/meta/README.md (1)
3-3: Add alt text to the logo image for accessibility.This accessibility issue was previously flagged and remains unresolved. The
<img>tag is missing analtattribute required by WCAG guidelines.- <img title="elbwalker" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/> + <img title="elbwalker" alt="walkerOS logo" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/>packages/server/destinations/gcp/README.md (1)
3-3: Add alt text to the logo image for accessibility.The
<img>tag on line 3 is still missing an alt attribute, which was flagged in a previous review. Per WCAG guidelines, all images should have descriptive alt text for screen reader support.Apply this diff to add the alt attribute:
- <img title="elbwalker" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/> + <img title="elbwalker" alt="elbwalker logo" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/>packages/core/README.md (1)
2-3: Add alt text to the logo image for accessibility (duplicate from previous review).The image on line 3 is still missing an
altattribute. Per WCAG guidelines, all images must have descriptive alt text for screen readers. This was previously flagged and should be addressed.- <img title="elbwalker" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/> + <img title="elbwalker" alt="walkerOS logo" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/>packages/server/destinations/aws/README.md (1)
3-3: Add alt text to the logo image for accessibility.The
<img>tag is missing an alt attribute, which is required per WCAG guidelines for all images. This was flagged in the previous review and remains unresolved.Apply this diff to add descriptive alt text:
<a href="https://www.walkeros.io"> - <img title="elbwalker" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/> + <img title="elbwalker" alt="walkerOS logo" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/> </a>
🧹 Nitpick comments (4)
packages/cli/README.md (1)
387-413: Addpushfunction to the Programmatic API examples.The Quick Start and Commands sections document the
pushCLI command, but the Programmatic API section (lines 387–413) only shows examples forbundle,simulate, andrunCommand. Add apushexample for consistency, or document why it's not available programmatically.If
pushis available programmatically, consider adding:// Push const result = await push( './flow.json', { name: 'page view', data: { title: 'Test' } }, { json: true }, );packages/collector/README.md (1)
130-135: Type Definitions section should reference additional new types introduced in the PR.According to the AI summary, this PR introduces new type files (
code.ts,index.ts) undersrc/types/that support the code-based destination and enriched context. Consider adding cross-references to these files alongside the existingflow.tsandcollector.tslinks.Apply this diff to expand the Type Definitions section:
## Type Definitions See [src/types/](./src/types/) for TypeScript interfaces: - [flow.ts](./src/types/flow.ts) - Flow configuration - [collector.ts](./src/types/collector.ts) - Collector interface + - [code.ts](./src/types/code.ts) - Code destination types + - [index.ts](./src/types/index.ts) - Type exportspackages/web/destinations/gtag/README.md (1)
28-75: Clarify the recommended usage pattern—Quick Start and subsequent examples show divergent APIs.The Quick Start (lines 61–74) demonstrates passing
destinationGtagtostartFlowwith a separateconfigproperty, while the Consent Mode (line 103), E-commerce (line 156), and Custom Event (line 199) examples all calldestinationGtag({...})directly as a function. This creates ambiguity about which pattern is the intended/recommended approach.Given the PR scope emphasizing "transitions to startFlow with destinations array," consider clarifying:
- Whether both patterns are supported or if Pattern B is legacy/deprecated
- If both are valid, explain the trade-offs and use cases for each
- Update all examples to demonstrate the recommended pattern consistently
Also applies to: 100-220
package.json (1)
35-38: Validation script wiring looks good; consider addingtsxas a devDependency.The new
validate:links,validate:docs, and aggregatevalidatescripts are consistent with the locations of the TS files and fit the repo‑root tooling approach. For faster, more predictable runs in CI and locally, consider addingtsxas a devDependency and invoking it directly (tsx apps/scripts/…) instead of vianpx, to avoid repeated on‑the‑fly resolution.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
apps/scripts/validate-docs.ts(1 hunks)apps/scripts/validate-links.ts(1 hunks)package.json(1 hunks)packages/cli/README.md(5 hunks)packages/collector/README.md(3 hunks)packages/core/README.md(4 hunks)packages/server/destinations/aws/README.md(3 hunks)packages/server/destinations/datamanager/README.md(2 hunks)packages/server/destinations/gcp/README.md(3 hunks)packages/web/destinations/api/README.md(3 hunks)packages/web/destinations/gtag/README.md(2 hunks)packages/web/destinations/meta/README.md(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/server/destinations/datamanager/README.md
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENT.md)
**/*.{ts,tsx}: NEVER useanytype - always use proper TypeScript types from @walkeros/core or package-specific types
Implement event structure with consistent properties: name (ENTITY ACTION format), data (entity-specific), context (state/environment), globals (global properties), user (identification), nested (related entities), consent, and system-generated fields (id, timestamp, entity, action)
Use core functions from @walkeros/core for common operations: getEvent(), createEvent() for event creation, getMappingEvent(), getMappingValue() for transformations, isString(), isObject(), isDefined() for type checking, assign(), clone() for object operations, tryCatch(), tryCatchAsync() for error handling
Useimport typefor type-only imports to ensure proper TypeScript compilation and tree-shaking
Avoid inline change comments like '// Fixed for version X' or '// Changed due to Y' - Git tracks history, use migrations and documentation for breaking changes instead
Use Root-level Turborepo commands for parallel operations across packages, navigate to individual packages for focused development, and test changes across all affected packages due to core package dependencies
Files:
apps/scripts/validate-links.tsapps/scripts/validate-docs.ts
🧠 Learnings (15)
📓 Common learnings
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to packages/*/src/**/*.ts : Implement Destination interface with standard structure: type, init(), push(), pushBatch() methods and config object with optional settings, mapping, data, consent, policy, queue, and dryRun properties
📚 Learning: 2025-11-25T16:24:48.032Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T16:24:48.032Z
Learning: Refer to AGENT.md for all relevant instructions and guidelines when working with the walkerOS codebase
Applied to files:
packages/collector/README.md
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Use the Source → Collector → Destination(s) architecture pattern for event processing: Sources capture events from various sources (browser DOM, dataLayer, server), Collector handles central event processing with consent management, Destinations transform and send events to third-party tools
Applied to files:
packages/collector/README.mdpackages/web/destinations/gtag/README.mdpackages/web/destinations/api/README.md
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to packages/*/src/**/*.ts : Implement Destination interface with standard structure: type, init(), push(), pushBatch() methods and config object with optional settings, mapping, data, consent, policy, queue, and dryRun properties
Applied to files:
packages/collector/README.mdpackages/web/destinations/gtag/README.mdpackages/web/destinations/api/README.mdpackages/web/destinations/meta/README.mdpackages/server/destinations/aws/README.mdpackages/core/README.mdpackages/cli/README.mdpackages/server/destinations/gcp/README.md
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to **/*.test.{ts,tsx} : Write component-level integration tests with mocked external APIs and boundaries: test destinations by mocking external APIs (gtag, fbq, etc.), test collector with mocked destinations and sources, test sources with mocked collector interface
Applied to files:
packages/web/destinations/gtag/README.md
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to **/*.{ts,tsx} : Use core functions from walkeros/core for common operations: getEvent(), createEvent() for event creation, getMappingEvent(), getMappingValue() for transformations, isString(), isObject(), isDefined() for type checking, assign(), clone() for object operations, tryCatch(), tryCatchAsync() for error handling
Applied to files:
packages/web/destinations/gtag/README.mdpackages/core/README.mdpackages/cli/README.mdpackages/server/destinations/gcp/README.md
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to **/*.test.{ts,tsx} : Mock external APIs in destination tests (gtag, fbq, etc.) and verify they are called correctly with the expected data - focus on verifying external API calls, not return values
Applied to files:
packages/web/destinations/gtag/README.md
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to **/*.{ts,tsx} : Implement event structure with consistent properties: name (ENTITY ACTION format), data (entity-specific), context (state/environment), globals (global properties), user (identification), nested (related entities), consent, and system-generated fields (id, timestamp, entity, action)
Applied to files:
packages/web/destinations/gtag/README.mdpackages/core/README.md
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: All walkerOS components communicate via push functions: sources use source.push(), collector uses collector.push(), destinations use destination.push(), and ELB uses elb() as the alias for collector.push()
Applied to files:
packages/web/destinations/api/README.mdpackages/cli/README.md
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to packages/*/src/**/*.ts : Implement event mapping with flexible patterns: exact entity-action matches, wildcard patterns using '*', conditional mappings with functions, and value mapping strategies including string key extraction, static values, custom functions, object transformation, array processing, and consent-based mapping
Applied to files:
packages/web/destinations/api/README.mdpackages/core/README.mdpackages/cli/README.md
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to **/*.test.{ts,tsx} : Before documenting code patterns, create functional, executable tests to validate them - use /workspaces/walkerOS/apps/quickstart as the source of truth for working examples
Applied to files:
apps/scripts/validate-links.tsapps/scripts/validate-docs.tspackages/core/README.mdpackages/cli/README.md
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to **/*.{ts,tsx} : Use Root-level Turborepo commands for parallel operations across packages, navigate to individual packages for focused development, and test changes across all affected packages due to core package dependencies
Applied to files:
apps/scripts/validate-links.tspackage.jsonpackages/cli/README.md
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to packages/*/tsconfig.json : Configure TypeScript with strict settings to enforce type safety and prevent use of `any` types
Applied to files:
apps/scripts/validate-docs.ts
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to **/*.{ts,tsx} : Avoid inline change comments like '// Fixed for version X' or '// Changed due to Y' - Git tracks history, use migrations and documentation for breaking changes instead
Applied to files:
apps/scripts/validate-docs.ts
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to **/*.{ts,tsx} : NEVER use `any` type - always use proper TypeScript types from walkeros/core or package-specific types
Applied to files:
packages/core/README.mdpackages/cli/README.md
🪛 markdownlint-cli2 (0.18.1)
packages/collector/README.md
3-3: Images should have alternate text (alt text)
(MD045, no-alt-text)
packages/web/destinations/meta/README.md
3-3: Images should have alternate text (alt text)
(MD045, no-alt-text)
packages/server/destinations/aws/README.md
3-3: Images should have alternate text (alt text)
(MD045, no-alt-text)
packages/cli/README.md
38-38: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
packages/server/destinations/gcp/README.md
3-3: Images should have alternate text (alt text)
(MD045, no-alt-text)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build and test
- GitHub Check: AWS Amplify Console Web Preview
- GitHub Check: Build and test
🔇 Additional comments (17)
packages/web/destinations/meta/README.md (2)
28-69: Documentation structure improvements.The new Quick Start and programmatic usage sections are well-structured and provide clear examples of both Flow JSON and TypeScript integration patterns. The examples align well with the destination architecture described in the introduction.
78-86: Type Definitions and Related sections add useful reference points.The new sections help users navigate to source types and related documentation, improving discoverability and reducing friction for developers integrating this destination.
packages/server/destinations/gcp/README.md (2)
24-71: Documentation clearly communicates config-driven and programmatic usage patterns.The new Quick Start section effectively demonstrates both declarative (Flow JSON) and imperative (TypeScript) approaches for configuring the BigQuery destination. The config structure aligns well with the Configuration table, and the
startFlowAPI usage is consistent with the broader PR's architecture updates.
2-2: Domain references updated to walkeros.io.The logo URL, documentation links, and website references have been consistently updated from the previous domain to
walkeros.io, aligning with the PR's documentation modernization efforts.Also applies to: 115-115, 123-123
packages/core/README.md (2)
18-50: Documentation additions are clear and well-structured.The new Quick Start section, installation instructions, and import examples provide good entry points for users. The Flow configuration and code snippets are accurate and helpful.
331-346: Type Definitions and Related sections improve documentation navigation.The new sections properly guide users to relevant source files and related packages. Links are consistent with the branding migration to walkeros.io.
packages/web/destinations/api/README.md (3)
30-36: Align parameter naming in configuration table with actual code examples.The Configuration table at line 35 documents the
transformparameter as(data, config, mapping), but the code example at line 103 uses(event, config, mapping)and referencesevent.entity,event.action,event.data, andevent.context. This naming inconsistency could confuse users about the actual API signature.Verify the correct parameter name from the source code and update the table to match the actual function signature.
Also applies to: 103-110
38-85: LGTM! Documentation structure and examples are well-organized.The restructuring effectively emphasizes JSON-based configuration with the Quick Start section, provides both JSON and programmatic examples, and covers common transport methods (fetch, xhr, beacon). The examples demonstrate realistic use cases including analytics, beacon transport, and custom data mapping, aligning well with walkerOS architecture patterns.
Based on learnings, the Quick Start JSON structure correctly reflects the Source → Collector → Destination architecture with proper Flow configuration, and the mapping examples demonstrate flexible pattern matching as documented in the destination interface guidelines.
Also applies to: 118-182
197-198: Verify documentation links are current and reachable.The "Related" section references external documentation links. Ensure these URLs are correct and will remain stable:
- Line 197:
https://www.walkeros.io/docs/destinations/web/api/- Line 198:
../../../core/src/types/destination.tsVerify the relative path at line 198 correctly resolves from this file location.
packages/cli/README.md (4)
27-42: LGTM — Quick Start section clearly introduces new functionality.
105-147: LGTM — Push command documentation is thorough and well-differentiated from simulate. The comparison table (lines 139–144) effectively communicates the key difference between real vs. mocked API calls.
265-333: Clarify thecodefield and verify it matches the underlying implementation.Lines 297 and 301 introduce a
"code"field for named exports (e.g.,"code": "destinationBigQuery"), but this field doesn't appear in the main flow configuration example (lines 230–261) or in the documented package configuration structure. Verify that:
- The
codefield is actually supported in the source/destination configuration objects.- It aligns with how packages are resolved during bundling.
- Update the main flow configuration example (lines 235–256) to show realistic usage of the
codefield if it's a recommended pattern.
38-38: Address Markdown lint warning: add language identifier to code block (if separate block).The static analysis tool flagged MD040 on line 38. Verify the fenced code block context—line 38 appears to be inside the
\``bashblock starting at line 29, so this may be a false positive. If there's an unclosed code block elsewhere, ensure all fenced code blocks have a language specified (e.g.,```bash,```json,```typescript`).packages/collector/README.md (2)
97-118: Logger configuration documentation is clear and well-integrated.The new logger configuration is well-documented with a practical example showing level and handler properties. The example aligns with the PR's logger-based configuration changes.
122-128: Configuration table successfully updated with logger documentation.The table now clearly documents the new
loggerconfiguration option with type, description, and examples. The removal ofverbose,onError, andonLogin favor of the logger object is properly reflected.packages/server/destinations/aws/README.md (2)
25-51: Documentation updates look good.The restructured Quick Start section clearly shows both JSON Flow configuration and programmatic usage patterns. The examples properly reflect the new
startFlowAPI with the destinations array structure, and the configuration nesting for AWS credentials is clear.Also applies to: 53-80
101-101: Update broken documentation link for type definitions.Line 101 references
./src/types/which does not exist. The TypeScript interfaces are located at./src/firehose/types/instead. The second reference to../../../core/src/types/destination.ts(line 106) is correct.
| /** | ||
| * Validates documentation quality standards | ||
| * Usage: npx tsx scripts/validate-docs.ts | ||
| */ |
There was a problem hiding this comment.
Usage comment path is outdated.
The usage line still points to scripts/validate-docs.ts, but the file lives under apps/scripts/… and package.json calls npx tsx apps/scripts/validate-docs.ts. Updating the comment will prevent confusion for anyone invoking it manually.
- * Usage: npx tsx scripts/validate-docs.ts
+ * Usage: npx tsx apps/scripts/validate-docs.ts🤖 Prompt for AI Agents
In apps/scripts/validate-docs.ts around lines 2 to 5, the usage comment
incorrectly references "scripts/validate-docs.ts"; update the comment to the
correct path used by package.json and manual invocation (e.g., "npx tsx
apps/scripts/validate-docs.ts") so anyone running the script sees the accurate
command. Ensure the comment text is replaced exactly where the Usage line
appears and keep formatting consistent with the existing block comment.
| async function main() { | ||
| console.log('📋 Validating documentation standards...\n'); | ||
|
|
||
| const patterns = [ | ||
| 'packages/**/README.md', | ||
| 'website/docs/**/*.mdx', | ||
| 'skills/**/*.md', | ||
| ]; | ||
|
|
||
| const files = await glob(patterns, { | ||
| cwd: ROOT, | ||
| ignore: ['**/node_modules/**'], | ||
| absolute: true, | ||
| }); | ||
|
|
||
| for (const file of files) { | ||
| const content = readFileSync(file, 'utf-8'); | ||
| checkEventNaming(file, content); | ||
| checkLegacyPatterns(file, content); | ||
| checkDomainRefs(file, content); | ||
| checkReadmeStructure(file, content); | ||
| } | ||
|
|
||
| const errors = issues.filter((i) => i.severity === 'error'); | ||
| const warnings = issues.filter((i) => i.severity === 'warning'); | ||
|
|
||
| if (issues.length === 0) { | ||
| console.log('✅ All documentation passes quality checks!\n'); | ||
| process.exit(0); | ||
| } | ||
|
|
||
| if (errors.length > 0) { | ||
| console.log(`❌ Found ${errors.length} errors:\n`); | ||
| for (const issue of errors) { | ||
| console.log(` ${issue.file}${issue.line ? `:${issue.line}` : ''}`); | ||
| console.log(` ${issue.message}\n`); | ||
| } | ||
| } | ||
|
|
||
| if (warnings.length > 0) { | ||
| console.log(`⚠️ Found ${warnings.length} warnings:\n`); | ||
| for (const issue of warnings) { | ||
| console.log(` ${issue.file}${issue.line ? `:${issue.line}` : ''}`); | ||
| console.log(` ${issue.message}\n`); | ||
| } | ||
| } | ||
|
|
||
| // Exit with error only for errors, not warnings | ||
| process.exit(errors.length > 0 ? 1 : 0); | ||
| } | ||
|
|
||
| main().catch(console.error); |
There was a problem hiding this comment.
Ensure unexpected errors cause a non‑zero exit code.
As with the link validator, main().catch(console.error); logs unexpected errors but then exits successfully, which can hide infrastructure or fs/glob problems in CI.
async function main() {
console.log('📋 Validating documentation standards...\n');
…
}
-main().catch(console.error);
+main().catch((error) => {
+ console.error(error);
+ process.exit(1);
+});This preserves existing behavior on validation success while making operational failures visible to pipelines.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async function main() { | |
| console.log('📋 Validating documentation standards...\n'); | |
| const patterns = [ | |
| 'packages/**/README.md', | |
| 'website/docs/**/*.mdx', | |
| 'skills/**/*.md', | |
| ]; | |
| const files = await glob(patterns, { | |
| cwd: ROOT, | |
| ignore: ['**/node_modules/**'], | |
| absolute: true, | |
| }); | |
| for (const file of files) { | |
| const content = readFileSync(file, 'utf-8'); | |
| checkEventNaming(file, content); | |
| checkLegacyPatterns(file, content); | |
| checkDomainRefs(file, content); | |
| checkReadmeStructure(file, content); | |
| } | |
| const errors = issues.filter((i) => i.severity === 'error'); | |
| const warnings = issues.filter((i) => i.severity === 'warning'); | |
| if (issues.length === 0) { | |
| console.log('✅ All documentation passes quality checks!\n'); | |
| process.exit(0); | |
| } | |
| if (errors.length > 0) { | |
| console.log(`❌ Found ${errors.length} errors:\n`); | |
| for (const issue of errors) { | |
| console.log(` ${issue.file}${issue.line ? `:${issue.line}` : ''}`); | |
| console.log(` ${issue.message}\n`); | |
| } | |
| } | |
| if (warnings.length > 0) { | |
| console.log(`⚠️ Found ${warnings.length} warnings:\n`); | |
| for (const issue of warnings) { | |
| console.log(` ${issue.file}${issue.line ? `:${issue.line}` : ''}`); | |
| console.log(` ${issue.message}\n`); | |
| } | |
| } | |
| // Exit with error only for errors, not warnings | |
| process.exit(errors.length > 0 ? 1 : 0); | |
| } | |
| main().catch(console.error); | |
| async function main() { | |
| console.log('📋 Validating documentation standards...\n'); | |
| const patterns = [ | |
| 'packages/**/README.md', | |
| 'website/docs/**/*.mdx', | |
| 'skills/**/*.md', | |
| ]; | |
| const files = await glob(patterns, { | |
| cwd: ROOT, | |
| ignore: ['**/node_modules/**'], | |
| absolute: true, | |
| }); | |
| for (const file of files) { | |
| const content = readFileSync(file, 'utf-8'); | |
| checkEventNaming(file, content); | |
| checkLegacyPatterns(file, content); | |
| checkDomainRefs(file, content); | |
| checkReadmeStructure(file, content); | |
| } | |
| const errors = issues.filter((i) => i.severity === 'error'); | |
| const warnings = issues.filter((i) => i.severity === 'warning'); | |
| if (issues.length === 0) { | |
| console.log('✅ All documentation passes quality checks!\n'); | |
| process.exit(0); | |
| } | |
| if (errors.length > 0) { | |
| console.log(`❌ Found ${errors.length} errors:\n`); | |
| for (const issue of errors) { | |
| console.log(` ${issue.file}${issue.line ? `:${issue.line}` : ''}`); | |
| console.log(` ${issue.message}\n`); | |
| } | |
| } | |
| if (warnings.length > 0) { | |
| console.log(`⚠️ Found ${warnings.length} warnings:\n`); | |
| for (const issue of warnings) { | |
| console.log(` ${issue.file}${issue.line ? `:${issue.line}` : ''}`); | |
| console.log(` ${issue.message}\n`); | |
| } | |
| } | |
| // Exit with error only for errors, not warnings | |
| process.exit(errors.length > 0 ? 1 : 0); | |
| } | |
| main().catch((error) => { | |
| console.error(error); | |
| process.exit(1); | |
| }); |
🤖 Prompt for AI Agents
In apps/scripts/validate-docs.ts around lines 105 to 156, the top‑level promise
rejection is only logged by main().catch(console.error) which still exits with
status 0; change the catch to log the error and exit non‑zero so unexpected
runtime failures fail CI. Update the call to handle errors by printing the error
(including stack) and then calling process.exit(1) (or wrap main() in an
immediately invoked async try/catch and exit 1 on catch) so only successful
validation results in a zero exit code.
| async function main() { | ||
| console.log('🔗 Validating internal links...\n'); | ||
|
|
||
| const files = await glob(PATTERNS, { | ||
| cwd: ROOT, | ||
| ignore: ['**/node_modules/**', ...SKIP_PATTERNS], | ||
| absolute: true, | ||
| }); | ||
|
|
||
| for (const file of files) { | ||
| const content = readFileSync(file, 'utf-8'); | ||
| const links = extractLinks(content); | ||
|
|
||
| for (const { link, line } of links) { | ||
| const error = validateLink(file, link, line); | ||
| if (error) { | ||
| errors.push(error); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (errors.length === 0) { | ||
| console.log('✅ All internal links are valid!\n'); | ||
| process.exit(0); | ||
| } | ||
|
|
||
| console.log(`❌ Found ${errors.length} broken links:\n`); | ||
| for (const error of errors) { | ||
| console.log(` ${error.file}:${error.line}`); | ||
| console.log(` Link: ${error.link}`); | ||
| console.log(` ${error.reason}\n`); | ||
| } | ||
|
|
||
| process.exit(1); | ||
| } | ||
|
|
||
| main().catch(console.error); |
There was a problem hiding this comment.
Unhandled failures currently exit with code 0; make script fail‑fast on errors.
Right now main().catch(console.error); only logs unexpected errors (glob/fs, etc.) and then the process exits successfully, which can mask real failures in CI. It’s safer to force a non‑zero exit code when the promise rejects.
-async function main() {
+async function main() {
console.log('🔗 Validating internal links...\n');
…
}
-main().catch(console.error);
+main().catch((error) => {
+ console.error(error);
+ process.exit(1);
+});This keeps the happy path unchanged while ensuring infra or I/O problems are treated as validation failures.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async function main() { | |
| console.log('🔗 Validating internal links...\n'); | |
| const files = await glob(PATTERNS, { | |
| cwd: ROOT, | |
| ignore: ['**/node_modules/**', ...SKIP_PATTERNS], | |
| absolute: true, | |
| }); | |
| for (const file of files) { | |
| const content = readFileSync(file, 'utf-8'); | |
| const links = extractLinks(content); | |
| for (const { link, line } of links) { | |
| const error = validateLink(file, link, line); | |
| if (error) { | |
| errors.push(error); | |
| } | |
| } | |
| } | |
| if (errors.length === 0) { | |
| console.log('✅ All internal links are valid!\n'); | |
| process.exit(0); | |
| } | |
| console.log(`❌ Found ${errors.length} broken links:\n`); | |
| for (const error of errors) { | |
| console.log(` ${error.file}:${error.line}`); | |
| console.log(` Link: ${error.link}`); | |
| console.log(` ${error.reason}\n`); | |
| } | |
| process.exit(1); | |
| } | |
| main().catch(console.error); | |
| async function main() { | |
| console.log('🔗 Validating internal links...\n'); | |
| const files = await glob(PATTERNS, { | |
| cwd: ROOT, | |
| ignore: ['**/node_modules/**', ...SKIP_PATTERNS], | |
| absolute: true, | |
| }); | |
| for (const file of files) { | |
| const content = readFileSync(file, 'utf-8'); | |
| const links = extractLinks(content); | |
| for (const { link, line } of links) { | |
| const error = validateLink(file, link, line); | |
| if (error) { | |
| errors.push(error); | |
| } | |
| } | |
| } | |
| if (errors.length === 0) { | |
| console.log('✅ All internal links are valid!\n'); | |
| process.exit(0); | |
| } | |
| console.log(`❌ Found ${errors.length} broken links:\n`); | |
| for (const error of errors) { | |
| console.log(` ${error.file}:${error.line}`); | |
| console.log(` Link: ${error.link}`); | |
| console.log(` ${error.reason}\n`); | |
| } | |
| process.exit(1); | |
| } | |
| main().catch((error) => { | |
| console.error(error); | |
| process.exit(1); | |
| }); |
🤖 Prompt for AI Agents
In apps/scripts/validate-links.ts around lines 125 to 161, the top-level promise
rejection is currently only logged which leaves the process exiting with code 0
on unexpected failures; update the final promise handler so that any caught
error is logged and then the process exits with a non-zero status (e.g., call
process.exit(1)) to fail fast in CI; replace the existing
main().catch(console.error) with a handler that prints the error and exits 1.
| <a href="https://www.walkeros.io"> | ||
| <img title="elbwalker" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/> |
There was a problem hiding this comment.
Add alt text to the logo image for accessibility.
The image is missing an alt attribute, which is required for accessibility compliance and SEO.
Apply this diff to add alt text:
- <img title="elbwalker" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/>
+ <img alt="elbwalker logo" title="elbwalker" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <a href="https://www.walkeros.io"> | |
| <img title="elbwalker" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/> | |
| <a href="https://www.walkeros.io"> | |
| <img alt="elbwalker logo" title="elbwalker" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/> |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
3-3: Images should have alternate text (alt text)
(MD045, no-alt-text)
🤖 Prompt for AI Agents
In packages/collector/README.md around lines 2 to 3, the logo image lacks an alt
attribute; add a descriptive alt text to the <img> tag (for example:
alt="Elbwalker logo") to satisfy accessibility and SEO requirements, ensuring
the attribute is included alongside the existing title, src and width
attributes.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/server/sources/gcp/README.md (1)
222-230: Resolve contradiction between Architecture description and usage examples.Line 230 states: "Direct assignment:
http('handler', source.push)" but both Usage examples (lines 26 and 71) instead registerelb(the collector's push method fromstartFlow), notsource.push. The Architecture description does not match the actual recommended patterns shown in the Usage examples.Either correct the Architecture section to reflect the
startFlow+elbpattern used in the examples, or clarify when/whysource.pushwould be used directly.packages/core/src/schemas/README.md (1)
47-47: Update README diagram and clarify that handler.ts does not exist.Line 47 shows
storage.ts + handler.ts ━→ utilities.ts, and line 158 states only "Mirrorstypes/storage.ts", buttypes/handler.tsdoes not exist in the repository. Handler schemas (ErrorHandlerSchema,LogHandlerSchema,HandlerSchema) are defined inline inutilities.tsitself, not mirrored from a separate file. Update line 47 to show onlystorage.ts ━→ utilities.ts, and correct the comment inutilities.ts(line 8) which incorrectly references the non-existenttypes/handler.ts.
♻️ Duplicate comments (8)
packages/server/sources/aws/README.md (2)
199-201: Add language specification to fenced code block.The GET request example lacks a language identifier for syntax highlighting.
Apply this diff:
**GET - Pixel Tracking:** -``` +```text GET /collect?event=page%20view&data[title]=Home&data[path]=/ -``` +```
282-286: Add language specification to fenced code block.The CloudWatch Insights query block lacks a language identifier for proper syntax highlighting.
Apply this diff:
configure CloudWatch Insights queries: -``` +```sql fields @timestamp, level, message, requestId, error | filter level = "error" | sort @timestamp desc -``` +```packages/web/sources/browser/README.md (1)
3-3: Add missing alt attribute to image for accessibility and linting compliance.The image element is still missing an
altattribute, which is required for markdown linting (MD045) and accessibility standards. This issue was flagged in a previous review and must be addressed.Apply this diff to add descriptive alt text:
- <img title="elbwalker" src='https://www.walkeros.io/img/elbwalker_logo.png' width="256px"/> + <img title="elbwalker" alt="walkerOS logo" src='https://www.walkeros.io/img/elbwalker_logo.png' width="256px"/>packages/web/destinations/plausible/README.md (1)
3-3: Unresolved accessibility issue: Add alt text to the logo image.This is the same issue previously flagged in an earlier review. The image is missing alt text, which impacts accessibility and SEO. Add a descriptive alt attribute to the image tag.
Apply this diff to add alt text:
- <img title="elbwalker" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/> + <img title="elbwalker" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px" alt="elbwalker logo"/>packages/server/sources/fetch/src/index.ts (1)
18-19: Remove unusedstartTimevariable.The variable is declared but never used - this is dead code that should be removed.
Apply this diff:
const push = async (request: Request): Promise<Response> => { - const startTime = Date.now(); - try {apps/scripts/validate-docs.ts (2)
2-5: Fix Usage path in the header commentThe usage line still references
scripts/validate-docs.ts, but the script lives underapps/scripts/…and is typically run asnpx tsx apps/scripts/validate-docs.ts. Updating this avoids confusion when invoked manually:- * Usage: npx tsx scripts/validate-docs.ts + * Usage: npx tsx apps/scripts/validate-docs.ts
124-128: Make unexpected validation errors fail CI with a non‑zero exit codeRight now
main().catch(console.error);logs unexpected errors (e.g., glob/fs issues) but still exits with status 0, which can hide real failures in CI.You can preserve existing behavior on success while failing hard on unexpected errors:
-main().catch(console.error); +main().catch((error) => { + console.error(error); + process.exit(1); +});That way only clean validation runs return exit code 0.
packages/server/destinations/meta/README.md (1)
3-3: Add missing alt text to the image.The
<img>tag lacks analtattribute required for accessibility and SEO. This was flagged in a previous review and needs to be resolved.Apply this diff to add descriptive alt text:
- <img title="elbwalker" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/> + <img title="elbwalker" alt="elbwalker logo" src="https://www.walkeros.io/img/elbwalker_logo.png" width="256px"/>
🧹 Nitpick comments (8)
website/docs/guides/session.mdx (1)
119-125: Use "whether" instead of "if" for grammatical clarity.Line 122 reads "decide if storage access is allowed or not," which is colloquial. For formal documentation, prefer "decide whether storage access is allowed."
- Setting a consent state to wait for before detecting a new session is used to - decide if storage access is allowed or not. If set, it registers an + Setting a consent state to wait for before detecting a new session is used to + decide whether storage access is allowed. If set, it registers anpackages/server/sources/gcp/README.md (1)
199-220: Clarify the relationship between the testing pattern and main examples.The Testing section shows calling
sourceCloudFunction()directly with environment injection ({ elb: mockElb }), which differs from the high-levelstartFlowpattern in the Basic Usage section. The relationship and use cases for each approach could be clearer (e.g., is this a unit test vs. integration test pattern?).packages/docker/README.md (1)
42-42: Consider integrating the production-focus statement into the Overview or Architecture sections.Line 42 introduces "This is a minimal runtime image optimized for production deployments," which contradicts the earlier Overview's emphasis on dual-purpose (demo-enabled for both testing and production). The statement feels disconnected from surrounding sections and would flow better either as part of the Overview narrative or removed entirely, since the Overview already conveys the production-ready nature alongside demo functionality.
website/docs/destinations/web/piwikpro.mdx (1)
39-55: Align Piwik PRO import style between live code and exampleAt the top of the doc you use a default import (
import destinationPiwikPro from '@walkeros/web-destination-piwikpro';), but the setup snippet shows a named import (import { destinationPiwikPro } …). Please confirm the package actually exportsdestinationPiwikProas a named export and either:
- Switch the snippet to a default import for consistency, or
- Update the top-level import to match the recommended public shape.
This keeps the docs copy‑pasteable.
skills/mapping-configuration/SKILL.md (1)
90-145: Make TypeScript mapping snippets syntactically validSeveral TS examples in this skill use semicolons where object literals expect commas or no punctuation (for example in the “Static Value”, “Function Transform”, and “Array Loop” sections). As written, these would not compile if pasted into a TS file.
For instance, you could adjust:
// Static values { value: 'USD' } { value: 99.99 } { value: true } // Array loop { loop: [ 'nested', { map: { item_id: 'data.id', quantity: 'data.quantity', }, }, ], }Aligning examples with valid TS makes them safer to copy/paste and closer to the real configs you describe.
skills/understanding-flow/SKILL.md (1)
21-29: Add a language to the flow diagram code fenceThe flow diagram block currently uses a bare triple‑backtick fence, which triggers markdownlint MD040. Consider annotating it, e.g.:
-``` +```text Sources → Collector → Destinations (Data Capture) (Processing) (Delivery) …This keeps the diagram readable and makes the linter happy.
skills/understanding-mapping/SKILL.md (1)
90-157: Tighten TS syntax in value/function/loop mapping examplesSimilar to the mapping-configuration skill, the “Static Value”, “Function Transform”, and “Array Loop” sections use semicolons in positions that make the snippets invalid TS (e.g., inside object literals and after arrays).
Consider normalizing them along these lines:
// Static values { value: 'USD' } { value: 99.99 } { value: true } // Function transforms { fn: (event) => event.data.price * 100 } // cents { fn: (event) => event.user.email?.split('@')[1] } // domain // Array loop { loop: [ 'nested', { map: { item_id: 'data.id', quantity: 'data.quantity', }, }, ], }That way the docs reflect real, compilable mapping configs.
website/docs/destinations/server/datamanager.mdx (1)
61-88: Guided Mapping Helpers section demonstrates advanced setup well; clarify path expression syntax.The section effectively shows how global helper fields (userData, userId, clientId, sessionAttributes) and consent mappings can reduce repetition. However, the path expressions like
'user.mail'and'data.phone'in the userData mapping may be unclear to users unfamiliar with walkerOS path notation. Consider adding a brief comment above the userData block explaining that these are path expressions evaluated at runtime against the event context (similar to the consent mapping clarification on line 90).Add a clarifying comment:
// Guided helpers (apply to all events) + // Path expressions evaluated at runtime (e.g., 'user.mail' extracts event.user.mail) userData: { email: 'user.mail', phone: 'data.phone', },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (37)
AGENT.md(1 hunks)apps/scripts/validate-docs.ts(1 hunks)packages/cli/README.md(5 hunks)packages/cli/demos/README.md(2 hunks)packages/cli/examples/README.md(2 hunks)packages/core/src/schemas/README.md(1 hunks)packages/docker/README.md(2 hunks)packages/docker/demos/README.md(1 hunks)packages/server/destinations/meta/README.md(2 hunks)packages/server/sources/aws/README.md(1 hunks)packages/server/sources/aws/examples/README.md(1 hunks)packages/server/sources/aws/examples/basic-handler.ts(1 hunks)packages/server/sources/fetch/README.md(1 hunks)packages/server/sources/fetch/src/index.ts(1 hunks)packages/server/sources/gcp/README.md(2 hunks)packages/web/destinations/piwikpro/README.md(2 hunks)packages/web/destinations/plausible/README.md(2 hunks)packages/web/sources/browser/README.md(3 hunks)skills/mapping-configuration/SKILL.md(1 hunks)skills/understanding-events/SKILL.md(1 hunks)skills/understanding-flow/SKILL.md(1 hunks)skills/understanding-mapping/SKILL.md(1 hunks)website/docs/collector/commands.mdx(1 hunks)website/docs/destinations/server/aws.mdx(1 hunks)website/docs/destinations/server/datamanager.mdx(2 hunks)website/docs/destinations/server/gcp.mdx(2 hunks)website/docs/destinations/server/meta-capi.mdx(1 hunks)website/docs/destinations/web/gtag/ads.mdx(1 hunks)website/docs/destinations/web/gtag/ga4.mdx(1 hunks)website/docs/destinations/web/gtag/gtm.mdx(1 hunks)website/docs/destinations/web/piwikpro.mdx(1 hunks)website/docs/destinations/web/plausible.mdx(4 hunks)website/docs/getting-started/event-model.mdx(1 hunks)website/docs/getting-started/index.mdx(1 hunks)website/docs/getting-started/quickstart/index.mdx(1 hunks)website/docs/guides/session.mdx(1 hunks)website/docs/sources/web/browser/tagging.mdx(1 hunks)
✅ Files skipped from review due to trivial changes (5)
- packages/docker/demos/README.md
- packages/cli/examples/README.md
- skills/understanding-events/SKILL.md
- website/docs/getting-started/quickstart/index.mdx
- website/docs/getting-started/event-model.mdx
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/web/destinations/piwikpro/README.md
- packages/server/sources/aws/examples/basic-handler.ts
- packages/server/sources/fetch/README.md
- packages/server/sources/aws/examples/README.md
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENT.md)
**/*.{ts,tsx}: NEVER useanytype - always use proper TypeScript types from @walkeros/core or package-specific types
Implement event structure with consistent properties: name (ENTITY ACTION format), data (entity-specific), context (state/environment), globals (global properties), user (identification), nested (related entities), consent, and system-generated fields (id, timestamp, entity, action)
Use core functions from @walkeros/core for common operations: getEvent(), createEvent() for event creation, getMappingEvent(), getMappingValue() for transformations, isString(), isObject(), isDefined() for type checking, assign(), clone() for object operations, tryCatch(), tryCatchAsync() for error handling
Useimport typefor type-only imports to ensure proper TypeScript compilation and tree-shaking
Avoid inline change comments like '// Fixed for version X' or '// Changed due to Y' - Git tracks history, use migrations and documentation for breaking changes instead
Use Root-level Turborepo commands for parallel operations across packages, navigate to individual packages for focused development, and test changes across all affected packages due to core package dependencies
Files:
packages/server/sources/fetch/src/index.tsapps/scripts/validate-docs.ts
🧠 Learnings (12)
📓 Common learnings
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to packages/*/src/**/*.ts : Implement Destination interface with standard structure: type, init(), push(), pushBatch() methods and config object with optional settings, mapping, data, consent, policy, queue, and dryRun properties
📚 Learning: 2025-11-25T16:24:48.032Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T16:24:48.032Z
Learning: Refer to AGENT.md for all relevant instructions and guidelines when working with the walkerOS codebase
Applied to files:
packages/cli/demos/README.mdAGENT.mdskills/understanding-flow/SKILL.md
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to packages/*/src/**/*.ts : Implement Destination interface with standard structure: type, init(), push(), pushBatch() methods and config object with optional settings, mapping, data, consent, policy, queue, and dryRun properties
Applied to files:
website/docs/destinations/server/gcp.mdxwebsite/docs/destinations/web/piwikpro.mdxwebsite/docs/destinations/web/gtag/gtm.mdxwebsite/docs/collector/commands.mdxwebsite/docs/destinations/server/meta-capi.mdxAGENT.mdpackages/cli/README.mdwebsite/docs/destinations/web/plausible.mdxskills/understanding-mapping/SKILL.mdwebsite/docs/destinations/web/gtag/ga4.mdxpackages/server/sources/fetch/src/index.tswebsite/docs/destinations/server/datamanager.mdxpackages/server/sources/gcp/README.mdwebsite/docs/destinations/web/gtag/ads.mdxskills/understanding-flow/SKILL.md
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Use the Source → Collector → Destination(s) architecture pattern for event processing: Sources capture events from various sources (browser DOM, dataLayer, server), Collector handles central event processing with consent management, Destinations transform and send events to third-party tools
Applied to files:
website/docs/destinations/server/gcp.mdxwebsite/docs/destinations/web/piwikpro.mdxwebsite/docs/destinations/web/gtag/gtm.mdxwebsite/docs/collector/commands.mdxwebsite/docs/destinations/server/meta-capi.mdxwebsite/docs/destinations/server/aws.mdxpackages/server/destinations/meta/README.mdAGENT.mdskills/understanding-mapping/SKILL.mdwebsite/docs/destinations/web/gtag/ga4.mdxpackages/server/sources/fetch/src/index.tswebsite/docs/destinations/server/datamanager.mdxwebsite/docs/destinations/web/gtag/ads.mdxskills/understanding-flow/SKILL.md
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: All walkerOS components communicate via push functions: sources use source.push(), collector uses collector.push(), destinations use destination.push(), and ELB uses elb() as the alias for collector.push()
Applied to files:
website/docs/destinations/server/gcp.mdxpackages/server/sources/aws/README.mdwebsite/docs/destinations/web/piwikpro.mdxwebsite/docs/destinations/web/gtag/gtm.mdxwebsite/docs/collector/commands.mdxwebsite/docs/destinations/server/meta-capi.mdxwebsite/docs/destinations/server/aws.mdxpackages/cli/README.mdwebsite/docs/destinations/web/plausible.mdxwebsite/docs/destinations/web/gtag/ga4.mdxpackages/server/sources/fetch/src/index.tswebsite/docs/destinations/server/datamanager.mdxpackages/server/sources/gcp/README.mdwebsite/docs/destinations/web/gtag/ads.mdxpackages/web/sources/browser/README.mdskills/understanding-flow/SKILL.md
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to packages/*/src/**/*.ts : Implement event mapping with flexible patterns: exact entity-action matches, wildcard patterns using '*', conditional mappings with functions, and value mapping strategies including string key extraction, static values, custom functions, object transformation, array processing, and consent-based mapping
Applied to files:
skills/mapping-configuration/SKILL.mdAGENT.mdpackages/cli/README.mdskills/understanding-mapping/SKILL.mdwebsite/docs/destinations/server/datamanager.mdx
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to **/*.{ts,tsx} : Use core functions from walkeros/core for common operations: getEvent(), createEvent() for event creation, getMappingEvent(), getMappingValue() for transformations, isString(), isObject(), isDefined() for type checking, assign(), clone() for object operations, tryCatch(), tryCatchAsync() for error handling
Applied to files:
skills/mapping-configuration/SKILL.mdAGENT.mdpackages/cli/README.mdskills/understanding-mapping/SKILL.mdpackages/core/src/schemas/README.md
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to **/*.test.{ts,tsx} : Before documenting code patterns, create functional, executable tests to validate them - use /workspaces/walkerOS/apps/quickstart as the source of truth for working examples
Applied to files:
AGENT.mdpackages/cli/README.mdpackages/docker/README.mdapps/scripts/validate-docs.ts
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to **/*.test.{ts,tsx} : Write component-level integration tests with mocked external APIs and boundaries: test destinations by mocking external APIs (gtag, fbq, etc.), test collector with mocked destinations and sources, test sources with mocked collector interface
Applied to files:
AGENT.md
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to **/*.{ts,tsx} : Use Root-level Turborepo commands for parallel operations across packages, navigate to individual packages for focused development, and test changes across all affected packages due to core package dependencies
Applied to files:
packages/cli/README.mdapps/scripts/validate-docs.ts
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Consent is optional and configurable at multiple levels: destination-level, event mapping level, value mapping level, and policy level - without explicit consent requirements, events process normally
Applied to files:
website/docs/destinations/server/datamanager.mdx
📚 Learning: 2025-11-25T16:25:10.439Z
Learnt from: CR
Repo: elbwalker/walkerOS PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-25T16:25:10.439Z
Learning: Applies to **/*.{ts,tsx} : Avoid inline change comments like '// Fixed for version X' or '// Changed due to Y' - Git tracks history, use migrations and documentation for breaking changes instead
Applied to files:
apps/scripts/validate-docs.ts
🧬 Code graph analysis (1)
packages/server/sources/fetch/src/index.ts (5)
packages/server/sources/fetch/src/types.ts (1)
FetchSource(27-29)packages/server/sources/fetch/src/utils.ts (2)
createCorsHeaders(3-53)createJsonResponse(72-81)packages/core/src/request.ts (1)
requestToData(12-54)packages/core/src/is.ts (1)
isDefined(49-51)packages/core/src/types/walkeros.ts (1)
DeepPartialEvent(10-10)
🪛 LanguageTool
website/docs/guides/session.mdx
[style] ~122-~122: In contexts where ‘if’ is followed by ‘or’, using ‘whether’ may be more appropriate (and formal).
Context: ...tecting a new session is used to decide if storage access is allowed or not. If se...
(IF_WHETHER)
🪛 markdownlint-cli2 (0.18.1)
packages/server/sources/aws/README.md
199-199: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
282-282: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
packages/web/destinations/plausible/README.md
3-3: Images should have alternate text (alt text)
(MD045, no-alt-text)
packages/server/destinations/meta/README.md
3-3: Images should have alternate text (alt text)
(MD045, no-alt-text)
packages/web/sources/browser/README.md
3-3: Images should have alternate text (alt text)
(MD045, no-alt-text)
skills/understanding-flow/SKILL.md
21-21: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build and test
- GitHub Check: AWS Amplify Console Web Preview
- GitHub Check: Build and test
🔇 Additional comments (47)
packages/web/sources/browser/README.md (4)
2-2: Domain migration to walkeros.io is properly applied.The href reference has been correctly updated to the new domain.
15-24: Quick Start section is clear and actionable.The new Quick Start section provides a concise entry point for users, with correct npm installation and a straightforward TypeScript usage example using the updated
@walkeros/web-source-browserscope. The pattern aligns with the broader package naming updates in the PR.
47-63: Verify that the detailed installation example matches current API.The JavaScript example in the Installation section uses
createSource(sourceBrowser, { settings: { ... } }), while the Quick Start section uses a simpler pattern:sources: { browser: { code: sourceBrowser } }. Please confirm both patterns are supported and document which pattern is recommended for new projects.
129-130: Documentation reference link correctly updated to walkeros.io.The Commands documentation link now properly points to the new domain structure.
packages/web/destinations/plausible/README.md (2)
36-47: Destination configuration pattern is consistent with the new API.The startFlow usage with a destinations map and nested plausible configuration aligns with the refactored destination implementation. The optional domain setting is appropriately documented inline.
52-54: Configuration table updated appropriately.The table formatting and domain example are consistent with the branding updates across the documentation.
website/docs/destinations/web/plausible.mdx (2)
42-52: Setup example correctly demonstrates the new destinations configuration API.The startFlow pattern with nested destinations object and plausible configuration is correct and aligns with the refactored architecture. The optional domain setting is properly documented.
64-64: Domain references consistently updated to walkeros.io.All three component examples now use the updated domain name, maintaining consistency across the interactive documentation.
Also applies to: 85-85, 101-101
website/docs/guides/session.mdx (1)
1-406: Excellent documentation coverage with clear structure and examples.This guide comprehensively documents session management with well-organized sections, clear configuration tables, realistic code examples, and helpful Mermaid diagrams illustrating detection flows. The progression from high-level concepts (Session Data, sessionStart) through detailed sub-utilities (sessionStorage, sessionWindow) with their respective config parameters and detection processes is logical and easy to follow. Cross-references between sections (e.g., links to #storage, #sessionwindow) are consistent and aid navigation.
website/docs/sources/web/browser/tagging.mdx (1)
106-106: URL reference update is consistent with current domain across documentation.The
previous_idexample has been correctly updated to usehttps://www.walkeros.io/, which is the current domain reflected in the codebase documentation (e.g., schema references). No staleelbwalker.comreferences remain, confirming this aligns with the broader documentation update.packages/server/sources/gcp/README.md (1)
38-72: Singleton handler pattern is well-suited for Cloud Functions.The approach of caching the handler across warm invocations and deferring initialization via
setup()is a sensible best practice for serverless environments. The per-request configuration example (line 57:settings: { cors: true }) aligns well with the Configuration Options section.packages/core/src/schemas/README.md (1)
1-420: Documentation quality is strong overall.The README is well-structured with clear sections on purpose, architecture, schema files, usage patterns, naming conventions, design decisions, and development guidelines. Examples are concrete and helpful. The dual-pattern explanation and schema builder documentation are particularly well done.
packages/server/sources/fetch/src/index.ts (18)
1-9: LGTM! Clean import structure.Proper use of
import typefor type-only imports and correct usage of core utilities from@walkeros/core.
11-17: LGTM! Proper initialization pattern.Settings validation with Zod schema and appropriate error propagation for invalid configuration.
21-26: LGTM! Solid request setup.Proper URL parsing, method normalization, and CORS header preparation with appropriate error handling.
27-34: LGTM! Appropriate health check implementation.Health check returns useful metadata and correctly avoids logging routine operations.
36-39: LGTM! Standard CORS preflight handling.Correct 204 response for OPTIONS requests with appropriate CORS headers.
41-48: LGTM! Proper pixel tracking implementation.Correct query string parsing with validation and appropriate pixel response. Good decision to skip logging for high-volume tracking requests.
50-70: LGTM! Effective defense-in-depth for request size.Content-Length validation before body parsing prevents resource exhaustion. Appropriate 413 status and detailed error logging.
72-102: LGTM! Robust body parsing with double size validation.Good defense-in-depth: validates both Content-Length header and actual body size. Proper error handling for JSON parse failures.
104-111: LGTM! Proper type validation.Correct use of
isDefinedandisObjectguards from@walkeros/corewith appropriate error response.
113-133: LGTM! Safe batch detection and size validation.Proper batch identification with size limits to prevent resource exhaustion. The cast to
unknown[]is safe given the prior array check.
134-158: LGTM! Excellent batch processing with partial failure support.Appropriate use of 207 Multi-Status for partial batch failures and detailed error reporting. Success case returns useful event IDs.
160-198: LGTM! Comprehensive single event validation and processing.Excellent validation error detail with path and message. The cast to
WalkerOS.DeepPartialEventon line 182 should be safe given the EventSchema validation.
201-205: LGTM! Appropriate fallback for unsupported methods.Correct 405 status for methods other than GET, POST, and OPTIONS.
206-218: LGTM! Comprehensive error handling.Top-level catch provides safety net with appropriate logging and 500 response. Creating CORS headers without origin context is acceptable for unexpected error scenarios.
221-222: LGTM! Correct source interface structure.Returns properly structured FetchSource with type, config, and push handler.
224-234: LGTM! Clean event processing helper.Well-structured wrapper that returns either success (with id) or error, making error handling straightforward for callers.
236-286: LGTM! Robust batch processing with excellent error handling.Properly validates each event, continues on failures, and aggregates detailed results. The per-event validation and error logging makes debugging straightforward.
288-292: LGTM! Well-organized public API surface.Clean export structure with namespaced schemas and examples to prevent naming conflicts, and proper use of
export type *for type-only exports.packages/cli/demos/README.md (2)
3-11: Installation & Usage sections are clear and validator-friendlyAdding explicit
## Installationand## Usageanchored on the main CLI README andquick-demo.shgives this README a proper entry point and should satisfy the new docs validator’s structural checks. No changes needed.
104-108: Docker package link path looks correctThe new “Docker Package” reference to
../../docker/README.mdis the right relative path frompackages/cli/demos/README.mdand ties the demos into the Docker docs nicely.packages/cli/README.md (4)
27-41: Quick Start sequence is cohesive and matches CLI responsibilitiesThe new Quick Start showing
bundle → simulate → push → runprovides a clear end‑to‑end story and pairs nicely with the new docs validator’s expectation for a Quick Start/Usage section. Nothing to change here.
105-147: Double‑checkpushCLI flags and event sources against implementationThe
pushdocs clearly distinguish it fromsimulateand nicely cover inline JSON, file, and URL event sources. Since this is a new command, please confirm that:
--eventreally accepts string, file path, and URL as documented, and- the listed flags (
--flow,--json,-v,-s,--local) match the actual CLI options.That will keep the README from drifting from the binary.
228-305: Flow & package configuration examples align with the new package-based modelThe updated flow example (using
packagespluspackagereferences on sources/destinations) and the “Package Configuration Patterns” section give a clear mental model of how the bundler infers imports (default vs named vs utility). The examples cover the main cases without overcomplicating the JSON.Looks good as a reference for configuring flows under the new system.
496-505: Type Definitions and Related links improve discoverabilityLinking directly to
src/types.tsand the website docs (CLI + Flow + Docker package) makes it much easier to find the underlying types and runtime docs from the README. Paths and URLs look correct.AGENT.md (1)
1-105: Skill‑centric AGENT structure and navigation look solidThe refactored AGENT.md cleanly routes people through “Understanding” skills, “Creating Things” tasks, and concrete package/file locations, while reiterating key constraints (no
any, XP principles, tests first). Links and paths look consistent with the repo layout, and the annotated package tree fence (```text) addresses the previous markdownlint concern.No changes needed from my side.
website/docs/getting-started/index.mdx (1)
136-136: LGTM - Link path update is sound.The change from relative to absolute path (
/docs/getting-started/quickstart/) improves link robustness across the documentation hierarchy.packages/server/destinations/meta/README.md (1)
30-45: Code example correctly reflects new startFlow initialization pattern.The example properly demonstrates the updated architecture: importing
startFlow, configuring the Meta destination viastartFlow({ destinations: { meta: { code, config } } }), and nesting settings underconfig.settings. This aligns with the Destination interface structure from the learnings.website/docs/destinations/server/meta-capi.mdx (1)
24-39: Code example correctly implements the startFlow pattern.The setup example properly demonstrates the initialization pattern with nested destination configuration under
destinations.meta.config.settings.website/docs/destinations/server/gcp.mdx (1)
28-44: Both code examples correctly implement the startFlow pattern.Setup and custom schema examples properly demonstrate the nested destinations configuration with settings and optional data mapping. The progression from basic to advanced (with
data.map) is clear and follows the architectural pattern.Also applies to: 118-142
website/docs/destinations/web/gtag/gtm.mdx (1)
38-52: Code example correctly uses the startFlow pattern with GTM configuration.The setup example properly nests GTM settings (
containerId,dataLayer) underdestinations.gtag.config.settings.gtm.website/docs/destinations/server/aws.mdx (1)
28-51: Code example correctly demonstrates AWS Firehose configuration via startFlow.The setup example properly nests the Firehose settings and AWS credentials under
destinations.firehose.config.settings.firehose, with credentials nested withinconfig.credentials.website/docs/collector/commands.mdx (1)
25-51: Documentation properly prioritizes initialization-time destination configuration.The restructuring effectively communicates that the recommended approach is configuring destinations via
startFlow()during initialization, while acknowledging that dynamic additions viaelb('walker destination')orcollector.addDestination()remain available for special cases. This balances the new architectural pattern with backward compatibility.website/docs/destinations/web/gtag/ads.mdx (1)
42-67: Code example properly demonstrates Google Ads configuration with mapping.The setup correctly nests both settings and event mapping under the destinations configuration:
settings.adsfor conversion tracking, andmapping.order.complete.settings.ads.labelfor conversion labels. The subsequent example demonstrates actual event tracking viaelb().website/docs/destinations/web/gtag/ga4.mdx (1)
42-55: Setup documentation accurately reflects new startFlow API.The example correctly shows the new inline destination configuration pattern. The structure is consistent with other destinations and the configuration nesting (code → config → settings → ga4) aligns with the evolving API surface.
website/docs/destinations/server/datamanager.mdx (1)
28-51: Setup documentation correctly shows new startFlow pattern with nested configuration.The basic setup example is clear and complete, with proper structure for accessToken and per-destination configuration via operatingAccount and productDestinationId.
| ## Source-Side Mapping | ||
|
|
||
| **Transform HTTP input to walkerOS event:** | ||
|
|
||
| ```typescript | ||
| // In source config | ||
| { | ||
| mapping: { | ||
| // Map incoming field names to walkerOS structure | ||
| data: { | ||
| map: { | ||
| name: { fn: (input) => `${input.entity} ${input.action}` }, | ||
| data: 'payload', | ||
| user: { | ||
| map: { | ||
| id: 'userId', | ||
| session: 'sessionId', | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| } | ||
| ``` |
There was a problem hiding this comment.
Source-side mapping example likely nests event fields under data by mistake
In the source-side example, name, data, and user are mapped inside data.map, which would result in event.data.name, event.data.data, and event.data.user… instead of setting the top-level walkerOS fields (event.name, event.data, event.user).
To match the “Map incoming field names to walkerOS structure” description, consider restructuring:
{
- mapping: {
- // Map incoming field names to walkerOS structure
- data: {
- map: {
- name: { fn: (input) => `${input.entity} ${input.action}` },
- data: 'payload',
- user: {
- map: {
- id: 'userId',
- session: 'sessionId',
- },
- },
- },
- },
- },
+ mapping: {
+ // Map incoming field names to walkerOS structure
+ name: { fn: (input) => `${input.entity} ${input.action}` },
+ data: 'payload',
+ user: {
+ map: {
+ id: 'userId',
+ session: 'sessionId',
+ },
+ },
+ },
}This better reflects the intended walkerOS event shape.
🤖 Prompt for AI Agents
In skills/mapping-configuration/SKILL.md around lines 240 to 263, the example
nests event fields under data.map which will produce event.data.name,
event.data.data and event.data.user instead of top-level walkerOS fields; update
the example so name, data and user mappings are at the top mapping level (not
inside data.map) so they produce event.name, event.data and event.user, and
adjust any nested user mapping accordingly to keep user.id and user.session
under event.user.
| const mapping = { | ||
| // Exact match | ||
| product: { | ||
| view: { name: 'view_item' }, | ||
| add: { name: 'add_to_cart' }, | ||
| }, | ||
|
|
||
| // Wildcard: any action | ||
| product: { | ||
| '*': { name: 'product_interaction' }, | ||
| }, | ||
|
|
||
| // Wildcard: any entity | ||
| '*': { | ||
| click: { name: 'generic_click' }, | ||
| }, | ||
| }; | ||
| ``` |
There was a problem hiding this comment.
Avoid duplicate product key in event mapping example
In the const mapping = { … } example, product is defined twice; in JS/TS object literals, the second definition overwrites the first, so the “Exact match” section is effectively lost.
You can combine exact and wildcard mappings under a single product key:
const mapping = {
- // Exact match
- product: {
- view: { name: 'view_item' },
- add: { name: 'add_to_cart' },
- },
-
- // Wildcard: any action
- product: {
- '*': { name: 'product_interaction' },
- },
+ // product-specific events
+ product: {
+ // Exact matches
+ view: { name: 'view_item' },
+ add: { name: 'add_to_cart' },
+ // Wildcard: any action
+ '*': { name: 'product_interaction' },
+ },
// Wildcard: any entity
'*': {
click: { name: 'generic_click' },
},
};That keeps the example both accurate and illustrative.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const mapping = { | |
| // Exact match | |
| product: { | |
| view: { name: 'view_item' }, | |
| add: { name: 'add_to_cart' }, | |
| }, | |
| // Wildcard: any action | |
| product: { | |
| '*': { name: 'product_interaction' }, | |
| }, | |
| // Wildcard: any entity | |
| '*': { | |
| click: { name: 'generic_click' }, | |
| }, | |
| }; | |
| ``` | |
| const mapping = { | |
| // product-specific events | |
| product: { | |
| // Exact matches | |
| view: { name: 'view_item' }, | |
| add: { name: 'add_to_cart' }, | |
| // Wildcard: any action | |
| '*': { name: 'product_interaction' }, | |
| }, | |
| // Wildcard: any entity | |
| '*': { | |
| click: { name: 'generic_click' }, | |
| }, | |
| }; |
🤖 Prompt for AI Agents
In skills/understanding-mapping/SKILL.md around lines 38 to 55, the mapping
example defines the "product" key twice which causes the first (Exact match)
object to be overwritten by the second (Wildcard) object; merge the two
"product" entries into a single "product" object that contains both the exact
action keys (view, add) and the wildcard key ('*') so the example accurately
demonstrates coexistence of exact and wildcard mappings while keeping the global
'*' entry for any-entity fallbacks.
Summary by CodeRabbit
New Features
Documentation
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.