Skip to content

fix(deps): move ai-factory to dependencies, probe at agent boot#114

Merged
lee-to merged 2 commits into
lee-to:mainfrom
sochkasov:fix/ai-factory-runtime-dep
May 24, 2026
Merged

fix(deps): move ai-factory to dependencies, probe at agent boot#114
lee-to merged 2 commits into
lee-to:mainfrom
sochkasov:fix/ai-factory-runtime-dep

Conversation

@sochkasov

Copy link
Copy Markdown

Closes #113.

Что и зачем

Цель — сделать ai-factory корректной runtime-зависимостью и превратить silent task-skip в видимую boot-time диагностику.

Проблема

ai-factory@^2.10.0 лежит в devDependencies корневого package.json, но фактически вызывается из production-runtime: packages/runtime/src/projectInit.ts:41 резолвит ai-factory/bin/ai-factory.js для каждого нового проекта, чтобы создать .ai-factory/ scaffold. С npm ci --omit=dev или NODE_ENV=production npm install пакет не попадает в node_modules; runtime тихо падает на fallback npx ai-factory ..., который требует сетевой доступ к npm registry на каждый task.

Полное описание и сценарий — в #113.

Изменения

Файл Изменение Обоснование
package.json ai-factory перенесён из devDependencies в dependencies Классификация по факту использования: production-runtime → dependencies
package-lock.json reflowed npm install под Node 22 / npm 10.9.7 Помимо моего ai-factory-блока, в diff попали удаления libc: glibc массивов и dev:true → peer:true на optional native binaries — это нормализация npm 10.9.7 существующего lockfile, не deliberate edits
packages/agent/src/index.ts probeAiFactory() в boot — require.resolve(\"ai-factory/bin/ai-factory.js\") с warning'ом Silent failure становится видимой первой строкой лога агента
README.md абзац под Quick Start про обязательность ai-factory и поведение при --omit=dev Закрывает doc gap — раньше ai-factory упоминался только как marketing-словосочетание

Обоснование выбранных решений

  • Почему не peerDependency. Peer'ы передают ответственность за установку downstream-консьюмеру. У aif-handoff downstream нет — это application, не библиотека. Прямая dependencies корректнее.
  • Почему не optionalDependency. ai-factory не optional. Без него агент не может scaffold ни один проект — все таски зависают. Optional подразумевает graceful degradation при отсутствии; здесь degradation = неработающий продукт.
  • Почему probe в agent/src/index.ts, а не dev.mjs. dev.mjs исполняется только в dev. Production-runtime (Docker / standalone) запускает агента напрямую. Probe в index.ts срабатывает в обоих сценариях.
  • Почему warning, а не fatal exit. Fallback на npx всё ещё технически работает в средах с сетевым доступом. Принудительный exit сломал бы инсталляции, где админ сознательно полагается на npx (CI с агрессивным кэшем). Warning + явное упоминание dependency-перемещения даёт actionable путь без breaking-change.
  • Почему diff package-lock.json такой большой. Большая часть строк — libc:[\"glibc\"] блоки и dev:true → peer:true нормализация на optional native binaries (sharp, swc, esbuild platform-deps). Это побочный эффект npm 10.9.7, не deliberate. Если у maintainer'а другой npm, можно сделать rm package-lock.json && npm install под их версией — мой коммит не зависит от конкретного формата нормализации.

Backward compatibility

  • Default npm install flow: без изменений — ai-factory как был, так и остаётся в node_modules.
  • npm ci --omit=dev flow: исправляется — ai-factory теперь устанавливается, агент работает offline-safe.
  • Docker (.docker/Dockerfile): без изменений — npm ci --ignore-scripts уже не использовал --omit=dev, поведение идентичное.
  • API/схема/конфиги: не затронуты.

Проверка

  • node -e \"console.log(require.resolve('ai-factory/bin/ai-factory.js'))\" после npm install — резолвится в node_modules/ai-factory/bin/ai-factory.js.
  • npm run build --workspace=@aif/agent — зелёный.
  • Probe-warning в index.ts: визуально проверен через временный rename node_modules/ai-factory → старт агента → лог \"ai-factory CLI is not installed locally...\" появляется первой строкой после getEnv().

Не вошло

  • Migration на pnpm/yarn (отдельный инфраструктурный choice).
  • Полная boot-time валидация всех runtime CLI (claude, codex, etc.) — потенциально полезно, но scope другой.

The agent invokes the ai-factory CLI at task-run time to scaffold
.ai-factory/ inside every user project (packages/runtime/projectInit.ts).
This is a hard runtime requirement, not a developer-only convenience —
yet the package was declared in devDependencies. With npm ci --omit=dev
or NODE_ENV=production npm install, ai-factory is not installed; the
runtime then falls back to `npx ai-factory ...`, which silently requires
network access to the npm registry at task time. In air-gapped or
offline-deployed installations every task fails with a buried
"Project .ai-factory/ scaffold missing" warning.

Changes:
- Move `ai-factory` from devDependencies to dependencies in root
  package.json. Reflowed package-lock.json: lines that look unrelated
  (libc array drops, dev→peer flag flips on optional native binaries)
  come from npm 10.9.7 normalising the existing lockfile and are not
  deliberate edits.
- Probe `require.resolve('ai-factory/bin/ai-factory.js')` at agent boot
  (packages/agent/src/index.ts). Log a clear warning if the CLI is not
  resolvable so the failure is diagnosable from the first agent log
  line, instead of surfacing later as a per-task skip.
- Document the requirement in the README Quick Start so installers know
  why ai-factory must be in node_modules and what happens otherwise.

No behavioural change for default `npm install` flows — ai-factory was
already resolvable; this PR makes it resolvable for production install
modes as well.
@lee-to

lee-to commented May 6, 2026

Copy link
Copy Markdown
Owner

Code review

This PR moves ai-factory into the production dependency set and adds an agent boot probe that turns a late per-task scaffold failure into early diagnostics. The change is narrow, documented in README.md, and I did not find a blocking correctness issue in the dependency move or the boot-time probe.

Must fix

None.

Should fix

None.

Nits

None.

Context gates

  • Architecture: pass - no new DB access or package-boundary violation; the agent change stays in boot diagnostics.
  • Rules: pass - no string-based error classification, no UI/CSS concerns, no adapter capability or migration changes.
  • Roadmap: pass - this is a focused fix linked to ai-factory классифицирован как devDependency, но является обязательной runtime-зависимостью #113.
  • CHECKLIST compliance (touched packages: packages/agent): pass - applicable items hold; no provider SDK calls, subagent definitions, direct DB access, coordinator scheduler, poll overlap, or watchdog behavior changed.
  • Docs sync: pass - README.md documents the runtime dependency and fallback behavior.
  • PR size (211 changed lines / 4 files / 1 concern): pass - small enough to review as one unit; lockfile churn is dependency metadata from the same concern.
  • Risk gating (feature flag): n/a - this does not introduce a new runtime path; it only changes install classification and logs an early warning when the existing fallback path would be used.

Verification

  • npm run build --workspace=@aif/agent - passed in a temporary PR worktree.
  • npm run lint --workspace=@aif/agent - passed in a temporary PR worktree.
  • npm test --workspace=@aif/agent - passed: 22 files, 325 tests.

Verdict: COMMENT - no blocking findings, but GitHub reports no CI check rollup for this PR.

@lee-to lee-to requested a review from ichinya May 6, 2026 15:02
@ichinya

ichinya commented May 7, 2026

Copy link
Copy Markdown
Collaborator

Code review

The PR appears to close #113 by moving ai-factory from root devDependencies to dependencies, adding an agent boot-time probe for the local CLI, and documenting the production-install behavior. The source diff itself is coherent and the runtime-dependency classification is correct. Merge is not currently safe because the current head has a failing Security Audit run and the branch is now behind current main.

Response pass

  • Addressed: The core ai-factory классифицирован как devDependency, но является обязательной runtime-зависимостью #113 fix is present: ai-factory is now in the root production dependency set, and the lockfile removes dev: true from ai-factory and its CLI dependency graph.
  • Addressed: The boot probe is non-fatal and preserves the existing npx ai-factory ... fallback semantics instead of introducing a breaking startup exit.
  • Still unresolved: No previous source-level blocker remains from the earlier review comment.
  • New blocker found: Current CI now has a failing Security Audit job, and this branch is 6 commits behind current main, including changes to .github/workflows/security-audit.yml and package-lock.json.

What looks good

  • The dependency move matches actual runtime behavior: packages/runtime/src/projectInit.ts resolves and executes ai-factory/bin/ai-factory.js while processing tasks, so ai-factory should not be classified as a dev-only dependency.
  • The probe in packages/agent/src/index.ts uses createRequire(import.meta.url) and checks the same local module path that the runtime path needs. It improves diagnostics without changing the existing fallback path.
  • The README update is aligned with the code: it clearly documents that ai-factory is required at runtime and that falling back to npx requires registry access.
  • No API/schema/web-client contract, DB migration, feature flag, runtime adapter contract, or coordinator state-machine behavior is changed by this PR.

Must fix

  1. [P1] Current head fails the required Security Audit gate — CI / dependency lockfile

    The latest workflow run for head 643f33a has Build, Lint, Tests, and MCP Unit passing, but Security Audit fails in the blocking Runtime dependency audit step. The failing command is npm audit --omit=dev, and the audit output reports a moderate ip-address <=10.1.0 issue through express-rate-limit.

    This is not safe to ignore in this PR because the change explicitly promotes a dependency graph into the runtime dependency set. Also, the branch is stale: PR merge-base is 6d0fc17, while current main is d74cc13. Current main has already changed the audit workflow to block high/critical runtime vulnerabilities only and has its own package-lock.json changes. Merging from the stale base risks preserving a red CI state or losing the current lockfile/audit-policy updates during conflict resolution.

    Expected fix:

    • Rebase this PR onto current main (d74cc13 at review time).

    • Regenerate/resolve package-lock.json against the rebased tree with the repo’s expected npm/CI behavior.

    • Rerun CI and ensure Security Audit passes under the current workflow, not the stale merge ref.

    • Smoke-test the actual regression scenario:

      • npm ci
      • npm audit --omit=dev --audit-level=high
      • rm -rf node_modules && npm ci --omit=dev --ignore-scripts
      • node -e "console.log(require.resolve('ai-factory/bin/ai-factory.js'))"
      • Start the agent once and verify it does not emit the new ai-factory CLI is not installed locally warning when installed through the production path.

Should fix

  1. [P2] Add a lightweight production-install regression guard

    The manual checks in the PR description cover the important scenario, but the existing workspace build/test flow would not prevent ai-factory from being moved back to devDependencies later. Since ai-factory классифицирован как devDependency, но является обязательной runtime-зависимостью #113 is specifically a production-install classification bug, a small CI/script guard would be valuable.

    Suggested check:

    • Add a smoke job or script that runs npm ci --omit=dev --ignore-scripts in a clean workspace and verifies require.resolve("ai-factory/bin/ai-factory.js").
    • This can be a follow-up if maintainers do not want the extra install cost in every PR, but it should be tracked explicitly.

Nits

None.

Context gates

  • Architecture: pass — the dependency classification matches runtime usage, and the agent change is boot diagnostics only.
  • Package CHECKLIST: pass — packages/agent/CHECKLIST.md applicable items hold; no direct provider SDK call, direct DB access, subagent definition, poll scheduler, watchdog, or coordinator state-machine change is introduced. packages/runtime source was not modified.
  • Docs sync: pass — README now documents ai-factory as a required runtime dependency and describes the npx fallback/network risk.
  • API/web/client sync: pass — no API payload, schema, generated client, web hook, WebSocket, or query invalidation contract changed.
  • Feature flag / risk gating: pass — no new feature flag or env var; warning-only probe does not alter enabled/disabled runtime semantics.
  • Migration / backward compatibility: pass — no DB/schema migration; default npm install behavior stays compatible, and npm ci --omit=dev is the path being fixed.
  • PR size / decomposition: warn — 4 files / about 207 changed lines is coherent for one concern, but most of the diff is lockfile metadata. Rebase should keep lockfile churn limited to the runtime dependency move plus current main updates.
  • Stack / merge order: error — no stacked PR dependency found, but the branch is 6 commits behind current main with overlapping package-lock.json and audit-workflow changes.
  • CI: error — Build/Lint/Tests/MCP Unit pass; Security Audit fails; AI Review is skipped.

Verdict

Verdict: REQUEST_CHANGES until the branch is rebased onto current main, the lockfile is resolved against that base, and Security Audit plus the standard CI suite pass.

ichinya
ichinya previously requested changes May 7, 2026

@ichinya ichinya left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

REQUEST_CHANGES

@lee-to

lee-to commented May 14, 2026

Copy link
Copy Markdown
Owner

@sochkasov ping

@lee-to lee-to dismissed ichinya’s stale review May 24, 2026 19:12

Stale after maintainer update: branch merged current main, lockfile resolved, and CI is green.

@lee-to lee-to merged commit 65016b4 into lee-to:main May 24, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ai-factory классифицирован как devDependency, но является обязательной runtime-зависимостью

3 participants