fix(api): mount host docker data for skiller#366
Conversation
|
Warning Review limit reached
More reviews will be available in 35 minutes and 18 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 (2)
📝 WalkthroughWalkthroughКонфигурации API-контейнера переключены на bind-mount для /var/lib/docker; README и фикстура обновлены; Dockerfile материализует Electron-бинарник в сборке; Skiller получил рефактор подготовки runtime, экспорт типов и новую генерацию команды запуска; добавлены соответствующие тесты. ИзмененияМиграция на bind-mount и обновление Skiller
Sequence Diagram(s)sequenceDiagram
participant openSkiller
participant prepareSkillerScopeHome
participant prepareSkillerRuntime
participant spawnProcess
openSkiller->>prepareSkillerScopeHome: ensure scope dirs & ownership
prepareSkillerScopeHome->>prepareSkillerRuntime: provide runtimeRoot & env
prepareSkillerRuntime->>spawnProcess: run prepareLaunchScript / materialize
openSkiller->>spawnProcess: launchSkillerProcess uses skillerLaunchCommand + env
🎯 3 (Moderate) | ⏱️ ~20 minutes Возможные связанные PRs:
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (6 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Скриншоты работы приложения для issue #365. Это не CI и не файлы: кадры сняты с web UI после реального CDP click по кнопке Проверка в proof-среде:
|
![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
🤖 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/api/Dockerfile`:
- Around line 124-130: The unzip step that writes into
node_modules/electron/dist using electron_zip is unsafe to path traversal;
before calling unzip in the Dockerfile, ensure electron_zip is non-empty (fail
with a clear error) and validate the archive contents for unsafe paths (reject
entries with '..' or leading '/') and/or verify a checksum for the expected
Electron version, then only run unzip when validation passes; update the block
referencing electron_zip and node_modules/electron/dist to perform these checks
and fail the build on detection of unsafe paths or missing zip.
In `@packages/api/src/services/skiller.ts`:
- Around line 366-369: The code currently launches Skiller via
skillerLaunchCommand as root/bash which runs the process under the controller
UID/GID; instead, ensure Skiller is executed with the owner UID/GID of
scope.hostHomePath (the same user chown'd in prepareSkillerScopeHome) by
changing skillerLaunchCommand to either wrap launchScript with a uid/gid switch
(e.g., gosu/su-exec or sudo -u equivalent) or by invoking an exec path that sets
process credentials before exec; also ensure the runtime mounts map
/home/<sshUser> and the project targetDir to the controller-visible Docker
volume paths so filesystem access is limited to the active project container.
Use the symbols skillerLaunchCommand and prepareSkillerScopeHome to locate where
to apply these changes.
In `@packages/api/tests/skiller-routes.test.ts`:
- Around line 35-41: The test 'launches Electron as the controller process user'
pins an implementation detail (absence of "setpriv") rather than the contract;
remove the expect(...not.toContain("setpriv")) assertion and instead assert that
skillerLaunchCommand() produces the expected Docker volume mappings for
controller-visible paths. Concretely, in the test that calls
skillerLaunchCommand(), keep the command and electron path assertions, drop the
"setpriv" negative check, and add assertions on the args array (e.g., using
expect.arrayContaining and expect.stringContaining or regex) that verify docker
volume flags ("-v" or "--volume") map "/home/<sshUser>" and the project
targetDir to controller-visible volume paths (look for strings containing
"/home/" and the project targetDir identifier) so the test verifies filesystem
scoping rather than forbidding a specific privilege-drop mechanism.
In `@packages/app/tests/docker-git/controller-resource-limits.test.ts`:
- Around line 98-108: The test in describe("controller Skiller Dockerfile") is
brittle because it asserts exact shell lines via toContain; update the test to
assert the key elements more flexibly: read the Dockerfile with
readComposeFile("packages/api/Dockerfile") as before but replace the exact
toContain checks with either regex matches or separate contains for the variable
definition (electron_zip), the unzip invocation (unzip -q ...
node_modules/electron/dist), and the executable check (test -x
node_modules/electron/dist/electron); also consider renaming the suite to
something specific (e.g., "materialize Electron binary in API Dockerfile") and
keep the test as a regression check if you must assert an exact pattern.
🪄 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: a1240847-ec81-4a84-9d05-f0db3923db98
📒 Files selected for processing (4)
packages/api/Dockerfilepackages/api/src/services/skiller.tspackages/api/tests/skiller-routes.test.tspackages/app/tests/docker-git/controller-resource-limits.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 cache)
- GitHub Check: E2E (Browser command)
- GitHub Check: E2E (Login context)
- GitHub Check: Lint
- GitHub Check: E2E (Clone auto-open SSH)
- GitHub Check: E2E (Runtime volumes + SSH)
- GitHub Check: Test
- GitHub Check: E2E (OpenCode)
- GitHub Check: Final build (windows-latest)
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{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/api/tests/skiller-routes.test.tspackages/api/src/services/skiller.tspackages/app/tests/docker-git/controller-resource-limits.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/api/tests/skiller-routes.test.tspackages/app/tests/docker-git/controller-resource-limits.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/api/tests/skiller-routes.test.tspackages/api/src/services/skiller.tspackages/app/tests/docker-git/controller-resource-limits.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/api/tests/skiller-routes.test.tspackages/app/tests/docker-git/controller-resource-limits.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/api/tests/skiller-routes.test.tspackages/api/src/services/skiller.tspackages/app/tests/docker-git/controller-resource-limits.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/api/tests/skiller-routes.test.tspackages/api/src/services/skiller.tspackages/app/tests/docker-git/controller-resource-limits.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/api/tests/skiller-routes.test.tspackages/api/src/services/skiller.tspackages/app/tests/docker-git/controller-resource-limits.test.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Ты строгий ревьюер SPEC DRIVEN DEVELOPMENT.Перед выводами изучи README.md, другие *.md файлы, linked issues,
PR description, PR comments/discussion и релевантную кодовую базу.Сверь изменения с исходным ТЗ/спекой и обсуждением. Флагай любой уход
от спеки, недокументированное изменение поведения, отсутствие тестов
для заявленного поведения и security-риск. Если спека не видна,
попроси автора добавить ее в issue или PR description.Проверь решение с точки зрения формальной верификации: какие инварианты,
предусловия и постусловия можно доказать математически, а где доказуемость
слабая. Оцени решение с точки зрения теории игр: устойчивы ли стимулы,
нет ли выгодного обхода правил, и какое решение было бы сильнее.
Files:
packages/api/tests/skiller-routes.test.tspackages/api/Dockerfilepackages/api/src/services/skiller.tspackages/app/tests/docker-git/controller-resource-limits.test.ts
**/{Dockerfile*,docker-compose*.{yml,yaml},.dockerignore}
📄 CodeRabbit inference engine (Custom checks)
Fail if changed files introduce unsafe Docker configuration such as privileged containers, broad host mounts, unbounded Docker socket access, or unnecessary write permissions
Files:
packages/api/Dockerfile
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
Repo: ProverCoderAI/docker-git PR: 0
File: docs/integrations/skiller.md:0-0
Timestamp: 2026-05-14T16:02:05.012Z
Learning: Applies to docs/integrations/**/*.(ts|js)?(@(routes|handlers|api|middleware))* : API must scope Skiller filesystem access to the active project container by mapping `/home/<sshUser>` and project `targetDir` to controller-visible Docker volume paths
Learnt from: CR
Repo: ProverCoderAI/docker-git PR: 0
File: docs/integrations/skiller.md:0-0
Timestamp: 2026-05-14T16:02:05.012Z
Learning: docker-git must not import Skiller source code into the docker-git web bundle; serve Skiller browser UI from Skiller's own built renderer and proxy tRPC requests to Skiller's separate backend process
Learnt from: CR
Repo: ProverCoderAI/docker-git PR: 0
File: docs/integrations/skiller.md:0-0
Timestamp: 2026-05-14T16:02:05.012Z
Learning: Applies to docs/integrations/**/*.(ts|js)?(@(routes|handlers|api))* : API endpoint `POST /projects/by-key/:projectKey/terminal-sessions/:sessionId/skiller/open` must launch Skiller Electron app, register terminal session filesystem scope, and write output to ~/.docker-git/logs/skiller.log
Learnt from: CR
Repo: ProverCoderAI/docker-git PR: 0
File: docs/integrations/skiller.md:0-0
Timestamp: 2026-05-14T16:02:05.012Z
Learning: Run `bun run skiller:check` after updating the Skiller pin in addition to docker-git `typecheck` and `check` scripts to verify integration
📚 Learning: 2026-05-14T16:02:05.012Z
Learnt from: CR
Repo: ProverCoderAI/docker-git PR: 0
File: docs/integrations/skiller.md:0-0
Timestamp: 2026-05-14T16:02:05.012Z
Learning: Applies to docs/integrations/**/*.(ts|js)?(@(routes|handlers|api))* : API endpoint `POST /projects/by-key/:projectKey/terminal-sessions/:sessionId/skiller/open` must launch Skiller Electron app, register terminal session filesystem scope, and write output to ~/.docker-git/logs/skiller.log
Applied to files:
packages/api/tests/skiller-routes.test.tspackages/api/src/services/skiller.tspackages/app/tests/docker-git/controller-resource-limits.test.ts
📚 Learning: 2026-05-14T16:02:05.012Z
Learnt from: CR
Repo: ProverCoderAI/docker-git PR: 0
File: docs/integrations/skiller.md:0-0
Timestamp: 2026-05-14T16:02:05.012Z
Learning: Applies to docs/integrations/**/*.(ts|js)?(@(routes|handlers|api|services))* : API must resolve Codex global skills from `~/.codex/skills` within the selected container volume, not falling back to controller's own `~/.codex/skills`
Applied to files:
packages/api/tests/skiller-routes.test.tspackages/app/tests/docker-git/controller-resource-limits.test.ts
📚 Learning: 2026-05-14T16:02:05.012Z
Learnt from: CR
Repo: ProverCoderAI/docker-git PR: 0
File: docs/integrations/skiller.md:0-0
Timestamp: 2026-05-14T16:02:05.012Z
Learning: docker-git must not import Skiller source code into the docker-git web bundle; serve Skiller browser UI from Skiller's own built renderer and proxy tRPC requests to Skiller's separate backend process
Applied to files:
packages/api/tests/skiller-routes.test.tspackages/api/Dockerfilepackages/app/tests/docker-git/controller-resource-limits.test.ts
📚 Learning: 2026-05-14T16:02:05.012Z
Learnt from: CR
Repo: ProverCoderAI/docker-git PR: 0
File: docs/integrations/skiller.md:0-0
Timestamp: 2026-05-14T16:02:05.012Z
Learning: Applies to docs/integrations/**/*.(ts|js|sh|bash)?(launcher|skiller)* : Skiller launcher must use `xvfb-run` if available when API process has no `$DISPLAY` environment variable to enable headless startup
Applied to files:
packages/api/tests/skiller-routes.test.tspackages/api/src/services/skiller.ts
📚 Learning: 2026-05-14T16:02:05.012Z
Learnt from: CR
Repo: ProverCoderAI/docker-git PR: 0
File: docs/integrations/skiller.md:0-0
Timestamp: 2026-05-14T16:02:05.012Z
Learning: Applies to docs/integrations/**/*.(ts|js)?(@(routes|handlers|api|middleware))* : API must scope Skiller filesystem access to the active project container by mapping `/home/<sshUser>` and project `targetDir` to controller-visible Docker volume paths
Applied to files:
packages/api/tests/skiller-routes.test.tspackages/app/tests/docker-git/controller-resource-limits.test.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/skiller.ts
📚 Learning: 2026-05-22T21:08:22.631Z
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:22.631Z
Learning: In the docker-git project, `${DOCKER_GIT_CONTROLLER_BUILD_SKILLER:-1}` in compose files is standard bash parameter expansion with default value `"1"` (not a literal "-1" mode). The supported runtime contract for `DOCKER_GIT_CONTROLLER_BUILD_SKILLER` in `packages/app/src/docker-git/controller-compose.ts` is: unset/`"1"`/`"true"` → `"1"`, `"0"`/`"false"` → `"0"`. There is no `-1` mode. The Dockerfile also declares `ARG DOCKER_GIT_CONTROLLER_BUILD_SKILLER=1`.
Applied to files:
packages/app/tests/docker-git/controller-resource-limits.test.ts
🔇 Additional comments (3)
packages/app/tests/docker-git/controller-resource-limits.test.ts (3)
21-22: LGTM!
56-61: LGTM!
88-93: LGTM!
|
Updated in What changed:
Verification run: Fresh runtime proof: Screenshot: Skiller dashboard after successful open:
Screenshot: after a real click on the
|
There was a problem hiding this comment.
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/api/src/services/skiller.ts (1)
302-314:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winСначала создайте
out/preload, иначе восстановление shim само себя ломает.Ветка с
ln -sf ... out/preload/index.jsпадает, еслиout/mainиout/rendererуже существуют, а каталогout/preloadещё нет. В Dockerfile этот же контракт уже реализован черезmkdir -p out/preloadпередln -sf, так что runtime-скрипт сейчас расходится с образом.Предлагаемое исправление
const prepareLaunchScript = [ "set -euo pipefail", "DOCKER_GIT_SKILLER_PATCH=../../patches/skiller/docker-git-browser-folder-picker.patch", "DOCKER_GIT_SKILLER_PATCH_MARKER=out/.docker-git-browser-folder-picker.patch", "if [ -f ../../scripts/skiller-apply-docker-git-patches.mjs ]; then bun ../../scripts/skiller-apply-docker-git-patches.mjs; fi", "if [ ! -d node_modules ]; then bun install --frozen-lockfile; fi", "if [ ! -f out/main/index.js ] || [ ! -f out/renderer/index.html ] || { [ -f \"$DOCKER_GIT_SKILLER_PATCH\" ] && [ ! -f \"$DOCKER_GIT_SKILLER_PATCH_MARKER\" ]; } || { [ -f \"$DOCKER_GIT_SKILLER_PATCH\" ] && [ \"$DOCKER_GIT_SKILLER_PATCH\" -nt \"$DOCKER_GIT_SKILLER_PATCH_MARKER\" ]; }; then", " bun run build", " mkdir -p out", " touch \"$DOCKER_GIT_SKILLER_PATCH_MARKER\"", "fi", - "if [ ! -e out/preload/index.js ]; then ln -sf index.mjs out/preload/index.js; fi" + "if [ ! -e out/preload/index.js ]; then", + " mkdir -p out/preload", + " ln -sf index.mjs out/preload/index.js", + "fi" ].join(\"\\n\")🤖 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/api/src/services/skiller.ts` around lines 302 - 314, The runtime launch script assigned to prepareLaunchScript attempts to create a symlink with the line 'ln -sf index.mjs out/preload/index.js' but does not ensure the out/preload directory exists, causing failures when out/main and out/renderer exist but out/preload does not; update prepareLaunchScript to run 'mkdir -p out/preload' (or equivalent) before the ln -sf command so the directory is created prior to creating the symlink, keeping the rest of the build checks and markers intact.
🤖 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/api/src/services/skiller.ts`:
- Around line 479-490: The current resolveSkillerProcessAccount function throws
if nameForId(readFileSync("/etc/passwd"...) or nameForId(..."/etc/group"...)
returns null; change it to tolerate missing local accounts by falling back: if
userName or groupName is null, do NOT throw—instead mark the account as
"unresolved" and return a SkillerProcessAccount variant indicating unresolved
names (keep uid/gid), and update the code path that launches the controller (the
caller that uses resolveSkillerProcessAccount and invokes runuser/runas logic)
to either (A) fall back to starting the controller with a safe "bash -c"
unscoped invocation when names are unresolved, or (B) create temporary
/etc/passwd and /etc/group entries (e.g. write to a temp file and use NSS
wrapper or pass via --userns options) before calling runuser; implement option A
as the minimal fix so POST .../skiller/open succeeds for non-local UIDs by using
the bash fallback when resolveSkillerProcessAccount indicates missing names.
Ensure the returned SkillerProcessAccount still includes uid/gid so downstream
logging and path setup (writing to ~/.docker-git/logs/skiller.log) can proceed.
- Around line 522-533: prepareSkillerRuntime currently uses spawnSync which
blocks the event loop when openSkiller is invoked from HTTP handlers; change it
to a non-blocking async implementation (e.g., use child_process.spawn or spawn
with a Promise wrapper, or use exec/execFile with a Promise) and await/result
outside the request path or run it in a background worker so HTTP handlers never
perform synchronous patch/install/build work; specifically replace
prepareSkillerRuntime's spawnSync usage with an asynchronous child-process
approach that pipes logs to logFd and rejects/throws on non-zero exit, and
ensure callers like openSkiller invoke the async function without blocking the
request (or trigger it off-request via background task).
---
Outside diff comments:
In `@packages/api/src/services/skiller.ts`:
- Around line 302-314: The runtime launch script assigned to prepareLaunchScript
attempts to create a symlink with the line 'ln -sf index.mjs
out/preload/index.js' but does not ensure the out/preload directory exists,
causing failures when out/main and out/renderer exist but out/preload does not;
update prepareLaunchScript to run 'mkdir -p out/preload' (or equivalent) before
the ln -sf command so the directory is created prior to creating the symlink,
keeping the rest of the build checks and markers intact.
🪄 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: f90dd2b4-58cc-4b84-926c-f28192e8a117
⛔ Files ignored due to path filters (2)
docs/pr-screenshots/issue-365/skiller-dashboard.pngis excluded by!**/*.pngdocs/pr-screenshots/issue-365/skiller-projects-after-click.pngis excluded by!**/*.png
📒 Files selected for processing (5)
docs/pr-screenshots/issue-365/skiller-projects-after-click.jsonpackages/api/Dockerfilepackages/api/src/services/skiller.tspackages/api/tests/skiller-routes.test.tspackages/app/tests/docker-git/controller-resource-limits.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 (Runtime volumes + SSH)
- GitHub Check: E2E (Clone auto-open SSH)
- GitHub Check: E2E (Login context)
- GitHub Check: E2E (Clone cache)
- GitHub Check: Lint
- GitHub Check: E2E (OpenCode)
- GitHub Check: Test
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{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/api/tests/skiller-routes.test.tspackages/app/tests/docker-git/controller-resource-limits.test.tspackages/api/src/services/skiller.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/api/tests/skiller-routes.test.tspackages/app/tests/docker-git/controller-resource-limits.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/api/tests/skiller-routes.test.tspackages/app/tests/docker-git/controller-resource-limits.test.tspackages/api/src/services/skiller.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/api/tests/skiller-routes.test.tspackages/app/tests/docker-git/controller-resource-limits.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/api/tests/skiller-routes.test.tspackages/app/tests/docker-git/controller-resource-limits.test.tspackages/api/src/services/skiller.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/api/tests/skiller-routes.test.tspackages/app/tests/docker-git/controller-resource-limits.test.tspackages/api/src/services/skiller.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/api/tests/skiller-routes.test.tsdocs/pr-screenshots/issue-365/skiller-projects-after-click.jsonpackages/app/tests/docker-git/controller-resource-limits.test.tspackages/api/src/services/skiller.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Ты строгий ревьюер SPEC DRIVEN DEVELOPMENT.Перед выводами изучи README.md, другие *.md файлы, linked issues,
PR description, PR comments/discussion и релевантную кодовую базу.Сверь изменения с исходным ТЗ/спекой и обсуждением. Флагай любой уход
от спеки, недокументированное изменение поведения, отсутствие тестов
для заявленного поведения и security-риск. Если спека не видна,
попроси автора добавить ее в issue или PR description.Проверь решение с точки зрения формальной верификации: какие инварианты,
предусловия и постусловия можно доказать математически, а где доказуемость
слабая. Оцени решение с точки зрения теории игр: устойчивы ли стимулы,
нет ли выгодного обхода правил, и какое решение было бы сильнее.
Files:
packages/api/tests/skiller-routes.test.tsdocs/pr-screenshots/issue-365/skiller-projects-after-click.jsonpackages/api/Dockerfilepackages/app/tests/docker-git/controller-resource-limits.test.tspackages/api/src/services/skiller.ts
**/{Dockerfile*,docker-compose*.{yml,yaml},.dockerignore}
📄 CodeRabbit inference engine (Custom checks)
Fail if changed files introduce unsafe Docker configuration such as privileged containers, broad host mounts, unbounded Docker socket access, or unnecessary write permissions
Files:
packages/api/Dockerfile
🧠 Learnings (19)
📓 Common learnings
Learnt from: ezocomp118-source
Repo: ProverCoderAI/docker-git PR: 366
File: packages/api/src/services/skiller.ts:366-369
Timestamp: 2026-06-03T17:35:44.689Z
Learning: In ProverCoderAI/docker-git, launching Electron via `setpriv --reuid/--regid` in the controller container causes a `Trace/breakpoint trap` crash even with `--no-sandbox`. The confirmed workaround is to launch Skiller as the controller user (bash -lc) and enforce project scope at the software level via SkillerContainerScope env vars (HOME, XDG_*, DOCKER_GIT_SKILLER_CONTAINER_HOME_PATH) and the add_project tRPC call. A future fix should use gosu/su-exec or a Chromium-sandbox-compatible credential-drop mechanism instead of setpriv. See PR `#366` for full context.
Learnt from: CR
Repo: ProverCoderAI/docker-git PR: 0
File: docs/integrations/skiller.md:0-0
Timestamp: 2026-05-14T16:02:05.012Z
Learning: Applies to docs/integrations/**/*.(ts|js)?(@(routes|handlers|api|middleware))* : API must scope Skiller filesystem access to the active project container by mapping `/home/<sshUser>` and project `targetDir` to controller-visible Docker volume paths
Learnt from: CR
Repo: ProverCoderAI/docker-git PR: 0
File: docs/integrations/skiller.md:0-0
Timestamp: 2026-05-14T16:02:05.012Z
Learning: docker-git must not import Skiller source code into the docker-git web bundle; serve Skiller browser UI from Skiller's own built renderer and proxy tRPC requests to Skiller's separate backend process
Learnt from: CR
Repo: ProverCoderAI/docker-git PR: 0
File: docs/integrations/skiller.md:0-0
Timestamp: 2026-05-14T16:02:05.012Z
Learning: Applies to docs/integrations/**/*.(ts|js)?(@(routes|handlers|api))* : API endpoint `POST /projects/by-key/:projectKey/terminal-sessions/:sessionId/skiller/open` must launch Skiller Electron app, register terminal session filesystem scope, and write output to ~/.docker-git/logs/skiller.log
📚 Learning: 2026-05-14T16:02:05.012Z
Learnt from: CR
Repo: ProverCoderAI/docker-git PR: 0
File: docs/integrations/skiller.md:0-0
Timestamp: 2026-05-14T16:02:05.012Z
Learning: Applies to docs/integrations/**/*.(ts|js)?(@(routes|handlers|api))* : API endpoint `POST /projects/by-key/:projectKey/terminal-sessions/:sessionId/skiller/open` must launch Skiller Electron app, register terminal session filesystem scope, and write output to ~/.docker-git/logs/skiller.log
Applied to files:
packages/api/tests/skiller-routes.test.tspackages/api/Dockerfilepackages/app/tests/docker-git/controller-resource-limits.test.tspackages/api/src/services/skiller.ts
📚 Learning: 2026-05-14T16:02:05.012Z
Learnt from: CR
Repo: ProverCoderAI/docker-git PR: 0
File: docs/integrations/skiller.md:0-0
Timestamp: 2026-05-14T16:02:05.012Z
Learning: Applies to docs/integrations/**/*.(ts|js|sh|bash)?(launcher|skiller)* : Skiller launcher must use `xvfb-run` if available when API process has no `$DISPLAY` environment variable to enable headless startup
Applied to files:
packages/api/tests/skiller-routes.test.tspackages/app/tests/docker-git/controller-resource-limits.test.tspackages/api/src/services/skiller.ts
📚 Learning: 2026-06-03T17:35:44.689Z
Learnt from: ezocomp118-source
Repo: ProverCoderAI/docker-git PR: 366
File: packages/api/src/services/skiller.ts:366-369
Timestamp: 2026-06-03T17:35:44.689Z
Learning: In ProverCoderAI/docker-git, launching Electron via `setpriv --reuid/--regid` in the controller container causes a `Trace/breakpoint trap` crash even with `--no-sandbox`. The confirmed workaround is to launch Skiller as the controller user (bash -lc) and enforce project scope at the software level via SkillerContainerScope env vars (HOME, XDG_*, DOCKER_GIT_SKILLER_CONTAINER_HOME_PATH) and the add_project tRPC call. A future fix should use gosu/su-exec or a Chromium-sandbox-compatible credential-drop mechanism instead of setpriv. See PR `#366` for full context.
Applied to files:
packages/api/tests/skiller-routes.test.tspackages/api/Dockerfilepackages/app/tests/docker-git/controller-resource-limits.test.tspackages/api/src/services/skiller.ts
📚 Learning: 2026-05-14T16:02:05.012Z
Learnt from: CR
Repo: ProverCoderAI/docker-git PR: 0
File: docs/integrations/skiller.md:0-0
Timestamp: 2026-05-14T16:02:05.012Z
Learning: Applies to docs/integrations/**/*.(ts|js)?(@(routes|handlers|api|middleware))* : API must scope Skiller filesystem access to the active project container by mapping `/home/<sshUser>` and project `targetDir` to controller-visible Docker volume paths
Applied to files:
packages/api/tests/skiller-routes.test.tspackages/app/tests/docker-git/controller-resource-limits.test.tspackages/api/src/services/skiller.ts
📚 Learning: 2026-05-14T16:02:16.256Z
Learnt from: CR
Repo: ProverCoderAI/docker-git PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2026-05-14T16:02:16.256Z
Learning: Applies to **/*.{py,js,ts,jsx,tsx,go,java,rb,php,sh,bash,c,cpp} : Fail if changed files introduce path traversal or writes outside intended project/container state directories
Applied to files:
packages/api/tests/skiller-routes.test.tspackages/api/Dockerfilepackages/api/src/services/skiller.ts
📚 Learning: 2026-05-14T16:02:05.012Z
Learnt from: CR
Repo: ProverCoderAI/docker-git PR: 0
File: docs/integrations/skiller.md:0-0
Timestamp: 2026-05-14T16:02:05.012Z
Learning: docker-git must not import Skiller source code into the docker-git web bundle; serve Skiller browser UI from Skiller's own built renderer and proxy tRPC requests to Skiller's separate backend process
Applied to files:
packages/api/Dockerfilepackages/app/tests/docker-git/controller-resource-limits.test.tspackages/api/src/services/skiller.ts
📚 Learning: 2026-05-14T16:02:05.012Z
Learnt from: CR
Repo: ProverCoderAI/docker-git PR: 0
File: docs/integrations/skiller.md:0-0
Timestamp: 2026-05-14T16:02:05.012Z
Learning: Run `bun run skiller:check` after updating the Skiller pin in addition to docker-git `typecheck` and `check` scripts to verify integration
Applied to files:
packages/api/Dockerfilepackages/app/tests/docker-git/controller-resource-limits.test.ts
📚 Learning: 2026-05-14T16:02:16.256Z
Learnt from: CR
Repo: ProverCoderAI/docker-git PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2026-05-14T16:02:16.256Z
Learning: Applies to **/{Dockerfile*,docker-compose*.{yml,yaml},.dockerignore} : Fail if changed files introduce unsafe Docker configuration such as privileged containers, broad host mounts, unbounded Docker socket access, or unnecessary write permissions
Applied to files:
packages/api/Dockerfilepackages/app/tests/docker-git/controller-resource-limits.test.tspackages/api/src/services/skiller.ts
📚 Learning: 2026-05-14T16:02:16.256Z
Learnt from: CR
Repo: ProverCoderAI/docker-git PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2026-05-14T16:02:16.256Z
Learning: Applies to **/{package*.json,requirements*.txt,setup.py,setup.cfg,Pipfile,Pipfile.lock,pyproject.toml,pom.xml,build.gradle,Gemfile,Gemfile.lock,go.mod,go.sum,composer.json,Cargo.toml,Cargo.lock} : Fail if dependency or package-manager changes materially increase supply-chain risk without justification
Applied to files:
packages/api/Dockerfile
📚 Learning: 2026-05-14T16:02:16.256Z
Learnt from: CR
Repo: ProverCoderAI/docker-git PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2026-05-14T16:02:16.256Z
Learning: Applies to **/*.{js,ts,jsx,tsx,py,java,go,rb,php,sh,bash,yml,yaml,json,env*,toml,cfg,config,dockerfile,dockerignore} : Fail if changed files expose credentials, tokens, private-keys, or PII in source, generated config, logs, or CI output
Applied to files:
packages/api/Dockerfile
📚 Learning: 2026-05-14T16:02:16.256Z
Learnt from: CR
Repo: ProverCoderAI/docker-git PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2026-05-14T16:02:16.256Z
Learning: Applies to **/*.{sh,bash,py,js,ts,jsx,tsx,go,java,rb,php} : Fail if changed files introduce command injection or unsafe shell/process execution with user-controlled input
Applied to files:
packages/api/Dockerfile
📚 Learning: 2026-05-13T07:10:13.213Z
Learnt from: CR
Repo: ProverCoderAI/docker-git PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-13T07:10:13.213Z
Learning: Applies to **/*.{test,spec}.{ts,tsx} : Property-based tests (fast-check) must verify mathematical invariants; unit tests must use Effect test utilities without async/await.
Applied to files:
packages/app/tests/docker-git/controller-resource-limits.test.ts
📚 Learning: 2026-05-13T07:09:47.992Z
Learnt from: CR
Repo: ProverCoderAI/docker-git PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-05-13T07:09:47.992Z
Learning: Applies to **/*.{ts,tsx} : 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).
Applied to files:
packages/app/tests/docker-git/controller-resource-limits.test.ts
📚 Learning: 2026-05-13T07:10:13.213Z
Learnt from: CR
Repo: ProverCoderAI/docker-git PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-13T07:10:13.213Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : 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.
Applied to files:
packages/app/tests/docker-git/controller-resource-limits.test.ts
📚 Learning: 2026-05-14T16:02:05.012Z
Learnt from: CR
Repo: ProverCoderAI/docker-git PR: 0
File: docs/integrations/skiller.md:0-0
Timestamp: 2026-05-14T16:02:05.012Z
Learning: Applies to docs/integrations/**/*.(ts|js)?(@(routes|handlers|api|services))* : API must resolve Codex global skills from `~/.codex/skills` within the selected container volume, not falling back to controller's own `~/.codex/skills`
Applied to files:
packages/app/tests/docker-git/controller-resource-limits.test.tspackages/api/src/services/skiller.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/skiller.ts
📚 Learning: 2026-05-14T16:02:05.012Z
Learnt from: CR
Repo: ProverCoderAI/docker-git PR: 0
File: docs/integrations/skiller.md:0-0
Timestamp: 2026-05-14T16:02:05.012Z
Learning: Skiller submodule must be pinned to version v0.2.14 at commit 6ff6b9ca1ff2d78d3af7dac47b03ed1c315dab6b from upstream https://github.com/beautyfree/skiller-desktop-skills-manager
Applied to files:
packages/api/src/services/skiller.ts
📚 Learning: 2026-05-22T21:08:22.631Z
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:22.631Z
Learning: In the docker-git project, `${DOCKER_GIT_CONTROLLER_BUILD_SKILLER:-1}` in compose files is standard bash parameter expansion with default value `"1"` (not a literal "-1" mode). The supported runtime contract for `DOCKER_GIT_CONTROLLER_BUILD_SKILLER` in `packages/app/src/docker-git/controller-compose.ts` is: unset/`"1"`/`"true"` → `"1"`, `"0"`/`"false"` → `"0"`. There is no `-1` mode. The Dockerfile also declares `ARG DOCKER_GIT_CONTROLLER_BUILD_SKILLER=1`.
Applied to files:
packages/api/src/services/skiller.ts
🔇 Additional comments (6)
docs/pr-screenshots/issue-365/skiller-projects-after-click.json (1)
1-12: LGTM!packages/app/tests/docker-git/controller-resource-limits.test.ts (3)
21-22: LGTM!
56-61: LGTM!Also applies to: 88-93
98-109: LGTM!packages/api/Dockerfile (1)
125-128: LGTM!packages/api/tests/skiller-routes.test.ts (1)
35-67: LGTM!
|
Addressed review 4421915309 in f5cdf39:
Validation:
Existing app proof screenshots in this PR:
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
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/api/src/services/skiller.ts`:
- Around line 191-207: runProcess currently spawns child processes without any
time limit; add a timeout mechanism using an AbortController/AbortSignal (create
controller inside runProcess and pass controller.signal into the spawn options)
and start a timer that aborts/kills the child when elapsed, clears the timer on
normal exit, and rejects with a distinct timeout Error; ensure you clean up
event listeners and call child.kill (or let the AbortSignal terminate the
process) and propagate the abort reason so callers of runProcess can distinguish
timeout vs other errors.
🪄 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: fe924ac4-e5b6-4abe-a9ab-ece127713ce0
📒 Files selected for processing (2)
packages/api/src/services/skiller.tspackages/api/tests/skiller-routes.test.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 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/api/tests/skiller-routes.test.tspackages/api/src/services/skiller.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/api/tests/skiller-routes.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/api/tests/skiller-routes.test.tspackages/api/src/services/skiller.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/api/tests/skiller-routes.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/api/tests/skiller-routes.test.tspackages/api/src/services/skiller.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/api/tests/skiller-routes.test.tspackages/api/src/services/skiller.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/api/tests/skiller-routes.test.tspackages/api/src/services/skiller.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Ты строгий ревьюер SPEC DRIVEN DEVELOPMENT.Перед выводами изучи README.md, другие *.md файлы, linked issues,
PR description, PR comments/discussion и релевантную кодовую базу.Сверь изменения с исходным ТЗ/спекой и обсуждением. Флагай любой уход
от спеки, недокументированное изменение поведения, отсутствие тестов
для заявленного поведения и security-риск. Если спека не видна,
попроси автора добавить ее в issue или PR description.Проверь решение с точки зрения формальной верификации: какие инварианты,
предусловия и постусловия можно доказать математически, а где доказуемость
слабая. Оцени решение с точки зрения теории игр: устойчивы ли стимулы,
нет ли выгодного обхода правил, и какое решение было бы сильнее.
Files:
packages/api/tests/skiller-routes.test.tspackages/api/src/services/skiller.ts
🧠 Learnings (14)
📓 Common learnings
Learnt from: ezocomp118-source
Repo: ProverCoderAI/docker-git PR: 366
File: packages/api/src/services/skiller.ts:366-369
Timestamp: 2026-06-03T17:35:44.689Z
Learning: In ProverCoderAI/docker-git, launching Electron via `setpriv --reuid/--regid` in the controller container causes a `Trace/breakpoint trap` crash even with `--no-sandbox`. The confirmed workaround is to launch Skiller as the controller user (bash -lc) and enforce project scope at the software level via SkillerContainerScope env vars (HOME, XDG_*, DOCKER_GIT_SKILLER_CONTAINER_HOME_PATH) and the add_project tRPC call. A future fix should use gosu/su-exec or a Chromium-sandbox-compatible credential-drop mechanism instead of setpriv. See PR `#366` for full context.
Learnt from: CR
Repo: ProverCoderAI/docker-git PR: 0
File: docs/integrations/skiller.md:0-0
Timestamp: 2026-05-14T16:02:05.012Z
Learning: Applies to docs/integrations/**/*.(ts|js)?(@(routes|handlers|api|middleware))* : API must scope Skiller filesystem access to the active project container by mapping `/home/<sshUser>` and project `targetDir` to controller-visible Docker volume paths
Learnt from: CR
Repo: ProverCoderAI/docker-git PR: 0
File: docs/integrations/skiller.md:0-0
Timestamp: 2026-05-14T16:02:05.012Z
Learning: Applies to docs/integrations/**/*.(ts|js)?(@(routes|handlers|api))* : API endpoint `POST /projects/by-key/:projectKey/terminal-sessions/:sessionId/skiller/open` must launch Skiller Electron app, register terminal session filesystem scope, and write output to ~/.docker-git/logs/skiller.log
Learnt from: CR
Repo: ProverCoderAI/docker-git PR: 0
File: docs/integrations/skiller.md:0-0
Timestamp: 2026-05-14T16:02:05.012Z
Learning: docker-git must not import Skiller source code into the docker-git web bundle; serve Skiller browser UI from Skiller's own built renderer and proxy tRPC requests to Skiller's separate backend process
📚 Learning: 2026-05-14T16:02:05.012Z
Learnt from: CR
Repo: ProverCoderAI/docker-git PR: 0
File: docs/integrations/skiller.md:0-0
Timestamp: 2026-05-14T16:02:05.012Z
Learning: Applies to docs/integrations/**/*.(ts|js)?(@(routes|handlers|api))* : API endpoint `POST /projects/by-key/:projectKey/terminal-sessions/:sessionId/skiller/open` must launch Skiller Electron app, register terminal session filesystem scope, and write output to ~/.docker-git/logs/skiller.log
Applied to files:
packages/api/tests/skiller-routes.test.tspackages/api/src/services/skiller.ts
📚 Learning: 2026-05-14T16:02:05.012Z
Learnt from: CR
Repo: ProverCoderAI/docker-git PR: 0
File: docs/integrations/skiller.md:0-0
Timestamp: 2026-05-14T16:02:05.012Z
Learning: Applies to docs/integrations/**/*.(ts|js|sh|bash)?(launcher|skiller)* : Skiller launcher must use `xvfb-run` if available when API process has no `$DISPLAY` environment variable to enable headless startup
Applied to files:
packages/api/tests/skiller-routes.test.tspackages/api/src/services/skiller.ts
📚 Learning: 2026-06-03T17:35:44.689Z
Learnt from: ezocomp118-source
Repo: ProverCoderAI/docker-git PR: 366
File: packages/api/src/services/skiller.ts:366-369
Timestamp: 2026-06-03T17:35:44.689Z
Learning: In ProverCoderAI/docker-git, launching Electron via `setpriv --reuid/--regid` in the controller container causes a `Trace/breakpoint trap` crash even with `--no-sandbox`. The confirmed workaround is to launch Skiller as the controller user (bash -lc) and enforce project scope at the software level via SkillerContainerScope env vars (HOME, XDG_*, DOCKER_GIT_SKILLER_CONTAINER_HOME_PATH) and the add_project tRPC call. A future fix should use gosu/su-exec or a Chromium-sandbox-compatible credential-drop mechanism instead of setpriv. See PR `#366` for full context.
Applied to files:
packages/api/tests/skiller-routes.test.tspackages/api/src/services/skiller.ts
📚 Learning: 2026-05-14T16:02:05.012Z
Learnt from: CR
Repo: ProverCoderAI/docker-git PR: 0
File: docs/integrations/skiller.md:0-0
Timestamp: 2026-05-14T16:02:05.012Z
Learning: Applies to docs/integrations/**/*.(ts|js)?(@(routes|handlers|api|middleware))* : API must scope Skiller filesystem access to the active project container by mapping `/home/<sshUser>` and project `targetDir` to controller-visible Docker volume paths
Applied to files:
packages/api/tests/skiller-routes.test.tspackages/api/src/services/skiller.ts
📚 Learning: 2026-05-14T16:02:16.256Z
Learnt from: CR
Repo: ProverCoderAI/docker-git PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2026-05-14T16:02:16.256Z
Learning: Applies to **/*.{py,js,ts,jsx,tsx,go,java,rb,php,sh,bash,c,cpp} : Fail if changed files introduce path traversal or writes outside intended project/container state directories
Applied to files:
packages/api/tests/skiller-routes.test.tspackages/api/src/services/skiller.ts
📚 Learning: 2026-05-14T16:02:05.012Z
Learnt from: CR
Repo: ProverCoderAI/docker-git PR: 0
File: docs/integrations/skiller.md:0-0
Timestamp: 2026-05-14T16:02:05.012Z
Learning: docker-git must not import Skiller source code into the docker-git web bundle; serve Skiller browser UI from Skiller's own built renderer and proxy tRPC requests to Skiller's separate backend process
Applied to files:
packages/api/src/services/skiller.ts
📚 Learning: 2026-05-14T16:02:05.012Z
Learnt from: CR
Repo: ProverCoderAI/docker-git PR: 0
File: docs/integrations/skiller.md:0-0
Timestamp: 2026-05-14T16:02:05.012Z
Learning: Applies to docs/integrations/**/*.(ts|js)?(@(routes|handlers|api|services))* : API must resolve Codex global skills from `~/.codex/skills` within the selected container volume, not falling back to controller's own `~/.codex/skills`
Applied to files:
packages/api/src/services/skiller.ts
📚 Learning: 2026-05-14T16:02:16.256Z
Learnt from: CR
Repo: ProverCoderAI/docker-git PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2026-05-14T16:02:16.256Z
Learning: Applies to **/{Dockerfile*,docker-compose*.{yml,yaml},.dockerignore} : Fail if changed files introduce unsafe Docker configuration such as privileged containers, broad host mounts, unbounded Docker socket access, or unnecessary write permissions
Applied to files:
packages/api/src/services/skiller.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/skiller.ts
📚 Learning: 2026-05-14T16:02:05.012Z
Learnt from: CR
Repo: ProverCoderAI/docker-git PR: 0
File: docs/integrations/skiller.md:0-0
Timestamp: 2026-05-14T16:02:05.012Z
Learning: Skiller submodule must be pinned to version v0.2.14 at commit 6ff6b9ca1ff2d78d3af7dac47b03ed1c315dab6b from upstream https://github.com/beautyfree/skiller-desktop-skills-manager
Applied to files:
packages/api/src/services/skiller.ts
📚 Learning: 2026-05-14T16:02:05.012Z
Learnt from: CR
Repo: ProverCoderAI/docker-git PR: 0
File: docs/integrations/skiller.md:0-0
Timestamp: 2026-05-14T16:02:05.012Z
Learning: Run `bun run skiller:check` after updating the Skiller pin in addition to docker-git `typecheck` and `check` scripts to verify integration
Applied to files:
packages/api/src/services/skiller.ts
📚 Learning: 2026-05-14T16:02:05.012Z
Learnt from: CR
Repo: ProverCoderAI/docker-git PR: 0
File: docs/integrations/skiller.md:0-0
Timestamp: 2026-05-14T16:02:05.012Z
Learning: Initialize Skiller submodule using `bun run skiller:init` command and do not include the submodule in root workspace configuration
Applied to files:
packages/api/src/services/skiller.ts
📚 Learning: 2026-05-13T07:10:13.213Z
Learnt from: CR
Repo: ProverCoderAI/docker-git PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-13T07:10:13.213Z
Learning: Interop with Promise/exceptions is permitted ONLY in SHELL via `Effect.try` or `Effect.tryPromise` with typed error mapping.
Applied to files:
packages/api/src/services/skiller.ts
🔇 Additional comments (12)
packages/api/src/services/skiller.ts (11)
52-69: LGTM!
331-335: LGTM!
365-385: LGTM!
500-517: LGTM!
519-534: LGTM!
536-566: LGTM!
568-578: LGTM!
580-606: LGTM!
608-613: LGTM!
651-696: LGTM!
758-764: LGTM!packages/api/tests/skiller-routes.test.ts (1)
69-75: LGTM!
|
Addressed review 4422425479 in 61ec4a9:
Validation:
Existing app proof screenshots remain in this PR:
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
2 participants
Source TZ / Issues
Summary
/var/lib/dockerinto the default host-mode controller so Skiller can access Docker volume paths returned bydocker inspect.docker_git_docker_datavolume.Requirements Alignment
/var/lib/docker, matching its existing host Docker socket access model.Verification
bun --cwd packages/app vitest run tests/docker-git/controller-resource-limits.test.tsbun --cwd packages/api vitest run tests/skiller-core.test.tsbun run --cwd packages/app test -- tests/docker-git/controller-resource-limits.test.tsbun run --cwd packages/api test -- tests/skiller-core.test.tsbun run testgit diff --checkdocker compose configfor host and isolated volume resolution