Conversation
'Model agnostic' meant any provider, not just any Claude model. The optional live-preview engine now takes a provider toggle (Claude / OpenAI / OpenRouter) plus a free-text model id: - Anthropic via the Messages API (browser-direct header), OpenAI and OpenRouter via the OpenAI-compatible /chat/completions endpoint (both allow browser CORS; OpenRouter adds the recommended HTTP-Referer/X-Title headers). - OpenRouter is the real 'any model' option — one key, any of Claude/GPT/ Gemini/Llama/etc. The model field is free text so any current id works. - Per-provider key placeholder, default model, and hint; remembers provider + model alongside the key. Graceful fallback to the scripted demo on any error. Engine label is now 'Real AI' rather than 'Real Claude'.
- The failure toast hardcoded 'Couldn't reach Claude' even when the call went to OpenAI or OpenRouter; it now names the actual provider. - OpenAI reasoning models (o-series, gpt-5) reject max_tokens and require max_completion_tokens. Since the model field is free text, a user could legitimately type one and get a 400. Now retries once with the alternate parameter when the error mentions it, so 'any model' really means any model.
|
@codex please do a code review Cross-vendor audit request (vibeOS doc 04). Context so you can review cold: What this PR does: makes the optional "live preview" in Everything is in one file: Please pay particular attention to:
Known gaps, already acknowledged — no need to re-report unless you think they're worse than stated: no PRD/acceptance criteria in Findings will be answered individually as fixed, disputed + reason, or deferred + ticket. Generated by Claude Code |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1323b0d7ee
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| }; | ||
| function provName(){return {anthropic:"Anthropic",openai:"OpenAI",openrouter:"OpenRouter"}[provider];} | ||
| function applyProvider(){const p=PROV[provider];$("#apikey").placeholder=p.ph;$("#model").value=p.model;$("#keyhint").innerHTML=p.hint;} | ||
| $("#provider").addEventListener("click",e=>{const b=e.target.closest(".pill");if(!b)return;provider=b.dataset.prov;[...$("#provider").children].forEach(x=>x.setAttribute("aria-pressed",x===b));applyProvider();saveKey();}); |
There was a problem hiding this comment.
Clear the API key when switching providers
When a visitor switches providers after entering a key, this handler changes provider but leaves both apiKey and the password field unchanged, so clicking Wake sends the previous provider's credential to the newly selected vendor. With “remember” enabled, saveKey() also relabels and persists that stale credential under the new provider, allowing a later auto-run from a shared idea URL to disclose it without another prompt. Clear the key on provider changes or store separate keys per provider.
Useful? React with 👍 / 👎.
| if(!res.ok){ | ||
| const first=await errMsg(res); | ||
| if(/max_tokens|max_completion_tokens/i.test(first))res=await send({max_completion_tokens:3000}); | ||
| if(!res.ok)throw new Error(first); |
There was a problem hiding this comment.
Surface the retry response's error
When the initial request triggers the max_completion_tokens retry and that retry also fails, this throws first, which describes the already-corrected max_tokens problem rather than the retry's actual error (for example, an unsupported model or another invalid parameter). Consume res with errMsg() after the retry so the visitor sees the actionable failure reason.
Useful? React with 👍 / 👎.
| const url=provider==="openai"?"https://api.openai.com/v1/chat/completions":"https://openrouter.ai/api/v1/chat/completions"; | ||
| const headers={"content-type":"application/json","authorization":"Bearer "+apiKey}; | ||
| if(provider==="openrouter"){headers["HTTP-Referer"]=location.origin;headers["X-Title"]="Ghost Office";} | ||
| const msgs=[{role:"system",content:sys},{role:"user",content:user}]; |
There was a problem hiding this comment.
Use developer messages for OpenAI reasoning models
When an OpenAI user selects an o-series reasoning model that requires a developer message instead of system, both the initial request and the token-parameter retry keep sending this unsupported role, so the advertised reasoning-model path falls back to the demo even with a valid key and model. Select the prompt role for the OpenAI model family, while retaining system where required by other providers.
Useful? React with 👍 / 👎.
Cross-vendor audit (vibeOS doc 04) found three issues; all three confirmed against the code and fixed. P1 — API key survived a provider switch. Changing provider updated `provider` but left `apiKey` and the password field populated, so clicking Wake sent the previous vendor's credential to the newly selected vendor's endpoint; with 'remember' on, saveKey() then re-persisted that stale key relabelled under the new provider, so a later auto-run from a shared ?idea= URL could disclose it without a prompt. Now a provider switch clears the key, the field, and every persisted entry (same-provider clicks are a no-op). Verified by simulating a switch with a remembered key: in-memory, field, and localStorage all empty. P2 — retry masked the real error. When the max_completion_tokens retry itself failed we threw the *first* error (the already-corrected max_tokens complaint) instead of the retry's actual cause. The retry now surfaces its own error; the non-retry path still reports the original. P2 — OpenAI reasoning models need a `developer` role, not `system`. o-series / gpt-5 ids are now detected up front and sent with role 'developer' plus max_completion_tokens directly, removing a guaranteed-failing round trip. Scoped to direct OpenAI only: OpenRouter normalises roles and token params across vendors and uses prefixed ids, so it keeps the standard shape with the retry as a safety net.
|
Thanks — all three confirmed against the code and fixed in 1. P1 — Clear the API key when switching providers → FIXEDConfirmed, and this was the most serious finding: the handler set A provider switch now clears the in-memory key, the input field, and every persisted entry ( Verified by simulating a switch with a remembered key: 2. P2 — Surface the retry response's error → FIXEDConfirmed. The retry threw 3. P2 — Use developer messages for OpenAI reasoning models → FIXEDConfirmed, and it interacted with the retry: if o-series / gpt-5 ids are now detected up front and sent with Two things I'd still value a look at, since they were in the original audit pack and didn't come back as findings — a clean pass on them is useful signal:
@codex review Generated by Claude Code |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Clean pass on Nothing has changed since
@codex review Generated by Claude Code |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bd6fa7bebc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| }else throw new Error(first); | ||
| } | ||
| const data=await res.json(); | ||
| text=(((data.choices||[])[0]||{}).message||{}).content||""; |
There was a problem hiding this comment.
Validate the complete palette color before interpolation
When either newly supported OpenAI-compatible provider returns a palette value such as hsl(0 0% 0%);background-image:url(https://attacker.example/pixel), this response flows into normalizeLanding(), where hex() accepts any string beginning with hsl; esc() is only HTML escaping and does not prevent CSS declaration injection into the generated <style> block. A prompt-injected or otherwise malicious model response can therefore alter the page CSS and trigger attacker-controlled network requests whenever the preview or downloaded page is opened, so require the entire value to match a strict color grammar before rendering it.
Useful? React with 👍 / 👎.
Second cross-vendor review pass (vibeOS doc 04) found a real one on the same
commit the first pass called clean — which is the argument for the two-pass rule.
hex() validated with /^#?[0-9a-fA-F]{3,8}$|^hsl/ — the second alternative was
unanchored, so any string merely *starting* with 'hsl' passed through verbatim.
Palette values are interpolated into a <style> block and esc() only escapes HTML,
not ';' or 'url(...)', so a prompt-injected or malicious model response could
inject arbitrary CSS declarations:
hsl(0 0% 0%);background-image:url(https://attacker.example/pixel)
That fires an attacker-controlled request whenever the page renders. The iframe
is sandboxed against scripts, but CSS url() still resolves — and the *downloaded*
.html has no sandbox at all, so the artifact the user keeps was the worse vector.
Fixed by narrowing to a single shape instead of patching the hsl grammar: hex()
now whole-string-anchors a complete hex colour (3/4/6/8 digits) and rejects
everything else to the default. The scripted generator emits hex via a new
hsl2hex() helper, so dropping hsl support costs nothing. One narrow rule beats
two loose ones.
Verified: 7 injection payloads (declaration-split, url(), brace-break,
expression()) all fall back to the default; valid hex forms still accepted;
scripted palette produces correct hues.
P2 — Validate the complete palette colour before interpolation → FIXED in
|
| payload | result |
|---|---|
hsl(0 0% 0%);background-image:url(…) |
blocked |
#fff;background:url(//evil.io) |
blocked |
#faf9f7 }body{background:url(//evil.io) |
blocked |
url(https://evil.io) / expression(alert(1)) / red;behavior:url(x) / bare hsl(…) |
blocked |
Valid forms (#faf9f7, faf9f7, #FFF, #12345678, whitespace-padded) still accepted; scripted palette produces correct hues.
Running total: 4 findings, 4 fixed, 0 disputed. New commit, so the two-consecutive-clean-pass counter resets to zero — requesting a fresh pass on 7aa6343. Still open from the original audit pack and not yet reported on: stuck-UI paths — anything that can throw outside the try in run() and leave running === true or the form permanently disabled.
@codex review
Generated by Claude Code
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7aa63433c8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| catch(e){toast("Couldn't reach Claude ("+e.message+") — running the scripted demo.");logLine("⚠","Live call failed — falling back to scripted demo.","em");liveData=null;} | ||
| bubble.querySelector(".txt").textContent="Briefing the model… one moment."; | ||
| logLine("⚡",`Calling ${provName()} (${$("#model").value.trim()||PROV[provider].model})…`,"em"); | ||
| try{liveData=await callModel(lastBrief,specialists);logLine("⚡","Model responded — team is working from it.","ok");} |
There was a problem hiding this comment.
Restore the UI when post-response processing throws
When a model returns valid JSON but uses a non-string object for a requested text field, post-call normalization can still throw outside this narrow try; for example, {"title":{"toString":null,"valueOf":null}} makes String(f.title) in normalizeLanding() throw a TypeError. Because run() has already set running = true and disabled every form control, the rejected event-handler promise bypasses finish() and leaves the page permanently disabled until reload. Cover response normalization and rendering with the recovery path, and restore the UI in a finally block.
Useful? React with 👍 / 👎.
Fifth cross-vendor finding (vibeOS doc 04). A model can legally return JSON whose
string fields are objects with no primitive conversion — {toString:null,valueOf:null}
— and String() throws TypeError on those. The normalizers ran *outside* run()'s only
try (which wrapped just the fetch), so the throw rejected the click handler's promise,
finish() never ran, and the page stayed disabled with running===true until reload.
Fixed at both levels:
Root cause — a validator must never throw. Added str(), a String() that returns
rather than throwing, and used it for every field coercion in normalizeLanding() and
normalizeSec() (and in esc()). Hostile fields now degrade to empty strings while the
rest of the model's output survives, instead of discarding the whole build.
Safety net — run() is now a guard that delegates to runBuild() inside
try/catch/finally. recoverUI() (clears running, re-enables the form, removes the skip
button, hides the progress bar, dismisses the bubble) runs in the finally, so nothing
thrown anywhere in the build can strand the page. finish() is now the success tail only.
Verified: the review's exact payload plus an all-fields-poisoned response and a
poisoned brief section all normalise without throwing (palette falls back to
defaults); a simulated mid-build throw leaves running=false and the form enabled.
P2 — Restore the UI when post-response processing throws → FIXED in
|
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Clean on Worth stating why I'm not calling it done on one: earlier in this PR a clean pass on @codex review Generated by Claude Code |
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Two consecutive clean cross-vendor passes on bd51160; 5 findings, 5 fixed, 0 disputed. Recorded in product/audits/ because doc 02 makes the repo the only channel between Builder and Auditor — PR threads are forge metadata that a git clone does not carry. Includes the retroactive acceptance criteria / click-through script (VERIFY is still unmet — no browser, no egress to provider hosts from the build sandbox), the HANDOFF note, and the process lessons. The vibeOS repo was not attached to this session, so ops/tasks.md could not be updated; that remains the record and the handoff needs mirroring there.
✅ Review gate clearedTwo consecutive clean passes on
The full record — findings, dispositions, what I verified, the retroactive acceptance criteria, and the handoff — is committed at One deviation, stated rather than hidden: Not merging. AUDIT is necessary, not sufficient — doc 02 §4 (VERIFY) is still unmet: no human has walked the criteria against a running page, and the live provider paths have never touched a real endpoint (no browser and no egress to those hosts from the build sandbox). Staying a draft, which is the honest signal for "audited, not verified". The click-through script is in the audit record; items 5 and 7 are the merge-blocking ones. Thanks @codex — genuinely useful. Two of those five were security bugs my own self-review pass missed. Generated by Claude Code |
Status
Checks
|
Provider-agnostic live preview
Clarified scope from the founder: "model-agnostic" means any provider's models — OpenAI, OpenRouter, Claude — not just any Claude model. The optional ⚡ live-preview engine now supports all three.
What changed
anthropic-dangerous-direct-browser-accessheader./v1/chat/completionsendpoint (both allow browser CORS; OpenRouter gets the recommendedHTTP-Referer/X-Titleheaders).The key always stays in the visitor's browser and calls the provider directly on their dime — still no backend, no cost or abuse exposure for the repo.
Verify
node --checkpasses (single inline script); no straycallClaudereferences;callModelbranches by provider.ghost-team/index.html→ ⚡ Real AI → pick a provider, paste a key, name a model → Wake the team.Draft — give it a spin with whichever provider you use.
https://claude.ai/code/session_015PzYENi7h9p946BzDXN9so
Generated by Claude Code