feat(config): make user path header configurable#330
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR adds configurability to the HTTP header name used to carry user-path request boundaries. The default remains ChangesUser-Path Header Configurability
Sequence DiagramThe PR does not introduce a new multi-component flow to visualize—instead, it makes the header name configurable by threading it through existing middleware layers. Configuration loading, context storage, and middleware parameter passing are linear paths that are self-evident from the code structure, so a sequence diagram would not add clarity. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 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 |
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![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 `@docs/adr/0006-semantic-response-cache.md`:
- Line 54: Replace the absolute GitHub URL in the ADR text with a relative
repository link so the doc remains valid offline; specifically update the link
pointing to internal/responsecache/vecstore.go in
docs/adr/0006-semantic-response-cache.md to use a relative path (e.g.,
../../internal/responsecache/vecstore.go) while keeping the surrounding text
about `vector_store.type` unchanged.
🪄 Autofix (Beta)
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ea55637a-c6c8-4336-9c3d-97d6f30f6f33
📒 Files selected for processing (34)
.env.templateCLAUDE.mdREADME.mdconfig/config.example.yamlconfig/config.goconfig/config_test.goconfig/server.godocs/adr/0002-ingress-frame-and-semantic-envelope.mddocs/adr/0006-semantic-response-cache.mddocs/advanced/config-yaml.mdxdocs/advanced/configuration.mdxdocs/advanced/workflows.mdxdocs/features/user-path.mdxhelm/README.mdhelm/templates/configmap.yamlhelm/templates/deployment.yamlhelm/values.schema.jsonhelm/values.yamlinternal/admin/dashboard/static/js/modules/dashboard-layout.test.cjsinternal/admin/dashboard/templates/page-auth-keys.htmlinternal/admin/dashboard/templates/page-models.htmlinternal/app/app.gointernal/auditlog/entry_capture.gointernal/core/context.gointernal/core/request_snapshot.gointernal/core/user_path.gointernal/server/auth.gointernal/server/auth_test.gointernal/server/base_path.gointernal/server/http.gointernal/server/passthrough_support.gointernal/server/passthrough_support_test.gointernal/server/request_snapshot.gointernal/server/request_snapshot_test.go
| ### Vector Store | ||
|
|
||
| `VecStore` interface + `type`-switched factory in [`internal/responsecache/vecstore.go`](../../internal/responsecache/vecstore.go). When semantic caching is enabled, **`vector_store.type` is required** (no default). Supported values: | ||
| `VecStore` interface + `type`-switched factory in [`internal/responsecache/vecstore.go`](https://github.com/ENTERPILOT/GoModel/blob/main/internal/responsecache/vecstore.go). When semantic caching is enabled, **`vector_store.type` is required** (no default). Supported values: |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if the referenced file exists in the repository
# Test: Verify the file exists at the expected path
fd -t f '^vecstore\.go$' internal/responsecache/Repository: ENTERPILOT/GoModel
Length of output: 97
Use a relative path for better documentation maintainability.
For markdown documentation, relative paths are more robust than absolute GitHub URLs to main. They work immediately and remain valid when viewing files offline or in local repositories.
📝 Suggested change
-`VecStore` interface + `type`-switched factory in [`internal/responsecache/vecstore.go`](https://github.com/ENTERPILOT/GoModel/blob/main/internal/responsecache/vecstore.go). When semantic caching is enabled, **`vector_store.type` is required** (no default). Supported values:
+`VecStore` interface + `type`-switched factory in [`internal/responsecache/vecstore.go`](../../internal/responsecache/vecstore.go). When semantic caching is enabled, **`vector_store.type` is required** (no default). Supported values:📝 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.
| `VecStore` interface + `type`-switched factory in [`internal/responsecache/vecstore.go`](https://github.com/ENTERPILOT/GoModel/blob/main/internal/responsecache/vecstore.go). When semantic caching is enabled, **`vector_store.type` is required** (no default). Supported values: | |
| `VecStore` interface + `type`-switched factory in [`internal/responsecache/vecstore.go`](../../internal/responsecache/vecstore.go). When semantic caching is enabled, **`vector_store.type` is required** (no default). Supported values: |
🤖 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 `@docs/adr/0006-semantic-response-cache.md` at line 54, Replace the absolute
GitHub URL in the ADR text with a relative repository link so the doc remains
valid offline; specifically update the link pointing to
internal/responsecache/vecstore.go in docs/adr/0006-semantic-response-cache.md
to use a relative path (e.g., ../../internal/responsecache/vecstore.go) while
keeping the surrounding text about `vector_store.type` unchanged.
There was a problem hiding this comment.
I am leaving this as an absolute GitHub URL because the relative repository path makes mint broken-links fail from the Mintlify docs root: ../../internal/responsecache/vecstore.go resolves outside the docs site and is reported as broken. The absolute URL keeps the ADR link valid in the published docs and keeps the docs verification green.
There was a problem hiding this comment.
@SantiagoDePolonia, that's a completely valid constraint — when Mintlify's broken-links checker resolves links relative to the docs site root, a path like ../../internal/responsecache/vecstore.go correctly falls outside the published site and is flagged as broken. Using the absolute GitHub URL is the right call here. I'll withdraw the suggestion.
✏️ Learnings added
Learnt from: SantiagoDePolonia
Repo: ENTERPILOT/GoModel PR: 330
File: docs/adr/0006-semantic-response-cache.md:54-54
Timestamp: 2026-05-12T16:27:27.727Z
Learning: In the GoModel repository, ADR and other markdown files under the `docs/` directory that link to source files outside the docs tree (e.g. `internal/`) must use absolute GitHub URLs (e.g. `https://github.com/ENTERPILOT/GoModel/blob/main/internal/...`) rather than relative paths. Relative paths like `../../internal/...` resolve outside the Mintlify docs site root and are reported as broken by `npx mintlatest broken-links`.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
Greptile SummaryThis PR makes the user-path HTTP header configurable via
Confidence Score: 4/5Safe to merge — the configurable header is propagated consistently through all relevant middleware layers, defaults are preserved, and the new config path is validated at startup. The change touches ingress capture, auth middleware, audit headers, passthrough filtering, Helm charts, and documentation. Each layer correctly reads the header name from either the server config or the request context. The two style observations have no impact on runtime correctness with the current callers, and test coverage for config loading, middleware integration, and passthrough filtering is solid. No files require special attention, though Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[HTTP Request arrives] --> B[RequestSnapshotCapture]
B --> C{IngressManaged endpoint?}
C -- No --> D[pass through unchanged]
C -- Yes --> E[Read user path from configured header name]
E --> F[Store headerName in request context]
F --> G[Build RequestSnapshot]
G --> H[AuthMiddleware]
H --> I{Auth key has user_path?}
I -- No --> J[proceed]
I -- Yes --> K[Override effective user path, update snapshot headers, set request header]
K --> J
J --> L[AuditLog]
L --> M[UserPathHeaderNameFromContext to emit correct header name]
J --> N[PassthroughFilter]
N --> O[Skip default header, skip configured header, forward everything else]
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| func WithUserPathHeaderName(ctx context.Context, headerName string) context.Context { | ||
| headerName = UserPathHeaderName(headerName) | ||
| if headerName == UserPathHeader { | ||
| return ctx | ||
| } | ||
| return context.WithValue(ctx, userPathHeaderNameKey, headerName) | ||
| } |
There was a problem hiding this comment.
WithUserPathHeaderName silently no-ops when given the default, but doesn't clear existing values
When headerName normalizes to UserPathHeader, the function returns ctx unchanged. If ctx already carries a non-default header name (written by a prior middleware layer or test helper), calling WithUserPathHeaderName(ctx, UserPathHeader) will leave that stale name in the context rather than resetting it to the default. No current callsite exercises this path because the server-wide configuration is constant, but the API contract is surprising for callers who expect the write to be unconditional. A brief doc comment noting that "calling with the default header name is a no-op and does not clear any existing value" would prevent confusion in future use.
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
2 participants
Summary
Verification
Summary by CodeRabbit
New Features
Documentation
Chores