565 ai readiness #566
565 ai readiness #566
Conversation
WalkthroughAdds many documentation and skill pointer files; implements a built-in "code" destination (runtime + types); enriches collector contexts (scoped logger, merged env) and destination lifecycle logging; adds AWS Lambda and Fetch server sources with schemas, utilities and tests; refactors CLI bundler to programmatic generation; updates demos, tests, devcontainer and site docs. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant Collector
participant Destination
participant Logger
Client->>Collector: push(event) / pushBatch(events)
Collector->>Logger: logger.scope(destination.type) -> scopedLogger
Collector->>Destination: init?(context { collector, config, env, logger: scopedLogger })
Destination-->>Collector: init complete
Collector->>Logger: scopedLogger.debug("push batch" / "push")
Collector->>Destination: push(event|batch, context { collector, config, env, logger: scopedLogger })
Destination-->>Collector: success / error
Collector->>Logger: scopedLogger.debug("push done" / "push batch done")
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Potential hotspots to review:
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
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.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
skills/writing-documentation/SKILL.md (1)
132-132: Fix capitalization: "Markdown" is a proper noun.Line 132: Change "hardcoded markdown tables" to "hardcoded Markdown tables" to match style guide conventions.
This issue was flagged in a previous review but remains unresolved.
🧹 Nitpick comments (2)
skills/writing-documentation/SKILL.md (2)
259-263: Resolve duplicate heading "Installation".Lines 259 and 263 both use
## Installation, violating markdownlint MD024. These appear in different templates (Package README vs Website Doc MDX), so distinguish them for clarity:### Package README Template ... - ## Installation + ## Quick Start ```bash npm install @walkeros/[package-name]Website Doc Template (MDX)
...
Installation
Installation
Alternatively, rename the first to avoid the duplicate or use H3 (`###`) for the Package README installation section. --- `322-322`: **Wrap bare URL in link syntax or angle brackets.** Line 322 contains a bare URL `www.walkeros.io` (markdownlint MD034). Wrap it for proper markdown parsing: ```diff - CORRECT: www.walkeros.io or relative paths DO NOT USE: legacy domain references + CORRECT: <https://www.walkeros.io> or relative paths DO NOT USE: legacy domain referencesOr use markdown link syntax:
[www.walkeros.io](https://www.walkeros.io)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
website/static/img/author/alexander.pngis excluded by!**/*.pngwebsite/static/img/author/ayla.pngis excluded by!**/*.pngwebsite/static/img/author/nicolas.jpegis excluded by!**/*.jpegwebsite/static/img/company/team.jpgis excluded by!**/*.jpg
📒 Files selected for processing (19)
.claude/skills/writing-documentation/SKILL.md(1 hunks).github/workflows/test.yml(1 hunks)skills/understanding-events/SKILL.md(1 hunks)skills/understanding-mapping/SKILL.md(1 hunks)skills/writing-documentation/SKILL.md(1 hunks)website/docusaurus.config.ts(3 hunks)website/src/components/company/mission.tsx(0 hunks)website/src/components/company/values.tsx(0 hunks)website/src/components/home/hero.tsx(1 hunks)website/src/components/organisms/supportNotice.tsx(1 hunks)website/src/components/services/collectionPrinciples.tsx(0 hunks)website/src/components/services/features.tsx(0 hunks)website/src/components/services/hero.tsx(0 hunks)website/src/components/services/projects.tsx(0 hunks)website/src/components/services/sparring.tsx(0 hunks)website/src/components/services/team.tsx(0 hunks)website/src/pages/company.tsx(0 hunks)website/src/pages/legal/terms.tsx(0 hunks)website/src/pages/services.tsx(0 hunks)
💤 Files with no reviewable changes (11)
- website/src/components/company/mission.tsx
- website/src/components/company/values.tsx
- website/src/pages/company.tsx
- website/src/components/services/team.tsx
- website/src/pages/services.tsx
- website/src/components/services/hero.tsx
- website/src/components/services/sparring.tsx
- website/src/components/services/projects.tsx
- website/src/components/services/collectionPrinciples.tsx
- website/src/pages/legal/terms.tsx
- website/src/components/services/features.tsx
✅ Files skipped from review due to trivial changes (1)
- .claude/skills/writing-documentation/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (1)
- skills/understanding-mapping/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:
website/src/components/organisms/supportNotice.tsxwebsite/src/components/home/hero.tsxwebsite/docusaurus.config.ts
🧠 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: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:
website/src/components/organisms/supportNotice.tsx
📚 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.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:
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.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/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
🪛 GitHub Actions: Build and test
skills/writing-documentation/SKILL.md
[error] 350-350: Broken link target not found: ../../docs/guidelines/documentation-principles.md
[error] 351-351: Broken link target not found: ../../docs/audit/SUMMARY.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
60-60: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
236-236: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
259-259: Multiple headings with the same content
(MD024, no-duplicate-heading)
263-263: Multiple headings with the same content
(MD024, no-duplicate-heading)
281-281: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
307-307: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
316-316: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
322-322: Bare URL used
(MD034, no-bare-urls)
324-324: 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 (8)
skills/understanding-events/SKILL.md (1)
1-176: Comprehensive event documentation with excellent alignment to project principles.This skill document clearly explains the walkerOS event model with strong emphasis on core requirements (space-separated entity-action naming, statelessness, vendor-agnostic design). The property documentation is thorough, examples are practical and accurate, and cross-references to source types and related skills are well-placed. The documentation directly reinforces the strict naming conventions and property structures defined in the codebase's Event interface.
website/src/components/home/hero.tsx (1)
16-16: LGTM! Consistent with domain migration strategy.The change from an internal '/services' route to an external company URL aligns with the broader pattern of separating the documentation site (walkeros.io) from company pages (elbwalker.com), as reflected in the docusaurus configuration.
website/docusaurus.config.ts (3)
16-16: LGTM! Domain migration to walkeros.io.The site URL update from elbwalker.com to walkeros.io is clear and aligns with the PR's domain migration objectives.
96-96: LGTM! Navbar updated to reflect new site structure.Replacing the Services link with Blog is consistent with externalizing company-related pages (Services now appears in the footer linking to elbwalker.com).
146-163: LGTM! Company links properly externalized.The footer's Company section correctly routes all company-related links (About, Services, legal pages) to external elbwalker.com URLs, maintaining a clean separation between the documentation site and company site.
skills/writing-documentation/SKILL.md (3)
60-76: Add language specifiers to fenced code blocks.Multiple code blocks are missing language identifiers (markdownlint MD040):
- Line 60: Change
```to```text(hierarchy diagram)- Line 236: Change
```to```markdown(Package README template)- Line 281: Change
```to```json(Website Doc MDX template - flow config)- Line 307: Change
```to```text(event naming non-negotiable)- Line 316: Change
```to```text(package references non-negotiable)- Line 324: Change
```to```text(domain references non-negotiable)- ``` + ```text TIER 1: apps/quickstart/Similar changes needed for other blocks listed above.
Also applies to: 236-281, 307-324
105-110: Review JSX syntax in PropertyTable example.Line 107 ends with
/>(self-closing tag), but the block spans to line 110 with a closing>. Verify the syntax is correct for the intended MDX/JSX context:The example should likely be:
import { schemas } from '@walkeros/web-destination-gtag/dev'; <PropertyTable schema={schemas.settings} />Confirm this is the intended pattern and that line 110's trailing
>is not extraneous.
350-351: Remove non-existent documentation links.Lines 350-351 reference files that do not exist in the repository:
../../docs/guidelines/documentation-principles.md- does not exist../../docs/audit/SUMMARY.md- does not existThe
docsdirectory does not exist. Remove these two links from the "Related" section.Wait, let me reconsider. The original review comment says to fix the links or remove them if files don't exist. The verification confirms the files don't exist and no alternatives are available. This is an actionable issue that needs to be fixed by removing the broken links. Let me provide the correct classification:
Remove non-existent documentation links.
Lines 350-351 reference files that do not exist in the repository:
../../docs/guidelines/documentation-principles.md- does not exist../../docs/audit/SUMMARY.md- does not existThe
docsdirectory and these files do not exist anywhere in the repository. Remove both broken links from the "Related" section.Likely an incorrect or invalid review comment.
| <Link href="https://www.elbwalker.com/services" target="_blank"> | ||
| services | ||
| </Link> |
There was a problem hiding this comment.
Add rel="noopener noreferrer" for external link opened in new tab
The external link uses target="_blank" without rel="noopener noreferrer", which can allow reverse‑tabnabbing and exposes window.opener.
Suggested change:
- <Link href="https://www.elbwalker.com/services" target="_blank">
+ <Link
+ href="https://www.elbwalker.com/services"
+ target="_blank"
+ rel="noopener noreferrer"
+ >🤖 Prompt for AI Agents
In website/src/components/organisms/supportNotice.tsx around lines 21 to 23, the
external Link uses target="_blank" without rel attributes, which risks
reverse-tabnabbing; update the Link to include rel="noopener noreferrer" on the
anchor element (or pass the rel prop if using the Link component) so external
links opened in a new tab cannot access window.opener and are safer.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
.claude/settings.local.json (1)
19-19: Remove duplicate permission entry.Line 19 already contains
"Bash(npm test:*)", so the same entry at line 27 is redundant. Deduplicate the allow list by removing the duplicate."WebSearch", - "Bash(npm test:*)", "Skill(understanding-sources)", "Bash(cat:*)"Also applies to: 27-27
website/docs/destinations/create-your-own.mdx (1)
232-306: Correct generic parameter documentation and init() method signature.The environment dependencies section has structural merit but contains critical inaccuracies that contradict actual codebase patterns:
Generic parameters:
DestinationWeb.Destinationaccepts one generic parameter<T extends TypesGeneric>, not three. The documented patternDestinationWeb.Destination<Settings, Mapping, Env>is incorrect. Actual usage showsDestinationWeb.Destination<Types>(seepackages/web/destinations/plausible/src/types/index.ts:35).init() signature: The example omits the
loggerparameter present in real implementations. Actual signature isinit({ config: partialConfig, env, logger }), notinit({ config, env })(seepackages/web/destinations/piwikpro/src/index.ts:14).getEnv() behavior: Document that
getEnv()returns environment globals with fallbacks to actualwindowanddocumentwhen env is not provided, spreading any provided overrides.Update the code examples to match actual Destination interface design and include the logger parameter.
website/docs/mapping.mdx (1)
49-87: Fix parameter name and type signatures in documented function signatures.The mapping.mdx documentation contains inaccuracies in the function signatures at lines 104 and 219:
getMappingValuesecond parameter is labeledmappingbut should bedata:
- Documented:
getMappingValue(value: unknown, mapping: Mapping.Data, ...)- Actual:
getMappingValue(value, data: Mapping.Data = {}, ...)
getMappingEventevent parameter accepts broader types than documented:
- Documented:
getMappingEvent(event: WalkerOS.PartialEvent, ...)- Actual:
getMappingEvent(event: WalkerOS.DeepPartialEvent | WalkerOS.PartialEvent | WalkerOS.Event, ...)The code examples themselves (lines 49-87, 435-507) are structurally correct and properly demonstrate the mapping patterns; only the signature labels and type annotations need updating.
website/docs/getting-started/quickstart/docker.mdx (1)
101-151: Clarify the two flow configuration formats: "code" with build section vs. direct "package" references.The docker.mdx example uses a different flow format than flow.mdx. Here,
"code": "sourceExpress"references imported names from thebuild.packagessection (e.g.,"@walkeros/server-source-express"imports["sourceExpress"]), whereas flow.mdx documents the simpler direct format using"package": "@walkeros/server-source-express". While both are valid, the docker.mdx snippet claims to show the "NEW format" but doesn't explain why it differs from flow.mdx, creating confusion about when to use each format. Add a note clarifying that this is a CLI-generated format with build mappings, distinct from the direct package reference pattern shown in flow.mdx.
🧹 Nitpick comments (3)
website/docs/getting-started/flow.mdx (1)
269-286: Web-specific options section should clarify platform defaults.Lines 275-285 document
windowCollectorandwindowElbconfiguration. This section should clarify:
- Are these defaults applied automatically for web platform, or must they be explicitly set?
- What happens if both
webandserverare specified?- Should this section mention that these only apply to web builds?
This would improve clarity for users building their first flow configuration.
Consider adding a note explaining that these options only apply when the
webplatform is selected and are ignored for server builds.website/sidebars.ts (1)
44-47: Server source entries are correctly integrated into the sidebarThe four new server docs (
express,fetch,aws,gcp) are wired as flat items under Sources, matching theirwebsite/docs/sources/server/*.mdxIDs and the new Server section in the sources index. Structure is valid forSidebarsConfig.If you later want parity with the docs’ “Server” grouping, consider wrapping these in a nested “Server” category under Sources, but it’s not required for correctness.
website/docs/sources/index.mdx (1)
122-142: Primary source and reactiveonhandling are documented clearly
- The Primary source section accurately describes how
elbis derived from the first (or explicitlyprimary) source, and falls back tocollector.pushwhen no sources are configured. The override example usingprimary: trueis straightforward.- The Reactive event handling snippet correctly illustrates defining a source with an optional
onmethod to react to collector events likeconsent, and the list of available events (consent,session,ready,run) is helpful.- The Next steps links at the bottom appropriately direct readers toward browser, dataLayer, and custom source docs.
If you want to make the example fully self-contained, you could add an
import type { Source } from '@walkeros/core';line above themySourcesnippet, but that’s optional.Also applies to: 144-169, 170-178
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (33)
.claude/settings.local.json(1 hunks)website/docs/apps/cli.mdx(15 hunks)website/docs/apps/docker.mdx(5 hunks)website/docs/apps/walkerjs.mdx(6 hunks)website/docs/core/index.mdx(7 hunks)website/docs/core/server.mdx(6 hunks)website/docs/core/web.mdx(9 hunks)website/docs/destinations/code.mdx(1 hunks)website/docs/destinations/create-your-own.mdx(6 hunks)website/docs/destinations/index.mdx(2 hunks)website/docs/destinations/server/gcp.mdx(5 hunks)website/docs/destinations/web/api.mdx(1 hunks)website/docs/getting-started/flow.mdx(11 hunks)website/docs/getting-started/index.mdx(2 hunks)website/docs/getting-started/quickstart/cdn.mdx(5 hunks)website/docs/getting-started/quickstart/docker.mdx(8 hunks)website/docs/getting-started/quickstart/index.mdx(1 hunks)website/docs/getting-started/quickstart/nextjs.mdx(2 hunks)website/docs/getting-started/quickstart/react.mdx(3 hunks)website/docs/guides/migration.mdx(5 hunks)website/docs/guides/session.mdx(1 hunks)website/docs/mapping.mdx(7 hunks)website/docs/sources/create-your-own.mdx(11 hunks)website/docs/sources/index.mdx(6 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)website/docs/sources/web/browser/commands.mdx(1 hunks)website/docs/sources/web/browser/index.mdx(2 hunks)website/docs/sources/web/browser/tagger.mdx(5 hunks)website/sidebars.ts(2 hunks)
✅ Files skipped from review due to trivial changes (9)
- website/docs/core/index.mdx
- website/docs/core/server.mdx
- website/docs/guides/migration.mdx
- website/docs/sources/create-your-own.mdx
- website/docs/sources/server/aws.mdx
- website/docs/sources/web/browser/tagger.mdx
- website/docs/getting-started/quickstart/react.mdx
- website/docs/apps/walkerjs.mdx
- website/docs/sources/web/browser/commands.mdx
🚧 Files skipped from review as they are similar to previous changes (4)
- website/docs/sources/server/fetch.mdx
- website/docs/destinations/code.mdx
- website/docs/sources/server/gcp.mdx
- website/docs/getting-started/index.mdx
🧰 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 (15)
📚 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:
website/docs/getting-started/quickstart/docker.mdxwebsite/docs/getting-started/quickstart/cdn.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: 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/create-your-own.mdxwebsite/docs/sources/index.mdxwebsite/docs/destinations/server/gcp.mdxwebsite/docs/getting-started/flow.mdxwebsite/docs/sources/server/index.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 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/create-your-own.mdxwebsite/docs/mapping.mdxwebsite/docs/getting-started/flow.mdxwebsite/docs/getting-started/quickstart/nextjs.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: 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/create-your-own.mdxwebsite/docs/getting-started/quickstart/index.mdxwebsite/docs/destinations/index.mdxwebsite/docs/mapping.mdxwebsite/docs/sources/index.mdxwebsite/docs/destinations/server/gcp.mdxwebsite/docs/getting-started/quickstart/cdn.mdxwebsite/docs/getting-started/flow.mdxwebsite/docs/sources/server/index.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:
website/docs/destinations/create-your-own.mdxwebsite/docs/mapping.mdxwebsite/docs/core/web.mdxwebsite/docs/getting-started/quickstart/nextjs.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} : NEVER use `any` type - always use proper TypeScript types from walkeros/core or package-specific types
Applied to files:
website/docs/destinations/create-your-own.mdxwebsite/docs/mapping.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 **/*.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:
website/docs/destinations/create-your-own.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 `import type` for type-only imports to ensure proper TypeScript compilation and tree-shaking
Applied to files:
website/docs/destinations/create-your-own.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 **/*.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:
website/docs/destinations/create-your-own.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 **/*.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:
website/docs/destinations/create-your-own.mdxwebsite/docs/mapping.mdxwebsite/docs/getting-started/quickstart/nextjs.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:
website/docs/mapping.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: 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:
website/docs/mapping.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: 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/mapping.mdxwebsite/docs/getting-started/flow.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} : 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:
website/docs/mapping.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 : Use Higher-Order Functions (HOF) and smart abstractions to avoid redundancy and keep implementations lean and performant
Applied to files:
website/docs/mapping.mdx
🪛 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 (27)
.claude/settings.local.json (1)
28-29: New permissions align well with PR scope.The addition of
Skill(understanding-sources)andBash(cat:*)appropriately enable Claude Code discovery of the new skill metadata (e.g.,.claude/skills/understanding-sources/SKILL.md) and support for documentation workflows introduced in this PR.website/docs/getting-started/quickstart/cdn.mdx (2)
16-16: Heading style consistency improved.The normalization of all section headings to sentence case (e.g., "Use Cases" → "Use cases") is a good improvement that aligns with markdown documentation conventions and enhances visual consistency throughout the guide.
Also applies to: 63-63, 175-175, 232-232, 273-273
20-21: Well-targeted use case additions.The new entries—"Quick prototypes" and "Third-party integrations"—accurately reflect common CDN deployment scenarios and improve clarity for users deciding whether this setup approach suits their needs.
website/docs/core/web.mdx (1)
21-21: Stylistic heading normalization aligns with documentation consistency efforts.These section-heading updates (converting to sentence case) are purely stylistic changes that improve consistency across the documentation. The content, code snippets, type definitions, and API documentation remain accurate and complete.
Also applies to: 67-67, 98-98, 120-120, 165-165, 200-200, 263-263, 280-280, 314-314
website/docs/destinations/server/gcp.mdx (3)
28-46: Code example correctly demonstrates the new startFlow-based configuration.The example properly shows the migrated API pattern with the nested destinations object, correct imports, and required settings fields. The structure aligns with the architecture described in the documentation.
52-52: Heading capitalization is consistently applied.All section headings follow sentence-style capitalization, improving consistency with modern documentation conventions.
Also applies to: 79-79, 108-108
118-144: Custom schema mapping example is correct and well-integrated.The example properly extends the base configuration with the optional
data.mapfield, demonstrating how to define a custom schema. The corresponding SQL table example (lines 149-154) correctly matches the mapped schema.website/docs/apps/cli.mdx (4)
40-40: Heading normalization looks good.All section headings are now consistently capitalized using title case. The documentation structure is clear and follows a logical progression from installation through troubleshooting.
Also applies to: 51-51, 61-61, 145-145, 185-185, 321-321, 468-468, 662-662, 825-825, 951-951, 1090-1090, 1179-1179, 1197-1197, 1233-1233, 1320-1320
150-181: Code examples are clear and syntactically sound.JSON configurations, shell commands, HTML snippets, and CI/CD examples are representative and follow best practices. Progressive complexity from basic to advanced use cases aids learning.
Also applies to: 214-252, 349-386, 496-521, 689-732, 852-891, 963-994, 1001-1037, 21-24, 33-36, 201-203, 257-259, 338-340, 738-782, 916-928, 1062-1078, 1238-1262, 1266-1276
1-1326: Documentation is comprehensive and user-focused.The CLI documentation covers all major commands, use cases, and integration patterns. Internal implementation changes (structured logging, startup banner) are not user-facing features requiring documentation updates here.
1320-1326: Next steps references are appropriately scoped.Links to flow configuration, docker deployment, sources, destinations, mapping, and tutorials follow expected documentation hierarchy. Consider running an automated link checker in CI to catch broken refs.
website/docs/destinations/web/api.mdx (1)
204-209: Beacon transport is properly implemented and documented.The beacon transport is correctly defined in the codebase as a valid SendWebTransport type ('fetch' | 'beacon' | 'xhr') in packages/web/core/src/sendWeb.ts, integrated into the API destination schema, and fully functional. The documentation at lines 204–209 is accurate.
website/docs/destinations/create-your-own.mdx (1)
140-157: Fix incorrect property name in destination configuration example.The example at lines 140-157 uses
destination: destinationWebhook, but the correct property name iscode. The type definition in@walkeros/coreand all examples in api.mdx usecode. Change line 147 fromdestination: destinationWebhook,tocode: destinationWebhook,.website/docs/destinations/index.mdx (1)
41-46: No action needed—Code destination documentation is complete.The
website/docs/destinations/code.mdxfile exists in this PR with comprehensive documentation, including setup instructions (lines 14–35), a complete configuration reference with settings and mapping tables (lines 37–56), detailed context variables for each code type (lines 57–97), and usage examples (line 98+). The sidebar position is correctly set to0, ensuring it appears first among destinations. The link inindex.mdxis valid and properly supported.website/docs/mapping.mdx (2)
1-516: Documentation enhancement is comprehensive and well-aligned with learnings.The expansion effectively demonstrates:
- Mapping patterns: Basic matches, wildcards, conditionals, and ignore rules (aligns with learnings on flexible patterns)
- Event format: Consistent use of space-separated ENTITY ACTION format (e.g., "product view", "order complete") as per learnings
- Core function usage: Proper integration of
getMappingEvent()andgetMappingValue()from@walkeros/core- Consent handling: Clear examples of consent-based mapping (e.g., lines 363–382) matching learnings on consent configuration
- Architecture context: Examples show Source → Destination flow which aligns with learnings on the three-layer architecture
- Practical use cases: Both source (normalization/filtering) and destination (transformation) scenarios are covered
The organization is clear, examples are practical, and the progression from basic to advanced is appropriate for documentation.
7-8: The component imports resolve correctly. ThecodeBox.tsxfile exists atwebsite/src/components/molecules/codeBox.tsx, andparseInputis properly imported and used in thelogEventandlogValuehelper functions. BothgetMappingEventandgetMappingValuefrom@walkeros/coreare correctly referenced in the import statement at line 7.website/docs/getting-started/flow.mdx (2)
119-124: No action needed—the package names are correct.Both
@walkeros/web-source-browserand@walkeros/web-destination-apiare valid npm package names that exist in the monorepo atpackages/web/sources/browserandpackages/web/destinations/apirespectively. The naming convention with the "web-" prefix is consistent throughout the codebase and documentation.
25-66: The complete documentation covers both web and server flows; the snippet shown is only a subset.The file flow.mdx contains a "Complete example" section (line 461) that demonstrates a production-ready server-side flow using Express source and BigQuery destination. Server-side examples are also documented in website/docs/apps/cli.mdx, docker.mdx, and website/docs/destinations/server/, showing active support across AWS and GCP platforms. The lines 25-66 snippet reflects the web-side configuration, but the full file maintains both Source → Collector → Destination patterns for web and server contexts as intended.
Likely an incorrect or invalid review comment.
website/docs/getting-started/quickstart/index.mdx (1)
189-191: Verify walkeros.io URLs are live before merging; current web searches return elbwalker.com paths instead.The updated URLs at www.walkeros.io/docs/sources/web/browser/tagging and www.walkeros.io/docs/mapping may not be live yet. Web searches for these paths returned documentation at elbwalker.com instead, suggesting the domain migration may be incomplete. Confirm these new URLs resolve correctly and contain the expected content before the documentation goes live.
website/docs/apps/docker.mdx (1)
861-865: All Next steps links in lines 861-865 are valid and correctly formatted:
/docs/apps/cli→ website/docs/apps/cli.mdx ✓/docs/getting-started/flow→ website/docs/getting-started/flow.mdx ✓/docs/sources/→ website/docs/sources/index.mdx ✓/docs/destinations/→ website/docs/destinations/index.mdx ✓https://hub.docker.com/r/walkeros/docker→ valid external URL ✓No issues found.
website/docs/sources/web/browser/index.mdx (2)
94-116: Enhanced elb section looks consistent and accurateHeading casing and description match the current
elbbehavior and the commands docs; the example clearly distinguishes collector vs browserelb. No issues from my side.
117-121: New “Tagger Tool” link is correctly wiredAdding the Tagger Tool entry under “Next steps” aligns with the new
sources/web/browser/taggerdoc and improves discoverability without breaking structure.website/docs/sources/server/index.mdx (1)
1-37: Server sources overview is clear and well-scopedThe new index cleanly summarizes Express/Fetch/AWS/GCP server sources, how they forward HTTP events to the collector, and shared capabilities (JSON, pixel, CORS, health). It fits well with the Source → Collector → Destination model without over-specifying implementation details.
website/sidebars.ts (1)
131-134: Session guide entry correctly added to GuidesLinking
guides/sessionhere makes the new session guide discoverable alongside consent, user-stitching, and migration docs. No sidebar issues spotted.website/docs/sources/server/express.mdx (1)
30-82: Express source quick start and app-only examples read correctlyStandalone and app-only examples are consistent with the config table:
portcontrols auto-listen vs app-only mode,pathcustomizes the endpoint, and the documented HTTP routes (/collect,/health,/ready) match the comments. Exposingcollector.sources.express.appfor custom middleware is clearly explained.website/docs/sources/index.mdx (2)
22-45: Web sources section is well-structured and matches detailed docsThe new “Web → Browser / dataLayer” breakdown clearly differentiates DOM-based tracking from dataLayer integration, and the Learn more links correctly point to
./web/browser/and./web/dataLayer/. This aligns nicely with the standardized event flow into the collector.
Based on learnings, this reflects the expected Source → Collector → Destination pattern.
46-89: Server sources summary aligns with dedicated server docsThe Server section concisely explains when to use Express, Fetch, AWS Lambda, and GCP Cloud Functions, and the bullet points match the capabilities described in their individual pages (edge Fetch handler, API Gateway variants, pixel tracking, CORS, health checks). The relative links (
./server/*) are consistent with the new server docs and sidebar entries.
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (1)
skills/writing-documentation/SKILL.md (1)
132-132: Capitalize "Markdown" as a proper noun.Line 132: Change "markdown tables" to "Markdown tables" to match style guide conventions.
-- 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 +- Use PropertyTable instead of hardcoded Markdown tables
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.claude/settings.local.json(0 hunks)skills/writing-documentation/SKILL.md(1 hunks)
💤 Files with no reviewable changes (1)
- .claude/settings.local.json
🧰 Additional context used
🧠 Learnings (2)
📚 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.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.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
60-60: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
236-236: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
259-259: Multiple headings with the same content
(MD024, no-duplicate-heading)
263-263: Multiple headings with the same content
(MD024, no-duplicate-heading)
281-281: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
307-307: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
316-316: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
322-322: Bare URL used
(MD034, no-bare-urls)
324-324: 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
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
website/docs/guides/session.mdx (1)
356-357: Lowercase "data" for consistency (unresolved from previous review).This was flagged in the previous review but remains unfixed. For consistency with the guide's editorial style, lowercase the second reference to "data":
-Domains can be extended, e.g., internal sub-domains. Data can be pre-defined, +Domains can be extended, e.g., internal sub-domains. data can be pre-defined,skills/writing-documentation/SKILL.md (1)
132-132: Capitalize "Markdown" as a proper noun.Line 132 uses lowercase "markdown" but the correct form for the formatting language is "Markdown".
-Use PropertyTable instead of hardcoded markdown tables +Use PropertyTable instead of hardcoded Markdown tables
🧹 Nitpick comments (3)
website/docs/guides/session.mdx (1)
121-122: Use "whether" instead of "if" for formal style.The static analysis tool suggests a minor style improvement for consistency:
-Setting a consent state to wait for before detecting a new session is used to -decide if storage access is allowed or not. +Setting a consent state to wait for before detecting a new session is used to +decide whether storage access is allowed or not.website/docs/destinations/server/datamanager.mdx (2)
44-47: Clarify the purpose of operatingAccount fields.The
operatingAccountobject structure is shown but not explained. Add brief documentation for:
accountId: Purpose and format (is it always numeric? does format vary by accountType?)accountType: Enum values and when each is usedThis will help users understand whether they need one or multiple operating accounts and how to structure their configuration.
84-94: Clarify consent mapping semantics in the documentation.The comment on line 94 mentions "string (for mapping to user consent field) or static values (boolean)" but this distinction isn't explained before the code example. Users seeing
consentAdUserData: 'marketing'may not immediately understand that the string maps to a field in user context.Consider expanding the explanation to show both patterns more explicitly:
// Map to user consent context field consentAdUserData: 'user.consents.marketing', // Or use static value consentAdUserData: true,This clarifies when to use strings vs booleans and improves discoverability of the feature.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
README.md(2 hunks)apps/demos/destination/README.md(1 hunks)apps/demos/source/README.md(1 hunks)apps/demos/storybook/README.md(1 hunks)apps/storybook-addon/README.md(1 hunks)apps/storybook-addon/package.json(1 hunks)apps/walkerjs/README.md(5 hunks)packages/core/src/__tests__/eventGenerator.test.ts(1 hunks)packages/core/src/__tests__/getMarketingParameters.test.ts(1 hunks)packages/core/src/eventGenerator.ts(1 hunks)packages/docker/docker-compose.bundle.yml(0 hunks)packages/docker/docker-compose.collect.yml(0 hunks)packages/docker/docker-compose.serve.yml(0 hunks)packages/web/core/src/__tests__/sendWeb.test.ts(1 hunks)packages/web/core/src/__tests__/sessionStorage.test.ts(1 hunks)packages/web/core/src/__tests__/sessionWindow.test.ts(2 hunks)packages/web/core/src/__tests__/storage.test.ts(1 hunks)packages/web/destinations/plausible/src/index.test.ts(1 hunks)skills/mapping-configuration/SKILL.md(1 hunks)skills/understanding-mapping/SKILL.md(1 hunks)skills/writing-documentation/SKILL.md(1 hunks)website/README.md(1 hunks)website/docs/destinations/server/datamanager.mdx(2 hunks)website/docs/guides/session.mdx(1 hunks)
💤 Files with no reviewable changes (3)
- packages/docker/docker-compose.bundle.yml
- packages/docker/docker-compose.serve.yml
- packages/docker/docker-compose.collect.yml
✅ Files skipped from review due to trivial changes (7)
- apps/demos/destination/README.md
- apps/demos/storybook/README.md
- apps/storybook-addon/package.json
- apps/walkerjs/README.md
- README.md
- apps/demos/source/README.md
- apps/storybook-addon/README.md
🧰 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/web/core/src/__tests__/sendWeb.test.tspackages/core/src/eventGenerator.tspackages/core/src/__tests__/eventGenerator.test.tspackages/web/destinations/plausible/src/index.test.tspackages/web/core/src/__tests__/storage.test.tspackages/web/core/src/__tests__/sessionWindow.test.tspackages/web/core/src/__tests__/sessionStorage.test.tspackages/core/src/__tests__/getMarketingParameters.test.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/web/core/src/__tests__/sendWeb.test.tspackages/core/src/__tests__/eventGenerator.test.tspackages/web/destinations/plausible/src/index.test.tspackages/web/core/src/__tests__/storage.test.tspackages/web/core/src/__tests__/sessionWindow.test.tspackages/web/core/src/__tests__/sessionStorage.test.tspackages/core/src/__tests__/getMarketingParameters.test.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/eventGenerator.tspackages/core/src/__tests__/eventGenerator.test.tspackages/core/src/__tests__/getMarketingParameters.test.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: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/web/core/src/__tests__/sendWeb.test.tspackages/web/core/src/__tests__/sessionWindow.test.tspackages/core/src/__tests__/getMarketingParameters.test.tsskills/writing-documentation/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} : 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/core/src/__tests__/sendWeb.test.tspackages/core/src/__tests__/eventGenerator.test.tspackages/web/destinations/plausible/src/index.test.tspackages/core/src/__tests__/getMarketingParameters.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/web/core/src/__tests__/sendWeb.test.tspackages/core/src/__tests__/eventGenerator.test.tspackages/web/destinations/plausible/src/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 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.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} : 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/mapping-configuration/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/mapping-configuration/SKILL.mdskills/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: 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-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: 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-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 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-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: 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/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: 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
🧬 Code graph analysis (2)
packages/web/core/src/__tests__/storage.test.ts (1)
packages/web/sources/browser/src/examples/env.ts (1)
document(120-122)
packages/web/core/src/__tests__/sessionWindow.test.ts (1)
packages/web/core/src/session/sessionWindow.ts (1)
sessionWindow(17-77)
🪛 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)
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
236-236: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
259-259: Multiple headings with the same content
(MD024, no-duplicate-heading)
263-263: Multiple headings with the same content
(MD024, no-duplicate-heading)
348-348: Multiple headings with the same content
(MD024, no-duplicate-heading)
352-352: 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 (14)
packages/web/core/src/__tests__/sessionStorage.test.ts (1)
229-272: Domain change in URL keeps test behavior unchangedSwitching the test URL to
https://www.walkeros.io/?utm_campaign=newis consistent with the broader domain migration and does not alter the test’s behavior, since only theutm_campaignquery parameter drives the session/campaign expectations here. Looks good as-is.packages/web/core/src/__tests__/storage.test.ts (1)
36-37: LGTM! Domain refactor correctly applied to test.The test data and assertions have been consistently updated from
elbwalker.comtowalkeros.io. The test still properly validates that the domain parameter is correctly written to cookies.website/docs/destinations/server/datamanager.mdx (1)
27-57: Document operatingAccount.accountType and provide examples for all supported systems.The introduction mentions three Google systems (Ads, DV360, GA4), but only one
accountTypeexample is shown. Users need to understand:
- What values are valid for
operatingAccount.accountType- How configuration differs per account type
- Which settings apply globally vs per-destination
Consider adding examples or a table showing valid accountType values and their purposes. For instance:
- How should DV360 or GA4 be configured differently?
- Are there differences in required/optional fields per account type?
You can verify the implementation by checking the package source or schema file at the referenced import path (line 8).
packages/web/core/src/__tests__/sessionWindow.test.ts (1)
5-5: LGTM! Domain references updated consistently.The test data has been updated from the old domain to
walkeros.ioconsistently across all test cases. The test logic and assertions remain unchanged, which is appropriate for a domain rebranding update.Also applies to: 70-72, 77-77
packages/web/destinations/plausible/src/index.test.ts (1)
103-138: Domain update in Plausible init test looks correctSwitching the
domaintest fixture towalkeros.iokeeps this test aligned with the new branding and doesn’t affect behavior; mocking and env wiring remain intact.packages/web/core/src/__tests__/sendWeb.test.ts (1)
6-6: sendWeb tests correctly target new API base URLUpdating the shared
urlconstant tohttps://api.walkeros.io/keeps all transport-mode tests in sync with the new endpoint without changing test semantics.website/README.md (1)
3-6: README branding and URL update is consistentThe README now correctly points to
www.walkeros.io, and the Docusaurus sentence remains clear despite the line wrap.packages/core/src/eventGenerator.ts (1)
191-200: Default page-view referrer updated cleanlyChanging the default
'page view'referrer tohttps://www.walkeros.io/removes the legacy domain reference while keeping the sample event structure intact.packages/core/src/__tests__/eventGenerator.test.ts (1)
54-69: Test expectation now matches updated default referrerThe
getEvent('page view')expectation correctly mirrors the newhttps://www.walkeros.io/referrer fromeventGenerator.ts; the rest of the event shape is unchanged.packages/core/src/__tests__/getMarketingParameters.test.ts (1)
4-6: Marketing parameters test input updated to new domainUsing
https://www.walkeros.io/?as the base URL keeps the test data consistent with the new site domain while leaving the parameter parsing behavior and assertions unchanged.skills/understanding-mapping/SKILL.md (1)
38-54: Mapping structure is clear and follows best practices.The event mapping pattern correctly demonstrates exact matches, wildcards, and entity-level patterns without duplication. All examples follow the "entity action" format and reference appropriate core functions (
getMappingEvent,getMappingValue).skills/mapping-configuration/SKILL.md (1)
240-259: Source-side mapping example is now correctly structured.The top-level
name,data, andusermappings now properly transform HTTP input to walkerOS event structure. Previous nesting issue has been resolved.skills/writing-documentation/SKILL.md (2)
240-281: Markdown Template code fence looks malformed.The Website Doc Template block opens at line 240 with
```mdxbut the example content (with triple backticks for the JSON example) creates ambiguous nesting. Verify the fence structure is correct and unambiguous for markdown parsers.Can you confirm the template is rendering correctly and that no inner code block accidentally closes the outer
```mdxfence? This may require adjusting backtick counts or escaping.
172-236: Add language specifier to Package README Template closing fence.The opening fence (line 172) uses
```markdownbut the closing fence (line 236) lacks a language specifier. For consistency, add the language specifier to the closing fence.... -```` +````markdown⛔ 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 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 mappingLearnt 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
| ```mdx | ||
| import { schemas } from '@walkeros/web-destination-gtag/dev'; | ||
| <PropertyTable schema={schemas.settings} />; | ||
|
|
||
| ; | ||
| ``` |
There was a problem hiding this comment.
Remove extraneous semicolon from MDX example.
Line 107 has a semicolon after the JSX closing tag, and line 109 has another semicolon. In MDX, the JSX component should not have semicolons.
```mdx
import { schemas } from '@walkeros/web-destination-gtag/dev';
-<PropertyTable schema={schemas.settings} />;
+<PropertyTable schema={schemas.settings} />
-;
<details>
<summary>🤖 Prompt for AI Agents</summary>
In skills/writing-documentation/SKILL.md around lines 105 to 110, the MDX
example includes extraneous semicolons after the import line and after the JSX
closing tag; remove the trailing semicolon after the import statement (if
present in the MDX block) and remove the semicolon immediately following the
<PropertyTable ... /> JSX closing tag so the MDX snippet contains a plain import
and a JSX component without semicolons.
</details>
<!-- fingerprinting:phantom:triton:mongoose -->
<!-- This is an auto-generated comment by CodeRabbit -->
Summary by CodeRabbit
New Features
Documentation
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.