fix(engine): small correctness fixes (drive-root target, concurrency bounds, process cleanup, activity log) #151

New Issue

2026-11-10T20:56:51Z

james.maes commented

2026-11-10 20:56:51 +00:00

A batch of small, independent engine defects found in the 1.1.1 review. Each gets a regression test.

A target folder that is a drive root (C:\) is rejected for every repository: the root keeps its separator and a second one is appended.
A concurrency value read from a saved account is used as stored; a value of zero or less, or far above the supported maximum, is not brought back into range. The engine's own handling of such a value has no test.
The process runner writes the child's input outside the block that cleans the child up, so a failure there can leave the child running, and a child that exits before reading its input fails the call with an I/O error instead of its exit code and error output.
The Windows-invalid character set is taken from the running platform, so the Windows rules answer differently off Windows. It should be the fixed Windows set.
SyncSummary.WasCanceled reports the token state at the end of the run, so a cancellation that arrives after the last repository finished marks a complete run as canceled.
The invalid-paths message always ends with "Nothing was checked out.", which is not the right wording for a repository that already has a working tree.
The GitHub gateway cache keeps clients for the life of the process and never disposes them.
The activity log: timestamps carry no offset, the clock is read twice per line, old log files are never removed, and two processes writing at once can drop lines.

Part of the 1.1.1 review.

A batch of small, independent engine defects found in the 1.1.1 review. Each gets a regression test. - A target folder that is a drive root (`C:\`) is rejected for every repository: the root keeps its separator and a second one is appended. - A concurrency value read from a saved account is used as stored; a value of zero or less, or far above the supported maximum, is not brought back into range. The engine's own handling of such a value has no test. - The process runner writes the child's input outside the block that cleans the child up, so a failure there can leave the child running, and a child that exits before reading its input fails the call with an I/O error instead of its exit code and error output. - The Windows-invalid character set is taken from the running platform, so the Windows rules answer differently off Windows. It should be the fixed Windows set. - `SyncSummary.WasCanceled` reports the token state at the end of the run, so a cancellation that arrives after the last repository finished marks a complete run as canceled. - The invalid-paths message always ends with "Nothing was checked out.", which is not the right wording for a repository that already has a working tree. - The GitHub gateway cache keeps clients for the life of the process and never disposes them. - The activity log: timestamps carry no offset, the clock is read twice per line, old log files are never removed, and two processes writing at once can drop lines. Part of the 1.1.1 review.

james.maes referenced this issue from a commit

2026-11-10 21:07:58 +00:00

fix(engine): small correctness fixes found in the 1.1.1 review

james.maes referenced a pull request that will close this issue

2026-11-10 21:08:14 +00:00

fix(engine): small correctness fixes found in the 1.1.1 review #152

james.maes referenced this issue from a commit

2026-11-10 21:32:34 +00:00

fix(engine): address the independent review of the small fixes

james.maes referenced this issue from a commit

2026-11-10 21:32:36 +00:00

fix(engine): small correctness fixes found in the 1.1.1 review (#152)