Repository navigation
Support custom Installation profiles and readable Preset starters - #2253
miguelbranco80 wants to merge 23 commits into
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
clawsweeper
Bot
added
P2
|
Codex review: blocked before merge. What this changesAdds trusted Installation profile files, optional bundled Preset seeding, and named quick-start from another readable Preset. Example: A Namespace contains Alpha starter and Zulu starter but no default-codex.
Review scores
ProductKind: Feature · Worth it: Needs a maintainer decision · Fix scope: Complete Merge readiness⛔ Blocked before merge - 3 items remain Keep this PR open for maintainer product acceptance; the implementation is focused, with one small test-policy correction and no identified runtime defect. Priority: P2 Decision needed
Before merge
Findings
Agent review detailsHow this fits togetherThe renderer converts trusted operator inputs into Helm and Installation configuration; the Console reads authorized Namespace Presets and applies their templates to the regular Agent form. flowchart TB
A[Operator profile and site inputs] --> B[Profile renderer]
B --> C[Helm and Installation configuration]
C --> D[Controller startup and Preset seeding]
D --> E[Namespace Presets]
E --> F[Authorized Console selection]
F --> G[Editable Agent form]
Technical reviewBest possible solution: Accept or decline the bounded customization contract while keeping rendering, seeding and exact-resource authorization with their existing owners. Do we have a high-confidence way to reproduce the issue? Current main directly establishes the bundled-profile restriction, forced default seeding and default-codex-only shortcut; no target code was executed during this review. Is this the best way to solve the issue? The implementation extends existing owners without new platform machinery, preserves stock defaults and exact-resource reads, and clearly documents startup validation and cross-family DNS limits. Full review comments:
Overall correctness: patch is correct AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 7eea9145e14a. Provenance checked
TestingProof path: shipped entry point. SecurityNone. EvidenceWhat I checked:
Likely related people:
Root-cause clusterRelationship: Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. LabelsLabel changes:
Label justifications:
Rating scale6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior. WorkflowClawSweeper edits this one comment on every review. Comment HistoryReview history (4 earlier review cycles)
Reviewed October 24, 2026, 11:17 AM ET / 15:17 UTC (Revision 5). |
|
Addressed the P2 diagnostic finding at Final runtime source passes 118 integration/conformance cases and 32 Console browser cases, with no skips. Updated Verification contains three native screenshots, an uncut 5.64-second browser video, actual renderer CLI output for hosted/curated custom profiles with disabled stock seeding, and the existing startup/saved-Preset preservation run. Media opens; the uploaded video plays through to the editable draft. The Console captures use the real Fastify test app with native IAM/OCC, in-memory state and substituted runtime dependencies; the limits are stated explicitly. Two owner questions remain:
No merge or auto-merge is requested. Upstream CI still needs its applicable maintainer workflow approval. @clawsweeper re-review |
|
🦞👀 Re-review progress:
|
clawsweeper
Bot
added
rating: 🦐 gold shrimp
|
Updated the shared branch to The requested PostgreSQL before/after scenario now passes from pinned before revision Verification contains the native PostgreSQL output/procedure, refreshed screenshots and an uncut 5.72-second Console video from the final head. All 119 focused profile/Preset cases and 32 Console browser cases pass without skips. Proof limits are explicit: Compute/unused Secret fixtures do not qualify Kubernetes or model execution. Maintainer product acceptance and applicable upstream CI remain pending. @clawsweeper re-review |
|
🦞👀 Re-review progress:
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| password: (await readFile(join(directory, "initial-admin-password"), "utf8")).trim(), | ||
| }); | ||
| async function request(method, path, payload, expectedStatus = 200) { | ||
| const response = await fetch(`${endpoint}${path}`, { |
| JSON.stringify({ | ||
| installation, | ||
| namespaceId: namespace.id, | ||
| edited, | ||
| custom, | ||
| savedConfiguration, | ||
| }), |
clawsweeper
Bot
added
proof: sufficient
|
Free's clanker here (an AI agent working on Free's behalf). Review-only pass at 775a7cf. The renderer and Console changes read correctly to me. Locally The shortcut still calls itself "default" after it stops meaning For the record: merge-tree is clean against main and against #2320, #2322, #2329 and #2330 (the other open renderer refusals); the only textual overlap is |
|
Free's clanker here (an AI agent working on Free's behalf). One more review pass on head What it changes
Issues
Compatibility with the pending Credential Proxy work: one trivial conflict in the @freeqaz-openai flagging this one for your review. |
|
Free's clanker here (an AI agent working on Free's behalf). Validation request on head What it does
Since our last pass ( What to validate
Still open from earlier reviews (none blocking)
CI and conflicts: @freeqaz-openai flagging this one for your validation. |
|
@clawsweeper re-review The last exact-head attempt stopped at the input scanner. Local reproduction with TruffleHog 3.97.4 identified the same synthetic test URL in the old base blob and removal patch. Both current endpoints now construct that URL at runtime. Scanning all 39 unique base/head blobs across this PR's 21 changed files, plus raw diff and patch, returned zero findings; historical positive controls reproduced. Credential verification was disabled. This is source-input evidence, not a claim that the complete hosted gate passed. No scanner policy or suppression changed. The previously reported live-DNS test issue is also fixed with the existing local DNS fixture. Required CI passes on this head. Product acceptance, both unresolved CodeQL discussions, and existing visual/runtime qualification limits remain outstanding; this requests review, not merge. |
|
🦞👀 Re-review progress:
|
clawsweeper
Bot
added
proof: 📸 screenshot
Problem and change
Fixes #2252. Operators need trusted Installation profile customization and a usable quick-start when a Namespace has only custom Presets.
--profile-filewith closed validation, matching profile family and supported bundled PluginDriver selection. Invalid input fails without fallback and revokes stale output; startup validates Driver options.presets.includeDefaults(defaulttrue). Disabling it preserves saved Presets and seeds only explicitly listed files.default-codex, otherwise the first readable Preset by case-insensitive browser locale order with exact-name tie-breaking. Name custom starters before selection and authorize the exact resource read.Both PRs share
raw-labs:codex/installation-customization: upstream #2253 and fork #11. The fork comparison snapshot follows the merged upstream revision. Both remain open and ready.Verification
Head
5acf92b89f9053fec747a802e3790822a5b4340dmerges upstreammainpinned at83916ce36a51d121c59166152058be5857d22f81without rewriting published history. The published tree matches the locally validated and independently reviewed candidate.ec30f54cf4d8fc6aa4273e09729b291248af96d3of this head into newer upstreambb5b58f7566c2935a09382e6936eef6f0e6c23fa, not the local pinned base. PostgreSQL Platform passed, including the Preset-preservation test (current-version restart proof); Console Browser 2, CodeQL analysis and Storybook build passed. Nonblocking First Agent Smoke was still running at required-gate completion. Native container qualification and advisory QA were skipped.Historical proof: at
68c7a1bfca09aa6939373c15658506f0c20acb06, two real PostgreSQL preservation runs passed without skips: current-version restart and historicalb195e54e9upgrade, preserving custom/edited Presets and saved Configuration across fresh processes with separate migrator/application roles. Owned resources were removed; see the procedure. Database, Kubernetes and model qualification were not rerun for this refresh. The earlier local TLS-fixture failures at7eefd21048f3b3886cb2fca1d67177ffa930b869remain historical evidence, not current-head passes or failures. The existing local DNS fixture still isolates hosted custom-profile rendering from public DNS.Console evidence
Preserved captures at
68c7a1bfca09aa6939373c15658506f0c20acb06cover Pages/Create Agent / CreateCustomPresetStarter and CreatePresetNameTie. Storybook simulates API/runtime responses. The native captures use production Fastify Console/API, native IAM/OCC, in-memory storage, a test-provisioned ready Namespace and substituted runtime dependencies, without intercepted Preset responses: Node 24.3.0, headless Chromium, 1440×1080. This refresh preserves starter behavior through upstream's typed migration. The current Storybook build and browser checks passed; media was not regenerated and does not claim a new visual acceptance walkthrough.Continuous 8.52-second walkthrough: custom shortcut → variables → editable draft → leave/return with edits retained → confirmed Cancel → fresh chooser. All three images load and the video decodes in GitHub preview.
console-walkthrough.mp4
PostgreSQL proof uses real production composition, authentication, native IAM/OCC, persistence and the same filesystem Configuration directory across processes. Compute and unused Secret storage are test Drivers. These checks do not qualify images, Kubernetes or model execution.
Remaining gates
Maintainer product acceptance and final-head upstream CI remain required. Existing CodeQL fixture alerts remain for maintainer triage. Hosted scanner qualification remains pending after the earlier input-safety hold. A new exact-head review was requested after source-input diagnosis and required CI passed; no scanner policy or suppression changed.
Both PRs remain open and non-draft on the same fork head. No force push, PR merge or auto-merge; fork
mainandcodex/raw-integrationare unchanged.