Skip to content

Commit e1f0729

Browse files
committed
fix(extmgr): address review follow-ups
1 parent ca275ef commit e1f0729

13 files changed

Lines changed: 113 additions & 28 deletions

README.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ When Pi is running without UI, extmgr still supports command-driven workflows:
109109
- `/extensions update [source]`
110110
- `/extensions history [options]`
111111
- `/extensions auto-update <duration>`
112-
- Use `1mo` for monthly schedules (history keeps `30m`/`24h` style durations for lookback windows)
112+
- Use `1mo` for monthly schedules (`/extensions history --since <duration>` also accepts `1mo`; `30m`/`24h` are just lookback examples)
113113

114114
Remote browsing/search menus require the full interactive TUI.
115115

@@ -128,14 +128,14 @@ History options (works in non-interactive mode too):
128128
- `--action <extension_toggle|extension_delete|package_install|package_update|package_remove|cache_clear|auto_update_config>`
129129
- `--success` / `--failed`
130130
- `--package <query>`
131-
- `--since <duration>` (e.g. `30m`, `24h`, `7d`, `1mo`)
131+
- `--since <duration>` (e.g. `30m`, `24h`, `7d`, `1mo`; `1mo` is supported for monthly lookbacks)
132132
- `--global` (non-interactive mode only; reads all persisted sessions under `~/.pi/agent/sessions`)
133133

134134
Examples:
135135

136136
- `/extensions history --failed --limit 50`
137137
- `/extensions history --action package_update --since 7d`
138-
- `/extensions history --global --package extmgr --since 24h`
138+
- `/extensions history --global --package extmgr --since 1mo`
139139

140140
### Install sources
141141

@@ -157,7 +157,7 @@ Examples:
157157
- After saving package extension config, restart pi to fully apply changes.
158158
- **Two install modes**:
159159
- **Managed** (npm): Auto-updates with `pi update`, stored in pi's package cache, supports Pi package manifest/convention loading
160-
- **Local** (standalone): Copies to `~/.pi/agent/extensions/{package}/`, so it only accepts runnable standalone layouts (manifest-declared/root entrypoints), requires `tar` on `PATH`, and rejects packages whose runtime `dependencies` are not already bundled in the tarball
160+
- **Local** (standalone): Copies to `~/.pi/agent/extensions/{package}/`, so it only accepts runnable standalone layouts (manifest-declared/root entrypoints), requires `tar` on `PATH`, and rejects packages whose runtime `dependencies` are not already bundled with the package contents
161161
- **Auto-update schedule is persistent**: `/extensions auto-update 1d` stays active across future Pi sessions and is restored when switching sessions.
162162
- **Auto-update coverage is npm-only today**: extmgr checks update availability for managed npm packages; git/local installs are not included in the background update badge yet.
163163
- **Settings/cache writes are hardened**: extmgr serializes writes and uses safe file replacement to reduce JSON corruption issues.

src/commands/history.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ function showHistoryHelp(ctx: ExtensionCommandContext): void {
189189
"Options:",
190190
" --limit <n> Maximum entries to show (default: 20)",
191191
" --action <type> Filter by action",
192-
" extension_toggle | extension_delete | package_install | package_update | package_remove | cache_clear | auto_update_config",
192+
` ${HISTORY_ACTIONS.join(" | ")}`,
193193
" --success Show only successful entries",
194194
" --failed Show only failed entries",
195195
" --package <q> Filter by package/source/extension id",

src/packages/discovery.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
getPackageSourceKind,
1717
normalizePackageIdentity,
1818
splitGitRepoAndRef,
19+
stripGitSourcePrefix,
1920
} from "../utils/package-source.js";
2021
import { execNpm } from "../utils/npm-exec.js";
2122

@@ -285,6 +286,12 @@ export async function isSourceInstalled(
285286
}
286287
}
287288

289+
/**
290+
* parseInstalledPackagesOutputAllScopes returns the raw parsed entries from
291+
* parseInstalledPackagesOutputInternal without deduplication or scope merging.
292+
* Prefer parseInstalledPackagesOutput for user-facing lists, since it applies
293+
* deduplication and normalized scope selection.
294+
*/
288295
export function parseInstalledPackagesOutputAllScopes(text: string): InstalledPackage[] {
289296
return parseInstalledPackagesOutputInternal(text);
290297
}
@@ -325,7 +332,7 @@ function parsePackageNameAndVersion(fullSource: string): {
325332

326333
const sourceKind = getPackageSourceKind(fullSource);
327334
if (sourceKind === "git") {
328-
const gitSpec = fullSource.startsWith("git:") ? fullSource.slice(4) : fullSource;
335+
const gitSpec = stripGitSourcePrefix(fullSource);
329336
const { repo } = splitGitRepoAndRef(gitSpec);
330337
return { name: extractGitPackageName(repo) };
331338
}

src/packages/install.ts

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -147,11 +147,20 @@ async function getStandaloneDependencyError(packageRoot: string): Promise<string
147147
}
148148

149149
async function cleanupStandaloneTempArtifacts(tempDir: string, extractDir?: string): Promise<void> {
150-
if (extractDir) {
151-
await rm(extractDir, { recursive: true, force: true });
152-
}
153-
154-
await rm(tempDir, { recursive: true, force: true });
150+
const paths = [extractDir, tempDir].filter((path): path is string => Boolean(path));
151+
152+
await Promise.all(
153+
paths.map(async (path) => {
154+
try {
155+
await rm(path, { recursive: true, force: true });
156+
} catch (error) {
157+
console.warn(
158+
`[extmgr] Failed to remove temporary standalone install artifact at ${path}:`,
159+
error
160+
);
161+
}
162+
})
163+
);
155164
}
156165

157166
export async function installPackage(
@@ -273,7 +282,7 @@ export async function installFromUrl(
273282
await mkdir(extensionDir, { recursive: true });
274283
notify(ctx, `Downloading ${fileName}...`, "info");
275284

276-
const response = await fetchWithTimeout(url, TIMEOUTS.fetchPackageInfo);
285+
const response = await fetchWithTimeout(url, TIMEOUTS.packageInstall);
277286
if (!response.ok) {
278287
throw new Error(`Download failed: ${response.status} ${response.statusText}`);
279288
}

src/ui/unified.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ import { logExtensionDelete, logExtensionToggle } from "../utils/history.js";
4242
import { getKnownUpdates, promptAutoUpdateWizard } from "../utils/auto-update.js";
4343
import { updateExtmgrStatus } from "../utils/status.js";
4444
import { parseChoiceByLabel } from "../utils/command.js";
45+
import { notify } from "../utils/notify.js";
4546
import { getPackageSourceKind, normalizePackageIdentity } from "../utils/package-source.js";
4647
import { hasCustomUI, runCustomUI } from "../utils/mode.js";
4748
import { getSettingsListSelectedIndex } from "../utils/settings-list.js";
@@ -61,7 +62,8 @@ export async function showInteractive(
6162
pi: ExtensionAPI
6263
): Promise<void> {
6364
if (!hasCustomUI(ctx)) {
64-
ctx.ui.notify(
65+
notify(
66+
ctx,
6567
"The unified extensions manager requires the full interactive TUI. Showing read-only local and installed package lists instead.",
6668
"warning"
6769
);

src/utils/auto-update.ts

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -71,13 +71,6 @@ export function startAutoUpdateTimer(
7171
},
7272
{ initialDelayMs }
7373
);
74-
75-
if (initialDelayMs > 0 && nextCheck !== undefined) {
76-
saveAutoUpdateConfig(pi, {
77-
...config,
78-
nextCheck,
79-
});
80-
}
8174
}
8275

8376
/**
@@ -179,7 +172,7 @@ export function getAutoUpdateStatus(ctx: ExtensionCommandContext | ExtensionCont
179172
}
180173

181174
/**
182-
* Return package names currently known to have updates available
175+
* Return normalized package identities currently known to have updates available
183176
* (from the latest background check).
184177
*/
185178
export function getKnownUpdates(ctx: ExtensionCommandContext | ExtensionContext): Set<string> {

src/utils/package-source.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,11 @@ export function normalizeLocalSourceIdentity(source: string): string {
5656
return looksWindowsPath ? normalized.toLowerCase() : normalized;
5757
}
5858

59+
export function stripGitSourcePrefix(source: string): string {
60+
const withoutGitPlus = source.startsWith("git+") ? source.slice(4) : source;
61+
return withoutGitPlus.startsWith("git:") ? withoutGitPlus.slice(4) : withoutGitPlus;
62+
}
63+
5964
export function normalizePackageIdentity(
6065
source: string,
6166
options?: { resolvedPath?: string }
@@ -69,7 +74,7 @@ export function normalizePackageIdentity(
6974
}
7075

7176
if (kind === "git") {
72-
const gitSpec = normalized.startsWith("git:") ? normalized.slice(4) : normalized;
77+
const gitSpec = stripGitSourcePrefix(normalized);
7378
const { repo } = splitGitRepoAndRef(gitSpec);
7479
return `git:${repo.replace(/\\/g, "/").toLowerCase()}`;
7580
}

src/utils/settings-list.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,5 +8,5 @@ export function getSettingsListSelectedIndex(settingsList: unknown): number | un
88
}
99

1010
const selectable = settingsList as SelectableListLike;
11-
return typeof selectable.selectedIndex === "number" ? selectable.selectedIndex : undefined;
11+
return Number.isInteger(selectable.selectedIndex) ? selectable.selectedIndex : undefined;
1212
}

src/utils/settings.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { readFile, writeFile, mkdir, rename, rm } from "node:fs/promises";
1111
import { homedir } from "node:os";
1212
import { join } from "node:path";
1313
import { fileExists } from "./fs.js";
14+
import { normalizePackageIdentity } from "./package-source.js";
1415

1516
export interface AutoUpdateConfig {
1617
intervalMs: number;
@@ -57,7 +58,9 @@ function sanitizeUpdateIdentities(value: unknown): string[] | undefined {
5758
const updates = sanitizeStringArray(value);
5859
if (!updates) return undefined;
5960

60-
const sanitized = updates.filter(isUpdateIdentity);
61+
const sanitized = updates
62+
.filter(isUpdateIdentity)
63+
.map((entry) => normalizePackageIdentity(entry));
6164
return sanitized.length > 0 ? sanitized : undefined;
6265
}
6366

test/auto-update.test.ts

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -298,9 +298,8 @@ void test("enableAutoUpdate records the next scheduled check without faking a co
298298
try {
299299
enableAutoUpdate(pi, ctx, 60 * 60 * 1000, "1 hour");
300300

301-
const latestConfig = entries[entries.length - 1]?.data as
302-
| { lastCheck?: number; nextCheck?: number; enabled?: boolean }
303-
| undefined;
301+
const latestConfig = entries.filter((entry) => entry.customType === "extmgr-auto-update").at(-1)
302+
?.data as { lastCheck?: number; nextCheck?: number; enabled?: boolean } | undefined;
304303

305304
assert.equal(latestConfig?.enabled, true);
306305
assert.equal(latestConfig?.lastCheck, undefined);
@@ -332,3 +331,33 @@ void test("getKnownUpdates ignores legacy name-only update markers", () => {
332331

333332
assert.deepEqual(Array.from(getKnownUpdates(ctx)), []);
334333
});
334+
335+
void test("getKnownUpdates normalizes stored update identities", () => {
336+
const ctx = {
337+
hasUI: false,
338+
cwd: "/tmp",
339+
sessionManager: {
340+
getEntries: () => [
341+
{
342+
type: "custom",
343+
customType: "extmgr-auto-update",
344+
data: {
345+
enabled: true,
346+
intervalMs: 60 * 60 * 1000,
347+
displayText: "1 hour",
348+
updatesAvailable: [
349+
"npm:Demo-Pkg",
350+
"git:HTTPS://GitHub.com/User/Repo.git@main",
351+
"not-an-identity",
352+
],
353+
},
354+
},
355+
],
356+
},
357+
} as unknown as ReturnType<typeof createMockHarness>["ctx"];
358+
359+
assert.deepEqual(Array.from(getKnownUpdates(ctx)).sort(), [
360+
"git:https://github.com/user/repo.git",
361+
"npm:demo-pkg",
362+
]);
363+
});

0 commit comments

Comments
 (0)