Skip to content

Commit 834b994

Browse files
committed
fix(app): address coderabbit full review
1 parent 4b079bb commit 834b994

5 files changed

Lines changed: 335 additions & 22 deletions

File tree

.github/actions/free-docker-disk/action.yml

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,16 @@ runs:
1717
1818
threshold_gib="${DOCKER_GIT_FREE_DOCKER_DISK_MIN_AVAILABLE_GIB:-40}"
1919
force_cleanup="${DOCKER_GIT_FORCE_FREE_DOCKER_DISK:-0}"
20+
force_cleanup_normalized="${force_cleanup,,}"
2021
force_cleanup_enabled=0
2122
2223
if [[ ! "$threshold_gib" =~ ^[0-9]+$ ]]; then
2324
echo "Invalid DOCKER_GIT_FREE_DOCKER_DISK_MIN_AVAILABLE_GIB: $threshold_gib" >&2
2425
exit 1
2526
fi
2627
27-
case "$force_cleanup" in
28-
1|true|TRUE|yes|YES|on|ON)
28+
case "$force_cleanup_normalized" in
29+
1|true|yes|on)
2930
force_cleanup_enabled=1
3031
;;
3132
*)

packages/app/src/docker-git/controller-image-revision.ts

Lines changed: 90 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -9,47 +9,119 @@ const inspectControllerRevisionLabelTemplate = String
99
.raw`{{ index .Config.Labels "io.prover-coder-ai.docker-git.controller-rev" }}`
1010

1111
/**
12-
* Returns the first non-empty line from Docker CLI output.
12+
* Builds a typed controller bootstrap error.
13+
*
14+
* @param message - Human-readable bootstrap failure message.
15+
* @returns Controller bootstrap error value.
16+
*
17+
* @pure true
18+
* @effect n/a
19+
* @invariant Returned error tag is always `ControllerBootstrapError`.
20+
* @precondition `message` is a finite string.
21+
* @postcondition The returned error preserves the provided message.
22+
* @complexity O(1) time and O(1) space.
23+
* @throws Never
24+
*/
25+
// CHANGE: represent deterministic image-resolution failures as typed bootstrap errors
26+
// WHY: ambiguous compose image output must fail through the Effect error channel
27+
// QUOTE(ТЗ): "хочу сузить время билда докер контейнера"
28+
// REF: user-request-2026-05-22-controller-build-speed
29+
// SOURCE: n/a
30+
// FORMAT THEOREM: error(message).message = message
31+
// PURITY: CORE
32+
// EFFECT: n/a
33+
// INVARIANT: error tag is stable
34+
// COMPLEXITY: O(1)
35+
const controllerBootstrapError = (message: string): ControllerBootstrapError => ({
36+
_tag: "ControllerBootstrapError",
37+
message
38+
})
39+
40+
/**
41+
* Returns all non-empty lines from Docker CLI output.
1342
*
1443
* @param output - Raw command output.
15-
* @returns The first trimmed non-empty line, or null when none exists.
44+
* @returns Trimmed non-empty output lines.
1645
*
1746
* @pure true
1847
* @effect n/a
19-
* @invariant Result is either null or a string with length > 0.
48+
* @invariant Every returned line has length > 0.
2049
* @precondition `output` is a finite string.
2150
* @postcondition Whitespace-only lines are ignored.
2251
* @complexity O(n) time and O(n) space where n = |output|.
2352
* @throws Never
2453
*/
2554
// CHANGE: normalize compose image output before image inspection
26-
// WHY: docker compose config --images emits line-oriented output and bootstrap needs one image name proof
55+
// WHY: docker compose config --images emits line-oriented output and bootstrap needs a deterministic image proof
2756
// QUOTE(ТЗ): "контейнер собирается минут 5-6"
2857
// REF: user-request-2026-05-22-controller-build-speed
2958
// SOURCE: n/a
30-
// FORMAT THEOREM: exists first non-empty line -> result = trim(first)
59+
// FORMAT THEOREM: result = map(trim, lines(output)) filtered by non-empty
3160
// PURITY: CORE
3261
// EFFECT: n/a
33-
// INVARIANT: result is null or non-empty
62+
// INVARIANT: every result entry is non-empty
3463
// COMPLEXITY: O(n)
35-
const firstNonEmptyLine = (output: string): string | null => {
36-
for (const line of output.split(/\r?\n/u)) {
37-
const trimmed = line.trim()
38-
if (trimmed.length > 0) {
39-
return trimmed
40-
}
64+
const nonEmptyLines = (output: string): ReadonlyArray<string> => {
65+
const lines = output.split(/\r?\n/u)
66+
return lines
67+
.map((line) => line.trim())
68+
.filter((line) => line.length > 0)
69+
}
70+
71+
/**
72+
* Resolves compose image output into exactly one controller image name.
73+
*
74+
* @param output - Raw `docker compose config --images` output.
75+
* @returns Effect with the single image, null for empty output, or a typed bootstrap error for ambiguity.
76+
*
77+
* @pure true
78+
* @effect Effect.succeed | Effect.fail
79+
* @invariant More than one non-empty line is rejected as ambiguous.
80+
* @precondition `output` is finite Docker CLI output.
81+
* @postcondition Success with a string implies exactly one non-empty image line existed.
82+
* @complexity O(n) time and O(n) space where n = |output|.
83+
* @throws Never - ambiguity is represented in the Effect error channel.
84+
*/
85+
// CHANGE: require deterministic controller image resolution from compose output
86+
// WHY: revision reuse is sound only when the inspected image is uniquely the controller image
87+
// QUOTE(ТЗ): "хочу сузить время билда докер контейнера"
88+
// REF: user-request-2026-05-22-controller-build-speed
89+
// SOURCE: n/a
90+
// FORMAT THEOREM: |images| = 0 -> null, |images| = 1 -> images[0], |images| > 1 -> error
91+
// PURITY: CORE
92+
// EFFECT: Effect<string | null, ControllerBootstrapError>
93+
// INVARIANT: multiple compose images never collapse to the first image
94+
// COMPLEXITY: O(n) where n = |output|
95+
const resolveSingleControllerImageName = (
96+
output: string
97+
): Effect.Effect<string | null, ControllerBootstrapError> => {
98+
const imageNames = nonEmptyLines(output)
99+
if (imageNames.length === 0) {
100+
return Effect.succeed(null)
41101
}
42-
return null
102+
const imageName = imageNames[0]
103+
if (imageNames.length === 1 && imageName !== undefined) {
104+
return Effect.succeed(imageName)
105+
}
106+
return Effect.fail(
107+
controllerBootstrapError(
108+
[
109+
"Expected exactly one docker-git controller image from docker compose config --images.",
110+
"Resolved images:",
111+
...imageNames.map((name) => `- ${name}`)
112+
].join("\n")
113+
)
114+
)
43115
}
44116

45117
/**
46118
* Resolves the Docker image name configured for the active controller compose files.
47119
*
48-
* @returns The first compose image name, or null when compose emits no images.
120+
* @returns The single compose image name, or null when compose emits no images.
49121
*
50122
* @pure false
51123
* @effect Docker CLI through ControllerRuntime.
52-
* @invariant Empty compose output is represented as null.
124+
* @invariant Multiple compose images fail rather than selecting the first line.
53125
* @precondition Compose files resolve for the current GPU mode.
54126
* @postcondition Returned image name is trimmed and non-empty.
55127
* @complexity O(1) compose invocations.
@@ -60,10 +132,10 @@ const firstNonEmptyLine = (output: string): string | null => {
60132
// QUOTE(ТЗ): "хочу сузить время билда докер контейнера"
61133
// REF: user-request-2026-05-22-controller-build-speed
62134
// SOURCE: n/a
63-
// FORMAT THEOREM: compose_image = null -> image_revision = null
135+
// FORMAT THEOREM: |compose_images| <= 1 or bootstrap fails
64136
// PURITY: SHELL
65137
// EFFECT: Effect<string | null, ControllerBootstrapError, ControllerRuntime>
66-
// INVARIANT: no image name is treated as missing revision proof
138+
// INVARIANT: ambiguous image lists are typed bootstrap errors
67139
// COMPLEXITY: O(1) Docker compose invocations
68140
const inspectControllerComposeImageName = (): Effect.Effect<
69141
string | null,
@@ -84,7 +156,7 @@ const inspectControllerComposeImageName = (): Effect.Effect<
84156
)
85157
)
86158

87-
return firstNonEmptyLine(output)
159+
return yield* _(resolveSingleControllerImageName(output))
88160
})
89161

90162
/**

packages/app/src/docker-git/menu-create-inputs.ts

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,16 @@ import type { CreateInputs } from "./menu-types.js"
1717
* @complexity O(n) time and O(n) space where n = |value|.
1818
* @throws Never
1919
*/
20+
// CHANGE: normalize leading separators on path fragments before joining
21+
// WHY: repository path parts must not reset the selected projects root
22+
// QUOTE(ТЗ): n/a
23+
// REF: issue-339
24+
// SOURCE: n/a
25+
// FORMAT THEOREM: forall s: trimLeftSlash(s) has no leading slash unless empty
26+
// PURITY: CORE
27+
// EFFECT: n/a
28+
// INVARIANT: result length never exceeds input length
29+
// COMPLEXITY: O(n) where n = |value|
2030
const trimLeftSlash = (value: string): string => {
2131
let start = 0
2232
while (start < value.length && value[start] === "/") {
@@ -39,6 +49,16 @@ const trimLeftSlash = (value: string): string => {
3949
* @complexity O(n) time and O(n) space where n = |value|.
4050
* @throws Never
4151
*/
52+
// CHANGE: normalize trailing separators on path fragments before joining
53+
// WHY: joined create-flow paths should not contain duplicate separators at boundaries
54+
// QUOTE(ТЗ): n/a
55+
// REF: issue-339
56+
// SOURCE: n/a
57+
// FORMAT THEOREM: forall s: trimRightSlash(s) has no trailing slash unless empty
58+
// PURITY: CORE
59+
// EFFECT: n/a
60+
// INVARIANT: result length never exceeds input length
61+
// COMPLEXITY: O(n) where n = |value|
4262
const trimRightSlash = (value: string): string => {
4363
let end = value.length
4464
while (end > 0 && value[end - 1] === "/") {
@@ -61,6 +81,16 @@ const trimRightSlash = (value: string): string => {
6181
* @complexity O(p + n) time and O(p + n) space where p = |parts| and n = total input length.
6282
* @throws Never
6383
*/
84+
// CHANGE: join create-flow path fragments while preserving absolute roots
85+
// WHY: browser-provided projectsRoot="/" must produce /owner/repo rather than a relative path
86+
// QUOTE(ТЗ): "Потеря абсолютного корня в joinPath при \"/\""
87+
// REF: CodeRabbit PR #344 review
88+
// SOURCE: n/a
89+
// FORMAT THEOREM: parts[0] = "/" -> joinPath(parts) startsWith "/"
90+
// PURITY: CORE
91+
// EFFECT: n/a
92+
// INVARIANT: non-root fragments cannot introduce duplicate boundary separators
93+
// COMPLEXITY: O(p + n) where p = |parts| and n = total input length
6494
const joinPath = (...parts: ReadonlyArray<string>): string => {
6595
const cleaned = parts
6696
.filter((part) => part.length > 0)
@@ -127,6 +157,16 @@ export const normalizeCreateFlowContext = (
127157
* @complexity O(n) time and O(n) space where n = |context.projectsRoot ?? context.cwd|.
128158
* @throws Never
129159
*/
160+
// CHANGE: select explicit browser projectsRoot before cwd-derived defaults
161+
// WHY: browser create-flow must honor server-provided workspace root
162+
// QUOTE(ТЗ): n/a
163+
// REF: issue-339
164+
// SOURCE: n/a
165+
// FORMAT THEOREM: trim(projectsRoot) != "" -> result = projectsRoot
166+
// PURITY: CORE
167+
// EFFECT: n/a
168+
// INVARIANT: non-blank projectsRoot has precedence over cwd defaults
169+
// COMPLEXITY: O(n) where n = |context.projectsRoot ?? context.cwd|
130170
const resolveProjectsRoot = (context: CreateFlowContext): string =>
131171
context.projectsRoot?.trim().length
132172
? context.projectsRoot

packages/app/tests/docker-git/create-flow-test-helpers.ts

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,34 @@ export const repositoryCreateInputArbitrary = fc.record({
3939
: `https://github.com/${owner}/${repo}/tree/${branch}`
4040
}))
4141

42-
export const expectedOutDirForRepoUrl = (repoUrl: string, projectsRoot: string): string =>
43-
`${projectsRoot}/${deriveRepoPathParts(resolveRepoInput(repoUrl).repoUrl).pathParts.join("/")}`
42+
/**
43+
* Resolves the expected create-flow output directory for a generated repo URL.
44+
*
45+
* @param repoUrl - Generated GitHub repository URL accepted by create-flow parsing.
46+
* @param projectsRoot - Browser projects root used as the output directory base.
47+
* @returns Expected POSIX output directory for the repository.
48+
* @pure true
49+
* @effect n/a
50+
* @invariant Root projectsRoot "/" is preserved as an absolute path prefix.
51+
* @precondition `repoUrl` and `projectsRoot` are finite strings.
52+
* @postcondition The result contains the derived repo path parts in order.
53+
* @complexity O(n) time and O(n) space where n = |repoUrl|.
54+
* @throws Never
55+
*/
56+
// CHANGE: preserve absolute root projectsRoot in generated create-flow expectations
57+
// WHY: property tests must assert "/" maps to /owner/repo, not //owner/repo
58+
// QUOTE(ТЗ): "Потеря абсолютного корня в joinPath при \"/\""
59+
// REF: CodeRabbit PR #344 review
60+
// SOURCE: n/a
61+
// FORMAT THEOREM: projectsRoot = "/" -> result startsWith "/"
62+
// PURITY: CORE
63+
// EFFECT: n/a
64+
// INVARIANT: root projectsRoot remains absolute
65+
// COMPLEXITY: O(n) where n = |repoUrl|
66+
export const expectedOutDirForRepoUrl = (repoUrl: string, projectsRoot: string): string => {
67+
const repoPath = deriveRepoPathParts(resolveRepoInput(repoUrl).repoUrl).pathParts.join("/")
68+
return projectsRoot === "/" ? `/${repoPath}` : `${projectsRoot}/${repoPath}`
69+
}
4470

4571
export const expectCreateContinueView = (
4672
next: ReturnType<typeof advanceCreateFlow>

0 commit comments

Comments
 (0)