Skip to content

Commit 639a31f

Browse files
authored
fix: review follow-ups for monorepo adapter separation (#783)
* fix: review follow-ups — better first-run log, OPENCLI_FETCH=1 skips version check - Clarify first-run log message: "copying adapters (one-time setup)" - Add comment explaining why scriptPath uses two levels of ../ - OPENCLI_FETCH=1 now bypasses version-skip to allow forced refresh * fix: update doc-coverage script path after clis/ move check-doc-coverage.sh still referenced src/clis/ after PR #782 moved adapters to root clis/. This caused CI to fail with "0/1 documented". * fix: resolve package root dynamically for symlink and first-run paths The symlink at ~/.opencli/node_modules/@jackwener/opencli pointed to dist/ instead of the package root in prod mode, breaking user TS CLIs that import from '@jackwener/opencli/registry'. The first-run scriptPath also resolved incorrectly in dev mode. Extract findPackageRoot() that walks up to find package.json, fixing both paths for dev (src/) and prod (dist/src/) layouts.
1 parent 80eef46 commit 639a31f

File tree

3 files changed

+21
-7
lines changed

3 files changed

+21
-7
lines changed

scripts/check-doc-coverage.sh

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#!/usr/bin/env bash
2-
# check-doc-coverage.sh — Verify every adapter in src/clis/ has a doc page.
2+
# check-doc-coverage.sh — Verify every adapter in clis/ has a doc page.
33
#
44
# Exit codes:
55
# 0 — all adapters have docs
@@ -19,7 +19,7 @@ fi
1919
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
2020
ROOT_DIR="$(cd "$SCRIPT_DIR/.." && pwd)"
2121

22-
SRC_DIR="$ROOT_DIR/src/clis"
22+
SRC_DIR="$ROOT_DIR/clis"
2323
DOCS_DIR="$ROOT_DIR/docs/adapters"
2424

2525
missing=()

scripts/fetch-adapters.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,9 @@ export function fetchAdapters() {
8787
const currentVersion = getPackageVersion();
8888
const oldManifest = readManifest();
8989

90-
// Skip if already installed at the same version
91-
if (currentVersion !== 'unknown' && oldManifest?.version === currentVersion) {
90+
// Skip if already installed at the same version (unless forced via OPENCLI_FETCH=1)
91+
const isForced = process.env.OPENCLI_FETCH === '1';
92+
if (!isForced && currentVersion !== 'unknown' && oldManifest?.version === currentVersion) {
9293
log(`Adapters already up to date (v${currentVersion})`);
9394
return;
9495
}

src/discovery.ts

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,19 @@ function parseStrategy(rawStrategy: string | undefined, fallback: Strategy = Str
3737

3838
import { isRecord } from './utils.js';
3939

40+
/**
41+
* Find the package root (directory containing package.json).
42+
* Dev: import.meta.url is in src/ → one level up.
43+
* Prod: import.meta.url is in dist/src/ → two levels up.
44+
*/
45+
function findPackageRoot(): string {
46+
let dir = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..');
47+
if (!fs.existsSync(path.join(dir, 'package.json'))) {
48+
dir = path.resolve(dir, '..');
49+
}
50+
return dir;
51+
}
52+
4053
function resolveHostRuntimeModulePath(moduleName: string): string {
4154
const runtimeDir = path.dirname(fileURLToPath(import.meta.url));
4255
for (const ext of ['.js', '.ts']) {
@@ -119,7 +132,7 @@ export async function ensureUserCliCompatShims(baseDir: string = USER_OPENCLI_DI
119132
// Create node_modules/@jackwener/opencli symlink so user TS CLIs can import
120133
// from '@jackwener/opencli/registry' (the package export).
121134
// This is needed because ~/.opencli/clis/ is outside opencli's node_modules tree.
122-
const opencliRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..');
135+
const opencliRoot = findPackageRoot();
123136
const symlinkDir = path.join(baseDir, 'node_modules', '@jackwener');
124137
const symlinkPath = path.join(symlinkDir, 'opencli');
125138
try {
@@ -162,10 +175,10 @@ export async function ensureUserAdapters(): Promise<void> {
162175
// Dir doesn't exist — needs fetch
163176
}
164177

165-
log.info('First run detected — fetching adapters...');
178+
log.info('First run detected — copying adapters (one-time setup)...');
166179
try {
167180
const { execFileSync } = await import('node:child_process');
168-
const scriptPath = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..', '..', 'scripts', 'fetch-adapters.js');
181+
const scriptPath = path.join(findPackageRoot(), 'scripts', 'fetch-adapters.js');
169182
execFileSync(process.execPath, [scriptPath], {
170183
stdio: 'inherit',
171184
env: { ...process.env, _OPENCLI_FIRST_RUN: '1' },

0 commit comments

Comments
 (0)