mirror of
https://github.com/Kaelio/ktx.git
synced 2026-06-19 08:28:06 +02:00
fix(ingest): honor storage.git.auto_commit and memory.auto_commit
Both documented flags were read only for status display; every ingest path squash-committed to main unconditionally, so setting either to false was a silent no-op (the reported symptom: 'Memory ingest (external_ingest): ...' commits despite memory.auto_commit: false). Gate the commit at the squash-merge onto main — the one point where ingest work becomes a permanent commit (intermediate session-worktree commits must still happen for the squash to collapse). When auto-commit is off, apply the squash to main's working tree and leave it staged instead of committing, so the run is never silently discarded: - GitService.stageSquashMergeIntoMain: shares the merge core with squashMergeIntoMain but stops before committing and returns the staged tree SHA (a valid diff/read ref). - memory.auto_commit gates MemoryAgentService (its DB writes are eager, so the staged files stay consistent); the commit-message job is skipped. - storage.git.auto_commit gates IngestBundleRunner; the wiki index is reconciled from the staged tree via the existing syncFromCommit (git diff/show accept a write-tree ref), and SL reindex already reads from files. Config descriptions now state precisely what each flag gates and the staged semantics when false.
This commit is contained in:
Andrey Avtomonov
parent
1a6da14f62
commit
a02fcab487
15 changed files with 303 additions and 43 deletions
|
|
@ -31,6 +31,10 @@ export type SquashMergeResult =
|
|||
| { ok: true; squashSha: string; touchedPaths: string[] }
|
||||
| { ok: false; conflict: true; conflictPaths: string[] };
|
||||
|
||||
export type StageSquashResult =
|
||||
| { ok: true; touchedPaths: string[]; stagedTree: string }
|
||||
| { ok: false; conflict: true; conflictPaths: string[] };
|
||||
|
||||
function mergeErrorMessage(error: unknown): string {
|
||||
if (error instanceof Error) {
|
||||
return error.message;
|
||||
|
|
@ -681,6 +685,53 @@ export class GitService {
|
|||
authorEmail: string,
|
||||
commitMessage: string,
|
||||
): Promise<SquashMergeResult> {
|
||||
const applied = await this.applySquashToIndex(branch);
|
||||
if (!applied.ok) {
|
||||
return applied;
|
||||
}
|
||||
if (applied.touchedPaths.length === 0) {
|
||||
const head = (await this.git.revparse(['HEAD'])).trim();
|
||||
return { ok: true, squashSha: head, touchedPaths: [] };
|
||||
}
|
||||
|
||||
await this.git.commit(commitMessage, { '--author': `${author} <${authorEmail}>` });
|
||||
const squashSha = (await this.git.revparse(['HEAD'])).trim();
|
||||
return { ok: true, squashSha, touchedPaths: applied.touchedPaths };
|
||||
}
|
||||
|
||||
/**
|
||||
* Like {@link squashMergeIntoMain} but stops before committing: applies `branch` onto the
|
||||
* current branch's index + working tree and leaves the result staged for the user to commit.
|
||||
* Returns the staged tree's SHA, which is a valid diff/read ref (`git diff A..<tree>`,
|
||||
* `git show <tree>:path`) so callers can reconcile derived indexes without a commit.
|
||||
*
|
||||
* This backs the `auto_commit: false` ingest path: changes still reach the working tree (so
|
||||
* the run is not silently discarded), they are just not committed automatically.
|
||||
*
|
||||
* Caller must hold the `config:repo` lock, as with {@link squashMergeIntoMain}.
|
||||
*/
|
||||
async stageSquashMergeIntoMain(branch: string): Promise<StageSquashResult> {
|
||||
return this.withMutationQueue(() => this.stageSquashMergeIntoMainUnlocked(branch));
|
||||
}
|
||||
|
||||
private async stageSquashMergeIntoMainUnlocked(branch: string): Promise<StageSquashResult> {
|
||||
const applied = await this.applySquashToIndex(branch);
|
||||
if (!applied.ok) {
|
||||
return applied;
|
||||
}
|
||||
const stagedTree = (await this.git.raw(['write-tree'])).trim();
|
||||
return { ok: true, touchedPaths: applied.touchedPaths, stagedTree };
|
||||
}
|
||||
|
||||
/**
|
||||
* Shared core of the squash-merge paths: applies `branch` onto the current branch's index +
|
||||
* working tree via `git merge --squash` WITHOUT committing, leaving the caller to either
|
||||
* commit (auto-commit on) or stage (auto-commit off). Returns the touched paths, or conflict
|
||||
* info after restoring a clean tree.
|
||||
*/
|
||||
private async applySquashToIndex(
|
||||
branch: string,
|
||||
): Promise<{ ok: true; touchedPaths: string[] } | { ok: false; conflict: true; conflictPaths: string[] }> {
|
||||
// Diff of HEAD..branch (two dots) lists commits/files reachable from `branch` that
|
||||
// aren't on HEAD — i.e. exactly what the squash would apply. Three dots (HEAD...branch)
|
||||
// is symmetric difference and would mis-classify cases where main moved ahead.
|
||||
|
|
@ -690,8 +741,7 @@ export class GitService {
|
|||
.map((l) => l.trim())
|
||||
.filter(Boolean);
|
||||
if (touchedPaths.length === 0) {
|
||||
const head = (await this.git.revparse(['HEAD'])).trim();
|
||||
return { ok: true, squashSha: head, touchedPaths: [] };
|
||||
return { ok: true, touchedPaths: [] };
|
||||
}
|
||||
|
||||
// `git merge --squash` may NOT throw on a textual conflict — it stages the clean
|
||||
|
|
@ -724,9 +774,7 @@ export class GitService {
|
|||
return { ok: false, conflict: true, conflictPaths };
|
||||
}
|
||||
|
||||
await this.git.commit(commitMessage, { '--author': `${author} <${authorEmail}>` });
|
||||
const squashSha = (await this.git.revparse(['HEAD'])).trim();
|
||||
return { ok: true, squashSha, touchedPaths };
|
||||
return { ok: true, touchedPaths };
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -2677,29 +2677,51 @@ export class IngestBundleRunner {
|
|||
throw error;
|
||||
}
|
||||
const commitMessage = this.buildCommitMessage(job, syncId, diffSummary, failedWorkUnits);
|
||||
// With auto-commit disabled, apply the run onto main's working tree and leave it staged
|
||||
// rather than committing. The wiki index is reconciled from the staged tree (a valid
|
||||
// diff/read ref), so search stays consistent with the staged files; only the git commit
|
||||
// and its message-enhancement job are skipped.
|
||||
const autoCommit = this.deps.storage.autoCommit;
|
||||
const squashResult = await this.deps.lockingService.withLock('config:repo', async () => {
|
||||
const preSquashSha = await this.deps.gitService.revParseHead();
|
||||
const merge = await this.deps.gitService.squashMergeIntoMain(
|
||||
sessionWorktree.branch,
|
||||
this.deps.storage.systemGitAuthor.name,
|
||||
this.deps.storage.systemGitAuthor.email,
|
||||
commitMessage,
|
||||
);
|
||||
return { preSquashSha, merge };
|
||||
if (autoCommit) {
|
||||
const merge = await this.deps.gitService.squashMergeIntoMain(
|
||||
sessionWorktree.branch,
|
||||
this.deps.storage.systemGitAuthor.name,
|
||||
this.deps.storage.systemGitAuthor.email,
|
||||
commitMessage,
|
||||
);
|
||||
return { preSquashSha, committed: true as const, merge };
|
||||
}
|
||||
const merge = await this.deps.gitService.stageSquashMergeIntoMain(sessionWorktree.branch);
|
||||
return { preSquashSha, committed: false as const, merge };
|
||||
});
|
||||
const mergeResult = squashResult.merge;
|
||||
if (!mergeResult.ok) {
|
||||
if (!squashResult.merge.ok) {
|
||||
await this.deps.runs.markFailed(runRow.id);
|
||||
throw new Error(`squash merge conflict: ${mergeResult.conflictPaths.join(', ')}`);
|
||||
throw new Error(`squash merge conflict: ${squashResult.merge.conflictPaths.join(', ')}`);
|
||||
}
|
||||
const touchedPaths = squashResult.merge.touchedPaths;
|
||||
const hasChanges = touchedPaths.length > 0;
|
||||
// `syncRef` is the tree-ish to diff/read when reconciling the wiki index: the new commit
|
||||
// SHA when committed, the staged tree SHA when staging. `commitSha` is only set when an
|
||||
// actual commit was created (it surfaces in the report and progress UI).
|
||||
let commitSha: string | null = null;
|
||||
let syncRef: string | null = null;
|
||||
if (hasChanges) {
|
||||
if (squashResult.committed) {
|
||||
commitSha = squashResult.merge.squashSha;
|
||||
syncRef = commitSha;
|
||||
} else {
|
||||
syncRef = squashResult.merge.stagedTree;
|
||||
}
|
||||
}
|
||||
const commitSha = mergeResult.touchedPaths.length === 0 ? null : mergeResult.squashSha;
|
||||
await runTrace.event(
|
||||
'debug',
|
||||
'squash',
|
||||
'squash_finished',
|
||||
{
|
||||
commitSha,
|
||||
touchedPaths: mergeResult.touchedPaths,
|
||||
touchedPaths,
|
||||
},
|
||||
undefined,
|
||||
Date.now() - squashStartedAt,
|
||||
|
|
@ -2714,18 +2736,28 @@ export class IngestBundleRunner {
|
|||
wikiCount: countMemoryFlowActions(memoryFlowSavedActions, 'wiki'),
|
||||
slCount: countMemoryFlowActions(memoryFlowSavedActions, 'sl'),
|
||||
});
|
||||
await stage6?.updateProgress(1.0, commitSha ? `Saved changes (${commitSha.slice(0, 8)})` : 'No changes to save');
|
||||
await stage6?.updateProgress(
|
||||
1.0,
|
||||
commitSha
|
||||
? `Saved changes (${commitSha.slice(0, 8)})`
|
||||
: hasChanges
|
||||
? 'Staged changes (auto-commit disabled)'
|
||||
: 'No changes to save',
|
||||
);
|
||||
|
||||
// Sync the shared `knowledge` index from the squashed diff in a single
|
||||
// transaction. If this throws, the run fails and no partial index state
|
||||
// survives (thanks to the transactional upsert in applyDiffTransactional).
|
||||
if (commitSha) {
|
||||
// `syncRef` is the new commit when committed, or the staged tree when staging.
|
||||
if (syncRef) {
|
||||
const indexSyncStartedAt = Date.now();
|
||||
// Multi-file squash → omit path so the handler diffs the whole commit
|
||||
// (a comma-joined pathspec would match nothing and the job would no-op).
|
||||
const pathFilter = mergeResult.touchedPaths.length === 1 ? mergeResult.touchedPaths[0] : '';
|
||||
await this.deps.commitMessages.enqueueForExternalCommit({ commitHash: commitSha }, commitMessage, pathFilter);
|
||||
await this.deps.wikiService.syncFromCommit(squashResult.preSquashSha, commitSha, runRow.id);
|
||||
const pathFilter = touchedPaths.length === 1 ? touchedPaths[0] : '';
|
||||
if (squashResult.committed) {
|
||||
await this.deps.commitMessages.enqueueForExternalCommit({ commitHash: syncRef }, commitMessage, pathFilter);
|
||||
}
|
||||
await this.deps.wikiService.syncFromCommit(squashResult.preSquashSha, syncRef, runRow.id);
|
||||
await this.syncKnowledgeSlRefsFromActions(job.connectionId, memoryFlowSavedActions);
|
||||
const touchedConnections = [
|
||||
...new Set(
|
||||
|
|
|
|||
|
|
@ -129,6 +129,10 @@ class LocalIngestStorage implements IngestStoragePort {
|
|||
this.homeDir = join(project.projectDir, '.ktx');
|
||||
}
|
||||
|
||||
get autoCommit(): boolean {
|
||||
return this.project.config.storage.git.auto_commit;
|
||||
}
|
||||
|
||||
resolveUploadDir(uploadId: string): string {
|
||||
return join(this.project.projectDir, '.ktx/cache/local-ingest', uploadId, 'upload');
|
||||
}
|
||||
|
|
|
|||
|
|
@ -159,6 +159,11 @@ interface IngestGitAuthor {
|
|||
export interface IngestStoragePort {
|
||||
homeDir: string;
|
||||
systemGitAuthor: IngestGitAuthor;
|
||||
/**
|
||||
* Mirror of config `storage.git.auto_commit`. When false, an ingest run applies its squash
|
||||
* onto the project's working tree and leaves it staged instead of committing it.
|
||||
*/
|
||||
autoCommit: boolean;
|
||||
resolveUploadDir(uploadId: string): string;
|
||||
resolvePullDir(jobId: string): string;
|
||||
resolveTranscriptDir(jobId: string): string;
|
||||
|
|
|
|||
|
|
@ -116,6 +116,7 @@ export function createLocalProjectMemoryIngest(
|
|||
knowledge: { userScopedKnowledgeEnabled: false },
|
||||
slValidation: { probeRowCount: 0 },
|
||||
llm: { memoryIngestionModel: project.config.llm.models.default ?? 'local-memory-model' },
|
||||
autoCommit: project.config.memory.auto_commit,
|
||||
},
|
||||
promptService: new PromptService({ promptsDir, partials: [] }),
|
||||
skillsRegistry: new SkillsRegistryService({ skillsDir }),
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@ import { join } from 'node:path';
|
|||
import * as YAML from 'yaml';
|
||||
import { z } from 'zod';
|
||||
import { type KtxLogger, noopLogger } from '../../context/core/config.js';
|
||||
import type { SquashMergeResult, StageSquashResult } from '../../context/core/git.service.js';
|
||||
import type { KtxRuntimeToolSet } from '../../context/llm/runtime-port.js';
|
||||
import { revertSourceToPreHead, type SlValidationDeps } from '../../context/sl/tools/sl-warehouse-validation.js';
|
||||
import type { SemanticLayerSource } from '../../context/sl/types.js';
|
||||
|
|
@ -253,13 +254,20 @@ export class MemoryAgentService {
|
|||
reconciledCrossRefs,
|
||||
gateRevertedSources,
|
||||
);
|
||||
const mergeResult = await this.deps.lockingService.withLock('config:repo', () =>
|
||||
this.deps.gitService.squashMergeIntoMain(
|
||||
sessionWorktree.branch,
|
||||
SYSTEM_GIT_AUTHOR.name,
|
||||
SYSTEM_GIT_AUTHOR.email,
|
||||
squashMessage,
|
||||
),
|
||||
// With auto-commit disabled, apply the session to main's working tree and leave it
|
||||
// staged rather than committing. The knowledge/SL DB is written eagerly during the
|
||||
// session (and rolled back on conflict below), so search stays consistent with the
|
||||
// staged files; we only skip the git commit and its message-enhancement job.
|
||||
const autoCommit = this.deps.settings.autoCommit;
|
||||
const mergeResult = await this.deps.lockingService.withLock<SquashMergeResult | StageSquashResult>('config:repo', () =>
|
||||
autoCommit
|
||||
? this.deps.gitService.squashMergeIntoMain(
|
||||
sessionWorktree.branch,
|
||||
SYSTEM_GIT_AUTHOR.name,
|
||||
SYSTEM_GIT_AUTHOR.email,
|
||||
squashMessage,
|
||||
)
|
||||
: this.deps.gitService.stageSquashMergeIntoMain(sessionWorktree.branch),
|
||||
);
|
||||
|
||||
if (!mergeResult.ok) {
|
||||
|
|
@ -269,17 +277,19 @@ export class MemoryAgentService {
|
|||
} else if (mergeResult.touchedPaths.length === 0) {
|
||||
sessionOutcome = 'empty';
|
||||
} else {
|
||||
squashSha = mergeResult.squashSha;
|
||||
touchedPaths = mergeResult.touchedPaths;
|
||||
// Single-file commits: pass the path so the handler diff is path-scoped.
|
||||
// Multi-file commits: omit path so the handler grabs the full commit diff
|
||||
// (a comma-joined pathspec would match nothing).
|
||||
const pathFilter = touchedPaths.length === 1 ? touchedPaths[0] : '';
|
||||
await this.deps.rootFileStore.enqueueCommitMessageJobForExternalCommit(
|
||||
{ commitHash: squashSha },
|
||||
squashMessage,
|
||||
pathFilter,
|
||||
);
|
||||
if ('squashSha' in mergeResult) {
|
||||
squashSha = mergeResult.squashSha;
|
||||
// Single-file commits: pass the path so the handler diff is path-scoped.
|
||||
// Multi-file commits: omit path so the handler grabs the full commit diff
|
||||
// (a comma-joined pathspec would match nothing).
|
||||
const pathFilter = touchedPaths.length === 1 ? touchedPaths[0] : '';
|
||||
await this.deps.rootFileStore.enqueueCommitMessageJobForExternalCommit(
|
||||
{ commitHash: squashSha },
|
||||
squashMessage,
|
||||
pathFilter,
|
||||
);
|
||||
}
|
||||
}
|
||||
} catch (error) {
|
||||
sessionCrashed = true;
|
||||
|
|
|
|||
|
|
@ -74,6 +74,11 @@ interface MemoryAgentSettings {
|
|||
llm: {
|
||||
memoryIngestionModel: string;
|
||||
};
|
||||
/**
|
||||
* When false (config `memory.auto_commit: false`), a completed session is applied to the
|
||||
* project's working tree and left staged instead of committed, so the user commits it.
|
||||
*/
|
||||
autoCommit: boolean;
|
||||
}
|
||||
|
||||
interface MemoryTelemetryPort {
|
||||
|
|
|
|||
|
|
@ -230,7 +230,12 @@ const setupSchema = z
|
|||
|
||||
const storageGitSchema = z
|
||||
.strictObject({
|
||||
auto_commit: z.boolean().default(true).describe('When true, KTX automatically commits state changes to the local Git-backed store.'),
|
||||
auto_commit: z
|
||||
.boolean()
|
||||
.default(true)
|
||||
.describe(
|
||||
'When true, a context-source ingest run (`ktx ingest <connection>`) commits its changes to the local Git-backed store. When false, the changes are applied to the working tree and left staged for you to commit.',
|
||||
),
|
||||
author: z
|
||||
.string()
|
||||
.min(1)
|
||||
|
|
@ -278,7 +283,12 @@ const agentSchema = z
|
|||
|
||||
const memorySchema = z
|
||||
.strictObject({
|
||||
auto_commit: z.boolean().default(true).describe('When true, KTX automatically commits memory updates to the Git-backed store.'),
|
||||
auto_commit: z
|
||||
.boolean()
|
||||
.default(true)
|
||||
.describe(
|
||||
'When true, a memory/wiki ingest run commits its updates to the Git-backed store. When false, the updates are applied to the working tree and left staged for you to commit.',
|
||||
),
|
||||
})
|
||||
.describe('Memory subsystem configuration.');
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue