Skip to content

Commit 1ea5543

Browse files
committed
fix: address code review findings on update lifecycle and token caps
- Centralize the remaining-context-window clamp in computeCompletionBudgetCap so every provider benefits, and pass usedContextTokens from compaction — the near-full request the cap exists for. Remove the per-provider MaxCompletionTokensOptions duplication from kosong. - Clear the pending update record once the activation failure limit is reached so startup and /update stop reporting a dead update forever. - Keep lastFailure across a successful prepare so invalid-artifact activations accumulate toward the failure threshold. - Retain a verified pending update while a newer preparation runs. - Scope failure-attempt increments by operation; record install failures with operation: 'install'. - Return whether background prepare/install actually started and map a refused start to 'in-progress' instead of claiming 'started'. - Make writeJsonFile fsync opt-in ({ durable: true }, install.json only). - Dedupe formatErrorMessage into cli/update/format-error.ts; normalize appendLog chunks instead of duplicated branches; z.uuid(); source-neutral update-preference wording; align pythinker upgrade docs; skip the POSIX-only detached-helper test on Windows; exact helper spawn asserts.
1 parent 2bb9e9d commit 1ea5543

22 files changed

Lines changed: 449 additions & 171 deletions

File tree

apps/pythinker-code/src/cli/update/activation.ts

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
activateHomebrewUpdate,
77
PreparedHomebrewUpdateInvalidError,
88
} from './homebrew';
9+
import { formatErrorMessage } from './format-error';
910
import { tryAcquireUpdateInstallLock, type UpdateInstallLockHandle } from './install-lock';
1011
import { readUpdateInstallState, writeUpdateInstallState } from './install-state';
1112
import { detectInstallSource } from './source';
@@ -52,10 +53,6 @@ function activationAttempts(state: UpdateInstallState, version: string): number
5253
return failure?.version === version && failure.operation === 'activate' ? failure.attempts : 0;
5354
}
5455

55-
function errorMessage(error: unknown): string {
56-
return error instanceof Error ? error.message : String(error);
57-
}
58-
5956
function isRunningPreparedVersion(currentVersion: string, preparedVersion: string): boolean {
6057
return (
6158
valid(currentVersion) !== null &&
@@ -77,6 +74,11 @@ export async function activatePendingUpdate(
7774
return { status: 'none' as const };
7875
}
7976

77+
if (await deps.detectSource() !== pending.source) {
78+
await deps.writeState({ ...state, active: null, pending: null });
79+
return { status: 'invalidated' as const, version: pending.version };
80+
}
81+
8082
if (isRunningPreparedVersion(currentVersion, pending.version)) {
8183
const installedAt = deps.now().toISOString();
8284
await deps.writeState({
@@ -92,12 +94,10 @@ export async function activatePendingUpdate(
9294
return { status: 'finalized' as const, version: currentVersion };
9395
}
9496

95-
if (await deps.detectSource() !== pending.source) {
96-
await deps.writeState({ ...state, active: null, pending: null });
97-
return { status: 'invalidated' as const, version: pending.version };
98-
}
99-
10097
if (activationAttempts(state, pending.version) >= ACTIVATION_FAILURE_LIMIT) {
98+
// Terminal: drop the pending record (keeping lastFailure for preflight)
99+
// so later launches stop retrying and reporting an in-progress update.
100+
await deps.writeState({ ...state, pending: null });
101101
return {
102102
status: 'failed' as const,
103103
version: pending.version,
@@ -116,7 +116,7 @@ export async function activatePendingUpdate(
116116
...state,
117117
active: {
118118
version: pending.version,
119-
source: 'homebrew',
119+
source: pending.source,
120120
operation: 'activate',
121121
jobId: pending.jobId,
122122
startedAt,
@@ -127,22 +127,34 @@ export async function activatePendingUpdate(
127127

128128
try {
129129
const activated = await deps.activateHomebrew(pending);
130+
await deps.writeState({
131+
...activatingState,
132+
active: null,
133+
lastFailure: null,
134+
});
130135
return {
131136
status: 'activated' as const,
132137
version: activated.version,
133138
executable: activated.executable,
134139
};
135140
} catch (error) {
136-
const message = errorMessage(error);
141+
const message = formatErrorMessage(error);
137142
if (error instanceof PreparedHomebrewUpdateInvalidError) {
143+
// Carry the cumulative prepare-failure count so repeated invalid
144+
// artifacts can reach the auto-install failure threshold.
145+
const priorFailure = activatingState.lastFailure;
146+
const prepareAttempts =
147+
priorFailure?.version === pending.version && priorFailure.operation === 'prepare'
148+
? priorFailure.attempts + 1
149+
: 1;
138150
await deps.writeState({
139151
...activatingState,
140152
active: null,
141153
pending: null,
142154
lastFailure: {
143155
version: pending.version,
144156
failedAt: deps.now().toISOString(),
145-
attempts: 1,
157+
attempts: prepareAttempts,
146158
operation: 'prepare',
147159
message,
148160
},
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
/** Shared failure-message formatter for update install/prepare/activate state. */
2+
export function formatErrorMessage(error: unknown): string {
3+
return error instanceof Error ? error.message : String(error);
4+
}

apps/pythinker-code/src/cli/update/homebrew.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,10 +107,12 @@ export async function runHomebrewCommand(
107107
let logWrites = Promise.resolve();
108108
const appendLog = (chunk: string | Uint8Array): void => {
109109
if (logFile === undefined) return;
110+
// Normalize to bytes: FileHandle.write has separate string/buffer
111+
// overloads that reject the union type.
112+
const data = typeof chunk === 'string' ? Buffer.from(chunk) : chunk;
110113
logWrites = logWrites
111114
.then(async () => {
112-
if (typeof chunk === 'string') await logFile.write(chunk);
113-
else await logFile.write(chunk);
115+
await logFile.write(data);
114116
})
115117
.catch(() => {});
116118
};

apps/pythinker-code/src/cli/update/install-state.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,13 @@ const UpdateInstallStateSchema: z.ZodType<UpdateInstallState> = z
2727
startedAt: z.string().min(1),
2828
pid: z.number().int().positive().optional(),
2929
operation: UpdateInstallOperationSchema.optional(),
30-
jobId: z.string().uuid().optional(),
30+
jobId: z.uuid().optional(),
3131
})
3232
.strict()
3333
.nullable(),
3434
pending: z
3535
.object({
36-
jobId: z.string().uuid(),
36+
jobId: z.uuid(),
3737
source: z.literal('homebrew'),
3838
version: z.string().min(1),
3939
preparedAt: z.string().min(1),
@@ -84,5 +84,5 @@ export async function writeUpdateInstallState(
8484
value: UpdateInstallState,
8585
filePath: string = getUpdateInstallStateFile(),
8686
): Promise<void> {
87-
await writeJsonFile(filePath, UpdateInstallStateSchema, value);
87+
await writeJsonFile(filePath, UpdateInstallStateSchema, value, { durable: true });
8888
}

0 commit comments

Comments
 (0)