feat(mcp): WS-mode к v8-client-session-manager + skill bootstrap reference#3
Open
steel-code-agent wants to merge 15 commits into
Open
feat(mcp): WS-mode к v8-client-session-manager + skill bootstrap reference#3steel-code-agent wants to merge 15 commits into
steel-code-agent wants to merge 15 commits into
Conversation
…layout Anthropic Skills convention: каталог скила = его имя в kebab-case, внутри обязательно SKILL.md + references/. Готовый каталог можно дроп-копировать в ~/.claude/skills/<name>/ без переименования. В этом репо frontmatter скила декларирует name: v8-runner, поэтому каталог приводим в соответствие. Что сделано: - git mv SKILL v8-runner (sохраняет историю) - ссылки обновлены: AGENTS.md (×2), spec/decisions/0022-*.md (×1) После рефакторинга разработчику достаточно сделать cp -r v8-runner ~/.claude/skills/ чтобы скил стал доступен Claude Code.
Adds src/use_cases/mcp_ws with: - McpClientTransport (Ws/Legacy/Auto) selector - ClientKind enum mapping entry-points to manager-side kind values - WsLaunchParams::payload_snippet() that produces the 'mcpMode=ws;manager_url=...;client_uid=...;kind=...;corr_id=...; mcp_log_level=...;mcp_ws_timeout_ms=...' segment for /C - select_transport() with probe-callback (Auto -> WS or Legacy) - probe_tcp() built on std::net::TcpStream::connect_timeout - parse_manager_addr() that extracts host:port from a ws://... URL - validate_log_level() for off|error|warn|info|debug|trace Pure module without project plumbing yet; later commits wire it into launch_app, run_tests, CLI args and config loading.
Adds new optional fields under tools.client_mcp: - transport: ws | legacy | auto - manager_url: ws://host:port/... - log_level: off | error | warn | info | debug | trace - ws_timeout_ms: u64 (>= 1) Reuses tools.client_mcp instead of mcp.client because the loader still rejects mcp.client as a legacy migration guard (LegacyMcpClientConfig). Putting the new keys next to port/extension keeps client-MCP config in one place. Schemas (ClientMcpToolSchema, PartialClientMcpToolSchema) and validate.rs are extended with bound checks: transport/log_level use the mcp_ws helpers, manager_url is parsed via parse_manager_addr, ws_timeout_ms must be > 0.
…id / --corr-id / --mcp-log-level / --mcp-ws-timeout-ms Adds a shared McpClientWsArgs struct flattened into LaunchArgs and TestArgs. Values are validated at the CLI boundary (map_mcp_ws_args): transport must be ws|legacy|auto, log_level must be one of off|error|warn|info|debug|trace, ws_timeout_ms must be >= 1, manager_url must include host:port, client_uid/corr_id must not contain ';' (the /C payload is semicolon-delimited). Wired through transport-neutral McpClientWsRequest in use_cases::request, threaded into LaunchRequest and TestRequest so that downstream MCP service callers can also opt into the WS-mode without changes once they need it. The actual /C payload assembly happens in the next commit. Per-launch random client_uid is left to the use-case layer. By contract the kind is computed internally from the entry-point and is NOT exposed via CLI.
launch_app: - effective_launch_options now returns (LaunchOptions, Option<McpResolutionMeta>); meta drives the new LaunchResult fields (transport, client_uid, kind, manager_url, corr_id, mcp_port). - decide_mcp_transport probes manager_url with PROBE_TIMEOUT_MS and returns ws|legacy according to CLI/config preference; auto falls back to legacy when probe fails. - WS-mode emits 'mcpMode=ws;manager_url=...;client_uid=<UUID>;kind= v8_runner_client|vanessa_test_client;corr_id=vr-<uid8>; mcp_log_level=...;mcp_ws_timeout_ms=...'. - Legacy branch keeps the existing 'runMcp[=...][;mcpPort=...]' exactly as before for back-compat. run_tests: - After build_platform_launch the coordinator calls apply_test_mcp_ws_payload, which appends the WS snippet to the existing /C (RunUnitTests=... or VA player payload). Resolution errors are logged as warn and skipped, preserving prior behavior. - yaxunit_runner / vanessa_test_client kinds are picked from PreparedRun. LaunchResult: new optional fields are skip-if-none, so JSON envelopes for non-MCP launches are byte-identical to the previous shape.
- launch_app::tests::client_mcp_launch_does_not_prepare_configured_tool_extension:
pin transport to Legacy so the test stays deterministic in
environments where a session-manager listener happens to be live
on 127.0.0.1:4000 (auto would otherwise pick WS).
- tests/cli_launch.rs::launch_mcp_va_builds_payload_from_configured_port_and_ordinary_mode:
add --mcp-transport legacy and assert the new JSON fields
transport=legacy and mcp_port=9874 in the launch envelope.
- Regenerate docs/schemas/v8project.schema.json and
docs/schemas/v8project.local.schema.json to include the new
tools.client_mcp.{transport,manager_url,log_level,ws_timeout_ms}
fields (UPDATE_CONFIG_SCHEMAS=1 cargo test
generated_schema_artifacts_are_current).
tests/cli_launch.rs: 6 new integration tests covering the four behaviors required by the spec: - launch mcp --mcp-transport=legacy emits /C"runMcp;mcpPort=..." and a JSON envelope with transport=legacy + mcp_port. - launch mcp --mcp-transport=ws against a live ephemeral listener emits /C"mcpMode=ws;manager_url=...;client_uid=...;kind= v8_runner_client;corr_id=...;mcp_log_level=...;mcp_ws_timeout_ms= ..." plus matching JSON fields (transport=ws, client_uid, kind, manager_url, corr_id). - launch mcp --mcp-transport=ws against an unreachable port fails with 'session-manager unreachable' diagnostic. - launch mcp --mcp-transport=auto falls back to legacy when the manager is unreachable. - launch mcp --manager-url with bare host (no port) is rejected at argument-mapping time. - launch mcp --mcp-ws-timeout-ms 0 is rejected as zero. src/use_cases/run_tests/helpers.rs: 3 unit tests for append_mcp_ws_snippet covering existing /C, missing /C and empty /C. Real 1C runs are not invoked in any of these tests — the script in $tempdir/platform/bin/1cv8c just dumps args to a log.
- README: add a 'Подключение к session-manager (WS-режим)' subsection pointing at docs/CONFIGURATION.md and the v8-client-session-manager repo. - docs/CONFIGURATION.md: extend tools.client_mcp YAML example with the new transport/manager_url/log_level/ws_timeout_ms fields and add a 'WS-режим к session-manager' subsection that documents the internal kind mapping, the /C payload shape, the CLI flags and the auto-probe behavior. - src/use_cases/mcp_ws.rs: simplify — derive Default for McpClientTransport instead of a hand-written impl, drop unused validate_log_level/UnsupportedLogLevel pair (the boundary uses is_supported_log_level directly via map_mcp_ws_args), and switch the matching test over. - cargo fmt.
Дополняет агентный скил v8-runner новой секцией. Источник правды — docs/CONFIGURATION.md (`tools.client_mcp` + «WS-режим к session-manager»), здесь — оперативная справка для агента. Что добавлено: - v8-runner/references/project-workflows.md: новая секция «WS-режим к session-manager» — транспорт/auto-probe, /C payload, internal kind mapping (v8_runner_client / vanessa_test_client / yaxunit_runner), override-флаги, JSON-output, замечание что менеджер v8-runner не поднимает (отдельный шаг). - v8-runner/references/command-selection.md: после блока launch mcp добавлены примеры WS-флагов (--mcp-transport, --manager-url, --client-uid, --corr-id, --mcp-log-level) с указанием на полный раздел в project-workflows.md. - v8-runner/SKILL.md: в Default Use-Case Routing — буллет про WS-режим с краткой логикой auto-probe и фиксированным kind-mapping (без CLI override).
…init
Скил знал про команду config init, но не описывал, как агенту решить
какие флаги передать без лишних вопросов пользователю. Decision-tree
прятался в коде src/use_cases/config_init.rs (auto-detect format,
discover sources, choose builder).
Что добавлено:
- v8-runner/references/bootstrap.md — новый reference. Содержит:
- проверка существующего v8project.yaml (force-семантика);
- signals в файловой системе для format=designer|edt|auto;
- когда переключать builder с DESIGNER на IBCMD;
- три формы --connection (File auto-managed / File existing /
Srvr server-bound) с дефолтом File=build/ib;
- правила «когда задавать вопросы»: 4 ситуации и формулировки;
- примеры flow для типичных проектов (File-Designer, EDT, mixed,
server-bound);
- чек-лист what-to-inspect после config init.
- v8-runner/SKILL.md: новый bullet в decision entrypoint, ссылающийся
на bootstrap.md.
Источник правды по фактам — src/use_cases/config_init.rs (discover_sources,
choose_format, build_source_sets) + src/cli/args.rs (ConfigInitArgs).
WalkthroughPR adds WS transport support for MCP launch and test flows, extends config/CLI/request contracts, wires transport resolution into use cases, and updates launch argument handling plus detached Unix process startup behavior. ChangesMCP WebSocket Session-Manager Support
Launch argument and process behavior
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/config/schema.rs (1)
234-236:⚠️ Potential issue | 🟠 Major | ⚡ Quick winСинхронизируй schema-boundary для
ws_timeout_msс runtime-правилами.Сейчас в генерации ограничений для
ClientMcpToolSchema/PartialClientMcpToolSchemaучитывается толькоport, поэтомуws_timeout_ms=0проходит JSON Schema и падает позже на прикладной валидации. Лучше отклонять это сразу на boundary-слое.💡 Предлагаемый патч
for def in ["ClientMcpToolSchema", "PartialClientMcpToolSchema"] { set_numeric_bounds(schema, &[def], "port", Some(1), None); + set_numeric_bounds(schema, &[def], "ws_timeout_ms", Some(1), None); }As per coding guidelines:
**/*.rs: Apply/rust-expert-best-practices-code-reviewskill for tasks involving Rust code review, refactoring, error handling, type safety, API design, or performance-sensitive changes.🤖 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 `@src/config/schema.rs` around lines 234 - 236, The JSON Schema generator currently only applies numeric bounds for "port" in ClientMcpToolSchema/PartialClientMcpToolSchema, so ws_timeout_ms can be zero and fails later; update the same boundary generation to also set a minimum > 0 for "ws_timeout_ms" by calling set_numeric_bounds(schema, &[def], "ws_timeout_ms", Some(1), None) alongside the existing call for "port" (refer to the loop over def values and the set_numeric_bounds function to locate where to add the new call).
🧹 Nitpick comments (4)
src/domain/launch.rs (1)
18-21: ⚡ Quick win
transportстоит выразить через типизированныйenumвместоOption<String>Поле документировано двумя значениями —
"ws"и"legacy". В том же файлеLaunchModeдемонстрирует устоявшийся паттернenum+#[serde(rename_all = "snake_case")], который даёт ту же JSON-сериализацию, но исключает опечатки и делает ветвление по значению безопасным на уровне типов. Поле новое — сменить тип сейчас дёшево.♻️ Предлагаемое изменение
+/// MCP-транспорт для данного запуска. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +#[serde(rename_all = "snake_case")] +pub enum McpTransport { + Ws, + Legacy, +} pub struct LaunchResult { ... - /// MCP transport selected for this launch (`ws` or `legacy`). - /// Present only for `launch mcp` and `test` flows. + /// MCP transport selected for this launch. + /// Present only for `launch mcp` and `test` flows. #[serde(skip_serializing_if = "Option::is_none", default)] - pub transport: Option<String>, + pub transport: Option<McpTransport>,🤖 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 `@src/domain/launch.rs` around lines 18 - 21, Поле transport в структуре Launch нужно заменить с Option<String> на типизированный enum — создать enum Transport { Ws, Legacy } с #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq)] и #[serde(rename_all = "snake_case")] и использовать Option<Transport> с теми же атрибутами #[serde(skip_serializing_if = "Option::is_none", default)] на поле transport; затем поправить все места, где сравнивали или использовали transport (паттерн-матчинг в коде) чтобы работать с Transport::Ws / Transport::Legacy, а также обновить тесты/сериализацию, где ожидалось строковое значение.src/use_cases/launch_app.rs (1)
243-257: ⚡ Quick winWS-ветка молча игнорирует
client_mcp.portиclient_mcp.config_path.В
TransportDecision::Wsclient_mcp.config_path/client_mcp.port(CLI или конфиг) не используются вовсе — это корректно семантически (порт назначает session-manager), но плохо для UX: пользователь, явно передавший--port 9874и получивший WS черезauto-detect, не получит ни warning'а, ни ошибки.Минимум — лог уровня
warn!при WS+port.is_some()/config_path.is_some(). Опционально — отклонять как validation-ошибку, если транспорт указан явно какWs.📝 Минимальный warn
TransportDecision::Ws => { + if client_mcp.port.is_some() || config.tools.client_mcp.port.is_some() { + tracing::warn!("ws transport: client_mcp.port is ignored; session-manager controls the port"); + } + if client_mcp.config_path.is_some() { + tracing::warn!("ws transport: client_mcp.config_path is ignored"); + } let params = resolve_ws_launch_params(config, &args.mcp_ws, kind);🤖 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 `@src/use_cases/launch_app.rs` around lines 243 - 257, Ветка TransportDecision::Ws игнорирует переданные client_mcp.port и client_mcp.config_path без уведомления пользователя; добавь проверку перед/внутри матча (в функции где вызываются launch_mcp_client_kind/decide_mcp_transport — вокруг переменных args.mcp_ws и client_mcp) и при выборе Ws логгируй warn! если client_mcp.port.is_some() || client_mcp.config_path.is_some(); дополнительно, если транспорт явно задан как Ws в args.mcp_ws, преобразуй это в validation error (Err) вместо молчалого игнорирования, используя те же символы TransportDecision::Ws, resolve_ws_launch_params и McpResolutionMeta::Ws для локализации изменения.src/use_cases/mcp_ws.rs (2)
345-356: 💤 Low valueТест
probe_tcp_fails_when_no_listenerтеоретически flaky.Между
drop(listener)иprobe_tcp(addr, …)ОС может выдать тот же ephemeral-порт другому процессу (CI с пулом параллельных тестов), и тогдаconnectнеожиданно успеет. Безопаснее использовать заведомо «тёмный» адрес:127.0.0.1:1илиIpAddr::from([127,0,0,2])без листенера, либо192.0.2.1:65535(TEST-NET-1, RFC 5737) с очень коротким таймаутом.🤖 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 `@src/use_cases/mcp_ws.rs` around lines 345 - 356, The test probe_tcp_fails_when_no_listener is flaky because dropping an ephemeral TcpListener can let the OS assign that port to another process before probe_tcp(addr, …) runs; update the test to use a guaranteed-unused "dark" address instead of binding/dropping a listener — e.g., call probe_tcp(SocketAddr::from(([127,0,0,1], 1)), Duration::from_millis(50)) or probe_tcp(SocketAddr::from(([192,0,2,1], 65535)), short_timeout) in the probe_tcp_fails_when_no_listener test so the OS cannot hand the port to another process and the assertion reliably fails when no listener exists.
73-79: ⚡ Quick winНесоответствие в обработке регистра букв:
log_levelболее строг, чемtransport.Обнаружена реальная проблема, но анализ в исходном комментарии содержит неточности:
Case-sensitivity несоответствие подтверждено:
McpClientTransport::from_str_value(строка 45) использует.to_ascii_lowercase()и принимаетLEGACY, ноis_supported_log_level(строка 77-79) случай-чувствителен и отклонитlog_level = "INFO"в конфиге.Исходный анализ неправильно указал точки вызова:
is_supported_log_levelвызывается не вlaunch_app, а в:
src/config/validate.rs:841— для валидации конфигаsrc/cli/execute.rs:1212— для валидации CLI аргументовДля CLI аргументов также существует
value_parser = ["off", "error", "warn", "info", "debug", "trace"]вsrc/cli/args.rs:348, который обеспечивает case-sensitive валидацию на уровне partera clap.Рекомендация: унифицировать обработку регистра в
is_supported_log_level, сделав её case-insensitive, или документировать требование lowercase в комментарии функции и в схеме конфига.♻️ Унификация case-insensitive сравнения
-pub fn is_supported_log_level(level: &str) -> bool { - ALLOWED_LOG_LEVELS.contains(&level) +pub fn is_supported_log_level(level: &str) -> bool { + ALLOWED_LOG_LEVELS + .iter() + .any(|allowed| allowed.eq_ignore_ascii_case(level)) }🤖 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 `@src/use_cases/mcp_ws.rs` around lines 73 - 79, Функция is_supported_log_level и константа ALLOWED_LOG_LEVELS обрабатывают регистр по-разному относительно McpClientTransport::from_str_value; исправьте это, сделав в is_supported_log_level сравнение нечувствительным к регистру: внутри функции привести входной параметр level к нижнему регистру (например через to_ascii_lowercase()) и затем проверять наличие в ALLOWED_LOG_LEVELS, либо явно задокументировать требование lowercase в комментарии функции и в схеме конфига; проверьте связанные места вызова (validate.rs и cli/execute.rs) и при необходимости обновите сообщения/доки для согласованности.
🤖 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 `@README.md`:
- Around line 96-97: Замените относительную ссылку ../v8-client-session-manager/
в README.md на абсолютный URL репозитория на GitHub (например
https://github.com/OWNER/v8-client-session-manager) или на правильную внешнюю
ссылку, сохранив текст ссылки `v8-client-session-manager`; обновите
соответствующую Markdown-скобочную запись, чтобы GitHub корректно переходил по
ссылке из README.
In `@src/config/validate.rs`:
- Around line 852-858: The current validation in validate.rs only checks
parse_manager_addr but does not forbid semicolons, allowing injection via
config.tools.client_mcp.manager_url; update the validation to reject any
manager_url containing ';' (or otherwise enforce a payload-safe character set)
before calling crate::use_cases::mcp_ws::parse_manager_addr and return
ConfigValidationError::InvalidMcpClientManagerUrl for such values; also reuse
the same payload-safe check in the CLI converter code path so both config
loading and CLI-derived manager_url use identical validation.
In `@src/use_cases/mcp_ws.rs`:
- Around line 154-157: Функция default_corr_id молча возвращает слишком короткий
идентификатор при коротком client_uid (например "abc" -> "vr-abc"), нарушая
контракт "vr-<uid8>"; исправь функцию default_corr_id так, чтобы она выравнивала
результат до 8 символов: при фильтрации символов из client_uid (в существующей
логике в default_corr_id) дополни строку справа нулями (или другим выбранным
fill-символом) до длины 8, либо — если предпочитаешь — при коротком/пустом
client_uid вместо паддинга генерируй новый идентификатор на основе
Uuid::new_v4() и используйте его первые 8 символов; внеси изменения именно в
default_corr_id и убедись, что формат финальной строки всегда "vr-<8char>".
- Around line 92-106: The payload_snippet in WsLaunchParams naively inlines
fields (manager_url, client_uid, kind.as_str(), corr_id, log_level,
ws_timeout_ms) into a semicolon/equals-delimited string, allowing
format-injection if any value contains ';' or '='; fix by adding validation or
escaping: either (a) validate each input with an allowlist regex and return
Result<String, Error> from payload_snippet on invalid input, or (b)
sanitize/escape forbidden characters (at minimum replace or percent-encode ';'
and '=') before formatting the snippet so the resulting "mcpMode=ws;..." cannot
be split into extra key/value pairs. Ensure changes are applied inside
WsLaunchParams::payload_snippet (or change its signature to return Result) and
include the field names above to locate the code.
- Around line 216-252: parse_manager_addr currently does a blocking DNS lookup
via host_port.to_socket_addrs() which can stall startup; replace the blocking
resolution with an async, timeout-bound lookup (use tokio::net::lookup_host
wrapped with tokio::time::timeout using PROBE_TIMEOUT_MS) and make
parse_manager_addr async (or add an async helper called e.g.
resolve_manager_addr_async) so you enforce the 200ms probe timeout and return
WsResolveError on timeout; alternatively, if you cannot make the call async,
update the doc-comment for parse_manager_addr to explicitly forbid hostnames
(only accept literal IP:port) and validate/return a clear error for non-IP
inputs. Ensure you update call sites that use parse_manager_addr to await the
new async resolver or to reject hostnames.
In `@src/use_cases/run_tests/helpers.rs`:
- Around line 448-453: The current match on
crate::use_cases::launch_app::decide_mcp_transport in apply_test_mcp_ws_payload
swallows the error (tracing::warn! then return), which lets an explicit
--mcp-transport ws fall back silently; change apply_test_mcp_ws_payload to
return Result<..., Error> and when decide_mcp_transport returns Err propagate
that Err upward (i.e., return Err(err.into())) for the explicit WS mode, while
preserving a soft fallback only for the auto path; update callers to handle the
Result and remove the early silent return so failures for explicit WS are
surfaced.
In `@v8-runner/references/bootstrap.md`:
- Line 48: The documentation incorrectly references the non-existent config key
`tools.connection`; update the text and any examples to use the correct key
`infobase.connection` so bootstrap produces a valid v8project.yaml; search for
and replace occurrences of `tools.connection` in the
v8-runner/references/bootstrap.md content (including the sentence describing the
three common connection shapes) and verify examples and sample YAML show
`infobase.connection` consistently.
---
Outside diff comments:
In `@src/config/schema.rs`:
- Around line 234-236: The JSON Schema generator currently only applies numeric
bounds for "port" in ClientMcpToolSchema/PartialClientMcpToolSchema, so
ws_timeout_ms can be zero and fails later; update the same boundary generation
to also set a minimum > 0 for "ws_timeout_ms" by calling
set_numeric_bounds(schema, &[def], "ws_timeout_ms", Some(1), None) alongside the
existing call for "port" (refer to the loop over def values and the
set_numeric_bounds function to locate where to add the new call).
---
Nitpick comments:
In `@src/domain/launch.rs`:
- Around line 18-21: Поле transport в структуре Launch нужно заменить с
Option<String> на типизированный enum — создать enum Transport { Ws, Legacy } с
#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq)] и
#[serde(rename_all = "snake_case")] и использовать Option<Transport> с теми же
атрибутами #[serde(skip_serializing_if = "Option::is_none", default)] на поле
transport; затем поправить все места, где сравнивали или использовали transport
(паттерн-матчинг в коде) чтобы работать с Transport::Ws / Transport::Legacy, а
также обновить тесты/сериализацию, где ожидалось строковое значение.
In `@src/use_cases/launch_app.rs`:
- Around line 243-257: Ветка TransportDecision::Ws игнорирует переданные
client_mcp.port и client_mcp.config_path без уведомления пользователя; добавь
проверку перед/внутри матча (в функции где вызываются
launch_mcp_client_kind/decide_mcp_transport — вокруг переменных args.mcp_ws и
client_mcp) и при выборе Ws логгируй warn! если client_mcp.port.is_some() ||
client_mcp.config_path.is_some(); дополнительно, если транспорт явно задан как
Ws в args.mcp_ws, преобразуй это в validation error (Err) вместо молчалого
игнорирования, используя те же символы TransportDecision::Ws,
resolve_ws_launch_params и McpResolutionMeta::Ws для локализации изменения.
In `@src/use_cases/mcp_ws.rs`:
- Around line 345-356: The test probe_tcp_fails_when_no_listener is flaky
because dropping an ephemeral TcpListener can let the OS assign that port to
another process before probe_tcp(addr, …) runs; update the test to use a
guaranteed-unused "dark" address instead of binding/dropping a listener — e.g.,
call probe_tcp(SocketAddr::from(([127,0,0,1], 1)), Duration::from_millis(50)) or
probe_tcp(SocketAddr::from(([192,0,2,1], 65535)), short_timeout) in the
probe_tcp_fails_when_no_listener test so the OS cannot hand the port to another
process and the assertion reliably fails when no listener exists.
- Around line 73-79: Функция is_supported_log_level и константа
ALLOWED_LOG_LEVELS обрабатывают регистр по-разному относительно
McpClientTransport::from_str_value; исправьте это, сделав в
is_supported_log_level сравнение нечувствительным к регистру: внутри функции
привести входной параметр level к нижнему регистру (например через
to_ascii_lowercase()) и затем проверять наличие в ALLOWED_LOG_LEVELS, либо явно
задокументировать требование lowercase в комментарии функции и в схеме конфига;
проверьте связанные места вызова (validate.rs и cli/execute.rs) и при
необходимости обновите сообщения/доки для согласованности.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0eb191ec-f001-4036-963e-b9572403f54b
📒 Files selected for processing (32)
.gitignoreAGENTS.mdREADME.mdSKILL/references/project-workflows.mddocs/CONFIGURATION.mddocs/schemas/v8project.local.schema.jsondocs/schemas/v8project.schema.jsonspec/decisions/0022-universalnyy-mehanizm-podgotovki-rasshireniy-i-client-mcp-extension.mdsrc/cli/args.rssrc/cli/execute.rssrc/config/model.rssrc/config/schema.rssrc/config/validate.rssrc/domain/launch.rssrc/mcp/service.rssrc/use_cases/launch_app.rssrc/use_cases/mcp_ws.rssrc/use_cases/mod.rssrc/use_cases/request.rssrc/use_cases/run_tests.rssrc/use_cases/run_tests/coordinator.rssrc/use_cases/run_tests/helpers.rstests/cli_launch.rsv8-runner/SKILL.mdv8-runner/agents/openai.yamlv8-runner/references/bootstrap.mdv8-runner/references/command-selection.mdv8-runner/references/config-and-backends.mdv8-runner/references/file-and-artifact-workflows.mdv8-runner/references/project-workflows.mdv8-runner/references/testing.mdv8-runner/references/troubleshooting.md
💤 Files with no reviewable changes (1)
- SKILL/references/project-workflows.md
- replace legacy transport config value with mcp\n- harden WS payload validation and schema boundaries\n- update docs, schemas, and launch tests
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@docs/CONFIGURATION.md`:
- Around line 457-458: Описание параметра ws_timeout_ms неверно ограничивает его
только режимом auto; обновите текст так, чтобы оно отражало, что ws_timeout_ms
передаётся как mcp_ws_timeout_ms в параметры WebSocket в целом (не только для
auto) и указывает таймаут установки WS-сессии в миллисекундах (>0). Обновите
упоминание `ws_timeout_ms` и ссылку на `mcp_ws_timeout_ms` в документации, убрав
фразу про режим `auto` и сохранив требование >0, чтобы соответствовать
фактической реализации.
In `@src/cli/execute.rs`:
- Around line 1234-1238: Вместо всегда возвращать одно и то же сообщение при
ошибке разбора менеджера, поймайте результат parse_manager_addr(url) и при
Err(e) пробросьте диагностичную ошибку, включающую текст причины из e (например
через e.to_string()) в UseCaseError::new(UseCaseErrorKind::Validation, ...),
заменив текущее фиксированное "must include host:port" на сообщение,
комбинирующее контекст ("--manager-url parse error") и самую причину из
parse_manager_addr; отредактируйте соответствующий условный блок в execute.rs
вокруг вызова parse_manager_addr(url).
In `@tests/cli_launch.rs`:
- Around line 721-723: The test hardcodes an invalid/unstable CLI contract by
passing "--mcp-transport" "mcp"; update the test(s) in tests/cli_launch.rs to
use the supported public transport value (e.g. "legacy" or "auto") or, better,
derive the expected value from the CLI enum/constant used by the parser instead
of the literal "mcp". Locate the occurrences of the "--mcp-transport" / "mcp"
pair in the test and replace them with either a valid literal ("legacy"/"auto")
or a reference to the canonical constant/enum used by the CLI implementation so
the test stays correct if the contract changes.
In `@v8-runner/references/command-selection.md`:
- Around line 169-173: Документирует несогласованность публичного флага
--mcp-transport: примеры и текст используют "mcp", тогда как контракт в PR —
{ws|legacy|auto}; приведите документацию в соответствие с контрактом: в примерах
команды (например "v8-runner launch mcp" и упоминания "test yaxunit ...", "test
va ...") замените значение "mcp" на "legacy" или добавьте явный алиас/примечание
о совместимости, и обновите поясняющий абзац про поведение по умолчанию
(--mcp-transport=auto) чтобы перечислить корректные варианты {ws|legacy|auto} и
описать коротко, как происходит выбор; убедитесь, что терминология в разделе
"WS-режим" и упоминания internal kind mapping/--json-message согласованы с этим
изменением.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0a8ab6df-b707-4da3-ad5c-89ccc9185e91
📒 Files selected for processing (20)
README.mddocs/CONFIGURATION.mddocs/schemas/v8project.local.schema.jsondocs/schemas/v8project.schema.jsonsrc/cli/args.rssrc/cli/execute.rssrc/config/model.rssrc/config/schema.rssrc/config/validate.rssrc/domain/launch.rssrc/use_cases/launch_app.rssrc/use_cases/mcp_ws.rssrc/use_cases/request.rssrc/use_cases/run_tests/coordinator.rssrc/use_cases/run_tests/helpers.rstests/cli_launch.rsv8-runner/SKILL.mdv8-runner/references/bootstrap.mdv8-runner/references/command-selection.mdv8-runner/references/project-workflows.md
✅ Files skipped from review due to trivial changes (2)
- v8-runner/SKILL.md
- README.md
🚧 Files skipped from review as they are similar to previous changes (7)
- src/config/validate.rs
- docs/schemas/v8project.local.schema.json
- src/config/model.rs
- docs/schemas/v8project.schema.json
- src/use_cases/run_tests/coordinator.rs
- src/cli/args.rs
- src/use_cases/launch_app.rs
Comment on lines
+169
to
+173
| v8-runner launch mcp --mcp-transport=mcp # force local MCP without probe | ||
| v8-runner launch mcp --mcp-log-level=debug --client-uid <UUID> --corr-id <STR> | ||
| ``` | ||
|
|
||
| `--mcp-transport=auto` (default) probes `manager_url` for 200 ms and chooses `ws` on success, `mcp` on failure. The same WS-flags work on `test yaxunit ...` and `test va ...`. See `project-workflows.md` for the full WS-режим section, internal `kind` mapping, and `--json-message` output shape. |
There was a problem hiding this comment.
Синхронизируйте публичный контракт --mcp-transport (mcp vs legacy).
В этих строках фиксируется значение mcp, но в целях PR заявлен контракт --mcp-transport={ws|legacy|auto}. Это риск несовместимости для пользователей и CI: либо документация неверна, либо фактический CLI-контракт изменён без явной миграции/алиаса.
🤖 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 `@v8-runner/references/command-selection.md` around lines 169 - 173,
Документирует несогласованность публичного флага --mcp-transport: примеры и
текст используют "mcp", тогда как контракт в PR — {ws|legacy|auto}; приведите
документацию в соответствие с контрактом: в примерах команды (например
"v8-runner launch mcp" и упоминания "test yaxunit ...", "test va ...") замените
значение "mcp" на "legacy" или добавьте явный алиас/примечание о совместимости,
и обновите поясняющий абзац про поведение по умолчанию (--mcp-transport=auto)
чтобы перечислить корректные варианты {ws|legacy|auto} и описать коротко, как
происходит выбор; убедитесь, что терминология в разделе "WS-режим" и упоминания
internal kind mapping/--json-message согласованы с этим изменением.
- clarify WS timeout documentation\n- include manager_url parse diagnostics\n- remove legacy wording from public docs
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@docs/CONFIGURATION.md`:
- Line 460: В строке документации с фрагментом `/C"runMcp...` есть непарная
двойная кавычка; исправьте эту последовательность на корректную пару кавычек
(например `/C"runMcp..."`) в описании команды `launch mcp` где упоминается
передача `port` как `mcpPort`, чтобы закрывающая кавычка после `...` была
добавлена и синтаксис строки стал корректным.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 35a61bfd-fdce-48d5-aece-67d3b895fd3c
📒 Files selected for processing (3)
docs/CONFIGURATION.mdsrc/cli/execute.rsv8-runner/references/bootstrap.md
🚧 Files skipped from review as they are similar to previous changes (1)
- src/cli/execute.rs
`/C"value"` was passed as a single argv element with literal `"` chars in the payload — `std::process::Command` bypasses the shell on both Linux (execve) and Windows (CreateProcess), so the platform received `/C"…"` as one unknown key and replied with «Неверные или отсутствующие параметры соединения с информационной базой». Splitting into `/C` and the payload as two argv tokens makes the platform parse `/C` as the command-line parameter key and the next token as its value. Tests adjusted to match the new format: argv pairs `["/C", "<value>"]` in unit tests and `/C\n<value>\n` in the integration args.log dump (printf '%s\n' "$@" puts each argv on its own line). Recovered from a dangling stash (cdb1e0a0 "fix/c-arg-quoting WIP") that had been authored locally but never committed; rebuilds lost the patch.
Plain enterprise launches now honor MCP WS flags by appending mcpMode=ws to /C without a kind field. Specialized launch mcp flows keep the existing kind-bearing payload. Tests: CC=/usr/bin/gcc cargo test --test cli_launch
Launch mcp va now mirrors the canonical Vanessa manager startup: /TESTMANAGER, unsafe-action protection disabled, /Execute vanessa-automation.epf, and VAParams without StartFeaturePlayer. WS mode publishes kind=vanessa_test_client and keeps the detached process alive in its own session.
Author
|
Обновил этот WS PR связанными follow-up fixes из fork:
Зачем это в этом PR: это не отдельная фича поверх WS, а доведение WS-режима до рабочего контура с Targeted checks in the update worktree:
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
v8-runner/SKILL.md (1)
70-103: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winУточните transport-контракт для обычных
launch thin|thick|ordinary.Сейчас этот блок читается так, будто для обычного UI-клиента поддержаны
autoи local HTTP MCP fallback, ноapply_direct_mcp_ws_launchреально пропускает там только WS-вариант; веткаMcpдля не-launch mcpзавершается validation-ошибкой. Лучше явно сузить этот текст до--mcp-transport=wsдля обычных клиентских launch-ов и оставитьauto/mcpтолько заlaunch mcp*. As per coding guidelines, "Keepv8-runner/SKILL.mdup to date when commands, workflow, configuration contracts, limits, or diagnostic practices change in ways that matter to external 1C projects; updates should be brief and avoid internal implementation detail."🤖 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 `@v8-runner/SKILL.md` around lines 70 - 103, Clarify the transport contract for the ordinary UI launch modes by updating the `launch thin|thick|ordinary` guidance to mention only `--mcp-transport=ws`; the current wording incorrectly implies `auto` and local HTTP MCP fallback are supported there. Adjust the affected bullets in `SKILL.md` around `launch mcp` / `launch mcp va` and the `Launch Modes` table so `auto` and `mcp` are documented only for `launch mcp*`, matching the actual behavior enforced by `apply_direct_mcp_ws_launch` and the non-`launch mcp` validation path.Source: Coding guidelines
🤖 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.
Outside diff comments:
In `@v8-runner/SKILL.md`:
- Around line 70-103: Clarify the transport contract for the ordinary UI launch
modes by updating the `launch thin|thick|ordinary` guidance to mention only
`--mcp-transport=ws`; the current wording incorrectly implies `auto` and local
HTTP MCP fallback are supported there. Adjust the affected bullets in `SKILL.md`
around `launch mcp` / `launch mcp va` and the `Launch Modes` table so `auto` and
`mcp` are documented only for `launch mcp*`, matching the actual behavior
enforced by `apply_direct_mcp_ws_launch` and the non-`launch mcp` validation
path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2830f5b6-b094-4fe8-95c1-74b604b251fe
📒 Files selected for processing (11)
docs/CAPABILITIES.mddocs/CONFIGURATION.mdsrc/platform/enterprise.rssrc/platform/process.rssrc/use_cases/launch_app.rssrc/use_cases/launch_keys.rssrc/use_cases/mcp_ws.rssrc/use_cases/vanessa.rstests/cli_launch.rstests/cli_test.rsv8-runner/SKILL.md
✅ Files skipped from review due to trivial changes (1)
- docs/CAPABILITIES.md
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/CONFIGURATION.md
- src/use_cases/mcp_ws.rs
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
2 participants
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.
Summary
Добавляет WS-режим для клиентского MCP: 1С-клиент может подключаться к
v8-client-session-managerпо WebSocket, а приtransport: autoбез доступного менеджера v8-runner использует локальный HTTP MCP (runMcp).После review исправлено имя публичного режима: локальный MCP-транспорт называется
mcp, неlegacy.What Changed
ws | mcp | auto.--mcp-transport=mcpфорсирует локальный HTTP MCP без probe.--mcp-transport=autoделает короткий TCP-probemanager_url: доступен manager ->ws, иначе ->mcp.--json-messageдляlaunch mcpвозвращает типизированныйtransport: "ws" | "mcp".tools.client_mcp.transportв JSON Schema ограничен enum-омws | mcp | auto;legacyтеперь отклоняется на schema/runtime boundary./Cpayload hardened: значения кодируются для защиты от;/=format injection; CLI/config дополнительно валидируют опасные значения.manager_urlвалидируется как IP:port, чтобы auto-probe не зависал на блокирующем DNS lookup.test yaxunit/test vaбольше не глотают ошибку explicit WS resolution, а возвращают structured setup failure.README,docs/CONFIGURATION.md, schemas и repo-local skillv8-runner/SKILL.md.Config
portподtools.client_mcpсохраняется для локального MCP (runMcp) режима.Review Fixes Included
v8-client-session-manageris absolute.v8-runner/references/bootstrap.md:tools.connection->infobase.connection.ws_timeout_msschema minimum is1.transport=legacyreplaced withtransport=mcpin CLI/config/docs/tests.default_corr_idpads short client ids to keepvr-<8 chars>shape.log_levelvalidation is case-insensitive and normalized for payload output.Test Plan
CC=/usr/bin/gcc UPDATE_CONFIG_SCHEMAS=1 ~/.cargo/bin/cargo test generated_schema_artifacts_are_currentCC=/usr/bin/gcc ~/.cargo/bin/cargo test schemas_and_loader_reject_invalid_client_mcp_transportCC=/usr/bin/gcc ~/.cargo/bin/cargo test mcp_wsCC=/usr/bin/gcc ~/.cargo/bin/cargo test --test cli_launchFull
cargo testwas also run locally: 684 passed, 2 environment-related failures unrelated to this PR surface (/usr/local/bin/1cedtcliautodiscovery in a not-found test, and one process kill liveness check). Existing warning remains:src/use_cases/tool_extension.rs:126unused variable.Summary by CodeRabbit
transport=auto/ws/mcpс настройкамиmanager_url,client_uid,corr_id,log_level,ws_timeout_ms, включая автопроверку и fallback на legacy MCP.v8project.yamlдляclient_mcp(транспортные параметры, ограничения и валидация)./Cи проверок параметров.