feat(mcp): WS-mode к v8-client-session-manager + skill bootstrap reference#3
Open
steel-code-agent wants to merge 12 commits into
Open
feat(mcp): WS-mode к v8-client-session-manager + skill bootstrap reference#3steel-code-agent wants to merge 12 commits into
steel-code-agent wants to merge 12 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 добавляет полную поддержку WebSocket подключений к ChangesMCP WebSocket Session-Manager Support
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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
+721
to
+723
| "--mcp-transport", | ||
| "mcp", | ||
| "--mcp-port", |
There was a problem hiding this comment.
Тесты закрепляют потенциально неверный CLI-контракт (mcp вместо legacy).
Здесь и далее новые тесты утверждают --mcp-transport mcp; если целевой публичный контракт действительно {ws|legacy|auto}, такие проверки консервируют несовместимость и маскируют регрессию в интерфейсе.
🤖 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 `@tests/cli_launch.rs` around lines 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.
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
|
|
||
| `launch mcp` передаёт это значение как `mcpPort` внутри `/C"runMcp..."` | ||
| если CLI не указал `--mcp-port`. | ||
| `launch mcp` передаёт `port` как `mcpPort` внутри `/C"runMcp..."` |
There was a problem hiding this comment.
Исправьте непарную кавычку.
В строке /C"runMcp..." отсутствует закрывающая двойная кавычка после .... Должно быть /C"runMcp..." с парными кавычками.
📝 Предлагаемое исправление
-`launch mcp` передаёт `port` как `mcpPort` внутри `/C"runMcp..."`
+`launch mcp` передаёт `port` как `mcpPort` внутри `/C"runMcp..."`🧰 Tools
🪛 LanguageTool
[typographical] ~460-~460: Непарный символ: «"» скорей всего пропущен
Context: ...pпередаётportкакmcpPortвнутри/C"runMcp..."если CLI не указал--mcp-port` и выбр...
(RU_UNPAIRED_BRACKETS)
🤖 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 `@docs/CONFIGURATION.md` at line 460, В строке документации с фрагментом
`/C"runMcp...` есть непарная двойная кавычка; исправьте эту последовательность
на корректную пару кавычек (например `/C"runMcp..."`) в описании команды `launch
mcp` где упоминается передача `port` как `mcpPort`, чтобы закрывающая кавычка
после `...` была добавлена и синтаксис строки стал корректным.
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
Новые возможности
Документация
Тесты