Skip to content

Commit d055012

Browse files
neo-fabletobiuneo-gpt
authored
refactor(orchestrator): ADR-0019 batch-2 — dbPath/dataDir provider leaves replace module-level env derivation (#12456) (#14941)
* refactor(orchestrator): ADR-0019 batch-2 — dbPath/dataDir provider leaves replace module-level env derivation (#12456) * fix(orchestrator): use-site leaf reads replace exported module-scope captures (#14957) * fix(orchestrator): drop unused config import (#14957) * fix(orchestrator): preserve reactive db path (#14957) --------- Co-authored-by: tobiu <tobiasuhlig78@gmail.com> Co-authored-by: Euclid <neo-gpt@neomjs.com>
1 parent 7cad11a commit d055012

9 files changed

Lines changed: 170 additions & 41 deletions

File tree

‎ai/config.template.mjs‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -484,6 +484,28 @@ class Config extends ConfigProvider {
484484
* @type {Object}
485485
*/
486486
orchestrator: {
487+
/**
488+
* Directory owning ALL orchestrator-daemon runtime state: the daemon + child-task
489+
* PID files, `orchestrator.log`, `orchestrator-state.json`, and the heavy-maintenance
490+
* lease + tenant-repo-sync revision files stored beside them. Kept relative on
491+
* purpose — it resolves against the daemon's cwd (repo root under the standard
492+
* launch). Owning the default AND the `NEO_AI_ORCHESTRATOR_DIR` env binding here
493+
* (instead of a module-level `process.env` read at the consumer) keeps the
494+
* config-is-SSOT contract: no consumer re-derives from env, no consumer holds a
495+
* hidden default.
496+
* @type {String}
497+
*/
498+
dataDir: leaf('.neo-ai-data/orchestrator-daemon', 'NEO_AI_ORCHESTRATOR_DIR', 'string'),
499+
/**
500+
* SQLite Memory Core graph database file the orchestrator opens for graph-backed
501+
* health checks and maintenance decisions. Kept relative on purpose — it resolves
502+
* against the daemon's cwd (repo root under the standard launch). Owning the
503+
* default AND the `NEO_AI_DB_PATH` env binding here (instead of a module-level
504+
* `process.env` read at the consumer) keeps the config-is-SSOT contract: no
505+
* consumer re-derives from env, no consumer holds a hidden default.
506+
* @type {String}
507+
*/
508+
dbPath: leaf('.neo-ai-data/sqlite/memory-core-graph.sqlite', 'NEO_AI_DB_PATH', 'string'),
487509
/**
488510
* Deployment profile for Agent OS maintenance ownership.
489511
* `local` preserves maintainer-checkout behavior; `cloud` disables local-only

‎ai/daemons/orchestrator/Orchestrator.mjs‎

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -57,11 +57,7 @@ import {
5757
buildOrchestratorSchedulingOptions,
5858
runSchedulingPipeline
5959
} from './scheduling/pipeline.mjs';
60-
import {
61-
DEFAULT_DB_PATH,
62-
DEFAULT_DATA_DIR,
63-
DEFAULT_SCRIPT_DIR
64-
} from './taskDefinitions.mjs';
60+
import {DEFAULT_SCRIPT_DIR} from './taskDefinitions.mjs';
6561
import {
6662
inspectHeavyMaintenanceLeaseSync
6763
} from './services/heavyMaintenanceLeasePrimitives.mjs';
@@ -198,7 +194,12 @@ export class Orchestrator extends Base {
198194
dataRecoveryActuatorService_ : null,
199195
dataIntegrityDiagnosisService_ : null,
200196
maintenanceBackpressureService_ : MaintenanceBackpressureService,
201-
dataDir_ : DEFAULT_DATA_DIR,
197+
// null = "resolve from the owning config leaf on read" (see beforeGetDataDir): a leaf
198+
// value in this static block would freeze at module load, not at the use site.
199+
dataDir_ : null,
200+
// Same contract as dataDir_: the singleton is constructed during module import, so a
201+
// class-field leaf read would still be a module-load capture.
202+
dbPath_ : null,
202203
taskDefinitions_ : null,
203204
taskStateService_ : TaskStateService,
204205
healthService_ : HealthService,
@@ -226,7 +227,6 @@ export class Orchestrator extends Base {
226227
isPolling = false
227228
pollHandle = null
228229
db = null
229-
dbPath = DEFAULT_DB_PATH
230230
logFile = null
231231
stateFile = null
232232
primaryDevSyncRootsConfig = null
@@ -355,6 +355,26 @@ export class Orchestrator extends Base {
355355
}
356356
}
357357

358+
/**
359+
* @summary Resolves the runtime-state directory from the owning config leaf when no explicit
360+
* value was set — a per-read use-site resolution, never a module-load capture.
361+
* @param {String|null} value
362+
* @returns {String}
363+
*/
364+
beforeGetDataDir(value) {
365+
return value ?? AiConfig.orchestrator.dataDir
366+
}
367+
368+
/**
369+
* @summary Resolves the graph database path from the owning config leaf when no explicit value
370+
* was set, preserving Provider refreshes before orchestrator start.
371+
* @param {String|null} value
372+
* @returns {String}
373+
*/
374+
beforeGetDbPath(value) {
375+
return value ?? AiConfig.orchestrator.dbPath
376+
}
377+
358378
/**
359379
* @param {Neo.ai.daemons.services.ProcessSupervisorService|Object|null} value
360380
* @returns {Neo.ai.daemons.services.ProcessSupervisorService}
@@ -811,15 +831,15 @@ export class Orchestrator extends Base {
811831
}
812832

813833
const scriptDir = options.scriptDir || DEFAULT_SCRIPT_DIR;
814-
const dataDir = options.dataDir || DEFAULT_DATA_DIR;
834+
const dataDir = options.dataDir || AiConfig.orchestrator.dataDir;
815835

816836
this.dataDir = dataDir;
817837
this.taskDefinitions = options.taskDefinitions || this.buildConfiguredTaskDefinitions({
818838
scriptDir,
819839
nodeBin: options.nodeBin || process.argv[0]
820840
});
821841

822-
this.dbPath = options.dbPath || DEFAULT_DB_PATH;
842+
this.dbPath = options.dbPath || AiConfig.orchestrator.dbPath;
823843
this.logFile = options.logFile || path.join(dataDir, 'orchestrator.log');
824844
this.stateFile = options.stateFile || path.join(dataDir, 'orchestrator-state.json');
825845
this.heavyMaintenanceLeasePath = options.heavyMaintenanceLeasePath ?? this.heavyMaintenanceLeasePath;

‎ai/daemons/orchestrator/daemon.mjs‎

Lines changed: 48 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -34,12 +34,40 @@ import AiConfig from '../../config.mjs';
3434
import Orchestrator, {rotateLogFileIfNewDay} from './Orchestrator.mjs';
3535
import {assertConfigFresh} from '../../scripts/setup/initServerConfigs.mjs';
3636

37-
const DAEMON_DATA_DIR = process.env.NEO_AI_ORCHESTRATOR_DIR || '.neo-ai-data/orchestrator-daemon';
38-
const PID_FILE = path.join(DAEMON_DATA_DIR, 'orchestrator-daemon.pid');
39-
const LOG_FILE = path.join(DAEMON_DATA_DIR, 'orchestrator.log');
4037
const ORCHESTRATOR_DAEMON_PATH_TAIL = 'ai/daemons/orchestrator/daemon.mjs';
4138
export const LOCAL_AI_CONFIG_FILE = fileURLToPath(new URL('../../config.mjs', import.meta.url));
4239

40+
/**
41+
* @summary Resolves the orchestrator daemon's runtime data directory from the config provider.
42+
*
43+
* The provider owns the default AND the `NEO_AI_ORCHESTRATOR_DIR` env binding, so this
44+
* entrypoint reads the resolved leaf instead of re-deriving it from `process.env` with a
45+
* shadow default. Resolved lazily at each use site (all of which run after the boot-time
46+
* config-freshness guard) rather than via an eager module-level `path.join`: on a stale
47+
* config overlay the leaf is missing, and an eager join would crash module evaluation
48+
* before the guard can name the actionable `--migrate-config` fix.
49+
* @returns {String}
50+
*/
51+
function daemonDataDir() {
52+
return AiConfig.orchestrator.dataDir;
53+
}
54+
55+
/**
56+
* @summary Resolves the daemon singleton PID-file path under {@link daemonDataDir}.
57+
* @returns {String}
58+
*/
59+
function pidFilePath() {
60+
return path.join(daemonDataDir(), 'orchestrator-daemon.pid');
61+
}
62+
63+
/**
64+
* @summary Resolves the shared orchestrator log-file path under {@link daemonDataDir}.
65+
* @returns {String}
66+
*/
67+
function logFilePath() {
68+
return path.join(daemonDataDir(), 'orchestrator.log');
69+
}
70+
4371
/**
4472
* @summary Checks whether a process command belongs to this daemon entry point.
4573
* Uses the orchestrator-specific path-tail to avoid sibling `daemon.mjs` collisions.
@@ -51,16 +79,18 @@ export function isOrchestratorDaemonCommand(cmd) {
5179
}
5280

5381
function writeLog(level, message) {
82+
const logFile = logFilePath();
83+
5484
// Both this wrapper writer AND Orchestrator.writeLog append to the same orchestrator.log, so
5585
// BOTH must rotate-before-append: an unguarded append here would advance the file's mtime past
5686
// the day boundary and defeat the mtime-based daily rotation.
57-
rotateLogFileIfNewDay(LOG_FILE);
87+
rotateLogFileIfNewDay(logFile);
5888

5989
const timestamp = new Date().toISOString();
6090
const line = `[${timestamp}] [PID:${process.pid}] [${level}] ${message}`;
6191

6292
try {
63-
fs.appendFileSync(LOG_FILE, line + '\n', 'utf8');
93+
fs.appendFileSync(logFile, line + '\n', 'utf8');
6494
} catch (e) {}
6595

6696
if (level === 'ERROR') {
@@ -90,17 +120,21 @@ async function waitForExit(pid, timeoutMs) {
90120
}
91121

92122
function removePidFile() {
123+
const pidFile = pidFilePath();
124+
93125
try {
94-
if (fs.existsSync(PID_FILE) && parseInt(fs.readFileSync(PID_FILE, 'utf8'), 10) === process.pid) {
95-
fs.unlinkSync(PID_FILE);
126+
if (fs.existsSync(pidFile) && parseInt(fs.readFileSync(pidFile, 'utf8'), 10) === process.pid) {
127+
fs.unlinkSync(pidFile);
96128
}
97129
} catch (e) {}
98130
}
99131

100132
async function enforceSingleton() {
101-
if (fs.existsSync(PID_FILE)) {
133+
const pidFile = pidFilePath();
134+
135+
if (fs.existsSync(pidFile)) {
102136
try {
103-
const oldPid = parseInt(fs.readFileSync(PID_FILE, 'utf8'), 10);
137+
const oldPid = parseInt(fs.readFileSync(pidFile, 'utf8'), 10);
104138
if (!Number.isNaN(oldPid) && oldPid > 0 && oldPid !== process.pid) {
105139
let isAlive = false;
106140

@@ -128,7 +162,7 @@ async function enforceSingleton() {
128162
}
129163
}
130164

131-
fs.writeFileSync(PID_FILE, process.pid.toString(), 'utf8');
165+
fs.writeFileSync(pidFile, process.pid.toString(), 'utf8');
132166
}
133167

134168
function setupCleanupHandlers() {
@@ -183,13 +217,15 @@ export async function loadLocalAiConfig({
183217
* @returns {Promise<void>}
184218
*/
185219
export async function startOrchestrator(options = {}) {
186-
fs.ensureDirSync(DAEMON_DATA_DIR);
220+
const dataDir = daemonDataDir();
221+
222+
fs.ensureDirSync(dataDir);
187223
await enforceSingleton();
188224
setupCleanupHandlers();
189225
await loadLocalAiConfig();
190226

191227
return Orchestrator.start({
192-
dataDir : DAEMON_DATA_DIR,
228+
dataDir,
193229
primaryDevSyncRootsConfig: AiConfig.orchestrator.devSyncRoots,
194230
...options
195231
});

‎ai/daemons/orchestrator/services/MaintenanceBackpressureService.mjs‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import {
66
acquireHeavyMaintenanceLeaseSync,
77
releaseHeavyMaintenanceLeaseSync
88
} from './HeavyMaintenanceLeaseService.mjs';
9-
import {DEFAULT_DATA_DIR} from '../taskDefinitions.mjs';
109

1110
/**
1211
* Canonical set of heavy-maintenance task names that participate in the cross-poll
@@ -177,7 +176,7 @@ export function getActiveGoldenPathDependencyTask({
177176
*/
178177
export function resolveHeavyMaintenanceLeasePath({heavyMaintenanceLeasePath, dataDir}) {
179178
if (heavyMaintenanceLeasePath) return heavyMaintenanceLeasePath;
180-
return path.join(dataDir || DEFAULT_DATA_DIR, 'heavy-maintenance-lease.json');
179+
return path.join(dataDir || AiConfig.orchestrator.dataDir, 'heavy-maintenance-lease.json');
181180
}
182181

183182
// ============================================================================
@@ -332,10 +331,12 @@ export class MaintenanceBackpressureService extends Base {
332331
*/
333332
heavyMaintenanceLeasePath_: null,
334333
/**
335-
* @member {String} dataDir_=DEFAULT_DATA_DIR
334+
* `null` = "resolve from the owning config leaf on read" (see `beforeGetDataDir`): a
335+
* leaf value in this static block would freeze at module load, not at the use site.
336+
* @member {String|null} dataDir_=null
336337
* @reactive
337338
*/
338-
dataDir_: DEFAULT_DATA_DIR,
339+
dataDir_: null,
339340
/**
340341
* @member {Object|null} taskStateService_=null
341342
* @reactive
@@ -381,6 +382,16 @@ export class MaintenanceBackpressureService extends Base {
381382
*/
382383
shedUntil = 0
383384

385+
/**
386+
* @summary Resolves the runtime-state directory from the owning config leaf when no explicit
387+
* value was set — a per-read use-site resolution, never a module-load capture.
388+
* @param {String|null} value
389+
* @returns {String}
390+
*/
391+
beforeGetDataDir(value) {
392+
return value ?? AiConfig.orchestrator.dataDir
393+
}
394+
384395
/**
385396
* @summary Opens an auto-expiring shed-window: until `now + durationMs`, `acquireLeaseAndExecute` defers ALL
386397
* heavy-maintenance. The actuation primitive the `throttle-shed` heal calls to relieve resource contention —

‎ai/daemons/orchestrator/services/RecoveryActuatorService.mjs‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import {
1515
appendHealEvent,
1616
validateHealLedgerRetention
1717
} from '../../../services/memory-core/helpers/healEventLedgerStore.mjs';
18-
import {DEFAULT_DATA_DIR} from '../taskDefinitions.mjs';
1918

2019
const DEFAULT_ACTIONS = Object.freeze(['restart', 'redeploy', 'warm-provider']);
2120
const DEFAULT_DEPLOY_TARGETS = Object.freeze(['cloud-deploy']);
@@ -107,11 +106,13 @@ export class RecoveryActuatorService extends Base {
107106
*/
108107
actuatorConfig_: null,
109108
/**
110-
* @member {String} dataDir_='.neo-ai-data/orchestrator-daemon'
109+
* `null` = "resolve from the owning config leaf on read" (see `beforeGetDataDir`): a
110+
* leaf value in this static block would freeze at module load, not at the use site.
111+
* @member {String|null} dataDir_=null
111112
* @protected
112113
* @reactive
113114
*/
114-
dataDir_: DEFAULT_DATA_DIR,
115+
dataDir_: null,
115116
/**
116117
* @member {Object|null} healthService_=null
117118
* @protected
@@ -460,6 +461,16 @@ export class RecoveryActuatorService extends Base {
460461
});
461462
}
462463

464+
/**
465+
* @summary Resolves the runtime-state directory from the owning config leaf when no explicit
466+
* value was set — a per-read use-site resolution, never a module-load capture.
467+
* @param {String|null} value
468+
* @returns {String}
469+
*/
470+
beforeGetDataDir(value) {
471+
return value ?? AiConfig.orchestrator.dataDir
472+
}
473+
463474
/**
464475
* @summary Resolves the strict recovery target entry for a service key and optional identity.
465476
* @param {Object} options

‎ai/daemons/orchestrator/services/TenantRepoSyncService.mjs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import fs from 'fs-extra';
22
import path from 'node:path';
33
import Base from '../../../../src/core/Base.mjs';
44
import AiConfig from '../../../config.mjs';
5-
import {DEFAULT_DATA_DIR} from '../taskDefinitions.mjs';
65
import GitMirror from '../../../services/knowledge-base/helpers/gitMirror.mjs';
76
import {buildIngestEnvelope} from '../../../services/knowledge-base/helpers/tenantRepoIngestEnvelopeBuilder.mjs';
87
import {isRepoDue} from '../scheduling/tenantRepoSync.mjs';
@@ -219,7 +218,7 @@ class TenantRepoSyncService extends Base {
219218
* @param {Object} [options.gitMirror=GitMirror] Injectable mirror primitive (test seam).
220219
* @param {Object} [options.knowledgeBaseIngestionService] KB ingestion service singleton (test seam). Resolved from `ai/services.mjs` if omitted.
221220
* @param {String[]} [options.onlyRepoSlugs] If provided, only sync repos whose `repoSlug` is in the list. Used by the manual CLI run path. Empty filter result against non-empty list surfaces `KB_TENANT_REPO_SYNC_REPO_NOT_CONFIGURED`.
222-
* @param {String} [options.revisionsFilePath] Override the per-tenant-repo lastIngestedRev persistence file path (test seam). Defaults to `<DEFAULT_DATA_DIR>/tenant-repo-sync-revisions.json`.
221+
* @param {String} [options.revisionsFilePath] Override the per-tenant-repo lastIngestedRev persistence file path (test seam). Defaults to `<orchestrator dataDir leaf>/tenant-repo-sync-revisions.json`.
223222
* @param {Function} [options.envelopeBuilder=buildIngestEnvelope] Injectable envelope-builder (test seam). Production callers omit; unit tests pass a fake that returns canned envelope shape.
224223
* @returns {Promise<Object>} `{status, details}` — status ∈ {`completed`, `failed`, `skipped`}.
225224
*/
@@ -631,15 +630,16 @@ class TenantRepoSyncService extends Base {
631630

632631
/**
633632
* Default per-tenant-repo lastIngestedRev persistence file path. Lives next to
634-
* the orchestrator state file (`<DEFAULT_DATA_DIR>/orchestrator-state.json`)
633+
* the orchestrator state file (`<orchestrator dataDir leaf>/orchestrator-state.json`)
635634
* so the two persistence surfaces share lifecycle (same data-dir = same recovery scope).
636635
* Separate file (not inlined into TaskStateService's state) prevents `markCompleted/markFailed`
637-
* task-lifecycle writes from racing with revision-map writes.
636+
* task-lifecycle writes from racing with revision-map writes. The dataDir resolves from the
637+
* owning config leaf inline — a use-site read, never a module-load capture.
638638
*
639639
* @returns {String}
640640
*/
641641
defaultRevisionsFilePath() {
642-
return path.join(DEFAULT_DATA_DIR, PERSISTED_REVISIONS_FILE_NAME);
642+
return path.join(AiConfig.orchestrator.dataDir, PERSISTED_REVISIONS_FILE_NAME);
643643
}
644644

645645
/**

‎ai/daemons/orchestrator/taskDefinitions.mjs‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,15 @@ import {fileURLToPath} from 'url';
55
const __filename = fileURLToPath(import.meta.url);
66
const __dirname = path.dirname(__filename);
77

8-
export const DEFAULT_DB_PATH = process.env.NEO_AI_DB_PATH || '.neo-ai-data/sqlite/memory-core-graph.sqlite';
9-
export const DEFAULT_DATA_DIR = process.env.NEO_AI_ORCHESTRATOR_DIR || '.neo-ai-data/orchestrator-daemon';
8+
/**
9+
* Default maintenance-script directory, resolved relative to this module.
10+
*
11+
* The orchestrator's db path + runtime-state dir carry NO re-export here: consumers read the
12+
* `AiConfig.orchestrator.dbPath` / `AiConfig.orchestrator.dataDir` leaves inline at each use
13+
* site — exporting a resolved leaf freezes it at module load and hands consumers a
14+
* config-shaped constant, both forbidden by the config-is-SSOT contract.
15+
* @type {String}
16+
*/
1017
export const DEFAULT_SCRIPT_DIR = path.resolve(__dirname, '../../scripts');
1118

1219
const DEFAULT_CHROMA_HEALTH_ENDPOINT = '/api/v2/heartbeat';

0 commit comments

Comments
 (0)