[codex] align API and session-sync compliance boundaries#420
Conversation
- add API and session-sync lint:effect gates and wire them into the root script - decode session-sync boundary payloads with schemas and run the CLI entrypoint through Effect NodeRuntime - move API agent snapshot persistence and terminal persistence queue toward Effect/platform primitives - keep command builder core pure by passing clonedOnHostname explicitly Proof of fix: - rtk bun run lint - rtk bun run typecheck - rtk bun run --cwd packages/api typecheck - rtk bun run --cwd packages/api lint - rtk bun run test - rtk bun run --cwd packages/api test - rtk bun run lint:effect
📝 WalkthroughWalkthroughPR добавляет опциональное поле ChangesПробрасывание поля clonedOnHostname
Миграция на Effect-TS
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related 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 |
There was a problem hiding this comment.
Actionable comments posted: 24
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/lib/src/core/command-builders.ts (1)
5-12: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winДобавьте комментарий об улучшении чистоты функции.
Изменение импортов (удаление
hostnameизnode:os) и переключение на явный параметрraw.clonedOnHostnameулучшает чистоту функцииbuildCreateCommand, убирая системный side-effect. Это важное архитектурное улучшение, которое следует задокументировать.📝 Рекомендуемый комментарий
import { expandContainerHome, nonEmpty, parseDockerNetworkMode, parseGpuMode, parseSshPort, parseSshUser, trimTrailingPathSeparators } from "./command-builders-shared.js" + +// CHANGE: Removed hostname() import from node:os +// WHY: Eliminate system side-effect; use explicit raw.clonedOnHostname parameter for FUNCTIONAL CORE purity +// QUOTE(ТЗ): "CORE: Исключительно чистые функции, неизменяемые данные, математические операции" +// REF: clonedOnHostname-purity-improvement +// SOURCE: n/a +// FORMAT THEOREM: ∀raw: build(raw) → deterministic(command) without OS calls +// PURITY: CORE (side-effect removed) +// INVARIANT: clonedOnHostname is now an explicit input, not computed from OS +// COMPLEXITY: O(1)Based on coding guidelines: "FUNCTIONAL CORE: Write only pure functions with immutable data and mathematical operations in core modules; no side effects, mutations, or external service calls"
🤖 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/lib/src/core/command-builders.ts` around lines 5 - 12, Add a documentation comment in the buildCreateCommand function explaining that the removal of the hostname import from node:os and the adoption of an explicit raw.clonedOnHostname parameter improves function purity by eliminating system side-effects. This change aligns with the functional core principle of using pure functions with no side effects in core modules, and should be documented to clarify this architectural decision for future maintainers.Source: Coding guidelines
packages/docker-git-session-sync/src/backup.ts (1)
590-596:⚠️ Potential issue | 🟠 Major | ⚡ Quick winИспользование
try/catchнарушает Effect-TS compliance.По guidelines и ESLint-конфигу (eslint.effect-ts-check.config.mjs, добавленной в этом же PR),
try/catchзапрещён в продукт-коде. Все операции с исключениями должны идти черезEffect.tryилиEffect.tryPromise.Текущий код:
try { return decodeBackgroundReadyState(JSON.parse(fs.readFileSync(readyFilePath, "utf8"))) } catch { return null }Рекомендуемый фикс с Effect:
import { Effect } from "effect" import { FileSystem } from "`@effect/platform`" const readBackgroundReadyState = (readyFilePath: string): Effect.Effect<BackgroundReadyState | null, never> => pipe( FileSystem.FileSystem, Effect.flatMap((fs) => fs.readFileString(readyFilePath)), Effect.flatMap((content) => Effect.try({ try: () => JSON.parse(content), catch: () => new Error("Invalid JSON") }) ), Effect.map(decodeBackgroundReadyState), Effect.catchAll(() => Effect.succeed(null)) )Либо, если нужно сохранить синхронность для CLI boundary (что допустимо), обернуть в Effect.try:
const readBackgroundReadyState = (readyFilePath: string): BackgroundReadyState | null => pipe( Effect.try({ try: () => decodeBackgroundReadyState(JSON.parse(fs.readFileSync(readyFilePath, "utf8"))), catch: () => null as const }), Effect.runSync )Однако второй вариант всё ещё не идеален, так как
runSyncдолжен быть только на самом верхнем уровне (в main.ts). Предпочтителен первый вариант с Effect-based возвратом.🤖 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/docker-git-session-sync/src/backup.ts` around lines 590 - 596, The function readBackgroundReadyState uses a raw try/catch block which violates Effect-TS compliance rules (as defined in eslint.effect-ts-check.config.mjs). Replace the try/catch block by wrapping the entire operation (the fs.readFileSync and JSON.parse call to decodeBackgroundReadyState) inside an Effect.try call. Import Effect from the "effect" module and use Effect.try with a try property containing the original logic and a catch property returning an appropriate error. Chain this with Effect.catchAll to handle any errors by returning Effect.succeed(null), and adjust the function signature to return Effect.Effect<BackgroundReadyState | null, never> instead of BackgroundReadyState | null. This ensures all exception handling follows Effect-TS patterns rather than raw try/catch.Source: Coding guidelines
packages/docker-git-session-sync/src/shell.ts (1)
541-548: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winРассмотрите усиление контракта decoder для обязательных полей.
decodeGitHubShaвозвращаетnullпри отсутствии поляsha, что требует explicit проверки в каждом call site. Еслиshaявляется обязательным полем для всех этих API responses (blob, tree, commit), decoder должен возвращатьEither<ParseError, string>или throw вместоstring | null, чтобы гарантировать type-level наличие sha после успешного декодирования.Текущая логика корректна, но type safety можно улучшить.
🤖 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/docker-git-session-sync/src/shell.ts` around lines 541 - 548, The `decodeGitHubSha` function currently returns `string | null`, which requires explicit null checking at every call site as shown in the code around line 541-545. Since `sha` is a mandatory field in GitHub API responses, strengthen the type contract by modifying `decodeGitHubSha` to throw an error directly when the sha field is missing instead of returning null. This way the function will either return a non-null string or throw, eliminating the need for the null check at line 542-544 and providing type-level guarantees that if the function returns successfully, sha is always present.
🤖 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/api/contracts.ts`:
- Line 483: The clonedOnHostname field is declared as optional using the `?:`
operator but also explicitly includes `| undefined` in the type annotation,
which is redundant since optional fields already have an implicit undefined
type. Remove the explicit `| undefined` from the type definition and keep only
`string`, so the field type becomes simply `readonly clonedOnHostname?: string`
instead of `readonly clonedOnHostname?: string | undefined`.
- Line 483: Add comprehensive documentation to the clonedOnHostname field in the
contracts file. Include a TSDoc comment block above the field that explains its
purpose with mathematical annotations if applicable, and add functional comments
using the CHANGE/WHY/QUOTE/REF/SOURCE format to document why this field was
added, what problem it solves, and any relevant references or sources. Ensure
the documentation follows your project's coding guidelines for new field
additions.
- Line 483: The clonedOnHostname field in the contracts accepts arbitrary
strings without validation, creating a potential security risk when interpolated
in logging and UI output. Add hostname validation by creating a HostnameSchema
that enforces length constraints (minimum 1, maximum 253 characters) and a
hostname pattern that allows only alphanumeric characters and hyphens in the
appropriate positions. Apply this HostnameSchema to the clonedOnHostname field
definition using Schema.optional wrapper. Make these changes in both
packages/api/src/api/schema.ts and packages/lib/src/shell/config.ts to ensure
consistent validation across the codebase.
In `@packages/api/src/api/schema.ts`:
- Around line 54-56: The clonedOnHostname field in the schema is defined with
OptionalString without any validation constraints, making it accept invalid
hostname values. Add a pattern validation to the clonedOnHostname field
definition that enforces RFC 1123 compliant hostname format. This pattern should
validate that the hostname contains only alphanumeric characters, hyphens, and
dots, following the standard hostname naming rules. Update the clonedOnHostname
field definition to include this pattern constraint alongside the OptionalString
type.
In `@packages/api/src/services/agents.ts`:
- Around line 49-58: The clearAgentRuntimeForTest function is synchronous but
does not wait for processes to actually terminate after sending SIGTERM signals,
which can cause race conditions in rapid test execution. Convert
clearAgentRuntimeForTest to an async function and add proper waiting for process
termination before clearing the records and projectIndex collections. Use
appropriate async waiting mechanisms (such as listening to process exit events
or using a timeout-based approach) to ensure processes are fully cleaned up
before the function resolves.
- Around line 389-396: The AgentRecord object being created has an empty
projectDir field which could cause data integrity issues if other code relies on
this field. Add a comment or TODO above the AgentRecord initialization
explaining that projectDir is intentionally left empty in hydrated records
because it needs to be passed separately as a parameter to functions like
stopAgent, clarifying this limitation for future maintainers.
- Around line 38-40: The SnapshotFileSchema definition uses Schema.Array which
infers a mutable Array type, but the SnapshotFile type on line 35 declares
sessions as readonly sessions: ReadonlyArray<AgentSession>. To fix this mismatch
and maintain consistency with the ReadonlyArray coding guidelines, replace the
Schema.Array call in the SnapshotFileSchema with the appropriate Schema
construct that produces a ReadonlyArray type instead of a mutable Array type,
ensuring the schema definition matches the explicit type declaration of
SnapshotFile.
- Around line 229-234: The persistSnapshotBestEffort function currently
suppresses all errors silently without any logging, which makes it difficult to
diagnose persistence issues. Modify the Effect.catchAll handler in the
persistSnapshot pipe to add error logging using Effect.logWarning before
returning Effect.void, so that any persistence failures are captured in logs
while maintaining the best-effort semantics.
- Around line 362-374: The error handling in the snapshot file loading logic is
overly complex, using Effect.either wrapped around fs.exists and
fs.readFileString calls followed by Either.match patterns. Instead of this
pattern, replace both instances with Effect.orElse or Effect.catchAll to handle
errors directly on the Effect level. For the fs.exists call, use Effect.orElse
to return false on any error, and for the fs.readFileString call, use
Effect.orElse to call emptySnapshotFile() on any error. This will eliminate the
intermediate Either conversion and matching, making the code more readable and
aligned with Effect-based error handling practices.
In `@packages/api/src/services/terminal-sessions.ts`:
- Around line 392-435: The issue is that the
drainTerminalSessionPersistenceQueue function sequences persistence effects for
a project, but other functions (registerRecord, deleteTerminalSession,
setProjectActiveTerminalSession) write directly to the file state outside this
queue, causing race conditions where queued reads can see stale data if direct
writes interleave with queued operations. To fix this, implement a per-project
sequencer or lock mechanism that routes all state write operations (from
drainTerminalSessionPersistenceQueue, registerRecord, deleteTerminalSession, and
setProjectActiveTerminalSession) through a single serialized path, ensuring
foreground operations return an Effect that completes only after the actual
write finishes. Additionally, add a regression test that forces interleaving of
queued reads/writes with direct writes to verify that delete or set-active
changes are not lost by subsequent queued writes; the test must fail before the
fix and pass after.
In `@packages/docker-git-session-sync/eslint.effect-ts-check.config.mjs`:
- Around line 15-40: The effectMigrationWarnings array is duplicated across
multiple ESLint configuration files in different packages, which can lead to
inconsistency when the rules are updated. Extract the effectMigrationWarnings
array into a shared configuration file or module that can be imported by both
packages/docker-git-session-sync/eslint.effect-ts-check.config.mjs and
packages/api/eslint.effect-ts-check.config.mjs. Replace the duplicated array
definitions with imports of this shared configuration to ensure consistency
across all packages.
- Around line 61-70: The no-restricted-types rule for Promise in the
configuration has a mismatch between its severity level and message. The rule is
set to "error" severity with a comment about catching unsafe bypasses, but the
message text "Avoid Promise in public types. Use Effect.Effect<A, E, R>." is
identical to the API config message and doesn't clarify whether Promise is truly
an unsafe bypass or just a migration blocker. Determine whether Promise should
be classified as an unsafe bypass (keep error severity but update message for
clarity) or as a migration blocker (change severity to warn to match the API
configuration pattern). Update either the severity level for the types object
containing Promise and Promise<*>, or update the message text, whichever
correctly reflects the intended classification.
In `@packages/docker-git-session-sync/src/backup.ts`:
- Around line 356-357: The parseUploadContext function is a simple one-line
wrapper that directly delegates to decodeSessionUploadContext. Review whether
parseUploadContext is exported from the module or used only internally. If it is
internal-only, remove the parseUploadContext function and replace its usage
(such as at the call site mentioned) with direct calls to
decodeSessionUploadContext. If parseUploadContext must be retained for backward
compatibility or public API stability reasons, add a comment above the function
explaining this intent (e.g., "maintained for backward compatibility" or "public
API stability").
In `@packages/docker-git-session-sync/src/main.ts`:
- Around line 7-37: The EFFECT comment documenting the type of the main variable
is inaccurate after the Effect.provide(NodeContext.layer) call at the end of the
pipe chain. The comment currently states the type is Effect<void, never,
NodeContext>, but after provide satisfies the NodeContext dependency
requirement, the actual final type becomes Effect<void, never, never>. Update
the EFFECT comment to accurately document the final Effect type after the
provide operation completes the type chain, either by documenting just the final
type or by showing both the intermediate type before provide and the final type
after provide is applied.
In `@packages/docker-git-session-sync/src/schemas.ts`:
- Around line 85-87: The GitHubTreeResponseSchema definition uses
Schema.Array(Schema.Unknown) which violates the boundary principle by allowing
untyped data to escape the decoding module. Replace Schema.Unknown with
TreeEntrySchema in the tree field definition to properly validate and type the
array elements at the schema level. This will ensure that only properly decoded
tree entries leave the decoding boundary, making the validation logic cleaner
and more maintainable than the current approach where decodeGitHubTreeEntries
individually decodes and filters elements.
- Around line 147-155: The decodeGitHubTreeEntries function uses a mutable Array
type for response.tree when the function signature declares a ReadonlyArray
return type, creating a type consistency issue. Either update the
GitHubTreeResponseSchema definition to explicitly define the tree property as a
ReadonlyArray type, or add an explicit type cast to ReadonlyArray<unknown> after
the decoding operation in the onRight callback to align the inferred Array type
from the schema with the ReadonlyArray requirement specified in the function's
return type annotation.
- Around line 99-155: Add comprehensive TSDoc comments to all exported decoding
functions that are currently missing documentation. For each of
decodeBackgroundReadyState, decodeGitHubRepoInfo, decodeGitHubPrComment,
decodeGitHubContentResponse, decodeGitHubSha, and decodeGitHubTreeEntries, add a
TSDoc block directly above the function definition that includes a description
of what the function decodes, `@param` annotation for the input value, `@returns`
annotation for the output type, `@pure` false annotation since these are boundary
decoding operations, `@effect` none annotation since they are synchronous
operations, `@invariant` annotation describing the decode contract, and
`@complexity` O(1)/O(1) annotation for time and space complexity.
- Around line 99-155: The decode functions in this file
(decodeSessionUploadContext, decodeBackgroundReadyState, decodeGitHubRepoInfo,
decodeGitHubPrComment, decodeGitHubContentResponse, decodeGitHubSha, and
decodeGitHubTreeEntries) currently return synchronous `Type | null` values, but
should be migrated to use the Effect monad for consistency with the rest of the
codebase and to enable better error composition. Change each function's return
type from `Type | null` to `Effect.Effect<Type, DecodeError>`, and refactor the
implementation to use `Effect.try()` with `ParseResult.decodeUnknownSync()` to
handle the decoding logic and catch errors by wrapping them in a DecodeError.
This allows downstream code to compose decoding operations through Effect's pipe
and flatMap utilities with proper typed error handling.
In `@packages/docker-git-session-sync/src/shell.ts`:
- Line 243: The current return statement at line 243 in the shell.ts file does
not distinguish between API failures (when result.success is false) and parsing
failures (when decodeGitHubPrComment fails or throws an error). Modify the logic
to handle these two failure scenarios separately: first check if result.success
is false and handle the API error case, then wrap the decodeGitHubPrComment call
in a try-catch block to handle parsing errors independently. Either return
different error types/reasons (such as "api_error" vs "parse_error") or add
separate logging for parse failures to improve observability and debugging
capabilities.
- Around line 184-185: The code uses fallback values for defaultBranch and
htmlUrl through the nullish coalescing operator without logging when these
fallbacks are actually used. Add logging logic before the return statement to
detect and log when repoInfo.defaultBranch or repoInfo.htmlUrl are missing or
undefined, so that you can track when the GitHub API returns incomplete data.
Check if these values are missing and if so, log an informational message
indicating which fallback value is being used to improve visibility into
potential API issues or configuration problems.
- Around line 227-229: The encoding validation should be moved from the runtime
check at the if statement into the GitHubContentResponseSchema definition by
using S.literal("base64") for the encoding field instead of validating it
manually in the conditional. Additionally, the content.length === 0 check
conflates two different scenarios: an invalid response versus a validly decoded
empty file. Determine whether empty files should be considered valid; if they
are, remove the content.length === 0 condition from the if statement and only
validate the encoding through the schema. If empty files are genuinely invalid,
clarify the error message to distinguish between the two error cases.
- Line 211: The decodeGitHubTreeEntries function silently returns an empty array
on parsing failure, which can cause silent data loss by creating empty snapshots
instead of failing explicitly. After calling decodeGitHubTreeEntries with
result.json, add a validation check to verify the returned entries array is not
empty (or handle the error state appropriately). If entries are empty, throw an
error with context about the failed parsing instead of allowing the empty array
to propagate. Alternatively, refactor the decodeGitHubTreeEntries decoder to
return a Result type (success or ParseError) instead of falling back to an empty
array, so callers can explicitly handle parsing failures.
In `@packages/docker-git-session-sync/tests/session-files.test.ts`:
- Around line 333-356: The test "rejects malformed background upload context
metadata" currently uses a single hardcoded example rather than property-based
testing. Refactor this test to use fast-check (fc.property) to generate multiple
valid upload context objects and verify mathematical invariants such as: for all
valid contexts, parseUploadContext returns a non-null result and the decoded
version equals 1. Additionally, if parseUploadContext can be migrated to use
Effect, convert the test to use Effect test utilities instead of relying on
synchronous calls, following the guideline that unit tests must use Effect test
utilities without async/await.
In `@packages/lib/src/core/command-options.ts`:
- Line 62: The clonedOnHostname field in the command options interface lacks an
inline comment explaining its purpose. Add a brief comment above the
clonedOnHostname field that describes what this optional string property
represents and why it is used, improving code clarity and maintainability for
future developers working with this interface.
---
Outside diff comments:
In `@packages/docker-git-session-sync/src/backup.ts`:
- Around line 590-596: The function readBackgroundReadyState uses a raw
try/catch block which violates Effect-TS compliance rules (as defined in
eslint.effect-ts-check.config.mjs). Replace the try/catch block by wrapping the
entire operation (the fs.readFileSync and JSON.parse call to
decodeBackgroundReadyState) inside an Effect.try call. Import Effect from the
"effect" module and use Effect.try with a try property containing the original
logic and a catch property returning an appropriate error. Chain this with
Effect.catchAll to handle any errors by returning Effect.succeed(null), and
adjust the function signature to return Effect.Effect<BackgroundReadyState |
null, never> instead of BackgroundReadyState | null. This ensures all exception
handling follows Effect-TS patterns rather than raw try/catch.
In `@packages/docker-git-session-sync/src/shell.ts`:
- Around line 541-548: The `decodeGitHubSha` function currently returns `string
| null`, which requires explicit null checking at every call site as shown in
the code around line 541-545. Since `sha` is a mandatory field in GitHub API
responses, strengthen the type contract by modifying `decodeGitHubSha` to throw
an error directly when the sha field is missing instead of returning null. This
way the function will either return a non-null string or throw, eliminating the
need for the null check at line 542-544 and providing type-level guarantees that
if the function returns successfully, sha is always present.
In `@packages/lib/src/core/command-builders.ts`:
- Around line 5-12: Add a documentation comment in the buildCreateCommand
function explaining that the removal of the hostname import from node:os and the
adoption of an explicit raw.clonedOnHostname parameter improves function purity
by eliminating system side-effects. This change aligns with the functional core
principle of using pure functions with no side effects in core modules, and
should be documented to clarify this architectural decision for future
maintainers.
🪄 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: b15ebb4f-beae-4486-a739-cc26fe4ab837
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (25)
package.jsonpackages/api/eslint.effect-ts-check.config.mjspackages/api/package.jsonpackages/api/src/api/contracts.tspackages/api/src/api/schema.tspackages/api/src/services/agents.tspackages/api/src/services/federation.tspackages/api/src/services/projects.tspackages/api/src/services/terminal-sessions.tspackages/api/tests/agents.test.tspackages/app/src/docker-git/frontend-lib/core/command-builders-shared.tspackages/app/src/docker-git/frontend-lib/core/command-builders.tspackages/app/src/docker-git/frontend-lib/core/command-options.tspackages/app/src/docker-git/frontend-lib/usecases/scrap-path.tspackages/docker-git-session-sync/eslint.effect-ts-check.config.mjspackages/docker-git-session-sync/package.jsonpackages/docker-git-session-sync/src/backup.tspackages/docker-git-session-sync/src/main.tspackages/docker-git-session-sync/src/schemas.tspackages/docker-git-session-sync/src/shell.tspackages/docker-git-session-sync/tests/session-files.test.tspackages/lib/src/core/command-builders-shared.tspackages/lib/src/core/command-builders.tspackages/lib/src/core/command-options.tspackages/lib/src/usecases/scrap-path.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 (Login context)
- GitHub Check: Lint
- GitHub Check: Test
- GitHub Check: E2E (Runtime volumes + SSH)
- GitHub Check: E2E (Clone auto-open SSH)
- GitHub Check: E2E (Clone cache)
- GitHub Check: E2E (OpenCode)
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{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/lib/src/core/command-options.tspackages/docker-git-session-sync/src/main.tspackages/app/src/docker-git/frontend-lib/core/command-options.tspackages/docker-git-session-sync/tests/session-files.test.tspackages/lib/src/core/command-builders-shared.tspackages/app/src/docker-git/frontend-lib/core/command-builders-shared.tspackages/api/src/services/federation.tspackages/lib/src/core/command-builders.tspackages/api/src/services/projects.tspackages/api/src/api/contracts.tspackages/lib/src/usecases/scrap-path.tspackages/app/src/docker-git/frontend-lib/core/command-builders.tspackages/app/src/docker-git/frontend-lib/usecases/scrap-path.tspackages/api/src/api/schema.tspackages/api/src/services/terminal-sessions.tspackages/api/tests/agents.test.tspackages/api/src/services/agents.tspackages/docker-git-session-sync/src/backup.tspackages/docker-git-session-sync/src/schemas.tspackages/docker-git-session-sync/src/shell.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/lib/src/core/command-options.tspackages/docker-git-session-sync/src/main.tspackages/app/src/docker-git/frontend-lib/core/command-options.tspackages/docker-git-session-sync/tests/session-files.test.tspackages/lib/src/core/command-builders-shared.tspackages/app/src/docker-git/frontend-lib/core/command-builders-shared.tspackages/api/src/services/federation.tspackages/lib/src/core/command-builders.tspackages/api/src/services/projects.tspackages/api/src/api/contracts.tspackages/lib/src/usecases/scrap-path.tspackages/app/src/docker-git/frontend-lib/core/command-builders.tspackages/app/src/docker-git/frontend-lib/usecases/scrap-path.tspackages/api/src/api/schema.tspackages/api/src/services/terminal-sessions.tspackages/api/tests/agents.test.tspackages/api/src/services/agents.tspackages/docker-git-session-sync/src/backup.tspackages/docker-git-session-sync/src/schemas.tspackages/docker-git-session-sync/src/shell.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/lib/src/core/command-options.tspackages/docker-git-session-sync/src/main.tspackages/app/src/docker-git/frontend-lib/core/command-options.tspackage.jsonpackages/docker-git-session-sync/tests/session-files.test.tspackages/lib/src/core/command-builders-shared.tspackages/app/src/docker-git/frontend-lib/core/command-builders-shared.tspackages/api/src/services/federation.tspackages/docker-git-session-sync/package.jsonpackages/lib/src/core/command-builders.tspackages/api/src/services/projects.tspackages/api/src/api/contracts.tspackages/api/package.jsonpackages/lib/src/usecases/scrap-path.tspackages/app/src/docker-git/frontend-lib/core/command-builders.tspackages/app/src/docker-git/frontend-lib/usecases/scrap-path.tspackages/api/src/api/schema.tspackages/api/src/services/terminal-sessions.tspackages/api/tests/agents.test.tspackages/api/src/services/agents.tspackages/docker-git-session-sync/src/backup.tspackages/docker-git-session-sync/src/schemas.tspackages/docker-git-session-sync/src/shell.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: FUNCTIONAL CORE: Write only pure functions with immutable data and mathematical operations in core modules; no side effects, mutations, or external service calls
IMPERATIVE SHELL: Isolate all side effects (IO, network, database, environment/process) in a thin SHELL layer; CORE never calls SHELL, only SHELL → CORE
Never useanytype annotation in TypeScript; useunknownonly at SHELL boundaries for decoding, never exportunknownoutside boundary modules
Never useastype assertions in normal code; only permitasin a single 'axiomatic' module (brands, constructors, constants) after which types flow safely without casts
Always use exhaustive pattern matching for union types through.exhaustive()orMatch.exhaustive()from effect-ts; never use switch statements or unhandled type branches
Use Effect<Success, Error, Requirements> monad from effect-ts for all effects; compose through pipe() and Effect.flatMap(); never use async/await, raw Promise chains (then/catch), or Promise.all in product code
Interoperate with Promise/exceptions only in SHELL through Effect.try/Effect.tryPromise with typed error mapping; never leave raw exceptions or untyped errors in the domain
Use Effect.acquireRelease + Effect.scoped for resource management with guaranteed finalization; never manage resources with try/finally or manual cleanup
All external services (database, HTTP, environment) must be accessed through Effect-based interfaces and Layer-based dependency injection; never call external APIs directly
Provide comprehensive TSDoc comments with mathematical notation:@pure,@effect,@invariant,@precondition,@postcondition,@complexity,@throws, and CHANGE/WHY/REF/SOURCE/FORMAT THEOREM functional comment markers
No console.*, process direct calls, or untyped environment access in product code; all such operations must be abstracted through Layer-based services in SHELL
Boundary data from external sources (HTTP, database, environment) must be decoded/valida...
Files:
packages/lib/src/core/command-options.tspackages/docker-git-session-sync/src/main.tspackages/app/src/docker-git/frontend-lib/core/command-options.tspackages/docker-git-session-sync/tests/session-files.test.tspackages/lib/src/core/command-builders-shared.tspackages/app/src/docker-git/frontend-lib/core/command-builders-shared.tspackages/api/src/services/federation.tspackages/lib/src/core/command-builders.tspackages/api/src/services/projects.tspackages/api/src/api/contracts.tspackages/lib/src/usecases/scrap-path.tspackages/app/src/docker-git/frontend-lib/core/command-builders.tspackages/app/src/docker-git/frontend-lib/usecases/scrap-path.tspackages/api/src/api/schema.tspackages/api/src/services/terminal-sessions.tspackages/api/tests/agents.test.tspackages/api/src/services/agents.tspackages/docker-git-session-sync/src/backup.tspackages/docker-git-session-sync/src/schemas.tspackages/docker-git-session-sync/src/shell.ts
**/{browser*,server*,app*,*.ts,*.js}
📄 CodeRabbit inference engine (README.md)
Web version must listen on 0.0.0.0 by default for accessibility across LAN devices
Files:
packages/lib/src/core/command-options.tspackages/docker-git-session-sync/src/main.tspackages/app/src/docker-git/frontend-lib/core/command-options.tspackages/docker-git-session-sync/tests/session-files.test.tspackages/lib/src/core/command-builders-shared.tspackages/app/src/docker-git/frontend-lib/core/command-builders-shared.tspackages/api/src/services/federation.tspackages/lib/src/core/command-builders.tspackages/api/src/services/projects.tspackages/api/src/api/contracts.tspackages/lib/src/usecases/scrap-path.tspackages/app/src/docker-git/frontend-lib/core/command-builders.tspackages/app/src/docker-git/frontend-lib/usecases/scrap-path.tspackages/api/src/api/schema.tspackages/api/src/services/terminal-sessions.tspackages/api/tests/agents.test.tspackages/api/src/services/agents.tspackages/docker-git-session-sync/src/backup.tspackages/docker-git-session-sync/src/schemas.tspackages/docker-git-session-sync/src/shell.ts
**/{cli*,command*,auto*,*.ts,*.tsx}
📄 CodeRabbit inference engine (README.md)
Implement auto-mode agent selection logic to choose Claude, Codex, Gemini, or Grok randomly from available authorized providers, or allow forced selection with --auto=
Files:
packages/lib/src/core/command-options.tspackages/docker-git-session-sync/src/main.tspackages/app/src/docker-git/frontend-lib/core/command-options.tspackages/docker-git-session-sync/tests/session-files.test.tspackages/lib/src/core/command-builders-shared.tspackages/app/src/docker-git/frontend-lib/core/command-builders-shared.tspackages/api/src/services/federation.tspackages/lib/src/core/command-builders.tspackages/api/src/services/projects.tspackages/api/src/api/contracts.tspackages/lib/src/usecases/scrap-path.tspackages/app/src/docker-git/frontend-lib/core/command-builders.tspackages/app/src/docker-git/frontend-lib/usecases/scrap-path.tspackages/api/src/api/schema.tspackages/api/src/services/terminal-sessions.tspackages/api/tests/agents.test.tspackages/api/src/services/agents.tspackages/docker-git-session-sync/src/backup.tspackages/docker-git-session-sync/src/schemas.tspackages/docker-git-session-sync/src/shell.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Ты строгий ревьюер SPEC DRIVEN DEVELOPMENT.Перед выводами изучи README.md, другие *.md файлы, linked issues,
PR description, PR comments/discussion и релевантную кодовую базу.Сверь изменения с исходным ТЗ/спекой и обсуждением. Флагай любой уход
от спеки, недокументированное изменение поведения, отсутствие тестов
для заявленного поведения и security-риск. Если спека не видна,
попроси автора добавить ее в issue или PR description.Проверь решение с точки зрения формальной верификации: какие инварианты,
предусловия и постусловия можно доказать математически, а где доказуемость
слабая. Оцени решение с точки зрения теории игр: устойчивы ли стимулы,
нет ли выгодного обхода правил, и какое решение было бы сильнее.
Files:
packages/lib/src/core/command-options.tspackages/docker-git-session-sync/src/main.tspackages/app/src/docker-git/frontend-lib/core/command-options.tspackage.jsonpackages/docker-git-session-sync/tests/session-files.test.tspackages/lib/src/core/command-builders-shared.tspackages/docker-git-session-sync/eslint.effect-ts-check.config.mjspackages/app/src/docker-git/frontend-lib/core/command-builders-shared.tspackages/api/src/services/federation.tspackages/docker-git-session-sync/package.jsonpackages/lib/src/core/command-builders.tspackages/api/src/services/projects.tspackages/api/src/api/contracts.tspackages/api/package.jsonpackages/lib/src/usecases/scrap-path.tspackages/app/src/docker-git/frontend-lib/core/command-builders.tspackages/api/eslint.effect-ts-check.config.mjspackages/app/src/docker-git/frontend-lib/usecases/scrap-path.tspackages/api/src/api/schema.tspackages/api/src/services/terminal-sessions.tspackages/api/tests/agents.test.tspackages/api/src/services/agents.tspackages/docker-git-session-sync/src/backup.tspackages/docker-git-session-sync/src/schemas.tspackages/docker-git-session-sync/src/shell.ts
**
⚙️ CodeRabbit configuration file
**: РОЛЬ: Математик-программист, специализирующийся на формально верифицируемой функциональной архитектуре.ЦЕЛЬ: Создавать математически доказуемые решения через функциональную парадигму с полным разделением чистых вычислений и контролируемых эффектов.
МОДЕЛЬ РАССУЖДЕНИЯ:
- Не выдавать “личные мнения”. Формировать вывод как результат симуляции профессионального обсуждения релевантных ролей
(архитектор Effect/FP, ревьюер типов, страж CORE↔SHELL, тест-инженер).- Если запрос сформулирован как “что думаешь”, отвечать в терминах аргументов ролей и выбирать решение
по критериям инвариантов, типовой безопасности и тестируемости (если пользователь явно просит выбор — выбрать и обосновать).ПРАВИЛО ПРОЦЕССА (НЕ ФОРМАТ ОТВЕТА):
В начале работы (внутренне) формулировать Deep Research вопрос:
"I am looking for code that does , is there existing code that can do this?"
Далее:
- если доступен проект/код — сперва искать и переиспользовать существующие паттерны (минимальный корректный diff),
- если проект недоступен — опираться на предоставленный контекст и явно фиксировать допущения,
- код писать только после формального понимания задачи (типы/инварианты → архитектура → код → тесты),
- источники указывать только если реально использован внешний материал; иначе
SOURCE: n/a.ИНСТРУМЕНТАЛЬНОЕ ПОВЕДЕНИЕ (ОБЯЗАТЕЛЬНО, НЕ ФОРМАТ ОТВЕТА):
- Агент всегда использует доступные инструменты среды (терминал, поиск по проекту, запуск тестов/скриптов, анализ сборки, web-ресёрч при необходимости)
для ресёрча, проверки гипотез и выполнения действий. Приоритет: проверяемость, воспроизводимость, минимальный риск.- Агент не предлагает “гайд” как замену действия. Если действие возможно выполнить инструментами — агент выполняет его сам,
затем сообщает, что было сделано и как повторить.- Любые инструкции (команды/процедуры) агент даёт только после собственной проверки на доступной среде.
Если проверить невозможно — явно фиксирует ограничение и перечисляе...
Files:
packages/lib/src/core/command-options.tspackages/docker-git-session-sync/src/main.tspackages/app/src/docker-git/frontend-lib/core/command-options.tspackage.jsonpackages/docker-git-session-sync/tests/session-files.test.tspackages/lib/src/core/command-builders-shared.tspackages/docker-git-session-sync/eslint.effect-ts-check.config.mjspackages/app/src/docker-git/frontend-lib/core/command-builders-shared.tspackages/api/src/services/federation.tspackages/docker-git-session-sync/package.jsonpackages/lib/src/core/command-builders.tspackages/api/src/services/projects.tspackages/api/src/api/contracts.tspackages/api/package.jsonpackages/lib/src/usecases/scrap-path.tspackages/app/src/docker-git/frontend-lib/core/command-builders.tspackages/api/eslint.effect-ts-check.config.mjspackages/app/src/docker-git/frontend-lib/usecases/scrap-path.tspackages/api/src/api/schema.tspackages/api/src/services/terminal-sessions.tspackages/api/tests/agents.test.tspackages/api/src/services/agents.tspackages/docker-git-session-sync/src/backup.tspackages/docker-git-session-sync/src/schemas.tspackages/docker-git-session-sync/src/shell.ts
**/{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}
📄 CodeRabbit inference engine (Custom checks)
Fail if dependency or package-manager changes materially increase supply-chain risk without justification
Files:
package.jsonpackages/docker-git-session-sync/package.jsonpackages/api/package.json
package.json
📄 CodeRabbit inference engine (AGENTS.md)
Dependencies must include effect ^3.x and
@effect/schema^0.x; prohibit downgrading these versions or introducing incompatible alternatives (async-only libraries without Effect support)Require Effect and
@effect/schemaas mandatory dependencies for type-safe effects and validation
Files:
package.json
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.test.{ts,tsx}: Write property-based tests using fast-check (fc.property) to verify mathematical invariants; unit tests must use Effect test utilities without async/await
Every bug fix must be accompanied by a reproducing test case; the test must fail before the fix and pass after; document the Proof of Fix with root cause and solution
Files:
packages/docker-git-session-sync/tests/session-files.test.tspackages/api/tests/agents.test.ts
🧠 Learnings (2)
📚 Learning: 2026-05-22T21:08:18.083Z
Learnt from: skulidropek
Repo: ProverCoderAI/docker-git PR: 344
File: packages/app/src/docker-git/controller-compose.ts:34-40
Timestamp: 2026-05-22T21:08:18.083Z
Learning: In this repo’s docker-git controller compose generation, `${DOCKER_GIT_CONTROLLER_BUILD_SKILLER:-1}` should be treated as standard bash parameter expansion: when `DOCKER_GIT_CONTROLLER_BUILD_SKILLER` is unset, it defaults to the string "1". There is no "-1" mode. The runtime contract enforced by `packages/app/src/docker-git/controller-compose.ts` is: unset / "1" / "true" => output "1"; "0" / "false" => output "0". If review code shows branching/behavior for "-1" or any numeric value other than this 0/1 contract, flag it. Also ensure the Dockerfile ARG `DOCKER_GIT_CONTROLLER_BUILD_SKILLER` stays consistent with default `1`.
Applied to files:
packages/app/src/docker-git/frontend-lib/core/command-options.tspackages/app/src/docker-git/frontend-lib/core/command-builders-shared.tspackages/app/src/docker-git/frontend-lib/core/command-builders.tspackages/app/src/docker-git/frontend-lib/usecases/scrap-path.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/federation.tspackages/api/src/services/projects.tspackages/api/src/services/terminal-sessions.tspackages/api/src/services/agents.ts
🔇 Additional comments (29)
packages/api/src/services/federation.ts (1)
1477-1479: LGTM!Also applies to: 1905-1907
packages/api/src/services/agents.ts (1)
403-414: LGTM!packages/api/tests/agents.test.ts (3)
25-40: Корректная изоляция тестового окружения.Хороший паттерн: временная директория + манипуляция
DOCKER_GIT_PROJECTS_ROOT+clearAgentRuntimeForTest()вbeforeEach/afterEachобеспечивают чистую изоляцию между тестами. Восстановление предыдущего значения env переменной также обрабатывает случай, когда переменная была undefined.
143-179: Тест гидратации валидного снапшота — проверяет ключевые инварианты.Тест корректно верифицирует:
- Нормализация статуса:
"running"→"exited"(так как процесс не восстановим после рестарта)- Сброс
hostPid: null(процесс недоступен)- Установка
stoppedAt(фиксация времени рестарта)Использование
Effect.runPromiseсEffect.sync/Effect.zipRight/Effect.tapсоответствует Effect-паттернам тестирования.
181-193: LGTM!packages/api/src/services/terminal-sessions.ts (2)
85-90: LGTM!
113-113: LGTM!packages/app/src/docker-git/frontend-lib/core/command-options.ts (1)
63-63: LGTM!packages/app/src/docker-git/frontend-lib/core/command-builders.ts (1)
272-273: LGTM!packages/api/src/services/projects.ts (1)
485-486: МаппингclonedOnHostnameвыполнен корректно.Поле пробрасывается из API request в RawOptions консистентно с остальными полями. Паттерн условного spread'а правильный.
Однако, убедитесь что валидация hostname'а добавлена в schema (см. комментарий к packages/api/src/api/schema.ts), поскольку на уровне этого маппинга валидация отсутствует.
packages/lib/src/core/command-builders.ts (1)
272-272: Удалениеhostname()подтверждено. Код успешно заменил вызовhostname()на параметрclonedOnHostnameиз RawOptions. В обоих файлах (packages/lib и packages/app frontend-lib) строка 272 показываетclonedOnHostname: raw.clonedOnHostname, что означает чтение значения из параметра, а не вызов системной функции. Поиск по кодовой базе подтверждает отсутствиеhostname()вызовов в файлах command-builders.ts — импорт изnode:osудален, и clonedOnHostname определен как опциональное строковое свойство в RawOptions.packages/lib/src/core/command-builders-shared.ts (1)
72-94: LGTM!packages/lib/src/usecases/scrap-path.ts (1)
3-3: LGTM!Also applies to: 63-63
packages/app/src/docker-git/frontend-lib/core/command-builders-shared.ts (1)
73-95: LGTM!packages/app/src/docker-git/frontend-lib/usecases/scrap-path.ts (1)
4-4: LGTM!Also applies to: 65-65
packages/docker-git-session-sync/src/backup.ts (1)
35-39: LGTM!Also applies to: 333-333
packages/docker-git-session-sync/src/main.ts (2)
17-21: Корректное использованиеEffect.tryдля постепенной миграции на Effect.Обёртывание
runCli(синхронной функции) вEffect.tryсоответствует guidelines: «Interoperate with Promise/exceptions only in SHELL through Effect.try/Effect.tryPromise with typed error mapping».Текущий подход допустим для постепенной миграции CLI на Effect. В будущем рекомендуется мигрировать сам
runCliна Effect для полной композиции:const runCliEffect = (args: ReadonlyArray<string>, cwd: string): Effect.Effect<number, CliError> => // ... Effect-based implementationНо для текущего PR это не требуется, так как фокус — на boundary-декодировании и entrypoint-миграции.
39-39: LGTM!packages/docker-git-session-sync/package.json (2)
15-15: LGTM!Also applies to: 40-45
46-57: LGTM!packages/docker-git-session-sync/src/shell.ts (1)
18-25: LGTM!packages/api/eslint.effect-ts-check.config.mjs (3)
1-10: LGTM!
15-40: LGTM!
42-78: LGTM!packages/api/package.json (3)
18-18: LGTM!
45-45: LGTM!
55-55: LGTM!packages/docker-git-session-sync/eslint.effect-ts-check.config.mjs (1)
1-10: LGTM!package.json (1)
49-49: Скриптlint:effectв пакете@prover-coder-ai/docker-git-containerуже присутствует и корректно сконфигурирован. Проверка root команды не вызовет ошибок.> Likely an incorrect or invalid review comment.
AI Session BackupCommit: 4a475e2
|
- Validate clonedOnHostname at API/config boundaries and keep command builders pure by passing host identity explicitly. - Serialize terminal session state mutations per project to prevent stale read/write races. - Harden session-sync GitHub decoders, readiness polling, PR comment logging, and regression/property tests. Proof of fix: - Cause: review found weak schema contracts, unsynchronized durable terminal writes, silent best-effort failures, and incomplete regression tests. - Solution: moved shared Effect lint rules into one module, strengthened decoders and hostname schemas, added project-scoped semaphores around state writers, and added focused unit/property tests. - Verification: api test/typecheck/lint, root typecheck/test/lint, session-sync typecheck/test, lint:effect, and git diff --check passed locally.
AI Session BackupCommit: 24dac93
|
Agent Plan UpdateBranch: 1. PlanSource: codex - Captured: 2026-06-17T19:20:21.136Z Codex PlanSteps
2. PlanSource: codex - Captured: 2026-06-17T19:22:16.682Z Codex PlanSteps
3. PlanSource: codex - Captured: 2026-06-17T19:24:09.201Z Codex PlanSteps
4. PlanSource: codex - Captured: 2026-06-17T19:24:20.485Z Codex PlanSteps
|
- migrate session-sync JSON decoders to typed Effect DecodeError contracts - add visible terminal persistence race regression coverage - log best-effort snapshot persistence failures and type readonly snapshot schema - document clonedOnHostname public contracts
Agent Plan UpdateBranch: 1. PlanSource: codex - Captured: 2026-06-17T19:20:21.136Z Codex PlanSteps
2. PlanSource: codex - Captured: 2026-06-17T19:22:16.682Z Codex PlanSteps
3. PlanSource: codex - Captured: 2026-06-17T19:24:09.201Z Codex PlanSteps
4. PlanSource: codex - Captured: 2026-06-17T19:24:20.485Z Codex PlanSteps
|
Summary
clonedOnHostnameat API/config schema boundaries and keep command-builder core pure by passing host identity explicitly.Effect.logWarninginpersistSnapshotBestEffort, explicitReadonlyArray<AgentSession>snapshot schema, and TSDoc on publicclonedOnHostnamecontracts.Proof of fix
Effect.Effect<*, DecodeError>, added project-scoped terminal state locking, added the terminal persistence race regression, logged best-effort snapshot persistence failures throughEffect.logWarning, typed snapshot sessions asReadonlyArray<AgentSession>, and documentedclonedOnHostnamepublic contracts.rtk bun run --cwd packages/docker-git-session-sync testrtk bun run --cwd packages/api test -- tests/terminal-sessions.test.ts tests/agents.test.tsrtk bun run typecheckrtk bun run lint:effectrtk bun run testrtk bun run lintrtk bun run --cwd packages/api testrtk bun run --cwd packages/api lintrtk git diff --checkNotes
lint:effectexits with 0 errors. Remaining warnings are migration backlog: 14 in session-sync and 135 in API.