mirror of
https://github.com/garrytan/gbrain.git
synced 2026-08-14 00:48:18 +00:00
put_page: report push state honestly instead of implying it via committed (#3936)
Wave-assembled from PR #3936 by @dovstern. Conflict resolution: kept master's resolveRepoRoot() block AND the PR's exported currentBranch in src/core/brain-repo-durability.ts. Adaptation: the new serial test now writes the simulated push log under $GBRAIN_HOME/.gbrain (CX2-8 parent-dir semantics landed on master after the PR's base). Co-Authored-By: Dov Stern <dovstern@users.noreply.github.com>
This commit is contained in:
committed by
Sina Matian
co-authored by
Dov Stern
parent
45bd04ff9f
commit
44eea64084
@@ -450,6 +450,58 @@ export function commitWriteThroughFile(repoPath: string, absPath: string, slug:
|
||||
}
|
||||
}
|
||||
|
||||
// ── Push-state query (D14) ───────────────────────────────────────────────────
|
||||
|
||||
export type PushLogStatus = 'ok' | 'needs_attention' | 'unknown';
|
||||
|
||||
export interface PushLogOutcome {
|
||||
status: PushLogStatus;
|
||||
detail: string;
|
||||
/** UTC timestamp parsed from the log line, when found. */
|
||||
at?: string;
|
||||
}
|
||||
|
||||
const PUSH_LOG_OK = /^(\S+) \[push\] (?:ok|ok-after-rebase) (\S+)\b/;
|
||||
const PUSH_LOG_LOCAL_ONLY = /^(\S+) \[push\] LOCAL-ONLY, NEEDS ATTENTION: (\S+) /;
|
||||
const PUSH_LOG_LOCK_TIMEOUT = /^(\S+) \[push\] lock-timeout (\S+)\b/;
|
||||
|
||||
/**
|
||||
* Best-effort read of the most recently logged push outcome for `branch`,
|
||||
* from the shared hook log ($GBRAIN_HOME/brain-push.log). The push itself
|
||||
* runs detached in the background (see `renderPostCommitHook`), so nothing
|
||||
* synchronous ever learns whether a given commit's own push landed — this is
|
||||
* the queryable substitute: "as of the last thing the hook logged for this
|
||||
* branch, were pushes landing?"
|
||||
*
|
||||
* The log is host-wide and keyed only by branch name, not repo path, so two
|
||||
* different hardened repos sharing a branch name (e.g. both on `main`) share
|
||||
* this signal. That's an acceptable approximation for a liveness check, not
|
||||
* a per-repo guarantee.
|
||||
*/
|
||||
export function getLastPushOutcome(branch: string): PushLogOutcome {
|
||||
const log = pushLogPath();
|
||||
if (!existsSync(log)) return { status: 'unknown', detail: 'no push attempts logged yet' };
|
||||
|
||||
let lines: string[];
|
||||
try {
|
||||
lines = readFileSync(log, 'utf-8').split('\n');
|
||||
} catch (e) {
|
||||
return { status: 'unknown', detail: `push log unreadable: ${(e as Error).message}` };
|
||||
}
|
||||
|
||||
for (let i = lines.length - 1; i >= 0; i--) {
|
||||
const line = lines[i];
|
||||
if (!line) continue;
|
||||
let m = line.match(PUSH_LOG_OK);
|
||||
if (m && m[2] === branch) return { status: 'ok', detail: line.trim(), at: m[1] };
|
||||
m = line.match(PUSH_LOG_LOCAL_ONLY);
|
||||
if (m && m[2] === branch) return { status: 'needs_attention', detail: line.trim(), at: m[1] };
|
||||
m = line.match(PUSH_LOG_LOCK_TIMEOUT);
|
||||
if (m && m[2] === branch) return { status: 'needs_attention', detail: line.trim(), at: m[1] };
|
||||
}
|
||||
return { status: 'unknown', detail: `no push attempt logged yet for branch '${branch}'` };
|
||||
}
|
||||
|
||||
// ── Committed helper ────────────────────────────────────────────────────────
|
||||
|
||||
function installHelper(repoPath: string, dryRun: boolean): { status: StepStatus; detail: string } {
|
||||
@@ -784,7 +836,7 @@ function resolveRepoRoot(path: string): string {
|
||||
}
|
||||
}
|
||||
|
||||
function currentBranch(repoPath: string): string {
|
||||
export function currentBranch(repoPath: string): string {
|
||||
try {
|
||||
return execFileSync('git', ['-C', repoPath, 'rev-parse', '--abbrev-ref', 'HEAD'], {
|
||||
stdio: ['ignore', 'pipe', 'ignore'], timeout: 10_000, env: { ...process.env, ...GIT_ENV },
|
||||
|
||||
@@ -10,7 +10,7 @@ import { clampSearchLimit } from './engine.ts';
|
||||
import type { GBrainConfig } from './config.ts';
|
||||
import type { PageType } from './types.ts';
|
||||
import { importFromContent } from './import-file.ts';
|
||||
import { writePageThrough } from './write-through.ts';
|
||||
import { writePageThrough, type WriteThroughResult } from './write-through.ts';
|
||||
import { hybridSearch, hybridSearchCached, stampContentFlags, stampUnverifiedExtractions } from './search/hybrid.ts';
|
||||
import { expandQuery } from './search/expansion.ts';
|
||||
import { dedupResults } from './search/dedup.ts';
|
||||
@@ -1323,7 +1323,10 @@ const put_page: Operation = {
|
||||
// Trust gating:
|
||||
// - Subagent sandbox (viaSubagent without allowedSlugPrefixes) → DB-only.
|
||||
// - All other writes → write-through.
|
||||
let writeThrough: { written: boolean; path?: string; skipped?: string; error?: string } | undefined;
|
||||
// put_page's own trust-gating produces two skip reasons ('subagent_sandbox',
|
||||
// 'dry_run') that never come out of writePageThrough itself — widen the
|
||||
// field rather than losing the commit/pushed/lastPushStatus typing.
|
||||
let writeThrough: (Omit<WriteThroughResult, 'skipped'> & { skipped?: WriteThroughResult['skipped'] | 'subagent_sandbox' | 'dry_run' }) | undefined;
|
||||
const isSandboxSubagent = ctx.viaSubagent === true
|
||||
&& !(Array.isArray(ctx.allowedSlugPrefixes) && ctx.allowedSlugPrefixes.length > 0);
|
||||
if (!ctx.dryRun && result.status !== 'error' && !isSandboxSubagent) {
|
||||
|
||||
@@ -27,7 +27,10 @@ import { randomBytes } from 'crypto';
|
||||
import type { BrainEngine } from './engine.ts';
|
||||
import { serializePageToMarkdown, resolvePageFilePath } from './markdown.ts';
|
||||
import { isWriteTargetContained } from './path-confine.ts';
|
||||
import { isDurabilityHardened, commitWriteThroughFile } from './brain-repo-durability.ts';
|
||||
import {
|
||||
isDurabilityHardened, commitWriteThroughFile, currentBranch, getLastPushOutcome,
|
||||
type PushLogOutcome,
|
||||
} from './brain-repo-durability.ts';
|
||||
|
||||
/** Minimal logger surface — structurally compatible with operations.ts `Logger`. */
|
||||
export interface WriteThroughLogger {
|
||||
@@ -39,11 +42,28 @@ export interface WriteThroughResult {
|
||||
path?: string;
|
||||
/**
|
||||
* True when the write was also committed to git (#2426). Only attempted on
|
||||
* repos hardened via `gbrain sources harden` (durability hook installed);
|
||||
* the hook then background-pushes the commit. Best-effort — a false/absent
|
||||
* value never blocks the write.
|
||||
* repos hardened via `gbrain sources harden` (durability hook installed).
|
||||
* Commit-only — this says nothing about whether the commit ever reached the
|
||||
* remote. Best-effort — a false/absent value never blocks the write.
|
||||
*/
|
||||
committed?: boolean;
|
||||
/**
|
||||
* Set alongside `committed: true`. The actual push runs detached in the
|
||||
* post-commit hook (see brain-repo-durability.ts), so at the moment this
|
||||
* result is returned the outcome for THIS commit is genuinely unknown —
|
||||
* 'pending' is the only honest value. Check `lastPushStatus` (or
|
||||
* $GBRAIN_HOME/brain-push.log directly) afterward to see whether pushes for
|
||||
* this branch are landing.
|
||||
*/
|
||||
pushed?: 'pending';
|
||||
/**
|
||||
* Best-effort snapshot of the most recently logged push outcome for this
|
||||
* branch (read from the hook's shared log), taken right after the commit
|
||||
* above. It reflects push history UP TO that point — not the push this
|
||||
* write just queued — so callers and health tooling can tell "pushes for
|
||||
* this branch have been failing" apart from "this write committed fine".
|
||||
*/
|
||||
lastPushStatus?: PushLogOutcome;
|
||||
/**
|
||||
* Non-error reasons the file was not written:
|
||||
* - no_repo_configured: the resolved target (source `local_path` or, for a
|
||||
@@ -284,13 +304,23 @@ export async function writePageThrough(
|
||||
// post-commit hook background-pushes the commit. Best-effort: a commit
|
||||
// failure never fails the write (the DB row + file are the durable sinks).
|
||||
let committed = false;
|
||||
let pushed: 'pending' | undefined;
|
||||
let lastPushStatus: PushLogOutcome | undefined;
|
||||
try {
|
||||
if (isDurabilityHardened(writeRoot)) {
|
||||
committed = commitWriteThroughFile(writeRoot, filePath, slug);
|
||||
if (committed) {
|
||||
pushed = 'pending';
|
||||
lastPushStatus = getLastPushOutcome(currentBranch(writeRoot));
|
||||
}
|
||||
}
|
||||
} catch { /* best-effort */ }
|
||||
|
||||
return { written: true, path: filePath, ...(committed ? { committed } : {}) };
|
||||
return {
|
||||
written: true,
|
||||
path: filePath,
|
||||
...(committed ? { committed, pushed, lastPushStatus } : {}),
|
||||
};
|
||||
} catch (e) {
|
||||
const msg = e instanceof Error ? e.message : String(e);
|
||||
opts.logger?.warn(`[write-through] failed for ${slug}: ${msg}`);
|
||||
|
||||
@@ -0,0 +1,144 @@
|
||||
/**
|
||||
* put_page's write-through response on a durability-hardened repo reports
|
||||
* `committed: true` for both a successful and a FAILED background push — the
|
||||
* commit lands locally either way, but the push runs detached in the
|
||||
* post-commit hook, so the caller had no field to distinguish "pushed fine"
|
||||
* from "still local-only". This pins the honest contract: `committed` is
|
||||
* commit-only, `pushed: 'pending'` says the push outcome isn't known yet, and
|
||||
* `lastPushStatus` surfaces the hook's own log so a caller (or health
|
||||
* tooling) can see whether pushes for this branch are currently landing.
|
||||
*/
|
||||
|
||||
import { describe, test, expect, beforeAll, beforeEach, afterEach, afterAll } from 'bun:test';
|
||||
import { mkdtempSync, writeFileSync, rmSync, mkdirSync, chmodSync, appendFileSync } from 'fs';
|
||||
import { execSync, execFileSync } from 'child_process';
|
||||
import { tmpdir } from 'os';
|
||||
import { join } from 'path';
|
||||
import { PGLiteEngine } from '../src/core/pglite-engine.ts';
|
||||
import { resetPgliteState } from './helpers/reset-pglite.ts';
|
||||
import { operations } from '../src/core/operations.ts';
|
||||
import type { OperationContext } from '../src/core/operations.ts';
|
||||
import { configureGateway, resetGateway, __setEmbedTransportForTests } from '../src/core/ai/gateway.ts';
|
||||
|
||||
const putPageOp = operations.find((o) => o.name === 'put_page')!;
|
||||
|
||||
let engine: PGLiteEngine;
|
||||
let repo: string;
|
||||
let gbrainHome: string;
|
||||
let oldGbrainHome: string | undefined;
|
||||
|
||||
function git(cwd: string, ...args: string[]): string {
|
||||
return execFileSync('git', ['-C', cwd, ...args], {
|
||||
stdio: ['ignore', 'pipe', 'pipe'], encoding: 'utf-8',
|
||||
}).trim();
|
||||
}
|
||||
|
||||
/** Same fixture as write-through-commit.serial.test.ts: a hook file carrying
|
||||
* the gbrain banner (the only thing `isDurabilityHardened` checks) with a
|
||||
* no-op body so tests never attempt a real network push. */
|
||||
function installFakeDurabilityHook(repoPath: string): void {
|
||||
const hooksDir = join(repoPath, '.git', 'hooks');
|
||||
mkdirSync(hooksDir, { recursive: true });
|
||||
const hookPath = join(hooksDir, 'post-commit');
|
||||
writeFileSync(hookPath, [
|
||||
'#!/usr/bin/env bash',
|
||||
'# gbrain brain-durability post-commit hook (v0.42.44+)',
|
||||
'exit 0',
|
||||
'',
|
||||
].join('\n'));
|
||||
chmodSync(hookPath, 0o755);
|
||||
}
|
||||
|
||||
function makeCtx(opts: Partial<OperationContext> = {}): OperationContext {
|
||||
return {
|
||||
engine,
|
||||
config: { engine: 'pglite' as const },
|
||||
logger: { info: () => {}, warn: () => {}, error: () => {} },
|
||||
dryRun: false,
|
||||
remote: false,
|
||||
sourceId: 'default',
|
||||
...opts,
|
||||
};
|
||||
}
|
||||
|
||||
describe('put_page write-through — commit/push reporting on a hardened repo', () => {
|
||||
beforeAll(async () => {
|
||||
configureGateway({
|
||||
embedding_model: 'openai:text-embedding-3-large',
|
||||
embedding_dimensions: 1536,
|
||||
env: { ...process.env, OPENAI_API_KEY: process.env.OPENAI_API_KEY || 'sk-test-stub' },
|
||||
});
|
||||
__setEmbedTransportForTests(async ({ values }: any) => ({
|
||||
embeddings: values.map(() => new Array(1536).fill(0)),
|
||||
usage: { tokens: 0 },
|
||||
}) as any);
|
||||
|
||||
engine = new PGLiteEngine();
|
||||
await engine.connect({});
|
||||
await engine.initSchema();
|
||||
}, 60_000);
|
||||
|
||||
afterAll(async () => {
|
||||
await engine.disconnect();
|
||||
__setEmbedTransportForTests(null);
|
||||
resetGateway();
|
||||
}, 60_000);
|
||||
|
||||
beforeEach(async () => {
|
||||
await resetPgliteState(engine);
|
||||
repo = mkdtempSync(join(tmpdir(), 'gbrain-ppr-'));
|
||||
execSync('git init -q -b main', { cwd: repo, stdio: 'pipe' });
|
||||
execSync('git config user.email "t@t.t"', { cwd: repo, stdio: 'pipe' });
|
||||
execSync('git config user.name "T"', { cwd: repo, stdio: 'pipe' });
|
||||
writeFileSync(join(repo, 'seed.md'), 'seed\n');
|
||||
execSync('git add -A && git commit -qm init', { cwd: repo, stdio: 'pipe' });
|
||||
installFakeDurabilityHook(repo);
|
||||
await engine.setConfig('sync.repo_path', repo);
|
||||
|
||||
gbrainHome = mkdtempSync(join(tmpdir(), 'gbrain-home-'));
|
||||
oldGbrainHome = process.env.GBRAIN_HOME;
|
||||
process.env.GBRAIN_HOME = gbrainHome;
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
if (oldGbrainHome === undefined) delete process.env.GBRAIN_HOME; else process.env.GBRAIN_HOME = oldGbrainHome;
|
||||
if (repo) rmSync(repo, { recursive: true, force: true });
|
||||
if (gbrainHome) rmSync(gbrainHome, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
test('reports committed + pending push, not an implied success, with no push history yet', async () => {
|
||||
const res: any = await putPageOp.handler(makeCtx(), {
|
||||
slug: 'notes/ppr-fresh',
|
||||
content: '---\ntype: concept\ntitle: PPR Fresh\n---\n\nbody',
|
||||
});
|
||||
|
||||
expect(res.write_through.committed).toBe(true);
|
||||
expect(res.write_through.pushed).toBe('pending');
|
||||
expect(res.write_through.lastPushStatus.status).toBe('unknown');
|
||||
});
|
||||
|
||||
test('surfaces a prior LOCAL-ONLY push failure instead of hiding it behind committed:true', async () => {
|
||||
// Simulate the hook having already logged an unresolved push failure for
|
||||
// this branch (e.g. from an earlier write in the same session).
|
||||
// CX2-8: GBRAIN_HOME is a PARENT dir — the resolved home is
|
||||
// $GBRAIN_HOME/.gbrain, so the push log lives under it.
|
||||
mkdirSync(join(gbrainHome, '.gbrain'), { recursive: true });
|
||||
appendFileSync(
|
||||
join(gbrainHome, '.gbrain', 'brain-push.log'),
|
||||
'2025-01-01T00:00:00Z [push] LOCAL-ONLY, NEEDS ATTENTION: main @ deadbee could not reach origin. Run: gbrain sources pull <id> && git push\n',
|
||||
);
|
||||
|
||||
const res: any = await putPageOp.handler(makeCtx(), {
|
||||
slug: 'notes/ppr-broken-push',
|
||||
content: '---\ntype: concept\ntitle: PPR Broken Push\n---\n\nbody',
|
||||
});
|
||||
|
||||
// The commit itself still succeeds (git commit doesn't touch the network) —
|
||||
// that's the honest part of `committed: true`. What must NOT happen is the
|
||||
// caller reading `committed: true` as "this is durably on the remote".
|
||||
expect(res.write_through.committed).toBe(true);
|
||||
expect(res.write_through.pushed).toBe('pending');
|
||||
expect(res.write_through.lastPushStatus.status).toBe('needs_attention');
|
||||
expect(res.write_through.lastPushStatus.detail).toContain('NEEDS ATTENTION');
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user