Skip to content

feat: add opt-in web research and slide source exports - #433

Open
L4b0R wants to merge 4 commits into
OpenCoworkAI:mainfrom
L4b0R:feat/web-search-sources
Open

L4b0R wants to merge 4 commits into
OpenCoworkAI:mainfrom
L4b0R:feat/web-search-sources

Conversation

@L4b0R

@L4b0R L4b0R commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Add opt-in Web Search v1 for research-backed slide decks: search and read public references, save evidence while working, associate it with stable slide IDs, and generate an independent Markdown sources document through normal exports. Slides do not get source footers, citation numbers, chart source captions, or a references page by default.

Draft for scope and architecture review. This is a larger first-version change (including tests and documentation), not a request to bypass the project's small-PR preference. The supporting startup fix is isolated in its own commit and can be split/cherry-picked if preferred.

Type of change

  • New feature
  • Bug fix
  • Documentation
  • Refactor (no behavior change)
  • Build / CI / tooling
  • Breaking change

Linked issue

Refs #432Opt-in web research for slides with independent source documents.

Implementation

  • Register web_search, web_fetch, and small research-record/evidence/slide/export tools through the existing manifest and final model-visible tool list. No agent-loop rewrite or MCP framework.
  • Use one provider-independent Tavily search adapter; main-process service owns credentials and HTTP requests. Distinguish disabled, missing credentials, empty results, service failures, timeouts, cancellation, and budget exhaustion.
  • Protect direct page reading with public HTTP(S)-only URLs, DNS validation and socket pinning, per-redirect checks, bounded body/output sizes, and abort propagation. HTML5 parsing uses lazy parse5 text-node extraction, not regex stripping or script execution. Output is untrusted text, not sanitized HTML or instructions.
  • Persist schema-versioned sources/evidence/usage in .codesign/research.json using the existing workspace writer lock and atomic replacement. Reject dangling IDs and quotations not present in saved excerpts; mark reading separately from fact verification.
  • Use data-slide-id and rendered semantic fingerprints rather than page numbers. Reorder/deletion follows current DOM order; changed content yields evidence gaps instead of old supporting links. Styling-only changes such as color/font changes retain evidence.
  • Generate collision-safe workspace Markdown and companion files beside normal exports or inside ZIPs. Include only used evidence plus calculation inputs; leave user-authored files and the slides themselves untouched.
  • Preserve opt-in and limits through provider/model changes, imports and settings saves. Use the registered ask IPC/AskModal consent path with explicit per-run allow/deny and the upstream design/run recovery scope.
  • Isolate unreadable credential migrations so an optional malformed key does not block opening Settings; preserve failed entries and keep strict decryption on use.

Configuration and usage

See WEB_SEARCH.md. With the app closed, add/update the active local config:

[webSearch]
enabled = true
maxCalls = 12
timeoutMs = 15000
maxChars = 10000

[secrets.tavily]
ciphertext = "plain:YOUR_TAVILY_API_KEY"

Restart, submit a research-backed slide request, and approve Allow this run when prompted. The key is never a tool parameter or renderer-facing setting. Feature remains disabled by default; configuring a key alone does not enable networking.

Validation

Rebased onto upstream ed5fed9 (v0.2.2-era main) and adapted research consent to the new persisted question/run scope. The normal pre-commit and pre-push hooks were used, not bypassed.

Check Result
pnpm lint Pass
pnpm -r typecheck (normal pre-push hook) Pass across the workspace
pnpm test 3,281 Vitest tests passed, 4 skipped; 61 CI-tool script tests passed
pnpm --filter @open-codesign/desktop build Pass after rebase
Focused question recovery, consent, cancellation and research integration after rebase 74 tests passed
Public patch scope, sensitive-pattern scan and git diff --check Pass

Local validation environment: Windows, Node 24.11.1, pnpm 10.33.4, installed system browser. The repository pins Node 22; a separate Node 22 local run and installer packaging were not performed. Remote CI, CodeQL and Linux packaging smoke have now passed for e05f8dd; maintainer review is still required.

Commits:

  • 21ddaf1 — keep unreadable credentials from blocking startup (independently reviewable supporting fix).
  • 198731c — opt-in web research and slide source exports.
  • e05f8dd — security review remediation: parsed HTML text extraction and linear EDITMODE scanning.

Automated integration uses mocked search/fetch responses but real system-browser HTML/JSX rendering and ZIP output, including record recovery, page reorder/deletion, visual-only changes without repeat searches, stale evidence, nested asset paths, and concurrent workspace writes. Network-boundary tests cover normalization, missing fields, private/metadata/mapped IPs, pinned DNS, redirects, budgets, failure, timeout, cancellation, and output limits.

A local tester reports successful Tavily search and basic slide usage. I have not independently run a comprehensive real-provider/autonomous-live-model E2E or tested every target OS. No credentials, private workspace records, generated artifacts, or local validation logs are included.

First-version limits

  • One search provider; no fallback/deep research, no PDF, authenticated or JavaScript-only article reading; direct page reader does not use proxies.
  • Research snapshots reuse an installed system browser. Text/SVG charts are supported; canvas/iframe/video research slides are rejected. Every slide section needs a stable ID.
  • Fingerprints are deliberately conservative: SVG geometry or image URL changes may require relinking even for a visual edit; arbitrary CSS-generated content and remote image bytes changing at the same URL are not semantically verified.
  • Natural-language offline instructions are in the agent guidance; the host-enforced gate is config opt-in plus explicit per-run consent. Source extraction and quote matching are not fact certification.
  • Previously exported Markdown is a snapshot; current-order sources are regenerated on the next export.

Dependency additions and security-review follow-up

The initial no-dependency HTML stripper was replaced in response to CodeQL alerts #188–191. No alert was dismissed or suppressed. parse5 parses without executing scripts or requesting resources; an iterative tree walk reads text nodes and skips active/non-content subtrees. Encoded tag literals remain untrusted text, not HTML to insert.

Package Role License Registry unpacked bytes
parse5 8.0.1 Direct desktop production dependency, imported on first HTML fetch only MIT 337,099
entities 8.1.0 Locked transitive entity decoder BSD-2-Clause 330,191

Total: 667,290 bytes (~652 KiB); not a measured installer delta. Verified package licenses and dependency paths with pnpm why. Existing parse5 5/6 copies are incidental transitive/dev dependencies; explicitly declaring the maintained, typed ESM parser avoids relying on those in a packaged app. Lightweight string helpers are not HTML5 parsers, and invoking a browser would introduce execution/resource-loading concerns. A peer dependency would leave the shipped reader dependent on a separate user install.

The fifth CodeQL annotation identifies polynomial runtime EDITMODE matching on repeated BEGIN markers without END. It is now a forward-only comment/marker scan, with complete-block compatibility and a 50,000-marker regression. The change is limited to the flagged binding path.

Review follow-up validation: parser/network tests (37), runtime tests (94), research/consent integration (included in a 72-test focused desktop run), desktop typechecks and build pass. The normal pre-push hooks also passed: 3,281 package tests plus 61 script tests, 4 skips, workspace typechecks and lint. Remote CI, Dependency Review, CodeQL and Linux packaging smoke all passed for e05f8dd; CodeQL reports no new alerts and zero annotations (see the follow-up comment).

Compatibility / upgradeability / lean / clarity

  • Compatibility: additive config fields and tool dependencies; existing designs without research remain usable; existing credentials/config settings are retained.
  • Upgradeability: versioned research file schema, reuse of current credential storage and native agent/export interfaces; changesets included, changelogs not edited.
  • Lean: one lazy HTML5 parser dependency (cost/alternatives below); no bundled browser/model/Python runtime, provider SDK, hosted service layer, or research panel.
  • Clarity: one adapter, a bounded main-process service, validated local records and deterministic export; no speculative provider/plugin registry.

Checklist

  • Reviewed relevant project context and linked the feature proposal.
  • Added/updated meaningful tests.
  • Added changesets for user-visible behavior.
  • Updated documentation.
  • Local lint, typechecks, tests and desktop build pass (details and skips above).

Screenshots / recordings

No new panel or visual layout is introduced. Consent reuses the existing question dialog, and companion-file paths use existing export notifications. No screenshot/recording is attached; this remains a draft.

@github-actions github-actions Bot added docs Documentation area:desktop apps/desktop (Electron shell, renderer) area:core packages/core (generation orchestration) area:exporters packages/exporters (PDF/PPTX/ZIP) labels Sep 20, 2026
Comment thread apps/desktop/src/main/web-research-network.ts Fixed
Comment thread apps/desktop/src/main/web-research-network.ts Fixed
Comment thread apps/desktop/src/main/web-research-network.ts Fixed
Comment thread apps/desktop/src/main/web-research-network.ts Fixed
@github-actions github-actions Bot added the area:build Turbo/Vite/Biome/tsconfig toolchain label Sep 21, 2026
@L4b0R

L4b0R commented Sep 21, 2026

Copy link
Copy Markdown
Contributor Author

Security review follow-up — e05f8dd

Addressed the five findings reported on the initial head:

  • Incomplete multi-character sanitization ([Bug]: 配置了中转但是点击生成就直接输出说已完成 但是右侧并没有看到信息 #188–191): removed the comment/raw-element/tag/title regex stripping pipeline. HTML is now parsed by lazy-loaded parse5, then an iterative walk collects text nodes and omits active/non-content subtrees. No scripts or resources are executed/loaded. The output contract is explicitly untrusted text, not sanitized HTML to insert. Added malformed fragments, quoted >, entity decoding, hidden/unfinished elements, table boundaries, deep nesting, and bounded fetch/excerpt regressions.
  • Polynomial EDITMODE regex (packages/runtime/src/index.ts): replaced the wildcard regex with a forward-only comment/marker scan. Tests cover complete/multiple blocks, marker whitespace, unmatched input, repeated BEGIN markers, and 50,000-marker input without repeated suffix scans.

No CodeQL alert was dismissed or suppressed.

Dependency impact

The PR body and WEB_SEARCH.md now explicitly disclose the new parser instead of claiming zero dependency additions: parse5 8.0.1 (MIT, 337,099 registry-unpacked bytes) plus locked entities 8.1.0 (BSD-2-Clause, 330,191 bytes), approximately 652 KiB combined. It is loaded only on HTML fetch; alternatives and why it cannot be an optional peer are documented. The lockfile changes are limited to this dependency chain.

Verification

  • Normal pre-push hooks passed: workspace typechecks/lint, 3,281 package tests + 61 script tests passed, 4 skipped.
  • Desktop build and research/source-export browser integration passed.
  • Remote CI, Dependency Review, CodeQL analysis/result check, and Linux packaging smoke all passed for e05f8dd.
  • The latest CodeQL result reports “No new alerts in code changed by this pull request”, with 0 annotations, replacing the previous failing five-alert result.

The PR remains a draft for maintainer scope/architecture review; the skipped Codex PR Review job is not a human approval.

@L4b0R
L4b0R marked this pull request as ready for review September 21, 2026 01:12

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings

  • [Minor] Research guidance is injected even when no research tools exist — packages/core/src/agent.ts:1216 builds baseAgenticGuidance by unconditionally prepending WEB_RESEARCH_GUIDANCE, while the tools are only registered under if (deps.research) (packages/core/src/agent.ts:1194) and availableToolNames({ research: deps.research !== undefined, ... }). Callers that omit research still receive a system prompt telling the model to put data-slide-id on every section and to call research_export(path) after the final page edits, which invites calls to tools that are not registered (and, on a plain deck, companion files made only of Evidence gap entries).
    Suggested fix: only prepend the block when deps.research is defined, and add a Vitest case asserting WEB_RESEARCH_GUIDANCE text is absent from state.systemPrompt when research is not passed.

  • [Minor] A companion-file failure now aborts the primary export — apps/desktop/src/main/exporter-ipc.ts:260 awaits prepareResearchExport(resolved) before exportArtifact with no guard. That path runs readResearchSlides (RESEARCH_SLIDES_SCRIPT throws on canvas/iframe/video slides, on a section without a unique data-slide-id, and on a deck that failed to render) and loadResearchStore (throws on a malformed .codesign/research.json). A design that exported fine before can now fail HTML/PDF/PPTX/ZIP/Markdown export entirely because of optional research metadata, with an error worded for research slides rather than for the export the user requested.
    Suggested fix: catch companion-generation errors, keep the export result, and append the message to researchWarnings so it is still surfaced in the existing export toast; or only hard-fail when the user explicitly asked for a sources file.

  • [Nit] research_records writes the file it only reads — apps/desktop/src/main/web-research.ts:120 routes readRecords through transaction(), which always calls saveResearchStore, so a read-only research_records call rewrites (and creates, when absent) .codesign/research.json.
    Suggested fix: add a read-only host path that skips saveResearchStore when nothing changed.

Questions

  • Was every remaining writeConfig call site that builds a fresh Config literal audited for webSearch preservation? The diff covers provider CRUD, Codex OAuth, external imports, image settings, design-system saves and onboarding reset, and apps/desktop/src/main/onboarding-ipc.test.ts exercises those. Any unaudited save path that still rebuilds the config would silently drop the opt-in and limits on the next ordinary save.

Summary

  • Review mode: initial. No blockers found; the two Minor items above are the only material concerns, and both are conditional on the research feature being in use.
  • Scope and constraints: the change is additive and offline-safe by default. parse5 is imported lazily inside readableHtml (apps/desktop/src/main/web-research-network.ts), so startup does not load it; packages/exporters/src/index.ts:18 lazy-imports the slide reader. New deps are permissive (parse5 8.0.1 MIT, entities 8.1.0 BSD-2-Clause) and pnpm-lock.yaml records entities engines node >=20.19.0, compatible with package.json engines node >=22. No provider SDK imports were added; LLM calls still route through the existing agent path.
  • Security posture looks sound from the diff: public HTTP(S) only on ports 80/443, no embedded credentials, reserved/private/loopback/link-local/CGNAT/IPv4-mapped IPv6 ranges blocked, all DNS answers validated before connect, the socket pinned to the validated address, every redirect re-validated, 1 MiB body cap, redirect cap, per-run call budget, timeout and cancellation, and sanitized error messages that do not echo the API key. HTML is parsed as untrusted text, never inserted as markup. Consent is genuinely host-enforced through the existing ask IPC and is per-run (apps/desktop/src/main/web-research.ts:96).
  • Credential-migration change is consistent with the no-silent-fallback rule: unreadable entries are preserved with a credential-free log line, and decryptSecret still fails strictly on use (apps/desktop/src/main/keychain.ts:96. Error-message narrowing in decryptSafeStorage is guidance-only.
  • The linked issue is referenced as Refs #432, which is the correct form for a partial/first slice; no closure claim needed validating. Changesets cover the four affected packages, docs and README were updated, and no release/packaging/distribution files are touched, so there is no checksum/manifest path to verify.
  • Residual risk: the PR body states live Tavily and a live-model autonomous E2E were not run in that session, and I could not independently verify the reported CI/CodeQL results from this diff. Windows-first validation plus a pinned Node 22 run remains a gap the author already flags.

Testing

  • Not run (automation). Suggested additions: a Vitest assertion that the research guidance and data-slide-id instructions are absent when deps.research is undefined; a test that a corrupt .codesign/research.json or a non-conforming deck does not fail the primary export (or that the failure is surfaced through researchWarnings); and a test that research_records does not create or rewrite .codesign/research.json.

Open-CoDesign Bot

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings

No new findings. The three items from the previous review (e05f8dd) are all addressed by this push:

  • [Resolved] Research guidance was injected without research tools — packages/core/src/agent.ts:1215 now builds researchGuidance only when researchTools.length > 0 and every research tool is present in the final tools list, and packages/core/src/agent.test.ts adds the two cases (no host; explicit tools: [] override) asserting ## Slides research and separate sources, data-slide-id, and research_export are absent from the prompt.
  • [Resolved] Companion failure aborted the primary export — apps/desktop/src/main/exporter-ipc.ts:257 wraps prepareResearchExport and the writeUniqueSources call (:274) in try/catch, converts failures into researchWarnings, and still returns status: 'saved'. apps/desktop/src/main/exporter-ipc.research.test.ts asserts the corrupt-store and render/prepare failures keep the primary file for all five formats, while primary exporter errors still reject (does not suppress primary exporter failures).
  • [Resolved] research_records rewrote the file it only reads — apps/desktop/src/main/web-research.ts:100 now uses a read-only inWorkspace path with no saveResearchStore, and apps/desktop/src/main/web-research-store.ts:36 gates .codesign creation behind storePath(root, true) used only by saveResearchStore. apps/desktop/src/main/web-research.test.ts (read-only research records) verifies no directory/file creation and unchanged bytes/mtime.

Questions

None.

Summary

  • Review mode: follow-up after new commits.
  • All three prior findings are fixed in the diff and covered by targeted Vitest cases; I did not find a new correctness, security, or regression issue introduced by this push. The prior Minor items are no longer merge-relevant.
  • Constraint posture is unchanged and sound: parse5 remains lazily imported inside readableHtml (apps/desktop/src/main/web-research-network.ts), so startup does not load it; the new dependency is MIT (parse5 8.0.1) with a BSD-2-Clause transitive (entities 8.1.0), and pnpm-lock.yaml pins entities engines node >=20.19.0, compatible with the repo's node >=22. No provider SDK imports were added; LLM calls still route through the existing pi-ai/agent path. The SSRF boundary (public HTTP(S) only, ports 80/443, no embedded credentials, reserved/private/loopback/link-local/CGNAT/IPv4-mapped IPv6 blocked, all DNS answers validated and the socket pinned, per-redirect revalidation, 1 MiB cap, redirect/call budgets, timeout and cancellation, sanitized errors) is unchanged from the previously reviewed state.
  • The linked issue is referenced as Refs #432, which is correct for this first slice, so no closure claim needed validating. Changesets cover the affected packages and no release/packaging/distribution files are touched.
  • Residual risk (unchanged, author-acknowledged): live Tavily and a live-model autonomous end-to-end run were not verified in this session, and I cannot independently reproduce the reported remote CI/CodeQL results from the diff. Node 22 and non-Windows packaging remain unverified gaps.

Testing

  • Not run (automation). The new coverage is proportional: apps/desktop/src/main/exporter-ipc.research.test.ts:79 (primary-export resilience across all formats), apps/desktop/src/main/web-research.test.ts:303 (read-only records), and packages/core/src/agent.test.ts:2710 (guidance/tool gating). A live-provider smoke test on Node 22 would close the remaining confidence gap, but that is environment work rather than a code change.

Open-CoDesign Bot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:build Turbo/Vite/Biome/tsconfig toolchain area:core packages/core (generation orchestration) area:desktop apps/desktop (Electron shell, renderer) area:exporters packages/exporters (PDF/PPTX/ZIP) docs Documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants