fix: make package app routes and runs diagnosable - #1308
Merged
Merged
Conversation
📝 WalkthroughWalkthroughPackage apps now receive computed ChangesPackage app routing
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant packageAppHandler
participant buildPackageAppWorker
participant packageContext
participant generatedPackageApp
participant packageRuntimeRunFinish
participant finishRunRecord
packageAppHandler->>buildPackageAppWorker: servingUsername and hostedOrigin
buildPackageAppWorker->>packageContext: compute appBasePath and hostedUrl
buildPackageAppWorker->>generatedPackageApp: expose packageContext
generatedPackageApp->>packageRuntimeRunFinish: query parameter names and HTTP status
packageRuntimeRunFinish->>finishRunRecord: persist merged run metadata
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
kody-bot
marked this pull request as ready for review
August 8, 2026 05:39
Contributor
|
🔎 Preview deployed: https://kody-pr-1308.kody-a99.workers.dev Worker: Mocks:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/worker/src/package-runtime/package-app.ts`:
- Around line 642-648: Stop persisting raw query values in the app fetch
metadata created by the runtime run flow: update the request metadata near
startRuntimeRun to store only query parameter names or use the established
query-value redaction policy. In
packages/worker/src/package-runtime/package-app.node.test.ts lines 588-658, add
coverage using sensitive OAuth callback parameters and assert their values are
absent from persisted run metadata.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: df9c1711-5f96-4b4f-b6b3-69d4d90fff20
📒 Files selected for processing (9)
docs/guides/package-authoring.mddocs/use/packages.mdpackages/worker/src/app/handlers/package-app.tspackages/worker/src/app/package-app-origin.tspackages/worker/src/app/package-app-origin.workers.test.tspackages/worker/src/mcp/package-app-storage.mcp-e2e.test.tspackages/worker/src/package-runtime/package-app.node.test.tspackages/worker/src/package-runtime/package-app.tspackages/worker/src/repo/checks.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/worker/src/package-runtime/package-app.node.test.ts (1)
68-87: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid evaluating raw source text in the test helper.
collectQueryParamNamesForTestreadspackage-app.ts, rewrites its text, and executes it withnew Function. The URL is passed as an argument, so this is not request-driven code injection. The helper still depends on exact source delimiters and escape rules. It can also test a reconstructed function instead of the implementation used by the generated wrapper.Extract
collectQueryParamNamesinto a normal pure module or expose a test seam. Use the same implementation from production and the test.Also applies to: 682-689
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/worker/src/package-runtime/package-app.node.test.ts` around lines 68 - 87, Replace the source-reading and new Function logic in collectQueryParamNamesForTest with a shared testable implementation: extract collectQueryParamNames into a pure module or expose an explicit test seam, then import or invoke that same symbol from both package-app.ts and the test. Remove the source delimiter, escape-rewriting, and dynamic evaluation dependencies while preserving the helper’s existing URL-to-name-array behavior.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/worker/src/package-runtime/package-app.node.test.ts`:
- Around line 68-87: Replace the source-reading and new Function logic in
collectQueryParamNamesForTest with a shared testable implementation: extract
collectQueryParamNames into a pure module or expose an explicit test seam, then
import or invoke that same symbol from both package-app.ts and the test. Remove
the source delimiter, escape-rewriting, and dynamic evaluation dependencies
while preserving the helper’s existing URL-to-name-array behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a71e33cd-a9cb-441b-bb86-f97aa60faec3
📒 Files selected for processing (2)
packages/worker/src/package-runtime/package-app.node.test.tspackages/worker/src/package-runtime/package-app.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/worker/src/package-runtime/package-app.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
Make hosted package apps self-aware of their public mount and make routing/run failures actionable, addressing field-report findings K-02, K-03, and K-06.
Summary
hostedUrlandappBasePaththrough package apps'packageContext, derived from the current serving username, Kody id, and originTesting
npm run validatepassed after implementation and again after reviewer fixSystem changes
System recap — extends existing primitives (medium risk)
Mode: recap · Base:
main@1916ffea· Head:b4159e58Classification: extends — package app runtime context, routing diagnostics, and run-record metadata contracts change; no primitive is added.
Primitives touched
package-appspackage-runtimeapp-uirepo-sessionsmcp-serverSystem map
A hosted request is parsed by the browser-app routing layer, enters the package runtime with its public mount context, and writes richer run metadata.
Legend: green = composes (wiring only) · amber = extended by this PR · red = new primitive · gray = context (unchanged, included only when an edge crosses it).
Before / after
packageContext.hostedUrlandappBasePathNot Foundapp_fetchhistoryConductor report
Track B report: STATUS(done); PR #1308; merged commit 0407c4b; deploy result success https://github.com/kentcdodds/kody/actions/runs/31244778124; evidence: CodeRabbit privacy finding fixed, local validate green, post-fix CI/CodeRabbit/Bugbot green, production healthcheck and execute smoke check passed; remains: none.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation