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: a90997014f
ℹ️ 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".
src/browser/base-browser-adapter.ts
Outdated
| headers, | ||
| body: body ?? undefined, | ||
| redirect: 'manual', // Surface redirects to executor for per-hop validation | ||
| credentials: 'include', // Include cookies (cf_clearance, session tokens) on all requests |
There was a problem hiding this comment.
Setting credentials: 'include' here turns every Tier-3 replay into a credentialed CORS request. That breaks any skill whose browser session is on one origin but the learned API lives on another (for example www.example.com calling api.example.com, or any public third-party API): browsers now require Access-Control-Allow-Credentials: true and reject the common Access-Control-Allow-Origin: * responses that worked before. The Cloudflare/session-cookie cases this change targets need credentials, but making it unconditional regresses cross-origin replays that previously succeeded.
Useful? React with 👍 / 👎.
src/server/tool-registry.ts
Outdated
| // When intent is provided, require at least one lexical match | ||
| const relevant = scored.filter(s => s.relevance > 0); |
There was a problem hiding this comment.
searchAndProjectSkills() still feeds FTS results through rankToolsByIntent(), and the FTS index uses the porter tokenizer in src/storage/database.ts. After this filter, queries like users can match user in FTS but still be dropped here because relevance only counts literal substring hits. In practice the REST/MCP search endpoints will now return no results for many singular/plural or stemmed queries even though SQLite already found valid matches.
Useful? React with 👍 / 👎.
…ser tools Node version enforcement (bin/schrute.cjs), test DB isolation via SCHRUTE_DATA_DIR, serve auth token hint, skills delete --yes, empty body parser, CLI rate-limit auto-retry, MCP HTTP admin opt-in, import preview with transactional writes + policy persistence, OpenAPI probe paths, progress indicators, doctor audit chain downgraded to warning, browser_fill_form schema, v0 deprecation header, --url/--token on subcommands, search ranking lexical filter, config get --reveal, skills list --status, batch execute rate-limit retry, README MCP note.
sheeki03
force-pushed
the
fix/22-user-frictions
branch
from
a909970 to
a7e1beb
Compare
Detect Cloudflare challenges during execution, apply permanent browser_required tier lock, suppress direct probes, persist browserRequired flag in site policy, and skip auto-validation for browser-required skills.
…wser idle cleanup Recovery CDP sessions now auto-close after 60s idle. Agent-browser exec sessions track lastUsedAt and are swept by the engine's periodic session sweep using the configured browser idle timeout. Metadata files on disk provide crash-recovery cleanup on startup. Chrome process-tree termination uses both async and sync paths for normal and exit-handler contexts.
… HTML extraction, workflows Response transforms (jsonpath, regex with worker isolation, CSS via cheerio). Export as curl/fetch.ts/requests.py/playwright.ts. Parallel batch execution with write barriers and policy-driven concurrency. HTML document classification in noise filter for server-rendered sites. Linear workflow executor with preflight validation, param mapping, per-step transforms, caching, and browser-handoff propagation.
Add async waitForPermit() that sleeps until tokens refill or backoff clears. Add minGapMs to SitePolicy (default 100ms) for minimum inter-request spacing. Workflow steps use waitForPermit instead of immediate fail-fast checkRate, fixing same-site rate-limit failures in sequential workflows.
Update 30+ test files for browser-required, lifecycle, transforms, export, batch, workflows, rate limiter, and HTML extraction. Add tests/live/ integration suite (httpbin, coingecko, hackernews) with dedicated vitest.live.config.ts. Live tests excluded from default runs via vitest.config.ts exclude.
…ison Update benchmarks with real CoinGecko data (310ms first, 63ms warm). Add browser-required lock explanation. Update competitive comparison with Browser-Use, Stagehand, Skyvern. Add CLI examples for new commands (--status, --reveal, --yes, set-transform, export).
1 participant
Summary
Fixes 22 user-facing frictions identified during exhaustive manual testing of the CLI, REST API, MCP HTTP, and browser tools — ranging from blocking first-run issues to minor paper cuts.
Changes by area
CLI (src/index.ts)
bin/schrute.cjslauncherskills delete --yesconfirmation promptconfig get --revealfor sensitive valuesskills list --statusfilter--url/--tokenon all remote-capable subcommandsexecute(both daemon and remote paths)serveprints masked auth token, REST vs MCP auth note, SCHRUTE_DATA_DIR helpImport (src/app/import-service.ts — new)
--yesflagdb.transaction()setSitePolicy()createdAton skill updatesREST Server
Deprecationheader on v0 routesMCP HTTP
mcp-http:caller ID prefix for admin authserver.mcpHttpAdminconfig flag (validated as boolean)Tools
browser_fill_formexplicit schema + better error messageschrute_capture_recentdescription clarifies CDP requirementInfrastructure
SCHRUTE_DATA_DIRtemp dir (vitest globalSetup)config-env.test.tsenv var save/restore to prevent cross-test leaksFiles changed
bin/schrute.cjs,src/app/import-service.ts,tests/global-setup.ts,tests/unit/import-service.test.tsTest plan
npx tsc --noEmit— cleannpx vitest run— 177 files, 2850 tests passschrute skills delete <id>prompts,--yesskipsschrute config get server.authTokenmasked,--revealshows fullschrute import <file>shows preview,--yesskips promptschrute serve --httpshows auth token hintDeprecation: trueheader