feat(terminal): add shared terminal package#362
Open
skulidropek wants to merge 1 commit into
Open
Conversation
Member
Author
|
Warning Review limit reached
More reviews will be available in 46 minutes and 18 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (46)
📝 WalkthroughПошаговый обзорЭтот PR создаёт новый пакет ИзмененияСоздание и настройка пакета terminal
Контракты и ядро функционала терминала
Серверная функциональность терминала
Рефакторинг пакета api
Рефакторинг пакета app
Тесты
🎯 4 (Complex) | ⏱️ ~60 minutes Возможно связанные PRs
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (5 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 |
Member
Author
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/api/tests/terminal-image-paste-core.test.ts (1)
8-70: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winРекомендуется обновить стиль тестов для консистентности с терминальным пакетом.
Импорт корректно обновлён на
@prover-coder-ai/docker-git-terminal/core, но тесты используют plainit()вместоit.effect(), что не соответствует стилю терминального пакета (packages/terminal/tests/core/image-paste.test.ts должен использовать аналогичный стиль).Для консистентности и явного следования Effect-паттернам рекомендуется обновить все тесты на использование
it.effect(() => Effect.sync(...)).♻️ Пример обновления
- it("creates a safe project-container path for supported images", () => { - const plan = createTerminalImagePastePlan({ + it.effect("creates a safe project-container path for supported images", () => + Effect.sync(() => { + const plan = createTerminalImagePastePlan({ + data: "aGVsbG8=", + mediaType: "image/png", + name: "../screen shot.png", + size: 5 + }, "paste-id") + + expect(plan).toEqual({ + _tag: "ValidTerminalImagePaste", + containerPath: `${terminalImagePasteDirectory}/paste-id-screen-shot.png`, + decodedBytes: 5, + normalizedBase64: "aGVsbG8=" + }) + })) - data: "aGVsbG8=", - mediaType: "image/png", - name: "../screen shot.png", - size: 5 - }, "paste-id") - - expect(plan).toEqual({ - _tag: "ValidTerminalImagePaste", - containerPath: `${terminalImagePasteDirectory}/paste-id-screen-shot.png`, - decodedBytes: 5, - normalizedBase64: "aGVsbG8=" - }) - })🤖 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/tests/terminal-image-paste-core.test.ts` around lines 8 - 70, Tests use plain it() but should follow the terminal package Effect style; update each test block that calls createTerminalImagePastePlan and sanitizeTerminalImageBaseName to use it.effect(() => Effect.sync(() => { ...expect assertions... })), wrapping the existing setup and expect(...) assertions inside Effect.sync; ensure you preserve references to createTerminalImagePastePlan, sanitizeTerminalImageBaseName, terminalImagePasteDirectory and terminalImagePasteMaxBytes and return the Effect from each it.effect wrapper so test runner executes the effect.packages/api/tests/terminal-image-fetch-core.test.ts (1)
3-155: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winРекомендуется обновить стиль тестов для консистентности с терминальным пакетом.
Импорт корректно обновлён на
@prover-coder-ai/docker-git-terminal/server, но стиль тестов не соответствует новому пакету:
- Терминальный пакет (packages/terminal/tests/server/image-fetch.test.ts): использует
it.effect(() => Effect.sync(...))- Этот файл: использует plain
it(() => { ... })Для консистентности и строгого следования Effect-TS паттернам рекомендуется обновить тесты на стиль с
it.effect.♻️ Пример обновления стиля тестов
- it("continues to accept an absolute path with a supported image extension", () => { - expect(planTerminalImageFetch("/tmp/issue232-main.png")).toEqual({ + it.effect("continues to accept an absolute path with a supported image extension", () => + Effect.sync(() => { + expect(planTerminalImageFetch("/tmp/issue232-main.png")).toEqual({ + _tag: "ValidTerminalImageFetch", + containerPath: "/tmp/issue232-main.png", + mediaType: "image/png" + }) + })) - _tag: "ValidTerminalImageFetch", - containerPath: "/tmp/issue232-main.png", - mediaType: "image/png" - }) - })Применить аналогичное изменение ко всем тестам в файле.
🤖 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/tests/terminal-image-fetch-core.test.ts` around lines 3 - 155, Tests use plain Jest-style it(...) blocks but should follow the terminal package pattern using it.effect with Effect.sync; update each test that calls planTerminalImageFetch to use it.effect("...", () => Effect.sync(() => { /* same assertions */ })) so assertions remain inside an Effect, preserve existing expect(...) calls and messages, and apply this change consistently for all it blocks in the file (keep the import of planTerminalImageFetch as-is).
🤖 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/terminal/src/app/main.ts`:
- Around line 6-16: The EFFECT comment is incorrect: the constant main is
created by piping program through Effect.provide(NodeContext.layer) so
NodeContext is already provided and main has type Effect<void, never, never>;
update the EFFECT annotation in the comment to read EFFECT: Effect<void, never,
never> (referencing main, program, NodeContext.layer and Effect.provide to
locate the code) so the comment reflects the actual signature of main.
In `@packages/terminal/src/cli/index.ts`:
- Around line 1-3: The current TerminalCliRuntimeBoundary is only a type-level
marker; replace it with an Effect-TS Context.Tag class (retain the name
TerminalCliRuntimeBoundary) that declares real methods returning Effect types
(e.g., executeCliCommand: (cmd: string) => Effect.Effect<unknown, CliError,
string>, readStdin: Effect.Effect<unknown, CliError, string>, etc.), and provide
a production Layer implementation (e.g., TerminalCliRuntimeBoundaryLive =
Layer.effect(...)) which uses Effect.gen to create the runtime methods; ensure
the implementation lives in the SHELL, pure logic remains in CORE, and all
consumers require the Tag rather than relying on the old _tag marker.
In `@packages/terminal/src/contracts/session.ts`:
- Around line 3-81: The exported schema contracts (TerminalSessionStatusSchema,
TerminalSessionSchema, TerminalClientMessageSchema, TerminalServerMessageSchema
and the underlying
TerminalServerMessagePayloadSchema/TerminalClientMessagePayloadSchema) lack the
required TSDoc and functional markers; add a TSDoc block above each exported
symbol containing `@pure` and the requested lifecycle/behavior tags
(`@precondition/`@postcondition or `@invariant` as applicable), an `@complexity`
annotation, and include the repository functional comment markers CHANGE, WHY,
PURITY, INVARIANT plus a QUOTE(ТЗ)/REF that links to the source specification
for the contract; ensure the docs describe expected inputs/outputs (types), any
invariants (e.g., allowed status literals), side‑effects (none — mark `@pure`),
and complexity so the exported schemas satisfy the repo guidelines.
In `@packages/terminal/src/core/image-paste.ts`:
- Around line 80-130: Add comprehensive TSDoc to the CORE pure functions
(createTerminalImagePastePlan and its helpers like terminalImageFileName,
normalizeBase64, decodedBase64Bytes) including the required functional markers
and tags: add `@pure` and a PURITY marker, an `@invariant` and INVARIANT marker that
states decodedBytes === payload.size and that mediaType/fileName are supported,
`@precondition` listing valid inputs (payload.mediaType present and lowercaseable,
payload.size is finite > 0 and ≤ terminalImagePasteMaxBytes, normalizedBase64
length ≤ terminalImagePasteMaxBase64Length), `@postcondition` describing the
returned union (_tag either "ValidTerminalImagePaste" with containerPath,
decodedBytes, normalizedBase64 or "InvalidTerminalImagePaste" with message), and
`@complexity` with O(n) where n is payload.data length; also include a REF or
QUOTE(ТЗ) marker linking to the validation requirements and mention any helper
invariants for terminalImageFileName and decodedBase64Bytes to guide future
reviewers.
- Around line 68-78: The terminalImageFileName function currently interpolates
id directly into the file name, risking path traversal if id ever contains
slashes or dots; update terminalImageFileName to validate/sanitize id (similar
to sanitizeTerminalImageBaseName) before composing the name: remove or reject
path separators (/, \), path-traversal segments (..), and any characters outside
a safe whitelist (e.g., [A-Za-z0-9_-]); if the sanitized id is empty or invalid,
return null (or throw) so callers like createTerminalImagePastePlan cannot
create names that escape terminalImagePasteDirectory. Ensure you reference
terminalImageFileName and reuse or mirror the sanitization approach used by
sanitizeTerminalImageBaseName so behavior is consistent.
In `@packages/terminal/src/core/output-buffer.ts`:
- Around line 16-35: trimTerminalOutputChunks mutates local state (kept array
and remainingOverflow) which violates CORE immutability rules; rewrite it to use
an immutable fold (e.g., Array.prototype.reduce) or a tail-recursive helper that
returns a new ReadonlyArray<string> and remainingOverflow as part of the
accumulator instead of push/reassignment, keeping the same behavior for cases
where chunk lengths exceed overflow, and ensure the function signature still
returns ReadonlyArray<string>. Add a TSDoc block on trimTerminalOutputChunks
with `@invariant` describing immutability guarantees, `@complexity` Big-O for
time/space, and include a short variant function note (or a small pure helper
name) to document the decreasing measure (remainingOverflow or index) that
proves termination.
- Around line 37-69: Add comprehensive TSDoc comments to both public functions
appendTerminalOutput and renderTerminalOutputBuffer: describe purpose,
parameters and return value, and include the required markers — add `@pure`, a
mathematical `@invariant` that specifies behavior (e.g., when boundedMax === 0
returns empty buffer; when data.length === 0 returns original buffer; result
contains at most boundedMax characters and is the suffix of the concatenated
input), and `@complexity` in Big-O (for appendTerminalOutput state the cost in
terms of data length/number of chunks; for renderTerminalOutputBuffer state O(n)
in total output length). Also include standard tags (`@param`, `@returns`) so the
functions meet the CORE TSDoc requirements.
In `@packages/terminal/src/server/image-fetch.ts`:
- Around line 151-213: Add TSDoc to the exported function
planTerminalImageFetch: place a docblock immediately above its declaration
describing purpose and behavior, and include the required tags `@pure`,
`@invariant`, `@precondition`, `@postcondition`, and `@complexity`; reference the
parameter TerminalImageFetchOptions and return type TerminalImageFetchPlan,
summarize input validation rules (empty path, normalization, baseDir rules,
invalid characters/traversal, extension and media type checks) in the pre/post
conditions, state that the function is pure in `@pure`, list any invariants (e.g.
returned ValidTerminalImageFetch always has containerPath and mediaType), and
note time/space complexity in `@complexity`. Ensure the text is concise and
written as standard TSDoc above planTerminalImageFetch.
In `@packages/terminal/src/server/index.ts`:
- Around line 23-27: В типе TerminalShellCommand уберите избыточные "|
undefined" у опциональных свойств: оставьте readonly cwd?: string; и readonly
env?: Readonly<Record<string, string>>; вместо текущих cwd?: string | undefined
и env?: Readonly<Record<string, string>> | undefined; (сохраните readonly и
неизменность типа command).
In `@packages/terminal/src/shell/index.ts`:
- Around line 1-3: The TerminalShellRuntimeBoundary currently is only a
type-level marker and must be replaced with an Effect-TS Context.Tag service and
a production Layer implementation: define TerminalShellRuntimeBoundary as a
Context.Tag class exposing methods (e.g., executeShellCommand, getEnvironment)
whose signatures return Effect types, then implement
TerminalShellRuntimeBoundaryLive as a Layer.effect (using Effect.gen or similar)
that provides the concrete implementations and wraps errors in a ShellError;
update all consumers to require the Context.Tag service instead of the plain
type marker.
- Around line 1-3: The exported type TerminalShellRuntimeBoundary lacks the
required TSDoc/functional comment; add a comprehensive TSDoc block above the
export for TerminalShellRuntimeBoundary that includes CHANGE, WHY, QUOTE or n/a,
REF, SOURCE or n/a, FORMAT THEOREM, PURITY (CORE|SHELL), INVARIANT, COMPLEXITY,
a short description of its purpose and invariants, and a minimal usage example
showing how to consume the type; ensure the comment follows the project's coding
guidelines and documents the readonly _tag invariant and typical use case.
In `@packages/terminal/src/web/index.ts`:
- Around line 1-3: The current TerminalWebRuntimeBoundary is only a type-level
marker and must be converted into a runtime Effect-TS Layer-based service:
replace the plain exported type TerminalWebRuntimeBoundary with a
Context.Tag-based service (using Context.Tag("TerminalWebRuntimeBoundary")) that
exposes concrete methods like handleWebSocketMessage and sendToClient typed to
return Effect.Effect, then provide a production Layer (e.g.,
TerminalWebRuntimeBoundaryLive) built via Layer.effect/Effect.gen that
implements those methods and wraps runtime side-effects (websocket send/receive)
in Effect/Either-style error handling; ensure all consumers inject this Layer
instead of relying on the marker type so the boundary enforces runtime
separation between SHELL and CORE.
- Around line 1-3: The exported type TerminalWebRuntimeBoundary lacks required
TSDoc/functional comments; add a comprehensive TSDoc block immediately above the
exported type TerminalWebRuntimeBoundary that describes its purpose, invariants,
and usage, includes the mandated fields CHANGE, WHY, QUOTE (or n/a), REF, SOURCE
(or n/a), FORMAT THEOREM, PURITY (CORE|SHELL), INVARIANT, and COMPLEXITY, and
provide a brief usage example showing how to construct/consume the boundary;
ensure the comment follows existing coding-guidelines formatting and covers any
invariants about the _tag literal.
In `@packages/terminal/tests/architecture/boundaries.test.ts`:
- Around line 1-2: Tests directly call node:fs/node:path (readFileSync,
readdirSync) and rely on a regex-based import parser; wrap all filesystem access
in typed Effect wrappers (e.g., implement readFile(path): Effect.Effect<string,
Error> and listFiles(directory): Effect.Effect<ReadonlyArray<string>, Error>
that internally call readFileSync/readdirSync) and replace the fragile regex
import extraction with a TypeScript AST-based extractor (e.g., use
ts.createSourceFile and an extractImports(sourceFile: ts.SourceFile) function
that traverses nodes and collects moduleSpecifier texts for
ImportDeclaration/ExportDeclaration), then update the test code to call these
new readFile/listFiles/extractImports helpers instead of direct fs calls and the
regex.
In `@packages/terminal/tests/core/image-paste.test.ts`:
- Around line 26-30: Add a property-based test for the
sanitizeTerminalImageBaseName invariant: write an it.effect test (in the same
test suite) that uses fast-check (fc.assert + fc.property(fc.string(), ...)) to
feed arbitrary strings to sanitizeTerminalImageBaseName and assert the returned
name never contains "/", "\\" or the sequence ".." (and choose a descriptive
test title like "санитизированное имя никогда не содержит разделителей пути");
use the existing Effect.sync wrapper as in other tests and keep the assertion
boolean style used in the repo.
In `@packages/terminal/tests/core/output-buffer.test.ts`:
- Around line 22-40: Add a property-based test using fast-check that verifies
the buffer invariant charLength <= budget for any sequence of appends: generate
an array of strings and a budget (e.g., 1..256), reduce the chunks with
appendTerminalOutput starting from emptyTerminalOutputBuffer using the generated
budget, and assert buffer.charLength <= budget (wrap in Effect.sync and use
fc.assert/fc.property as in other tests); reference appendTerminalOutput,
emptyTerminalOutputBuffer, and charLength so the test locates the right
functions/fields.
In `@packages/terminal/tests/server/image-fetch.test.ts`:
- Around line 6-32: Add property-based tests using fast-check that exercise
planTerminalImageFetch: assert idempotence of normalization
(normalize(normalize(p)) === normalize(p)), assert any path containing '.' or
'..' segments yields an InvalidTerminalImageFetch, and assert deterministic
mapping from file extension to mediaType for all valid paths; implement these as
new it.effect cases calling fc.assert + fc.property and use
planTerminalImageFetch inside the property functions to check _tag or equality,
referencing the existing tests for pattern and reusing the same error/message
expectations.
---
Outside diff comments:
In `@packages/api/tests/terminal-image-fetch-core.test.ts`:
- Around line 3-155: Tests use plain Jest-style it(...) blocks but should follow
the terminal package pattern using it.effect with Effect.sync; update each test
that calls planTerminalImageFetch to use it.effect("...", () => Effect.sync(()
=> { /* same assertions */ })) so assertions remain inside an Effect, preserve
existing expect(...) calls and messages, and apply this change consistently for
all it blocks in the file (keep the import of planTerminalImageFetch as-is).
In `@packages/api/tests/terminal-image-paste-core.test.ts`:
- Around line 8-70: Tests use plain it() but should follow the terminal package
Effect style; update each test block that calls createTerminalImagePastePlan and
sanitizeTerminalImageBaseName to use it.effect(() => Effect.sync(() => {
...expect assertions... })), wrapping the existing setup and expect(...)
assertions inside Effect.sync; ensure you preserve references to
createTerminalImagePastePlan, sanitizeTerminalImageBaseName,
terminalImagePasteDirectory and terminalImagePasteMaxBytes and return the Effect
from each it.effect wrapper so test runner executes the 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: 7479afaf-03f8-4fbe-be81-b656e322b86d
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (44)
.github/workflows/check.ymlpackage.jsonpackages/api/package.jsonpackages/api/src/services/auth-terminal-sessions.tspackages/api/src/services/terminal-image-fetch-core.tspackages/api/src/services/terminal-image-paste-core.tspackages/api/src/services/terminal-output-buffer.tspackages/api/src/services/terminal-sessions.tspackages/api/tests/terminal-image-fetch-core.test.tspackages/api/tests/terminal-image-paste-core.test.tspackages/api/tests/terminal-output-buffer.test.tspackages/app/package.jsonpackages/app/src/docker-git/api-terminal-codec.tspackages/app/src/docker-git/terminal-session-client.tspackages/app/src/shared/terminal-session-schema.tspackages/app/src/web/api-terminal-schema.tspackages/app/src/web/api-types.tspackages/app/src/web/terminal.tspackages/app/tests/docker-git/core-templates.test.tspackages/terminal/biome.jsonpackages/terminal/eslint.config.mjspackages/terminal/eslint.effect-ts-check.config.mjspackages/terminal/package.jsonpackages/terminal/src/app/main.tspackages/terminal/src/app/program.tspackages/terminal/src/cli/index.tspackages/terminal/src/contracts/index.tspackages/terminal/src/contracts/session.tspackages/terminal/src/core/image-paste.tspackages/terminal/src/core/index.tspackages/terminal/src/core/output-buffer.tspackages/terminal/src/index.tspackages/terminal/src/server/image-fetch.tspackages/terminal/src/server/index.tspackages/terminal/src/shell/index.tspackages/terminal/src/web/index.tspackages/terminal/tests/architecture/boundaries.test.tspackages/terminal/tests/contracts/session.test.tspackages/terminal/tests/core/image-paste.test.tspackages/terminal/tests/core/output-buffer.test.tspackages/terminal/tests/server/image-fetch.test.tspackages/terminal/tsconfig.jsonpackages/terminal/vitest.config.tspnpm-workspace.yaml
📜 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). (1)
- GitHub Check: Lint
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{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/terminal/src/app/program.tspackages/app/src/docker-git/api-terminal-codec.tspackages/app/tests/docker-git/core-templates.test.tspackages/terminal/src/contracts/index.tspackages/terminal/src/core/index.tspackages/terminal/vitest.config.tspackages/terminal/src/cli/index.tspackages/terminal/tests/server/image-fetch.test.tspackages/terminal/src/web/index.tspackages/app/src/docker-git/terminal-session-client.tspackages/terminal/src/server/index.tspackages/terminal/src/shell/index.tspackages/app/src/web/api-types.tspackages/api/tests/terminal-image-fetch-core.test.tspackages/api/tests/terminal-image-paste-core.test.tspackages/terminal/src/app/main.tspackages/terminal/tests/contracts/session.test.tspackages/app/src/web/api-terminal-schema.tspackages/terminal/src/index.tspackages/api/tests/terminal-output-buffer.test.tspackages/terminal/src/core/output-buffer.tspackages/api/src/services/terminal-output-buffer.tspackages/terminal/tests/architecture/boundaries.test.tspackages/api/src/services/terminal-image-paste-core.tspackages/app/src/shared/terminal-session-schema.tspackages/terminal/src/contracts/session.tspackages/terminal/tests/core/output-buffer.test.tspackages/terminal/tests/core/image-paste.test.tspackages/api/src/services/terminal-image-fetch-core.tspackages/terminal/src/server/image-fetch.tspackages/app/src/web/terminal.tspackages/terminal/src/core/image-paste.tspackages/api/src/services/terminal-sessions.tspackages/api/src/services/auth-terminal-sessions.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/terminal/src/app/program.tspackages/app/src/docker-git/api-terminal-codec.tspackages/app/tests/docker-git/core-templates.test.tspackages/terminal/src/contracts/index.tspackages/terminal/src/core/index.tspackages/terminal/vitest.config.tspackages/terminal/src/cli/index.tspackages/terminal/tests/server/image-fetch.test.tspackages/terminal/src/web/index.tspackages/app/src/docker-git/terminal-session-client.tspackages/terminal/src/server/index.tspackages/terminal/src/shell/index.tspackages/app/src/web/api-types.tspackages/api/tests/terminal-image-fetch-core.test.tspackages/api/tests/terminal-image-paste-core.test.tspackages/terminal/src/app/main.tspackages/terminal/tests/contracts/session.test.tspackages/app/src/web/api-terminal-schema.tspackages/terminal/src/index.tspackages/api/tests/terminal-output-buffer.test.tspackages/terminal/src/core/output-buffer.tspackages/api/src/services/terminal-output-buffer.tspackages/terminal/tests/architecture/boundaries.test.tspackages/api/src/services/terminal-image-paste-core.tspackages/app/src/shared/terminal-session-schema.tspackages/terminal/src/contracts/session.tspackages/terminal/tests/core/output-buffer.test.tspackages/terminal/tests/core/image-paste.test.tspackages/api/src/services/terminal-image-fetch-core.tspackages/terminal/src/server/image-fetch.tspackages/app/src/web/terminal.tspackages/terminal/src/core/image-paste.tspackages/api/src/services/terminal-sessions.tspackages/api/src/services/auth-terminal-sessions.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/terminal/src/app/program.tspackages/app/src/docker-git/api-terminal-codec.tspackages/app/tests/docker-git/core-templates.test.tspackages/terminal/src/contracts/index.tspackages/terminal/src/core/index.tspackages/terminal/vitest.config.tspackages/terminal/src/cli/index.tspackages/terminal/tests/server/image-fetch.test.tspackages/terminal/src/web/index.tspackages/app/src/docker-git/terminal-session-client.tspackages/terminal/src/server/index.tspackages/terminal/src/shell/index.tspackages/app/src/web/api-types.tspackages/api/tests/terminal-image-fetch-core.test.tspackages/api/tests/terminal-image-paste-core.test.tspackages/terminal/src/app/main.tspackages/terminal/tests/contracts/session.test.tspackages/app/src/web/api-terminal-schema.tspackages/terminal/src/index.tspackages/api/tests/terminal-output-buffer.test.tspackages/terminal/src/core/output-buffer.tspackages/api/src/services/terminal-output-buffer.tspackages/terminal/tests/architecture/boundaries.test.tspackages/api/src/services/terminal-image-paste-core.tspackages/app/src/shared/terminal-session-schema.tspackages/terminal/src/contracts/session.tspackages/terminal/tests/core/output-buffer.test.tspackages/terminal/tests/core/image-paste.test.tspackages/api/src/services/terminal-image-fetch-core.tspackages/terminal/src/server/image-fetch.tspackages/app/src/web/terminal.tspackages/terminal/src/core/image-paste.tspackages/api/src/services/terminal-sessions.tspackages/api/src/services/auth-terminal-sessions.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/terminal/src/app/program.tspackages/app/src/docker-git/api-terminal-codec.tspackages/app/tests/docker-git/core-templates.test.tspackages/terminal/src/contracts/index.tspackages/terminal/src/core/index.tspackages/terminal/vitest.config.tspackages/terminal/src/cli/index.tspackages/terminal/tests/server/image-fetch.test.tspackages/terminal/src/web/index.tspackages/app/src/docker-git/terminal-session-client.tspackages/terminal/src/server/index.tspackages/terminal/src/shell/index.tspackages/app/src/web/api-types.tspackages/api/tests/terminal-image-fetch-core.test.tspackages/api/tests/terminal-image-paste-core.test.tspackages/terminal/src/app/main.tspackages/terminal/tests/contracts/session.test.tspackages/app/src/web/api-terminal-schema.tspackages/terminal/src/index.tspackages/api/tests/terminal-output-buffer.test.tspackages/terminal/src/core/output-buffer.tspackages/api/src/services/terminal-output-buffer.tspackages/terminal/tests/architecture/boundaries.test.tspackages/api/src/services/terminal-image-paste-core.tspackages/app/src/shared/terminal-session-schema.tspackages/terminal/src/contracts/session.tspackages/terminal/tests/core/output-buffer.test.tspackages/terminal/tests/core/image-paste.test.tspackages/api/src/services/terminal-image-fetch-core.tspackages/terminal/src/server/image-fetch.tspackages/app/src/web/terminal.tspackages/terminal/src/core/image-paste.tspackages/api/src/services/terminal-sessions.tspackages/api/src/services/auth-terminal-sessions.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/terminal/src/app/program.tspackages/app/src/docker-git/api-terminal-codec.tspnpm-workspace.yamlpackages/app/tests/docker-git/core-templates.test.tspackages/terminal/src/contracts/index.tspackages/terminal/src/core/index.tspackages/terminal/vitest.config.tspackages/terminal/src/cli/index.tspackages/terminal/tests/server/image-fetch.test.tspackages/terminal/biome.jsonpackages/terminal/src/web/index.tspackages/app/src/docker-git/terminal-session-client.tspackages/terminal/src/server/index.tspackages/terminal/src/shell/index.tspackages/app/src/web/api-types.tspackages/api/tests/terminal-image-fetch-core.test.tspackages/terminal/tsconfig.jsonpackages/api/tests/terminal-image-paste-core.test.tspackages/terminal/src/app/main.tspackages/terminal/tests/contracts/session.test.tspackages/app/src/web/api-terminal-schema.tspackages/terminal/src/index.tspackages/api/tests/terminal-output-buffer.test.tspackages/terminal/src/core/output-buffer.tspackages/api/src/services/terminal-output-buffer.tspackages/terminal/tests/architecture/boundaries.test.tspackages/terminal/package.jsonpackages/api/src/services/terminal-image-paste-core.tspackages/app/src/shared/terminal-session-schema.tspackages/terminal/src/contracts/session.tspackages/api/package.jsonpackages/terminal/tests/core/output-buffer.test.tspackages/terminal/tests/core/image-paste.test.tspackages/app/package.jsonpackages/api/src/services/terminal-image-fetch-core.tspackages/terminal/src/server/image-fetch.tspackages/app/src/web/terminal.tspackages/terminal/src/core/image-paste.tspackages/api/src/services/terminal-sessions.tspackages/api/src/services/auth-terminal-sessions.tspackage.json
**/*
⚙️ CodeRabbit configuration file
**/*: Ты строгий ревьюер SPEC DRIVEN DEVELOPMENT.Перед выводами изучи README.md, другие *.md файлы, linked issues,
PR description, PR comments/discussion и релевантную кодовую базу.Сверь изменения с исходным ТЗ/спекой и обсуждением. Флагай любой уход
от спеки, недокументированное изменение поведения, отсутствие тестов
для заявленного поведения и security-риск. Если спека не видна,
попроси автора добавить ее в issue или PR description.Проверь решение с точки зрения формальной верификации: какие инварианты,
предусловия и постусловия можно доказать математически, а где доказуемость
слабая. Оцени решение с точки зрения теории игр: устойчивы ли стимулы,
нет ли выгодного обхода правил, и какое решение было бы сильнее.
Files:
packages/terminal/src/app/program.tspackages/app/src/docker-git/api-terminal-codec.tspnpm-workspace.yamlpackages/app/tests/docker-git/core-templates.test.tspackages/terminal/src/contracts/index.tspackages/terminal/src/core/index.tspackages/terminal/vitest.config.tspackages/terminal/src/cli/index.tspackages/terminal/tests/server/image-fetch.test.tspackages/terminal/biome.jsonpackages/terminal/eslint.effect-ts-check.config.mjspackages/terminal/src/web/index.tspackages/app/src/docker-git/terminal-session-client.tspackages/terminal/src/server/index.tspackages/terminal/src/shell/index.tspackages/app/src/web/api-types.tspackages/api/tests/terminal-image-fetch-core.test.tspackages/terminal/tsconfig.jsonpackages/terminal/eslint.config.mjspackages/api/tests/terminal-image-paste-core.test.tspackages/terminal/src/app/main.tspackages/terminal/tests/contracts/session.test.tspackages/app/src/web/api-terminal-schema.tspackages/terminal/src/index.tspackages/api/tests/terminal-output-buffer.test.tspackages/terminal/src/core/output-buffer.tspackages/api/src/services/terminal-output-buffer.tspackages/terminal/tests/architecture/boundaries.test.tspackages/terminal/package.jsonpackages/api/src/services/terminal-image-paste-core.tspackages/app/src/shared/terminal-session-schema.tspackages/terminal/src/contracts/session.tspackages/api/package.jsonpackages/terminal/tests/core/output-buffer.test.tspackages/terminal/tests/core/image-paste.test.tspackages/app/package.jsonpackages/api/src/services/terminal-image-fetch-core.tspackages/terminal/src/server/image-fetch.tspackages/app/src/web/terminal.tspackages/terminal/src/core/image-paste.tspackages/api/src/services/terminal-sessions.tspackages/api/src/services/auth-terminal-sessions.tspackage.json
**/*.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/core-templates.test.tspackages/terminal/tests/server/image-fetch.test.tspackages/api/tests/terminal-image-fetch-core.test.tspackages/api/tests/terminal-image-paste-core.test.tspackages/terminal/tests/contracts/session.test.tspackages/api/tests/terminal-output-buffer.test.tspackages/terminal/tests/architecture/boundaries.test.tspackages/terminal/tests/core/output-buffer.test.tspackages/terminal/tests/core/image-paste.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/core-templates.test.tspackages/terminal/tests/server/image-fetch.test.tspackages/api/tests/terminal-image-fetch-core.test.tspackages/api/tests/terminal-image-paste-core.test.tspackages/terminal/tests/contracts/session.test.tspackages/api/tests/terminal-output-buffer.test.tspackages/terminal/tests/architecture/boundaries.test.tspackages/terminal/tests/core/output-buffer.test.tspackages/terminal/tests/core/image-paste.test.ts
**/.github/workflows/*.{yml,yaml}
📄 CodeRabbit inference engine (Custom checks)
Fail if changed files introduce unsafe GitHub Actions configuration such as unsafe
pull_request_target, or unnecessary write permissions
Files:
.github/workflows/check.yml
**/{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:
packages/terminal/package.jsonpackages/api/package.jsonpackages/app/package.jsonpackage.json
package.json
📄 CodeRabbit inference engine (CLAUDE.md)
Dependencies must include Effect-TS (
effect: ^3.x) for monadic effects and@effect/schema: ^0.xfor validation with strong typing.Dependencies like effect (^3.x) and
@effect/schema(^0.x) are mandatory; project must enforce monadic composition through Effect.
Files:
package.json
🧠 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/api-terminal-codec.tspackages/app/src/docker-git/terminal-session-client.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/terminal-output-buffer.tspackages/api/src/services/terminal-image-paste-core.tspackages/api/src/services/terminal-image-fetch-core.tspackages/api/src/services/terminal-sessions.tspackages/api/src/services/auth-terminal-sessions.ts
🔇 Additional comments (41)
packages/api/src/services/terminal-output-buffer.ts (1)
1-7: LGTM!packages/api/src/services/auth-terminal-sessions.ts (1)
3-9: LGTM!packages/api/src/services/terminal-image-paste-core.ts (1)
1-9: LGTM!packages/api/src/services/terminal-image-fetch-core.ts (1)
1-6: LGTM!packages/api/src/services/terminal-sessions.ts (1)
21-38: LGTM!packages/api/package.json (1)
10-10: LGTM!Also applies to: 15-15, 18-18, 23-23
packages/terminal/src/server/image-fetch.ts (3)
1-36: LGTM!
48-75: LGTM!
77-135: LGTM!packages/terminal/src/server/index.ts (2)
1-5: LGTM!
29-51: LGTM!packages/terminal/src/contracts/index.ts (1)
1-1: LGTM!packages/terminal/src/core/index.ts (1)
1-2: LGTM!packages/terminal/src/index.ts (1)
1-2: LGTM!packages/terminal/src/shell/index.ts (1)
1-3: Уточнить применение TerminalShellRuntimeBoundary и соответствие issue#361
- В репозитории (кроме
packages/terminal/src/shell/index.ts) не найдено ни одной ссылки наTerminalShellRuntimeBoundary, поэтому тип выглядит неиспользуемым.- Требуется подтвердить реализацию требований
issue#361`` и наличие тестов/проверок поведения, которые подтверждают назначение runtime boundary (например, вpackages/terminal/testsи/или релевантных интеграционных сценариях).packages/terminal/src/cli/index.ts (1)
1-3:⚠️ Potential issue | 🟠 Major | ⚡ Quick winОтсутствует обязательная документация для публичного API.
Экспортируемый тип
TerminalCliRuntimeBoundaryне содержит ни TSDoc комментариев, ни функциональных комментариев. Согласно coding guidelines, все экспортируемые сущности должны быть задокументированы с описанием назначения, инвариантов и примеров использования.📝 Предлагаемая документация
+/** + * Runtime boundary marker для CLI-адаптера терминального пакета. + * + * CHANGE: Добавлен тип-маркер для разграничения CLI runtime boundary + * WHY: Обеспечение type-level разделения между CLI, Shell и Web runtime слоями + * QUOTE(ТЗ): [требуется ссылка на issue `#361` или спецификацию] + * REF: n/a + * SOURCE: n/a + * FORMAT THEOREM: ∀x: TerminalCliRuntimeBoundary: x._tag = "TerminalCliRuntimeBoundary" + * PURITY: CORE + * INVARIANT: _tag является string literal константой для type discrimination + * + * `@remarks` + * Этот тип используется для маркировки границ между runtime слоями в FCIS архитектуре. + * Конкретное использование: [требуется описание, где и как используется] + * + * `@example` + * ```typescript + * // [требуется пример использования] + * ``` + */ export type TerminalCliRuntimeBoundary = { readonly _tag: "TerminalCliRuntimeBoundary"; };As per coding guidelines: "Functional comments must include: CHANGE, WHY, QUOTE(ТЗ) or n/a, REF, SOURCE or n/a, FORMAT THEOREM, PURITY (CORE|SHELL), INVARIANT, and COMPLEXITY" and "TypeScript functions must include comprehensive TSDoc with
@puremarker,@effectdependencies,@invariant(mathematical),@precondition,@postcondition, and@complexityO-notation."> Likely an incorrect or invalid review comment.packages/terminal/src/web/index.ts (1)
1-3: TerminalWebRuntimeBoundary не используется и нет web-интеграции; нужно сверить с issue#361
TerminalWebRuntimeBoundaryнайден только вpackages/terminal/src/web/index.ts(других использований по TS-кодовой базе нет).packages/terminal/src/index.tsэкспортирует только./contractsи./core,./web(иTerminalWebRuntimeBoundary) через root-barrel не выводится.- В
packages/app/src/не обнаружены ссылки видаdocker-git-terminal.*web, т.е. web-client wiring не выявлен.- Для соответствия
issue#361`` и проверки наличия/достаточности тестов нужна формулировка требований из issue (и/или результаты проверок, которые её прямо подтверждают).packages/app/src/shared/terminal-session-schema.ts (1)
1-11: LGTM!packages/app/src/docker-git/terminal-session-client.ts (1)
2-2: LGTM!packages/app/src/web/api-terminal-schema.ts (1)
2-2: LGTM!Also applies to: 32-32
packages/app/src/web/api-types.ts (1)
99-99: LGTM!packages/app/src/web/terminal.ts (1)
2-5: LGTM!Also applies to: 239-242
packages/app/package.json (1)
16-16: LGTM!Also applies to: 25-25, 29-29, 31-31, 40-40, 42-42, 68-68
packages/app/src/docker-git/api-terminal-codec.ts (1)
18-21: ⚡ Quick winПроверка type guard
isTerminalSessionStatusна соответствиеTerminalSessionStatusSchemaМножество статусов в guard (
"ready" | "attached" | "exited" | "failed") совпадает сTerminalSessionStatusSchemaизpackages/terminal/src/contracts/session.ts, расхождений по текущей схеме нет.pnpm-workspace.yaml (1)
6-6: LGTM!.github/workflows/check.yml (1)
26-27: LGTM!Also applies to: 63-64, 87-88, 112-113, 136-137
package.json (2)
12-12: LGTM!
16-16: LGTM!Also applies to: 22-22, 46-46, 48-50
packages/terminal/package.json (1)
1-74: LGTM!packages/terminal/src/app/program.ts (1)
1-13: LGTM!packages/terminal/tsconfig.json (1)
1-15: LGTM!packages/terminal/vitest.config.ts (1)
1-23: LGTM!packages/terminal/eslint.config.mjs (1)
1-27: LGTM!packages/terminal/eslint.effect-ts-check.config.mjs (1)
1-3: LGTM!packages/terminal/biome.json (1)
1-34: LGTM!packages/terminal/tests/contracts/session.test.ts (2)
23-58: LGTM!
1-10: Убедиться, чтоParseResultимпортируется из того же источника, что и схемы в контрактах
- В
packages/terminal/package.jsonобъявлена зависимость@effect/schema(^0.75.5).- В контрактах (например,
packages/terminal/src/contracts/session.ts) схемы импортируются из@effect/schema/Schema, что согласуется с использованием@effect/schemaв тесте.- Нужна проверка: в
packages/terminal/src/contracts/**ParseResultимпортируется из@effect/schema/ParseResult(а не изeffect/ParseResult), чтобы типы не разъезжались.packages/terminal/tests/core/image-paste.test.ts (1)
10-24: LGTM!packages/terminal/tests/core/output-buffer.test.ts (1)
11-20: LGTM!packages/api/tests/terminal-output-buffer.test.ts (1)
3-7: LGTM!packages/app/tests/docker-git/core-templates.test.ts (1)
75-78: LGTM!
Comment on lines
+6
to
+16
| // CHANGE: add a thin Node runtime entrypoint for the terminal package. | ||
| // WHY: follow the effect-template shape and keep runtime provision outside core/contracts. | ||
| // QUOTE(ТЗ): "Используй так же модуль" | ||
| // REF: issue-361-terminal-package | ||
| // SOURCE: https://github.com/ProverCoderAI/effect-template/tree/main/packages/app | ||
| // FORMAT THEOREM: main = provide(program, NodeContext) | ||
| // PURITY: SHELL | ||
| // EFFECT: Effect<void, never, NodeContext> | ||
| // INVARIANT: NodeContext is provided only at the package runtime boundary. | ||
| // COMPLEXITY: O(1)/O(1) | ||
| const main = pipe(program, Effect.provide(NodeContext.layer)); |
There was a problem hiding this comment.
Некорректная аннотация типа в EFFECT комментарии.
Строка 13 указывает EFFECT: Effect<void, never, NodeContext>, но константа main (строка 16) имеет тип Effect<void, never, never>, так как NodeContext.layer уже предоставлен через Effect.provide.
Согласно coding guidelines, комментарий EFFECT должен описывать Effect-сигнатуру объявляемой сущности (в данном случае — main), а не промежуточных значений. Правильная аннотация: EFFECT: Effect<void, never, never>.
Предлагаемое исправление
// PURITY: SHELL
-// EFFECT: Effect<void, never, NodeContext>
+// EFFECT: Effect<void, never, never>
// INVARIANT: NodeContext is provided only at the package runtime boundary.🤖 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/terminal/src/app/main.ts` around lines 6 - 16, The EFFECT comment is
incorrect: the constant main is created by piping program through
Effect.provide(NodeContext.layer) so NodeContext is already provided and main
has type Effect<void, never, never>; update the EFFECT annotation in the comment
to read EFFECT: Effect<void, never, never> (referencing main, program,
NodeContext.layer and Effect.provide to locate the code) so the comment reflects
the actual signature of main.
Comment on lines
+1
to
+3
| export type TerminalCliRuntimeBoundary = { | ||
| readonly _tag: "TerminalCliRuntimeBoundary"; | ||
| }; |
There was a problem hiding this comment.
Несоответствие архитектурным требованиям FCIS и Effect-TS.
Тип назван RuntimeBoundary, но реализован как простой маркер с полем _tag. Согласно coding guidelines, границы между слоями должны реализовываться через Effect-TS Layer pattern с Context.Tag и полноценными сервисами:
- "Define all external service dependencies as Context.Tag classes with fully typed methods returning Effect types"
- "Implement service implementations as Layer.effect with Effect.gen pattern for dependency injection"
- "Implement Functional Core, Imperative Shell (FCIS) pattern: CORE layer contains only pure functions; SHELL layer isolates all effects. Strict dependency direction: SHELL → CORE"
Текущая реализация не обеспечивает реальную boundary - это только type-level marker без runtime гарантий разделения эффектов.
🏗️ Рекомендуемая архитектура с Effect Layer pattern
Если это действительно runtime boundary, должна быть примерно такая структура:
import { Context, Effect, Layer } from "effect";
/**
* CLI Runtime Boundary Service
* PURITY: SHELL
* EFFECT: Effect<TerminalCliCapabilities, CliError, never>
*/
export class TerminalCliRuntimeBoundary extends Context.Tag("TerminalCliRuntimeBoundary")<
TerminalCliRuntimeBoundary,
{
readonly executeCliCommand: (cmd: string) => Effect.Effect<string, CliError>;
readonly readStdin: Effect.Effect<string, CliError>;
// ... other CLI-specific capabilities
}
>() {}
/**
* Production implementation
*/
export const TerminalCliRuntimeBoundaryLive = Layer.effect(
TerminalCliRuntimeBoundary,
Effect.gen(function* (_) {
return {
executeCliCommand: (cmd) => Effect.tryPromise({
try: async () => {
// implementation
},
catch: (error) => new CliError({ cause: error })
}),
// ...
};
})
);As per coding guidelines: "All external service dependencies must be wrapped through typed interfaces and injected via Effect-TS Layer pattern" and "Implement Functional Core, Imperative Shell (FCIS) pattern."
🤖 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/terminal/src/cli/index.ts` around lines 1 - 3, The current
TerminalCliRuntimeBoundary is only a type-level marker; replace it with an
Effect-TS Context.Tag class (retain the name TerminalCliRuntimeBoundary) that
declares real methods returning Effect types (e.g., executeCliCommand: (cmd:
string) => Effect.Effect<unknown, CliError, string>, readStdin:
Effect.Effect<unknown, CliError, string>, etc.), and provide a production Layer
implementation (e.g., TerminalCliRuntimeBoundaryLive = Layer.effect(...)) which
uses Effect.gen to create the runtime methods; ensure the implementation lives
in the SHELL, pure logic remains in CORE, and all consumers require the Tag
rather than relying on the old _tag marker.
Comment on lines
+3
to
+81
| export const TerminalSessionStatusSchema = Schema.Union( | ||
| Schema.Literal("ready"), | ||
| Schema.Literal("attached"), | ||
| Schema.Literal("exited"), | ||
| Schema.Literal("failed"), | ||
| ); | ||
|
|
||
| export const TerminalSessionSchema = Schema.Struct({ | ||
| id: Schema.String, | ||
| projectId: Schema.String, | ||
| sshCommand: Schema.String, | ||
| status: TerminalSessionStatusSchema, | ||
| createdAt: Schema.String, | ||
| attachedClients: Schema.optional(Schema.Number), | ||
| startedAt: Schema.optional(Schema.String), | ||
| closedAt: Schema.optional(Schema.String), | ||
| exitCode: Schema.optional(Schema.Number), | ||
| signal: Schema.optional(Schema.Number), | ||
| }); | ||
|
|
||
| export const TerminalClientMessagePayloadSchema = Schema.Union( | ||
| Schema.Struct({ | ||
| type: Schema.Literal("input"), | ||
| data: Schema.String, | ||
| }), | ||
| Schema.Struct({ | ||
| type: Schema.Literal("resize"), | ||
| cols: Schema.Number, | ||
| rows: Schema.Number, | ||
| }), | ||
| Schema.Struct({ | ||
| type: Schema.Literal("image"), | ||
| data: Schema.String, | ||
| mediaType: Schema.String, | ||
| name: Schema.String, | ||
| size: Schema.Number, | ||
| }), | ||
| Schema.Struct({ | ||
| type: Schema.Literal("close"), | ||
| }), | ||
| ); | ||
|
|
||
| const TerminalServerMessagePayloadSchema = Schema.Union( | ||
| Schema.Struct({ | ||
| type: Schema.Literal("ready"), | ||
| session: TerminalSessionSchema, | ||
| }), | ||
| Schema.Struct({ | ||
| type: Schema.Literal("output"), | ||
| data: Schema.String, | ||
| }), | ||
| Schema.Struct({ | ||
| type: Schema.Literal("exit"), | ||
| exitCode: Schema.NullOr(Schema.Number), | ||
| signal: Schema.NullOr(Schema.Number), | ||
| }), | ||
| Schema.Struct({ | ||
| type: Schema.Literal("error"), | ||
| message: Schema.String, | ||
| }), | ||
| ); | ||
|
|
||
| export const TerminalClientMessageSchema = Schema.parseJson( | ||
| TerminalClientMessagePayloadSchema, | ||
| ); | ||
| export const TerminalServerMessageSchema = Schema.parseJson( | ||
| TerminalServerMessagePayloadSchema, | ||
| ); | ||
|
|
||
| export type TerminalSessionStatus = Schema.Schema.Type< | ||
| typeof TerminalSessionStatusSchema | ||
| >; | ||
| export type TerminalSession = Schema.Schema.Type<typeof TerminalSessionSchema>; | ||
| export type TerminalClientMessage = Schema.Schema.Type< | ||
| typeof TerminalClientMessagePayloadSchema | ||
| >; | ||
| export type TerminalServerMessage = Schema.Schema.Type< | ||
| typeof TerminalServerMessagePayloadSchema | ||
| >; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Отсутствует обязательная документация для экспортируемых контрактов.
Согласно руководящим принципам репозитория, все функции/сущности TypeScript должны сопровождаться TSDoc (@pure, @invariant, @precondition, @postcondition, @complexity) и функциональными маркерами (CHANGE, WHY, QUOTE(ТЗ), REF, PURITY, INVARIANT). Экспортируемые схемы (TerminalSessionStatusSchema, TerminalSessionSchema, TerminalClientMessageSchema, TerminalServerMessageSchema и др.) добавлены без этой документации. Пожалуйста, добавьте требуемые аннотации, в частности QUOTE(ТЗ)/REF со ссылкой на исходную спеку контракта.
As per coding guidelines: "TypeScript functions must include comprehensive TSDoc ... @pure marker, @effect dependencies, @invariant ..." и "Functional comments must include: CHANGE, WHY, QUOTE(ТЗ) ... INVARIANT, and COMPLEXITY".
🤖 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/terminal/src/contracts/session.ts` around lines 3 - 81, The exported
schema contracts (TerminalSessionStatusSchema, TerminalSessionSchema,
TerminalClientMessageSchema, TerminalServerMessageSchema and the underlying
TerminalServerMessagePayloadSchema/TerminalClientMessagePayloadSchema) lack the
required TSDoc and functional markers; add a TSDoc block above each exported
symbol containing `@pure` and the requested lifecycle/behavior tags
(`@precondition/`@postcondition or `@invariant` as applicable), an `@complexity`
annotation, and include the repository functional comment markers CHANGE, WHY,
PURITY, INVARIANT plus a QUOTE(ТЗ)/REF that links to the source specification
for the contract; ensure the docs describe expected inputs/outputs (types), any
invariants (e.g., allowed status literals), side‑effects (none — mark `@pure`),
and complexity so the exported schemas satisfy the repo guidelines.
Comment on lines
+68
to
+78
| const terminalImageFileName = ( | ||
| id: string, | ||
| name: string, | ||
| mediaType: string, | ||
| ): string | null => { | ||
| const extension = extensionForMediaType(mediaType); | ||
| if (extension === null) { | ||
| return null; | ||
| } | ||
| return `${id}-${sanitizeTerminalImageBaseName(name)}.${extension}`; | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Найти вызовы createTerminalImagePastePlan и контекст формирования id
rg -nP -C5 '\bcreateTerminalImagePastePlan\s*\(' --type=tsRepository: ProverCoderAI/docker-git
Length of output: 6723
Path traversal через id: текущий риск не реализуется, но инвариант лучше зафиксировать в коде
В прод-коллеках id для createTerminalImagePastePlan(...) передаётся из packages/api/src/services/terminal-sessions.ts как randomUUID(), а в тестах — строками-литералами; UUID не содержит / и .., поэтому выход за terminalImagePasteDirectory по этому пути невозможен из текущих call sites.
При этом packages/terminal/src/core/image-paste.ts строит fileName как ${id}-... без санитизации/валидации id; чтобы не полагаться на внешний инвариант и предотвратить будущий misuse, стоит добавить проверку/очистку id на уровне terminalImageFileName (аналогично name).
🤖 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/terminal/src/core/image-paste.ts` around lines 68 - 78, The
terminalImageFileName function currently interpolates id directly into the file
name, risking path traversal if id ever contains slashes or dots; update
terminalImageFileName to validate/sanitize id (similar to
sanitizeTerminalImageBaseName) before composing the name: remove or reject path
separators (/, \), path-traversal segments (..), and any characters outside a
safe whitelist (e.g., [A-Za-z0-9_-]); if the sanitized id is empty or invalid,
return null (or throw) so callers like createTerminalImagePastePlan cannot
create names that escape terminalImagePasteDirectory. Ensure you reference
terminalImageFileName and reuse or mirror the sanitization approach used by
sanitizeTerminalImageBaseName so behavior is consistent.
Comment on lines
+80
to
+130
| export const createTerminalImagePastePlan = ( | ||
| payload: TerminalImagePastePayload, | ||
| id: string, | ||
| ): TerminalImagePastePlan => { | ||
| const mediaType = payload.mediaType.toLowerCase(); | ||
| const fileName = terminalImageFileName(id, payload.name, mediaType); | ||
| if (fileName === null) { | ||
| return { | ||
| _tag: "InvalidTerminalImagePaste", | ||
| message: `Unsupported image type: ${payload.mediaType || "unknown"}.`, | ||
| }; | ||
| } | ||
| if (!Number.isFinite(payload.size) || payload.size <= 0) { | ||
| return { | ||
| _tag: "InvalidTerminalImagePaste", | ||
| message: "Image payload is empty.", | ||
| }; | ||
| } | ||
| if (payload.size > terminalImagePasteMaxBytes) { | ||
| return { | ||
| _tag: "InvalidTerminalImagePaste", | ||
| message: `Image is too large. Max size is ${terminalImagePasteMaxBytes} bytes.`, | ||
| }; | ||
| } | ||
| const normalizedBase64 = normalizeBase64(payload.data); | ||
| if (normalizedBase64.length > terminalImagePasteMaxBase64Length) { | ||
| return { | ||
| _tag: "InvalidTerminalImagePaste", | ||
| message: `Image is too large. Max size is ${terminalImagePasteMaxBytes} bytes.`, | ||
| }; | ||
| } | ||
| const decodedBytes = decodedBase64Bytes(normalizedBase64); | ||
| if (decodedBytes === null) { | ||
| return { | ||
| _tag: "InvalidTerminalImagePaste", | ||
| message: "Image payload is not valid base64.", | ||
| }; | ||
| } | ||
| if (decodedBytes !== payload.size) { | ||
| return { | ||
| _tag: "InvalidTerminalImagePaste", | ||
| message: "Image payload size does not match its base64 data.", | ||
| }; | ||
| } | ||
| return { | ||
| _tag: "ValidTerminalImagePaste", | ||
| containerPath: `${terminalImagePasteDirectory}/${fileName}`, | ||
| decodedBytes, | ||
| normalizedBase64, | ||
| }; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Отсутствует обязательная документация для CORE-функции планирования.
createTerminalImagePastePlan и вспомогательные функции — чистые CORE-функции, но без требуемого TSDoc (@pure, @invariant, @precondition, @postcondition, @complexity) и функциональных маркеров (PURITY, INVARIANT, QUOTE(ТЗ)/REF). Добавьте их, описав инварианты валидации (соответствие decodedBytes === payload.size, ограничения по размеру).
As per coding guidelines: "TypeScript functions must include comprehensive TSDoc ... @precondition, @postcondition, and @complexity O-notation."
🤖 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/terminal/src/core/image-paste.ts` around lines 80 - 130, Add
comprehensive TSDoc to the CORE pure functions (createTerminalImagePastePlan and
its helpers like terminalImageFileName, normalizeBase64, decodedBase64Bytes)
including the required functional markers and tags: add `@pure` and a PURITY
marker, an `@invariant` and INVARIANT marker that states decodedBytes ===
payload.size and that mediaType/fileName are supported, `@precondition` listing
valid inputs (payload.mediaType present and lowercaseable, payload.size is
finite > 0 and ≤ terminalImagePasteMaxBytes, normalizedBase64 length ≤
terminalImagePasteMaxBase64Length), `@postcondition` describing the returned union
(_tag either "ValidTerminalImagePaste" with containerPath, decodedBytes,
normalizedBase64 or "InvalidTerminalImagePaste" with message), and `@complexity`
with O(n) where n is payload.data length; also include a REF or QUOTE(ТЗ) marker
linking to the validation requirements and mention any helper invariants for
terminalImageFileName and decodedBase64Bytes to guide future reviewers.
Comment on lines
+1
to
+3
| export type TerminalWebRuntimeBoundary = { | ||
| readonly _tag: "TerminalWebRuntimeBoundary"; | ||
| }; |
There was a problem hiding this comment.
Несоответствие архитектурным требованиям FCIS и Effect-TS.
Тип назван RuntimeBoundary, но реализован как простой маркер с полем _tag. Согласно coding guidelines, границы между слоями должны реализовываться через Effect-TS Layer pattern с Context.Tag и полноценными сервисами.
Текущая реализация не обеспечивает реальную boundary - это только type-level marker без runtime гарантий разделения эффектов между SHELL и CORE слоями.
🏗️ Рекомендуемая архитектура с Effect Layer pattern
Если это действительно runtime boundary для Web-адаптера, должна быть примерно такая структура:
import { Context, Effect, Layer } from "effect";
/**
* Web Runtime Boundary Service
* PURITY: SHELL
* EFFECT: Effect<TerminalWebCapabilities, WebError, never>
*/
export class TerminalWebRuntimeBoundary extends Context.Tag("TerminalWebRuntimeBoundary")<
TerminalWebRuntimeBoundary,
{
readonly handleWebSocketMessage: (msg: unknown) => Effect.Effect<void, WebError>;
readonly sendToClient: (data: string) => Effect.Effect<void, WebError>;
// ... other Web-specific capabilities
}
>() {}
/**
* Production implementation
*/
export const TerminalWebRuntimeBoundaryLive = Layer.effect(
TerminalWebRuntimeBoundary,
Effect.gen(function* (_) {
return {
handleWebSocketMessage: (msg) => Effect.tryPromise({
try: async () => {
// implementation
},
catch: (error) => new WebError({ cause: error })
}),
// ...
};
})
);As per coding guidelines: "All external service dependencies must be wrapped through typed interfaces and injected via Effect-TS Layer pattern" and "Implement Functional Core, Imperative Shell (FCIS) pattern."
🤖 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/terminal/src/web/index.ts` around lines 1 - 3, The current
TerminalWebRuntimeBoundary is only a type-level marker and must be converted
into a runtime Effect-TS Layer-based service: replace the plain exported type
TerminalWebRuntimeBoundary with a Context.Tag-based service (using
Context.Tag("TerminalWebRuntimeBoundary")) that exposes concrete methods like
handleWebSocketMessage and sendToClient typed to return Effect.Effect, then
provide a production Layer (e.g., TerminalWebRuntimeBoundaryLive) built via
Layer.effect/Effect.gen that implements those methods and wraps runtime
side-effects (websocket send/receive) in Effect/Either-style error handling;
ensure all consumers inject this Layer instead of relying on the marker type so
the boundary enforces runtime separation between SHELL and CORE.
Отсутствует обязательная документация для публичного API.
Экспортируемый тип TerminalWebRuntimeBoundary не содержит ни TSDoc комментариев, ни функциональных комментариев. Согласно coding guidelines, все экспортируемые сущности должны быть задокументированы с описанием назначения, инвариантов и примеров использования.
📝 Предлагаемая документация
+/**
+ * Runtime boundary marker для Web-адаптера терминального пакета.
+ *
+ * CHANGE: Добавлен тип-маркер для разграничения Web runtime boundary
+ * WHY: Обеспечение type-level разделения между CLI, Shell и Web runtime слоями
+ * QUOTE(ТЗ): [требуется ссылка на issue `#361` или спецификацию]
+ * REF: n/a
+ * SOURCE: n/a
+ * FORMAT THEOREM: ∀x: TerminalWebRuntimeBoundary: x._tag = "TerminalWebRuntimeBoundary"
+ * PURITY: CORE
+ * INVARIANT: _tag является string literal константой для type discrimination
+ *
+ * `@remarks`
+ * Этот тип используется для маркировки границ между runtime слоями в FCIS архитектуре.
+ * Конкретное использование: [требуется описание, где и как используется]
+ *
+ * `@example`
+ * ```typescript
+ * // [требуется пример использования]
+ * ```
+ */
export type TerminalWebRuntimeBoundary = {
readonly _tag: "TerminalWebRuntimeBoundary";
};As per coding guidelines: "Functional comments must include: CHANGE, WHY, QUOTE(ТЗ) or n/a, REF, SOURCE or n/a, FORMAT THEOREM, PURITY (CORE|SHELL), INVARIANT, and COMPLEXITY" and "TypeScript functions must include comprehensive TSDoc."
🤖 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/terminal/src/web/index.ts` around lines 1 - 3, The exported type
TerminalWebRuntimeBoundary lacks required TSDoc/functional comments; add a
comprehensive TSDoc block immediately above the exported type
TerminalWebRuntimeBoundary that describes its purpose, invariants, and usage,
includes the mandated fields CHANGE, WHY, QUOTE (or n/a), REF, SOURCE (or n/a),
FORMAT THEOREM, PURITY (CORE|SHELL), INVARIANT, and COMPLEXITY, and provide a
brief usage example showing how to construct/consume the boundary; ensure the
comment follows existing coding-guidelines formatting and covers any invariants
about the _tag literal.
Comment on lines
+1
to
+2
| import { readFileSync, readdirSync } from "node:fs" | ||
| import { join, relative } from "node:path" |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
Необходимо обернуть файловые операции в Effect и улучшить парсинг импортов.
Обнаружены два архитектурных несоответствия:
-
Прямое использование
node:fs/node:path: Файловые операцииreadFileSync,readdirSyncвызываются напрямую без обёртки в Effect. Согласно coding guidelines, все внешние зависимости должны оборачиваться через типизированные интерфейсы и Effect. -
Хрупкий парсинг через regex: Регулярное выражение на строке 23 не обработает корректно многострочные импорты, комментарии с import-подобными строками, динамические импорты и сложные реэкспорты. Для надёжного извлечения импортов следует использовать TypeScript Compiler API или AST-парсер.
♻️ Рекомендуемые улучшения
Решение 1: Обернуть fs-операции в Effect
import { Effect } from "effect"
import { readFileSync, readdirSync } from "node:fs"
const readFile = (path: string): Effect.Effect<string, Error> =>
Effect.try({
try: () => readFileSync(path, "utf8"),
catch: (error) => new Error(`Failed to read ${path}: ${error}`)
})
const listFiles = (directory: string): Effect.Effect<ReadonlyArray<string>, Error> =>
Effect.try({
try: () => {
const entries = readdirSync(directory, { withFileTypes: true })
return entries.flatMap((entry) => {
const path = join(directory, entry.name)
if (entry.isDirectory()) {
return [...sourceFiles(path)]
}
return entry.isFile() && path.endsWith(".ts") ? [path] : []
})
},
catch: (error) => new Error(`Failed to list ${directory}: ${error}`)
})Решение 2: Использовать TypeScript Compiler API для надёжного парсинга
import * as ts from "typescript"
const extractImports = (sourceFile: ts.SourceFile): ReadonlyArray<string> => {
const imports: string[] = []
const visit = (node: ts.Node) => {
if (ts.isImportDeclaration(node) || ts.isExportDeclaration(node)) {
const moduleSpecifier = node.moduleSpecifier
if (moduleSpecifier && ts.isStringLiteral(moduleSpecifier)) {
imports.push(moduleSpecifier.text)
}
}
ts.forEachChild(node, visit)
}
visit(sourceFile)
return imports
}Also applies to: 9-26
🤖 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/terminal/tests/architecture/boundaries.test.ts` around lines 1 - 2,
Tests directly call node:fs/node:path (readFileSync, readdirSync) and rely on a
regex-based import parser; wrap all filesystem access in typed Effect wrappers
(e.g., implement readFile(path): Effect.Effect<string, Error> and
listFiles(directory): Effect.Effect<ReadonlyArray<string>, Error> that
internally call readFileSync/readdirSync) and replace the fragile regex import
extraction with a TypeScript AST-based extractor (e.g., use ts.createSourceFile
and an extractImports(sourceFile: ts.SourceFile) function that traverses nodes
and collects moduleSpecifier texts for ImportDeclaration/ExportDeclaration),
then update the test code to call these new readFile/listFiles/extractImports
helpers instead of direct fs calls and the regex.
Comment on lines
+26
to
+30
| it.effect("sanitizes unsafe image names", () => | ||
| Effect.sync(() => { | ||
| expect(sanitizeTerminalImageBaseName("../..//.bad name!!.png")).toBe("bad-name") | ||
| expect(sanitizeTerminalImageBaseName("////.png")).toBe("clipboard-image") | ||
| })) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Добавьте property-based тест для инварианта санации имени.
Санация имени — защита от path traversal, и для неё есть чёткий математический инвариант: для любого входа результат не содержит /, \ и последовательности ... Текущие два примера не покрывают пространство входов. As per coding guidelines: "Property-based tests (fast-check) must verify mathematical invariants".
♻️ Пример property-based теста
import * as fc from "fast-check"
it.effect("санитизированное имя никогда не содержит разделителей пути", () =>
Effect.sync(() => {
fc.assert(
fc.property(fc.string(), (raw) => {
const name = sanitizeTerminalImageBaseName(raw)
return !name.includes("/") && !name.includes("\\") && !name.includes("..")
})
)
}))🤖 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/terminal/tests/core/image-paste.test.ts` around lines 26 - 30, Add a
property-based test for the sanitizeTerminalImageBaseName invariant: write an
it.effect test (in the same test suite) that uses fast-check (fc.assert +
fc.property(fc.string(), ...)) to feed arbitrary strings to
sanitizeTerminalImageBaseName and assert the returned name never contains "/",
"\\" or the sequence ".." (and choose a descriptive test title like
"санитизированное имя никогда не содержит разделителей пути"); use the existing
Effect.sync wrapper as in other tests and keep the assertion boolean style used
in the repo.
Comment on lines
+22
to
+40
| it.effect("keeps only the newest output when the replay budget is exceeded", () => | ||
| Effect.sync(() => { | ||
| const buffer = appendTerminalOutput( | ||
| appendTerminalOutput(emptyTerminalOutputBuffer, "abcdef", 8), | ||
| "ghij", | ||
| 8 | ||
| ) | ||
|
|
||
| expect(buffer.charLength).toBe(8) | ||
| expect(renderTerminalOutputBuffer(buffer)).toBe("cdefghij") | ||
| })) | ||
|
|
||
| it.effect("trims an oversized chunk to the replay budget", () => | ||
| Effect.sync(() => { | ||
| const buffer = appendTerminalOutput(emptyTerminalOutputBuffer, "0123456789", 4) | ||
|
|
||
| expect(buffer.charLength).toBe(4) | ||
| expect(renderTerminalOutputBuffer(buffer)).toBe("6789") | ||
| })) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Добавьте property-based тест для инварианта бюджета буфера.
Буфер обладает строгим инвариантом: charLength <= budget после любого количества appendTerminalOutput с любыми чанками. Пример-ориентированные кейсы не доказывают это в общем виде. As per coding guidelines: "Property-based tests (fast-check) must verify mathematical invariants".
♻️ Пример property-based теста
import * as fc from "fast-check"
it.effect("длина буфера никогда не превышает бюджет", () =>
Effect.sync(() => {
fc.assert(
fc.property(
fc.array(fc.string()),
fc.integer({ min: 1, max: 256 }),
(chunks, budget) => {
const buffer = chunks.reduce(
(acc, chunk) => appendTerminalOutput(acc, chunk, budget),
emptyTerminalOutputBuffer
)
return buffer.charLength <= budget
}
)
)
}))🤖 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/terminal/tests/core/output-buffer.test.ts` around lines 22 - 40, Add
a property-based test using fast-check that verifies the buffer invariant
charLength <= budget for any sequence of appends: generate an array of strings
and a budget (e.g., 1..256), reduce the chunks with appendTerminalOutput
starting from emptyTerminalOutputBuffer using the generated budget, and assert
buffer.charLength <= budget (wrap in Effect.sync and use fc.assert/fc.property
as in other tests); reference appendTerminalOutput, emptyTerminalOutputBuffer,
and charLength so the test locates the right functions/fields.
Comment on lines
+6
to
+32
| describe("terminal image fetch planning", () => { | ||
| it.effect("accepts absolute supported image paths", () => | ||
| Effect.sync(() => { | ||
| expect(planTerminalImageFetch("/home/dev/image.png")).toEqual({ | ||
| _tag: "ValidTerminalImageFetch", | ||
| containerPath: "/home/dev/image.png", | ||
| mediaType: "image/png" | ||
| }) | ||
| })) | ||
|
|
||
| it.effect("joins relative paths to a valid base directory", () => | ||
| Effect.sync(() => { | ||
| expect(planTerminalImageFetch("screens/shot.webp", { baseDir: "/home/dev/project" })).toEqual({ | ||
| _tag: "ValidTerminalImageFetch", | ||
| containerPath: "/home/dev/project/screens/shot.webp", | ||
| mediaType: "image/webp" | ||
| }) | ||
| })) | ||
|
|
||
| it.effect("rejects traversal paths", () => | ||
| Effect.sync(() => { | ||
| expect(planTerminalImageFetch("/home/dev/../secret.png")).toEqual({ | ||
| _tag: "InvalidTerminalImageFetch", | ||
| message: "Image path must not contain '.' or '..' segments." | ||
| }) | ||
| })) | ||
| }) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Рекомендуется дополнить property-based тестами для проверки инвариантов.
Текущие тесты проверяют конкретные примеры, но не покрывают математические инварианты обработки путей. Согласно coding guidelines, для математических свойств следует использовать fast-check.
Инварианты для проверки:
- Нормализация путей идемпотентна:
normalize(normalize(p)) === normalize(p) - Любой путь с сегментами
..или.→InvalidTerminalImageFetch - Соответствие расширения и mediaType детерминировано для всех валидных путей
🧪 Пример property-based теста с fast-check
import * as fc from "fast-check"
describe("terminal image fetch properties", () => {
it.effect("rejects all paths with traversal segments", () =>
Effect.sync(() => {
fc.assert(
fc.property(
fc.array(fc.constantFrom("..", "."), { minLength: 1 }),
fc.array(fc.stringMatching(/^[a-z0-9-]+$/), { minLength: 1 }),
(traversal, segments) => {
const path = "/" + [...segments, ...traversal, "image.png"].join("/")
const result = planTerminalImageFetch(path)
expect(result._tag).toBe("InvalidTerminalImageFetch")
}
)
)
}))
it.effect("maps extensions consistently to media types", () =>
Effect.sync(() => {
fc.assert(
fc.property(
fc.constantFrom("png", "jpg", "jpeg", "gif", "webp"),
fc.stringMatching(/^\/[a-z0-9/-]+$/),
(ext, basePath) => {
const path = `${basePath}/file.${ext}`
const result1 = planTerminalImageFetch(path)
const result2 = planTerminalImageFetch(path)
expect(result1).toEqual(result2)
}
)
)
}))
})🤖 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/terminal/tests/server/image-fetch.test.ts` around lines 6 - 32, Add
property-based tests using fast-check that exercise planTerminalImageFetch:
assert idempotence of normalization (normalize(normalize(p)) === normalize(p)),
assert any path containing '.' or '..' segments yields an
InvalidTerminalImageFetch, and assert deterministic mapping from file extension
to mediaType for all valid paths; implement these as new it.effect cases calling
fc.assert + fc.property and use planTerminalImageFetch inside the property
functions to check _tag or equality, referencing the existing tests for pattern
and reusing the same error/message expectations.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Source TZ / Issues
Summary
@prover-coder-ai/docker-git-terminalas a workspace package withcontracts,core,server,web,cli, andshellentrypoints plus an Effect-style app shell.Requirements Alignment
packages/apiterminal services now consume shared contracts/helpers instead of owning duplicate schema/core code.packages/appbrowser and CLI terminal clients now share the terminal contract package without publishing terminal as a runtime dependency of the CLI tarball.node-ptyruntime service and the full React/xterm UI intopackages/terminal; those remain in API/app and are now prepared for a follow-up extraction.Verification
bun run --cwd packages/terminal typecheckbun run --cwd packages/terminal buildbun run --cwd packages/terminal testbun run --cwd packages/terminal lintbun run --cwd packages/terminal lint:effectbun run --cwd packages/api typecheckbun run --cwd packages/api lintbun run --cwd packages/api vitest run tests/terminal-output-buffer.test.ts tests/terminal-image-paste-core.test.ts tests/terminal-image-fetch-core.test.tsbun run --cwd packages/app typecheckbun run --cwd packages/app lintbun run --cwd packages/app vitest run tests/docker-git/terminal.test.ts tests/docker-git/terminal-session-client.test.tsbun run --cwd packages/app build:docker-git:reuse-installbash scripts/e2e/local-package-cli.shdocker build -f packages/api/Dockerfile --target workspace-static --build-arg DOCKER_GIT_CONTROLLER_BUILD_SKILLER=0 .bun run buildbun run --cwd packages/app build:web:strictbun run check:dist-deps-prunebun run typecheckbun run testgit diff --check