feat(grok-build): add production-ready observability support - #208
feat(grok-build): add production-ready observability support#208fangxiu-wf wants to merge 12 commits into
Conversation
Adds grok-build (Grok CLI) coding agent observability — hook processor
parses grok transcript turns into a 5-level span tree (ENTRY -> AGENT ->
STEP -> LLM/TOOL) and emits ARMS GenAI semantic attributes.
Core changes:
- src/inputs/grok-build-log/grok-build-log-input.ts: transcript tailing input
- assets/hooks/grok-build-{hook-processor.mjs, transcript-parser.mjs, state.mjs}
+ grok-build-loongsuite-pilot-hook.sh launcher
- agents.d/grok-build.json: agent registration
- flusher/otlp-trace: AgentSpanEnrichingHandler injects gen_ai.agent.* +
gen_ai.data_source.id + acs.* resource attrs + cache_creation tokens
- F1/F3a: token usage alignment + system_instructions array shape
- F2/F4/F6/F8: tool pairing / finish_reason / result structure alignment
with semconv
- R2 Step C: AGENT span aggregation fields (input/output token sums,
cache_creation_read, cache_read across child LLM spans)
Tests: grok-build hook 41/41 + flusher 104/104 unit PASS.
E2E: 3 scenarios (long conversation / error path / concurrent sessions)
all PASS; validate-trace 0 ERROR on real grok 0.2.101 (5bc4b5dfad) CLI.
Known non-blocking:
- STEP count for long-conversation < 20 threshold (grok batches Read/Grep
into single turns); data quality PASS.
- OTLP flusher ECONNREFUSED 4318 (local OTEL collector not started);
otlp-debug dump used for validation; non-regression vs prior rounds.
Co-Authored-By: fangxiu-wf <fangxiu.wf@alibaba-inc.com>
# Conflicts: # assets/hooks/qoderwork-hook-processor.mjs # src/flushers/otlp-trace-flusher.ts # src/inputs/codex-transcript/codex-transcript-input.ts # tests/unit/inputs/codex-transcript/codex-transcript-input.test.ts
# Conflicts: # deploy/installer-opensource.ps1 # deploy/installer-opensource.sh # tests/unit/deploy/installer-uninstall-cleanup.test.mjs # tests/unit/deployment/hook-strategy.test.ts # tests/unit/normalization/agent-system-map.test.ts
# Conflicts: # src/flushers/otlp-trace-flusher.ts # tests/unit/flushers/otlp-trace-flusher/conversion.test.ts
ralf0131
left a comment
ralf0131
left a comment
There was a problem hiding this comment.
Summary
This PR adds production-ready Grok Build observability support to loongsuite-pilot, superseding #171. It implements four fail-open hooks (stop, stop_failure, user_prompt_submit, session_end) and fuses three native Grok data sources (chat_history.jsonl, updates.jsonl, unified.jsonl) into a coherent trace structure.
The implementation is well-structured with clear separation of concerns: transcript parsing, updates parsing, unified parsing, state management with file-based locking, and a fusion layer that correlates data across sources. Privacy handling (captureMessageContent=false) correctly strips sensitive content. The test coverage is comprehensive (12+ test files with realistic fixture data).
Key strengths:
- Clean data source separation with proper checkpoint-based offset tracking
- Fail-open hook design that never disrupts the host application
- Robust state management with stale lock detection and bounded deduplication
- Good privacy controls with content policy enforcement
- Comprehensive test fixtures covering single-turn, multi-turn, parallel tools, and long conversations
Minor observations (non-blocking, see inline comments):
stripSystemRoleMessagesJSON roundtrip could be documented as normalizing- File-based lock recovery path could use an explanatory comment
- Asset restore is self-healing but not atomic
LGTM — ready to merge.
Automated review by github-manager-bot
| || (message as Record<string, unknown>).role !== 'system'); | ||
| } | ||
| if (typeof value !== 'string' || value.length === 0) return value; | ||
| try { |
There was a problem hiding this comment.
[Info] stripSystemRoleMessages does a JSON.parse → filter → JSON.stringify roundtrip on string-typed gen_ai.input.messages. If the original string used single quotes or non-standard formatting, the re-serialized output will differ. Consider documenting that this function normalizes formatting, or preserving the original string when no system messages are found (early return before JSON.stringify). Not blocking — the downstream consumer likely parses JSON anyway.
There was a problem hiding this comment.
[not-fixed] 建议未落地，属 [Info] 非阻断，保持 open。
证据: head 5ed8efb8 src/flushers/otlp-trace-flusher.ts:106-116——string 分支仍无条件 JSON.parse → filter → JSON.stringify，未按建议在无 system 消息时提前返回原字符串，格式规范化行为仍在。因作者已注明下游本就解析 JSON，不计入 merge gate。
Generated by LoongSuite-Pilot Code Review Agent
| session_id: sessionId, | ||
| chat_checkpoint: normalizeCheckpoint(raw.chat_checkpoint), | ||
| updates_checkpoint: normalizeCheckpoint(raw.updates_checkpoint), | ||
| recent_prompt_ids: Array.isArray(raw.recent_prompt_ids) |
There was a problem hiding this comment.
[Info] The file-based session lock uses STATE_LOCK_TIMEOUT_MS = 2_000 with STATE_LOCK_RETRY_MS = 25 (~80 retries). The stale detection at STATE_LOCK_STALE_MS = 30_000 is good. One edge case: if the hook process is killed mid-write (SIGKILL), the lock file persists but the stale check should catch it on the next invocation. Consider adding a comment documenting this recovery path for future maintainers.
There was a problem hiding this comment.
[not-fixed] 未新增文档注释；且本轮复核发现该恢复路径本身存在竞态（Finding M1，Medium，阻断），保持 open。
证据: head state.mjs:105 stale 分支 if (Date.now()-statSync(lockPath).mtimeMs > staleMs){ fs.unlinkSync(lockPath); continue; } 无条件按路径删锁、无 token 校验，与 removeLockIfOwned（校验 current?.token===token）不对称。两个同 session 并发 hook 可同时判定 stale、同时删并重建锁、同时进入回调 → 对 offset/recent_prompt_ids 丢失更新（重复导出或 offset 跳过）。详见 inline 评论 M1。
Generated by LoongSuite-Pilot Code Review Agent
ralf0131
left a comment
ralf0131
left a comment
There was a problem hiding this comment.
Summary
This PR adds comprehensive Grok Build observability support — fusing three native data sources (chat_history.jsonl, updates.jsonl, unified.jsonl) into structured trace events with proper ENTRY → AGENT → STEP → LLM/TOOL hierarchy. The architecture is well-designed with clear separation of concerns (parsers, fusion, state, hook processor), fail-open semantics, and robust state management with file locking.
Overall: LGTM — solid engineering with a few minor observations.
Findings
- [Warning]
grok-build-hook-processor.mjs:316—readSystemPromptreads up to 2MB synchronously; consider limiting to first N lines for large transcripts - [Info]
fusion.mjs:22—takeByNamefallback to first unused item could misattribute when same-named tools appear in consecutive LLM calls (mitigated by time-window filter) - [Info]
otlp-trace-flusher.ts:222—AgentSpanEnrichingHandlermutable instance state is safe for current single-turn-per-buffer design, but would need per-invocation scoping if concurrent flush paths are added
Highlights
- Clean 3-source fusion with graceful degradation when any source is incomplete
- Proper fail-open hook design — never disrupts the host agent
- Robust state management with O_EXCL file locks, stale lock recovery, and bounded dedup (MAX_RECENT_PROMPT_IDS=64)
- Smart tool matching cascade: real ID → deterministic name → order
- Watchdog integration with precondition + health check pattern for dynamic agents
- Cross-platform support (shell + PowerShell) with Node.js version validation
- OTLP flusher correctly strips system role from input messages and maps Grok-specific error types
Cross-repo Note
No related changes needed in loongsuite-python — this is a pilot-only addition.
Automated review by github-manager-bot
| function terminalTargets(trigger, event, state, updateTurns) { | ||
| if (trigger === 'stop' || trigger === 'stop-failure') { | ||
| const updateTurn = currentUpdateTurn(updateTurns, event.prompt_id); | ||
| const promptId = event.prompt_id |
There was a problem hiding this comment.
[Performance] readSystemPrompt reads up to 2MB synchronously via fs.readSync. For a hook that runs on every Stop event, this could introduce noticeable latency if chat_history.jsonl grows large. Consider reading only the first N lines (e.g. 50) since the system prompt is typically the first record, or caching the result keyed by file size+mtime.
There was a problem hiding this comment.
[not-fixed] 建议未落地，保持 open（Finding M3，Medium，阻断）。
证据: head grok-build-hook-processor.mjs:220-227 + 调用点 :699——readSystemPrompt 仍是 min(size,2MB) 的 fs.readSync(offset 0) + toString + split('\n')，未改为读前 N 行或按 size/mtime 缓存。因 parseGrokTranscript 增量推进 offset，首轮后 chatResult.systemPrompt 恒为 null，该 fallback 每个 Stop 都触发。详见 inline 评论 M3。
Generated by LoongSuite-Pilot Code Review Agent
| } | ||
|
|
||
| function actualResultFrom(updatePair, chatDetails) { | ||
| const updateResult = updatePair?.completion?.toolOutput; |
There was a problem hiding this comment.
[Correctness] takeByName falls back to the first unused item when no name match is found. This is a reasonable last-resort strategy, but if two consecutive LLM calls both use a tool named bash, the second call's tools could consume the first call's leftover results. The isWithin time-window filter in updateEventsForGroup mitigates this, but worth documenting the assumption that tool names are mostly unique within a turn.
There was a problem hiding this comment.
[partially-fixed] 常见路径已缓解且可观测，但无 unified 日志时的顺序兜底仍可能跨 call 误取；假设文档注释未新增。保持 open（Finding L2，Low，不阻断）。
证据: head fusion.mjs:11-64——已加入分组 + updateEventsForGroup 的 isWithin ±2000ms 时间窗 + matchStrategy=name_order/timingSource 标注，unified 存在时按 id 分组并受窗口约束。缺口：group 为 null（unified.jsonl 缺失/已轮转）时 :57 if (!group || isWithin(...)) 短路返回全部未用事件、不再受时间窗约束，takeByName 名称未命中即回退首个未用事件（:15），仍可能取到相邻 call 的 update 工具事件。
Generated by LoongSuite-Pilot Code Review Agent
|
This PR has conflicts with the git fetch origin
git checkout codex/grok-build-observability-v2
git rebase origin/main
# resolve conflicts, then:
git push --force-with-leaseThis is a one-time reminder. Feel free to @mention me for a re-review after conflicts are resolved. Automated notification by github-manager-bot |
| $HOOK_MARKERS = @( | ||
| "-loongsuite-pilot-hook." | ||
| ) | ||
| $hookMarkersJson = $HOOK_MARKERS | ConvertTo-Json -Compress |
There was a problem hiding this comment.
[High] Windows 卸载静默失效（本 PR 引入的回归）
$HOOK_MARKERS = @("-loongsuite-pilot-hook.") 单元素数组经 $HOOK_MARKERS | ConvertTo-Json -Compress 通过管道时被 PowerShell 解包为标量，序列化成 JSON 字符串（"-loongsuite-pilot-hook."）而非数组。内嵌 node 脚本 const markers = JSON.parse(argv[2]); markers.some(...) 对字符串调用 .some 抛 TypeError → process.exit(1)；但 PowerShell 对外部程序非零退出码不抛异常，catch 不触发，仍打印 ✅ 已清理 / ✅ Cleaned。
影响: Windows 上执行卸载时，任何配置中的 Pilot hook 条目都不会被移除，却报告清理成功；hook 在“卸载”后仍每次 Stop/UserPromptSubmit 触发、继续采集会话数据（合规/隐私风险）。base 985fb60c 处为标量 $HOOK_MARKER = ".loongsuite-pilot" + cmd.includes(marker)，正确；本 PR 引入该回归。新测试仅跑 POSIX 路径，PS1 仅做源码静态断言，未捕获。
建议: 强制数组序列化 —— ConvertTo-Json -Compress -InputObject $HOOK_MARKERS（或 ,$HOOK_MARKERS | ConvertTo-Json），或像 .sh 版把 marker 作为独立 argv 传入并用 process.argv.slice(2)；同时让 PS 调用方在打印成功前校验 $LASTEXITCODE，并补一条 PS1 执行级测试。
Generated by LoongSuite-Pilot Code Review Agent
| if (Date.now() - fs.statSync(lockPath).mtimeMs > staleMs) { | ||
| fs.unlinkSync(lockPath); | ||
| continue; | ||
| } |
There was a problem hiding this comment.
[Medium] stale-lock 回收竞态 → 重复导出/状态覆盖（同时触发 Lifecycle 状态恢复 FAIL）
stale 恢复分支 if (Date.now() - fs.statSync(lockPath).mtimeMs > staleMs) { fs.unlinkSync(lockPath); continue; } 无条件按路径删锁、无 token/身份校验，与 removeLockIfOwned（校验 current?.token === token）不对称。
影响: 当上一个 hook 被 SIGKILL 遗留过期锁（>30s），两个同 session 并发 hook（如 SessionEnd 与 Stop）可能都判定 stale、都 unlinkSync 并各自重建锁、都进入回调，对 offset / recent_prompt_ids 形成读改写丢失更新 → 重复导出 turn 或 offset 回退/跳过。恢复路径自身存在竞态。
建议: stale 回收改为“带身份的抢占”——将过期锁 rename 到唯一临时名再校验/删除，或复用 openSync('wx') 语义在删后立即以自身 token 重建并校验回读一致；避免纯按路径删除。
Generated by LoongSuite-Pilot Code Review Agent
| } | ||
| if (stat.size <= 0) return []; | ||
|
|
||
| let start = Math.max(0, stat.size - maxBytes); |
There was a problem hiding this comment.
[Medium] 同步 hook 热路径每次读取至多 50MB 跨 session 全局日志
readBoundedCompleteTail 在每次 Stop/StopFailure 读取共享、滚动的 ~/.grok/logs/unified.jsonl 尾部（start = max(0, size - MAX_UNIFIED_BYTES)，MAX_UNIFIED_BYTES = 50MB），Buffer.alloc + toString('utf-8') + split('\n') + JSON.parse 整段后再按 raw.sid 过滤；与 transcript/updates 不同，无 per-session offset checkpoint。
影响: 活跃用户的全局日志可增长到数十 MB，每个 turn 结束都付出一次全量尾部读取+解码+JSONL 解析（数百 ms 至秒级 CPU + 大瞬时分配），而宿主 Agent 同步等待 hook 返回 {}。开销随全部 grok 活动量而非本 session 增长，且每 turn 重复。
建议: 为 unified 增加 per-session checkpoint（offset + inode，同 chat/updates），或大幅下调尾部上限并在集齐本 session 的 inference group 后短路。
Generated by LoongSuite-Pilot Code Review Agent
| try { | ||
| fd = fs.openSync(chatHistoryPath, 'r'); | ||
| const stat = fs.fstatSync(fd); | ||
| const length = Math.min(stat.size, 2 * 1024 * 1024); |
There was a problem hiding this comment.
[Medium] readSystemPrompt 每 turn 2MB 头部读（历史评论 3689843323 未修复）
systemPrompt = chatResult.systemPrompt || readSystemPrompt(chatHistoryPath)。因 parseGrokTranscript 从已存 offset 增量读取，首轮后 offset 越过头部 system 记录，chatResult.systemPrompt 每轮恒为 null，fallback 每个 Stop 触发；readSystemPrompt 恒 openSync + Buffer.alloc(min(size,2MB)) + readSync(2MB, 0) + toString + split('\n') 后才扫描第一条 system。
影响: 每个 Stop 一次 2MB 头部读 + 全串 split（目标通常在第 1 行），贯穿整个 session；与 M2 叠加在同一条同步路径上。
建议: 首次抽取后把（每 session 不可变的）system prompt 缓存进 session state；或逐行读取、遇首个 \n 分隔的 system 记录即停，不做整段 split。
Generated by LoongSuite-Pilot Code Review Agent
🔍 Code Review Summary评审对象：PR #208
Lifecycle Verdict
状态恢复 FAIL 证据： Merge Gate（合入门禁）⛔ BLOCK（阻断级别： 未解决的阻断项：
总体结论实现质量整体很高——隐私 off 路径无内容泄漏、注入面干净、
Highlights（正向实践）
评审报告详见: |
3 participants
Summary
Add production-ready Grok Build observability support and supersede #171.
Pilot keeps Grok Build unchanged. It installs four fail-open hooks (
stop,stop_failure,user_prompt_submit, andsession_end) and fuses three native Grok data sources:chat_history.jsonlfor messages, model metadata, tool arguments, and the system instruction.updates.jsonlfor prompt/turn identity, terminal state, tool state, and incremental completion.unified.jsonlfor inference timing, token usage, tool timing, and execution results.Subagent hooks are explicitly unsupported and retired.
Data and trace semantics
ENTRY -> AGENT -> STEP -> LLM/TOOLusing real inference/tool timestamps; retain true zero-duration tools.tool_callsemantics while closing turn-level success, cancellation, and failure correctly.error.typeand fixed summary; do not copy upstream error details.gen_ai.agent.system=grok, providerx_ai, and include agent description/data-source metadata on every turn.Privacy
When
captureMessageContent=false, user/assistant/system content, tool arguments, tool results, and raw error details are all removed. No synthetic placeholder result is emitted when Grok has no real result.Installation, uninstall, and watchdog
{}output.HookStrategy.needsRedeploy()for snake_case hooks, retired hooks, missing settings, and asset integrity.repairedonly when the target is actually healthy.Compatibility
origin/mainwith a regular merge commit; no history rewrite or force-push.Verification
Current PR head:
5ed8efb8954ec4680dc8d976d9bb35df28549129(documentation-only follow-up)E2E-tested implementation commit:
6a5a60261d8d50103588faedb63a44125bf4cf07npm run typecheckandnpm run buildpassed.Multica and real ARMS/SLS E2E
Multica task:
AGE-1186Provenance:
6a5a60261d8d50103588faedb63a44125bf4cf07f1352fc61d43452f5452f33a32a3d10bd360ecc4b72c234d6a01932840e747cdsha256:9107b674e0866a535799fc371158d83babbc1ec2fc474718a2a4c4bfa90664b30.2.114(0c78503879)grok6a5final3-20260731123000age1186-6a5a-final3-on-grok-build-grok-buildage1186-6a5a-final3-off-grok-build-grok-build2026-07-31T04:30:49Zto2026-07-31T04:38:03ZThe final run used a pinned Grok binary with auto-update disabled. Across 16 turns and 36 version/binary checks, no version drift occurred.
Results from the authoritative ARMS tracing SLS logstore, queried with the exact run, service names, time window, and trace IDs:
gen_ai.agent.system=grok, LLM providerx_ai,loongsuite.grok.timing.source=unified, and deterministic name/order matching were present as expected.ToolError,authentication_error, andcancelled, with fixed summaries only. No raw authentication endpoint/detail or synthetic placeholder was present.Lifecycle validation:
ARMS
SearchTraceshad not yet indexed these new service names during final verification (ServiceNameNotExist). The same trace IDs were already complete in the authoritativelogstore-tracingbackend, so the server-side SLS spans above were used for acceptance and the indexing delay is recorded explicitly.Supersedes
This PR replaces #171 with the three-source implementation, corrected lifecycle/time semantics, privacy guarantees, Windows support, and production installation/uninstall/watchdog behavior. The earlier PR is intentionally left open for maintainers to close or compare.