fix(build): track startup advisory and reflection sources#132
Open
qq919927036-glitch wants to merge 1 commit intolinuxhsj:mainfrom
Open
fix(build): track startup advisory and reflection sources#132qq919927036-glitch wants to merge 1 commit intolinuxhsj:mainfrom
qq919927036-glitch wants to merge 1 commit intolinuxhsj:mainfrom
Conversation
Author
|
Review note: this is the landing PR for the stack and should be reviewed first. The follow-up stack remains in the fork for sequencing only: PR B qq919927036-glitch#2, then PR C qq919927036-glitch#1. In order: source integrity first, failover telemetry second, JSON purity last. |
📝 Walkthrough走查本提交引入运行时反射持久化、启动建议生成和上下文推荐功能。新增了运行时反射工件的文件系统存储和检索、启动任务分类及路由合约解析、外部推荐执行和缓存管理、后台刷新调度、自动附加策略应用，以及支持的类型定义。总计添加七个新文件和类型扩展。 变更
序列图sequenceDiagram
participant Client
participant StartupAdvisory as 启动建议模块
participant Cache as 磁盘缓存
participant RecommendScript as 推荐脚本
participant RuntimeReflection as 运行时反射
Client->>StartupAdvisory: buildStartupContextAdvisory(taskText)
alt 缓存存在且有效
StartupAdvisory->>Cache: 读取缓存建议
Cache-->>StartupAdvisory: 返回缓存建议
StartupAdvisory->>StartupAdvisory: 标记缓存(ineligibleAutoAttach)
StartupAdvisory->>StartupAdvisory: 调度后台刷新(异步)
else 缓存不存在或无效
StartupAdvisory->>RecommendScript: execFileUtf8(推荐脚本)
RecommendScript-->>StartupAdvisory: JSON 推荐载荷
StartupAdvisory->>StartupAdvisory: 验证并解析 JSON
StartupAdvisory->>Cache: 写入新建议至缓存
Cache-->>StartupAdvisory: 缓存完成
end
StartupAdvisory->>RuntimeReflection: writeRuntimeReflection(反射记录)
RuntimeReflection-->>StartupAdvisory: 反射工件已写入
StartupAdvisory-->>Client: SessionStartupAdvisory(ready/unavailable)
sequenceDiagram
participant Client
participant AutoAttachPolicy as 自动附加策略
participant Cache as 磁盘缓存
participant RuntimeReflection as 运行时反射
Client->>AutoAttachPolicy: applyStartupAutoAttachPolicy(advisory, contextFiles)
alt 建议就绪 & 满足附加条件
AutoAttachPolicy->>Cache: 读取推荐文件内容
Cache-->>AutoAttachPolicy: 文件内容
AutoAttachPolicy->>AutoAttachPolicy: 根据分数/路径过滤文件
AutoAttachPolicy->>AutoAttachPolicy: 注入至 contextFiles
AutoAttachPolicy->>AutoAttachPolicy: 更新 autoAttachStatus = 'attached'
else 缓存建议或源发现
AutoAttachPolicy->>AutoAttachPolicy: 跳过附加(ineligible)
AutoAttachPolicy->>AutoAttachPolicy: 更新 autoAttachStatus = 'skipped'
else 建议不可用
AutoAttachPolicy->>AutoAttachPolicy: autoAttachStatus = 'unavailable'
end
AutoAttachPolicy->>RuntimeReflection: writeRuntimeReflection(反射结果)
RuntimeReflection-->>AutoAttachPolicy: 反射工件已写入
AutoAttachPolicy-->>Client: { advisory, contextFiles }
代码审查工作量估算🎯 4 (复杂) | ⏱️ ~75 分钟 诗
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
src/agents/startup-advisory.ts (3)
100-129: Fire-and-forget 反射写入会静默吞掉错误。
void writeRuntimeReflection(...).catch(() => {})会静默忽略所有写入错误，这可能导致难以诊断的问题。建议至少记录错误：
🛠️ 建议改进错误处理
void writeRuntimeReflection({ surface: "startup_advisory", // ... - }).catch(() => {}); + }).catch((error) => { + console.warn("[startup-advisory] Failed to emit reflection:", error); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/agents/startup-advisory.ts` around lines 100 - 129, The fire-and-forget writeRuntimeReflection call currently swallows all errors via .catch(() => {}); change this to surface and log the error instead so failures are visible — e.g., replace .catch(() => {}) with .catch(err => processLogger?.error?.("writeRuntimeReflection failed", { surface: "startup_advisory", entityId, sessionKey: params.sessionKey, err }) ?? console.error("writeRuntimeReflection failed", { surface: "startup_advisory", entityId, sessionKey: params.sessionKey, err })); ensure you use the existing writeRuntimeReflection invocation and include entityId/sessionKey/context in the log.
584-588: Python 回退重试逻辑的条件较为复杂，建议添加注释说明。当原始执行失败后，重试逻辑会在多种情况下选择使用重试结果。当前条件判断逻辑不够直观：
if (retried.code === 0 || (!retried.stderr && !retried.stdout)) { finalResult = retried; } else if (!result.stderr && !result.stdout) { finalResult = retried; }建议添加注释说明这些条件的意图，以提高代码可读性。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/agents/startup-advisory.ts` around lines 584 - 588, The conditional deciding whether to use the retried command result is non-obvious; add concise inline comments directly above the if/else block referencing retried, result, and finalResult that explain each branch: state that the first branch chooses retried when the retry exited successfully (retried.code === 0) or when the retry produced no stdout/stderr (treat as empty-success edge case), and the else-if chooses retried when the original result had no stdout/stderr (i.e., prefer the retry if the original returned empty output despite non-zero code); keep comments short and focused so future readers understand the intent of these specific checks.
1-1287: 文件过大，建议拆分以提高可维护性。此文件约 1287 行，显著超过编码指南建议的 ~500 LOC（src 目录）或 ~700 LOC（通用）限制。
建议按职责拆分：
startup-advisory-types.ts- 类型定义和常量startup-advisory-cache.ts- 缓存读写逻辑startup-advisory-classify.ts- 任务分类和路由合约startup-advisory-auto-attach.ts- 自动附加策略startup-advisory.ts- 主入口和协调逻辑As per coding guidelines:
src/**/*.{ts,tsx}: Aim to keep files under ~500 LOC when feasible (split/refactor as needed)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/agents/startup-advisory.ts` around lines 1 - 1287, This file is too large—split by responsibility: extract all type definitions and constants (e.g., StartupAdvisoryCacheEntry, StartupContextRecommendationPayload, STARTUP_ADVISORY_CACHE_FILENAME, AUTO_ATTACH_* constants, STARTUP_TASK_TEXT_MAX_CHARS) into startup-advisory-types.ts; move cache logic (resolveStartupAdvisoryCachePath, validateStartupAdvisoryCacheEntry, readStartupAdvisoryCache, persistStartupAdvisoryCache, scheduleStartupAdvisoryRefresh, startupAdvisoryBackgroundRefreshes, resetStartupAdvisoryBackgroundRefreshesForTest) into startup-advisory-cache.ts; move classification/routing logic (classifyStartupTaskShape, isSourceDiscoveryTask, isSkillRoutingPolicyTask, resolveStartupRouteContract) into startup-advisory-classify.ts; move auto-attach utilities and policy (isEligibleAutoAttachPath, isEligiblePolicyAutoAttachPath, topRecommendationScore, loadAutoAttachCandidate, applyStartupAutoAttachPolicy and related helpers) into startup-advisory-auto-attach.ts; leave orchestration and external integration (fetchStartupContextRecommendations, buildFreshStartupContextAdvisory, buildStartupContextAdvisory, emitStartupAdvisoryReflection, normalizeTaskTextCandidate, resolveStartupTaskText, extractLatestUserMessageText) in startup-advisory.ts; update imports/exports accordingly and ensure shared helpers (e.g., buildBootstrapContextFiles, writeRuntimeReflection, execFileUtf8) are imported from their original modules so tests and callers keep the same public API.src/agents/runtime-reflection.ts (1)
96-106: 无界循环存在潜在风险。
while (true)循环依赖fs.access抛出异常来退出。虽然实际运行中不太可能出现问题，但建议添加迭代上限作为安全保护：🛡️ 建议添加迭代上限
+const MAX_UNIQUE_SUFFIX = 1000; + async function resolveUniqueRuntimeReflectionArtifact(record: RuntimeReflectionRecord): Promise<{ record: RuntimeReflectionRecord; path: string; }> { let nextRecord = record; let filePath = resolveRuntimeReflectionPath(nextRecord); let suffix = 2; - while (true) { + while (suffix <= MAX_UNIQUE_SUFFIX) { try { await fs.access(filePath); nextRecord = { ...record, id: `${record.id}-${suffix}` }; filePath = resolveRuntimeReflectionPath(nextRecord); suffix += 1; } catch { return { record: nextRecord, path: filePath }; } } + throw new Error(`Failed to resolve unique path after ${MAX_UNIQUE_SUFFIX} attempts`); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/agents/runtime-reflection.ts` around lines 96 - 106, The infinite loop using while (true) that relies on fs.access throwing to exit (around the block that builds nextRecord, filePath, suffix and calls resolveRuntimeReflectionPath) needs a safety iteration limit: introduce a maxAttempts constant (e.g. 1000), add a counter that increments each loop, and if counter >= maxAttempts return an error or throw a clear exception (or return a failure result) instead of looping forever; keep the existing behavior of returning { record: nextRecord, path: filePath } when fs.access fails, but ensure the loop also breaks when the attempt limit is reached.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/agents/runtime-reflection.ts`:
- Around line 4-5: DEFAULT_RUNTIME_REFLECTIONS_ROOT is hard-coded to a
user-specific absolute path which breaks on other machines; change it to derive
from a configurable or project-relative location (e.g., use process.env, a
config value, or path.resolve(__dirname, '..', '..', 'artifacts',
'self-improve')) and update the export so DEFAULT_RUNTIME_REFLECTIONS_ROOT falls
back to a sensible project-relative default when the env/config is not set;
modify the constant definition in runtime-reflection.ts to read from the chosen
config/env and construct the path with Node's path.join/path.resolve instead of
the literal "/Users/lixun/..." string.
In `@src/agents/startup-advisory.ts`:
- Around line 17-18: DEFAULT_RECOMMEND_BIN is a hardcoded, user-specific
absolute path that will break on other machines; change it to load from
configuration or an environment variable (e.g., process.env.RECOMMEND_BIN) with
a sensible relative default resolved at runtime (using path.resolve relative to
__dirname or project root) and fall back with a clear error if missing; update
the reference to DEFAULT_RECOMMEND_BIN in startup-advisory.ts so callers use the
resolved value instead of the literal string.
- Around line 918-930: The function loadAutoAttachCandidate currently does
unnecessary dynamic imports of node:fs/promises and node:path; replace the await
import calls inside loadAutoAttachCandidate with the already statically imported
modules (use the top-level fs/promises import binding and the path binding) so
the function reads the file and derives basename via those existing symbols and
returns the WorkspaceBootstrapFile as before, removing the await
import("node:fs/promises") and await import("node:path") lines.
---
Nitpick comments:
In `@src/agents/runtime-reflection.ts`:
- Around line 96-106: The infinite loop using while (true) that relies on
fs.access throwing to exit (around the block that builds nextRecord, filePath,
suffix and calls resolveRuntimeReflectionPath) needs a safety iteration limit:
introduce a maxAttempts constant (e.g. 1000), add a counter that increments each
loop, and if counter >= maxAttempts return an error or throw a clear exception
(or return a failure result) instead of looping forever; keep the existing
behavior of returning { record: nextRecord, path: filePath } when fs.access
fails, but ensure the loop also breaks when the attempt limit is reached.
In `@src/agents/startup-advisory.ts`:
- Around line 100-129: The fire-and-forget writeRuntimeReflection call currently
swallows all errors via .catch(() => {}); change this to surface and log the
error instead so failures are visible — e.g., replace .catch(() => {}) with
.catch(err => processLogger?.error?.("writeRuntimeReflection failed", { surface:
"startup_advisory", entityId, sessionKey: params.sessionKey, err }) ??
console.error("writeRuntimeReflection failed", { surface: "startup_advisory",
entityId, sessionKey: params.sessionKey, err })); ensure you use the existing
writeRuntimeReflection invocation and include entityId/sessionKey/context in the
log.
- Around line 584-588: The conditional deciding whether to use the retried
command result is non-obvious; add concise inline comments directly above the
if/else block referencing retried, result, and finalResult that explain each
branch: state that the first branch chooses retried when the retry exited
successfully (retried.code === 0) or when the retry produced no stdout/stderr
(treat as empty-success edge case), and the else-if chooses retried when the
original result had no stdout/stderr (i.e., prefer the retry if the original
returned empty output despite non-zero code); keep comments short and focused so
future readers understand the intent of these specific checks.
- Around line 1-1287: This file is too large—split by responsibility: extract
all type definitions and constants (e.g., StartupAdvisoryCacheEntry,
StartupContextRecommendationPayload, STARTUP_ADVISORY_CACHE_FILENAME,
AUTO_ATTACH_* constants, STARTUP_TASK_TEXT_MAX_CHARS) into
startup-advisory-types.ts; move cache logic (resolveStartupAdvisoryCachePath,
validateStartupAdvisoryCacheEntry, readStartupAdvisoryCache,
persistStartupAdvisoryCache, scheduleStartupAdvisoryRefresh,
startupAdvisoryBackgroundRefreshes,
resetStartupAdvisoryBackgroundRefreshesForTest) into startup-advisory-cache.ts;
move classification/routing logic (classifyStartupTaskShape,
isSourceDiscoveryTask, isSkillRoutingPolicyTask, resolveStartupRouteContract)
into startup-advisory-classify.ts; move auto-attach utilities and policy
(isEligibleAutoAttachPath, isEligiblePolicyAutoAttachPath,
topRecommendationScore, loadAutoAttachCandidate, applyStartupAutoAttachPolicy
and related helpers) into startup-advisory-auto-attach.ts; leave orchestration
and external integration (fetchStartupContextRecommendations,
buildFreshStartupContextAdvisory, buildStartupContextAdvisory,
emitStartupAdvisoryReflection, normalizeTaskTextCandidate,
resolveStartupTaskText, extractLatestUserMessageText) in startup-advisory.ts;
update imports/exports accordingly and ensure shared helpers (e.g.,
buildBootstrapContextFiles, writeRuntimeReflection, execFileUtf8) are imported
from their original modules so tests and callers keep the same public API.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3bae5f1b-9aaa-4612-9372-f96cea38a809
📒 Files selected for processing (7)
src/agents/runtime-reflection.test.tssrc/agents/runtime-reflection.tssrc/agents/startup-advisory.test.tssrc/agents/startup-advisory.tssrc/auto-reply/reply/commands-context-recommend.test.tssrc/auto-reply/reply/commands-context-recommend.tssrc/config/sessions/types.ts
Comment on lines
+4
to
+5
| export const DEFAULT_RUNTIME_REFLECTIONS_ROOT = | ||
| "/Users/lixun/.openclaw/workspace/artifacts/self-improve"; |
There was a problem hiding this comment.
硬编码的本地路径会导致其他环境构建/运行失败。
DEFAULT_RUNTIME_REFLECTIONS_ROOT 包含用户特定的本地路径 /Users/lixun/.openclaw/workspace/artifacts/self-improve，这在其他开发者机器或 CI 环境中将无法正常工作。
建议使用相对于项目根目录或通过配置解析的路径，例如：
🛠️ 建议修复
-export const DEFAULT_RUNTIME_REFLECTIONS_ROOT =
- "/Users/lixun/.openclaw/workspace/artifacts/self-improve";
+import { resolveStateDir } from "../config/paths.js";
+
+export const DEFAULT_RUNTIME_REFLECTIONS_ROOT =
+ path.join(resolveStateDir(), "artifacts", "self-improve");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/agents/runtime-reflection.ts` around lines 4 - 5,
DEFAULT_RUNTIME_REFLECTIONS_ROOT is hard-coded to a user-specific absolute path
which breaks on other machines; change it to derive from a configurable or
project-relative location (e.g., use process.env, a config value, or
path.resolve(__dirname, '..', '..', 'artifacts', 'self-improve')) and update the
export so DEFAULT_RUNTIME_REFLECTIONS_ROOT falls back to a sensible
project-relative default when the env/config is not set; modify the constant
definition in runtime-reflection.ts to read from the chosen config/env and
construct the path with Node's path.join/path.resolve instead of the literal
"/Users/lixun/..." string.
Comment on lines
+17
to
+18
| const DEFAULT_RECOMMEND_BIN = | ||
| "/Users/lixun/.openclaw/workspace/scripts/pageindex/recommend-startup-context"; |
There was a problem hiding this comment.
硬编码的本地路径会导致其他环境运行失败。
DEFAULT_RECOMMEND_BIN 包含用户特定路径 /Users/lixun/.openclaw/workspace/scripts/pageindex/recommend-startup-context，这在其他机器上将无法工作。
建议通过配置或相对路径解析：
🛠️ 建议修复
+import { resolveWorkspaceDir } from "./workspace.js";
+
const DEFAULT_RECOMMEND_BIN =
- "/Users/lixun/.openclaw/workspace/scripts/pageindex/recommend-startup-context";
+ path.join(resolveWorkspaceDir(), "scripts", "pageindex", "recommend-startup-context");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/agents/startup-advisory.ts` around lines 17 - 18, DEFAULT_RECOMMEND_BIN
is a hardcoded, user-specific absolute path that will break on other machines;
change it to load from configuration or an environment variable (e.g.,
process.env.RECOMMEND_BIN) with a sensible relative default resolved at runtime
(using path.resolve relative to __dirname or project root) and fall back with a
clear error if missing; update the reference to DEFAULT_RECOMMEND_BIN in
startup-advisory.ts so callers use the resolved value instead of the literal
string.
Comment on lines
+918
to
+930
| async function loadAutoAttachCandidate( | ||
| file: SessionStartupRecommendationFile, | ||
| ): Promise<WorkspaceBootstrapFile> { | ||
| const fs = await import("node:fs/promises"); | ||
| const path = await import("node:path"); | ||
| const content = await fs.readFile(file.path, "utf8"); | ||
| return { | ||
| name: path.basename(file.path), | ||
| path: file.path, | ||
| content, | ||
| missing: false, | ||
| }; | ||
| } |
There was a problem hiding this comment.
loadAutoAttachCandidate 中的动态导入是不必要的。
文件顶部已经静态导入了 node:fs/promises 和 node:path，此处再次动态导入是冗余的：
♻️ 建议使用已导入的模块
async function loadAutoAttachCandidate(
file: SessionStartupRecommendationFile,
): Promise<WorkspaceBootstrapFile> {
- const fs = await import("node:fs/promises");
- const path = await import("node:path");
- const content = await fs.readFile(file.path, "utf8");
+ const content = await readFile(file.path, "utf8");
return {
name: path.basename(file.path),
path: file.path,
content,
missing: false,
};
}As per coding guidelines: Do not mix await import("x") and static import ... from "x" for the same module in production code paths.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/agents/startup-advisory.ts` around lines 918 - 930, The function
loadAutoAttachCandidate currently does unnecessary dynamic imports of
node:fs/promises and node:path; replace the await import calls inside
loadAutoAttachCandidate with the already statically imported modules (use the
top-level fs/promises import binding and the path binding) so the function reads
the file and derives basename via those existing symbols and returns the
WorkspaceBootstrapFile as before, removing the await import("node:fs/promises")
and await import("node:path") lines.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR restores clean-checkout source integrity for the startup-advisory and runtime-reflection path.
It tracks the source modules that were already being imported by command, status, and cron surfaces, but were not yet checked in as source files on the landing branch.
Why this PR is the right landing PR
This stack has three logical layers:
This PR is the first layer. It restores the baseline repository contract that a clean checkout should build from tracked source only.
Without this, later runtime and operator-facing changes can look green in a dirty tree while still failing in clean verification.
Follow-up stack
The follow-up PRs remain in the fork stack for review sequencing:
Problem
The branch could appear to work in a dirty worktree, but clean-checkout builds failed because imports were resolving to files that only existed as untracked local changes.
That breaks the most basic repository contract:
What changed
src/agents/runtime-reflection.tssrc/agents/startup-advisory.tssrc/auto-reply/reply/commands-context-recommend.tssrc/config/sessions/types.tswith the startup-advisory/task-text contracts those modules requireValidation
Validated in a clean worktree:
Non-goals
This PR does not:
/memoryor operator workflow workSummary by CodeRabbit
发布说明
新增功能
测试