mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-11-14 10:39:32 +01:00
fix(discord): report a refused post as a failure, bound the file probe, drop threads of a previous channel
- codeman notify exits 1 when the route answers 200 with data.ok false - notify request times out at 30 s instead of hanging - fileProblem probes the file with the bounded probe before realpath - the thread map records its parent channel; a new channel starts afresh - threadFor returns its failure instead of sharing this.last
This commit is contained in:
1 parent
0db9b55961
commit
1ab54b9e9b
7 files changed
+142
-29
No files matched your search
+19
-5
@@ -14,6 +14,8 @@ import https from 'node:https';
|
||||
import { isAbsolute, join } from 'node:path';
|
||||
import { readCodemanCredentials } from './codeman-credentials.js';
|
||||
|
||||
const REQUEST_TIMEOUT_MS = 30_000;
|
||||
|
||||
export interface NotifyOptions {
|
||||
message: string;
|
||||
urgent?: boolean;
|
||||
@@ -32,7 +34,7 @@ export type JsonPost = (
|
||||
apiUrl: string,
|
||||
apiPath: string,
|
||||
payload: unknown
|
||||
) => Promise<{ status: number; error?: string }>;
|
||||
) => Promise<{ status: number; error?: string; delivery?: { ok: boolean; error?: string } }>;
|
||||
|
||||
/** POST JSON to the Codeman API with the same credentials `codeman attach` uses. Status 0 means no answer. */
|
||||
export const postJson: JsonPost = (apiUrl, apiPath, payload) => {
|
||||
@@ -56,21 +58,28 @@ export const postJson: JsonPost = (apiUrl, apiPath, payload) => {
|
||||
path: `${url.pathname}${url.search}`,
|
||||
rejectUnauthorized: false,
|
||||
headers,
|
||||
timeout: REQUEST_TIMEOUT_MS,
|
||||
},
|
||||
(res) => {
|
||||
let text = '';
|
||||
res.on('data', (c) => (text += c));
|
||||
res.on('end', () => {
|
||||
let error: string | undefined;
|
||||
let delivery: { ok: boolean; error?: string } | undefined;
|
||||
try {
|
||||
error = (JSON.parse(text) as { error?: string }).error;
|
||||
const body = JSON.parse(text) as { error?: string; data?: { ok?: unknown; error?: string } };
|
||||
error = body.error;
|
||||
// A 200 only says Codeman took the request; `data.ok` says whether Discord took the post.
|
||||
if (typeof body.data?.ok === 'boolean') delivery = { ok: body.data.ok, error: body.data.error };
|
||||
} catch {
|
||||
/* not JSON */
|
||||
}
|
||||
resolve({ status: res.statusCode ?? 0, error });
|
||||
resolve({ status: res.statusCode ?? 0, error, delivery });
|
||||
});
|
||||
}
|
||||
);
|
||||
// A server that takes the connection and never answers must not hang the agent: status 0 is "no answer".
|
||||
req.on('timeout', () => req.destroy());
|
||||
req.on('error', () => resolve({ status: 0 }));
|
||||
req.write(body);
|
||||
req.end();
|
||||
@@ -91,12 +100,17 @@ export async function runNotify(
|
||||
const payload: { message: string; urgent?: boolean; file?: string } = { message: opts.message };
|
||||
if (opts.urgent) payload.urgent = true;
|
||||
if (opts.file) payload.file = isAbsolute(opts.file) ? opts.file : join(cwd, opts.file);
|
||||
const { status, error } = await post(
|
||||
const { status, error, delivery } = await post(
|
||||
apiUrl,
|
||||
`/api/sessions/${encodeURIComponent(sessionId)}/discord-notify`,
|
||||
payload
|
||||
);
|
||||
if (status >= 200 && status < 300) return { code: 0, out: 'Posted to Discord' };
|
||||
if (status >= 200 && status < 300) {
|
||||
if (delivery?.ok === false) {
|
||||
return { code: 1, err: `Could not post to Discord${delivery.error ? `: ${delivery.error}` : ''}` };
|
||||
}
|
||||
return { code: 0, out: 'Posted to Discord' };
|
||||
}
|
||||
// The bridge being off is not a failure: the caller just says it in the terminal.
|
||||
if (status === 409) return { code: 0, out: 'The Discord bridge is off. Say it in the terminal instead.' };
|
||||
if (status === 429)
|
||||
|
||||
+44
-21
@@ -306,20 +306,32 @@ export async function writeDiscordConfig(configDir: string, cfg: DiscordConfig):
|
||||
await writePrivate(discordConfigPath(configDir), coerce(cfg), 0o600);
|
||||
}
|
||||
|
||||
export async function readThreadMap(configDir: string): Promise<Record<string, string>> {
|
||||
/** The thread map plus the parent channel it was built under (empty for an older flat file). */
|
||||
async function readThreadFile(configDir: string): Promise<{ parent: string; threads: Record<string, string> }> {
|
||||
try {
|
||||
const raw = JSON.parse(await fs.readFile(join(configDir, THREADS_FILE), 'utf-8')) as Record<string, unknown>;
|
||||
const out: Record<string, string> = {};
|
||||
for (const [k, v] of Object.entries(raw)) if (typeof v === 'string' && SNOWFLAKE.test(v)) out[k] = v;
|
||||
return out;
|
||||
const nested = typeof raw.threads === 'object' && raw.threads !== null;
|
||||
const entries = Object.entries(nested ? (raw.threads as Record<string, unknown>) : raw);
|
||||
const threads: Record<string, string> = {};
|
||||
for (const [k, v] of entries) if (typeof v === 'string' && SNOWFLAKE.test(v)) threads[k] = v;
|
||||
const parent = nested && typeof raw.parentChannelId === 'string' ? raw.parentChannelId : '';
|
||||
return { parent, threads };
|
||||
} catch {
|
||||
return {};
|
||||
return { parent: '', threads: {} };
|
||||
}
|
||||
}
|
||||
|
||||
async function writeThreadMap(configDir: string, map: Record<string, string>): Promise<void> {
|
||||
export async function readThreadMap(configDir: string): Promise<Record<string, string>> {
|
||||
return (await readThreadFile(configDir)).threads;
|
||||
}
|
||||
|
||||
async function writeThreadMap(
|
||||
configDir: string,
|
||||
parentChannelId: string,
|
||||
threads: Record<string, string>
|
||||
): Promise<void> {
|
||||
if (!existsSync(configDir)) mkdirSync(configDir, { recursive: true });
|
||||
await writePrivate(join(configDir, THREADS_FILE), map, 0o600);
|
||||
await writePrivate(join(configDir, THREADS_FILE), { parentChannelId, threads }, 0o600);
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
@@ -657,7 +669,9 @@ export class DiscordBridge {
|
||||
private names = new Map<string, string>();
|
||||
/** Brand-new sessions that have not yet been sent the startup hint (non-Claude CLIs). */
|
||||
private fresh = new Set<string>();
|
||||
private pending = new Map<string, Promise<string | null>>();
|
||||
private pending = new Map<string, Promise<string | DiscordResult>>();
|
||||
/** The parent channel the thread map was built under. */
|
||||
private threadsParent = '';
|
||||
/** Thread-map writes run one after another: concurrent creations must not interleave on the tmp file. */
|
||||
private persistChain: Promise<void> = Promise.resolve();
|
||||
private mapLoad?: Promise<void>;
|
||||
@@ -693,11 +707,23 @@ export class DiscordBridge {
|
||||
this.cfg = await readDiscordConfig(this.deps.configDir);
|
||||
// The thread map is read once, by whoever gets here first; everyone else waits for that read. A second
|
||||
// read finishing later would replace the map and drop threads made in between.
|
||||
this.mapLoad ??= readThreadMap(this.deps.configDir).then((saved) => {
|
||||
this.threads = { ...saved, ...this.threads };
|
||||
this.mapLoad ??= readThreadFile(this.deps.configDir).then((saved) => {
|
||||
this.threads = { ...saved.threads, ...this.threads };
|
||||
this.threadsParent ||= saved.parent;
|
||||
this.loaded = true;
|
||||
});
|
||||
await this.mapLoad;
|
||||
// Threads belong to the channel they were made under: a new parent channel (or guild) starts afresh,
|
||||
// or every tab would keep posting into the old one.
|
||||
const parent = this.cfg.parentChannelId;
|
||||
if (parent && parent !== this.threadsParent) {
|
||||
if (this.threadsParent !== '') {
|
||||
this.threads = {};
|
||||
this.pending.clear();
|
||||
}
|
||||
this.threadsParent = parent;
|
||||
await this.persistThreads();
|
||||
}
|
||||
this.gateway?.stop();
|
||||
this.gateway = null;
|
||||
activeStartupHint = this.cfg.enabled && this.cfg.token && this.cfg.startupHint ? DISCORD_STARTUP_HINT : undefined;
|
||||
@@ -742,13 +768,13 @@ export class DiscordBridge {
|
||||
|
||||
private persistThreads(): Promise<void> {
|
||||
this.persistChain = this.persistChain
|
||||
.then(() => writeThreadMap(this.deps.configDir, { ...this.threads }))
|
||||
.then(() => writeThreadMap(this.deps.configDir, this.threadsParent, { ...this.threads }))
|
||||
.catch(() => undefined);
|
||||
return this.persistChain;
|
||||
}
|
||||
|
||||
/** The thread for a session, creating it when absent. Concurrent callers share one creation. */
|
||||
private threadFor(sessionId: string, name: string | undefined): Promise<string | null> {
|
||||
private threadFor(sessionId: string, name: string | undefined): Promise<string | DiscordResult> {
|
||||
const existing = this.threads[sessionId];
|
||||
if (existing) return Promise.resolve(existing);
|
||||
const inflight = this.pending.get(sessionId);
|
||||
@@ -756,16 +782,12 @@ export class DiscordBridge {
|
||||
const p = (async () => {
|
||||
try {
|
||||
const t = await this.api.createThread(this.cfg.parentChannelId, threadNameFor(name, sessionId));
|
||||
if (!t?.id) {
|
||||
this.last = this.fail(new Error('Discord did not return a thread'));
|
||||
return null;
|
||||
}
|
||||
if (!t?.id) return this.fail(new Error('Discord did not return a thread'));
|
||||
this.threads[sessionId] = t.id;
|
||||
await this.persistThreads();
|
||||
return t.id;
|
||||
} catch (err) {
|
||||
this.last = this.fail(err);
|
||||
return null;
|
||||
return this.fail(err);
|
||||
} finally {
|
||||
this.pending.delete(sessionId);
|
||||
}
|
||||
@@ -799,7 +821,7 @@ export class DiscordBridge {
|
||||
let target = this.cfg.parentChannelId;
|
||||
if (msg.sessionId) {
|
||||
const thread = await this.threadFor(msg.sessionId, msg.sessionName);
|
||||
if (!thread) return this.last;
|
||||
if (typeof thread !== 'string') return (this.last = thread);
|
||||
target = thread;
|
||||
}
|
||||
const send = (to: string) => (msg.file ? this.api.postWithFile(to, post, msg.file) : this.api.post(to, post));
|
||||
@@ -810,7 +832,7 @@ export class DiscordBridge {
|
||||
if (msg.sessionId && err instanceof DiscordHttpError && err.status === 404) {
|
||||
delete this.threads[msg.sessionId];
|
||||
const fresh = await this.threadFor(msg.sessionId, msg.sessionName);
|
||||
if (!fresh) return this.last;
|
||||
if (typeof fresh !== 'string') return (this.last = fresh);
|
||||
await send(fresh);
|
||||
} else {
|
||||
throw err;
|
||||
@@ -852,7 +874,8 @@ export class DiscordBridge {
|
||||
// Only a tab made just now: the same event also fires for sessions restored at server start.
|
||||
if (createdAt !== undefined && this.now() - createdAt < 60_000) this.fresh.add(sessionId);
|
||||
if (!this.cfg.enabled || this.cfg.threadCreation !== 'onSessionStart') return;
|
||||
await this.threadFor(sessionId, name);
|
||||
const made = await this.threadFor(sessionId, name);
|
||||
if (typeof made !== 'string') this.last = made;
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -96,6 +96,10 @@ async function fileProblem(
|
||||
if ((await probePathKind(workingDir, { pastCap: true })) !== 'directory') {
|
||||
return describeUnknownPath('The session folder', workingDir, { pastCap: true });
|
||||
}
|
||||
// The agent chose this path: probe it bounded too, before realpath can hold a worker on a dead mount.
|
||||
const kind = await probePathKind(file, { pastCap: true });
|
||||
if (kind === 'unknown') return describeUnknownPath('The file', file, { pastCap: true });
|
||||
if (kind !== 'file') return 'Not a regular file';
|
||||
try {
|
||||
const [root, real] = await Promise.all([realpath(workingDir), realpath(file)]);
|
||||
const rel = relative(root, real);
|
||||
|
||||
Reference in new issue
Block a user