Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/platform/adapters/fs/cache/file-cache.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,12 @@ describe("Distributed cache functions", () => {
circular.self = circular;
distributedCache.set("cyclic", circular);
assertEquals(backendWrites, 0);

// Positive control: a serializable entry must reach the fake backend,
// proving the harness is live and the zero-write assertion above is not
// vacuously passing because the backend was never wired up.
distributedCache.set("serializable", { ok: true });
assertEquals(backendWrites, 1);
});

it("should return boolean", async () => {
Expand Down
6 changes: 0 additions & 6 deletions src/platform/adapters/fs/github/adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,6 @@ export class GitHubFSAdapter implements FSAdapter {
retry: githubConfig.retry,
};

if (!rawConfig.token) {
throw CONFIG_INVALID.create({
detail: "GitHub adapter requires a token; set GITHUB_TOKEN or pass config.github.token",
});
}

this.config = createGitHubConfig(rawConfig);
this.client = new GitHubApiClient(this.config);

Expand Down
9 changes: 7 additions & 2 deletions src/platform/adapters/fs/github/github-api-client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@ import "#veryfront/schemas/_test-setup.ts";
import {
assertEquals,
assertExists,
assertInstanceOf,
assertRejects,
assertThrows,
} from "#veryfront/testing/assert.ts";
import { describe, it } from "#veryfront/testing/bdd.ts";
import { withMockFetch } from "#veryfront/testing/mock-fetch.ts";
import { VeryfrontError } from "#veryfront/errors/types.ts";
import { GitHubApiClient } from "./github-api-client.ts";

const mockConfig = {
Expand Down Expand Up @@ -59,11 +61,14 @@ describe("GitHubApiClient", () => {
["repo", "r".repeat(257)],
] as const
) {
assertThrows(
// Repository identity failures retain stable CONFIG error semantics.
const error = assertThrows(
() => new GitHubApiClient({ ...mockConfig, [field]: value }),
TypeError,
VeryfrontError,
"GitHub",
);
assertInstanceOf(error, VeryfrontError);
assertEquals(error.slug, "config-validation-failed");
Comment thread
kojiwakayama marked this conversation as resolved.
}
});
});
Expand Down
15 changes: 12 additions & 3 deletions src/platform/adapters/fs/github/github-api-client.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { createError, retryWithBackoff, toError } from "#veryfront/errors";
import { CONFIG_VALIDATION_FAILED } from "#veryfront/errors/error-registry/config.ts";
import { logger } from "#veryfront/utils";
import type { ResolvedGitHubConfig } from "./types.ts";
import {
Expand Down Expand Up @@ -105,9 +106,17 @@ export class GitHubApiClient {
private rateLimitInfo: RateLimitInfo | null = null;

constructor(private readonly config: ResolvedGitHubConfig) {
const owner = encodeRepositorySegment(config.owner, "owner");
const repo = encodeRepositorySegment(config.repo, "repository");
this.repositoryEndpoint = `/repos/${owner}/${repo}`;
// Invalid repository identity remains a CONFIG-category boundary error.
try {
const owner = encodeRepositorySegment(config.owner, "owner");
const repo = encodeRepositorySegment(config.repo, "repository");
this.repositoryEndpoint = `/repos/${owner}/${repo}`;
} catch (cause) {
throw CONFIG_VALIDATION_FAILED.create({
detail: cause instanceof Error ? cause.message : "GitHub repository identity is invalid",
cause,
});
}
}

get repoId(): string {
Expand Down
32 changes: 15 additions & 17 deletions src/platform/adapters/fs/github/types.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { createError, toError } from "#veryfront/errors";
import { CONFIG_INVALID } from "#veryfront/errors";

export type { DirectoryEntry } from "../shared-types.ts";

Expand Down Expand Up @@ -68,25 +68,23 @@ const DEFAULT_MAX_RETRIES = 3;
const DEFAULT_INITIAL_RETRY_DELAY_MS = 1_000;
const DEFAULT_MAX_RETRY_DELAY_MS = 10_000;

function isBlankConfigValue(value: unknown): boolean {
return typeof value !== "string" || value.trim().length === 0;
}

export function createGitHubConfig(config: GitHubConfig): ResolvedGitHubConfig {
if (!config.token) {
throw toError(
createError({
type: "config",
message:
"GitHub adapter requires a token. Set GITHUB_TOKEN environment variable or provide token in config.",
}),
);
if (isBlankConfigValue(config.token)) {
throw CONFIG_INVALID.create({
detail:
"GitHub adapter requires a token. Set GITHUB_TOKEN environment variable or provide token in config.",
});
}

if (!config.owner || !config.repo) {
throw toError(
createError({
type: "config",
message:
"GitHub adapter requires owner and repo. Provide them in config or via GITHUB_OWNER and GITHUB_REPO environment variables.",
}),
);
if (isBlankConfigValue(config.owner) || isBlankConfigValue(config.repo)) {
throw CONFIG_INVALID.create({
detail:
"GitHub adapter requires owner and repo. Provide them in config or via GITHUB_OWNER and GITHUB_REPO environment variables.",
});
}

return {
Expand Down
175 changes: 157 additions & 18 deletions src/platform/adapters/fs/integration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@ import {
assertExists,
assertInstanceOf,
assertRejects,
assertStrictEquals,
} from "#veryfront/testing/assert.ts";
import { describe, it } from "#veryfront/testing/bdd.ts";
import { withMockFetch } from "#veryfront/testing/mock-fetch.ts";
import {
createFSAdapterFromConfig,
enhanceAdapterWithFS,
Expand Down Expand Up @@ -99,7 +101,7 @@ describe("integration.ts", () => {
assertEquals(getFSAdapterType({ fs: {} }), "local");
});

describe("enhanceAdapterWithFS error fallback", () => {
describe("enhanceAdapterWithFS error propagation", () => {
it("should preserve invalid retry configuration instead of changing filesystems", async () => {
let rejection: unknown;
try {
Expand Down Expand Up @@ -140,28 +142,165 @@ describe("integration.ts", () => {
assertEquals(error.slug, "config-validation-failed");
});

it("should fall back to original adapter for unsupported type", async () => {
const adapter = await enhanceAdapterWithFS(denoAdapter, {
fs: { type: "unsupported-type" as any },
});
assertEquals(adapter, denoAdapter);
it("should fail closed when GitHub repository identity is invalid", async () => {
const error = await assertRejects(
() =>
enhanceAdapterWithFS(denoAdapter, {
fs: {
type: "github",
github: { token: "test-token", owner: "team/other", repo: "repo" },
},
}),
VeryfrontError,
"GitHub owner",
);
assertInstanceOf(error, VeryfrontError);
assertEquals(error.slug, "config-validation-failed");
});

it("should fall back to original adapter for github type without config", async () => {
const adapter = await enhanceAdapterWithFS(denoAdapter, {
fs: { type: "github" },
});
assertEquals(adapter, denoAdapter);
it("should fail closed when the GitHub adapter has no token", async () => {
// token: "" is explicit so the GITHUB_TOKEN environment variable cannot
// satisfy the requirement and mask the regression in CI.
const error = await assertRejects(
() =>
enhanceAdapterWithFS(denoAdapter, {
fs: {
type: "github",
github: { token: "", owner: "owner", repo: "repo" },
},
}),
VeryfrontError,
"token",
);
assertInstanceOf(error, VeryfrontError);
assertEquals(error.slug, "config-invalid");
});

it("should pass projectDir to FSAdapter config", async () => {
// With an unsupported type, it will fail and fall back, but the branch is exercised
const adapter = await enhanceAdapterWithFS(
denoAdapter,
{ fs: { type: "unknown-type" as any } },
"/some/project/dir",
it("should fail closed when the GitHub token contains only whitespace", async () => {
let requests = 0;
const error = await withMockFetch(
() => {
requests += 1;
return Promise.resolve(new Response("Unauthorized", { status: 401 }));
},
() =>
assertRejects(
() =>
enhanceAdapterWithFS(denoAdapter, {
fs: {
type: "github",
github: { token: " ", owner: "owner", repo: "repo" },
},
}),
VeryfrontError,
"token",
),
);
assertEquals(adapter, denoAdapter);
assertInstanceOf(error, VeryfrontError);
assertEquals(error.slug, "config-invalid");
assertEquals(requests, 0);
});

it("should propagate GitHub network initialization failures", async () => {
const networkFailure = new Error("simulated GitHub outage");
const error = await withMockFetch(
() => Promise.reject(networkFailure),
() =>
assertRejects(() =>
enhanceAdapterWithFS(denoAdapter, {
fs: {
type: "github",
github: {
token: "test-token",
owner: "owner",
repo: "repo",
retry: { maxRetries: 1, initialDelay: 0, maxDelay: 0 },
},
},
})
),
);
assertStrictEquals(error, networkFailure);
});

it("should propagate GitHub authentication failures", async () => {
const error = await withMockFetch(
() => Promise.resolve(new Response("Unauthorized", { status: 401 })),
() =>
assertRejects(
() =>
enhanceAdapterWithFS(denoAdapter, {
fs: {
type: "github",
github: {
token: "invalid-token",
owner: "owner",
repo: "repo",
retry: { maxRetries: 1, initialDelay: 0, maxDelay: 0 },
},
},
}),
Error,
"authentication",
),
);
assertInstanceOf(error, Error);
});

it("should propagate unsupported adapter failures", async () => {
await assertRejects(
() =>
enhanceAdapterWithFS(denoAdapter, {
fs: { type: "unsupported-type" as any },
}),
Error,
'FSAdapter type "unsupported-type" is not implemented',
);
});

it("should fail closed for github type without config", async () => {
await assertRejects(
() =>
enhanceAdapterWithFS(denoAdapter, {
fs: { type: "github" },
}),
Error,
"GitHub adapter requires github configuration",
);
});

it("should not consult VeryfrontError Symbol.hasInstance while propagating", async () => {
const originalHasInstance = Object.getOwnPropertyDescriptor(
VeryfrontError,
Symbol.hasInstance,
);
Object.defineProperty(VeryfrontError, Symbol.hasInstance, {
configurable: true,
value() {
throw new Error("poisoned VeryfrontError Symbol.hasInstance was used");
},
});

let caught: unknown;
try {
await enhanceAdapterWithFS(denoAdapter, {
fs: {
type: "github",
github: { token: "test-token", owner: "team/other", repo: "repo" },
},
});
} catch (error) {
caught = error;
} finally {
if (originalHasInstance) {
Object.defineProperty(VeryfrontError, Symbol.hasInstance, originalHasInstance);
} else {
Reflect.deleteProperty(VeryfrontError, Symbol.hasInstance);
}
}

assertInstanceOf(caught, VeryfrontError);
assertEquals(caught.slug, "config-validation-failed");
});
});

Expand Down
Loading