Add WebBrain Cloud CI integration - #236
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 66cfb5347e
ℹ️ 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".
| const matched = calls.some((call) => ( | ||
| call.name === expected.tool | ||
| && (!expected.origin || call.args?.url_origin === expected.origin) | ||
| && (!expected.pathRoot || call.args?.url_path_root === expected.pathRoot) | ||
| && (!expected.method || String(call.args?.method || 'GET').toUpperCase() === expected.method.toUpperCase()) |
There was a problem hiding this comment.
The request rubric considers only tool_call updates, so a rejected or failed request is graded as successfully exercised without checking its corresponding tool_result. In the new signup smoke test this is especially problematic because the Mail.tm POST is blocked unless API mutations are enabled: the blocked call can still satisfy “Created a disposable Mail.tm account,” and the scenario may subsequently complete through the UI, producing a false pass. Correlate calls with successful results and, for HTTP evidence, verify the returned success/status.
Useful? React with 👍 / 👎.
|
@codex pls review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cd80970f2b
ℹ️ 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".
| for (const skillId of scenario.verify?.skills || []) { | ||
| const loaded = calls.some((call) => call.name === 'load_skill' && call.args?.skill_id === skillId); | ||
| add(`skill:${skillId}`, `Loaded ${skillId}`, 10, loaded, loaded ? skillId : 'not observed'); | ||
| } | ||
| for (const toolName of scenario.verify?.tools || []) { | ||
| const used = calls.some((call) => call.name === toolName); | ||
| add(`tool:${toolName}`, `Used ${toolName}`, 10, used, used ? 'observed' : 'not observed'); |
There was a problem hiding this comment.
When load_skill or another required tool is rejected or fails, these checks still pass solely because a matching tool_call exists. Although the updated toolRequests path now correlates calls with successful results, this fresh evidence shows the skills and tools loops still do not, so the signup smoke test can report that both skills were loaded and required instrumentation was exercised even when their results have success: false; correlate these checks with their tool_result records as well.
Useful? React with 👍 / 👎.
| mode: trace.run.mode || 'act', | ||
| final_url: trace.run.final_url || '', |
There was a problem hiding this comment.
For the sensitive Gnippets scenario, final_url is the high-entropy fixture capability URL derived from setup.app_url, but both this sanitizer and sanitizeRun retain it verbatim. If fixture deletion fails, the workflow still uploads artifacts via its if: always() step, exposing a live bearer-style URL until expiration despite the sensitive-artifact policy; omit the URL or retain only non-capability origin information.
Useful? React with 👍 / 👎.
|
@codex pls review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7885329a55
ℹ️ 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".
| jobs: | ||
| smoke: | ||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 45 |
There was a problem hiding this comment.
With the workflow running cloud-smoke at concurrency 1, the four scenarios allow 50 minutes of agent execution alone, because the Wikipedia and NYTimes scenarios each perform a preload run with the same timeout before their main run; waitForRun also adds two-minute buffers and browser provisioning and tests consume additional time. When multiple external sites are slow but still within their scenario budgets, this 45-minute job limit terminates Node before it can write the suite report or execute the browser/fixture cleanup in finally, so increase the job timeout or run scenarios concurrently.
Useful? React with 👍 / 👎.
|
@codex pls review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c53cb8c5cd
ℹ️ 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".
|
@codex pls review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 56e9b3e21b
ℹ️ 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".
|
@codex pls review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 44161f0027
ℹ️ 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".
|
@codex pls review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 38d906722a
ℹ️ 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".
| const structuredDone = doneJsonTool(opts.outputSchema); | ||
| return closeToolDefinitions(base.map(tool => (tool.function.name === 'done' ? structuredDone : tool))); |
There was a problem hiding this comment.
When strictSecretMode is enabled for the new disposable-email/OTP scenario, this structured-output branch bypasses the strict done variants below and builds done_json without passing the strict flag. Its required summary remains an unrestricted string, and both the done_json call arguments and terminal summary are published by cloud-runs.js; the generic scrubber cannot recognize a password, token, or OTP embedded in prose. Consequently, if the model includes one of the generated credentials in its summary, the value is sent to and persisted by WebBrain Cloud despite strict mode, so use a strict done_json definition and/or redact its summary before publishing.
Useful? React with 👍 / 👎.
|
@codex pls review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0cc5619ca9
ℹ️ 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".
| case 'cloud_scheduled_jobs': | ||
| return { ok: true, jobs: await scheduler.listJobs({ tabId: null }) }; |
There was a problem hiding this comment.
When the cloud bridge is enabled, the newly allowlisted cloud_scheduled_jobs action returns scheduler.listJobs({ tabId: null }), which includes every locally stored scheduled job rather than only the job IDs created by the requesting cloud run. These summaries contain target URLs, prompts/results, and clarification state, so a managed sidecar polling two CI jobs can also receive unrelated user task history; require the caller's expected IDs and filter the response to those managed jobs.
Useful? React with 👍 / 👎.
|
@codex pls review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 29d09b892d
ℹ️ 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".
| apiKey: 'test-cloud-key', | ||
| baseUrl: 'https://webbrain.example', | ||
| fetchImpl: async (url, options = {}) => { | ||
| if (url.endsWith('/scheduled-jobs')) { |
There was a problem hiding this comment.
waitForScheduledJobs requests /scheduled-jobs?job_id=task_1, so this endsWith('/scheduled-jobs') check never matches. The mock then falls through to JSON.parse(options.body) for a GET request whose body is undefined, causing npm run test:ci to fail before the smoke suite can run; match the URL pathname or the route prefix including its query string.
Useful? React with 👍 / 👎.
| result: { | ||
| ...result, | ||
| ...(Object.hasOwn(result, 'result') ? { result: redactStrictStructuredStrings(result.result) } : {}), | ||
| ...(Object.hasOwn(result, 'cloudResult') ? { cloudResult: redactStrictStructuredStrings(result.cloudResult) } : {}), |
There was a problem hiding this comment.
When strictSecretMode is enabled for any structured cloud run whose schema contains legitimate text, this replaces every string in cloudResult with the same placeholder before pushUpdate assigns it to run.result. The run can therefore report completed while API consumers receive corrupted headlines, URLs, filenames, and other non-secret output; retain the validated result for the public response and sanitize only secret-bearing trace or persistence data.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 28324ec5ef
ℹ️ 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".
| item, | ||
| into, | ||
| depth + 1, | ||
| sensitiveParent || isSensitiveCloudKey(key), |
There was a problem hiding this comment.
When a strict run calls top-frame verify_form on a prefilled credential, the result represents it as sibling fields such as {name: "password", value: "secret"}; this recursion only treats descendants of a sensitive key as sensitive, so the value is never registered. Although the immediate verify_form update is masked, the model received the raw value and can repeat it in a later clarification or terminal result, which is then published unchanged. Fresh evidence beyond the earlier verify_form result finding is this sibling-label path through the new value registry; correlate name/type/label with the field's value when collecting secrets.
Useful? React with 👍 / 👎.
| // A request body is a string argument, so the key-walk above cannot see into | ||
| // it. Both encodings a mutating call realistically uses are cheap to parse. | ||
| function collectRequestBodySecrets(body, into) { | ||
| if (typeof body !== 'string' || !body || body.length > WORKFLOW_PARAMETER_VALUE_LIMIT) return; |
There was a problem hiding this comment.
When a strict fetch_url call has a request body over 10,000 characters, this early return skips credential collection entirely. The tool-call body itself is masked, but the model still saw its password/token fields and can quote one later in a clarification or public terminal result; because the value registry never learned it, that later publication is not redacted. Fresh evidence beyond the earlier fetch-body finding is this hard size cutoff; oversized bodies should trigger fail-closed scalar redaction or bounded credential extraction rather than silently disabling protection.
Useful? React with 👍 / 👎.
|
@codex pls review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0f7af02940
ℹ️ 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".
| if (typeof type === 'string' && JSON_SCHEMA_TYPES.has(type)) return true; | ||
| if (Array.isArray(type) && type.length && type.every(item => JSON_SCHEMA_TYPES.has(item))) return true; |
There was a problem hiding this comment.
When an output_schema combines a recognized type with constraints such as minItems, maxItems, pattern, minimum, or maximum, this classifies and advertises the whole object as JSON Schema, but validateCloudOutput never evaluates those keywords. For example, [] passes {type:"array",minItems:3} and 10 passes {type:"number",maximum:5}, so done_json can complete a cloud run with a result that violates the caller's contract whenever the provider does not enforce the tool schema itself; implement these constraints or reject schemas containing unsupported keywords.
Useful? React with 👍 / 👎.
|
@codex pls review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0f7af02940
ℹ️ 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".
| const firstSegment = url.pathname.split('/').filter(Boolean)[0] || ''; | ||
| return { | ||
| url_origin: url.origin, | ||
| url_path_root: firstSegment ? `/${firstSegment}` : '/', |
There was a problem hiding this comment.
When a strict-secret run calls fetch_url with a capability URL whose first path segment is itself a secret (for example, https://files.example/<share-token>), this preserves that segment verbatim in the published tool_call update. rememberStrictSecrets also does not register values embedded in args.url, so URL credentials cannot be removed from later clarification or terminal text either. Avoid publishing arbitrary path segments in strict mode, or first detect and register credential-bearing URL components.
Useful? React with 👍 / 👎.
|
@codex pls review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fb6ccb71b1
ℹ️ 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".
| const structuredDone = doneJsonTool(opts.outputSchema, { | ||
| strictSecretMode: opts.strictSecretMode === true, | ||
| }); | ||
| return closeToolDefinitions(base.map(tool => (tool.function.name === 'done' ? structuredDone : tool))); |
There was a problem hiding this comment.
When a JSON Schema object declares properties but omits additionalProperties, JSON Schema permits extra keys by default; however, passing the generated done_json tool through closeToolDefinitions recursively adds additionalProperties: false to the caller's result schema. A result such as {name: "x", extra: 1} therefore passes validateCloudOutput against {type: "object", properties: {name: {type: "string"}}} but is rejected by validateToolArguments before runtime validation, so preserve explicit caller semantics rather than closing these embedded output-schema nodes.
Useful? React with 👍 / 👎.
| description: strictSecretMode | ||
| ? 'Machine-readable result matching the requested output schema. Must not contain credentials, passwords, API keys, tokens, OTPs, recovery codes, or other secrets.' | ||
| : 'Machine-readable result matching the requested output schema.', | ||
| ...doneJsonResultSchema(outputSchema), |
There was a problem hiding this comment.
When the forwarded output schema uses minLength or maxLength and a result contains an astral Unicode character, the two validators disagree: validateCloudOutput correctly counts code points with [...item].length, while validateToolArguments counts UTF-16 code units with value.length. For example, an emoji is valid under {type: "string", maxLength: 1} at runtime but the advertised done_json argument gate rejects it before dispatch; use the same JSON Schema length semantics in both validators.
Useful? React with 👍 / 👎.
|
@codex pls review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0382cecaa0
ℹ️ 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".
| if (type === 'tool_call') { | ||
| const args = data?.args && typeof data.args === 'object' ? data.args : {}; | ||
| if (CLOUD_TEXT_ENTRY_TOOLS.has(name)) candidates.push(args.text, args.value); | ||
| if (name === 'fetch_url') collectUrlSecretStrings(args.url, candidates); |
There was a problem hiding this comment.
When a strict run follows a credential-bearing capability URL through navigate, this collector registers URL components only for fetch_url. The generic strict-mode masking hides the navigate arguments, but run.finalUrl is later value-redacted without knowing that URL and cloudSnapshot publishes its full path and query. Unlike the previously addressed fetch_url path, the fresh evidence here is that navigation URLs never enter the secret registry; collect URL-bearing arguments from navigate and equivalent navigation tools as well.
Useful? React with 👍 / 👎.
| if (spec.properties || spec.required || Object.hasOwn(spec, 'additionalProperties')) { | ||
| if (!isObject(item)) { | ||
| push(path, 'expected object with properties'); | ||
| return; |
There was a problem hiding this comment.
When a caller supplies a type-agnostic JSON Schema such as a $schema-marked {properties: {...}}, {required: [...]}, or {additionalProperties: false}, JSON Schema requires those keywords to be ignored for non-object instances. This branch instead rejects every scalar with expected object with properties; for example, the valid string result for {$schema: "…", properties: {a: {type: "string"}}} is rejected and can exhaust the run's single repair attempt. Evaluate these keywords only when item is an object and let an explicit type: "object" enforce objectness.
Useful? React with 👍 / 👎.
|
@codex pls review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5a125a3941
ℹ️ 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".
| // than relying on the model not to repeat an unknown credential. | ||
| if (body.length > WORKFLOW_PARAMETER_VALUE_LIMIT) return true; | ||
| try { | ||
| collectSensitiveStrings(JSON.parse(body), into); |
There was a problem hiding this comment.
When a strict run sends an OTP through fetch_url as {"verification_code":123456} or {"code":123456}, this parsed-body walk passes it to collectSensitiveStrings, but isSensitiveCloudKey recognizes neither normalized key, so the literal never enters the run's secret registry. The immediate call is masked, but a later clarification, unstructured terminal response, or public done_json result can publish 123456 unchanged. Fresh evidence beyond the earlier numeric-secret finding is this common request-body label path; extend the credential-key detector or fail closed for unrecognized scalar body fields.
Useful? React with 👍 / 👎.
| return Object.entries(JSON_SCHEMA_KEYWORD_SHAPES) | ||
| .some(([keyword, hasSchemaShape]) => Object.hasOwn(spec, keyword) && hasSchemaShape(spec[keyword])); |
There was a problem hiding this comment.
When a caller supplies a valid type-agnostic schema such as {minLength: 3} without the optional $schema or type, this detector returns false because the supported constraint keywords are absent from JSON_SCHEMA_KEYWORD_SHAPES. Validation then treats the document as shorthand and rejects even "abc" with expected object; boolean items and boolean combinator branches are misclassified similarly. Fresh evidence beyond the earlier constraint finding is that the implemented constraints are reachable only after this incomplete classification, so recognize their schema-shaped values here.
Useful? React with 👍 / 👎.
|
@codex pls review |
There was a problem hiding this comment.
💡 Codex Review
webbrain/src/chrome/src/cloud-runs.js
Lines 939 to 941 in 3c0b5b2
Preserve a false root output schema
When a caller supplies the valid JSON Schema boolean false, this || chain converts it to null. The agent therefore receives no done_json tool, and the controller can report an ordinary prose completion instead of a structured run that necessarily fails validation. Use presence/nullish checks rather than truthiness so boolean root schemas retain their JSON Schema semantics.
ℹ️ 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".
| addEncoded(url.pathname); | ||
| for (const segment of url.pathname.split('/').filter(Boolean)) addEncoded(segment); | ||
| add(url.search); | ||
| for (const [key, item] of url.searchParams) { | ||
| add(key); | ||
| add(item); |
There was a problem hiding this comment.
In a strict structured run that visits an ordinary URL such as https://example.com/profile, this registers profile as a secret, so a valid public result like {section: "profile"} is changed to a redaction placeholder after schema validation. Query keys and benign values are registered just as broadly, corrupting legitimate caller-visible results whenever they repeat normal URL components. Fresh evidence beyond the earlier result-redaction finding is the later URL collector's unconditional registration of every path segment and query entry; restrict value redaction to credential-like URL components while continuing to suppress complete URLs from trace data.
Useful? React with 👍 / 👎.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
@codex pls check |
Summary
Testing
|
|
@codex pls review |
|
Codex Review: Didn't find any major issues. Another round soon, please! 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". |
2 participants
No description provided.