Skip to content

Commit 49548fb

Browse files
authored
fix: print-mode telemetry, upgrade -y, telemetry dedup, staleGuard removal, goal budgets, media names (#307)
## Related Issue No tracked issue — internal reconciliation pass. Stacked on #306; review that one first. ## Problem Six independent defects and small features, none of which were reachable from the CLI or server as shipped: - Print mode (`pythinker -p`) ignored `PYTHINKER_DISABLE_TELEMETRY` and never attached a sink to the client the process-wide crash handlers report through, so print-mode crashes went unreported. - `pythinker upgrade` could only be answered interactively, so scripts and non-interactive shells were shown the manual install command instead. - Every session open was counted twice in telemetry: the harness and the engine each emitted `session_started`, and the provider/reload commands mirrored the engine's `model_switch`. - The `staleGuard` feature had no consumers but still owned a replayable state key and two wire records. - Goal time budgets were capped at 24 hours and kept accruing while the session was closed, so a long goal could return already over budget without doing any work. - Image and video filenames were dropped: the gateway re-derived a name from the mime type or the stored file. ## What changed - **Print telemetry** — the config value routes through `shouldEnableTelemetry` (which honours the env var), and the v1 pipeline is initialized beside the v2 cloud appender so crash handlers have a sink. The session id and resolved model are reconciled once known. - **`upgrade -y/--yes`** — skips the confirmation and installs directly. The existing native-install path keeps its own no-confirmation behaviour. - **Telemetry dedup** — harness-assembled clients drop the engine's `session_started`; direct `SDKRpcClientV2` consumers keep it, since it is their only producer. `activateModelAfterLogin` now reports whether the engine already tracked the switch, so the command layer stays the producer for exactly the paths it does not. - **`staleGuard` removed** — replay still works for wires that carry the retired records: the dispatcher skips them without a warning, and the visualizer declares both record shapes locally. - **Goal budgets** — no upper duration limit, time accrues only while the agent is live, and an active goal pauses when its agent closes so resume continues with the remainder. - **Media names** — image and video parts carry an optional name through the wire contract; an unsupported-format image keeps that name in the notice and in the file written to the attachments directory. Two things worth a reviewer's attention: - `v1↔v2` goal parity now pins one deliberate difference. v2 reports `Paused after agent closed`; v1 reports `Paused after agent resume` because agent-core v1 has no agent close hook to attach the behaviour to. Every other field still compares in full, and each engine's own reason is asserted explicitly. - The `dist-web` bundle is restaged in the media-names commit because `packages/protocol` and `packages/transcript` are web build inputs. Only `.web-bundle-manifest.json` changed. ## Checklist - [x] I have read the [CONTRIBUTING](https://github.com/PyModel/pythinker-code/blob/main/CONTRIBUTING.md) document. - [ ] I have linked a related issue (external PRs: the issue must have a maintainer's `/approve`). - [x] I have added tests that prove my feature works. - [x] Ran `gen-changesets` skill, or this PR needs no changeset. - [x] Ran `gen-docs` skill, or this PR needs no doc update. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added `-y, --yes` to install upgrades without confirmation. * Preserved image and video filenames across prompts, sessions, and history. * **Bug Fixes** * Goal timers now pause while sessions are closed and resume with remaining time. * Removed the 24-hour limit on goal time budgets. * Print mode now respects telemetry disable settings. * Improved session reload, model-switch tracking, telemetry, and compatibility with older sessions. * **Documentation** * Updated goal timing and upgrade command documentation. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent 3c21d16 commit 49548fb

70 files changed

Lines changed: 1024 additions & 961 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@pymodel/pythinker-code": patch
3+
---
4+
5+
Exclude time spent with the session closed from goal time budgets.
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@pymodel/pythinker-code": patch
3+
---
4+
5+
Preserve image and video filenames in session history.
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@pymodel/pythinker-code": patch
3+
---
4+
5+
Fix print mode (`pythinker -p`) ignoring the `PYTHINKER_DISABLE_TELEMETRY` environment variable.

.changeset/remove-goal-time-cap.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@pymodel/pythinker-code": patch
3+
---
4+
5+
Remove the 24-hour limit on goal time budgets.

.changeset/update-yes-flag.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@pymodel/pythinker-code": patch
3+
---
4+
5+
Add `-y, --yes` to `pythinker upgrade` (alias `pythinker update`) to skip the confirmation prompt and install the update directly.
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
{
2-
"sourceHash": "3856336183464397291f1fe4b79cf621f9251cfe1d579884874392027f2c376c",
2+
"sourceHash": "0670132fd6ebf38fbe6620f3f406ab07bcd253ce8fe8c89e34558d03fd2a4d0f",
33
"sourceFileCount": 493
44
}

apps/pythinker-code/src/cli/commands.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import { registerWebCommand } from './sub/web';
1313

1414
export type MainCommandHandler = (opts: CLIOptions) => void;
1515
export type PluginNodeRunnerHandler = (entry: string, args: readonly string[]) => void;
16-
export type UpgradeCommandHandler = () => void | Promise<void>;
16+
export type UpgradeCommandHandler = (yes: boolean) => void | Promise<void>;
1717
export type UpdateDownloadHandler = (version: string, manual: boolean) => void;
1818

1919
export function createProgram(
@@ -27,6 +27,7 @@ export function createProgram(
2727
.description('The Starting Point for Next-Gen Agents')
2828
.version(version, '-V, --version')
2929
.allowUnknownOption(false)
30+
.enablePositionalOptions()
3031
.configureHelp({ helpWidth: 100 })
3132
.helpOption('-h, --help', 'Show help.')
3233
.usage('[options] [command]')
@@ -125,8 +126,9 @@ export function createProgram(
125126
.command('upgrade')
126127
.alias('update')
127128
.description('Upgrade Pythinker Code to the latest version.')
128-
.action(async () => {
129-
await onUpgrade();
129+
.option('-y, --yes', 'Skip the confirmation prompt and install the update directly.', false)
130+
.action(async (options: { yes?: boolean }) => {
131+
await onUpgrade(options.yes === true);
130132
});
131133

132134
program

apps/pythinker-code/src/cli/sub/upgrade.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ export interface UpgradeDeps {
4444
readonly stdout: WritableLike;
4545
readonly stderr: WritableLike;
4646
readonly isInteractive: boolean;
47+
readonly yes: boolean;
4748
readonly track: UpgradeTrack;
4849
readonly logger: UpgradeLogger;
4950
}
@@ -87,7 +88,7 @@ export async function handleUpgrade(
8788
const source = await deps.detectInstallSource().catch(() => 'unsupported' as const);
8889
const installCommand = installCommandFor(source, target.version, deps.platform);
8990
const needsConfirmation = source !== 'native';
90-
if (!canAutoInstall(source, deps.platform) || (!deps.isInteractive && needsConfirmation)) {
91+
if (!canAutoInstall(source, deps.platform) || (!deps.yes && !deps.isInteractive && needsConfirmation)) {
9192
trackUpgradeEvent(deps.track, 'upgrade_command_manual_command', {
9293
current_version: currentVersion,
9394
target_version: target.version,
@@ -102,7 +103,7 @@ export async function handleUpgrade(
102103
return 0;
103104
}
104105

105-
if (deps.isInteractive) {
106+
if (!deps.yes && deps.isInteractive) {
106107
trackUpgradeEvent(deps.track, 'upgrade_command_prompted', {
107108
current_version: currentVersion,
108109
target_version: target.version,
@@ -185,6 +186,7 @@ function createDefaultUpgradeDeps(overrides: Partial<UpgradeDeps>): UpgradeDeps
185186
stdout: overrides.stdout ?? process.stdout,
186187
stderr: overrides.stderr ?? process.stderr,
187188
isInteractive: overrides.isInteractive ?? (process.stdin.isTTY && process.stdout.isTTY),
189+
yes: overrides.yes ?? false,
188190
track: overrides.track ?? trackTelemetry,
189191
logger: overrides.logger ?? log,
190192
};

apps/pythinker-code/src/cli/v2/run-v2-print.ts

Lines changed: 40 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,14 @@ import {
6464
resolveMcpJsonPaths,
6565
} from '@pymodel/agent-core-v2/app/mcpConfig/configLoader';
6666
import { createPythinkerDefaultHeaders, createPythinkerDeviceId } from '@pymodel/pythinker-code-oauth';
67+
import {
68+
initializeTelemetry,
69+
setCrashPhase,
70+
setTelemetryContext,
71+
setTelemetryModel,
72+
shouldEnableTelemetry,
73+
shutdownTelemetry,
74+
} from '@pymodel/pythinker-telemetry';
6775
import type { GoalUpdated } from '@pymodel/agent-core-v2/features/goal/goalOps';
6876
import type { TurnEnded } from '@pymodel/agent-core-v2/agent/loop/turnOps';
6977
import type {
@@ -85,6 +93,7 @@ import {
8593
CLI_USER_AGENT_PRODUCT,
8694
PROMPT_CLEANUP_TIMEOUT_MS,
8795
} from '#/constant/app';
96+
import { currentPythinkerProfile } from '#/utils/region';
8897

8998
import {
9099
formatGoalSummaryText,
@@ -174,12 +183,13 @@ export async function runV2Print(
174183
// user left unset are filled, in the memory layer.
175184
await applyPrintModeConfigDefaults(configService);
176185
const defaultModel = configService.get<string>('defaultModel') ?? undefined;
177-
let telemetryEnabled: boolean;
186+
let configTelemetryEnabled: boolean;
178187
try {
179-
telemetryEnabled = configService.get('telemetry') !== false;
188+
configTelemetryEnabled = configService.get('telemetry') !== false;
180189
} catch {
181-
telemetryEnabled = true;
190+
configTelemetryEnabled = true;
182191
}
192+
const telemetryEnabled = shouldEnableTelemetry({ enabled: configTelemetryEnabled });
183193
for (const diagnostic of configService.diagnostics()) {
184194
if (diagnostic.severity === 'warning') {
185195
stderr.write(`Warning: ${diagnostic.message}\n`);
@@ -193,13 +203,18 @@ export async function runV2Print(
193203
const cleanup = async (): Promise<void> => {
194204
const pending = (cleanupPromise ??= (async () => {
195205
removeTerminationCleanup?.();
206+
setCrashPhase('shutdown');
196207
try {
197208
await restorePermission();
198209
} finally {
199-
if (telemetryService !== undefined) {
200-
await raceWithTimeout(telemetryService.shutdown(), CLI_SHUTDOWN_TIMEOUT_MS);
210+
try {
211+
if (telemetryService !== undefined) {
212+
await raceWithTimeout(telemetryService.shutdown(), CLI_SHUTDOWN_TIMEOUT_MS);
213+
}
214+
} finally {
215+
await shutdownTelemetry({ timeoutMs: CLI_SHUTDOWN_TIMEOUT_MS }).catch(() => {});
216+
app.dispose();
201217
}
202-
app.dispose();
203218
}
204219
})());
205220
await raceWithTimeout(pending, PROMPT_CLEANUP_TIMEOUT_MS);
@@ -211,7 +226,10 @@ export async function runV2Print(
211226
// `session_load_failed` fire inside create()/resume(), so an appender wired
212227
// up only after resolveNativeSession() would drop them to the null appender.
213228
// The model below is the best known up front; a resumed session's real
214-
// model is reconciled via setContext once resolved.
229+
// model is reconciled once resolved (v2 via setContext, v1 via
230+
// setTelemetryModel). The v1 pipeline is initialized here too: the
231+
// process-wide crash handlers installed in main() report through its
232+
// default client, so its sink must be attached before the run can crash.
215233
telemetryService = app.accessor.get(ITelemetryService);
216234
if (telemetryEnabled) {
217235
telemetryService.setAppender(
@@ -222,6 +240,17 @@ export async function runV2Print(
222240
model: opts.model ?? defaultModel,
223241
}),
224242
);
243+
// No `first_launch` on the v1 client: the v2 side already tracks it via
244+
// `telemetryService.track2` below, so tracking here would double-send.
245+
initializeTelemetry({
246+
homeDir,
247+
deviceId,
248+
appName: CLI_USER_AGENT_PRODUCT,
249+
version,
250+
uiMode: PROMPT_UI_MODE,
251+
model: opts.model ?? defaultModel,
252+
endpoint: () => currentPythinkerProfile().telemetryEndpoint,
253+
});
225254
}
226255

227256
try {
@@ -235,6 +264,9 @@ export async function runV2Print(
235264
restorePermission = resolved.restorePermission;
236265

237266
telemetryService.setContext({ sessionId: resolved.session.id, model: resolved.telemetryModel });
267+
setTelemetryContext({ sessionId: resolved.session.id });
268+
setTelemetryModel(resolved.telemetryModel);
269+
setCrashPhase('runtime');
238270
if (firstLaunch) {
239271
telemetryService.track2('first_launch');
240272
}
@@ -318,7 +350,7 @@ export function formatTrustGatedMcpWarning(servers: readonly TrustGatedMcpServer
318350
}
319351

320352
function escapeControlChars(value: string): string {
321-
return value.replaceAll(/[\u0000-\u001f\u007f-\u009f]/g, (char) => {
353+
return value.replaceAll(/[\u0000-\u001F\u007F-\u009F]/g, (char) => {
322354
const code = char.codePointAt(0) ?? 0;
323355
return `\\x${code.toString(16).padStart(2, '0')}`;
324356
});

apps/pythinker-code/src/main.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ export async function handleMainCommand(
9595
return { headlessCompleted: false };
9696
}
9797

98-
export async function handleUpgradeCommand(version: string): Promise<void> {
98+
export async function handleUpgradeCommand(version: string, yes: boolean): Promise<void> {
9999
const telemetryBootstrap = createCliTelemetryBootstrap();
100100
const telemetryClient: TelemetryClient = {
101101
track,
@@ -118,7 +118,7 @@ export async function handleUpgradeCommand(version: string): Promise<void> {
118118
version,
119119
uiMode: CLI_UI_MODE,
120120
});
121-
exitCode = await handleUpgrade(version, { track, logger: log });
121+
exitCode = await handleUpgrade(version, { track, logger: log, yes });
122122
} finally {
123123
await shutdownTelemetry({ timeoutMs: CLI_SHUTDOWN_TIMEOUT_MS }).catch(() => {});
124124
await harness.close().catch(() => {});
@@ -236,8 +236,8 @@ function bootstrap(): void {
236236
process.exit(1);
237237
});
238238
},
239-
() => {
240-
void handleUpgradeCommand(version).catch(async (error: unknown) => {
239+
(yes) => {
240+
void handleUpgradeCommand(version, yes).catch(async (error: unknown) => {
241241
await logStartupFailure('upgrade', error);
242242
process.stderr.write(formatStartupError(error, { operation: 'upgrade' }));
243243
process.stderr.write(`See log: ${resolveGlobalLogPath(resolvePythinkerHome())}\n`);

0 commit comments

Comments
 (0)