Skip to content

fix(web): make Skiller launch button responsive#314

Merged
skulidropek merged 5 commits into
mainfrom
issue-311
May 18, 2026
Merged

fix(web): make Skiller launch button responsive#314
skulidropek merged 5 commits into
mainfrom
issue-311

Conversation

@skulidropek
Copy link
Copy Markdown
Member

Summary

  • Pre-open the Skiller browser tab synchronously from the click handler, then navigate it to the backend-returned appPath after launch succeeds.
  • Show terminal-workspace busy/message feedback while Skiller is opening.
  • Wire Skiller into standalone terminal handlers and add regression coverage for session-scoped launches.

Fixes #311

Mathematical guarantees

Invariants

  • A Skiller click immediately creates visible feedback before the async backend launch completes.
  • Successful launches navigate only to the appPath returned by the typed Skiller launch response.
  • Project terminal Skiller actions preserve the selected projectKey and terminal session id.
  • Failed launches close the prepared popup and report the typed failure message.

Preconditions

  • Session-scoped Skiller actions require project id, project key, and terminal session id.
  • Browser popup creation may be allowed or blocked by the browser.

Postconditions

  • If popup creation is allowed, the prepared tab is navigated exactly once after launch succeeds.
  • If popup creation is blocked, the existing manual-open fallback message remains available.

Complexity

  • Skiller click handling is O(1) time / O(1) space.

Verification

  • bun run --cwd packages/app typecheck
  • bun run --cwd packages/app lint
  • bun run --cwd packages/app test -- tests/docker-git/actions-skiller.test.ts tests/docker-git/app-terminal-session-handlers.test.ts tests/docker-git/panel-terminal-skiller.test.ts
  • git diff --check

SOURCE: n/a

@skulidropek
Copy link
Copy Markdown
Member Author

skulidropek commented May 18, 2026

AI Session Backup

Commit: 69cedab
Status: success
Files: 3 (1.96 MB)
Links: README | Manifest

git status

On branch issue-311
Your branch is up to date with 'origin/issue-311'.

nothing to commit, working tree clean

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 18, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: a1c8d614-b29c-464e-83fd-ac5a6b6f3645

📥 Commits

Reviewing files that changed from the base of the PR and between 711da5e and 68d7557.

📒 Files selected for processing (4)
  • packages/app/src/web/app-ready-url.ts
  • packages/app/src/web/menu.ts
  • packages/app/src/web/screen.ts
  • packages/app/tests/docker-git/app-ready-url.test.ts
📜 Recent review details
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: E2E (Clone cache)
  • GitHub Check: E2E (Runtime volumes + SSH)
  • GitHub Check: E2E (Clone auto-open SSH)
  • GitHub Check: E2E (Login context)
  • GitHub Check: E2E (OpenCode)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: Implement Functional Core, Imperative Shell (FCIS) pattern: CORE layer contains only pure functions with immutable data and mathematical operations; SHELL layer isolates all effects (IO, network, database). Strict dependency direction: SHELL → CORE (never reverse).
Never use any, unknown, eslint-disable, ts-ignore, or as type assertions (except in rigorously justified cases with documentation). Always use exhaustive union type analysis through .exhaustive() pattern matching.
All external dependencies must be wrapped through typed interfaces and injected via Effect-TS Layer pattern. Never call external services directly from CORE functions.
Use monadic composition with Effect-TS for all effects: Effect<Success, Error, Requirements>. Compose effects through pipe() and Effect.flatMap(). Implement dependency injection via Layer pattern. Handle errors without try/catch blocks.
All functions must be pure in the CORE layer: no side effects (logging, console output, IO operations, mutations). Separate all side effects into the SHELL layer.
Use exhaustive pattern matching with Effect.Match instead of switch statements. Example: Match.value(item).pipe(Match.when(...), Match.exhaustive).
Document all functions with comprehensive TSDoc including: @pure (true/false), @effect (required services), @invariant (mathematical invariants), @precondition, @postcondition, @complexity (time and space), @throws Never (errors must be typed in Effect).
Use functional comment markers for code clarity: CHANGE (brief description), WHY (mathematical/architectural justification), QUOTE(ТЗ) (requirement citation), REF (RTM or message ID), SOURCE (external source with quote), FORMAT THEOREM (∀x ∈ Domain: P(x) → Q(f(x))), PURITY (CORE|SHELL), EFFECT (Effect type signature), INVARIANT (mathematical invariant), COMPLEXITY (time/space).
Define all external service dependencies as Context.Tag classes with fully typed methods returning Effect types. Example: `class Da...

Files:

  • packages/app/src/web/menu.ts
  • packages/app/src/web/app-ready-url.ts
  • packages/app/src/web/screen.ts
  • packages/app/tests/docker-git/app-ready-url.test.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx,js,jsx}: Forbidden constructs in CORE code: any, eslint-disable, ts-ignore, async/await, raw Promise chains (then/catch), Promise.all, try/catch for logic control, console.*, switch statements (use Match with .exhaustive() instead)
All functions must use Effect-TS for composing effects: Effect<Success, Error, Requirements>. No direct async/await, Promise chains, or try/catch in product logic.
Functional comments must include: CHANGE, WHY, QUOTE(ТЗ) or n/a, REF, SOURCE or n/a, FORMAT THEOREM, PURITY (CORE|SHELL), EFFECT signature for SHELL functions, INVARIANT, and COMPLEXITY.
All data mutations must use immutable patterns (ReadonlyArray, readonly properties, Object.freeze); mutation in SHELL only when absolutely necessary and documented.

Files:

  • packages/app/src/web/menu.ts
  • packages/app/src/web/app-ready-url.ts
  • packages/app/src/web/screen.ts
  • packages/app/tests/docker-git/app-ready-url.test.ts
**/*.{sh,bash,py,js,ts,jsx,tsx,go,java,rb,php}

📄 CodeRabbit inference engine (Custom checks)

Fail if changed files introduce command injection or unsafe shell/process execution with user-controlled input

Files:

  • packages/app/src/web/menu.ts
  • packages/app/src/web/app-ready-url.ts
  • packages/app/src/web/screen.ts
  • packages/app/tests/docker-git/app-ready-url.test.ts
**/*.{py,js,ts,jsx,tsx,go,java,rb,php,sh,bash,c,cpp}

📄 CodeRabbit inference engine (Custom checks)

Fail if changed files introduce path traversal or writes outside intended project/container state directories

Files:

  • packages/app/src/web/menu.ts
  • packages/app/src/web/app-ready-url.ts
  • packages/app/src/web/screen.ts
  • packages/app/tests/docker-git/app-ready-url.test.ts
**/*.{js,ts,jsx,tsx,py,java,go,rb,php,sh,bash,yml,yaml,json,env*,toml,cfg,config,dockerfile,dockerignore}

📄 CodeRabbit inference engine (Custom checks)

Fail if changed files expose credentials, tokens, private-keys, or PII in source, generated config, logs, or CI output

Files:

  • packages/app/src/web/menu.ts
  • packages/app/src/web/app-ready-url.ts
  • packages/app/src/web/screen.ts
  • packages/app/tests/docker-git/app-ready-url.test.ts
**/*

⚙️ CodeRabbit configuration file

**/*: Ты строгий ревьюер SPEC DRIVEN DEVELOPMENT.

Перед выводами изучи README.md, другие *.md файлы, linked issues,
PR description, PR comments/discussion и релевантную кодовую базу.

Сверь изменения с исходным ТЗ/спекой и обсуждением. Флагай любой уход
от спеки, недокументированное изменение поведения, отсутствие тестов
для заявленного поведения и security-риск. Если спека не видна,
попроси автора добавить ее в issue или PR description.

Проверь решение с точки зрения формальной верификации: какие инварианты,
предусловия и постусловия можно доказать математически, а где доказуемость
слабая. Оцени решение с точки зрения теории игр: устойчивы ли стимулы,
нет ли выгодного обхода правил, и какое решение было бы сильнее.

Files:

  • packages/app/src/web/menu.ts
  • packages/app/src/web/app-ready-url.ts
  • packages/app/src/web/screen.ts
  • packages/app/tests/docker-git/app-ready-url.test.ts
**/*.test.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.test.{ts,tsx}: Implement property-based testing using fast-check for mathematical properties and invariants. Example: fc.property(fc.array(messageArbitrary), (messages) => isChronologicallySorted(sortMessagesByTimestamp(messages))).
Mock external dependencies in unit tests using Effect's testing utilities. Run tests without Effect runtime for speed. Example: Effect.provide(MockService), Effect.runPromise.

Files:

  • packages/app/tests/docker-git/app-ready-url.test.ts
**/*.{test,spec}.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Property-based tests (fast-check) must verify mathematical invariants; unit tests must use Effect test utilities without async/await.

Files:

  • packages/app/tests/docker-git/app-ready-url.test.ts
🔇 Additional comments (4)
packages/app/tests/docker-git/app-ready-url.test.ts (1)

1-141: LGTM!

Also applies to: 218-269

packages/app/src/web/app-ready-url.ts (1)

127-127: LGTM!

packages/app/src/web/menu.ts (1)

23-23: LGTM!

packages/app/src/web/screen.ts (1)

35-52: LGTM!


📝 Walkthrough

Summary by CodeRabbit

  • Новые возможности

    • Открытие Skiller прямо из терминальной сессии (добавлен обработчик открытия).
  • Улучшения

    • Отдельный блок статуса для терминальной рабочей области.
    • Предварительная подготовка и более надёжное открытие вкладок/всплывающих окон при переходах.
    • При выборе проекта через меню Select формируется /select/... вместо /ssh/... при отсутствии терминальной сессии.
  • Исправления

    • popup Skiller теперь закрывается при неудачном запуске.
  • Тесты

    • Обновлены и добавлены тесты, покрывающие открытие окон и обработку ошибок.

Walkthrough

Рефакторит открытие URL — добавляет prepareOpenUrl/PreparedOpenUrl, интегрирует подготовленное окно в запуск Skiller, добавляет onOpenSkiller для терминальной сессии, подключает UI и обновляет тесты с фикстурой для mock-окна браузера.

Changes

Интеграция открытия Skiller через подготовленные окна браузера

Слой / Файлы Описание
Механизм открытия URL с подготовкой окна
packages/app/src/web/open-url.ts
Добавлены PreparedOpenUrl тип и prepareOpenUrl функция для предварительного открытия пустого окна перед навигацией с условным откатом на прямое открытие при недоступности; существующая openUrl остаётся без изменений.
Запуск Skiller с использованием подготовленного URL
packages/app/src/web/actions-skiller.ts
skillerLaunchMessage и тип SkillerLaunch экспортируются; добавлена openPreparedSkillerLaunch обёртка; openSkillerApp подготавливает URL до выполнения и закрывает подготовленное окно при ошибке, при успешном запуске использует openPreparedSkillerLaunch.
Обработчики терминальной сессии
packages/app/src/web/app-terminal-session-handlers.ts
Добавлен onOpenSkiller в ProjectHandlers; реализована runOpenSkiller, которая подготавливает окно, вызывает openSkiller и закрывает prepared window при ошибке; ProjectActionHandlersArgs расширен terminalSessionId; useProjectActionHandlers переработан с валидирующими хелперами.
Подключение обработчиков в UI и сессию
packages/app/src/web/app-terminal-session-ui.tsx, packages/app/src/web/app-terminal-session.tsx, packages/app/src/web/app-ready-layout.tsx
Добавлен локальный TerminalWorkspaceStatus для статуса терминальной области; передан onOpenSkiller в TerminalPanel; terminalSessionId передаётся в useProjectActionHandlers из контекста сессии.
Фикстура для мокирования открытия браузера
packages/app/tests/docker-git/browser-open-fixture.ts
Добавлены BrowserOpenMockWindow тип и утилиты makeBrowserOpenMockWindow и stubBrowserOpen для тестирования поведения открытия окна путём подмены globalThis.open.
Тесты запуска Skiller приложения
packages/app/tests/docker-git/actions-skiller.test.ts
Переработаны тесты: использование браузерной фикстуры, проверки предварительного открытия окна, навигации по URL, фокусировки и закрытия при ошибке; добавлен тест на закрытие подготовленного popup при неудаче.
Тесты обработчиков терминальной сессии
packages/app/tests/docker-git/app-terminal-session-handlers.test.ts
Добавлено мокирование terminal API и api.js, хелперы для проверки матрицы доступности обработчиков по комбинациям отсутствующих полей и эффект-тест для onOpenSkiller с проверкой навигации окна и координации с API.
Обновлённый тест readyUrlPath
packages/app/tests/docker-git/app-ready-url.test.ts
Ожидаем путь /select/<owner>/<repo> для сценария выбора SSH‑проекта при активном меню Select и экране ProjectPicker без активной терминальной сессии; добавлены property-based round-trip проверки.
Публичные экспорты вспомогательных констант
packages/app/src/web/app-ready-url.ts, packages/app/src/web/menu.ts, packages/app/src/web/screen.ts
activeScreenFromMenu, browserMenuOrder и browserProjectMenuTags стали экспортируемыми константами/функциями; внутренняя инициализация projectMenuTags теперь строится из browserProjectMenuTags.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 7
✅ Passed checks (7 passed)
Check name Status Explanation
Title check ✅ Passed Заголовок точно отражает основное изменение: делает кнопку запуска Skiller отзывчивой (responsive), что соответствует целям PR #314 и объективам issue #311.
Description check ✅ Passed Описание содержит основные секции: Summary с конкретными действиями (pre-open, show feedback, wire handlers), привязку к issue #311, математические гарантии (Invariants, Preconditions, Postconditions, Complexity) и Verification с командами проверки.
Linked Issues check ✅ Passed Изменения кода соответствуют требованиям issue #311: добавлена обработчик onOpenSkiller в терминальный session, реализована подготовка и навигация URL с видимой обратной связью пользователю, кнопка Skiller теперь функциональна и кликабельна.
Out of Scope Changes check ✅ Passed Все изменения связаны с целью make Skiller launch button responsive: инфраструктура prepareOpenUrl для async-friendly popup-handling, экспорт вспомогательных функций (activeScreenFromMenu, browserMenuOrder, browserProjectMenuTags) для поддержки тестов и рефакторинга, обновление тестов. Изменений вне scope не выявлено.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Requirements Alignment ✅ Passed PR полностью соответствует заявленным требованиям и целям. Все 4 математических гарантии реализованы, протестированы и документированы в коде.
Security Regression ✅ Passed No high-confidence security regressions detected. URL operations use proper security attributes. URLs validated against schema. Dynamic parameters properly encoded. No new risky dependencies.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-311

Comment @coderabbitai help to get the list of available commands and usage tips.

@skulidropek
Copy link
Copy Markdown
Member Author

skulidropek commented May 18, 2026

AI Session Backup

Commit: 2a9dabe
Status: success
Files: 3 (2.12 MB)
Links: README | Manifest

git status

On branch issue-311
Your branch is up to date with 'origin/issue-311'.

nothing to commit, working tree clean

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/app/tests/docker-git/actions-skiller.test.ts (1)

83-171: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Добавьте регрессионный тест для popup-blocked ветки (Line 83–171).

Сейчас нет проверки инварианта: при window.open = null должен сохраняться fallback-месседж для ручного открытия и не должно быть навигации/focus/close. Это заявлено в целях PR, но не покрыто тестом.

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/app/tests/docker-git/actions-skiller.test.ts` around lines 83 - 171,
Add a new it.effect test in the same file that simulates a blocked popup by
making browserOpenMock.mockReturnValueOnce(null), then call
openSkillerApp(context) (use makeBrowserActionContext to get context and
setMessage) and assert that the fallback/manual-open message was left via
setMessage, that there was no navigation to openedWindow
(openedWindow.location.href remains empty) and neither openedWindow.focus nor
openedWindow.close were called; reference the existing mocks openSkillerMock,
browserOpenMock and the helper makeBrowserActionContext and the function
openSkillerApp to locate where to add this regression test.
🤖 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/app/src/web/open-url.ts`:
- Around line 32-35: Функция navigate всегда возвращает true even если навигация
не произошла; исправьте вью: перед возвратом успеха проверяйте состояние
целевого окна (используя openedWindow.closed) и оборачивайте присвоение
openedWindow.location.href/focus в try/catch, возвращая false при исключении или
если openedWindow.closed === true; при false вызывайте fallback (например
открыть URL через window.open или показать инструкцию пользователю). Обновите
логику в методе navigate и там, где он вызывается, чтобы реагировать на false и
предлагать ручное открытие.

In `@packages/app/tests/docker-git/app-terminal-session-handlers.test.ts`:
- Around line 97-141: Replace the fixed examples with fast-check property tests:
generate arbitrary host URLs and combinations of projectId (string|undefined),
projectKey (string|undefined) and terminalSessionId (string|undefined) and
assert invariants using newProjectTerminalUrl and buildHandlers; for
newProjectTerminalUrl assert the returned URL preserves the host and path ends
with /ssh/{projectKey} and only includes the t= query when terminalSessionId is
defined; for buildHandlers call expectProjectHandlers(buildHandlers({ projectId,
projectKey, terminalSessionId }), expected) where expected booleans are computed
from the inputs (apply/browser/taskManager = Boolean(projectId), skiller =
Boolean(projectKey && terminalSessionId), terminal = Boolean(projectId &&
projectKey)); use fc.string()/fc.option to produce empty/defined values and run
many examples to validate invariants.

---

Outside diff comments:
In `@packages/app/tests/docker-git/actions-skiller.test.ts`:
- Around line 83-171: Add a new it.effect test in the same file that simulates a
blocked popup by making browserOpenMock.mockReturnValueOnce(null), then call
openSkillerApp(context) (use makeBrowserActionContext to get context and
setMessage) and assert that the fallback/manual-open message was left via
setMessage, that there was no navigation to openedWindow
(openedWindow.location.href remains empty) and neither openedWindow.focus nor
openedWindow.close were called; reference the existing mocks openSkillerMock,
browserOpenMock and the helper makeBrowserActionContext and the function
openSkillerApp to locate where to add this regression test.
🪄 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: 275716ec-c982-4d07-91fb-68a381fd2dcc

📥 Commits

Reviewing files that changed from the base of the PR and between 1cb62d4 and 69cedab.

📒 Files selected for processing (9)
  • packages/app/src/web/actions-skiller.ts
  • packages/app/src/web/app-ready-layout.tsx
  • packages/app/src/web/app-terminal-session-handlers.ts
  • packages/app/src/web/app-terminal-session-ui.tsx
  • packages/app/src/web/app-terminal-session.tsx
  • packages/app/src/web/open-url.ts
  • packages/app/tests/docker-git/actions-skiller.test.ts
  • packages/app/tests/docker-git/app-terminal-session-handlers.test.ts
  • packages/app/tests/docker-git/browser-open-fixture.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: Implement Functional Core, Imperative Shell (FCIS) pattern: CORE layer contains only pure functions with immutable data and mathematical operations; SHELL layer isolates all effects (IO, network, database). Strict dependency direction: SHELL → CORE (never reverse).
Never use any, unknown, eslint-disable, ts-ignore, or as type assertions (except in rigorously justified cases with documentation). Always use exhaustive union type analysis through .exhaustive() pattern matching.
All external dependencies must be wrapped through typed interfaces and injected via Effect-TS Layer pattern. Never call external services directly from CORE functions.
Use monadic composition with Effect-TS for all effects: Effect<Success, Error, Requirements>. Compose effects through pipe() and Effect.flatMap(). Implement dependency injection via Layer pattern. Handle errors without try/catch blocks.
All functions must be pure in the CORE layer: no side effects (logging, console output, IO operations, mutations). Separate all side effects into the SHELL layer.
Use exhaustive pattern matching with Effect.Match instead of switch statements. Example: Match.value(item).pipe(Match.when(...), Match.exhaustive).
Document all functions with comprehensive TSDoc including: @pure (true/false), @effect (required services), @invariant (mathematical invariants), @precondition, @postcondition, @complexity (time and space), @throws Never (errors must be typed in Effect).
Use functional comment markers for code clarity: CHANGE (brief description), WHY (mathematical/architectural justification), QUOTE(ТЗ) (requirement citation), REF (RTM or message ID), SOURCE (external source with quote), FORMAT THEOREM (∀x ∈ Domain: P(x) → Q(f(x))), PURITY (CORE|SHELL), EFFECT (Effect type signature), INVARIANT (mathematical invariant), COMPLEXITY (time/space).
Define all external service dependencies as Context.Tag classes with fully typed methods returning Effect types. Example: `class Da...

Files:

  • packages/app/src/web/app-terminal-session-ui.tsx
  • packages/app/src/web/open-url.ts
  • packages/app/src/web/app-terminal-session.tsx
  • packages/app/tests/docker-git/browser-open-fixture.ts
  • packages/app/src/web/app-ready-layout.tsx
  • packages/app/src/web/app-terminal-session-handlers.ts
  • packages/app/src/web/actions-skiller.ts
  • packages/app/tests/docker-git/app-terminal-session-handlers.test.ts
  • packages/app/tests/docker-git/actions-skiller.test.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx,js,jsx}: Forbidden constructs in CORE code: any, eslint-disable, ts-ignore, async/await, raw Promise chains (then/catch), Promise.all, try/catch for logic control, console.*, switch statements (use Match with .exhaustive() instead)
All functions must use Effect-TS for composing effects: Effect<Success, Error, Requirements>. No direct async/await, Promise chains, or try/catch in product logic.
Functional comments must include: CHANGE, WHY, QUOTE(ТЗ) or n/a, REF, SOURCE or n/a, FORMAT THEOREM, PURITY (CORE|SHELL), EFFECT signature for SHELL functions, INVARIANT, and COMPLEXITY.
All data mutations must use immutable patterns (ReadonlyArray, readonly properties, Object.freeze); mutation in SHELL only when absolutely necessary and documented.

Files:

  • packages/app/src/web/app-terminal-session-ui.tsx
  • packages/app/src/web/open-url.ts
  • packages/app/src/web/app-terminal-session.tsx
  • packages/app/tests/docker-git/browser-open-fixture.ts
  • packages/app/src/web/app-ready-layout.tsx
  • packages/app/src/web/app-terminal-session-handlers.ts
  • packages/app/src/web/actions-skiller.ts
  • packages/app/tests/docker-git/app-terminal-session-handlers.test.ts
  • packages/app/tests/docker-git/actions-skiller.test.ts
**/*.{sh,bash,py,js,ts,jsx,tsx,go,java,rb,php}

📄 CodeRabbit inference engine (Custom checks)

Fail if changed files introduce command injection or unsafe shell/process execution with user-controlled input

Files:

  • packages/app/src/web/app-terminal-session-ui.tsx
  • packages/app/src/web/open-url.ts
  • packages/app/src/web/app-terminal-session.tsx
  • packages/app/tests/docker-git/browser-open-fixture.ts
  • packages/app/src/web/app-ready-layout.tsx
  • packages/app/src/web/app-terminal-session-handlers.ts
  • packages/app/src/web/actions-skiller.ts
  • packages/app/tests/docker-git/app-terminal-session-handlers.test.ts
  • packages/app/tests/docker-git/actions-skiller.test.ts
**/*.{py,js,ts,jsx,tsx,go,java,rb,php,sh,bash,c,cpp}

📄 CodeRabbit inference engine (Custom checks)

Fail if changed files introduce path traversal or writes outside intended project/container state directories

Files:

  • packages/app/src/web/app-terminal-session-ui.tsx
  • packages/app/src/web/open-url.ts
  • packages/app/src/web/app-terminal-session.tsx
  • packages/app/tests/docker-git/browser-open-fixture.ts
  • packages/app/src/web/app-ready-layout.tsx
  • packages/app/src/web/app-terminal-session-handlers.ts
  • packages/app/src/web/actions-skiller.ts
  • packages/app/tests/docker-git/app-terminal-session-handlers.test.ts
  • packages/app/tests/docker-git/actions-skiller.test.ts
**/*.{js,ts,jsx,tsx,py,java,go,rb,php,sh,bash,yml,yaml,json,env*,toml,cfg,config,dockerfile,dockerignore}

📄 CodeRabbit inference engine (Custom checks)

Fail if changed files expose credentials, tokens, private-keys, or PII in source, generated config, logs, or CI output

Files:

  • packages/app/src/web/app-terminal-session-ui.tsx
  • packages/app/src/web/open-url.ts
  • packages/app/src/web/app-terminal-session.tsx
  • packages/app/tests/docker-git/browser-open-fixture.ts
  • packages/app/src/web/app-ready-layout.tsx
  • packages/app/src/web/app-terminal-session-handlers.ts
  • packages/app/src/web/actions-skiller.ts
  • packages/app/tests/docker-git/app-terminal-session-handlers.test.ts
  • packages/app/tests/docker-git/actions-skiller.test.ts
**/*

⚙️ CodeRabbit configuration file

**/*: Ты строгий ревьюер SPEC DRIVEN DEVELOPMENT.

Перед выводами изучи README.md, другие *.md файлы, linked issues,
PR description, PR comments/discussion и релевантную кодовую базу.

Сверь изменения с исходным ТЗ/спекой и обсуждением. Флагай любой уход
от спеки, недокументированное изменение поведения, отсутствие тестов
для заявленного поведения и security-риск. Если спека не видна,
попроси автора добавить ее в issue или PR description.

Проверь решение с точки зрения формальной верификации: какие инварианты,
предусловия и постусловия можно доказать математически, а где доказуемость
слабая. Оцени решение с точки зрения теории игр: устойчивы ли стимулы,
нет ли выгодного обхода правил, и какое решение было бы сильнее.

Files:

  • packages/app/src/web/app-terminal-session-ui.tsx
  • packages/app/src/web/open-url.ts
  • packages/app/src/web/app-terminal-session.tsx
  • packages/app/tests/docker-git/browser-open-fixture.ts
  • packages/app/src/web/app-ready-layout.tsx
  • packages/app/src/web/app-terminal-session-handlers.ts
  • packages/app/src/web/actions-skiller.ts
  • packages/app/tests/docker-git/app-terminal-session-handlers.test.ts
  • packages/app/tests/docker-git/actions-skiller.test.ts
**/*.test.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.test.{ts,tsx}: Implement property-based testing using fast-check for mathematical properties and invariants. Example: fc.property(fc.array(messageArbitrary), (messages) => isChronologicallySorted(sortMessagesByTimestamp(messages))).
Mock external dependencies in unit tests using Effect's testing utilities. Run tests without Effect runtime for speed. Example: Effect.provide(MockService), Effect.runPromise.

Files:

  • packages/app/tests/docker-git/app-terminal-session-handlers.test.ts
  • packages/app/tests/docker-git/actions-skiller.test.ts
**/*.{test,spec}.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Property-based tests (fast-check) must verify mathematical invariants; unit tests must use Effect test utilities without async/await.

Files:

  • packages/app/tests/docker-git/app-terminal-session-handlers.test.ts
  • packages/app/tests/docker-git/actions-skiller.test.ts
🔇 Additional comments (6)
packages/app/src/web/open-url.ts (1)

1-27: LGTM!

Also applies to: 29-31, 36-38

packages/app/src/web/actions-skiller.ts (1)

3-3: LGTM!

Also applies to: 5-5, 17-17, 30-34, 42-53

packages/app/src/web/app-terminal-session-handlers.ts (1)

4-4: LGTM!

Also applies to: 12-12, 18-18, 26-26, 122-205

packages/app/src/web/app-ready-layout.tsx (1)

204-224: LGTM!

Also applies to: 236-236

packages/app/src/web/app-terminal-session-ui.tsx (1)

136-136: LGTM!

packages/app/src/web/app-terminal-session.tsx (1)

144-145: LGTM!

Comment on lines +32 to +35
navigate: (url) => {
openedWindow.location.href = url
openedWindow.focus()
return true
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

navigate возвращает успех без валидации факта навигации.

На Line 35 всегда возвращается true, из-за чего можно получить ложное "Opened ..." сообщение, хотя переход не состоялся (например, вкладка уже недоступна). Лучше вернуть реальный результат и дать fallback на ручное открытие.

💡 Предлагаемое исправление
     navigate: (url) => {
-      openedWindow.location.href = url
-      openedWindow.focus()
-      return true
+      if (openedWindow.closed) {
+        return openUrl(url)
+      }
+      openedWindow.location.href = url
+      openedWindow.focus()
+      return !openedWindow.closed
     }
🤖 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/app/src/web/open-url.ts` around lines 32 - 35, Функция navigate
всегда возвращает true even если навигация не произошла; исправьте вью: перед
возвратом успеха проверяйте состояние целевого окна (используя
openedWindow.closed) и оборачивайте присвоение openedWindow.location.href/focus
в try/catch, возвращая false при исключении или если openedWindow.closed ===
true; при false вызывайте fallback (например открыть URL через window.open или
показать инструкцию пользователю). Обновите логику в методе navigate и там, где
он вызывается, чтобы реагировать на false и предлагать ручное открытие.

Comment on lines 97 to 141
it("builds new terminal popups as project SSH links with a terminal selector", () => {
expect(
newProjectTerminalUrl("https://docker-git.local", "octocat/hello-world", "session-1")
).toBe("https://docker-git.local/ssh/octocat/hello-world?t=session-1")
})

it("returns all four project action handlers when project context is present", () => {
const handlers = buildHandlers()
expect(typeof handlers.onApplyProject).toBe("function")
expect(typeof handlers.onOpenBrowser).toBe("function")
expect(typeof handlers.onOpenTaskManager).toBe("function")
expect(typeof handlers.onOpenTerminal).toBe("function")
it("returns all project action handlers when project context is present", () => {
expectProjectHandlers(buildHandlers(), {
apply: true,
browser: true,
skiller: true,
taskManager: true,
terminal: true
})
})

it("hides all handlers when projectId is missing", () => {
const handlers = buildHandlers({ projectId: undefined })
expect(handlers.onApplyProject).toBeUndefined()
expect(handlers.onOpenBrowser).toBeUndefined()
expect(handlers.onOpenTaskManager).toBeUndefined()
expect(handlers.onOpenTerminal).toBeUndefined()
expectProjectHandlers(buildHandlers({ projectId: undefined }), {
apply: false,
browser: false,
skiller: false,
taskManager: false,
terminal: false
})
})

it("hides project-key dependent handlers when projectKey is missing", () => {
expectProjectHandlers(buildHandlers({ projectKey: undefined }), {
apply: true,
browser: true,
skiller: false,
taskManager: true,
terminal: false
})
})

it("hides only onOpenTerminal when projectKey is missing", () => {
const handlers = buildHandlers({ projectKey: undefined })
expect(typeof handlers.onApplyProject).toBe("function")
expect(typeof handlers.onOpenBrowser).toBe("function")
expect(typeof handlers.onOpenTaskManager).toBe("function")
expect(handlers.onOpenTerminal).toBeUndefined()
it("hides only onOpenSkiller when terminalSessionId is missing", () => {
expectProjectHandlers(buildHandlers({ terminalSessionId: undefined }), {
apply: true,
browser: true,
skiller: false,
taskManager: true,
terminal: true
})
})
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift

Усилите инвариантные тесты через fast-check (Line 97–141).

Для newProjectTerminalUrl и матрицы useProjectActionHandlers нужны property-based тесты (генерация комбинаций projectId/projectKey/terminalSessionId и проверка инвариантов доступности handler-ов), а не только фиксированные кейсы.

As per coding guidelines "**/*.test.{ts,tsx}: Implement property-based testing using fast-check for mathematical properties and invariants" и "**/*.{test,spec}.{ts,tsx}: Property-based tests (fast-check) must verify mathematical invariants".

🤖 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/app/tests/docker-git/app-terminal-session-handlers.test.ts` around
lines 97 - 141, Replace the fixed examples with fast-check property tests:
generate arbitrary host URLs and combinations of projectId (string|undefined),
projectKey (string|undefined) and terminalSessionId (string|undefined) and
assert invariants using newProjectTerminalUrl and buildHandlers; for
newProjectTerminalUrl assert the returned URL preserves the host and path ends
with /ssh/{projectKey} and only includes the t= query when terminalSessionId is
defined; for buildHandlers call expectProjectHandlers(buildHandlers({ projectId,
projectKey, terminalSessionId }), expected) where expected booleans are computed
from the inputs (apply/browser/taskManager = Boolean(projectId), skiller =
Boolean(projectKey && terminalSessionId), terminal = Boolean(projectId &&
projectKey)); use fc.string()/fc.option to produce empty/defined values and run
many examples to validate invariants.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

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 `@packages/app/tests/docker-git/app-ready-url.test.ts`:
- Around line 108-116: Add a new unit test that verifies parseReadyUrlNavigation
round-trips the `/select/` deep-link back into navigation state: call
parseReadyUrlNavigation("https://docker-git.local/select/octocat/hello-world",
dashboard.projects) and assert it equals { activeScreen: { tag: "ProjectPicker"
}, menu: "Select", projectNavigationArmed: false, selectedProjectId: "project-1"
}; this mirrors the existing readyUrlPath test for Select and the /browser/
parse test, and ensures parseReadyUrlNavigation correctly handles project menus
(Select is in projectMenuTags).
🪄 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: 22971695-a6c0-4280-bbbc-8fb7e56e59b2

📥 Commits

Reviewing files that changed from the base of the PR and between 69cedab and 2a9dabe.

📒 Files selected for processing (1)
  • packages/app/tests/docker-git/app-ready-url.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
  • GitHub Check: E2E (Browser command)
  • GitHub Check: E2E (OpenCode)
  • GitHub Check: E2E (Clone cache)
  • GitHub Check: E2E (Runtime volumes + SSH)
  • GitHub Check: E2E (Clone auto-open SSH)
  • GitHub Check: E2E (Login context)
  • GitHub Check: Lint
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: Implement Functional Core, Imperative Shell (FCIS) pattern: CORE layer contains only pure functions with immutable data and mathematical operations; SHELL layer isolates all effects (IO, network, database). Strict dependency direction: SHELL → CORE (never reverse).
Never use any, unknown, eslint-disable, ts-ignore, or as type assertions (except in rigorously justified cases with documentation). Always use exhaustive union type analysis through .exhaustive() pattern matching.
All external dependencies must be wrapped through typed interfaces and injected via Effect-TS Layer pattern. Never call external services directly from CORE functions.
Use monadic composition with Effect-TS for all effects: Effect<Success, Error, Requirements>. Compose effects through pipe() and Effect.flatMap(). Implement dependency injection via Layer pattern. Handle errors without try/catch blocks.
All functions must be pure in the CORE layer: no side effects (logging, console output, IO operations, mutations). Separate all side effects into the SHELL layer.
Use exhaustive pattern matching with Effect.Match instead of switch statements. Example: Match.value(item).pipe(Match.when(...), Match.exhaustive).
Document all functions with comprehensive TSDoc including: @pure (true/false), @effect (required services), @invariant (mathematical invariants), @precondition, @postcondition, @complexity (time and space), @throws Never (errors must be typed in Effect).
Use functional comment markers for code clarity: CHANGE (brief description), WHY (mathematical/architectural justification), QUOTE(ТЗ) (requirement citation), REF (RTM or message ID), SOURCE (external source with quote), FORMAT THEOREM (∀x ∈ Domain: P(x) → Q(f(x))), PURITY (CORE|SHELL), EFFECT (Effect type signature), INVARIANT (mathematical invariant), COMPLEXITY (time/space).
Define all external service dependencies as Context.Tag classes with fully typed methods returning Effect types. Example: `class Da...

Files:

  • packages/app/tests/docker-git/app-ready-url.test.ts
**/*.test.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.test.{ts,tsx}: Implement property-based testing using fast-check for mathematical properties and invariants. Example: fc.property(fc.array(messageArbitrary), (messages) => isChronologicallySorted(sortMessagesByTimestamp(messages))).
Mock external dependencies in unit tests using Effect's testing utilities. Run tests without Effect runtime for speed. Example: Effect.provide(MockService), Effect.runPromise.

Files:

  • packages/app/tests/docker-git/app-ready-url.test.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx,js,jsx}: Forbidden constructs in CORE code: any, eslint-disable, ts-ignore, async/await, raw Promise chains (then/catch), Promise.all, try/catch for logic control, console.*, switch statements (use Match with .exhaustive() instead)
All functions must use Effect-TS for composing effects: Effect<Success, Error, Requirements>. No direct async/await, Promise chains, or try/catch in product logic.
Functional comments must include: CHANGE, WHY, QUOTE(ТЗ) or n/a, REF, SOURCE or n/a, FORMAT THEOREM, PURITY (CORE|SHELL), EFFECT signature for SHELL functions, INVARIANT, and COMPLEXITY.
All data mutations must use immutable patterns (ReadonlyArray, readonly properties, Object.freeze); mutation in SHELL only when absolutely necessary and documented.

Files:

  • packages/app/tests/docker-git/app-ready-url.test.ts
**/*.{test,spec}.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Property-based tests (fast-check) must verify mathematical invariants; unit tests must use Effect test utilities without async/await.

Files:

  • packages/app/tests/docker-git/app-ready-url.test.ts
**/*.{sh,bash,py,js,ts,jsx,tsx,go,java,rb,php}

📄 CodeRabbit inference engine (Custom checks)

Fail if changed files introduce command injection or unsafe shell/process execution with user-controlled input

Files:

  • packages/app/tests/docker-git/app-ready-url.test.ts
**/*.{py,js,ts,jsx,tsx,go,java,rb,php,sh,bash,c,cpp}

📄 CodeRabbit inference engine (Custom checks)

Fail if changed files introduce path traversal or writes outside intended project/container state directories

Files:

  • packages/app/tests/docker-git/app-ready-url.test.ts
**/*.{js,ts,jsx,tsx,py,java,go,rb,php,sh,bash,yml,yaml,json,env*,toml,cfg,config,dockerfile,dockerignore}

📄 CodeRabbit inference engine (Custom checks)

Fail if changed files expose credentials, tokens, private-keys, or PII in source, generated config, logs, or CI output

Files:

  • packages/app/tests/docker-git/app-ready-url.test.ts
**/*

⚙️ CodeRabbit configuration file

**/*: Ты строгий ревьюер SPEC DRIVEN DEVELOPMENT.

Перед выводами изучи README.md, другие *.md файлы, linked issues,
PR description, PR comments/discussion и релевантную кодовую базу.

Сверь изменения с исходным ТЗ/спекой и обсуждением. Флагай любой уход
от спеки, недокументированное изменение поведения, отсутствие тестов
для заявленного поведения и security-риск. Если спека не видна,
попроси автора добавить ее в issue или PR description.

Проверь решение с точки зрения формальной верификации: какие инварианты,
предусловия и постусловия можно доказать математически, а где доказуемость
слабая. Оцени решение с точки зрения теории игр: устойчивы ли стимулы,
нет ли выгодного обхода правил, и какое решение было бы сильнее.

Files:

  • packages/app/tests/docker-git/app-ready-url.test.ts

Comment thread packages/app/tests/docker-git/app-ready-url.test.ts Outdated
@skulidropek
Copy link
Copy Markdown
Member Author

skulidropek commented May 18, 2026

AI Session Backup

Commit: 58b5a08
Status: success
Files: 3 (2.32 MB)
Links: README | Manifest

git status

On branch issue-311
Your branch is up to date with 'origin/issue-311'.

nothing to commit, working tree clean

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 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/app/tests/docker-git/app-ready-url.test.ts`:
- Around line 129-138: Add a property-based test using fast-check that generates
valid navigation states and asserts the round-trip invariant: for each generated
state call readyUrlPath(state) to produce a URL, feed it into
parseReadyUrlNavigation(fullUrl, dashboard.projects), and verify the parsed
result matches the normalized key fields of the original state (e.g.,
menu/currentMenu and selectedProjectId, and activeScreen tag) or null only when
expected; implement a navigationState arbitrary that mirrors valid shapes used
by readyUrlPath (activeScreen tags, currentMenu values, selectedProjectId
optional, etc.), and use fc.assert(fc.property(...)) to run the round-trip
checks.
- Line 108: Rename the test case title to accurately reflect that it checks the
Select menu URL template (not an active SSH session); update the it(...) string
currently "renders SSH project selection as a readable Select project deep link"
to a clearer name such as "renders Select project deep link for non-terminal
sessions" or "renders Select menu project deep link (not SSH)" so the test
description matches the checked URL template in the test file and avoids
implying an active SSH/terminal session.
🪄 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: a7705f8c-68c8-4f6e-9c77-8a60a5f1e26e

📥 Commits

Reviewing files that changed from the base of the PR and between 2a9dabe and 58b5a08.

📒 Files selected for processing (1)
  • packages/app/tests/docker-git/app-ready-url.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
  • GitHub Check: E2E (Browser command)
  • GitHub Check: E2E (OpenCode)
  • GitHub Check: Lint
  • GitHub Check: E2E (Clone cache)
  • GitHub Check: E2E (Login context)
  • GitHub Check: E2E (Runtime volumes + SSH)
  • GitHub Check: E2E (Clone auto-open SSH)
  • GitHub Check: Test
  • GitHub Check: Final build (windows-latest)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: Implement Functional Core, Imperative Shell (FCIS) pattern: CORE layer contains only pure functions with immutable data and mathematical operations; SHELL layer isolates all effects (IO, network, database). Strict dependency direction: SHELL → CORE (never reverse).
Never use any, unknown, eslint-disable, ts-ignore, or as type assertions (except in rigorously justified cases with documentation). Always use exhaustive union type analysis through .exhaustive() pattern matching.
All external dependencies must be wrapped through typed interfaces and injected via Effect-TS Layer pattern. Never call external services directly from CORE functions.
Use monadic composition with Effect-TS for all effects: Effect<Success, Error, Requirements>. Compose effects through pipe() and Effect.flatMap(). Implement dependency injection via Layer pattern. Handle errors without try/catch blocks.
All functions must be pure in the CORE layer: no side effects (logging, console output, IO operations, mutations). Separate all side effects into the SHELL layer.
Use exhaustive pattern matching with Effect.Match instead of switch statements. Example: Match.value(item).pipe(Match.when(...), Match.exhaustive).
Document all functions with comprehensive TSDoc including: @pure (true/false), @effect (required services), @invariant (mathematical invariants), @precondition, @postcondition, @complexity (time and space), @throws Never (errors must be typed in Effect).
Use functional comment markers for code clarity: CHANGE (brief description), WHY (mathematical/architectural justification), QUOTE(ТЗ) (requirement citation), REF (RTM or message ID), SOURCE (external source with quote), FORMAT THEOREM (∀x ∈ Domain: P(x) → Q(f(x))), PURITY (CORE|SHELL), EFFECT (Effect type signature), INVARIANT (mathematical invariant), COMPLEXITY (time/space).
Define all external service dependencies as Context.Tag classes with fully typed methods returning Effect types. Example: `class Da...

Files:

  • packages/app/tests/docker-git/app-ready-url.test.ts
**/*.test.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.test.{ts,tsx}: Implement property-based testing using fast-check for mathematical properties and invariants. Example: fc.property(fc.array(messageArbitrary), (messages) => isChronologicallySorted(sortMessagesByTimestamp(messages))).
Mock external dependencies in unit tests using Effect's testing utilities. Run tests without Effect runtime for speed. Example: Effect.provide(MockService), Effect.runPromise.

Files:

  • packages/app/tests/docker-git/app-ready-url.test.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx,js,jsx}: Forbidden constructs in CORE code: any, eslint-disable, ts-ignore, async/await, raw Promise chains (then/catch), Promise.all, try/catch for logic control, console.*, switch statements (use Match with .exhaustive() instead)
All functions must use Effect-TS for composing effects: Effect<Success, Error, Requirements>. No direct async/await, Promise chains, or try/catch in product logic.
Functional comments must include: CHANGE, WHY, QUOTE(ТЗ) or n/a, REF, SOURCE or n/a, FORMAT THEOREM, PURITY (CORE|SHELL), EFFECT signature for SHELL functions, INVARIANT, and COMPLEXITY.
All data mutations must use immutable patterns (ReadonlyArray, readonly properties, Object.freeze); mutation in SHELL only when absolutely necessary and documented.

Files:

  • packages/app/tests/docker-git/app-ready-url.test.ts
**/*.{test,spec}.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Property-based tests (fast-check) must verify mathematical invariants; unit tests must use Effect test utilities without async/await.

Files:

  • packages/app/tests/docker-git/app-ready-url.test.ts
**/*.{sh,bash,py,js,ts,jsx,tsx,go,java,rb,php}

📄 CodeRabbit inference engine (Custom checks)

Fail if changed files introduce command injection or unsafe shell/process execution with user-controlled input

Files:

  • packages/app/tests/docker-git/app-ready-url.test.ts
**/*.{py,js,ts,jsx,tsx,go,java,rb,php,sh,bash,c,cpp}

📄 CodeRabbit inference engine (Custom checks)

Fail if changed files introduce path traversal or writes outside intended project/container state directories

Files:

  • packages/app/tests/docker-git/app-ready-url.test.ts
**/*.{js,ts,jsx,tsx,py,java,go,rb,php,sh,bash,yml,yaml,json,env*,toml,cfg,config,dockerfile,dockerignore}

📄 CodeRabbit inference engine (Custom checks)

Fail if changed files expose credentials, tokens, private-keys, or PII in source, generated config, logs, or CI output

Files:

  • packages/app/tests/docker-git/app-ready-url.test.ts
**/*

⚙️ CodeRabbit configuration file

**/*: Ты строгий ревьюер SPEC DRIVEN DEVELOPMENT.

Перед выводами изучи README.md, другие *.md файлы, linked issues,
PR description, PR comments/discussion и релевантную кодовую базу.

Сверь изменения с исходным ТЗ/спекой и обсуждением. Флагай любой уход
от спеки, недокументированное изменение поведения, отсутствие тестов
для заявленного поведения и security-риск. Если спека не видна,
попроси автора добавить ее в issue или PR description.

Проверь решение с точки зрения формальной верификации: какие инварианты,
предусловия и постусловия можно доказать математически, а где доказуемость
слабая. Оцени решение с точки зрения теории игр: устойчивы ли стимулы,
нет ли выгодного обхода правил, и какое решение было бы сильнее.

Files:

  • packages/app/tests/docker-git/app-ready-url.test.ts
🔇 Additional comments (1)
packages/app/tests/docker-git/app-ready-url.test.ts (1)

129-138: LGTM!

Comment thread packages/app/tests/docker-git/app-ready-url.test.ts Outdated
Comment thread packages/app/tests/docker-git/app-ready-url.test.ts
@skulidropek
Copy link
Copy Markdown
Member Author

skulidropek commented May 18, 2026

AI Session Backup

Commit: 711da5e
Status: success
Files: 3 (2.81 MB)
Links: README | Manifest

git status

On branch issue-311
Your branch is up to date with 'origin/issue-311'.

nothing to commit, working tree clean

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/app/tests/docker-git/app-ready-url.test.ts (1)

112-318: 🧹 Nitpick | 🔵 Trivial | 💤 Low value

Отличная реализация property-based тестирования! ✨

Код демонстрирует правильное применение fast-check для проверки математических инвариантов:

  • ✅ Определены чёткие arbitraries для различных сценариев навигации
  • ✅ Проверяется инвариант round-trip: ∀state: parseReadyUrlNavigation(readyUrlPath(state)) ≈ normalize(state)
  • ✅ Покрыты различные типы меню (project/non-project) и состояний выбора проекта
  • ✅ Использовано достаточное количество запусков (75) для надёжности

Это значительно повышает уверенность в корректности навигации и обработки URL при изменении схемы роутинга. As per coding guidelines: "Implement property-based testing using fast-check for mathematical properties and invariants."

Единственная рекомендация: добавьте краткий TSDoc-комментарий с формальным определением проверяемого инварианта в формате THEOREM для будущих разработчиков:

/**
 * THEOREM (Round-trip invariant):
 * ∀state ∈ ValidNavigationState:
 *   parseReadyUrlNavigation(readyUrlPath(state), projects) = normalize(state)
 * 
 * where normalize removes fields not preserved in URL encoding
 * (e.g., activeTerminalSession, selectedProjectSummary)
 */
it("preserves ready URL round-trip invariants for valid navigation states", () => {
🤖 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/app/tests/docker-git/app-ready-url.test.ts` around lines 112 - 318,
Add a short TSDoc THEOREM comment immediately above the test titled "preserves
ready URL round-trip invariants for valid navigation states" that formally
states the round-trip invariant for readyUrlPath and parseReadyUrlNavigation
(e.g., ∀state ∈ ValidNavigationState:
parseReadyUrlNavigation(readyUrlPath(state), projects) = normalize(state)), and
mention which fields are not preserved (activeTerminalSession,
selectedProjectSummary); place the comment directly before the it(...) block so
future readers can see the formal property being tested for readyUrlPath and
parseReadyUrlNavigation.
🤖 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/app/tests/docker-git/app-ready-url.test.ts`:
- Around line 91-110: Replace the hard-coded parsedScreenByMenu map with a
dynamic build that calls the existing production function screenForMenu for each
menu tag (use browserMenuItems to iterate over items and produce a
Record<BrowserMenuTag, BrowserScreen>), so parsedScreenByMenu is constructed
from Object.fromEntries(browserMenuItems.map(item => [item.tag,
screenForMenu(item.tag)])) and cast back to the appropriate Record type; update
any references to parsedScreenByMenu to use this new value so the test relies on
the production mapping logic rather than a duplicated static map.
- Around line 309-312: The test contains contradictory null handling: it asserts
expect(path).not.toBeNull() but then silently returns if path === null; remove
the redundant guard so the assertion fails loudly (delete the if (path === null)
{ return } block), or if null is an acceptable outcome for some generated
inputs, instead enforce that by filtering inputs with fast-check using
fc.pre(...) before calling readyUrlPath so the test only runs on inputs that
should produce non-null; locate the check around the readyUrlPath call and the
variable path to apply the change.
- Around line 52-89: The test duplicates private menu constants which risks
drifting from the source; update the test to import the canonical definitions
instead of hardcoding them: export and import browserMenuOrder from
packages/app/src/web/menu.ts (currently mirrored by menuTags) and reuse the
projectMenuTags definition (or derive via the exported isProjectMenu function
and BrowserProjectMenuTag type from packages/app/src/web/screen.ts) in the test,
and compute nonProjectMenuTags dynamically as the set difference/inversion of
browserMenuOrder vs projectMenuTags rather than a hardcoded array.
- Around line 160-162: Test contains special-case logic for "Select" that
diverges from production: change the test so activeScreen uses the same behavior
as screenForMenu by removing the Select-specific branch and always using
parsedScreenByMenu[menu] (so project menus like "Select" resolve to
projectPickerScreen), i.e., delete the conditional that returns menuScreen for
Select with null selection and let parsedScreenByMenu/activeScreen mirror
production; alternatively, if the special-case is intended, implement matching
logic in screenForMenu/isProjectMenu to return menuScreen for that scenario, but
prefer updating the test to rely on parsedScreenByMenu, projectPickerScreen,
screenForMenu and isProjectMenu to match production.

---

Outside diff comments:
In `@packages/app/tests/docker-git/app-ready-url.test.ts`:
- Around line 112-318: Add a short TSDoc THEOREM comment immediately above the
test titled "preserves ready URL round-trip invariants for valid navigation
states" that formally states the round-trip invariant for readyUrlPath and
parseReadyUrlNavigation (e.g., ∀state ∈ ValidNavigationState:
parseReadyUrlNavigation(readyUrlPath(state), projects) = normalize(state)), and
mention which fields are not preserved (activeTerminalSession,
selectedProjectSummary); place the comment directly before the it(...) block so
future readers can see the formal property being tested for readyUrlPath and
parseReadyUrlNavigation.
🪄 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: 8ea9d8cf-3e05-4578-b0b6-79a3421edf05

📥 Commits

Reviewing files that changed from the base of the PR and between 58b5a08 and 711da5e.

📒 Files selected for processing (1)
  • packages/app/tests/docker-git/app-ready-url.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
  • GitHub Check: E2E (Runtime volumes + SSH)
  • GitHub Check: E2E (Clone auto-open SSH)
  • GitHub Check: E2E (Browser command)
  • GitHub Check: E2E (Clone cache)
  • GitHub Check: Lint
  • GitHub Check: Final build (windows-latest)
  • GitHub Check: E2E (OpenCode)
  • GitHub Check: Test
  • GitHub Check: E2E (Login context)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: Implement Functional Core, Imperative Shell (FCIS) pattern: CORE layer contains only pure functions with immutable data and mathematical operations; SHELL layer isolates all effects (IO, network, database). Strict dependency direction: SHELL → CORE (never reverse).
Never use any, unknown, eslint-disable, ts-ignore, or as type assertions (except in rigorously justified cases with documentation). Always use exhaustive union type analysis through .exhaustive() pattern matching.
All external dependencies must be wrapped through typed interfaces and injected via Effect-TS Layer pattern. Never call external services directly from CORE functions.
Use monadic composition with Effect-TS for all effects: Effect<Success, Error, Requirements>. Compose effects through pipe() and Effect.flatMap(). Implement dependency injection via Layer pattern. Handle errors without try/catch blocks.
All functions must be pure in the CORE layer: no side effects (logging, console output, IO operations, mutations). Separate all side effects into the SHELL layer.
Use exhaustive pattern matching with Effect.Match instead of switch statements. Example: Match.value(item).pipe(Match.when(...), Match.exhaustive).
Document all functions with comprehensive TSDoc including: @pure (true/false), @effect (required services), @invariant (mathematical invariants), @precondition, @postcondition, @complexity (time and space), @throws Never (errors must be typed in Effect).
Use functional comment markers for code clarity: CHANGE (brief description), WHY (mathematical/architectural justification), QUOTE(ТЗ) (requirement citation), REF (RTM or message ID), SOURCE (external source with quote), FORMAT THEOREM (∀x ∈ Domain: P(x) → Q(f(x))), PURITY (CORE|SHELL), EFFECT (Effect type signature), INVARIANT (mathematical invariant), COMPLEXITY (time/space).
Define all external service dependencies as Context.Tag classes with fully typed methods returning Effect types. Example: `class Da...

Files:

  • packages/app/tests/docker-git/app-ready-url.test.ts
**/*.test.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.test.{ts,tsx}: Implement property-based testing using fast-check for mathematical properties and invariants. Example: fc.property(fc.array(messageArbitrary), (messages) => isChronologicallySorted(sortMessagesByTimestamp(messages))).
Mock external dependencies in unit tests using Effect's testing utilities. Run tests without Effect runtime for speed. Example: Effect.provide(MockService), Effect.runPromise.

Files:

  • packages/app/tests/docker-git/app-ready-url.test.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx,js,jsx}: Forbidden constructs in CORE code: any, eslint-disable, ts-ignore, async/await, raw Promise chains (then/catch), Promise.all, try/catch for logic control, console.*, switch statements (use Match with .exhaustive() instead)
All functions must use Effect-TS for composing effects: Effect<Success, Error, Requirements>. No direct async/await, Promise chains, or try/catch in product logic.
Functional comments must include: CHANGE, WHY, QUOTE(ТЗ) or n/a, REF, SOURCE or n/a, FORMAT THEOREM, PURITY (CORE|SHELL), EFFECT signature for SHELL functions, INVARIANT, and COMPLEXITY.
All data mutations must use immutable patterns (ReadonlyArray, readonly properties, Object.freeze); mutation in SHELL only when absolutely necessary and documented.

Files:

  • packages/app/tests/docker-git/app-ready-url.test.ts
**/*.{test,spec}.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Property-based tests (fast-check) must verify mathematical invariants; unit tests must use Effect test utilities without async/await.

Files:

  • packages/app/tests/docker-git/app-ready-url.test.ts
**/*.{sh,bash,py,js,ts,jsx,tsx,go,java,rb,php}

📄 CodeRabbit inference engine (Custom checks)

Fail if changed files introduce command injection or unsafe shell/process execution with user-controlled input

Files:

  • packages/app/tests/docker-git/app-ready-url.test.ts
**/*.{py,js,ts,jsx,tsx,go,java,rb,php,sh,bash,c,cpp}

📄 CodeRabbit inference engine (Custom checks)

Fail if changed files introduce path traversal or writes outside intended project/container state directories

Files:

  • packages/app/tests/docker-git/app-ready-url.test.ts
**/*.{js,ts,jsx,tsx,py,java,go,rb,php,sh,bash,yml,yaml,json,env*,toml,cfg,config,dockerfile,dockerignore}

📄 CodeRabbit inference engine (Custom checks)

Fail if changed files expose credentials, tokens, private-keys, or PII in source, generated config, logs, or CI output

Files:

  • packages/app/tests/docker-git/app-ready-url.test.ts
**/*

⚙️ CodeRabbit configuration file

**/*: Ты строгий ревьюер SPEC DRIVEN DEVELOPMENT.

Перед выводами изучи README.md, другие *.md файлы, linked issues,
PR description, PR comments/discussion и релевантную кодовую базу.

Сверь изменения с исходным ТЗ/спекой и обсуждением. Флагай любой уход
от спеки, недокументированное изменение поведения, отсутствие тестов
для заявленного поведения и security-риск. Если спека не видна,
попроси автора добавить ее в issue или PR description.

Проверь решение с точки зрения формальной верификации: какие инварианты,
предусловия и постусловия можно доказать математически, а где доказуемость
слабая. Оцени решение с точки зрения теории игр: устойчивы ли стимулы,
нет ли выгодного обхода правил, и какое решение было бы сильнее.

Files:

  • packages/app/tests/docker-git/app-ready-url.test.ts
🔇 Additional comments (3)
packages/app/tests/docker-git/app-ready-url.test.ts (3)

1-19: LGTM!


272-280: LGTM!


293-302: LGTM!

Comment thread packages/app/tests/docker-git/app-ready-url.test.ts Outdated
Comment thread packages/app/tests/docker-git/app-ready-url.test.ts Outdated
Comment thread packages/app/tests/docker-git/app-ready-url.test.ts Outdated
Comment thread packages/app/tests/docker-git/app-ready-url.test.ts Outdated
@skulidropek
Copy link
Copy Markdown
Member Author

skulidropek commented May 18, 2026

AI Session Backup

Commit: 68d7557
Status: success
Files: 3 (3.34 MB)
Links: README | Manifest

git status

On branch issue-311
Your branch is up to date with 'origin/issue-311'.

nothing to commit, working tree clean

@skulidropek skulidropek merged commit 6066ede into main May 18, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Разобраться почему не работает skiller

1 participant