Open a folder chip on its folder, in the folder that really holds it
A bot's folder chip went to bare /admin/files instead of the folder it named, and even once there the computer volume pointed at the wrong directory, so the folder a bot linked to looked empty. The chip lost its folder because folderRefUrl only accepted a ref's volume name (computer) while the chip hands it the browser's (computers), so storageVolumeFromRef found nothing and the whole query was dropped. Both spellings resolve now, through parseStorageSpec, so a bad owner falls back cleanly instead of emitting owner=0. The deeper bug: a computer's home was assumed to be DATA_DIR/computers/<id>, but on a bind mount it can be anywhere on the docker host, and a computer the app didn't provision (the main one on a server reached at MAIN_COMPUTER_URL, i.e. a dev server outside docker) records nothing at all. resolveComputerHomeDir holds the precedence: the directory provisionComputer actually bound in (containers.metadata.data_dir), else the new MAIN_COMPUTER_DATA_DIR app setting for the main computer, else the conventional folder. A path that isn't on disk also no longer reads as an empty folder: listStorageEntries reports it as missing and the page says so, so a stale folder link can't look like a folder nothing has written yet.
This commit is contained in:
@@ -66,6 +66,12 @@ export const EnvDict: readonly KnownEnv[] = [
|
||||
required: false,
|
||||
secret: false,
|
||||
},
|
||||
{
|
||||
key: "MAIN_COMPUTER_DATA_DIR",
|
||||
description: `Where the main computer's home (/home/gem) is on this server's disk, for a computer the app didn't create itself (see MAIN_COMPUTER_URL), e.g. the bind mount your sandbox's compose file uses. Empty reads DATA_DIR/computers/<id>.`,
|
||||
required: false,
|
||||
secret: false,
|
||||
},
|
||||
...HarnessProviderKinds.flatMap((provider) =>
|
||||
provider.key_env
|
||||
? [
|
||||
|
||||
@@ -0,0 +1,70 @@
|
||||
import { describe, expect, test } from "bun:test";
|
||||
import resolveComputerHomeDir from "./resolve-computer-home-dir";
|
||||
|
||||
const data_dir = "/data";
|
||||
|
||||
describe("resolveComputerHomeDir", () => {
|
||||
test("uses the directory the app bind-mounted", () => {
|
||||
/**
|
||||
* The truth for a container the app provisioned: on a bind mount
|
||||
* that can be anywhere on the docker host, not under DATA_DIR.
|
||||
*/
|
||||
expect(
|
||||
resolveComputerHomeDir({
|
||||
computer_id: 2,
|
||||
kind: "sandbox",
|
||||
metadata: JSON.stringify({ data_dir: "/srv/homes/2", host_dir: "/host/2" }),
|
||||
main_data_dir: "/wrong",
|
||||
data_dir,
|
||||
}),
|
||||
).toBe("/srv/homes/2");
|
||||
});
|
||||
|
||||
test("falls back to MAIN_COMPUTER_DATA_DIR for the main computer", () => {
|
||||
expect(
|
||||
resolveComputerHomeDir({
|
||||
computer_id: 1,
|
||||
kind: "main",
|
||||
metadata: null,
|
||||
main_data_dir: "/media/docker/aio-sandbox/data/home",
|
||||
data_dir,
|
||||
}),
|
||||
).toBe("/media/docker/aio-sandbox/data/home");
|
||||
});
|
||||
|
||||
test("leaves a sandbox alone whatever the main setting says", () => {
|
||||
expect(
|
||||
resolveComputerHomeDir({
|
||||
computer_id: 3,
|
||||
kind: "sandbox",
|
||||
metadata: null,
|
||||
main_data_dir: "/media/docker/aio-sandbox/data/home",
|
||||
data_dir,
|
||||
}),
|
||||
).toBe("/data/computers/3");
|
||||
});
|
||||
|
||||
test("uses the conventional folder when nothing is recorded", () => {
|
||||
expect(resolveComputerHomeDir({ computer_id: 4, kind: "main", data_dir })).toBe(
|
||||
"/data/computers/4",
|
||||
);
|
||||
});
|
||||
|
||||
test("ignores an empty or unreadable record", () => {
|
||||
expect(
|
||||
resolveComputerHomeDir({ computer_id: 5, metadata: JSON.stringify({ data_dir: " " }), data_dir }),
|
||||
).toBe("/data/computers/5");
|
||||
expect(resolveComputerHomeDir({ computer_id: 5, metadata: "not json", data_dir })).toBe(
|
||||
"/data/computers/5",
|
||||
);
|
||||
expect(resolveComputerHomeDir({ computer_id: 5, metadata: null, data_dir })).toBe(
|
||||
"/data/computers/5",
|
||||
);
|
||||
});
|
||||
|
||||
test("normalizes a recorded relative path", () => {
|
||||
expect(
|
||||
resolveComputerHomeDir({ computer_id: 6, metadata: JSON.stringify({ data_dir: "/srv/a/../b" }), data_dir }),
|
||||
).toBe("/srv/b");
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,63 @@
|
||||
import path from "node:path";
|
||||
import { AppData } from "@/src/data/app-data";
|
||||
import parseJobJson from "@/src/bots-queue/parse-job-json";
|
||||
|
||||
type Params = {
|
||||
computer_id: number | string;
|
||||
/**
|
||||
* The computer's kind: only the install's main computer can be
|
||||
* reached at `MAIN_COMPUTER_URL`, so only it uses
|
||||
* `MAIN_COMPUTER_DATA_DIR`.
|
||||
*/
|
||||
kind?: string | null;
|
||||
/**
|
||||
* Its container's `metadata`, as `provisionComputer` wrote it (the
|
||||
* `data_dir` it bound into the container).
|
||||
*/
|
||||
metadata?: string | null;
|
||||
/**
|
||||
* What `MAIN_COMPUTER_DATA_DIR` is set to, already read
|
||||
* (`grabEnv`).
|
||||
*/
|
||||
main_data_dir?: string | null;
|
||||
/**
|
||||
* The data volume's root as this app sees it (`DATA_DIR`).
|
||||
*/
|
||||
data_dir: string;
|
||||
};
|
||||
|
||||
/**
|
||||
* Where a computer's home (`AppData.ComputerHomeDir` in its container)
|
||||
* actually is on this app's disk, for the files browser and for anything
|
||||
* else that has to read a computer's files.
|
||||
*
|
||||
* A container the app provisioned recorded the directory it bind-mounted
|
||||
* (`containers.metadata.data_dir`), which is the truth: on a bind mount
|
||||
* that path can be anywhere the docker host allows. A computer the app
|
||||
* didn't provision has no record. That is the install's main computer on a
|
||||
* server reached at `MAIN_COMPUTER_URL` (a dev server outside docker),
|
||||
* whose sandbox keeps its home wherever its compose file says, so
|
||||
* `MAIN_COMPUTER_DATA_DIR` names that directory for this app. Without
|
||||
* either, the conventional place every app-provisioned computer uses:
|
||||
* `DATA_DIR/<AppData.ComputerDataDir>/<id>`.
|
||||
*
|
||||
* Kept apart from the docker and database calls so the precedence is
|
||||
* testable on its own.
|
||||
*/
|
||||
export default function resolveComputerHomeDir({
|
||||
computer_id,
|
||||
kind,
|
||||
metadata,
|
||||
main_data_dir,
|
||||
data_dir,
|
||||
}: Params) {
|
||||
const recorded = parseJobJson({ text: metadata || "" })?.data_dir;
|
||||
|
||||
if (typeof recorded == "string" && recorded.trim()) return path.resolve(recorded.trim());
|
||||
|
||||
const external = kind == "main" ? main_data_dir?.trim() : "";
|
||||
|
||||
if (external) return path.resolve(external);
|
||||
|
||||
return path.join(data_dir, AppData.ComputerDataDir, String(computer_id));
|
||||
}
|
||||
@@ -70,9 +70,11 @@ export default async function listStorageEntries({
|
||||
/**
|
||||
* A folder nothing has written to yet (a computer's home before it
|
||||
* starts, a bot's before its first file) shows as empty rather than as
|
||||
* an error.
|
||||
* an error. `missing` keeps that apart from a path that isn't there at
|
||||
* all, which a stale folder link lands on (the page says so, instead of
|
||||
* claiming the folder is empty).
|
||||
*/
|
||||
if (!existsSync(absolute)) return { entries: [], total: 0, offset };
|
||||
if (!existsSync(absolute)) return { entries: [], total: 0, offset, missing: true };
|
||||
|
||||
const rows = readdirSync(absolute, { withFileTypes: true });
|
||||
const indexed = await botFileRows({ resolved, paths: rows.map((row) => joinPath({ relative, name: row.name })) });
|
||||
@@ -145,6 +147,7 @@ export default async function listStorageEntries({
|
||||
entries: entries.slice(offset, offset + AppData.StorageBrowserPageSize),
|
||||
total: entries.length,
|
||||
offset,
|
||||
missing: false,
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -10,6 +10,8 @@ import { storageVolumeLabel, type StorageSpec } from "@/src/utils/storage-volume
|
||||
import grabCrewRole from "../db/access/grab-crew-role";
|
||||
import grabUserBot from "../db/bots/grab-user-bot";
|
||||
import grabUserComputer from "../db/computers/grab-user-computer";
|
||||
import resolveComputerHomeDir from "../db/computers/resolve-computer-home-dir";
|
||||
import grabEnv from "../db/envs/grab-env";
|
||||
import grabUserWorkspace from "../db/workspaces/grab-user-workspace";
|
||||
|
||||
type Params = {
|
||||
@@ -69,10 +71,19 @@ async function storageRoot({
|
||||
? path.join(DATA_DIR, AppData.WorkspaceDataDir, String(spec.owner_id))
|
||||
: null;
|
||||
|
||||
case "computers":
|
||||
return (await grabUserComputer({ computer_id: spec.owner_id, user_id }))
|
||||
? path.join(DATA_DIR, AppData.ComputerDataDir, String(spec.owner_id))
|
||||
: null;
|
||||
case "computers": {
|
||||
const computer = await grabUserComputer({ computer_id: spec.owner_id, user_id });
|
||||
|
||||
if (!computer) return null;
|
||||
|
||||
return resolveComputerHomeDir({
|
||||
computer_id: spec.owner_id,
|
||||
kind: computer.kind,
|
||||
metadata: await computerContainerMetadata({ container_id: computer.container_id }),
|
||||
main_data_dir: await grabEnv({ key: "MAIN_COMPUTER_DATA_DIR" }),
|
||||
data_dir: DATA_DIR,
|
||||
});
|
||||
}
|
||||
|
||||
case "media":
|
||||
/**
|
||||
@@ -100,6 +111,23 @@ async function storageRoot({
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* A computer's container `metadata`, where `provisionComputer` recorded
|
||||
* the directory it bind-mounted as the computer's home. Null when the app
|
||||
* didn't create the container (a computer reached at
|
||||
* `MAIN_COMPUTER_URL`), which is what `MAIN_COMPUTER_DATA_DIR` is for.
|
||||
*/
|
||||
async function computerContainerMetadata({ container_id }: { container_id?: number | "" }) {
|
||||
if (!container_id) return null;
|
||||
|
||||
const res = await BunSQLite.sql<{ metadata?: string | null }>({
|
||||
sql: `SELECT metadata FROM containers WHERE id = ? LIMIT 1`,
|
||||
values: [container_id],
|
||||
});
|
||||
|
||||
return res.payload?.[0]?.metadata || null;
|
||||
}
|
||||
|
||||
/**
|
||||
* What the page calls the folder: the bot's, workspace's or computer's
|
||||
* own name where one is known.
|
||||
|
||||
@@ -0,0 +1,57 @@
|
||||
import { describe, expect, test } from "bun:test";
|
||||
import type { StorageFolderPayload } from "@/src/types";
|
||||
import { emptyFolderText, folderSummary } from "./folder-text";
|
||||
|
||||
function folder(overrides: Partial<StorageFolderPayload> = {}): StorageFolderPayload {
|
||||
return {
|
||||
spec: { volume: "computers", owner_id: 1 },
|
||||
label: "Main computer",
|
||||
path: "seo/coderank",
|
||||
breadcrumbs: [],
|
||||
entries: [],
|
||||
total: 0,
|
||||
offset: 0,
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
describe("folderSummary", () => {
|
||||
test("counts what is in the folder", () => {
|
||||
expect(folderSummary({ folder: folder({ total: 4 }) })).toContain("4 item(s)");
|
||||
});
|
||||
|
||||
test("says a missing folder isn't on disk, not that it is empty", () => {
|
||||
const text = folderSummary({ folder: folder({ missing: true }) });
|
||||
|
||||
expect(text).toContain("isn't on disk");
|
||||
expect(text).not.toContain("0 item(s)");
|
||||
});
|
||||
|
||||
test("asks for a folder when there is none", () => {
|
||||
expect(folderSummary({ folder: null })).toBe("Pick a folder to look inside.");
|
||||
});
|
||||
});
|
||||
|
||||
describe("emptyFolderText", () => {
|
||||
test("an empty folder says so", () => {
|
||||
expect(emptyFolderText({ folder: folder(), search: "" })).toBe("This folder is empty.");
|
||||
});
|
||||
|
||||
test("a path that isn't there says so instead", () => {
|
||||
expect(emptyFolderText({ folder: folder({ missing: true }), search: "" })).toBe(
|
||||
"There's no seo/coderank in here. It may have been moved or deleted.",
|
||||
);
|
||||
});
|
||||
|
||||
test("loading, an error and a search each win over the folder's own state", () => {
|
||||
expect(emptyFolderText({ folder: folder({ missing: true }), search: "", loading: true })).toBe(
|
||||
"Loading...",
|
||||
);
|
||||
expect(
|
||||
emptyFolderText({ folder: folder({ missing: true }), search: "", error: "Not found." }),
|
||||
).toBe("Not found.");
|
||||
expect(emptyFolderText({ folder: folder({ missing: true }), search: "raw" })).toBe(
|
||||
'Nothing matches "raw".',
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,42 @@
|
||||
import type { StorageFolderPayload } from "@/src/types";
|
||||
|
||||
/**
|
||||
* The line under the open folder's name: how much is in it, or, for a path
|
||||
* that isn't on disk, that it isn't there. A stale folder link must never
|
||||
* read as an empty folder.
|
||||
*/
|
||||
export function folderSummary({ folder }: { folder: StorageFolderPayload | null }) {
|
||||
if (!folder) return "Pick a folder to look inside.";
|
||||
|
||||
if (folder.missing) {
|
||||
return `${folder.label}: this folder isn't on disk. Nothing has written it yet, or its link is out of date.`;
|
||||
}
|
||||
|
||||
return `${folder.label}: ${folder.total} item(s). Everything a bot, a workspace or a computer keeps for you.`;
|
||||
}
|
||||
|
||||
/**
|
||||
* What an empty listing says, and why it is empty when the folder the
|
||||
* user asked for isn't there at all.
|
||||
*/
|
||||
export function emptyFolderText({
|
||||
folder,
|
||||
search,
|
||||
loading,
|
||||
error,
|
||||
}: {
|
||||
folder: StorageFolderPayload | null;
|
||||
search: string;
|
||||
loading?: boolean;
|
||||
error?: string;
|
||||
}) {
|
||||
if (loading) return "Loading...";
|
||||
if (error) return error;
|
||||
if (search) return `Nothing matches "${search}".`;
|
||||
|
||||
if (folder?.missing) {
|
||||
return `There's no ${folder.path || "folder"} in here. It may have been moved or deleted.`;
|
||||
}
|
||||
|
||||
return "This folder is empty.";
|
||||
}
|
||||
@@ -4,6 +4,7 @@ import Span from "@moduletrace/twui/layout/Span";
|
||||
import Stack from "@moduletrace/twui/layout/Stack";
|
||||
import type { StorageFolderPayload, StorageSpecOption } from "@/src/types";
|
||||
import type { StorageSpec } from "@/src/utils/storage-volumes";
|
||||
import { emptyFolderText, folderSummary } from "../(functions)/folder-text";
|
||||
import FilesBreadcrumbRow from "../(partials)/files-breadcrumb-row";
|
||||
import FilesSearchInput from "../(partials)/files-search-input";
|
||||
import FilesVolumeRow from "../(partials)/files-volume-row";
|
||||
@@ -81,9 +82,7 @@ export default function FilesBrowser({
|
||||
"Files"}
|
||||
</H2>
|
||||
<Span className="text-[13px] leading-[18px] text-ink-secondary">
|
||||
{folder
|
||||
? `${folder.label}: ${folder.total} item(s). Everything a bot, a workspace or a computer keeps for you.`
|
||||
: "Pick a folder to look inside."}
|
||||
{folderSummary({ folder })}
|
||||
</Span>
|
||||
</Stack>
|
||||
<FilesBreadcrumbRow
|
||||
@@ -111,12 +110,7 @@ export default function FilesBrowser({
|
||||
))
|
||||
) : (
|
||||
<Span className="py-8 text-[13px] leading-[18px] text-ink-secondary">
|
||||
{loading
|
||||
? "Loading..."
|
||||
: error ||
|
||||
(search
|
||||
? `Nothing matches "${search}".`
|
||||
: "This folder is empty.")}
|
||||
{emptyFolderText({ folder, search, loading, error })}
|
||||
</Span>
|
||||
)}
|
||||
</Stack>
|
||||
|
||||
@@ -80,5 +80,6 @@ async function folderPayload({
|
||||
entries: listing.entries,
|
||||
total: listing.total,
|
||||
offset: listing.offset,
|
||||
missing: listing.missing,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -61,6 +61,7 @@ export const handler: BunextAPIRouteHandler<APIResponseObject<StorageFolderPaylo
|
||||
entries: listing.entries,
|
||||
total: listing.total,
|
||||
offset: listing.offset,
|
||||
missing: listing.missing,
|
||||
},
|
||||
};
|
||||
} catch (error: unknown) {
|
||||
|
||||
@@ -775,6 +775,11 @@ export type StorageFolderPayload = {
|
||||
entries: StorageEntry[];
|
||||
total: number;
|
||||
offset: number;
|
||||
/**
|
||||
* The folder asked for isn't on disk (a stale link, or a computer whose
|
||||
* home directory the app can't find), as opposed to an empty one.
|
||||
*/
|
||||
missing?: boolean;
|
||||
};
|
||||
|
||||
/**
|
||||
|
||||
@@ -55,6 +55,26 @@ describe("folderRefUrl", () => {
|
||||
`/admin/files?volume=computers&owner=3&path=seo%2Fraw`,
|
||||
);
|
||||
});
|
||||
|
||||
test("keeps the folder when given the browser's own volume name", () => {
|
||||
/**
|
||||
* What the chip does: it holds a parsed spec, whose `volume` is the
|
||||
* browser's (`computers`), not the ref's (`computer`). Losing that
|
||||
* dropped the whole query and landed on the files browser's first
|
||||
* folder.
|
||||
*/
|
||||
expect(folderRefUrl({ volume: "computers", owner_id: 1, path: "seo/coderank" })).toBe(
|
||||
`/admin/files?volume=computers&owner=1&path=seo%2Fcoderank`,
|
||||
);
|
||||
expect(folderRefUrl({ volume: "bots", owner_id: 6, path: "reports" })).toBe(
|
||||
`/admin/files?volume=bots&owner=6&path=reports`,
|
||||
);
|
||||
});
|
||||
|
||||
test("falls back to the files browser for a volume it doesn't know", () => {
|
||||
expect(folderRefUrl({ volume: "secrets", owner_id: 1, path: "a" })).toBe("/admin/files");
|
||||
expect(folderRefUrl({ volume: "computers", owner_id: 0 })).toBe("/admin/files");
|
||||
});
|
||||
});
|
||||
|
||||
describe("linkComputerRefs", () => {
|
||||
@@ -68,6 +88,16 @@ describe("linkComputerRefs", () => {
|
||||
});
|
||||
});
|
||||
|
||||
test("a linked folder ref still opens on that folder", () => {
|
||||
const linked = linkComputerRefs({ text: "straight off the machine: [folder:computer:1:seo/coderank]" });
|
||||
const ref = parseFolderRefHref({ href: linked.match(/\((#folder-[^)]+)\)/)?.[1] });
|
||||
|
||||
expect(ref).not.toBeNull();
|
||||
expect(
|
||||
folderRefUrl({ volume: ref!.spec.volume, owner_id: ref!.spec.owner_id, path: ref!.path }),
|
||||
).toBe(`/admin/files?volume=computers&owner=1&path=seo%2Fcoderank`);
|
||||
});
|
||||
|
||||
test("leaves a file ref to the file chip", () => {
|
||||
const linked = linkComputerRefs({ text: "The report: [file:12]." });
|
||||
|
||||
|
||||
+14
-2
@@ -10,6 +10,8 @@
|
||||
* name's extension: the ref says which one it is.
|
||||
*/
|
||||
import {
|
||||
isStorageVolume,
|
||||
parseStorageSpec,
|
||||
StorageBrowserPath,
|
||||
storageSpecHref,
|
||||
storageVolumeFromRef,
|
||||
@@ -96,11 +98,21 @@ export function parseFolderRefHref({
|
||||
/**
|
||||
* Where a folder ref opens: the files browser, on that folder. A volume the
|
||||
* browser doesn't know lands on its first folder.
|
||||
*
|
||||
* Both spellings of a volume come through here: a ref names it the way a
|
||||
* bot writes it (`computer`), the files browser the way it stores it
|
||||
* (`computers`). Either one resolves, so a chip never loses its folder and
|
||||
* drops back to the files browser's first folder.
|
||||
*/
|
||||
export function folderRefUrl({ volume, owner_id, path }: RefParams) {
|
||||
const known = storageVolumeFromRef({ name: volume });
|
||||
const from_ref = storageVolumeFromRef({ name: volume });
|
||||
const known = from_ref || (isStorageVolume(volume) ? volume : null);
|
||||
|
||||
if (!known) return StorageBrowserPath;
|
||||
|
||||
return storageSpecHref({ spec: { volume: known, owner_id: Number(owner_id) }, path });
|
||||
const spec = parseStorageSpec({ volume: known, owner_id });
|
||||
|
||||
if (!spec) return StorageBrowserPath;
|
||||
|
||||
return storageSpecHref({ spec, path });
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user