fix(shell): prepare host-backed controller builds#351
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
📜 Recent review details⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
🧰 Additional context used📓 Path-based instructions (12)**/*.{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)
Files:
**/{Dockerfile*,docker-compose*.{yml,yaml},.dockerignore}📄 CodeRabbit inference engine (Custom checks)
Files:
docker-compose*.yml📄 CodeRabbit inference engine (README.md)
Files:
docker-compose.api.yml📄 CodeRabbit inference engine (README.md)
Files:
**/*⚙️ CodeRabbit configuration file
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.test.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{test,spec}.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{sh,bash,py,js,ts,jsx,tsx,go,java,rb,php}📄 CodeRabbit inference engine (Custom checks)
Files:
**/*.{py,js,ts,jsx,tsx,go,java,rb,php,sh,bash,c,cpp}📄 CodeRabbit inference engine (Custom checks)
Files:
packages/app/**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (README.md)
Files:
🧠 Learnings (1)📚 Learning: 2026-05-22T21:08:18.083ZApplied to files:
🔇 Additional comments (8)
📝 WalkthroughSummary by CodeRabbit
WalkthroughПереключение Docker runtime контроллера/API на host (монтирование /var/run/docker.sock и параметризация privileged) и добавление инициализации pinned Skiller submodule, интегрированной в prepareControllerRevision, с соответствующими тестами и расширением входов ревизии. ИзмененияDocker Host Runtime
Инициализация Skiller подмодуля
🎯 4 (Сложно) | ⏱️ ~45 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)
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: 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/README.md`:
- Line 64: Update the README to remove the contradiction by making both the
runtime contract and the `DOCKER_GIT_DOCKER_RUNTIME` description consistent:
state that the default mode is host-backed (`host`) and that `isolated` is an
optional fallback for cases where an embedded controller daemon is explicitly
required, rather than implying full DinD support; specifically update the
`DOCKER_GIT_DOCKER_RUNTIME` bullet (the `DOCKER_GIT_DOCKER_RUNTIME` symbol) to
"(default: host; set to isolated as an optional fallback to use an embedded
controller daemon)" and adjust the earlier "runtime contract" paragraph to
explicitly note that host-backed is the primary model and `isolated` is an
opt-in fallback.
🪄 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: 183fb467-94b9-4d2d-b5cf-46994ee7d685
📒 Files selected for processing (7)
docker-compose.api.ymldocker-compose.ymlpackages/api/Dockerfilepackages/api/README.mdpackages/api/scripts/start-controller.shpackages/app/src/docker-git/controller-compose.tspackages/app/src/docker-git/controller-revision.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{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/scripts/start-controller.shpackages/app/src/docker-git/controller-compose.tspackages/app/src/docker-git/controller-revision.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/scripts/start-controller.shpackages/app/src/docker-git/controller-compose.tspackages/app/src/docker-git/controller-revision.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/scripts/start-controller.shdocker-compose.api.ymlpackages/app/src/docker-git/controller-compose.tsdocker-compose.ymlpackages/app/src/docker-git/controller-revision.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Ты строгий ревьюер SPEC DRIVEN DEVELOPMENT.Перед выводами изучи README.md, другие *.md файлы, linked issues,
PR description, PR comments/discussion и релевантную кодовую базу.Сверь изменения с исходным ТЗ/спекой и обсуждением. Флагай любой уход
от спеки, недокументированное изменение поведения, отсутствие тестов
для заявленного поведения и security-риск. Если спека не видна,
попроси автора добавить ее в issue или PR description.Проверь решение с точки зрения формальной верификации: какие инварианты,
предусловия и постусловия можно доказать математически, а где доказуемость
слабая. Оцени решение с точки зрения теории игр: устойчивы ли стимулы,
нет ли выгодного обхода правил, и какое решение было бы сильнее.
Files:
packages/api/scripts/start-controller.shpackages/api/README.mddocker-compose.api.ymlpackages/app/src/docker-git/controller-compose.tsdocker-compose.ymlpackages/api/Dockerfilepackages/app/src/docker-git/controller-revision.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:
docker-compose.api.ymldocker-compose.ymlpackages/api/Dockerfile
docker-compose*.yml
📄 CodeRabbit inference engine (README.md)
docker-compose*.yml: Per-project containers should ship with a default CPU limit of 30% and RAM limit of 30% (resolved against the host), overridable via --cpu / --ram flags or per-project docker-git.json
GPU mode should default to 'none' for non-GPU projects; GPU is enabled only explicitly via --gpu all or saved 'gpu': 'all' in docker-git.json
Files:
docker-compose.api.ymldocker-compose.yml
docker-compose.api.yml
📄 CodeRabbit inference engine (README.md)
Controller container (docker-git-api) should be capped with default CPU/RAM at 90% of host resources with memory swap defaulting to twice the resolved RAM limit, overridable via CLI flags or environment variables
Files:
docker-compose.api.yml
**/*.{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/docker-git/controller-compose.tspackages/app/src/docker-git/controller-revision.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/docker-git/controller-compose.tspackages/app/src/docker-git/controller-revision.ts
packages/app/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (README.md)
App layer (APP) should work only with API and not have direct access to LIB layer
Files:
packages/app/src/docker-git/controller-compose.tspackages/app/src/docker-git/controller-revision.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-compose.tspackages/app/src/docker-git/controller-revision.ts
🔇 Additional comments (7)
docker-compose.api.yml (1)
11-11: LGTM!Also applies to: 15-15, 36-36
docker-compose.yml (1)
13-13: LGTM!Also applies to: 17-17, 38-38
packages/api/Dockerfile (1)
99-99: LGTM!packages/api/scripts/start-controller.sh (1)
4-4: LGTM!packages/api/README.md (1)
8-10: LGTM!Also applies to: 68-69
packages/app/src/docker-git/controller-compose.ts (1)
1-1: LGTM!Also applies to: 8-8, 21-23, 69-108, 109-147, 188-195, 224-235
packages/app/src/docker-git/controller-revision.ts (1)
11-11: LGTM!Also applies to: 20-21, 24-24
- Default controller runtime to host Docker with socket mount - Initialize the pinned Skiller submodule before controller revision/build - Include Skiller inputs in controller revision hashing
rikohomeless
force-pushed
the
codex/controller-host-runtime-skiller
branch
from
9fd66e9 to
fefcc47
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/api/Dockerfile (1)
137-157:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftДобавьте non-root
USERв финальный stage.
controller-finalзапускается под root. Для контроллера с доступом к Docker это заметно ухудшает security posture. Нужен явный non-root пользователь в финальном образе с минимально необходимыми правами.As per coding guidelines: "
**/{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".🤖 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/Dockerfile` around lines 137 - 157, The final stage named "controller-final" currently runs as root; create and switch to a dedicated non-root user (e.g., add a user/group such as "controller" or "appuser") inside that stage, ensure any runtime needs (access to the Docker socket at DOCKER_HOST, required files under /workspace or packages/api) are given by minimal ownership/permissions or group membership (chown/chgrp the specific runtime directories or add the user to a docker group instead of leaving root), and then set USER to that non-root account before CMD; update any file permissions needed for tsc output or start-controller.sh so the non-root user can execute them but give no broader privileges.
🤖 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 `@docker-compose.api.yml`:
- Around line 14-15: The compose file currently sets DOCKER_GIT_DOCKER_RUNTIME
and DOCKER_HOST which, combined with a bind-mount of /var/run/docker.sock and
privileged: true, grants the controller full host control; remove or disable
privileged by default (remove privileged: true from the service) and restrict
Docker socket access (avoid unconditional bind-mount of /var/run/docker.sock or
make it read-only) and move any privileged requirements into a separate
override/profile (e.g., an isolated docker-compose override or profile) so
DOCKER_GIT_DOCKER_RUNTIME and DOCKER_HOST can still be used for host-mode but
without broad host privileges; apply the same changes to the other service block
that uses /var/run/docker.sock and privileged (the entries referenced around the
second DOCKER_GIT_DOCKER_RUNTIME/DOCKER_HOST usage).
In `@packages/app/tests/docker-git/controller-compose.test.ts`:
- Around line 4-6: Replace the banned node:* imports in
packages/app/tests/docker-git/controller-compose.test.ts by rewriting the
temporary directory/file/path helpers to use the FileSystem and Path services
from `@effect/platform` (or `@effect/platform-node`) instead of
node:fs/node:os/node:path; remove the node:* imports and change helper
implementations (the temp dir/file/path creation utilities used by the tests) to
call FileSystem and Path APIs and ensure those services are provided to your
Effects via a Layer (e.g., provideLayer with the appropriate FileSystem/Path
Layer) so tests use the injected platform services rather than direct node
imports.
- Around line 149-162: Replace the two-case unit test with (or add) a fast-check
property test that repeatedly calls prepareRevisionInTemporaryRoot with
generated values for buildSkillerMode and includeSkillerPackage (use a generator
that covers undefined and the allowed string/number modes for buildSkillerMode
and booleans for includeSkillerPackage), and for each generated input assert via
expectPreparedRevision that the produced revision matches the expected suffix
pattern (check the skiller suffix is "skiller1" vs "skiller0" per mode) and that
the persistedRevision equals revision (i.e., persistedRevision === revision);
locate and modify the test around prepareRevisionInTemporaryRoot and
expectPreparedRevision in controller-compose.test.ts to implement the
fc.property assertion.
---
Outside diff comments:
In `@packages/api/Dockerfile`:
- Around line 137-157: The final stage named "controller-final" currently runs
as root; create and switch to a dedicated non-root user (e.g., add a user/group
such as "controller" or "appuser") inside that stage, ensure any runtime needs
(access to the Docker socket at DOCKER_HOST, required files under /workspace or
packages/api) are given by minimal ownership/permissions or group membership
(chown/chgrp the specific runtime directories or add the user to a docker group
instead of leaving root), and then set USER to that non-root account before CMD;
update any file permissions needed for tsc output or start-controller.sh so the
non-root user can execute them but give no broader privileges.
🪄 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: 71671645-6963-4162-a535-ee9532c83c9a
📒 Files selected for processing (8)
docker-compose.api.ymldocker-compose.ymlpackages/api/Dockerfilepackages/api/README.mdpackages/api/scripts/start-controller.shpackages/app/src/docker-git/controller-compose.tspackages/app/src/docker-git/controller-revision.tspackages/app/tests/docker-git/controller-compose.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). (6)
- GitHub Check: E2E (Clone auto-open SSH)
- GitHub Check: E2E (Login context)
- GitHub Check: E2E (Clone cache)
- GitHub Check: E2E (OpenCode)
- GitHub Check: E2E (Runtime volumes + SSH)
- GitHub Check: Lint
🧰 Additional context used
📓 Path-based instructions (12)
**/{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/Dockerfiledocker-compose.api.ymldocker-compose.yml
**/*
⚙️ CodeRabbit configuration file
**/*: Ты строгий ревьюер SPEC DRIVEN DEVELOPMENT.Перед выводами изучи README.md, другие *.md файлы, linked issues,
PR description, PR comments/discussion и релевантную кодовую базу.Сверь изменения с исходным ТЗ/спекой и обсуждением. Флагай любой уход
от спеки, недокументированное изменение поведения, отсутствие тестов
для заявленного поведения и security-риск. Если спека не видна,
попроси автора добавить ее в issue или PR description.Проверь решение с точки зрения формальной верификации: какие инварианты,
предусловия и постусловия можно доказать математически, а где доказуемость
слабая. Оцени решение с точки зрения теории игр: устойчивы ли стимулы,
нет ли выгодного обхода правил, и какое решение было бы сильнее.
Files:
packages/api/Dockerfilepackages/api/scripts/start-controller.shdocker-compose.api.ymldocker-compose.ymlpackages/api/README.mdpackages/app/tests/docker-git/controller-compose.test.tspackages/app/src/docker-git/controller-revision.tspackages/app/src/docker-git/controller-compose.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/scripts/start-controller.shpackages/app/tests/docker-git/controller-compose.test.tspackages/app/src/docker-git/controller-revision.tspackages/app/src/docker-git/controller-compose.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/scripts/start-controller.shpackages/app/tests/docker-git/controller-compose.test.tspackages/app/src/docker-git/controller-revision.tspackages/app/src/docker-git/controller-compose.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/scripts/start-controller.shdocker-compose.api.ymldocker-compose.ymlpackages/app/tests/docker-git/controller-compose.test.tspackages/app/src/docker-git/controller-revision.tspackages/app/src/docker-git/controller-compose.ts
docker-compose*.yml
📄 CodeRabbit inference engine (README.md)
docker-compose*.yml: Per-project containers should ship with a default CPU limit of 30% and RAM limit of 30% (resolved against the host), overridable via --cpu / --ram flags or per-project docker-git.json
GPU mode should default to 'none' for non-GPU projects; GPU is enabled only explicitly via --gpu all or saved 'gpu': 'all' in docker-git.json
Files:
docker-compose.api.ymldocker-compose.yml
docker-compose.api.yml
📄 CodeRabbit inference engine (README.md)
Controller container (docker-git-api) should be capped with default CPU/RAM at 90% of host resources with memory swap defaulting to twice the resolved RAM limit, overridable via CLI flags or environment variables
Files:
docker-compose.api.yml
**/*.{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-compose.test.tspackages/app/src/docker-git/controller-revision.tspackages/app/src/docker-git/controller-compose.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-compose.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-compose.test.tspackages/app/src/docker-git/controller-revision.tspackages/app/src/docker-git/controller-compose.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-compose.test.ts
packages/app/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (README.md)
App layer (APP) should work only with API and not have direct access to LIB layer
Files:
packages/app/tests/docker-git/controller-compose.test.tspackages/app/src/docker-git/controller-revision.tspackages/app/src/docker-git/controller-compose.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-revision.tspackages/app/src/docker-git/controller-compose.ts
🪛 Checkov (3.2.529)
packages/api/Dockerfile
[low] 1-157: Ensure that HEALTHCHECK instructions have been added to container images
(CKV_DOCKER_2)
[low] 1-157: Ensure that a user for the container has been created
(CKV_DOCKER_3)
🪛 GitHub Check: Lint Effect-TS
packages/app/tests/docker-git/controller-compose.test.ts
[failure] 6-6:
'node:path' import is restricted from being used by a pattern. Do not import from node:* directly. Use @effect/platform-node or @effect/platform services
[failure] 6-6:
'node:path' import is restricted from being used. Use @effect/platform Path instead of node:path
[failure] 5-5:
'node:os' import is restricted from being used by a pattern. Do not import from node:* directly. Use @effect/platform-node or @effect/platform services
[failure] 4-4:
'node:fs' import is restricted from being used by a pattern. Do not import from node:* directly. Use @effect/platform-node or @effect/platform services
[failure] 4-4:
'node:fs' import is restricted from being used. Use @effect/platform FileSystem instead of node:fs
🪛 Hadolint (2.14.0)
packages/api/Dockerfile
[info] 89-89: Multiple consecutive RUN instructions. Consider consolidation.
(DL3059)
[warning] 105-105: Use WORKDIR to switch to a directory
(DL3003)
🪛 Trivy (0.69.3)
packages/api/Dockerfile
[error] 1-1: Image user should not be 'root'
Specify at least 1 USER command in Dockerfile with non-root user as argument
Rule: DS-0002
(IaC/Dockerfile)
[info] 1-1: No HEALTHCHECK defined
Add HEALTHCHECK instruction in your Dockerfile
Rule: DS-0026
(IaC/Dockerfile)
[error] 1-1: Image user should not be 'root'
Specify at least 1 USER command in Dockerfile with non-root user as argument
Rule: DS-0002
(IaC/Dockerfile)
[info] 1-1: No HEALTHCHECK defined
Add HEALTHCHECK instruction in your Dockerfile
Rule: DS-0026
(IaC/Dockerfile)
[warning] 105-135: 'RUN cd ...' to change directory
RUN should not be used to change directory: 'if [ "$DOCKER_GIT_CONTROLLER_BUILD_SKILLER" = "1" ]; then test -f third_party/skiller-desktop-skills-manager/package.json && cd third_party/skiller-desktop-skills-manager && for attempt in 1 2 3 4 5; do if bun install --frozen-lockfile --silent; then break; fi; if [ "$attempt" = "5" ]; then echo "skiller bun install failed after retries" >&2; exit 1; fi; echo "skiller bun install attempt
Rule: DS-0013
(IaC/Dockerfile)
🔇 Additional comments (7)
docker-compose.yml (1)
14-15: Дублирующийся security-риск с Docker socket.Та же проблема уже отмечена в
docker-compose.api.yml(host socket + privileged). Не дублирую отдельное замечание по существу.Also applies to: 39-40
packages/api/scripts/start-controller.sh (1)
4-4: LGTM!packages/api/README.md (1)
8-10: LGTM!Also applies to: 14-19, 65-65, 69-70
packages/app/src/docker-git/controller-compose.ts (2)
1-25: LGTM!Also applies to: 39-77, 93-111, 133-170, 213-270
112-131: ⚡ Quick winДобавьте timeout/abort для
git submodule updateв bootstrap-пути и маппинг вControllerBootstrapError.
runSkillerSubmoduleInit(packages/app/src/docker-git/controller-compose.ts, строки 112–131) запускает внешнийgitчерезrunCommandWithCapturedOutputбез явного ограничения времени; зависание (ожидание ввода/доступа/сети) может привести к бесконечному ожиданию в процессе подготовки контроллера. Ограничьте время выполнения и корректно конвертируйте таймаут в типизированную ошибкуControllerBootstrapError(отдельный_tag/случай).packages/app/src/docker-git/controller-revision.ts (1)
10-24: LGTM!Also applies to: 26-37, 119-146
packages/app/tests/docker-git/controller-compose.test.ts (1)
133-147: 🏗️ Heavy liftСделать тест детерминированным: замокировать
CommandExecutorвместо реальногоgitВ показанном фрагменте теста
CommandExecutorне видно как замоканный (используется толькоEffect.provide(NodeContext.layer)), поэтомуensureSkillerSubmoduleInitialized(rootDir)может исполнять системныйgit, делая проверку недетерминированной. ПодменитеCommandExecutorчерез Layer в этом тесте и проверяйте как минимум_tagошибки (не толькоerror.message).
rikohomeless
force-pushed
the
codex/controller-host-runtime-skiller
branch
from
fefcc47 to
85dc948
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/app/tests/docker-git/controller-compose.test.ts (1)
154-167:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftДобавьте property-based тест с
fast-checkдля инварианта ревизий контроллераТекущий тест проверяет только 2 фиксированных случая (
buildSkillerMode: undefinedи"0"), но по требованию guidelines для**/*.test.{ts,tsx}и**/*.{test,spec}.{ts,tsx}необходимо использовать property-based testing сfast-checkдля проверки математических инвариантов на всем домене допустимых значений.Инварианты, которые нужно проверить:
- Формат revision:
[16-char-hex]-none-skiller[0|1]в зависимости отbuildSkillerModepersistedRevision === revisionвсегда- Суффикс
skiller1дляundefinedили"1", суффиксskiller0для"0"Добавьте
fc.property(...)с генераторами дляbuildSkillerMode(undefined, "0", "1") иincludeSkillerPackage(boolean), чтобы проверить инвариант для большого количества сгенерированных комбинаций.As per coding guidelines: "Implement property-based testing using fast-check for mathematical properties and invariants" (
**/*.test.{ts,tsx}) and "Property-based tests (fast-check) must verify mathematical invariants" (**/*.{test,spec}.{ts,tsx}).🤖 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/controller-compose.test.ts` around lines 154 - 167, Replace the two fixed cases in the it.effect test by a fast-check property: use fc.property with an fc.constantFrom generator for buildSkillerMode (undefined, "0", "1") and fc.boolean() for includeSkillerPackage, run many samples, and for each generated pair call prepareRevisionInTemporaryRoot(buildSkillerMode, includeSkillerPackage) and assert the invariants using expectPreparedRevision and direct checks: (1) revision matches /^[a-f0-9]{16}-none-skiller[01]$/u, (2) persistedRevision === revision, and (3) the suffix is "skiller1" when buildSkillerMode is undefined or "1" and "skiller0" when buildSkillerMode is "0"; keep using the existing helpers prepareRevisionInTemporaryRoot and expectPreparedRevision and integrate fc.assert inside the existing it.effect.
🤖 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/README.md`:
- Around line 12-19: Add a short security note after the "Runtime contract:
host-Docker-backed" paragraph explaining that mounting /var/run/docker.sock
grants the controller container root-equivalent access to the host Docker
daemon; mention this is an intended trade-off for the host-backed architecture
and recommend running the controller only in trusted environments and reviewing
the threat model before exposing the API. Reference the README section title
"Runtime contract: host-Docker-backed" to locate where to insert the note.
---
Duplicate comments:
In `@packages/app/tests/docker-git/controller-compose.test.ts`:
- Around line 154-167: Replace the two fixed cases in the it.effect test by a
fast-check property: use fc.property with an fc.constantFrom generator for
buildSkillerMode (undefined, "0", "1") and fc.boolean() for
includeSkillerPackage, run many samples, and for each generated pair call
prepareRevisionInTemporaryRoot(buildSkillerMode, includeSkillerPackage) and
assert the invariants using expectPreparedRevision and direct checks: (1)
revision matches /^[a-f0-9]{16}-none-skiller[01]$/u, (2) persistedRevision ===
revision, and (3) the suffix is "skiller1" when buildSkillerMode is undefined or
"1" and "skiller0" when buildSkillerMode is "0"; keep using the existing helpers
prepareRevisionInTemporaryRoot and expectPreparedRevision and integrate
fc.assert inside the existing it.effect.
🪄 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: 6385609a-a2af-4b8e-90bf-b625e7352885
📒 Files selected for processing (2)
packages/api/README.mdpackages/app/tests/docker-git/controller-compose.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 (Login context)
- GitHub Check: E2E (OpenCode)
- GitHub Check: E2E (Clone cache)
- GitHub Check: E2E (Browser command)
- GitHub Check: Test
- GitHub Check: Lint
- 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/app/tests/docker-git/controller-compose.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-compose.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-compose.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-compose.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-compose.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-compose.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-compose.test.ts
packages/app/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (README.md)
App layer (APP) should work only with API and not have direct access to LIB layer
Files:
packages/app/tests/docker-git/controller-compose.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-compose.test.tspackages/api/README.md
🔇 Additional comments (2)
packages/api/README.md (2)
14-19: LGTM!
65-65: LGTM!
rikohomeless
force-pushed
the
codex/controller-host-runtime-skiller
branch
2 times, most recently
from
cabdf6d to
a2eacf4
Compare
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/app/src/docker-git/controller-compose.ts (1)
163-163:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winНекорректный маппер ошибок: сообщение о docker-compose.yml для проверки Skiller package.json.
mapComposePathErrorгенерирует сообщение"Failed to resolve docker-compose.yml path", но используется для проверки существованияskillerPackagePath. При ошибке файловой системы пользователь получит вводящее в заблуждение сообщение.🛠️ Предлагаемое исправление
Добавить специализированный маппер:
const mapSkillerPathError = (error: PlatformError): ControllerBootstrapError => controllerBootstrapError(`Failed to check Skiller submodule path.\nDetails: ${String(error)}`)И использовать его в строках 163 и 171:
- const existsBeforeInit = yield* _(fs.exists(packagePath).pipe(Effect.mapError(mapComposePathError))) + const existsBeforeInit = yield* _(fs.exists(packagePath).pipe(Effect.mapError(mapSkillerPathError)))- const existsAfterInit = yield* _(fs.exists(packagePath).pipe(Effect.mapError(mapComposePathError))) + const existsAfterInit = yield* _(fs.exists(packagePath).pipe(Effect.mapError(mapSkillerPathError)))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/app/src/docker-git/controller-compose.ts` at line 163, The FS error mapper used for checking the Skiller package path is wrong: replace use of mapComposePathError with a new specialized mapper (e.g., mapSkillerPathError) that returns a ControllerBootstrapError with a message like "Failed to check Skiller submodule path.\nDetails: ${String(error)}"; add this new function and use it wherever skillerPackagePath existence/reads are checked (references: the existsBeforeInit call and the subsequent check that currently also passes mapComposePathError), ensuring the skiller path checks produce the correct, non-misleading error message.
♻️ Duplicate comments (1)
packages/app/tests/docker-git/controller-compose.test.ts (1)
162-175:⚠️ Potential issue | 🟠 Major | ⚡ Quick winНедостаточное покрытие инварианта ревизии для режимов Skiller.
В Line 162-175 проверяются только два фиксированных случая, поэтому инвариант
persistedRevision === revisionи формирование суффикса по допустимым режимам не доказаны на всём домене входов. Здесь нужен property-based тест.#!/bin/bash rg -n "fast-check|fc\\.(assert|property|asyncProperty)|prepares and persists host controller revisions" packages/app/tests/docker-git/controller-compose.test.tsAs 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/controller-compose.test.ts` around lines 162 - 175, Current test only covers two fixed inputs; replace or augment the it.effect case that calls prepareRevisionInTemporaryRoot and expectPreparedRevision with a fast-check property that generates the allowed buildSkillerMode values and includeSkillerPackage booleans, then for each generated input call prepareRevisionInTemporaryRoot(buildSkillerMode, includeSkillerPackage) and assert the invariants persistedRevision === revision and that the produced suffix matches the expected pattern (use the same regex logic currently asserted). Reference prepareRevisionInTemporaryRoot, expectPreparedRevision, buildSkillerMode, includeSkillerPackage, persistedRevision and revision when locating and updating the test.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/app/src/docker-git/controller-compose.ts`:
- Around line 125-136: The error message hardcodes "Timeout: 60 seconds" and can
drift from the skillerSubmoduleInitTimeout constant; update the
controllerBootstrapError call inside Effect.timeoutFail to compute the timeout
string from skillerSubmoduleInitTimeout (e.g., convert milliseconds to seconds
or use the same unit the constant represents) and interpolate that computed
value instead of the literal "60 seconds" so the message always matches the
skillerSubmoduleInitTimeout used by Effect.timeoutFail (references:
Effect.timeoutFail, skillerSubmoduleInitTimeout, controllerBootstrapError,
skillerSubmoduleCommand, rootDir).
In `@packages/app/tests/docker-git/controller-compose.test.ts`:
- Around line 129-137: The test for ensureSkillerSubmoduleInitialized only
asserts success but doesn’t prove git submodule update wasn’t run; replace or
inject a mocked CommandExecutor (the executor used by
ensureSkillerSubmoduleInitialized) in the test setup (alongside
temporaryControllerRoot and writeSkillerPackage) that records invocations, run
ensureSkillerSubmoduleInitialized(rootDir), and then assert the mock’s recorded
command list is empty to prove no git commands were executed.
---
Outside diff comments:
In `@packages/app/src/docker-git/controller-compose.ts`:
- Line 163: The FS error mapper used for checking the Skiller package path is
wrong: replace use of mapComposePathError with a new specialized mapper (e.g.,
mapSkillerPathError) that returns a ControllerBootstrapError with a message like
"Failed to check Skiller submodule path.\nDetails: ${String(error)}"; add this
new function and use it wherever skillerPackagePath existence/reads are checked
(references: the existsBeforeInit call and the subsequent check that currently
also passes mapComposePathError), ensuring the skiller path checks produce the
correct, non-misleading error message.
---
Duplicate comments:
In `@packages/app/tests/docker-git/controller-compose.test.ts`:
- Around line 162-175: Current test only covers two fixed inputs; replace or
augment the it.effect case that calls prepareRevisionInTemporaryRoot and
expectPreparedRevision with a fast-check property that generates the allowed
buildSkillerMode values and includeSkillerPackage booleans, then for each
generated input call prepareRevisionInTemporaryRoot(buildSkillerMode,
includeSkillerPackage) and assert the invariants persistedRevision === revision
and that the produced suffix matches the expected pattern (use the same regex
logic currently asserted). Reference prepareRevisionInTemporaryRoot,
expectPreparedRevision, buildSkillerMode, includeSkillerPackage,
persistedRevision and revision when locating and updating the test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 85502edc-5e4c-44a7-b0d2-51864838b9b0
📒 Files selected for processing (8)
docker-compose.api.ymldocker-compose.ymlpackages/api/README.mdpackages/app/src/docker-git/controller-compose.tspackages/app/tests/docker-git/controller-compose.test.tspackages/app/tests/docker-git/controller-image-revision.test.tspackages/app/tests/docker-git/controller.test.tspackages/app/tests/docker-git/fixtures/command-executor.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 (OpenCode)
- GitHub Check: E2E (Login context)
- GitHub Check: E2E (Browser command)
- GitHub Check: E2E (Clone auto-open SSH)
- GitHub Check: E2E (Runtime volumes + SSH)
- GitHub Check: Lint
- GitHub Check: E2E (Clone cache)
- GitHub Check: Test
- GitHub Check: Final build (windows-latest)
🧰 Additional context used
📓 Path-based instructions (12)
**/*.{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:
docker-compose.api.ymlpackages/app/tests/docker-git/fixtures/command-executor.tspackages/app/tests/docker-git/controller.test.tsdocker-compose.ymlpackages/app/src/docker-git/controller-compose.tspackages/app/tests/docker-git/controller-compose.test.tspackages/app/tests/docker-git/controller-image-revision.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:
docker-compose.api.ymldocker-compose.yml
docker-compose*.yml
📄 CodeRabbit inference engine (README.md)
docker-compose*.yml: Per-project containers should ship with a default CPU limit of 30% and RAM limit of 30% (resolved against the host), overridable via --cpu / --ram flags or per-project docker-git.json
GPU mode should default to 'none' for non-GPU projects; GPU is enabled only explicitly via --gpu all or saved 'gpu': 'all' in docker-git.json
Files:
docker-compose.api.ymldocker-compose.yml
docker-compose.api.yml
📄 CodeRabbit inference engine (README.md)
Controller container (docker-git-api) should be capped with default CPU/RAM at 90% of host resources with memory swap defaulting to twice the resolved RAM limit, overridable via CLI flags or environment variables
Files:
docker-compose.api.yml
**/*
⚙️ CodeRabbit configuration file
**/*: Ты строгий ревьюер SPEC DRIVEN DEVELOPMENT.Перед выводами изучи README.md, другие *.md файлы, linked issues,
PR description, PR comments/discussion и релевантную кодовую базу.Сверь изменения с исходным ТЗ/спекой и обсуждением. Флагай любой уход
от спеки, недокументированное изменение поведения, отсутствие тестов
для заявленного поведения и security-риск. Если спека не видна,
попроси автора добавить ее в issue или PR description.Проверь решение с точки зрения формальной верификации: какие инварианты,
предусловия и постусловия можно доказать математически, а где доказуемость
слабая. Оцени решение с точки зрения теории игр: устойчивы ли стимулы,
нет ли выгодного обхода правил, и какое решение было бы сильнее.
Files:
docker-compose.api.ymlpackages/app/tests/docker-git/fixtures/command-executor.tspackages/api/README.mdpackages/app/tests/docker-git/controller.test.tsdocker-compose.ymlpackages/app/src/docker-git/controller-compose.tspackages/app/tests/docker-git/controller-compose.test.tspackages/app/tests/docker-git/controller-image-revision.test.ts
**/*.{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/fixtures/command-executor.tspackages/app/tests/docker-git/controller.test.tspackages/app/src/docker-git/controller-compose.tspackages/app/tests/docker-git/controller-compose.test.tspackages/app/tests/docker-git/controller-image-revision.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/fixtures/command-executor.tspackages/app/tests/docker-git/controller.test.tspackages/app/src/docker-git/controller-compose.tspackages/app/tests/docker-git/controller-compose.test.tspackages/app/tests/docker-git/controller-image-revision.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/fixtures/command-executor.tspackages/app/tests/docker-git/controller.test.tspackages/app/src/docker-git/controller-compose.tspackages/app/tests/docker-git/controller-compose.test.tspackages/app/tests/docker-git/controller-image-revision.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/fixtures/command-executor.tspackages/app/tests/docker-git/controller.test.tspackages/app/src/docker-git/controller-compose.tspackages/app/tests/docker-git/controller-compose.test.tspackages/app/tests/docker-git/controller-image-revision.test.ts
packages/app/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (README.md)
App layer (APP) should work only with API and not have direct access to LIB layer
Files:
packages/app/tests/docker-git/fixtures/command-executor.tspackages/app/tests/docker-git/controller.test.tspackages/app/src/docker-git/controller-compose.tspackages/app/tests/docker-git/controller-compose.test.tspackages/app/tests/docker-git/controller-image-revision.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.test.tspackages/app/tests/docker-git/controller-compose.test.tspackages/app/tests/docker-git/controller-image-revision.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.test.tspackages/app/tests/docker-git/controller-compose.test.tspackages/app/tests/docker-git/controller-image-revision.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-compose.ts
🔇 Additional comments (10)
docker-compose.api.yml (1)
40-40: LGTM!docker-compose.yml (1)
40-40: LGTM!packages/api/README.md (1)
14-19: LGTM!Also applies to: 65-67
packages/app/src/docker-git/controller-compose.ts (4)
5-5: LGTM!
100-100: LGTM!
226-244: LGTM!
267-282: LGTM!packages/app/tests/docker-git/controller-image-revision.test.ts (1)
4-4: LGTM!Also applies to: 9-14
packages/app/tests/docker-git/controller.test.ts (1)
51-56: LGTM!Also applies to: 228-239
packages/app/tests/docker-git/fixtures/command-executor.ts (1)
8-68: LGTM!
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/app/src/docker-git/controller-compose.ts (1)
125-134:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winУберите захардкоженный таймаут из текста ошибки.
В Line 133 литерал
"Timeout: 60 seconds"может разойтись сskillerSubmoduleInitTimeout, если константу поменяют. Лучше формировать значение из самойDuration.💡 Предлагаемое исправление
Effect.timeoutFail({ duration: skillerSubmoduleInitTimeout, onTimeout: () => controllerBootstrapError( [ "Timed out while initializing Skiller submodule before building docker-git controller.", `Command: git ${skillerSubmoduleCommand.join(" ")}`, `Working directory: ${rootDir}`, - "Timeout: 60 seconds" + `Timeout: ${Duration.toSeconds(skillerSubmoduleInitTimeout)} seconds` ].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/app/src/docker-git/controller-compose.ts` around lines 125 - 134, The error message hardcodes "Timeout: 60 seconds" which can diverge from the actual skillerSubmoduleInitTimeout; update the timeout text to be derived from skillerSubmoduleInitTimeout/its Duration representation instead of a literal. Locate the Effect.timeoutFail invocation that calls controllerBootstrapError (and references skillerSubmoduleCommand and rootDir) and replace the fixed string with a computed string built from skillerSubmoduleInitTimeout (e.g., formatting the Duration or using its seconds/ms value) so the error message always reflects the real timeout value.
🤖 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.
Duplicate comments:
In `@packages/app/src/docker-git/controller-compose.ts`:
- Around line 125-134: The error message hardcodes "Timeout: 60 seconds" which
can diverge from the actual skillerSubmoduleInitTimeout; update the timeout text
to be derived from skillerSubmoduleInitTimeout/its Duration representation
instead of a literal. Locate the Effect.timeoutFail invocation that calls
controllerBootstrapError (and references skillerSubmoduleCommand and rootDir)
and replace the fixed string with a computed string built from
skillerSubmoduleInitTimeout (e.g., formatting the Duration or using its
seconds/ms value) so the error message always reflects the real timeout value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d9f56ec7-097e-40b6-a5e3-0a4af8860032
📒 Files selected for processing (8)
docker-compose.api.ymldocker-compose.ymlpackages/api/README.mdpackages/app/src/docker-git/controller-compose.tspackages/app/tests/docker-git/controller-compose.test.tspackages/app/tests/docker-git/controller-image-revision.test.tspackages/app/tests/docker-git/controller.test.tspackages/app/tests/docker-git/fixtures/command-executor.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). (5)
- GitHub Check: E2E (OpenCode)
- GitHub Check: E2E (Runtime volumes + SSH)
- GitHub Check: E2E (Clone auto-open SSH)
- GitHub Check: E2E (Clone cache)
- GitHub Check: E2E (Login context)
🧰 Additional context used
📓 Path-based instructions (12)
**/*.{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:
docker-compose.ymlpackages/app/tests/docker-git/controller.test.tspackages/app/tests/docker-git/controller-compose.test.tspackages/app/tests/docker-git/fixtures/command-executor.tsdocker-compose.api.ymlpackages/app/src/docker-git/controller-compose.tspackages/app/tests/docker-git/controller-image-revision.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:
docker-compose.ymldocker-compose.api.yml
docker-compose*.yml
📄 CodeRabbit inference engine (README.md)
docker-compose*.yml: Per-project containers should ship with a default CPU limit of 30% and RAM limit of 30% (resolved against the host), overridable via --cpu / --ram flags or per-project docker-git.json
GPU mode should default to 'none' for non-GPU projects; GPU is enabled only explicitly via --gpu all or saved 'gpu': 'all' in docker-git.json
Files:
docker-compose.ymldocker-compose.api.yml
**/*
⚙️ CodeRabbit configuration file
**/*: Ты строгий ревьюер SPEC DRIVEN DEVELOPMENT.Перед выводами изучи README.md, другие *.md файлы, linked issues,
PR description, PR comments/discussion и релевантную кодовую базу.Сверь изменения с исходным ТЗ/спекой и обсуждением. Флагай любой уход
от спеки, недокументированное изменение поведения, отсутствие тестов
для заявленного поведения и security-риск. Если спека не видна,
попроси автора добавить ее в issue или PR description.Проверь решение с точки зрения формальной верификации: какие инварианты,
предусловия и постусловия можно доказать математически, а где доказуемость
слабая. Оцени решение с точки зрения теории игр: устойчивы ли стимулы,
нет ли выгодного обхода правил, и какое решение было бы сильнее.
Files:
docker-compose.ymlpackages/app/tests/docker-git/controller.test.tspackages/app/tests/docker-git/controller-compose.test.tspackages/api/README.mdpackages/app/tests/docker-git/fixtures/command-executor.tsdocker-compose.api.ymlpackages/app/src/docker-git/controller-compose.tspackages/app/tests/docker-git/controller-image-revision.test.ts
**/*.{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.test.tspackages/app/tests/docker-git/controller-compose.test.tspackages/app/tests/docker-git/fixtures/command-executor.tspackages/app/src/docker-git/controller-compose.tspackages/app/tests/docker-git/controller-image-revision.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.test.tspackages/app/tests/docker-git/controller-compose.test.tspackages/app/tests/docker-git/controller-image-revision.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.test.tspackages/app/tests/docker-git/controller-compose.test.tspackages/app/tests/docker-git/fixtures/command-executor.tspackages/app/src/docker-git/controller-compose.tspackages/app/tests/docker-git/controller-image-revision.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.test.tspackages/app/tests/docker-git/controller-compose.test.tspackages/app/tests/docker-git/controller-image-revision.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.test.tspackages/app/tests/docker-git/controller-compose.test.tspackages/app/tests/docker-git/fixtures/command-executor.tspackages/app/src/docker-git/controller-compose.tspackages/app/tests/docker-git/controller-image-revision.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.test.tspackages/app/tests/docker-git/controller-compose.test.tspackages/app/tests/docker-git/fixtures/command-executor.tspackages/app/src/docker-git/controller-compose.tspackages/app/tests/docker-git/controller-image-revision.test.ts
packages/app/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (README.md)
App layer (APP) should work only with API and not have direct access to LIB layer
Files:
packages/app/tests/docker-git/controller.test.tspackages/app/tests/docker-git/controller-compose.test.tspackages/app/tests/docker-git/fixtures/command-executor.tspackages/app/src/docker-git/controller-compose.tspackages/app/tests/docker-git/controller-image-revision.test.ts
docker-compose.api.yml
📄 CodeRabbit inference engine (README.md)
Controller container (docker-git-api) should be capped with default CPU/RAM at 90% of host resources with memory swap defaulting to twice the resolved RAM limit, overridable via CLI flags or environment variables
Files:
docker-compose.api.yml
🧠 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-compose.ts
🔇 Additional comments (8)
docker-compose.api.yml (1)
40-40: LGTM!docker-compose.yml (1)
40-40: LGTM!packages/api/README.md (1)
14-25: LGTM!Also applies to: 71-73
packages/app/src/docker-git/controller-compose.ts (1)
5-5: LGTM!Also applies to: 100-100, 156-183, 226-283
packages/app/tests/docker-git/controller-compose.test.ts (1)
1-206: LGTM!packages/app/tests/docker-git/controller-image-revision.test.ts (1)
4-14: LGTM!packages/app/tests/docker-git/controller.test.ts (1)
51-55: LGTM!Also applies to: 228-239
packages/app/tests/docker-git/fixtures/command-executor.ts (1)
1-68: LGTM!
rikohomeless
force-pushed
the
codex/controller-host-runtime-skiller
branch
from
a2eacf4 to
faf0058
Compare
1 participant
Summary
/var/run/docker.sockin compose..gitmodulesandthird_party/skiller-desktop-skills-managerin controller revision inputs.Mathematical guarantees
Invariants
missing(skiller/package.json) -> submoduleInit(root) -> exists(skiller/package.json) or ControllerBootstrapErrorcontrollerRevision = hash(compose, api, lib, skillerInputs)changes when Skiller controller sources change.runtime=host -> dockerHost=/var/run/docker.sockfor default controller startup.Preconditions
rootDiris the docker-git repository root.gitis available in the shell boundary used byCommandExecutor.Postconditions
Complexity
O(1).O(git submodule update).Verification
bun run checkpassed.bun run lintpassed.bun run --cwd packages/app vitest run tests/docker-git/controller.test.tspassed: 8 tests.bun run testwas attempted and failed on Windows-specific environment issues unrelated to this diff: missing/usr/bin/which, symlinkEPERM, and path separator expectations in existing tests.SOURCE: n/a