fix(web): retain prepared browser popup handle#360
Conversation
|
Warning Review limit reached
More reviews will be available in 35 minutes and 3 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughИзменения расширяют логику инициализации контроллера и проверки здоровья браузера: добавлена проверка ревизии контроллера при выборе достижимого endpoint'а, вынесена диагностика достижимости в отдельный модуль, реализована проверка готовности браузерного runtime через TCP-зонды к портам NoVNC/VNC/CDP, и оптимизировано открытие окна браузера без предварительной передачи "noopener". ChangesИнициализация контроллера и готовность браузера
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5 | ❌ 2❌ Failed checks (2 inconclusive)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/app/tests/docker-git/actions-browser.test.ts (1)
53-97: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winДобавьте property-based проверку инварианта открытия popup.
Сейчас проверки только примерные; для этого файла по правилам нужны fast-check свойства (например, что для любого валидного URL сообщение/ветка
opened vs blockedсогласованы с результатомnavigate).As per coding guidelines:
**/*.test.{ts,tsx}: Implement property-based testing using fast-check for mathematical properties and 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/actions-browser.test.ts` around lines 53 - 97, Add a fast-check property-based test that asserts the popup-open invariant for openProjectBrowserById and openSelectedProjectBrowser: generate valid URL strings (use fast-check's webUrl or oneOf/arbitrary for path fragments), stubBrowserOpen to simulate both opened and blocked outcomes and mock openedWindow.location.navigate (or its navigate result) to return success/failure, then for each generated URL call the action via makeBrowserActionContext and waitForAssertion and assert that setProjectBrowser, startProjectBrowserMock (when applicable), openedWindow.close, openMock, and setMessage are all consistent with the navigate result (i.e., when navigate succeeds expect opened vs blocked message and browser state to match; when blocked expect closed window and blocked message). Reference the existing helpers openProjectBrowserById, openSelectedProjectBrowser, stubBrowserOpen, makeBrowserActionContext, startProjectBrowserMock, openedWindow, setMessage and setProjectBrowser so the property test plugs into current mocks and assertions.packages/app/tests/docker-git/actions-skiller.test.ts (1)
83-104: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winНужен fast-check тест на инварианты запуска Skiller.
Добавьте property-based сценарий (например, для множества
appPath/projectKey) с инвариантом: при успешном запуске URL навигации и финальное сообщение всегда консистентны сopened.As per coding guidelines:
**/*.test.{ts,tsx}: Implement property-based testing using fast-check for mathematical properties and 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/actions-skiller.test.ts` around lines 83 - 104, Add a fast-check property-based test that generates diverse appPath and projectKey values and asserts the Skiller launch invariants: for each generated input, mock openSkillerMock to return a skillerLaunch result tied to those values, call openSkillerApp using the test harness from makeBrowserActionContext, and assert that browserOpenMock was called with "about:blank", openedWindow.location.href resolves to the expected "/api/skiller/app/" (or derived path using the generated appPath), openedWindow.focus was called, and context.setMessage/setBusyLabel final messages include the same launch pid/log/path derived from the mocked skillerLaunch; use waitForAssertion as in the existing test to await async effects and parametrize inputs with fast-check arbitraries for appPath and projectKey.
🤖 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/docker-git/controller-health.ts`:
- Around line 121-140: The direct health-probe path must respect
expectedRevision: add an expectedRevision?: string parameter to
findReachableDirectHealthProbe and propagate it into
findReachableHealthProbeOrNull (also adding expectedRevision?: string there),
ensure both functions use expectedRevision when evaluating health.revision
(reject mismatches), and update all callers (including ensureControllerReady’s
early pre-bootstrap probe and any other places that call
findReachableDirectHealthProbe/findReachableHealthProbeOrNull) to pass the
expectedRevision from
findReachableApiBaseUrl/findReachableApiBaseUrlMatchingRevision so the early
direct probe cannot return a stale controller.
In `@packages/app/src/docker-git/controller-reachability-diagnostics.ts`:
- Around line 6-21: Add a TSDoc functional comment above the exported
collectReachabilityDiagnostics function that documents its contract and markers:
include `@pure` (false), `@effect` (requires
ControllerDocker.inspectControllerPublishedPorts service), `@invariant` (returns a
diagnostic string for controller runtime), `@complexity` (O(n) where n is
candidateUrls.length), and top-line functional markers (PURITY, EFFECT,
INVARIANT, COMPLEXITY, CHANGE/WHY). Also describe parameters (candidateUrls,
currentContainerNetworks, controllerNetworks), the Effect return type
(Effect.Effect<string, never, ControllerDocker.ControllerRuntime>), and note the
dependency on ControllerDocker.inspectControllerPublishedPorts so reviewers can
verify side-effects and dependencies.
In `@packages/app/tests/docker-git/controller-health.test.ts`:
- Around line 25-40: Replace the global mutation helper with a test
Layer/Effect-based mock and convert the tests to property-based checks: remove
withFetchMock (which mutates globalThis.fetch) and instead create a test Layer
or provideService that supplies a mock Fetch implementation to the Effect under
test (use Effect.provideLayer / Effect.provideService or the project's test
utilities), then rewrite the scenarios using fast-check to assert the two
invariants: for arbitrary lists of endpoints the Effect (the controller
selection function invoked in controller-health.test.ts) picks the first
reachable candidate whose revision === expectedRevision, and when no matching
revision exists the produced error lists all reachable endpoints with mismatched
revisions; ensure each property run constructs isolated mock responses via the
Layer so there is no global state mutation.
In `@packages/app/tests/docker-git/open-url.test.ts`:
- Around line 12-47: Add property-based tests (using fast-check) for openUrl and
prepareOpenUrl to assert the invariants across arbitrary URL strings: generate
arbitrary non-empty URL strings and for each case assert (1) when global open is
stubbed as null both openUrl(url) and prepareOpenUrl().navigate(url) return
false; (2) when the prepared popup is blocked (simulate open returning null for
"about:blank" and real window for others via stubBrowserOpen or a custom vi.fn)
the second open call is always invoked with the target url, "_blank", and the
"noopener" feature; and (3) when the prepared popup is allowed (use
makeBrowserOpenMockWindow + stubBrowserOpen) the flow always calls
open("about:blank","_blank") then navigate(url) sets openedWindow.location.href
to url and focus/close are called. Use prepareOpenUrl, openUrl,
makeBrowserOpenMockWindow and stubBrowserOpen to locate the code under test.
---
Outside diff comments:
In `@packages/app/tests/docker-git/actions-browser.test.ts`:
- Around line 53-97: Add a fast-check property-based test that asserts the
popup-open invariant for openProjectBrowserById and openSelectedProjectBrowser:
generate valid URL strings (use fast-check's webUrl or oneOf/arbitrary for path
fragments), stubBrowserOpen to simulate both opened and blocked outcomes and
mock openedWindow.location.navigate (or its navigate result) to return
success/failure, then for each generated URL call the action via
makeBrowserActionContext and waitForAssertion and assert that setProjectBrowser,
startProjectBrowserMock (when applicable), openedWindow.close, openMock, and
setMessage are all consistent with the navigate result (i.e., when navigate
succeeds expect opened vs blocked message and browser state to match; when
blocked expect closed window and blocked message). Reference the existing
helpers openProjectBrowserById, openSelectedProjectBrowser, stubBrowserOpen,
makeBrowserActionContext, startProjectBrowserMock, openedWindow, setMessage and
setProjectBrowser so the property test plugs into current mocks and assertions.
In `@packages/app/tests/docker-git/actions-skiller.test.ts`:
- Around line 83-104: Add a fast-check property-based test that generates
diverse appPath and projectKey values and asserts the Skiller launch invariants:
for each generated input, mock openSkillerMock to return a skillerLaunch result
tied to those values, call openSkillerApp using the test harness from
makeBrowserActionContext, and assert that browserOpenMock was called with
"about:blank", openedWindow.location.href resolves to the expected
"/api/skiller/app/" (or derived path using the generated appPath),
openedWindow.focus was called, and context.setMessage/setBusyLabel final
messages include the same launch pid/log/path derived from the mocked
skillerLaunch; use waitForAssertion as in the existing test to await async
effects and parametrize inputs with fast-check arbitraries for appPath and
projectKey.
🪄 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: 1586c9ba-141b-4064-937d-3076faded368
📒 Files selected for processing (9)
packages/api/src/services/project-browser.tspackages/app/src/docker-git/controller-health.tspackages/app/src/docker-git/controller-reachability-diagnostics.tspackages/app/src/docker-git/controller.tspackages/app/src/web/open-url.tspackages/app/tests/docker-git/actions-browser.test.tspackages/app/tests/docker-git/actions-skiller.test.tspackages/app/tests/docker-git/controller-health.test.tspackages/app/tests/docker-git/open-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 (Clone auto-open SSH)
- GitHub Check: E2E (Runtime volumes + SSH)
- GitHub Check: E2E (Clone cache)
- GitHub Check: E2E (Browser command)
- GitHub Check: E2E (OpenCode)
- GitHub Check: Lint
- GitHub Check: Test
- GitHub Check: E2E (Login context)
- 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 useany,unknown,eslint-disable,ts-ignore, orastype 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 throughpipe()andEffect.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),@throwsNever (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/open-url.tspackages/app/tests/docker-git/open-url.test.tspackages/app/tests/docker-git/actions-skiller.test.tspackages/app/src/docker-git/controller-reachability-diagnostics.tspackages/app/tests/docker-git/actions-browser.test.tspackages/app/tests/docker-git/controller-health.test.tspackages/api/src/services/project-browser.tspackages/app/src/docker-git/controller-health.tspackages/app/src/docker-git/controller.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/catchfor 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/open-url.tspackages/app/tests/docker-git/open-url.test.tspackages/app/tests/docker-git/actions-skiller.test.tspackages/app/src/docker-git/controller-reachability-diagnostics.tspackages/app/tests/docker-git/actions-browser.test.tspackages/app/tests/docker-git/controller-health.test.tspackages/api/src/services/project-browser.tspackages/app/src/docker-git/controller-health.tspackages/app/src/docker-git/controller.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/open-url.tspackages/app/tests/docker-git/open-url.test.tspackages/app/tests/docker-git/actions-skiller.test.tspackages/app/src/docker-git/controller-reachability-diagnostics.tspackages/app/tests/docker-git/actions-browser.test.tspackages/app/tests/docker-git/controller-health.test.tspackages/api/src/services/project-browser.tspackages/app/src/docker-git/controller-health.tspackages/app/src/docker-git/controller.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/open-url.tspackages/app/tests/docker-git/open-url.test.tspackages/app/tests/docker-git/actions-skiller.test.tspackages/app/src/docker-git/controller-reachability-diagnostics.tspackages/app/tests/docker-git/actions-browser.test.tspackages/app/tests/docker-git/controller-health.test.tspackages/api/src/services/project-browser.tspackages/app/src/docker-git/controller-health.tspackages/app/src/docker-git/controller.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/open-url.tspackages/app/tests/docker-git/open-url.test.tspackages/app/tests/docker-git/actions-skiller.test.tspackages/app/src/docker-git/controller-reachability-diagnostics.tspackages/app/tests/docker-git/actions-browser.test.tspackages/app/tests/docker-git/controller-health.test.tspackages/api/src/services/project-browser.tspackages/app/src/docker-git/controller-health.tspackages/app/src/docker-git/controller.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/open-url.tspackages/app/tests/docker-git/open-url.test.tspackages/app/tests/docker-git/actions-skiller.test.tspackages/app/src/docker-git/controller-reachability-diagnostics.tspackages/app/tests/docker-git/actions-browser.test.tspackages/app/tests/docker-git/controller-health.test.tspackages/api/src/services/project-browser.tspackages/app/src/docker-git/controller-health.tspackages/app/src/docker-git/controller.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/open-url.test.tspackages/app/tests/docker-git/actions-skiller.test.tspackages/app/tests/docker-git/actions-browser.test.tspackages/app/tests/docker-git/controller-health.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/open-url.test.tspackages/app/tests/docker-git/actions-skiller.test.tspackages/app/tests/docker-git/actions-browser.test.tspackages/app/tests/docker-git/controller-health.test.ts
🧠 Learnings (2)
📚 Learning: 2026-05-22T21:08:18.083Z
Learnt from: skulidropek
Repo: ProverCoderAI/docker-git PR: 344
File: packages/app/src/docker-git/controller-compose.ts:34-40
Timestamp: 2026-05-22T21:08:18.083Z
Learning: In this repo’s docker-git controller compose generation, `${DOCKER_GIT_CONTROLLER_BUILD_SKILLER:-1}` should be treated as standard bash parameter expansion: when `DOCKER_GIT_CONTROLLER_BUILD_SKILLER` is unset, it defaults to the string "1". There is no "-1" mode. The runtime contract enforced by `packages/app/src/docker-git/controller-compose.ts` is: unset / "1" / "true" => output "1"; "0" / "false" => output "0". If review code shows branching/behavior for "-1" or any numeric value other than this 0/1 contract, flag it. Also ensure the Dockerfile ARG `DOCKER_GIT_CONTROLLER_BUILD_SKILLER` stays consistent with default `1`.
Applied to files:
packages/app/src/docker-git/controller-reachability-diagnostics.tspackages/app/src/docker-git/controller-health.tspackages/app/src/docker-git/controller.ts
📚 Learning: 2026-05-18T07:43:38.131Z
Learnt from: skulidropek
Repo: ProverCoderAI/docker-git PR: 313
File: packages/api/src/services/panel-cloudflare-tunnel.ts:123-135
Timestamp: 2026-05-18T07:43:38.131Z
Learning: In this repo’s ProverCoderAI/docker-git architecture, it is intentional for service modules under packages/api/src/services/ to call Node.js APIs directly (e.g., fs, child_process, fetch). Do not treat direct Node API usage in these service modules as a dependency-injection (DI) violation. This codebase provides the NodeContext at the program boundary (entry point) rather than injecting Context.Tag/Layer services into individual service modules; reviewers should only flag DI/context issues if the expected boundary setup is missing.
Applied to files:
packages/api/src/services/project-browser.ts
🔇 Additional comments (2)
packages/api/src/services/project-browser.ts (1)
13-13: LGTM!Also applies to: 99-99, 252-277, 469-469
packages/app/src/web/open-url.ts (1)
23-27: LGTM!
AI Session BackupCommit: 1235301
|
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 `@packages/app/tests/docker-git/controller-health.test.ts`:
- Around line 75-83: Replace the use of Effect.acquireUseRelease in
withHealthServer with a scoped acquireRelease pattern: call
Effect.acquireRelease(listenHealthServer(revisions), ({ server }) =>
closeHealthServer(server)) to create the resource effect, then wrap it with
Effect.scoped and flatMap the acquired value into the provided effect, e.g.
Effect.scoped(Effect.flatMap(Effect.acquireRelease(...), ({ baseUrl }) =>
effect(baseUrl))). Keep the same parameters (withHealthServer,
listenHealthServer, closeHealthServer) and ensure the release uses
closeHealthServer(server).
🪄 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: d0d9916e-2675-4335-9247-98499436707d
📒 Files selected for processing (8)
packages/app/src/docker-git/controller-health.tspackages/app/src/docker-git/controller-reachability-diagnostics.tspackages/app/src/docker-git/controller.tspackages/app/tests/docker-git/actions-browser.test.tspackages/app/tests/docker-git/actions-skiller.test.tspackages/app/tests/docker-git/controller-health.test.tspackages/app/tests/docker-git/controller-ready.test.tspackages/app/tests/docker-git/open-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). (8)
- GitHub Check: E2E (Login context)
- GitHub Check: E2E (Browser command)
- GitHub Check: E2E (Runtime volumes + SSH)
- GitHub Check: E2E (Clone cache)
- GitHub Check: E2E (OpenCode)
- GitHub Check: Lint
- GitHub Check: E2E (Clone auto-open SSH)
- 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 useany,unknown,eslint-disable,ts-ignore, orastype 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 throughpipe()andEffect.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),@throwsNever (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/controller-ready.test.tspackages/app/tests/docker-git/actions-skiller.test.tspackages/app/src/docker-git/controller-reachability-diagnostics.tspackages/app/tests/docker-git/controller-health.test.tspackages/app/tests/docker-git/open-url.test.tspackages/app/src/docker-git/controller.tspackages/app/src/docker-git/controller-health.tspackages/app/tests/docker-git/actions-browser.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/controller-ready.test.tspackages/app/tests/docker-git/actions-skiller.test.tspackages/app/tests/docker-git/controller-health.test.tspackages/app/tests/docker-git/open-url.test.tspackages/app/tests/docker-git/actions-browser.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/catchfor 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/controller-ready.test.tspackages/app/tests/docker-git/actions-skiller.test.tspackages/app/src/docker-git/controller-reachability-diagnostics.tspackages/app/tests/docker-git/controller-health.test.tspackages/app/tests/docker-git/open-url.test.tspackages/app/src/docker-git/controller.tspackages/app/src/docker-git/controller-health.tspackages/app/tests/docker-git/actions-browser.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/controller-ready.test.tspackages/app/tests/docker-git/actions-skiller.test.tspackages/app/tests/docker-git/controller-health.test.tspackages/app/tests/docker-git/open-url.test.tspackages/app/tests/docker-git/actions-browser.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/controller-ready.test.tspackages/app/tests/docker-git/actions-skiller.test.tspackages/app/src/docker-git/controller-reachability-diagnostics.tspackages/app/tests/docker-git/controller-health.test.tspackages/app/tests/docker-git/open-url.test.tspackages/app/src/docker-git/controller.tspackages/app/src/docker-git/controller-health.tspackages/app/tests/docker-git/actions-browser.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/controller-ready.test.tspackages/app/tests/docker-git/actions-skiller.test.tspackages/app/src/docker-git/controller-reachability-diagnostics.tspackages/app/tests/docker-git/controller-health.test.tspackages/app/tests/docker-git/open-url.test.tspackages/app/src/docker-git/controller.tspackages/app/src/docker-git/controller-health.tspackages/app/tests/docker-git/actions-browser.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/controller-ready.test.tspackages/app/tests/docker-git/actions-skiller.test.tspackages/app/src/docker-git/controller-reachability-diagnostics.tspackages/app/tests/docker-git/controller-health.test.tspackages/app/tests/docker-git/open-url.test.tspackages/app/src/docker-git/controller.tspackages/app/src/docker-git/controller-health.tspackages/app/tests/docker-git/actions-browser.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/controller-ready.test.tspackages/app/tests/docker-git/actions-skiller.test.tspackages/app/src/docker-git/controller-reachability-diagnostics.tspackages/app/tests/docker-git/controller-health.test.tspackages/app/tests/docker-git/open-url.test.tspackages/app/src/docker-git/controller.tspackages/app/src/docker-git/controller-health.tspackages/app/tests/docker-git/actions-browser.test.ts
🧠 Learnings (1)
📚 Learning: 2026-05-22T21:08:18.083Z
Learnt from: skulidropek
Repo: ProverCoderAI/docker-git PR: 344
File: packages/app/src/docker-git/controller-compose.ts:34-40
Timestamp: 2026-05-22T21:08:18.083Z
Learning: In this repo’s docker-git controller compose generation, `${DOCKER_GIT_CONTROLLER_BUILD_SKILLER:-1}` should be treated as standard bash parameter expansion: when `DOCKER_GIT_CONTROLLER_BUILD_SKILLER` is unset, it defaults to the string "1". There is no "-1" mode. The runtime contract enforced by `packages/app/src/docker-git/controller-compose.ts` is: unset / "1" / "true" => output "1"; "0" / "false" => output "0". If review code shows branching/behavior for "-1" or any numeric value other than this 0/1 contract, flag it. Also ensure the Dockerfile ARG `DOCKER_GIT_CONTROLLER_BUILD_SKILLER` stays consistent with default `1`.
Applied to files:
packages/app/src/docker-git/controller-reachability-diagnostics.tspackages/app/src/docker-git/controller.tspackages/app/src/docker-git/controller-health.ts
🪛 GitHub Check: Lint Effect-TS
packages/app/tests/docker-git/controller-health.test.ts
[failure] 4-4:
'node:http' import is restricted from being used by a pattern. Do not import from node:* directly. Use @effect/platform-node or @effect/platform services
🪛 OpenGrep (1.22.0)
packages/app/tests/docker-git/controller-health.test.ts
[ERROR] 25-25: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
🔇 Additional comments (2)
packages/app/tests/docker-git/controller-ready.test.ts (1)
133-138: ⚡ Quick winСинхронизируйте тип мока
findReachableDirectHealthProbeMockсexpectedRevisionв ожидании.В
packages/app/tests/docker-git/controller-ready.test.ts(стр. 133–138) ожидание передаётexpectedRevision: "local-revision", поэтому hoisted-тип/сигнатура параметров мока тоже должна включать это поле (например, как необязательное), иначе теряется type-level защита от дрейфа контракта.packages/app/tests/docker-git/controller-health.test.ts (1)
4-5: ⚡ Quick winЗаменить прямой
node:httpи привести финализацию ресурсов кEffect.acquireRelease+Effect.scopedвpackages/app/tests/docker-git/controller-health.test.ts.
- Уберите прямой импорт
node:http(у вас сейчасimport { createServer, ... } from "node:http"); используйте repo-approved серверный fixture/платформенный слой (@effect/platform-node/@effect/platform).- В helper’ах с ресурсами используйте
Effect.acquireRelease+Effect.scopedдля финализации; если в тесте применяетсяEffect.acquireUseRelease, замените на требуемый паттерн.
AI Session BackupCommit: 78ade66
|
AI Session BackupCommit: 4f40f6c
|
1 participant
Summary
noopenerso Chrome returns a usable WindowProxyopeneron the returned handleVerification