feat(rust-browser-connection): Rust module for noVNC + browser connection per #347#349
Open
skulidropek wants to merge 4 commits into
Open
feat(rust-browser-connection): Rust module for noVNC + browser connection per #347#349skulidropek wants to merge 4 commits into
skulidropek wants to merge 4 commits into
Conversation
…onnection (issue #347) - Extracted BrowserConnection Effect Layer, pure helpers, invariants - Supports both MCP and built-in Hermes browser tools out of the box - Single browser session with noVNC/CDP (port 9223) - Follows effect-template + AGENTS.md style (formal comments, types, Layer) Closes #347
…tion per #347 - New package based on rust-ai-driven-development-pipeline-template - BrowserConnection with start_browser, get_novnc_url, get_cdp_url, invariant - Uses bollard for Docker, replicates docker-git MCP Playright + noVNC behavior - Out of the box for both MCP and Hermes built-in browser tools Closes #347
📝 WalkthroughSummary by CodeRabbit
WalkthroughДобавлены архитектурные планы; новый npm-пакет TS-контракта и дублирующий core TS-файл; полностью реализован Rust-крейт BrowserConnection с CLI, набором релизных Rust-скриптов; добавлены CI workflows, тесты и множество case-study/документов. ChangesЕдиная поставка: BrowserConnection (TS) + Rust-крейт и пайплайн
Estimated code review effort ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
Member
Author
AI Session BackupCommit: 72b751d
|
…eline-template + our noVNC/browser module for #347
Member
Author
AI Session BackupCommit: 572a163
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 59
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/rust-browser-connection/docs/case-studies/issue-38/template-data/js-template-ci-tree.txt (1)
1-23:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winНепереносимые абсолютные пути в template-data.
На Line 1-23 зафиксированы локальные
/tmp/...пути. Для репозиторных template-data лучше хранить относительные пути от корня проекта, иначе артефакт зависит от окружения генерации и шумит в сравнении.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/rust-browser-connection/docs/case-studies/issue-38/template-data/js-template-ci-tree.txt` around lines 1 - 23, В файле js-template-ci-tree.txt найдено множество зафиксированных абсолютных путей вида /tmp/js-ai-driven-development-pipeline-template/..., которые делают артефакт зависимым от окружения; замените все такие абсолютные пути на репозиторно-относительные пути (например относительные от корня проекта или шаблонные маркеры ./ или {repo_root}/), обновите перечисления в файле (каждую строку начинающуюся с /tmp/js-ai-driven-development-pipeline-template/) чтобы не включать /tmp префикс, и убедитесь, что после изменения сравнениe шаблон-данных стабильно между машинами сборки.packages/rust-browser-connection/docs/case-studies/issue-38/raw-data/issue-38.json (1)
1-2:⚠️ Potential issue | 🟠 Major | ⚡ Quick winСанитизируйте author/commenter данные в case-study raw JSON.
Сейчас в артефакт включены персональные поля (
name,login,id) и прочие user-идентификаторы. Для анализа CI/CD проблемы они не обязательны; удалите/обезличьте user-блоки при сохранении в репозиторий.As per coding guidelines, "
**/*.{js,ts,jsx,tsx,py,java,go,rb,php,sh,bash,yml,yaml,json,env*,toml,cfg,config,dockerfile,dockerignore}: Fail if changed files expose credentials, tokens, private-keys, or PII in source, generated config, logs, or CI output".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/rust-browser-connection/docs/case-studies/issue-38/raw-data/issue-38.json` around lines 1 - 2, Sanitize the case-study raw JSON by removing/obfuscating all personal user fields inside author and comments.author blocks in the artifact (e.g., the top-level "author" object and each "comments[].author"); specifically strip or replace id, login, name, url and any other user-identifiers with non-PII placeholders (e.g., "anon_user_1") while preserving non-PII fields needed for analysis (timestamps, body, labels, title, number, state), and update the code that writes issue-38.json to perform this transformation before saving so generated artifacts never contain PII and thus satisfy the repository CI credential/PII scanning rules.packages/lib/src/core/browser-connection.ts (1)
16-131:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winКритично: browser-connection.ts содержит дублирующиеся экспортируемые объявления и неразрешённый тип
- В файле
packages/lib/src/core/browser-connection.tsдважды объявлены и экспортируютсяBrowserError,BrowserConnection,BrowserConnectionLive, а также дважды встречаетсяexport default BrowserConnection(пример: строки ~16–67 и ~72–130) — это блокирует сборку.- Тип
ProjectBrowserProxyPathиспользуется вparseProxyPath, но не импортируется/не объявлен в модуле — ещё один компиляционный блокер (строка ~82).- Есть запрещённые по coding-guidelines конструкции:
unknown(в интерфейсах/конструкторе) иas-утверждения (as const,return undefined as void) в коде этого модуля.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/lib/src/core/browser-connection.ts` around lines 16 - 131, This file contains duplicate exported declarations (BrowserError, BrowserConnection, BrowserConnectionLive and two default exports) and uses forbidden constructs (`unknown`, `as const`, `as void`) and an undeclared type (ProjectBrowserProxyPath). Consolidate into a single set of exports: keep one BrowserError class (replace cause: unknown with cause?: Error | undefined), one BrowserConnection interface (use parseProxyPath: Effect.Effect<ProjectBrowserProxyPath | null, never>), one BrowserConnectionLive implementation, and a single default export BrowserConnection; remove the duplicate blocks and merge the intended runtime stubs (startBrowser/getCdpUrl/getNoVncUrl/getVncUrl/parseProxyPath/rewriteCdpUrl) into that single implementation. Replace `as const` and `as void` assertions with proper typing/returns, and import or declare the ProjectBrowserProxyPath type at the top so parseProxyPath compiles. Ensure rewriteCdpUrl/getCdpUrl implementations remain consistent after consolidation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/browser-connection/package.json`:
- Around line 35-43: В package.json пакета `@prover-coder-ai/browser-connection`
зафиксируйте версии зависимостей — замените caret-диапазоны в полях
"dependencies" и "devDependencies" на конкретные версии (удалите "^" у effect,
typescript, vitest, `@effect/vitest`, `@types/node` и т.д.) чтобы публикуемый пакет
через bun publish содержал точные версии и не расширял supply-chain поверхность
для потребителей.
In `@packages/browser-connection/src/index.ts`:
- Around line 23-33: BrowserConnectionLive currently doesn't implement the
runtime contract: implement startBrowser to actually start/record a single
browser session for the given projectId (create/store session state and log
errors instead of returning undefined), implement parseProxyPath to parse the
incoming pathname and return a project id or session descriptor instead of
always null, make rewriteCdpUrl use the passed externalOrigin and projectId to
rewrite the CDP URL correctly (not just return value), and make getVncUrl
include projectId (and any per-project port or path mapping) rather than
returning a constant; update the implementations in BrowserConnectionLive
(startBrowser, parseProxyPath, rewriteCdpUrl, getVncUrl) so they coordinate with
whatever session storage/registry the runtime uses to ensure a single managed
browser session per project.
- Around line 3-47: Add required functional TSDoc comments for all exported
symbols: BrowserError, BrowserConnection, BrowserConnectionLive, renderNoVncUrl,
renderCdpUrl, and isSingleBrowserSession. For each exported type/class/function
place a TSDoc block immediately above its declaration and include at minimum: a
short description, and the required markers such as `@CHANGE` or `@WHY` (purpose),
`@INVARIANT` (behavior guarantees), `@COMPLEXITY` (complexity/cost), and
`@PURITY/`@EFFECT as applicable; for BrowserConnectionLive document side-effects
and environment wiring with `@EFFECT`, for helpers document purity with `@PURITY`
and examples/usage where helpful. Ensure tags and descriptions follow the
project coding-guidelines format and appear for every exported symbol listed.
- Around line 3-15: Fix the public TS contract by removing "as" casts and
"unknown" from exported types: change BrowserError to use a plain string literal
tag (no "as const") and give cause a concrete domain/error type (not unknown),
update BrowserConnection.parseProxyPath to return
Effect.Effect<ProjectBrowserProxyPath | null, never> (and replace
Effect.succeed(null) with the typed success), and stop using "as void" in
startBrowser implementation—return a proper Effect like Effect.unit or
Effect.succeedVoid instead; also add minimal TSDoc/functional comment markers
above the exported BrowserError and BrowserConnection to document the public
API. Ensure you reference and update the symbols BrowserError,
BrowserConnection, startBrowser, and parseProxyPath in the file.
In `@packages/lib/src/core/browser-connection.ts`:
- Around line 32-46: The current BrowserConnectionLive implementation provides
stubbed startBrowser and URL methods that are not tied to the actual
dg-*-browser container, so it does not enforce the required single unified
browser session invariant; update BrowserConnectionLive so startBrowser
negotiates/creates or looks up a single shared browser container/session (e.g.,
by calling the container manager or session registry), store and reuse the
session handle, and make getCdpUrl, getNoVncUrl, getVncUrl, parseProxyPath and
rewriteCdpUrl derive their values from that stored session (failing or
refreshing the session if missing/expired) instead of returning hardcoded
values; ensure rewriteCdpUrl performs origin-aware rewriting using the real
external origin and stored project/session metadata.
- Line 17: The core module violates project bans on "as" casts and unknown:
update the BrowserError class to declare readonly _tag: "BrowserError" =
"BrowserError" (remove as const); change the constructor signature from cause?:
unknown to a concrete type such as cause?: Error | undefined (remove unknown),
and remove any duplicate cause?: unknown declarations; change parseProxyPath's
Effect type from Effect.Effect<unknown, never> to a concrete environment type
(e.g., Effect.Effect<never, never> or a specific env/interface used by your
effects) instead of unknown; and replace any return undefined as void with an
explicit bare return (or return void 0) to avoid using as. Ensure you update all
occurrences of these symbols (BrowserError, constructor(cause), parseProxyPath,
and the return site) consistently.
- Line 82: The parseProxyPath signature references the missing type
ProjectBrowserProxyPath; add a proper type definition or an import for
ProjectBrowserProxyPath into this CORE module and update the declaration for
readonly parseProxyPath accordingly; locate parseProxyPath in
browser-connection.ts and either (a) import the ProjectBrowserProxyPath type
from the API package where it’s defined, or (b) copy/declare a compatible local
type in this file/module and export it if needed, then run type-checks to ensure
no unresolved type errors remain.
In `@packages/rust-browser-connection/.github/workflows/release.yml`:
- Around line 478-483: The run step in the "Version and commit" job is
interpolating workflow_dispatch inputs directly into the shell command
(rust-script ... --bump-type "${{ github.event.inputs.bump_type }}"
--description "${{ github.event.inputs.description }}"), which is unsafe; update
the step to use the already-declared environment variables (BUMP_TYPE and
DESCRIPTION) instead (e.g., reference $BUMP_TYPE and $DESCRIPTION in the run
command) and make the same change for the other occurrence referenced (lines
596-600) so all inputs are consumed via env variables rather than direct
github.event.inputs interpolation; keep the step id/version and env names (id:
version, env: BUMP_TYPE, DESCRIPTION) intact.
- Around line 655-657: В workflow_dispatch-triggered job the checkout is pinned
to "ref: main", so manual releases started from a non-main branch will still
build/deploy docs for main; update the workflow to either restrict manual
dispatches to main (add a condition limiting the workflow/job to github.ref ==
'refs/heads/main' or restrict triggers to branches: main) or change the
actions/checkout step (actions/checkout@v6) to check out the current run
ref/commit by using the workflow context (use github.ref or github.sha as the
ref) so the docs are built from the exact commit that triggered the run.
In `@packages/rust-browser-connection/Cargo.toml`:
- Line 21: Remove the unused Tokio dependency entry "tokio = { version = \"1\",
features = [\"full\"] }" from Cargo.toml (or if you plan to use Tokio, replace
it with a minimal feature set) and clean up the unused import "use tokio;" in
src/lib.rs so the crate does not declare an unnecessary dependency; ensure no
other code references tokio::... or #[tokio::...] before removing.
In
`@packages/rust-browser-connection/changelog.d/20251227_224645_changeset_support.md`:
- Around line 7-14: The changelog lists JavaScript script names (.mjs) but the
package uses Rust scripts; update the fragment to reference the actual Rust
filenames and documentation: replace `get-bump-type.mjs` → `get-bump-type.rs`,
`collect-changelog.mjs` → `collect-changelog.rs`, and `version-and-commit.mjs` →
`version-and-commit.rs`, and ensure the `changelog.d/README.md` reference
remains correct; keep wording consistent in the Added/Changed bullets so the
names match the repository's scripts (e.g., in the "Enhanced release workflow"
bullet mention `get-bump-type.rs` if applicable).
In
`@packages/rust-browser-connection/changelog.d/20260111_multi_language_support.md`:
- Around line 6-19: Запись в changelog о `rust-paths.mjs` и ряде `scripts/*.mjs`
не соответствует текущему toolchain, где используются Rust-скрипты
(`scripts/*.rs`); приведи changelog (файл 20260111_multi_language_support.md) в
соответствие с реальным состоянием: либо заменяй упоминания `*.mjs` на
соответствующие `*.rs`-скрипты и обнови список затронутых скриптов (например
заменить `scripts/bump-version.mjs` → соответствующий Rust-скрипт), либо если
проект действительно добавил `rust-paths.mjs`, обнови кодовую базу/CI чтобы
включить этот файл и параметры (`--rust-root`, `RUST_ROOT`) — выбор должен быть
последовательным; проверь и поправь все упоминания `rust-paths.mjs`,
`--rust-root` и `RUST_ROOT` в changelog и в скриптах (`scripts/*.rs` или
`rust-paths.mjs`) чтобы документация отражала фактическую реализацию.
In
`@packages/rust-browser-connection/docs/case-studies/issue-19/pr-114-data/pr-commits.json`:
- Line 1: The commit dump includes personal email addresses in the
commits[*].authors.email fields (see the "commits" array and individual commit
"oid" entries); remove or redact all authors.email values (replace with null,
empty string, or a non-PII placeholder) for every commit object in the JSON and
commit the sanitized file so the file no longer contains personal emails while
preserving other fields (authoredDate, authors.login/name, messageHeadline,
oid).
In
`@packages/rust-browser-connection/docs/case-studies/issue-19/pr-114-data/pr-conversation-comments.json`:
- Line 1: The JSON file currently contains PII fields (login, id, avatar_url,
url, html_url and similar profile fields) committed into the repo; remove or
redact those fields and update the generator so it emits anonymized placeholders
instead of real values, or move the file out of source control and add it to
.gitignore; specifically edit the object entries that contain "login", "id",
"avatar_url", "url", and "html_url" to replace real values with non-identifying
placeholders (e.g., user_N, id_N, avatar_placeholder) and add/enable a CI lint
check to fail on commits containing these keys with non-placeholder formats to
prevent future PII leaks.
In
`@packages/rust-browser-connection/docs/case-studies/issue-19/pr-114-data/pr-details.json`:
- Line 1: The PR dump JSON contains PII in fields like author.id, author.login,
author.name and in comments[].author.login and comments[].body; update the
generated pr-details.json content to remove or anonymize all personal
identifiers and free-text user comments (e.g., replace author.id/login/name with
"<REDACTED>" or remove the author object, and strip or redact comments[].body),
ensure any similar keys (author, comments, createdAt) are sanitized
consistently, and add a small sanitizer step in the generation process that
avoids embedding raw user identifiers into the exported JSON.
In `@packages/rust-browser-connection/docs/case-studies/issue-21/README.md`:
- Around line 167-173: В текущем примерe env-мэппинга используется ключ
CARGO_TOKEN, что конфликтует с остальной документацией, где используется
CARGO_REGISTRY_TOKEN; исправьте пример так, чтобы ключ был CARGO_REGISTRY_TOKEN,
а значение падало назад на старое имя секрета (например: CARGO_REGISTRY_TOKEN:
${{ secrets.CARGO_REGISTRY_TOKEN || secrets.CARGO_TOKEN }}), чтобы устранить
неоднозначность и сохранить совместимость с существующими секретами.
In
`@packages/rust-browser-connection/docs/case-studies/issue-38/raw-data/downstream-meta-ontology-issue-3.json`:
- Line 1: The raw JSON commit contains PII in the author object (author.id,
author.login, author.name); remove or mask these fields before committing the
file in docs/case-studies/issue-{id} (e.g., replace values with null, hashes, or
a generic placeholder) and ensure any other PII-sensitive keys in the snapshot
(emails, user IDs, tokens) are similarly redacted; add a validation step
(pre-commit hook or CI check) that scans files matching the repository
sensitive-glob pattern (*.json, *.yml, *.env*, etc.) and fails the commit if
unmasked PII is detected.
In
`@packages/rust-browser-connection/docs/case-studies/issue-38/raw-data/downstream-meta-ontology-pr-4.json`:
- Line 1: The raw JSON includes personal identifiers in keys like "author",
"author.id", "author.login", "author.name", and within "comments[].author" which
must be depersonalized before archiving: implement a sanitization step that
walks the JSON, replaces PII fields (id, login, name, email, viewerDidAuthor)
with stable non-reversible hashes or fixed placeholders, strips or redacts
free-text fields in "body" that contain email/IDs, and ensure comments array
entries are sanitized too; locate and update the code that writes this artifact
(the routine that serializes or saves the PR raw-data JSON) to call this
sanitizer prior to persist, and add tests verifying that "author" and
"comments[].author" no longer contain raw PII.
In
`@packages/rust-browser-connection/docs/case-studies/issue-38/raw-data/issue-38-comments.json`:
- Line 1: The committed raw JSON contains PII fields (e.g., "login", "id",
"html_url", "url", "avatar_url", "gravatar_id", "followers_url",
"following_url", "gists_url", "starred_url", "subscriptions_url",
"organizations_url", "repos_url", "events_url", "received_events_url") in
packages/rust-browser-connection/docs/case-studies/issue-38/raw-data/issue-38-comments.json;
redact or anonymize those fields before committing by removing or replacing
values with non-identifying placeholders or hashes (keep only necessary metadata
for analysis such as "created_at", "body", "author_association", and the comment
"id" if required but obfuscated), update the file in the case-studies/issue-38
folder accordingly, and re-run any CI/validation that checks for exposed PII to
ensure the change passes.
In
`@packages/rust-browser-connection/docs/case-studies/issue-38/raw-data/pr-39-reviews.json`:
- Line 1: The raw-data file
packages/rust-browser-connection/docs/case-studies/issue-38/raw-data/pr-39-reviews.json
was wiped to [] with no explanation; either restore the original raw review
array or, within the same PR, add an explicit justification for clearing it
(e.g., a note in the PR description and a documented change entry next to
pr-39-reviews.json explaining why data was removed, what replaced it, and how to
reproduce previous results) so the change is traceable and not treated as an
undocumented loss of artifacts.
In
`@packages/rust-browser-connection/docs/case-studies/issue-38/raw-data/pr-39.json`:
- Line 1: The PR export is currently writing raw author PII (author.id,
author.login, author.name) into the saved JSON; update the PR
serialization/export routine that produces this JSON to redact or hash those
fields before writing (e.g., replace author.id/login/name with hashed tokens or
remove them and keep a non-reversible fingerprint), ensure the sanitized shape
preserves any needed non-PII fields (e.g., createdAt, title, number), and add a
unit/test verifying that author.id, author.login and author.name are not present
in the persisted output.
In
`@packages/rust-browser-connection/docs/case-studies/issue-52/raw-data/issue-52.json`:
- Line 1: The JSON snapshot contains PII in the "user" block (fields like login,
id, node_id, avatar_url, gravatar_id, url, html_url, followers_url,
following_url, gists_url, starred_url, subscriptions_url, organizations_url,
repos_url, events_url, received_events_url) and similar actor fields (assignee,
closed_by) — redact or remove those PII fields and replace them with an
anonymized minimal user object (e.g., keep only a non-identifying type/status
and optionally an anonymized id or "anonymous": true) while preserving technical
issue fields (title, body, number, created_at, updated_at, labels, state,
repository_url, html_url) so the case study remains reproducible and compliant
with the project's PII rule.
In
`@packages/rust-browser-connection/docs/case-studies/issue-52/raw-data/js-issue-62.json`:
- Line 1: Sanity check: the exported issue JSON includes PII (user.login,
user.id, node_id, avatar_url, profile endpoint URLs, gravatar_id, and many
user-related URLs); redact or remove those fields and preserve only minimal
technical fields (e.g., title, number, state, created_at, updated_at, body,
html_url) and non-identifying user metadata if needed. Fix by updating the JSON
export/generator to strip or hash these keys (user.login, user.id, user.node_id,
user.avatar_url, user.gravatar_id, user.url, user.html_url, user.followers_url,
user.following_url, user.gists_url, user.starred_url, user.subscriptions_url,
user.organizations_url, user.repos_url, user.events_url,
user.received_events_url, user.site_admin) and replace with null or a
deterministic hash; update consumers that expect the "user" object to accept a
reduced shape (or emit a sanitized placeholder like {"type":"User"}), and add a
test to assert no PII keys remain in the produced js-issue-62.json dump.
In
`@packages/rust-browser-connection/docs/case-studies/issue-52/raw-data/vk-bot-desktop-issue-51.json`:
- Line 1: The raw JSON snapshot contains PII on Line 1 (fields like user.login,
user.id, user.avatar_url, html_url and any profile URLs); remove or redact those
fields and keep only minimal technical metadata (e.g., number, state,
created_at, aggregated status fields). Replace personal identifiers with hashed
or anonymized placeholders (not reversible), strip avatar and profile URLs, and
ensure only non-PII keys remain in the object; update the snapshot for
vk-bot-desktop issue 51 accordingly and add/verify a validation step that
rejects commits exposing user.* fields to prevent regressions.
In
`@packages/rust-browser-connection/docs/case-studies/issue-52/raw-data/vk-bot-desktop-pr-52.json`:
- Line 1: Summary: The raw JSON snapshot contains PII (user objects: login, id,
avatar_url and many profile URLs) that must be removed or anonymized. Fix: edit
the raw snapshot file vk-bot-desktop-pr-52.json to strip or hash all personal
fields inside "user", "head.user", "base.user", "assignees", "merged_by" and any
nested profile URLs — remove keys like login, id, avatar_url, url, html_url,
followers_url, following_url, gists_url, starred_url, subscriptions_url,
organizations_url, repos_url, events_url, received_events_url; keep only
necessary technical PR fields (number, title, state, created_at, updated_at,
merged_at, merge_commit_sha, commits/additions/deletions/changed_files,
html_url/diff_url) or replace user entries with a single anonymized object (e.g.
{"user_id":"anon-1234"}). Commit the sanitized file and run the repo’s
PII/credentials check before push.
In `@packages/rust-browser-connection/docs/ci-cd/troubleshooting.md`:
- Around line 94-99: В документе осталась ссылка на несуществующий Node-скрипт
`scripts/publish-crate.mjs`; нужно заменить вызов на реальный Rust-entrpoint:
вместо `node scripts/publish-crate.mjs` использовать команду запуска бинарного
скрипта через Cargo (например `cargo run --bin publish-crate` или `cargo run
--package <crate_name> --bin <bin_name>` в зависимости от настроек Cargo.toml) и
сохранить переменную окружения `CARGO_REGISTRY_TOKEN`; также обновите все
аналогичные вхождения `scripts/publish-crate.mjs` в разделе (207-225) на
соответствующие `cargo run` команды и при необходимости укажите имя бина из
`scripts/*.rs`.
In `@packages/rust-browser-connection/examples/basic_usage.rs`:
- Around line 1-7: Текущий пример демонстрирует только функцию sum, но PR обязан
показать поведение BrowserConnection/noVNC в single-session workflow; замените
содержимое examples/basic_usage.rs на пример, который импортирует и использует
BrowserConnection (или функцию-конструктор, например BrowserConnection::new /
start_browser_connection), создает одну сессию, вызывает методы получения
адресов (например generate_novnc_url и get_cdp_url или аналогичные методы в API)
и выводит оба URL в stdout, затем корректно закрывает сессию/соединение;
убедитесь что используются реальные символы из библиотеки (BrowserConnection,
Session, generate_novnc_url, get_cdp_url, close_session) и что пример
компилируется и демонстрирует single-session flow для верификации issue `#347`.
In
`@packages/rust-browser-connection/experiments/test-version-check-dependencies.sh`:
- Around line 9-22: The script hardcodes a restore to issue-14-9d4fe6371f90 and
blindly suppresses errors, leaving the repo in an inconsistent state on
failures; update the flow in test-version-check-dependencies.sh to capture the
current branch (e.g. via git rev-parse --abbrev-ref HEAD) into an ORIG_BRANCH
variable, create a deterministically-named temporary branch
(test-deps-change-branch) only when needed, and register a trap on EXIT to
perform idempotent cleanup: checkout $ORIG_BRANCH only if it exists, restore
Cargo.toml from Cargo.toml.bak only if the backup exists, and delete the
temporary branch only if it was created; ensure errors are not suppressed
silently and that the check invocation (scripts/check-version-modification.mjs)
runs between creation and cleanup.
In `@packages/rust-browser-connection/experiments/test-version-check.sh`:
- Line 5: The script calls the wrong entrypoint: it invokes node
scripts/check-version-modification.mjs but the package provides a Rust
implementation scripts/check-version-modification.rs, causing
MODULE_NOT_FOUND/ENOENT. Update each invocation (the strings "node
scripts/check-version-modification.mjs") to run the Rust implementation instead
(for example replace with a Cargo invocation such as "cargo run --bin
check-version-modification" or the appropriate project-specific command that
runs scripts/check-version-modification.rs), ensuring all occurrences referenced
(the lines that call scripts/check-version-modification.mjs) are changed to the
Rust-run command so the experiment runs successfully.
- Around line 24-37: The script mutates git state unsafely: replace the
hardcoded branch/checkout and add robust cleanup by saving the current branch
(git rev-parse --abbrev-ref HEAD) and trapping EXIT/ERR to restore it, restore
the original Cargo.toml from a backup, and delete the temporary branch; ensure
the temp branch name (test-version-change-branch) is deterministic or randomized
to avoid collisions, create a Cargo.toml.bak before sed, and run the version
check (node scripts/check-version-modification.mjs) within this guarded section
so any failure triggers the trap that runs the cleanup steps.
In `@packages/rust-browser-connection/README.md`:
- Around line 1-3: README currently describes a different template and must be
replaced to reflect the actual package and PR purpose: rewrite the README for
the rust-browser-connection module to document the BrowserConnection API/CLI
(public structs/functions/classes and commands), state the single-session
invariant (how BrowserConnection enforces one active session), and include
concrete usage examples for the MCP and Hermes scenarios referenced in PR `#347`
(example CLI commands, minimal code snippets showing session creation/teardown,
error handling and configuration flags). Update the sections noted (lines ~6-7,
213-219, 283-291) to remove template references (e.g.,
"rust-ai-driven-development-pipeline-template" and "example-sum-package-name"),
replace with package name rust-browser-connection, list public entrypoints
(e.g., BrowserConnection::new, BrowserConnection::start_session,
BrowserConnection::stop_session or the actual function/class names used in the
crate), and add a short compatibility/contract note for integrators describing
expected behavior, versioning and migration notes.
In `@packages/rust-browser-connection/scripts/bump-version.rs`:
- Around line 127-128: The CLI `--rust-root` value is never passed into
rust_paths::get_rust_root (it currently calls get_rust_root(None, true)), so the
parsed argument is ignored; change the call to pass the parsed CLI value (the
variable that holds the `--rust-root` option) instead of None so
rust_paths::get_rust_root receives the user-specified path and rust_root is
resolved correctly; update the call site in bump-version.rs (the rust_root
assignment) to use the CLI variable name used when parsing `--rust-root` (e.g.,
the matches/value variable) and preserve the existing second argument (true).
- Around line 100-102: The current replacement uses the Regex stored in re and
blindly replaces the first "version = ..." in content when constructing
new_content, which can match outside the [package] section; update
bump-version.rs to target the package table specifically by either (a) parsing
content with a TOML-aware crate (e.g., toml_edit) and set package.version =
new_version, then write the serialized document back to new_content, or (b)
modify the regex to first locate the "[package]" header and only replace the
"version" key within that block (use a regex/search for
r"(?ms)^\[package\].*?^\s*version\s*=\s*\"[^\"]+\"" and replace the captured
value with new_version). Ensure you still reference new_version and produce
new_content from the modified document/string.
In `@packages/rust-browser-connection/scripts/check-changelog-fragment.rs`:
- Around line 64-77: The current implementation in get_changed_files uses "git
diff --name-only" which returns any modified/removed files; change the git
invocation to only list newly added files by using the diff filter (e.g. "git
diff --diff-filter=A --name-only origin/...") so only files added in the PR are
returned. Update the exec call in get_changed_files to include "--diff-filter=A"
(and likewise replace any other similar git diff invocation later in the file
that gathers changed filenames) so the changelog fragment check only considers
fragments newly added by the PR.
- Around line 80-90: The regex for detecting the "scripts/" path in
is_source_file() currently uses the literal "?scripts" token
(format!(r"^{}?scripts/", ...)) which produces an invalid regex when rust_root
== "." and causes a panic on Regex::new(...). Fix by building the scripts
pattern without the stray '?': ensure prefix already includes or omits the
trailing slash and use format!(r"^{}scripts/", regex::escape(&prefix)) (or
conditionally choose "^scripts/" when prefix is empty) inside the
source_patterns array so Regex::new(...) always receives a valid pattern.
In `@packages/rust-browser-connection/scripts/check-file-size.rs`:
- Around line 90-93: В текущем обходе директорий вызов
filter_map(std::result::Result::ok) подавляет ошибки WalkDir и может вызвать
ложный успех; замените подавление на явную обработку ошибок: при итерации
результата из WalkDir::new(cwd) (в месте с filter_map(Result::ok)) проверяйте
Result и для Err логируйте ошибку (или пишите в stderr) и завершайте процесс с
ненулевым кодом (или возвращайте Err из main), чтобы CI провалил прогон, а для
Ok продолжайте с file entries.
In `@packages/rust-browser-connection/scripts/check-release-needed.rs`:
- Around line 194-199: Функция docker_hub_image_to_check привязывает проверку
наличия Dockerfile к текущей рабочей директории; замените проверку
Path::new("Dockerfile").exists() на проверку относительно rust_root (например
проверять rust_root.join("Dockerfile").exists()) so that dockerhub_required
корректно определяется для multi-root проектов; аналогично исправьте другие
проверки на строках, упомянутых в комментарии (около 316-317) чтобы везде
использовать rust_root при поиске Dockerfile/путей.
- Around line 371-382: The branch where release_is_complete(...) returns true
currently calls set_output("should_release","false") but leaves skip_bump
implicit; update that branch (around the release_is_complete check in
check-release-needed.rs) to also call set_output("skip_bump","true") so both
outputs are always explicitly set when a release is complete, ensuring
determined downstream behavior; keep the existing message and other logic
unchanged.
In `@packages/rust-browser-connection/scripts/check-version-modification.rs`:
- Around line 51-67: The current should_skip_version_check function uses broad
prefixes in automated_branch_prefixes (e.g. "release/" and "automated-release/")
which lets any developer branch named release/* bypass the check; tighten the
whitelist by replacing those generic prefixes with the exact branch names or
strictly-patterned identifiers produced by automation (or a specific regex list)
and update the matching logic that checks head_ref to only skip when head_ref
exactly equals or strictly matches those automated branch identifiers; locate
and modify automated_branch_prefixes and the for-loop that checks head_ref in
should_skip_version_check to implement the stricter matching.
- Around line 111-120: The current has_version_change function uses a regex that
only matches literal version = "" and thus misses typical bumps like 0.2.0 ->
0.2.1; update the Regex in has_version_change to match any added/removed line
that assigns version (i.e., start-of-line + or - followed by optional
whitespace, the token "version", optional whitespace, =, optional whitespace,
then any quoted or unquoted version string or semantic-version characters) so
lines like "+version = \"0.2.1\"" or "- version=0.2.0" are detected; replace the
pattern on the Regex::new call accordingly and keep the rest of
has_version_change unchanged.
In `@packages/rust-browser-connection/scripts/collect-changelog.rs`:
- Around line 84-95: The get_version_from_cargo function currently scans the
entire Cargo.toml with a global regex and may grab a version from another table;
change it to only read the [package] table (or better: parse the file as TOML)
and extract package.version. Concretely, update get_version_from_cargo to either
(a) use a TOML parser to deserialize into a struct and return package.version,
or (b) first locate the "[package]" section and apply the version regex only
within that section; ensure the function still returns Result<String, String>
and keep error messages informative (e.g., "Could not find version in [package]
of Cargo.toml" or include parse errors) while preserving the existing function
name get_version_from_cargo and its signature.
In `@packages/rust-browser-connection/scripts/create-changelog-fragment.rs`:
- Around line 88-113: The code currently uses fs::write on fragment_file which
silently overwrites an existing fragment if the timestamp collides; replace the
direct fs::write call with logic that attempts to create the file using
OpenOptions::new().write(true).create_new(true).open(...) (referencing
fragment_file and fragment_content), and if it fails with AlreadyExists, retry
by appending a short unique suffix or counter to the filename (use timestamp +
bump_type + suffix pattern) until create_new succeeds or a max retry count is
reached; on other errors, log and exit as before. Ensure the directory creation
and error logging around create_dir_all, fragment_file and fragment_content
remain unchanged.
In `@packages/rust-browser-connection/scripts/create-github-release.rs`:
- Around line 325-339: The current check treats any "Validation Failed" as
"already exists", which masks other validation errors; change the conditional
around the output handling (variables output, stderr, stdout, combined) to only
skip when the API indicates a definite "already_exists" condition — e.g., parse
stderr/stdout as JSON and look for errors[].code == "already_exists" or a
message explicitly saying "release already exists" (or check for a specific 422
error with that code), and otherwise treat "Validation Failed" as a real
failure: log the full combined output (stderr/stdout) with eprintln and exit(1).
Ensure you replace the broad combined.contains("Validation Failed") branch in
the release creation logic with this targeted JSON/code check.
In `@packages/rust-browser-connection/scripts/git-config.rs`:
- Line 49: The current println! call prints sensitive PII by outputting the
variables name and email; update the logging at the println! invocation so it no
longer emits raw PII — either log a neutral message like "Configuring git user"
or mask the values before logging (e.g., redact email and name) and keep the
actual name/email usage for the git configuration logic that uses the name and
email variables; modify the println! that references name and email to use the
neutral or masked text instead.
In `@packages/rust-browser-connection/scripts/rust-paths.rs`:
- Around line 208-218: The loop that returns the first publishable crate
(iterating over members and using has_package_section/is_publish_false) is
unsafe because it silently picks one when multiple publishable members exist;
change rust-paths.rs to instead collect all candidate manifests into a Vec (use
the existing manifest variable and predicates), then enforce an invariant: if
vec.len() == 1 return that manifest, if 0 return an Err explaining "no
publishable crate found", and if >1 return an Err asking for an explicit target
(or read a provided env/arg like TARGET_CRATE and validate it against the
collected manifests); ensure errors include the list of candidates to aid the
caller.
- Around line 40-95: get_rust_root currently accepts explicit_root, CLI
--rust-root and RUST_ROOT as-is which allows paths like ../.. or absolute paths
to escape the repo; update get_rust_root to canonicalize and validate any
candidate root (explicit_root, CLI-supplied root, env var, or discovered
"."/"rust") using Path::new(...).canonicalize() and ensure the canonicalized
path is a descendant of the repository root (use
env::current_dir().canonicalize() as the repo root) and reject any path that is
outside that root (return Err). Reference get_rust_root, explicit_root, args
(CLI parsing) and env::var("RUST_ROOT") and perform the
normalization+containment check immediately after selecting a candidate before
returning Ok(...).
In `@packages/rust-browser-connection/scripts/version-and-commit.rs`:
- Around line 351-419: collect_changelog currently gathers markdown fragments
from changelog_dir into files and writes a new entry to changelog_file but never
removes the source fragments, causing repeated release bumps; after successfully
writing the updated changelog in collect_changelog (i.e., after
fs::write(changelog_file, content) succeeds), iterate the same files vector and
remove each fragment file (use fs::remove_file on each Path in files),
handling/remotely logging/remove-file errors gracefully (e.g., ignore non-fatal
errors or log them) so fragments are cleaned up and won't be reprocessed; ensure
you use the existing variables files and changelog_file and only delete after a
successful write.
- Around line 139-143: The current regex (re) replaces any "version ="
occurrence in the file (including dependency tables) and should be changed to
only update the package.version; fix this by parsing and updating the
Cargo.toml's [package] table (e.g., using the toml or toml_edit crate to load
content, set package.version = new_version, then serialize and write to
cargo_toml_path), or alternatively restrict the regex to operate only inside the
[package] section before creating new_content; update the code that computes
new_content (and keep variables content, new_version, cargo_toml_path) so only
the package.version is modified.
In `@packages/rust-browser-connection/src/lib.rs`:
- Around line 81-86: The test calls BrowserConnection::new() which requires a
live Docker daemon; instead construct a pure BrowserConnection value or call a
new test-only constructor so URL helpers can be tested without Docker. Modify
the test to use a lightweight constructor (e.g.,
BrowserConnection::from_base_url or BrowserConnection::with_mock) or directly
instantiate BrowserConnection with fake base values, then call
get_novnc_url("issue-347"), get_cdp_url("issue-347") and assert
is_single_browser_session(&cdp, &novnc); avoid calling BrowserConnection::new()
in the unit test.
- Around line 72-74: Метод is_single_browser_session неправильно считает
совпадение по наличию подстрок; вместо этого распарсьте cdp_url и novnc_url и
убедитесь, что они относятся к одной и той же сессии: сравните их origin (scheme
+ host + port) и/или явные идентификаторы сессии в пути/параметрах, и только
если origin совпадает и cdp_url содержит порт 9223, а novnc_url содержит путь
/vnc.html — возвращайте true; обновите функцию is_single_browser_session, чтобы
использовать URL-парсинг и чёткое сравнение origin/session-id вместо простого
contains.
- Around line 24-42: The port bindings in Config -> HostConfig -> port_bindings
currently bind CDP/noVNC/VNC to "0.0.0.0", which exposes ports 5900, 6080 and
9223 to the network; change the host_ip for the PortBinding entries for
"5900/tcp", "6080/tcp", and "9223/tcp" from "0.0.0.0" to "127.0.0.1" so they are
only accessible locally (update the PortBinding host_ip values where
port_bindings is defined in Config/HostConfig).
In `@packages/rust-browser-connection/src/main.rs`:
- Around line 5-18: The current CLI (struct Args, main and the sum usage)
implements a simple sum calculator which deviates from the PR goal for a
BrowserConnection module; replace or refactor this file to implement the
BrowserConnection entrypoint as specified: remove the ad-hoc Args/sum behavior
and instead wire up the BrowserConnection initializer that enforces the
single-session invariant and exposes the unified MCP/Hermes interface and
backends (noVNC/CDP/VNC) per the spec; ensure clear constructors/traits for
BrowserConnection, session lifecycle enforcement, configuration via CLI/env
parsing matching the spec, add unit/integration tests for single-session and
interface behavior, and update or add docs/comments to document the behavior and
security considerations rather than leaving the sample sum CLI.
In `@packages/rust-browser-connection/tests/unit/ci-cd/mod.rs`:
- Around line 8-10: Replace the broad #[allow(clippy::all, clippy::nursery,
clippy::pedantic, dead_code)] on the version_and_commit module with targeted
allows: remove clippy::all, clippy::nursery and clippy::pedantic and only keep
specific lints that actually appear for mod version_and_commit (for example keep
dead_code if needed, or allow particular lints such as clippy::needless_return
or clippy::missing_docs_in_private_items) — run cargo clippy to see the exact
warnings, then update the attribute to #[allow(...)] listing only those specific
clippy lints and/or dead_code instead of the blanket suppressions so mod
version_and_commit is no longer hiding unrelated clippy issues.
In
`@packages/rust-browser-connection/tests/unit/ci-cd/workspace_manifest_resolution.rs`:
- Around line 143-146: The test
rust_root_cli_parser_returns_none_without_configuration currently removes the
RUST_ROOT env var without restoring it, risking future flakiness; modify the
test to capture the original value of RUST_ROOT (e.g., via std::env::var_os or
var) before calling std::env::remove_var, then ensure the original value is
restored at the end of the test (use a drop guard or a defer-like closure/RAII
pattern) so parse_rust_root_from_args() is executed with RUST_ROOT removed but
the environment is returned to its prior state afterwards.
In `@packages/rust-browser-connection/tests/unit/mod.rs`:
- Around line 1-4: The unit test aggregator currently only includes `mod sum;`
and `mod ci_cd;` but lacks the unit module that verifies BrowserConnection
invariants; add and include a unit test module (e.g., `mod browser_connection;`
or `mod browser_connection_tests;`) that contains tests for the single-session
invariant (test name like `single_session_invariant`), the noVNC/CDP URL
formation helpers (tests for `make_no_vnc_url` and `make_cdp_url`), and
container start error handling (test for `start_container` error paths); update
the aggregator to `mod browser_connection;` so the CI will run these required
tests for issue `#347`.
In `@packages/rust-browser-connection/tests/unit/sum.rs`:
- Line 1: В тесте sum.rs импорт крейта указан как шаблонный плейсхолдер
`example_sum_package_name`, поэтому сборка упадёт; откройте Cargo.toml и
замените этот импорт на фактическое имя библиотечного крейта или, если тест
находится внутри того же крейта, переключитесь на внутренний путь (например
`crate::sum`) в строке, которая импортирует функцию `sum`, чтобы ссылка на
функцию `sum` была корректной.
In `@rust-browser-connection/src/lib.rs`:
- Around line 20-22: start_browser currently builds container_name and creates a
container without enforcing the single-session invariant;
is_single_browser_session only checks URL substrings so concurrent starts and
port conflicts are possible. Fix by making start_browser perform an atomic
existence/creation check: derive the exact container_name (use the same
project_id -> container_name logic), acquire a project-scoped lock (e.g.,
file/OS mutex or in-process async mutex) around the docker inspect/create
sequence, call Docker to inspect for an existing container with that exact name
and if running return its URL instead of creating a new one, otherwise create
the container with that name (letting Docker fail if the name already exists) or
explicitly remove/stop stale containers first; also ensure ports are either
dynamically allocated or checked for availability before binding to avoid
fixed-port conflicts; update is_single_browser_session to match exact
container_name or container label rather than substring URL checks so the
single-session invariant holds.
- Around line 80-83: The test test_urls_and_invariant currently calls
BrowserConnection::new().unwrap(), which depends on the local Docker daemon;
change the unit test to avoid that by constructing a BrowserConnection with a
deterministic base URL (e.g., add/use a constructor like
BrowserConnection::with_base_url or build the struct directly) and then call
BrowserConnection::get_novnc_url("issue-347") to assert the expected URL; move
the Docker-dependent BrowserConnection::new() usage into a separate integration
test that runs with Docker available.
- Around line 27-38: В текущем месте, где задаются PortBinding для портов
"5900/tcp", "6080/tcp" и "9223/tcp", host_ip жестко стоит "0.0.0.0" — нужно
заменить это на безопасный адрес по умолчанию (например "127.0.0.1") и/или
сделать адрес настраиваемым через переменную окружения (напр. BIND_ADDRESS) так,
чтобы вместо "0.0.0.0" в этих вызовах PortBinding использовалось
Some(bind_addr.to_string()); обновите код, который создает эти кортежи
("5900/tcp", ...), ("6080/tcp", ...), ("9223/tcp", ...) и добавьте
документированный конфиг/переменную окружения с безопасным значением по
умолчанию.
---
Outside diff comments:
In `@packages/lib/src/core/browser-connection.ts`:
- Around line 16-131: This file contains duplicate exported declarations
(BrowserError, BrowserConnection, BrowserConnectionLive and two default exports)
and uses forbidden constructs (`unknown`, `as const`, `as void`) and an
undeclared type (ProjectBrowserProxyPath). Consolidate into a single set of
exports: keep one BrowserError class (replace cause: unknown with cause?: Error
| undefined), one BrowserConnection interface (use parseProxyPath:
Effect.Effect<ProjectBrowserProxyPath | null, never>), one BrowserConnectionLive
implementation, and a single default export BrowserConnection; remove the
duplicate blocks and merge the intended runtime stubs
(startBrowser/getCdpUrl/getNoVncUrl/getVncUrl/parseProxyPath/rewriteCdpUrl) into
that single implementation. Replace `as const` and `as void` assertions with
proper typing/returns, and import or declare the ProjectBrowserProxyPath type at
the top so parseProxyPath compiles. Ensure rewriteCdpUrl/getCdpUrl
implementations remain consistent after consolidation.
In
`@packages/rust-browser-connection/docs/case-studies/issue-38/raw-data/issue-38.json`:
- Around line 1-2: Sanitize the case-study raw JSON by removing/obfuscating all
personal user fields inside author and comments.author blocks in the artifact
(e.g., the top-level "author" object and each "comments[].author"); specifically
strip or replace id, login, name, url and any other user-identifiers with
non-PII placeholders (e.g., "anon_user_1") while preserving non-PII fields
needed for analysis (timestamps, body, labels, title, number, state), and update
the code that writes issue-38.json to perform this transformation before saving
so generated artifacts never contain PII and thus satisfy the repository CI
credential/PII scanning rules.
In
`@packages/rust-browser-connection/docs/case-studies/issue-38/template-data/js-template-ci-tree.txt`:
- Around line 1-23: В файле js-template-ci-tree.txt найдено множество
зафиксированных абсолютных путей вида
/tmp/js-ai-driven-development-pipeline-template/..., которые делают артефакт
зависимым от окружения; замените все такие абсолютные пути на
репозиторно-относительные пути (например относительные от корня проекта или
шаблонные маркеры ./ или {repo_root}/), обновите перечисления в файле (каждую
строку начинающуюся с /tmp/js-ai-driven-development-pipeline-template/) чтобы не
включать /tmp префикс, и убедитесь, что после изменения сравнениe шаблон-данных
стабильно между машинами сборки.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ab128a98-5902-48ec-ac98-62d7a529be3c
⛔ Files ignored due to path filters (7)
packages/rust-browser-connection/Cargo.lockis excluded by!**/*.lockpackages/rust-browser-connection/docs/case-studies/issue-19/pr-114-data/solution-draft-log-1.txt.gzis excluded by!**/*.gzpackages/rust-browser-connection/docs/case-studies/issue-19/pr-114-data/solution-draft-log-2.txt.gzis excluded by!**/*.gzpackages/rust-browser-connection/docs/case-studies/issue-38/raw-data/downstream-meta-after-run-24985948212.log.gzis excluded by!**/*.gzpackages/rust-browser-connection/docs/case-studies/issue-38/raw-data/downstream-meta-before-run-24983875003.log.gzis excluded by!**/*.gzpackages/rust-browser-connection/docs/case-studies/issue-38/raw-data/main-run-24465255225.log.gzis excluded by!**/*.gzpackages/rust-browser-connection/docs/case-studies/issue-38/raw-data/pr-run-25212295127.log.gzis excluded by!**/*.gz
📒 Files selected for processing (133)
.hermes/plans/2026-05-23_092139-initial-formal-architecture-plan.md.hermes/plans/2026-05-23_092712-mcp-playwright-hermes-noVNC-integration.md.hermes/plans/2026-05-23_095118-clean-builtin-browser-noVNC-no-mcp.md.hermes/plans/2026-05-23_102144-rust-only-noVNC-browser-module-separate-repo.mdpackages/browser-connection/package.jsonpackages/browser-connection/src/index.tspackages/browser-connection/tsconfig.jsonpackages/lib/src/core/browser-connection.tspackages/rust-browser-connection/.github/workflows/release.ymlpackages/rust-browser-connection/.gitignorepackages/rust-browser-connection/.gitkeeppackages/rust-browser-connection/.pre-commit-config.yamlpackages/rust-browser-connection/CHANGELOG.mdpackages/rust-browser-connection/CONTRIBUTING.mdpackages/rust-browser-connection/Cargo.tomlpackages/rust-browser-connection/LICENSEpackages/rust-browser-connection/README.mdpackages/rust-browser-connection/changelog.d/20251227_224645_changeset_support.mdpackages/rust-browser-connection/changelog.d/20251229_143823_fix_ci_workflow_dependencies.mdpackages/rust-browser-connection/changelog.d/20251231_115800_fix_readme_script_references.mdpackages/rust-browser-connection/changelog.d/20260107_apply_best_practices.mdpackages/rust-browser-connection/changelog.d/20260108_171124_fix_changelog_check.mdpackages/rust-browser-connection/changelog.d/20260108_171435_prevent_manual_version_modification.mdpackages/rust-browser-connection/changelog.d/20260108_apply_lino_objects_codec_fixes.mdpackages/rust-browser-connection/changelog.d/20260111_multi_language_support.mdpackages/rust-browser-connection/changelog.d/20260119_best_practices_from_browser_commander.mdpackages/rust-browser-connection/changelog.d/20260311_translate_scripts_to_rust.mdpackages/rust-browser-connection/changelog.d/20260413-ci-cd-best-practices.mdpackages/rust-browser-connection/changelog.d/20260413_fix_cargo_token_fallback.mdpackages/rust-browser-connection/changelog.d/20260413_fix_crates_io_check.mdpackages/rust-browser-connection/changelog.d/20260413_fix_lookahead_regex.mdpackages/rust-browser-connection/changelog.d/20260414_fix_per_commit_diff.mdpackages/rust-browser-connection/changelog.d/20260415_fix_workspace_release_scripts.mdpackages/rust-browser-connection/changelog.d/20260501_decouple_docs_deploy.mdpackages/rust-browser-connection/changelog.d/20260503_111500_ci_timeouts.mdpackages/rust-browser-connection/changelog.d/20260503_111700_file_size_warning_threshold.mdpackages/rust-browser-connection/changelog.d/20260509_031015_human_readable_release_titles.mdpackages/rust-browser-connection/changelog.d/20260509_205000_docker_hub_release_publishing.mdpackages/rust-browser-connection/changelog.d/20260512_172908_github_pages_artifact_deploy.mdpackages/rust-browser-connection/changelog.d/20260512_230053_document_pages_source_prerequisite.mdpackages/rust-browser-connection/changelog.d/20260515_074404_track_browser_commander_preview_regen.mdpackages/rust-browser-connection/changelog.d/20260515_223000_cargo_lock_release_sync.mdpackages/rust-browser-connection/changelog.d/README.mdpackages/rust-browser-connection/docs/case-studies/issue-11/README.mdpackages/rust-browser-connection/docs/case-studies/issue-11/analysis-crates-io.mdpackages/rust-browser-connection/docs/case-studies/issue-11/analysis-set-output.mdpackages/rust-browser-connection/docs/case-studies/issue-11/analysis-workflow-dispatch.mdpackages/rust-browser-connection/docs/case-studies/issue-11/online-research.mdpackages/rust-browser-connection/docs/case-studies/issue-17/README.mdpackages/rust-browser-connection/docs/case-studies/issue-19/README.mdpackages/rust-browser-connection/docs/case-studies/issue-19/pr-114-data/issue-113-details.txtpackages/rust-browser-connection/docs/case-studies/issue-19/pr-114-data/pr-commits.jsonpackages/rust-browser-connection/docs/case-studies/issue-19/pr-114-data/pr-conversation-comments.jsonpackages/rust-browser-connection/docs/case-studies/issue-19/pr-114-data/pr-details.jsonpackages/rust-browser-connection/docs/case-studies/issue-19/pr-114-data/pr-diff.patchpackages/rust-browser-connection/docs/case-studies/issue-19/pr-114-data/pr-review-comments.jsonpackages/rust-browser-connection/docs/case-studies/issue-19/pr-114-data/pr-reviews.jsonpackages/rust-browser-connection/docs/case-studies/issue-21/README.mdpackages/rust-browser-connection/docs/case-studies/issue-21/browser-commander-issue-27.mdpackages/rust-browser-connection/docs/case-studies/issue-21/browser-commander-issue-29.mdpackages/rust-browser-connection/docs/case-studies/issue-21/browser-commander-issue-31.mdpackages/rust-browser-connection/docs/case-studies/issue-21/browser-commander-issue-33.mdpackages/rust-browser-connection/docs/case-studies/issue-21/browser-commander-rust.ymlpackages/rust-browser-connection/docs/case-studies/issue-25/README.mdpackages/rust-browser-connection/docs/case-studies/issue-29/README.mdpackages/rust-browser-connection/docs/case-studies/issue-32/README.mdpackages/rust-browser-connection/docs/case-studies/issue-34/README.mdpackages/rust-browser-connection/docs/case-studies/issue-38/README.mdpackages/rust-browser-connection/docs/case-studies/issue-38/raw-data/downstream-meta-after-run-24985948212.jsonpackages/rust-browser-connection/docs/case-studies/issue-38/raw-data/downstream-meta-before-run-24983875003.jsonpackages/rust-browser-connection/docs/case-studies/issue-38/raw-data/downstream-meta-ontology-issue-3.jsonpackages/rust-browser-connection/docs/case-studies/issue-38/raw-data/downstream-meta-ontology-pr-4.jsonpackages/rust-browser-connection/docs/case-studies/issue-38/raw-data/issue-38-comments.jsonpackages/rust-browser-connection/docs/case-studies/issue-38/raw-data/issue-38.jsonpackages/rust-browser-connection/docs/case-studies/issue-38/raw-data/js-template-issue-search.jsonpackages/rust-browser-connection/docs/case-studies/issue-38/raw-data/main-run-24465255225.jsonpackages/rust-browser-connection/docs/case-studies/issue-38/raw-data/main-runs.jsonpackages/rust-browser-connection/docs/case-studies/issue-38/raw-data/pr-39-conversation-comments.jsonpackages/rust-browser-connection/docs/case-studies/issue-38/raw-data/pr-39-review-comments.jsonpackages/rust-browser-connection/docs/case-studies/issue-38/raw-data/pr-39-reviews.jsonpackages/rust-browser-connection/docs/case-studies/issue-38/raw-data/pr-39.jsonpackages/rust-browser-connection/docs/case-studies/issue-38/raw-data/pr-branch-runs.jsonpackages/rust-browser-connection/docs/case-studies/issue-38/raw-data/pr-run-25212295127.jsonpackages/rust-browser-connection/docs/case-studies/issue-38/raw-data/rust-template-issue-search.jsonpackages/rust-browser-connection/docs/case-studies/issue-38/template-data/js-template-ci-tree.txtpackages/rust-browser-connection/docs/case-studies/issue-38/template-data/js-template-links.ymlpackages/rust-browser-connection/docs/case-studies/issue-38/template-data/js-template-release.ymlpackages/rust-browser-connection/docs/case-studies/issue-38/template-data/rust-template-ci-tree.txtpackages/rust-browser-connection/docs/case-studies/issue-38/template-data/rust-template-release-after.ymlpackages/rust-browser-connection/docs/case-studies/issue-38/template-data/rust-template-release-before.ymlpackages/rust-browser-connection/docs/case-studies/issue-52/README.mdpackages/rust-browser-connection/docs/case-studies/issue-52/raw-data/issue-52-comments.jsonpackages/rust-browser-connection/docs/case-studies/issue-52/raw-data/issue-52.jsonpackages/rust-browser-connection/docs/case-studies/issue-52/raw-data/js-issue-62.jsonpackages/rust-browser-connection/docs/case-studies/issue-52/raw-data/vk-bot-desktop-issue-51.jsonpackages/rust-browser-connection/docs/case-studies/issue-52/raw-data/vk-bot-desktop-pr-52.jsonpackages/rust-browser-connection/docs/ci-cd/troubleshooting.mdpackages/rust-browser-connection/examples/basic_usage.rspackages/rust-browser-connection/experiments/test-changelog-parsing.rspackages/rust-browser-connection/experiments/test-crates-io-check.rspackages/rust-browser-connection/experiments/test-detect-code-changes.shpackages/rust-browser-connection/experiments/test-version-check-dependencies.shpackages/rust-browser-connection/experiments/test-version-check.shpackages/rust-browser-connection/scripts/bump-version.rspackages/rust-browser-connection/scripts/check-changelog-fragment.rspackages/rust-browser-connection/scripts/check-file-size.rspackages/rust-browser-connection/scripts/check-release-needed.rspackages/rust-browser-connection/scripts/check-version-modification.rspackages/rust-browser-connection/scripts/collect-changelog.rspackages/rust-browser-connection/scripts/create-changelog-fragment.rspackages/rust-browser-connection/scripts/create-github-release.rspackages/rust-browser-connection/scripts/detect-code-changes.rspackages/rust-browser-connection/scripts/get-bump-type.rspackages/rust-browser-connection/scripts/get-version.rspackages/rust-browser-connection/scripts/git-config.rspackages/rust-browser-connection/scripts/publish-crate.rspackages/rust-browser-connection/scripts/rust-paths.rspackages/rust-browser-connection/scripts/version-and-commit.rspackages/rust-browser-connection/scripts/wait-for-crate.rspackages/rust-browser-connection/src/lib.rspackages/rust-browser-connection/src/main.rspackages/rust-browser-connection/src/sum.rspackages/rust-browser-connection/tests/integration/mod.rspackages/rust-browser-connection/tests/integration/sum.rspackages/rust-browser-connection/tests/unit/ci-cd/changelog_parsing.rspackages/rust-browser-connection/tests/unit/ci-cd/mod.rspackages/rust-browser-connection/tests/unit/ci-cd/workflow_release.rspackages/rust-browser-connection/tests/unit/ci-cd/workspace_manifest_resolution.rspackages/rust-browser-connection/tests/unit/mod.rspackages/rust-browser-connection/tests/unit/sum.rspnpm-workspace.yamlrust-browser-connection/Cargo.tomlrust-browser-connection/src/lib.rs
Comment on lines
+35
to
+43
| "devDependencies": { | ||
| "@effect/vitest": "^0.29.0", | ||
| "@types/node": "^25.9.1", | ||
| "typescript": "^6.0.3", | ||
| "vitest": "^4.1.7" | ||
| }, | ||
| "dependencies": { | ||
| "effect": "^3.12.0" | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python - <<'PY'
import json, urllib.request, urllib.parse
packages = {
"`@effect/vitest`": "0.29.0",
"`@types/node`": "25.9.1",
"typescript": "6.0.3",
"vitest": "4.1.7",
"effect": "3.12.0",
}
for name, version in packages.items():
url = "https://registry.npmjs.org/" + urllib.parse.quote(name, safe='@')
with urllib.request.urlopen(url) as r:
data = json.load(r)
exists = version in data.get("versions", {})
print(f"{name}@{version}: {'OK' if exists else 'MISSING'}")
PYRepository: ProverCoderAI/docker-git
Length of output: 2651
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Target file =="
if [ -f "packages/browser-connection/package.json" ]; then
nl -ba packages/browser-connection/package.json | sed -n '1,120p'
else
echo "MISSING: packages/browser-connection/package.json"
fi
echo
echo "== Check for lockfiles near this package =="
ls -la packages/browser-connection 2>/dev/null || true
find packages/browser-connection -maxdepth 2 -type f \( -name 'package-lock.json' -o -name 'pnpm-lock.yaml' -o -name 'yarn.lock' \) -print 2>/dev/null || true
echo
echo "== Check repo root for lockfiles =="
ls -la 2>/dev/null || true
find . -maxdepth 3 -type f \( -name 'package-lock.json' -o -name 'pnpm-lock.yaml' -o -name 'yarn.lock' -o -name 'bun.lockb' \) -print 2>/dev/null || trueRepository: ProverCoderAI/docker-git
Length of output: 147
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check file existence =="
ls -la packages/browser-connection/package.json 2>/dev/null || echo "MISSING: packages/browser-connection/package.json"
echo
echo "== Print first ~120 lines of package.json (if exists) =="
if [ -f "packages/browser-connection/package.json" ]; then
cat -n packages/browser-connection/package.json | sed -n '1,140p'
fi
echo
echo "== List files in packages/browser-connection (maxdepth 2) =="
find packages/browser-connection -maxdepth 2 -type f -print 2>/dev/null || true
echo
echo "== Search for lockfiles near this package =="
find packages/browser-connection -maxdepth 4 -type f \( -name 'package-lock.json' -o -name 'pnpm-lock.yaml' -o -name 'yarn.lock' -o -name 'bun.lockb' \) -print 2>/dev/null || true
echo
echo "== Search for lockfiles in repo (maxdepth 4) =="
find . -maxdepth 4 -type f \( -name 'package-lock.json' -o -name 'pnpm-lock.yaml' -o -name 'yarn.lock' -o -name 'bun.lockb' \) -print 2>/dev/null || trueRepository: ProverCoderAI/docker-git
Length of output: 1965
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Lockfiles anywhere in repo (bun/yarn/pnpm/npm) =="
find . -type f \( -name 'bun.lockb' -o -name 'pnpm-lock.yaml' -o -name 'yarn.lock' -o -name 'package-lock.json' \) -print || true
echo
echo "== git diff for target package.json (if git history present) =="
git diff -- packages/browser-connection/package.json || true
echo
echo "== Search other package.json files for packageManager/bun/pnpm =="
rg -n "\"packageManager\"|pnpm-lock.yaml|bun.lockb" --glob 'packages/**/package.json' . || trueRepository: ProverCoderAI/docker-git
Length of output: 612
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Root README(s) =="
ls -la README* 2>/dev/null || true
find . -maxdepth 3 -type f -name 'README*.md' -print 2>/dev/null || true
echo
echo "== Search in README/MD for lockfile / pinning policy =="
rg -n --hidden -S "bun\.lockb|lockfile|pin(n|ning)|^\\^|caret|reproduc|reproducib|dependency versions|version policy" --glob '**/*.md' . || true
echo
echo "== Search for publishing steps (npm publish / bun publish) =="
rg -n --hidden -S "npm publish|pnpm publish|yarn publish|bun publish|publishConfig|prepack|files\" *: *\\[ *\"dist\"\\]|`@prover-coder-ai/browser-connection`" . || true
echo
echo "== Search for bun.lockb mentions in non-MD files =="
rg -n --hidden -S "bun\.lockb" . || trueRepository: ProverCoderAI/docker-git
Length of output: 11398
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Find lockfiles anywhere in repo (no maxdepth) =="
find . -type f \( -name 'bun.lockb' -o -name 'pnpm-lock.yaml' -o -name 'yarn.lock' -o -name 'package-lock.json' -o -name 'npm-shrinkwrap.json' \) -print || true
echo
echo "== Inspect release workflow publish section =="
if [ -f ".github/workflows/release.yml" ]; then
sed -n '330,460p' .github/workflows/release.yml
else
echo "MISSING: .github/workflows/release.yml"
fiRepository: ProverCoderAI/docker-git
Length of output: 5660
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== git status --porcelain =="
git status --porcelain || true
echo
echo "== git diff --stat =="
git diff --stat || true
echo
echo "== git diff --name-only =="
git diff --name-only || true
echo
echo "== git diff packages/browser-connection/package.json (first 40 lines) =="
git diff -- packages/browser-connection/package.json | head -n 40 || trueRepository: ProverCoderAI/docker-git
Length of output: 222
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== git status --porcelain =="
git status --porcelain || true
echo
echo "== git diff --stat =="
git diff --stat || true
echo
echo "== git diff --name-only =="
git diff --name-only || true
echo
echo "== git diff packages/browser-connection/package.json (first 40 lines) =="
git diff -- packages/browser-connection/package.json | head -n 40 || trueRepository: ProverCoderAI/docker-git
Length of output: 222
Зафиксируйте версии зависимостей в публикуемом пакете @prover-coder-ai/browser-connection
В packages/browser-connection/package.json зависимости указаны с caret-диапазонами (^), например effect, typescript, vitest, что ухудшает воспроизводимость и расширяет supply-chain поверхность для потребителей (публикация идёт через bun publish без lockfile-фиксации).
Предлагаемое изменение
"devDependencies": {
- "`@effect/vitest`": "^0.29.0",
- "`@types/node`": "^25.9.1",
- "typescript": "^6.0.3",
- "vitest": "^4.1.7"
+ "`@effect/vitest`": "0.29.0",
+ "`@types/node`": "25.9.1",
+ "typescript": "6.0.3",
+ "vitest": "4.1.7"
},
"dependencies": {
- "effect": "^3.12.0"
+ "effect": "3.12.0"
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "devDependencies": { | |
| "@effect/vitest": "^0.29.0", | |
| "@types/node": "^25.9.1", | |
| "typescript": "^6.0.3", | |
| "vitest": "^4.1.7" | |
| }, | |
| "dependencies": { | |
| "effect": "^3.12.0" | |
| } | |
| "devDependencies": { | |
| "`@effect/vitest`": "0.29.0", | |
| "`@types/node`": "25.9.1", | |
| "typescript": "6.0.3", | |
| "vitest": "4.1.7" | |
| }, | |
| "dependencies": { | |
| "effect": "3.12.0" | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/browser-connection/package.json` around lines 35 - 43, В
package.json пакета `@prover-coder-ai/browser-connection` зафиксируйте версии
зависимостей — замените caret-диапазоны в полях "dependencies" и
"devDependencies" на конкретные версии (удалите "^" у effect, typescript,
vitest, `@effect/vitest`, `@types/node` и т.д.) чтобы публикуемый пакет через bun
publish содержал точные версии и не расширял supply-chain поверхность для
потребителей.
Comment on lines
+3
to
+15
| export class BrowserError { | ||
| readonly _tag = "BrowserError" as const | ||
| constructor(readonly message: string, readonly cause?: unknown) {} | ||
| } | ||
|
|
||
| export interface BrowserConnection { | ||
| readonly startBrowser: (projectId: string) => Effect.Effect<void, BrowserError> | ||
| readonly getCdpUrl: (projectId: string) => Effect.Effect<string, BrowserError> | ||
| readonly getNoVncUrl: (projectId: string) => Effect.Effect<string, BrowserError> | ||
| readonly getVncUrl: (projectId: string) => Effect.Effect<string, BrowserError> | ||
| readonly parseProxyPath: (pathname: string) => Effect.Effect<unknown, never> | ||
| readonly rewriteCdpUrl: (value: string, externalOrigin: string, projectId: string) => string | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and display the target file with line numbers (bounded output)
FILE="packages/browser-connection/src/index.ts"
if [ -f "$FILE" ]; then
echo "=== $FILE (first 200 lines) ==="
nl -ba "$FILE" | sed -n '1,200p'
else
echo "File not found: $FILE" >&2
exit 1
fi
# Show any other occurrences of BrowserError / parseProxyPath / rewriteCdpUrl in the repo
rg -n --hidden --no-ignore -S "class BrowserError|interface BrowserConnection|parseProxyPath|rewriteCdpUrl" .Repository: ProverCoderAI/docker-git
Length of output: 196
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="packages/browser-connection/src/index.ts"
echo "=== existence/state ==="
ls -la "$FILE" 2>/dev/null || { echo "File not found: $FILE" >&2; exit 1; }
wc -l "$FILE"
echo
echo "=== content (up to 200 lines) ==="
# cat -n is generally available; fallback to sed
if command -v cat >/dev/null 2>&1; then
cat -n "$FILE" | sed -n '1,200p'
else
sed -n '1,200p' "$FILE"
fi
echo
echo "=== repo search for key symbols ==="
rg -n --hidden --no-ignore -S "class BrowserError|interface BrowserConnection|parseProxyPath|rewriteCdpUrl" .Repository: ProverCoderAI/docker-git
Length of output: 4893
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="packages/lib/src/core/browser-connection.ts"
ls -la "$FILE"
wc -l "$FILE"
echo "=== content (first 180 lines) ==="
sed -n '1,180p' "$FILE" | nl -ba | sed -n '1,180p'
echo "=== search for key type names ==="
rg -n "ProjectBrowserProxyPath|parseProxyPath|rewriteCdpUrl|BrowserError|_tag" "$FILE"Repository: ProverCoderAI/docker-git
Length of output: 304
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="packages/lib/src/core/browser-connection.ts"
echo "=== content (first 200 lines) ==="
cat -n "$FILE" | sed -n '1,220p'
echo "=== search for key symbols/types in core file ==="
rg -n --hidden --no-ignore -S "export class BrowserError|interface BrowserConnection|ProjectBrowserProxyPath|parseProxyPath|rewriteCdpUrl|_tag" "$FILE"Repository: ProverCoderAI/docker-git
Length of output: 8922
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n --hidden --no-ignore -S "type ProjectBrowserProxyPath|interface ProjectBrowserProxyPath|enum ProjectBrowserProxyPath|export (type|interface|enum) ProjectBrowserProxyPath" packages/lib/src/core/browser-connection.ts
rg -n --hidden --no-ignore -S "ProjectBrowserProxyPath" packages/lib/src/core/browser-connection.tsRepository: ProverCoderAI/docker-git
Length of output: 50
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="packages/lib/src/core/browser-connection.ts"
echo "=== imports / top (1-40) ==="
sed -n '1,60p' "$FILE" | cat -n
echo "=== around second BrowserConnection interface (60-110) ==="
sed -n '60,120p' "$FILE" | cat -n
echo "=== all occurrences of 'ProjectBrowserProxyPath' in the file ==="
rg -n --hidden --no-ignore "ProjectBrowserProxyPath" "$FILE" || true
echo "=== search repo for ProjectBrowserProxyPath definition ==="
rg -n --hidden --no-ignore "ProjectBrowserProxyPath" packages/lib/src/core packages/lib/src -SRepository: ProverCoderAI/docker-git
Length of output: 8072
Исправить публичный TS-контракт BrowserConnection в packages/browser-connection/src/index.ts: убрать as/unknown из API
BrowserError:_tag = "BrowserError" as constиcause?: unknown(строки 4-5) нарушают запреты наasиunknownв типах контракта.startBrowser:return undefined as void(строки 23-27) — запрещённыйasв обычном коде.parseProxyPath:Effect.Effect<unknown, never>(строка 13) и реализацияEffect.succeed(null)(строка 31) должны быть типизированы доменно (согласовать с CORE, гдеparseProxyPathобъявлен какEffect.Effect<ProjectBrowserProxyPath | null, never>).- Экспортируемые сущности (
BrowserError,BrowserConnection) не имеют обязательных TSDoc/functional comment markers.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/browser-connection/src/index.ts` around lines 3 - 15, Fix the public
TS contract by removing "as" casts and "unknown" from exported types: change
BrowserError to use a plain string literal tag (no "as const") and give cause a
concrete domain/error type (not unknown), update
BrowserConnection.parseProxyPath to return Effect.Effect<ProjectBrowserProxyPath
| null, never> (and replace Effect.succeed(null) with the typed success), and
stop using "as void" in startBrowser implementation—return a proper Effect like
Effect.unit or Effect.succeedVoid instead; also add minimal TSDoc/functional
comment markers above the exported BrowserError and BrowserConnection to
document the public API. Ensure you reference and update the symbols
BrowserError, BrowserConnection, startBrowser, and parseProxyPath in the file.
Comment on lines
+3
to
+47
| export class BrowserError { | ||
| readonly _tag = "BrowserError" as const | ||
| constructor(readonly message: string, readonly cause?: unknown) {} | ||
| } | ||
|
|
||
| export interface BrowserConnection { | ||
| readonly startBrowser: (projectId: string) => Effect.Effect<void, BrowserError> | ||
| readonly getCdpUrl: (projectId: string) => Effect.Effect<string, BrowserError> | ||
| readonly getNoVncUrl: (projectId: string) => Effect.Effect<string, BrowserError> | ||
| readonly getVncUrl: (projectId: string) => Effect.Effect<string, BrowserError> | ||
| readonly parseProxyPath: (pathname: string) => Effect.Effect<unknown, never> | ||
| readonly rewriteCdpUrl: (value: string, externalOrigin: string, projectId: string) => string | ||
| } | ||
|
|
||
| export const BrowserConnection = Context.GenericTag<BrowserConnection>("@prover-coder-ai/browser-connection/BrowserConnection") | ||
|
|
||
| export const BrowserConnectionLive = Layer.effect( | ||
| BrowserConnection, | ||
| Effect.gen(function* () { | ||
| return { | ||
| startBrowser: (projectId: string) => | ||
| Effect.gen(function* () { | ||
| yield* Effect.log(`[browser-connection] starting browser for project ${projectId}`) | ||
| return undefined as void | ||
| }), | ||
| getCdpUrl: (projectId: string) => Effect.succeed(`http://localhost:9223?project=${projectId}`), | ||
| getNoVncUrl: (projectId: string) => Effect.succeed(`/b/${projectId}/vnc.html?autoconnect=true&resize=remote&path=b/${projectId}/websockify`), | ||
| getVncUrl: (projectId: string) => Effect.succeed(`vnc://localhost:5900`), | ||
| parseProxyPath: (_pathname: string) => Effect.succeed(null), | ||
| rewriteCdpUrl: (value: string, _externalOrigin: string, _projectId: string) => value | ||
| } | ||
| }) | ||
| ) | ||
|
|
||
| // Pure helpers | ||
| export const renderNoVncUrl = (projectId: string): string => | ||
| `/b/${projectId}/vnc.html?autoconnect=true&resize=remote&path=b/${projectId}/websockify` | ||
|
|
||
| export const renderCdpUrl = (projectId: string): string => | ||
| `http://localhost:9223/json/version?project=${projectId}` | ||
|
|
||
| export const isSingleBrowserSession = (cdpUrl: string, noVncUrl: string): boolean => | ||
| cdpUrl.includes("9223") && noVncUrl.includes("/vnc.html") | ||
|
|
||
| export default BrowserConnection |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="packages/browser-connection/src/index.ts"
if [ ! -f "$FILE" ]; then
echo "Missing file: $FILE" >&2
exit 1
fi
echo "== File: $FILE =="
wc -l "$FILE"
echo
echo "== Exported symbols + nearby lines (1-200) =="
nl -ba "$FILE" | sed -n '1,200p'
echo
echo "== TSDoc marker occurrences =="
rg -n "CHANGE|WHY|QUOTE\\(ТЗ\\)|REF\\b|SOURCE\\b|FORMAT THEOREM|PURITY\\b|EFFECT\\b|INVARIANT\\b|COMPLEXITY\\b|`@pure`\\b|`@effect`\\b|`@invariant`\\b|`@precondition`\\b|`@postcondition`\\b|`@complexity`\\b|`@throws`\\b|`@invariant`\\b" "$FILE" || true
echo
echo "== Exported declarations without leading /** =="
python3 - <<'PY'
import re, pathlib
p = pathlib.Path("packages/browser-connection/src/index.ts")
s = p.read_text(encoding='utf-8')
# Heuristic: exported ... lines and ensure preceding non-empty line is /** ... */
lines = s.splitlines()
export_re = re.compile(r'^\s*(export\s+(default\s+)?(class|interface|const|function|type|enum)\b|export\s*{\b)')
# For each export line, check last preceding line that's non-empty.
for i, line in enumerate(lines, start=1):
if export_re.search(line):
j=i-1
while j>=1 and lines[j-1].strip()=="":
j-=1
prev=lines[j-1] if j-1>=0 else ""
# if prev doesn't start with /** or *
ok = prev.strip().startswith("*/") or prev.strip().startswith("*") or prev.strip().startswith("/**")
# If prev starts with "*" or "/**" it's likely part of block.
# If immediately previous non-empty line isn't doc, report.
if not prev.strip().startswith("*/") and not prev.strip().startswith("*") and not prev.strip().startswith("/**"):
print(f"{i}: {line.strip()}")
PYRepository: ProverCoderAI/docker-git
Length of output: 274
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="packages/browser-connection/src/index.ts"
if [ ! -f "$FILE" ]; then
echo "Missing file: $FILE" >&2
exit 1
fi
echo "== File: $FILE =="
wc -l "$FILE"
echo
echo "== File excerpt (1-120) =="
sed -n '1,120p' "$FILE" | cat -n
echo
echo "== TSDoc/marker occurrences =="
rg -n "CHANGE|WHY|QUOTE\\(ТЗ\\)|REF\\b|SOURCE\\b|FORMAT THEOREM|PURITY\\b|EFFECT\\b|INVARIANT\\b|COMPLEXITY\\b|`@pure`\\b|`@effect`\\b|`@invariant`\\b|`@precondition`\\b|`@postcondition`\\b|`@complexity`\\b|`@throws`\\b|`@throws`\\b" "$FILE" || true
echo
echo "== Exported declarations likely missing leading /** =="
python3 - <<'PY'
import re, pathlib
p = pathlib.Path("packages/browser-connection/src/index.ts")
s = p.read_text(encoding='utf-8')
lines = s.splitlines()
export_re = re.compile(r'^\s*export\s+(default\s+)?(class|interface|const|function|type|enum)\b')
# also handle export default BrowserConnection; not relevant
results=[]
for i,line in enumerate(lines, start=1):
if export_re.search(line):
j=i-1
while j>=2 and lines[j-2].strip()=="":
j-=1
prev = lines[j-2] if j-2>=0 else ""
if not (prev.strip().startswith("/**") or prev.strip().startswith("*") or prev.strip().startswith("*/")):
results.append((i,line.strip()))
for i,line in results:
print(f"{i}: {line}")
PYRepository: ProverCoderAI/docker-git
Length of output: 3247
Не хватает обязательной функциональной TSDoc для экспортируемых типов и helper-функций (packages/browser-connection/src/index.ts)
Для BrowserError, BrowserConnection, BrowserConnectionLive, а также renderNoVncUrl, renderCdpUrl, isSingleBrowserSession отсутствует требуемая функциональная документация и маркеры (CHANGE/WHY/INVARIANT/COMPLEXITY/PURITY/EFFECT и т.д.) в TSDoc-формате по coding guidelines.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/browser-connection/src/index.ts` around lines 3 - 47, Add required
functional TSDoc comments for all exported symbols: BrowserError,
BrowserConnection, BrowserConnectionLive, renderNoVncUrl, renderCdpUrl, and
isSingleBrowserSession. For each exported type/class/function place a TSDoc
block immediately above its declaration and include at minimum: a short
description, and the required markers such as `@CHANGE` or `@WHY` (purpose),
`@INVARIANT` (behavior guarantees), `@COMPLEXITY` (complexity/cost), and
`@PURITY/`@EFFECT as applicable; for BrowserConnectionLive document side-effects
and environment wiring with `@EFFECT`, for helpers document purity with `@PURITY`
and examples/usage where helpful. Ensure tags and descriptions follow the
project coding-guidelines format and appear for every exported symbol listed.
Comment on lines
+23
to
+33
| startBrowser: (projectId: string) => | ||
| Effect.gen(function* () { | ||
| yield* Effect.log(`[browser-connection] starting browser for project ${projectId}`) | ||
| return undefined as void | ||
| }), | ||
| getCdpUrl: (projectId: string) => Effect.succeed(`http://localhost:9223?project=${projectId}`), | ||
| getNoVncUrl: (projectId: string) => Effect.succeed(`/b/${projectId}/vnc.html?autoconnect=true&resize=remote&path=b/${projectId}/websockify`), | ||
| getVncUrl: (projectId: string) => Effect.succeed(`vnc://localhost:5900`), | ||
| parseProxyPath: (_pathname: string) => Effect.succeed(null), | ||
| rewriteCdpUrl: (value: string, _externalOrigin: string, _projectId: string) => value | ||
| } |
There was a problem hiding this comment.
BrowserConnectionLive сейчас не реализует заявленный runtime-контракт.
startBrowser не поднимает сессию, parseProxyPath всегда null, rewriteCdpUrl игнорирует аргументы, getVncUrl не учитывает projectId. Это расходится с целью PR про управляемое подключение к единственной browser-сессии.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/browser-connection/src/index.ts` around lines 23 - 33,
BrowserConnectionLive currently doesn't implement the runtime contract:
implement startBrowser to actually start/record a single browser session for the
given projectId (create/store session state and log errors instead of returning
undefined), implement parseProxyPath to parse the incoming pathname and return a
project id or session descriptor instead of always null, make rewriteCdpUrl use
the passed externalOrigin and projectId to rewrite the CDP URL correctly (not
just return value), and make getVncUrl include projectId (and any per-project
port or path mapping) rather than returning a constant; update the
implementations in BrowserConnectionLive (startBrowser, parseProxyPath,
rewriteCdpUrl, getVncUrl) so they coordinate with whatever session
storage/registry the runtime uses to ensure a single managed browser session per
project.
|
|
||
| [dependencies] | ||
| bollard = "0.16" | ||
| tokio = { version = "1", features = ["full"] } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Проверка: какие части tokio реально используются в коде crate
rg -n --type rust -C2 '\btokio::' packages/rust-browser-connection/src packages/rust-browser-connection/tests packages/rust-browser-connection/examples
# Проверка текущего объявления зависимости
rg -n '^\s*tokio\s*=' packages/rust-browser-connection/Cargo.tomlRepository: ProverCoderAI/docker-git
Length of output: 118
🏁 Script executed:
#!/bin/bash
set -euo pipefail
CRATE_DIR="packages/rust-browser-connection"
echo "== Tree (top-level) =="
ls -la "$CRATE_DIR"
echo "== Cargo.toml tokio line =="
rg -n '^\s*tokio\s*=' "$CRATE_DIR/Cargo.toml" || true
echo "== Any tokio mention in crate (broad) =="
rg -n --type rust '\btokio\b' "$CRATE_DIR/src" "$CRATE_DIR/tests" "$CRATE_DIR/examples" 2>/dev/null || true
echo "== Any tokio:: usage (narrow) =="
rg -n --type rust '\btokio::' "$CRATE_DIR/src" "$CRATE_DIR/tests" "$CRATE_DIR/examples" 2>/dev/null || true
echo "== Any attribute macro usage #[tokio::...] =="
rg -n --type rust '#\s*\[\s*tokio::' "$CRATE_DIR/src" "$CRATE_DIR/tests" "$CRATE_DIR/examples" 2>/dev/null || true
echo "== Any 'use tokio::' imports =="
rg -n --type rust 'use\s+tokio::' "$CRATE_DIR/src" "$CRATE_DIR/tests" "$CRATE_DIR/examples" 2>/dev/null || true
echo "== Cargo features for tokio (resolved features not available, but Cargo.toml can show overrides) =="
rg -n 'tokio\s*=\s*\{|\[features\]|\[dependencies\]' "$CRATE_DIR/Cargo.toml" || trueRepository: ProverCoderAI/docker-git
Length of output: 1726
🏁 Script executed:
#!/bin/bash
set -euo pipefail
CRATE_DIR="packages/rust-browser-connection"
echo "== Files under src =="
find "$CRATE_DIR/src" -maxdepth 2 -type f -print
echo "== Files under tests =="
find "$CRATE_DIR/tests" -maxdepth 2 -type f -print || true
echo "== Files under examples =="
find "$CRATE_DIR/examples" -maxdepth 2 -type f -print || true
echo "== Show src/lib.rs around tokio usage =="
if [ -f "$CRATE_DIR/src/lib.rs" ]; then
nl -ba "$CRATE_DIR/src/lib.rs" | sed -n '1,120p'
fi
echo "== Show any files containing 'tokio::' or 'tokio ' (non-import) =="
rg -n --type rust '\btokio::' "$CRATE_DIR/src" "$CRATE_DIR/tests" "$CRATE_DIR/examples" || true
rg -n --type rust 'tokio\b' "$CRATE_DIR/src" "$CRATE_DIR/tests" "$CRATE_DIR/examples" || trueRepository: ProverCoderAI/docker-git
Length of output: 652
🏁 Script executed:
#!/bin/bash
set -euo pipefail
CRATE_DIR="packages/rust-browser-connection"
echo "== Show src/lib.rs (first 200 lines) =="
if [ -f "$CRATE_DIR/src/lib.rs" ]; then
sed -n '1,200p' "$CRATE_DIR/src/lib.rs" | cat -n
fi
echo "== Show src/main.rs (first 200 lines) =="
if [ -f "$CRATE_DIR/src/main.rs" ]; then
sed -n '1,200p' "$CRATE_DIR/src/main.rs" | cat -n
fi
echo "== Show any tokio:: usage (narrow) =="
rg -n --type rust '\btokio::' "$CRATE_DIR/src" "$CRATE_DIR/tests" "$CRATE_DIR/examples" || true
echo "== Show any tokio macro/attribute usage =="
rg -n --type rust '#\s*\[\s*tokio' "$CRATE_DIR/src" "$CRATE_DIR/tests" "$CRATE_DIR/examples" || true
echo "== Show any `use tokio` imports =="
rg -n --type rust 'use\s+tokio::' "$CRATE_DIR/src" "$CRATE_DIR/tests" "$CRATE_DIR/examples" || true
echo "== Show any `tokio ` (mentions) in lib.rs specifically =="
rg -n --type rust '\btokio\b' "$CRATE_DIR/src/lib.rs" || trueRepository: ProverCoderAI/docker-git
Length of output: 4731
Удалите tokio/full или уберите зависимость tokio, т.к. она не используется
В packages/rust-browser-connection/src/lib.rs есть только use tokio;, при этом нет ни tokio::..., ни #[tokio::...]/use tokio::... импортов. Поэтому tokio = { version = "1", features = ["full"] } расширяет dependency-поверхность без подтверждённой необходимости.
tokio = { version = "1", features = ["full"] }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/rust-browser-connection/Cargo.toml` at line 21, Remove the unused
Tokio dependency entry "tokio = { version = \"1\", features = [\"full\"] }" from
Cargo.toml (or if you plan to use Tokio, replace it with a minimal feature set)
and clean up the unused import "use tokio;" in src/lib.rs so the crate does not
declare an unnecessary dependency; ensure no other code references tokio::... or
#[tokio::...] before removing.
Comment on lines
+1
to
+4
| mod sum; | ||
|
|
||
| #[path = "ci-cd/mod.rs"] | ||
| mod ci_cd; |
There was a problem hiding this comment.
В unit-агрегаторе нет тестов для BrowserConnection-инвариантов из ТЗ.
Подключены только sum и ci_cd, но не видно unit-модуля, который проверяет заявленные в PR свойства (single-session invariant, формирование noVNC/CDP URL, ошибки старта контейнера). Для issue #347 это обязательный проверяемый контракт.
As per coding guidelines, "Флагай ... отсутствие тестов для заявленного поведения ..."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/rust-browser-connection/tests/unit/mod.rs` around lines 1 - 4, The
unit test aggregator currently only includes `mod sum;` and `mod ci_cd;` but
lacks the unit module that verifies BrowserConnection invariants; add and
include a unit test module (e.g., `mod browser_connection;` or `mod
browser_connection_tests;`) that contains tests for the single-session invariant
(test name like `single_session_invariant`), the noVNC/CDP URL formation helpers
(tests for `make_no_vnc_url` and `make_cdp_url`), and container start error
handling (test for `start_container` error paths); update the aggregator to `mod
browser_connection;` so the CI will run these required tests for issue `#347`.
| @@ -0,0 +1,26 @@ | |||
| use example_sum_package_name::sum; | |||
There was a problem hiding this comment.
Неверный путь импорта крейта в тесте
example_sum_package_name выглядит как шаблонный плейсхолдер; в таком виде тесты, вероятно, не соберутся. Замените импорт на фактическое имя library-крейта из Cargo.toml (или используйте crate::..., если уместно для структуры тестов).
Предлагаемая правка
-use example_sum_package_name::sum;
+use rust_browser_connection::sum;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| use example_sum_package_name::sum; | |
| use rust_browser_connection::sum; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/rust-browser-connection/tests/unit/sum.rs` at line 1, В тесте sum.rs
импорт крейта указан как шаблонный плейсхолдер `example_sum_package_name`,
поэтому сборка упадёт; откройте Cargo.toml и замените этот импорт на фактическое
имя библиотечного крейта или, если тест находится внутри того же крейта,
переключитесь на внутренний путь (например `crate::sum`) в строке, которая
импортирует функцию `sum`, чтобы ссылка на функцию `sum` была корректной.
Comment on lines
+20
to
+22
| pub async fn start_browser(&self, project_id: &str) -> Result<String> { | ||
| let container_name = format!("dg-{}-browser", project_id.replace('/', '-')); | ||
|
|
There was a problem hiding this comment.
Инвариант single-session не обеспечен, только декларируется.
start_browser создает контейнер по project_id, а is_single_browser_session проверяет лишь подстроки URL. Это не предотвращает конкурирующие запуски и конфликты по фиксированным портам, хотя PR заявляет single-session поведение.
Also applies to: 50-58, 71-73
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@rust-browser-connection/src/lib.rs` around lines 20 - 22, start_browser
currently builds container_name and creates a container without enforcing the
single-session invariant; is_single_browser_session only checks URL substrings
so concurrent starts and port conflicts are possible. Fix by making
start_browser perform an atomic existence/creation check: derive the exact
container_name (use the same project_id -> container_name logic), acquire a
project-scoped lock (e.g., file/OS mutex or in-process async mutex) around the
docker inspect/create sequence, call Docker to inspect for an existing container
with that exact name and if running return its URL instead of creating a new
one, otherwise create the container with that name (letting Docker fail if the
name already exists) or explicitly remove/stop stale containers first; also
ensure ports are either dynamically allocated or checked for availability before
binding to avoid fixed-port conflicts; update is_single_browser_session to match
exact container_name or container label rather than substring URL checks so the
single-session invariant holds.
Comment on lines
+27
to
+38
| ("5900/tcp".to_string(), Some(vec![bollard::models::PortBinding { | ||
| host_ip: Some("0.0.0.0".to_string()), | ||
| host_port: Some("5900".to_string()), | ||
| }])), | ||
| ("6080/tcp".to_string(), Some(vec![bollard::models::PortBinding { | ||
| host_ip: Some("0.0.0.0".to_string()), | ||
| host_port: Some("6080".to_string()), | ||
| }])), | ||
| ("9223/tcp".to_string(), Some(vec![bollard::models::PortBinding { | ||
| host_ip: Some("0.0.0.0".to_string()), | ||
| host_port: Some("9223".to_string()), | ||
| }])), |
There was a problem hiding this comment.
Не открывайте VNC/noVNC/CDP на 0.0.0.0 по умолчанию.
Сейчас сервисы удаленного управления браузером становятся доступными на всех интерфейсах хоста. Это избыточно и создает прямой вектор неавторизованного доступа.
💡 Предлагаемая правка
- host_ip: Some("0.0.0.0".to_string()),
+ host_ip: Some("127.0.0.1".to_string()),
@@
- host_ip: Some("0.0.0.0".to_string()),
+ host_ip: Some("127.0.0.1".to_string()),
@@
- host_ip: Some("0.0.0.0".to_string()),
+ host_ip: Some("127.0.0.1".to_string()),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ("5900/tcp".to_string(), Some(vec![bollard::models::PortBinding { | |
| host_ip: Some("0.0.0.0".to_string()), | |
| host_port: Some("5900".to_string()), | |
| }])), | |
| ("6080/tcp".to_string(), Some(vec![bollard::models::PortBinding { | |
| host_ip: Some("0.0.0.0".to_string()), | |
| host_port: Some("6080".to_string()), | |
| }])), | |
| ("9223/tcp".to_string(), Some(vec![bollard::models::PortBinding { | |
| host_ip: Some("0.0.0.0".to_string()), | |
| host_port: Some("9223".to_string()), | |
| }])), | |
| ("5900/tcp".to_string(), Some(vec![bollard::models::PortBinding { | |
| host_ip: Some("127.0.0.1".to_string()), | |
| host_port: Some("5900".to_string()), | |
| }])), | |
| ("6080/tcp".to_string(), Some(vec![bollard::models::PortBinding { | |
| host_ip: Some("127.0.0.1".to_string()), | |
| host_port: Some("6080".to_string()), | |
| }])), | |
| ("9223/tcp".to_string(), Some(vec![bollard::models::PortBinding { | |
| host_ip: Some("127.0.0.1".to_string()), | |
| host_port: Some("9223".to_string()), | |
| }])), |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@rust-browser-connection/src/lib.rs` around lines 27 - 38, В текущем месте,
где задаются PortBinding для портов "5900/tcp", "6080/tcp" и "9223/tcp", host_ip
жестко стоит "0.0.0.0" — нужно заменить это на безопасный адрес по умолчанию
(например "127.0.0.1") и/или сделать адрес настраиваемым через переменную
окружения (напр. BIND_ADDRESS) так, чтобы вместо "0.0.0.0" в этих вызовах
PortBinding использовалось Some(bind_addr.to_string()); обновите код, который
создает эти кортежи ("5900/tcp", ...), ("6080/tcp", ...), ("9223/tcp", ...) и
добавьте документированный конфиг/переменную окружения с безопасным значением по
умолчанию.
Comment on lines
+80
to
+83
| #[test] | ||
| fn test_urls_and_invariant() { | ||
| let conn = BrowserConnection::new().unwrap(); | ||
| let novnc = conn.get_novnc_url("issue-347"); |
There was a problem hiding this comment.
Unit-тест зависит от локального Docker daemon и будет флапать в CI.
BrowserConnection::new().unwrap() делает тест инфраструктурно-зависимым. Для unit-уровня лучше тестировать pure URL-логику без подключения к Docker, а Docker-сценарий вынести в integration-тест.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@rust-browser-connection/src/lib.rs` around lines 80 - 83, The test
test_urls_and_invariant currently calls BrowserConnection::new().unwrap(), which
depends on the local Docker daemon; change the unit test to avoid that by
constructing a BrowserConnection with a deterministic base URL (e.g., add/use a
constructor like BrowserConnection::with_base_url or build the struct directly)
and then call BrowserConnection::get_novnc_url("issue-347") to assert the
expected URL; move the Docker-dependent BrowserConnection::new() usage into a
separate integration test that runs with Docker available.
| // ==================== CORE INTERFACE (pure, mathematical) ==================== | ||
|
|
||
| export class BrowserError { | ||
| readonly _tag = "BrowserError" as const |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="packages/lib/src/core/browser-connection.ts"
# Show file with line numbers around the mentioned lines
nl -ba "$FILE" | sed -n '1,140p'
# Also search for specific forbidden constructs in this file
rg -n --fixed-string " as " "$FILE" || true
rg -n --fixed-string "unknown" "$FILE" || true
rg -n --fixed-string "as const" "$FILE" || true
rg -n --fixed-string "as void" "$FILE" || trueRepository: ProverCoderAI/docker-git
Length of output: 111
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="packages/lib/src/core/browser-connection.ts"
# Print first 140 lines with line numbers
cat -n "$FILE" | sed -n '1,140p'
echo "---- rg: 'unknown' ----"
rg -n --fixed-string "unknown" "$FILE" || true
echo "---- rg: 'as const' ----"
rg -n --fixed-string "as const" "$FILE" || true
echo "---- rg: 'as void' ----"
rg -n --fixed-string "as void" "$FILE" || true
echo "---- rg: pattern for ' as ' (type assertions / casts) ----"
# Note: this can match legitimate "as" keywords; we'll inspect matches if any.
rg -n --pcre2 "(^|[^\\w])as\\s+[^;]+;" "$FILE" || trueRepository: ProverCoderAI/docker-git
Length of output: 9370
Нарушены запреты проекта на as/unknown в CORE-модуле browser-connection.ts
В packages/lib/src/core/browser-connection.ts есть несколько as-кастов и unknown-типы, что противоречит строгим TS-ограничениям для **/*.{ts,tsx}:
readonly _tag = "BrowserError" as const(стр. 17) — запрещёнasconstructor(readonly message: string, readonly cause?: unknown)(стр. 18) и дублирующийсяcause?: unknown(стр. 74) — запрещёнunknownв COREparseProxyPath: ... Effect.Effect<unknown, never>(стр. 26) — запрещёнunknownв COREreturn undefined as void(стр. 39) — запрещёнas
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/lib/src/core/browser-connection.ts` at line 17, The core module
violates project bans on "as" casts and unknown: update the BrowserError class
to declare readonly _tag: "BrowserError" = "BrowserError" (remove as const);
change the constructor signature from cause?: unknown to a concrete type such as
cause?: Error | undefined (remove unknown), and remove any duplicate cause?:
unknown declarations; change parseProxyPath's Effect type from
Effect.Effect<unknown, never> to a concrete environment type (e.g.,
Effect.Effect<never, never> or a specific env/interface used by your effects)
instead of unknown; and replace any return undefined as void with an explicit
bare return (or return void 0) to avoid using as. Ensure you update all
occurrences of these symbols (BrowserError, constructor(cause), parseProxyPath,
and the return site) consistently.
Comment on lines
+32
to
+46
| export const BrowserConnectionLive = Layer.effect( | ||
| BrowserConnection, | ||
| Effect.gen(function* (_) { | ||
| return { | ||
| startBrowser: (projectId: string) => | ||
| Effect.gen(function* () { | ||
| yield* _(Effect.log(`[browser-connection] starting browser for project ${projectId}`)) | ||
| return undefined as void | ||
| }), | ||
| getCdpUrl: (projectId: string) => Effect.succeed(`http://localhost:9223?project=${projectId}`), | ||
| getNoVncUrl: (projectId: string) => Effect.succeed(`/b/${projectId}/vnc.html?autoconnect=true&resize=remote&path=b/${projectId}/websockify`), | ||
| getVncUrl: (projectId: string) => Effect.succeed(`vnc://localhost:5900`), | ||
| parseProxyPath: (_pathname: string) => Effect.succeed(null), | ||
| rewriteCdpUrl: (value: string, _externalOrigin: string, _projectId: string) => value | ||
| } |
There was a problem hiding this comment.
Реализация не обеспечивает заявленный инвариант единой браузерной сессии из ТЗ/целей PR.
startBrowser и URL-методы — заглушки без проверки/связывания с фактическим контейнером dg-*-browser, поэтому требование “single unified browser session via CDP + noVNC” не доказуемо.
Also applies to: 89-103
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/lib/src/core/browser-connection.ts` around lines 32 - 46, The
current BrowserConnectionLive implementation provides stubbed startBrowser and
URL methods that are not tied to the actual dg-*-browser container, so it does
not enforce the required single unified browser session invariant; update
BrowserConnectionLive so startBrowser negotiates/creates or looks up a single
shared browser container/session (e.g., by calling the container manager or
session registry), store and reuse the session handle, and make getCdpUrl,
getNoVncUrl, getVncUrl, parseProxyPath and rewriteCdpUrl derive their values
from that stored session (failing or refreshing the session if missing/expired)
instead of returning hardcoded values; ensure rewriteCdpUrl performs
origin-aware rewriting using the real external origin and stored project/session
metadata.
| readonly getCdpUrl: (projectId: string) => Effect.Effect<string, BrowserError> | ||
| readonly getNoVncUrl: (projectId: string) => Effect.Effect<string, BrowserError> | ||
| readonly getVncUrl: (projectId: string) => Effect.Effect<string, BrowserError> | ||
| readonly parseProxyPath: (pathname: string) => Effect.Effect<ProjectBrowserProxyPath | null, never> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Проверка объявления/импорта символа ProjectBrowserProxyPath в репозитории
rg -n "type ProjectBrowserProxyPath|interface ProjectBrowserProxyPath|import .*ProjectBrowserProxyPath" --type ts --type tsxRepository: ProverCoderAI/docker-git
Length of output: 96
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Локально в файле: что объявлено вокруг parseProxyPath и есть ли импорт/тип рядом
echo "---- browser-connection.ts (context) ----"
sed -n '1,140p' packages/lib/src/core/browser-connection.ts
# 2) По репозиторию: где вообще встречается ProjectBrowserProxyPath
echo "---- occurrences of ProjectBrowserProxyPath ----"
rg -n "ProjectBrowserProxyPath" -g'*.ts' -g'*.tsx' || true
# 3) Быстрый поиск по объявлениям типа/интерфейса/экспорта
echo "---- declarations of ProjectBrowserProxyPath ----"
rg -n "(type|interface|class|enum)\s+ProjectBrowserProxyPath" -g'*.ts' -g'*.tsx' || true
# 4) Проверка импортов в конкретном файле
echo "---- imports from browser-connection.ts ----"
rg -n "^import\s+.*ProjectBrowserProxyPath|ProjectBrowserProxyPath\s+from" packages/lib/src/core/browser-connection.ts -g'*.ts' -g'*.tsx' || trueRepository: ProverCoderAI/docker-git
Length of output: 9484
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="packages/lib/src/core/browser-connection.ts"
echo "---- file length ----"
wc -l "$FILE"
echo "---- imports in browser-connection.ts ----"
rg -n "^import\s" "$FILE" || true
echo "---- ProjectBrowserProxyPath occurrences in browser-connection.ts ----"
rg -n "ProjectBrowserProxyPath" "$FILE" || true
echo "---- surrounding parseProxyPath (near line ~82) ----"
sed -n '60,110p' "$FILE"Repository: ProverCoderAI/docker-git
Length of output: 2939
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- find global/ambient declarations for ProjectBrowserProxyPath ----"
rg -n "declare\s+global|declare\s+namespace|export\s+type\s+ProjectBrowserProxyPath|interface\s+ProjectBrowserProxyPath|type\s+ProjectBrowserProxyPath" -S -g'*.ts' -g'*.d.ts' .
echo "---- show exact export in project-browser-core.ts ----"
sed -n '1,120p' packages/api/src/services/project-browser-core.ts
echo "---- confirm whether browser-connection.ts imports it (any other file references) ----"
rg -n "from\s+\".*project-browser-core\"|project-browser-core" packages/lib/src/core/browser-connection.ts || trueRepository: ProverCoderAI/docker-git
Length of output: 3720
[критично] Исправьте отсутствующий тип ProjectBrowserProxyPath в packages/lib/src/core/browser-connection.ts (строка ~82)
В сигнатуре parseProxyPath используется ProjectBrowserProxyPath, но в файле он ни импортирован, ни объявлен; определение находится в packages/api/src/services/project-browser-core.ts. Нужен импорт/перенос типа в соответствующий CORE-модуль (или замена на локальный тип).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/lib/src/core/browser-connection.ts` at line 82, The parseProxyPath
signature references the missing type ProjectBrowserProxyPath; add a proper type
definition or an import for ProjectBrowserProxyPath into this CORE module and
update the declaration for readonly parseProxyPath accordingly; locate
parseProxyPath in browser-connection.ts and either (a) import the
ProjectBrowserProxyPath type from the API package where it’s defined, or (b)
copy/declare a compatible local type in this file/module and export it if
needed, then run type-checks to ensure no unresolved type errors remain.
Comment on lines
+478
to
+483
| - name: Version and commit | ||
| id: version | ||
| env: | ||
| BUMP_TYPE: ${{ github.event.inputs.bump_type }} | ||
| DESCRIPTION: ${{ github.event.inputs.description }} | ||
| run: rust-script scripts/version-and-commit.rs --bump-type "${{ github.event.inputs.bump_type }}" --description "${{ github.event.inputs.description }}" |
There was a problem hiding this comment.
Не подставляйте workflow_dispatch inputs прямо в shell-команду.
github.event.inputs.description и другие inputs интерполируются до запуска shell, поэтому значения вроде $(...) или обратных кавычек будут выполнены на runner'е. Здесь уже объявлены безопасные env, их и нужно использовать.
💡 Безопасный вариант
- run: rust-script scripts/version-and-commit.rs --bump-type "${{ github.event.inputs.bump_type }}" --description "${{ github.event.inputs.description }}"
+ run: rust-script scripts/version-and-commit.rs --bump-type "$BUMP_TYPE" --description "$DESCRIPTION"- run: rust-script scripts/create-changelog-fragment.rs --bump-type "${{ github.event.inputs.bump_type }}" --description "${{ github.event.inputs.description }}"
+ run: rust-script scripts/create-changelog-fragment.rs --bump-type "$BUMP_TYPE" --description "$DESCRIPTION"As per coding guidelines, **/*: Флагай любой уход от спеки, недокументированное изменение поведения, отсутствие тестов для заявленного поведения и security-риск.
Also applies to: 596-600
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/rust-browser-connection/.github/workflows/release.yml` around lines
478 - 483, The run step in the "Version and commit" job is interpolating
workflow_dispatch inputs directly into the shell command (rust-script ...
--bump-type "${{ github.event.inputs.bump_type }}" --description "${{
github.event.inputs.description }}"), which is unsafe; update the step to use
the already-declared environment variables (BUMP_TYPE and DESCRIPTION) instead
(e.g., reference $BUMP_TYPE and $DESCRIPTION in the run command) and make the
same change for the other occurrence referenced (lines 596-600) so all inputs
are consumed via env variables rather than direct github.event.inputs
interpolation; keep the step id/version and env names (id: version, env:
BUMP_TYPE, DESCRIPTION) intact.
Comment on lines
+655
to
+657
| - uses: actions/checkout@v6 | ||
| with: | ||
| ref: main |
There was a problem hiding this comment.
workflow_dispatch публикует docs не из того ref.
actions/checkout здесь всегда берёт main, поэтому ручной instant release, запущенный не с main, может задеплоить документацию для другого коммита/версии. Либо ограничьте workflow только main, либо checkout'ьте текущий ref/SHA запуска.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/rust-browser-connection/.github/workflows/release.yml` around lines
655 - 657, В workflow_dispatch-triggered job the checkout is pinned to "ref:
main", so manual releases started from a non-main branch will still build/deploy
docs for main; update the workflow to either restrict manual dispatches to main
(add a condition limiting the workflow/job to github.ref == 'refs/heads/main' or
restrict triggers to branches: main) or change the actions/checkout step
(actions/checkout@v6) to check out the current run ref/commit by using the
workflow context (use github.ref or github.sha as the ref) so the docs are built
from the exact commit that triggered the run.
Comment on lines
+139
to
+143
| let re = Regex::new(r#"(?m)^(version\s*=\s*")[^"]+(")"#).unwrap(); | ||
| let new_content = re.replace(&content, format!("${{1}}{}${{2}}", new_version).as_str()); | ||
|
|
||
| fs::write(cargo_toml_path, new_content.as_ref()) | ||
| .map_err(|e| format!("Failed to write {}: {}", cargo_toml_path, e))?; |
There was a problem hiding this comment.
Обновление версии в Cargo.toml сейчас затрагивает любые version =, не только пакет.
Такой regex может изменить версии в других таблицах ([dependencies.*]), что ломает манифест и увеличивает supply-chain риск.
Предложение исправления
- let re = Regex::new(r#"(?m)^(version\s*=\s*")[^"]+(")"#).unwrap();
+ let re = Regex::new(r#"(?ms)^(\[package\].*?^version\s*=\s*")[^"]+(")"#).unwrap();И далее заменять только матч внутри [package].
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/rust-browser-connection/scripts/version-and-commit.rs` around lines
139 - 143, The current regex (re) replaces any "version =" occurrence in the
file (including dependency tables) and should be changed to only update the
package.version; fix this by parsing and updating the Cargo.toml's [package]
table (e.g., using the toml or toml_edit crate to load content, set
package.version = new_version, then serialize and write to cargo_toml_path), or
alternatively restrict the regex to operate only inside the [package] section
before creating new_content; update the code that computes new_content (and keep
variables content, new_version, cargo_toml_path) so only the package.version is
modified.
Comment on lines
+351
to
+419
| fn collect_changelog(changelog_dir: &str, changelog_file: &str, version: &str) { | ||
| let dir_path = Path::new(changelog_dir); | ||
| if !dir_path.exists() { | ||
| return; | ||
| } | ||
|
|
||
| let mut files: Vec<_> = match fs::read_dir(dir_path) { | ||
| Ok(entries) => entries | ||
| .filter_map(|e| e.ok()) | ||
| .map(|e| e.path()) | ||
| .filter(|p| { | ||
| p.extension().map_or(false, |ext| ext == "md") | ||
| && p.file_name().map_or(false, |name| name != "README.md") | ||
| }) | ||
| .collect(), | ||
| Err(_) => return, | ||
| }; | ||
|
|
||
| if files.is_empty() { | ||
| return; | ||
| } | ||
|
|
||
| files.sort(); | ||
|
|
||
| let fragments: Vec<String> = files | ||
| .iter() | ||
| .filter_map(|f| fs::read_to_string(f).ok()) | ||
| .map(|c| strip_frontmatter(&c)) | ||
| .filter(|c| !c.is_empty()) | ||
| .collect(); | ||
|
|
||
| if fragments.is_empty() { | ||
| return; | ||
| } | ||
|
|
||
| let date_str = Utc::now().format("%Y-%m-%d").to_string(); | ||
| let new_entry = format!( | ||
| "\n## [{}] - {}\n\n{}\n", | ||
| version, | ||
| date_str, | ||
| fragments.join("\n\n") | ||
| ); | ||
|
|
||
| if Path::new(changelog_file).exists() { | ||
| let mut content = fs::read_to_string(changelog_file).unwrap_or_default(); | ||
| let lines: Vec<&str> = content.lines().collect(); | ||
| let mut insert_index = None; | ||
|
|
||
| for (i, line) in lines.iter().enumerate() { | ||
| if line.starts_with("## [") { | ||
| insert_index = Some(i); | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| if let Some(idx) = insert_index { | ||
| let mut new_lines: Vec<String> = lines[..idx].iter().map(|s| s.to_string()).collect(); | ||
| new_lines.push(new_entry.clone()); | ||
| new_lines.extend(lines[idx..].iter().map(|s| s.to_string())); | ||
| content = new_lines.join("\n"); | ||
| } else { | ||
| content.push_str(&new_entry); | ||
| } | ||
|
|
||
| fs::write(changelog_file, content).expect("Failed to write changelog"); | ||
| } | ||
|
|
||
| println!("Collected {} changelog fragment(s)", files.len()); | ||
| } |
There was a problem hiding this comment.
После сборки changelog фрагменты не удаляются, что провоцирует повторные релизные бампы.
В текущем потоке auto-release это может оставлять changelog.d/*.md в ветке и триггерить повторную обработку одних и тех же фрагментов на следующих пушах.
Предложение исправления
fn collect_changelog(changelog_dir: &str, changelog_file: &str, version: &str) {
@@
println!("Collected {} changelog fragment(s)", files.len());
+
+ for file in files {
+ if let Err(e) = fs::remove_file(&file) {
+ eprintln!("Warning: Failed to remove {}: {}", file.display(), e);
+ }
+ }
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/rust-browser-connection/scripts/version-and-commit.rs` around lines
351 - 419, collect_changelog currently gathers markdown fragments from
changelog_dir into files and writes a new entry to changelog_file but never
removes the source fragments, causing repeated release bumps; after successfully
writing the updated changelog in collect_changelog (i.e., after
fs::write(changelog_file, content) succeeds), iterate the same files vector and
remove each fragment file (use fs::remove_file on each Path in files),
handling/remotely logging/remove-file errors gracefully (e.g., ignore non-fatal
errors or log them) so fragments are cleaned up and won't be reprocessed; ensure
you use the existing variables files and changelog_file and only delete after a
successful write.
Comment on lines
+24
to
+42
| let config = Config { | ||
| image: Some("dg-docker-git-browser:latest".to_string()), // adjust to your image | ||
| host_config: Some(HostConfig { | ||
| port_bindings: Some(HashMap::from([ | ||
| ("5900/tcp".to_string(), Some(vec![bollard::models::PortBinding { | ||
| host_ip: Some("0.0.0.0".to_string()), | ||
| host_port: Some("5900".to_string()), | ||
| }])), | ||
| ("6080/tcp".to_string(), Some(vec![bollard::models::PortBinding { | ||
| host_ip: Some("0.0.0.0".to_string()), | ||
| host_port: Some("6080".to_string()), | ||
| }])), | ||
| ("9223/tcp".to_string(), Some(vec![bollard::models::PortBinding { | ||
| host_ip: Some("0.0.0.0".to_string()), | ||
| host_port: Some("9223".to_string()), | ||
| }])), | ||
| ])), | ||
| ..Default::default() | ||
| }), |
There was a problem hiding this comment.
Не биньдите CDP/noVNC/VNC на 0.0.0.0 по умолчанию.
Сейчас любой узел, который видит эту машину, может подключиться к CDP или noVNC и перехватить браузерную сессию. Для локального bridge безопаснее публиковать порты на 127.0.0.1 и делать внешний доступ только через явно настроенный proxy.
💡 Минимальное ужесточение
- host_ip: Some("0.0.0.0".to_string()),
+ host_ip: Some("127.0.0.1".to_string()),Примените это ко всем трём binding'ам.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| let config = Config { | |
| image: Some("dg-docker-git-browser:latest".to_string()), // adjust to your image | |
| host_config: Some(HostConfig { | |
| port_bindings: Some(HashMap::from([ | |
| ("5900/tcp".to_string(), Some(vec![bollard::models::PortBinding { | |
| host_ip: Some("0.0.0.0".to_string()), | |
| host_port: Some("5900".to_string()), | |
| }])), | |
| ("6080/tcp".to_string(), Some(vec![bollard::models::PortBinding { | |
| host_ip: Some("0.0.0.0".to_string()), | |
| host_port: Some("6080".to_string()), | |
| }])), | |
| ("9223/tcp".to_string(), Some(vec![bollard::models::PortBinding { | |
| host_ip: Some("0.0.0.0".to_string()), | |
| host_port: Some("9223".to_string()), | |
| }])), | |
| ])), | |
| ..Default::default() | |
| }), | |
| let config = Config { | |
| image: Some("dg-docker-git-browser:latest".to_string()), // adjust to your image | |
| host_config: Some(HostConfig { | |
| port_bindings: Some(HashMap::from([ | |
| ("5900/tcp".to_string(), Some(vec![bollard::models::PortBinding { | |
| host_ip: Some("127.0.0.1".to_string()), | |
| host_port: Some("5900".to_string()), | |
| }])), | |
| ("6080/tcp".to_string(), Some(vec![bollard::models::PortBinding { | |
| host_ip: Some("127.0.0.1".to_string()), | |
| host_port: Some("6080".to_string()), | |
| }])), | |
| ("9223/tcp".to_string(), Some(vec![bollard::models::PortBinding { | |
| host_ip: Some("127.0.0.1".to_string()), | |
| host_port: Some("9223".to_string()), | |
| }])), | |
| ])), | |
| ..Default::default() | |
| }), |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/rust-browser-connection/src/lib.rs` around lines 24 - 42, The port
bindings in Config -> HostConfig -> port_bindings currently bind CDP/noVNC/VNC
to "0.0.0.0", which exposes ports 5900, 6080 and 9223 to the network; change the
host_ip for the PortBinding entries for "5900/tcp", "6080/tcp", and "9223/tcp"
from "0.0.0.0" to "127.0.0.1" so they are only accessible locally (update the
PortBinding host_ip values where port_bindings is defined in Config/HostConfig).
Comment on lines
+72
to
+74
| pub fn is_single_browser_session(&self, cdp_url: &str, novnc_url: &str) -> bool { | ||
| cdp_url.contains("9223") && novnc_url.contains("/vnc.html") | ||
| } |
There was a problem hiding this comment.
is_single_browser_session не проверяет инвариант одной сессии.
Этот метод возвращает true для любых несвязанных URL, если в одном есть 9223, а в другом /vnc.html. Для API, которое заявляет single-session invariant из #347, такая проверка даёт ложное чувство корректности.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/rust-browser-connection/src/lib.rs` around lines 72 - 74, Метод
is_single_browser_session неправильно считает совпадение по наличию подстрок;
вместо этого распарсьте cdp_url и novnc_url и убедитесь, что они относятся к
одной и той же сессии: сравните их origin (scheme + host + port) и/или явные
идентификаторы сессии в пути/параметрах, и только если origin совпадает и
cdp_url содержит порт 9223, а novnc_url содержит путь /vnc.html — возвращайте
true; обновите функцию is_single_browser_session, чтобы использовать URL-парсинг
и чёткое сравнение origin/session-id вместо простого contains.
Comment on lines
+81
to
+86
| #[test] | ||
| fn test_urls() { | ||
| let conn = BrowserConnection::new().unwrap(); | ||
| let novnc = conn.get_novnc_url("issue-347"); | ||
| let cdp = conn.get_cdp_url("issue-347"); | ||
| assert!(conn.is_single_browser_session(&cdp, &novnc)); |
There was a problem hiding this comment.
Unit-test не должен требовать живой Docker daemon для проверки URL helper'ов.
BrowserConnection::new().unwrap() делает тест хрупким и зависящим от окружения, хотя здесь проверяются только строковые URL. Лучше тестировать pure helper-логику без подключения к Docker.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/rust-browser-connection/src/lib.rs` around lines 81 - 86, The test
calls BrowserConnection::new() which requires a live Docker daemon; instead
construct a pure BrowserConnection value or call a new test-only constructor so
URL helpers can be tested without Docker. Modify the test to use a lightweight
constructor (e.g., BrowserConnection::from_base_url or
BrowserConnection::with_mock) or directly instantiate BrowserConnection with
fake base values, then call get_novnc_url("issue-347"), get_cdp_url("issue-347")
and assert is_single_browser_session(&cdp, &novnc); avoid calling
BrowserConnection::new() in the unit test.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
New Rust package based on the rust-ai-driven-development-pipeline-template.
Changes
Closes #347
The module can be called from TS via Command or integrated as dependency.
Tested with cargo check (after Rust installed).