From 84b71b249cdfbdb6be1b09f69f65b21e818d4a94 Mon Sep 17 00:00:00 2001 From: tmoonlight Date: Fri, 4 Sep 2026 01:29:45 +0800 Subject: [PATCH 1/5] =?UTF-8?q?fix:=20=E5=8E=8B=E7=BC=A9=E4=B8=8D=E5=86=8D?= =?UTF-8?q?=E5=9B=A0=E4=B8=80=E6=AC=A1=E4=B8=8A=E6=B8=B8=E6=8A=BD=E9=A3=8E?= =?UTF-8?q?=E8=80=8C=E4=B8=AD=E6=AD=A2=E6=95=B4=E4=B8=AA=E5=9B=9E=E5=90=88?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 现场(feedback IQSZ-D0423,0.9.9):压缩摘要请求发出后网关卡了 2 分 6 秒, 超时后往 SSE 流里插了一行非 JSON 的 `data:504:Gateway Timeout`,随后流没有 [DONE] 就结束。结果压缩失败、整个回合中止,用户只能重开会话: Auto full compact failed; error=Summarization failed: Malformed JSON in streaming response ... parse error at line 1, column 4: unexpected ':' 三个独立问题叠加,分三层修: 1. saw_parse_error 的粘性误判(openai_provider) 流里任何一帧解析失败就永久置位,叠加「没等到 [DONE]」即判为终止性的 MalformedJson。而同样的流中断若没有那一帧脏数据,会判为可重试的 MalformedSse 并大概率重试成功 —— 一行网关纯文本把可恢复错误降级成了 致命错误。改为只有整条流**没有任何一帧解析成功**时才算 MalformedJson; 对面根本没在说这个协议的情况(现有用例)行为不变。 2. 压缩失败缺兜底(agent_loop) 摘要压缩要发一次模型请求,于是继承了那次请求的全部失败模式。可上下文 已经满了,这时放弃等于让回合中止。新增 run_mechanical_compact_fallback: 摘要失败后改用不调模型的 apply_thread_repair 机械修剪腾出空间,回合 继续。修剪不动时(历史太短)保持原有的失败行为。 3. 瞬时故障被塞进 4xx(src/pa) 网关把「模型服务异常，请稍候重试」编成 HTTP 400,撞上 retry_policy 「4xx 一律不重试」的规则成了永久失败。新增 FaultKind::TransientUpstream, 由 provider_http_error_is_retryable 在 4xx 规则前兜底。收录标准比上下文 超限更严:必须是报文自己写着可以再试。刻意不收 LLMRequestError 前缀 (同时罩着参数错误)与单独的「重试」二字(额度用完写的是「请切换模型后 重试」,那是要人换模型)。上下文超限优先于瞬时故障 —— 盲目重试超大请求 只会再撞一次墙。 另注:自适应上下文收敛(0.9.9 引入)把该模型阈值从 126000 降到 61967,这个 收敛本身正确(服务端确实在 72903 处拒收过),但压缩触发频率随之上升,把上面 这个既有缺陷从偶发放大成了常见。 新增 5 个单测(SSE 分类 2 / 压缩兜底 1 / 瞬时故障 4);受影响的 268 个用例 与全部 229 个 provider 用例通过。 --- src/agent_loop.cpp | 58 ++++++++++++++++++ src/agent_loop.hpp | 6 ++ src/pa/README.md | 25 ++++++++ src/pa/pa_quirks.cpp | 33 ++++++++++ src/pa/pa_quirks.hpp | 7 +++ src/provider/openai_provider.cpp | 11 +++- src/provider/retry_policy.cpp | 7 +++ .../agent_loop_compact_events_test.cpp | 47 ++++++++++++++ tests/pa/pa_quirks_test.cpp | 61 ++++++++++++++++++- .../openai_provider_error_recovery_test.cpp | 54 ++++++++++++++++ 10 files changed, 306 insertions(+), 3 deletions(-) diff --git a/src/agent_loop.cpp b/src/agent_loop.cpp index 4b13902f2..cab3c6d13 100644 --- a/src/agent_loop.cpp +++ b/src/agent_loop.cpp @@ -1360,6 +1360,55 @@ void AgentLoop::apply_compact_result( make_compact_notice_metadata(notice_id, "warning", true)); } +bool AgentLoop::run_mechanical_compact_fallback( + int request_tokens, + int context_window, + const std::string& compact_notice_id, + const std::string& summarization_error) { + // 目标规模与上下文溢出恢复路径同口径:降到窗口的 70%,给下一轮留出余量。 + const int history_tokens = estimate_message_tokens( + recovered_provider_messages(messages_, "compact-fallback-estimate")); + const int fixed_tokens = (std::max)(0, request_tokens - history_tokens); + int target_total = (std::max)(1, request_tokens * 2 / 3); + if (context_window > 0) { + target_total = (std::min)(target_total, context_window * 7 / 10); + } + + ThreadRepairOptions options; + options.trigger = "repair-compact-fallback"; + options.target_tokens = (std::max)(1, target_total - fixed_tokens); + // 强制至少丢掉一组:摘要已经失败了,原地不动地"成功"只会让调用方以为 + // 腾出了空间,下一轮继续撞同一堵墙。 + options.force_prune_one_group = true; + + auto repair = apply_thread_repair(session_manager_, messages_, options); + LOG_WARN("[compact-fallback] mechanical prune after summarization failure; " + "status=" + std::string(to_string(repair.status)) + + " pre_tokens=" + std::to_string(repair.pre_tokens) + + " post_tokens=" + std::to_string(repair.post_tokens) + + " pruned_groups=" + std::to_string(repair.pruned_groups) + + " target_tokens=" + std::to_string(options.target_tokens) + + " reason=" + repair.reason); + if (!repair.repaired()) { + return false; + } + + compact_generation_.fetch_add(1, std::memory_order_relaxed); + last_api_total_tokens_.store(0, std::memory_order_relaxed); + + events_.emit(SessionEventKind::AgentProgress, nlohmann::json{ + {"phase", "context_repair"}, + {"label", "Compaction failed; pruned oldest history instead"}, + {"detail", repair.reason}, + }); + emit_transcript_system_message( + "[智能压缩] 摘要压缩失败(" + log_truncate(summarization_error, 160) + + "),已改为丢弃最旧的 " + std::to_string(repair.pruned_groups) + + " 组历史腾出空间,会话继续。", + make_compact_notice_metadata(compact_notice_id, "warning")); + return true; +} + bool AgentLoop::maybe_run_auto_compact() { const int context_window = compaction_context_window(); auto initial_context = build_compaction_initial_context(); @@ -1428,6 +1477,15 @@ bool AgentLoop::maybe_run_auto_compact() { log_truncate(result.error, 500) + " active_estimated_tokens=" + std::to_string(pre_tokens) + " threshold=" + std::to_string(threshold)); + // 摘要压缩要发一次模型请求,于是它继承了那次请求的全部失败模式 —— + // 网关超时、流中断、上游抽风。可上下文已经满了,这时候放弃等于让整个 + // 回合中止,用户只能重开会话。机械修剪不调用模型,因此不会以同样的方式 + // 失败;它保不住语义(丢的是最旧的整组消息,不是摘要),但能腾出空间让 + // 会话继续,这在"卡死"面前是明显更好的结果。 + if (run_mechanical_compact_fallback(pre_tokens, context_window, + compact_notice_id, result.error)) { + return true; + } emit_transcript_system_message( "[Auto-compact] " + result.error, make_compact_notice_metadata(compact_notice_id, "error")); diff --git a/src/agent_loop.hpp b/src/agent_loop.hpp index 525333f41..fe16d2086 100644 --- a/src/agent_loop.hpp +++ b/src/agent_loop.hpp @@ -460,6 +460,12 @@ class AgentLoop { nlohmann::json abort_notice_metadata() const; std::size_t close_active_turn_and_discard(); bool maybe_run_auto_compact(); + // 摘要压缩失败后的兜底:改用不调用模型的机械修剪腾出空间。返回 true 表示 + // 空间已经腾出、回合可以继续。见 maybe_run_auto_compact 里的失败分支。 + bool run_mechanical_compact_fallback(int request_tokens, + int context_window, + const std::string& compact_notice_id, + const std::string& summarization_error); bool active_estimate_exceeds_auto_threshold( const UserInput* pending_input = nullptr) const; diff --git a/src/pa/README.md b/src/pa/README.md index 12dd934f6..77157230d 100644 --- a/src/pa/README.md +++ b/src/pa/README.md @@ -44,6 +44,31 @@ `is_context_overflow_error` 兜底调用。**故意不限制状态码** —— 该网关的状态码 本身就是不可信的那一环,而中文特征串足够专指,不会误伤其它错误。 +### 3. 瞬时故障被塞进 4xx 返回(`pa_quirks.{hpp,cpp}`) + +**观测报文**: + +```json +{"code":400,"message":"LLMRequestError: 模型服务异常，请稍候重试，如持续出现，请联系技术支持","type":"BadRequestError"} +{"code":502,"message":"LLMRequestError: 网络波动或模型处理超时，请稍候重试"} +``` + +**通用路径为什么不够**:`retry_policy.cpp` 对 4xx 一律不重试(注释写着「客户端 +错误即使提到 overload 也是终止性的」)。这条规则对守规矩的上游是对的,但这套 +网关把瞬时故障也编成 400,一次本该自动恢复的抽风就成了永久失败。 + +**适配**:`pa::is_transient_upstream()` 由 `provider_http_error_is_retryable` +在 4xx 规则之前兜底。收录标准比上下文超限**更严** —— 必须是报文自己写着「这是 +暂时的、可以再试」,否则真正的客户端错误会被无限重试。两条刻意不收: + +- `LLMRequestError` 这个网关统一前缀同时罩着瞬时故障和参数错误,按它重试等于 + 对所有 4xx 都重试。 +- 单独的「重试」二字 —— 额度用完的报文写的是「请切换模型后重试」,那是要人换 + 模型,不是让程序再发一次。 + +分类顺序上**上下文超限优先**:超限报文也可能带「请稍候重试」的客套话,但盲目 +重试同一个超大请求只会再撞一次墙,必须先走压缩。 + ### 2. 声明的上下文窗口不准(`pa_context_budget.{hpp,cpp}`) **现象**:同一个模型的实际可用上下文会变,常常远小于 `saved_models` 里声明的 diff --git a/src/pa/pa_quirks.cpp b/src/pa/pa_quirks.cpp index ccd6a9cfa..10b4c84d5 100644 --- a/src/pa/pa_quirks.cpp +++ b/src/pa/pa_quirks.cpp @@ -33,6 +33,30 @@ const std::vector& context_overflow_needles() { return needles; } +// 上游瞬时故障的中文特征串。收录标准比上下文超限更严:必须是报文**自己** +// 表达了「这是暂时的、可以再试」,否则一条真正的客户端错误会被无限重试。 +// +// 刻意不收 "LLMRequestError" 这个网关统一前缀 —— 它同时罩着瞬时故障和真正的 +// 参数错误,按它重试等于对所有 4xx 都重试。也刻意不收单独的「重试」二字: +// 额度用完的报文写的是「请切换模型后重试」,那是要人换模型,不是让程序再发一次。 +const std::vector& transient_upstream_needles() { + static const std::vector needles = { + // 实测:{"code":400,"message":"LLMRequestError: 模型服务异常，请稍候重试， + // 如持续出现，请联系技术支持"} + "请稍候重试", + "请稍后重试", + "模型服务异常", + // 实测:{"code":502,"message":"LLMRequestError: 网络波动或模型处理超时， + // 请稍候重试…"} + "网络波动", + "模型处理超时", + "服务繁忙", + "系统繁忙", + "服务暂时不可用", + }; + return needles; +} + bool contains_any(const std::string& text, const std::vector& needles) { for (const auto& needle : needles) { @@ -49,9 +73,18 @@ FaultKind classify_error_text(const std::string& text) { if (contains_any(text, context_overflow_needles())) { return FaultKind::ContextOverflow; } + // 顺序有意:上下文超限优先。它也可能带「请稍候重试」的客套话,但盲目重试 + // 同一个超大请求只会再撞一次墙,必须先走压缩。 + if (contains_any(text, transient_upstream_needles())) { + return FaultKind::TransientUpstream; + } return FaultKind::None; } +bool is_transient_upstream(const std::string& error_text) { + return classify_error_text(error_text) == FaultKind::TransientUpstream; +} + FaultKind classify(const ProviderErrorInfo& info) { if (!enabled()) return FaultKind::None; if (!info.has_error()) return FaultKind::None; diff --git a/src/pa/pa_quirks.hpp b/src/pa/pa_quirks.hpp index f0746c50a..f33890cb0 100644 --- a/src/pa/pa_quirks.hpp +++ b/src/pa/pa_quirks.hpp @@ -19,6 +19,9 @@ enum class FaultKind { None, // 请求超出服务端实际能接受的上下文规模,应当压缩历史后重试。 ContextOverflow, + // 上游的瞬时故障(服务抽风、网络波动、处理超时)。报文自己就写着「请稍候 + // 重试」,但被塞在 4xx 里返回,通用重试策略会当成客户端错误直接放弃。 + TransientUpstream, }; // 纯文本判定。text 可以是 display_message、raw_body、pretty_json 的任意 @@ -28,6 +31,10 @@ FaultKind classify_error_text(const std::string& text); // 结构化判定。会把 ProviderErrorInfo 里所有可能携带文案的字段合起来看。 FaultKind classify(const ProviderErrorInfo& info); +// classify() == TransientUpstream 的便捷形式,供 retry_policy 的通用判定兜底。 +// 只吃报文文本,不依赖状态码 —— 调用方通常正是因为状态码不可信才来问。 +bool is_transient_upstream(const std::string& error_text); + // classify() == ContextOverflow 的便捷形式,供 compact.cpp 的通用判定兜底。 bool is_context_overflow(const ProviderErrorInfo& info); diff --git a/src/provider/openai_provider.cpp b/src/provider/openai_provider.cpp index 9c212ec93..34c9b3a84 100644 --- a/src/provider/openai_provider.cpp +++ b/src/provider/openai_provider.cpp @@ -1270,6 +1270,9 @@ ChatResponse OpenAiCompatProvider::parse_sse_stream( bool saw_done = false; bool saw_sse_data = false; bool saw_parse_error = false; + // 流里是否成功解析过至少一帧。用来区分两种"没等到 [DONE]"——连接一直 + // 是好的、只是中途断了(可重试),还是从头到尾就没说过人话(不可重试)。 + bool saw_valid_frame = false; bool saw_payload_error = false; std::string payload_error_body; std::string payload_error_message; @@ -1417,6 +1420,7 @@ ChatResponse OpenAiCompatProvider::parse_sse_stream( try { auto j = nlohmann::json::parse(event_data); + saw_valid_frame = true; if (has_stream_error_payload(j)) { saw_payload_error = true; @@ -1653,7 +1657,12 @@ ChatResponse OpenAiCompatProvider::parse_sse_stream( r.error.message, true); } else if (!saw_done) { - const ProviderErrorKind kind = saw_parse_error + // 一帧解析失败**不足以**判定整条流是垃圾。实测网关超时会往正常流里 + // 插一行非 JSON 的 `data:504:Gateway Timeout`,而 saw_parse_error 是 + // 粘性的 —— 旧写法据此把一次本可重试的"流提前结束"降级成不可重试的 + // MalformedJson,压缩摘要请求因此直接失败、整个回合中止。 + // 只有从头到尾没有一帧解析成功,才说明对面根本没在说这个协议。 + const ProviderErrorKind kind = (saw_parse_error && !saw_valid_frame) ? ProviderErrorKind::MalformedJson : ProviderErrorKind::MalformedSse; const std::string transport_message = saw_sse_data diff --git a/src/provider/retry_policy.cpp b/src/provider/retry_policy.cpp index 8197738bc..e79333cc1 100644 --- a/src/provider/retry_policy.cpp +++ b/src/provider/retry_policy.cpp @@ -1,5 +1,7 @@ #include "retry_policy.hpp" +#include "../pa/pa_quirks.hpp" + #include #include @@ -105,6 +107,11 @@ bool provider_http_error_is_retryable(int status_code, break; } + // 客制化网关会把瞬时故障塞进 4xx 返回(报文自己写着「请稍候重试」), + // 落到下面那条 4xx 规则上就成了永久失败。判定收在 src/pa/,只认明确表达 + // 瞬时性的报文;上下文超限不在此列 —— 那个要走压缩,重试没有意义。 + if (pa::is_transient_upstream(body)) return true; + // Authentication, invalid-request, and other client errors remain // terminal even if an upstream message happens to mention overload. if (status_code >= 400 && status_code < 500) return false; diff --git a/tests/agent_loop/agent_loop_compact_events_test.cpp b/tests/agent_loop/agent_loop_compact_events_test.cpp index a089f47a6..eb727af9a 100644 --- a/tests/agent_loop/agent_loop_compact_events_test.cpp +++ b/tests/agent_loop/agent_loop_compact_events_test.cpp @@ -641,6 +641,53 @@ TEST(AgentLoopCompactEvents, ManySmallMessagesDoNotTriggerStructuralCompaction) EXPECT_EQ(provider->stream_calls, 1); } +// 触发场景:摘要压缩失败(上游抽风),但历史足够长、机械修剪能腾出空间。 +// 期望行为:退化到机械修剪,丢掉最旧的历史让回合继续,而不是中止整个回合。 +// 回归背景:摘要压缩要发一次模型请求,于是继承了那次请求的全部失败模式。 +// 旧行为是失败即 `return false`,agent_loop 随后直接 break —— 上下文已经满了, +// 用户只能重开会话(feedback IQSZ-D0423:网关超时导致摘要请求失败,回合中止)。 +// 机械修剪不调用模型,不会以同样的方式失败。 +TEST(AgentLoopCompactEvents, FailedAutoCompactFallsBackToMechanicalPrune) { + auto provider = std::make_shared(); + provider->fail_compact = true; + acecode::ToolExecutor tools; + acecode::PermissionManager permissions; + const auto cwd = make_temp_cwd("compact_fallback_prune"); + std::filesystem::remove_all( + acecode::SessionStorage::get_project_dir(cwd.string())); + // 机械修剪要落 checkpoint,没有 SessionManager 会直接判 Failed —— 兜底也就 + // 无从谈起,所以这里必须挂一个真实会话。 + acecode::SessionManager session; + session.start_session(cwd.string(), "stub", "model"); + acecode::AgentLoop loop( + [&]() -> std::shared_ptr { return provider; }, + tools, {}, cwd.string(), permissions); + loop.set_session_manager(&session); + loop.set_context_window(4000); + // 历史要足够长,机械修剪才有得可丢 —— 这正是与下一个用例的区别:那里只有 + // 两组历史,修剪不动,于是保持「失败即报错」的旧行为。 + add_history(loop, 20); + + const auto events = wait_for_done(loop, [&] { + loop.submit("continue after a failed summarization"); + }); + + // 摘要失败的 error 通知被兜底成功取代,阶段是 warning 而不是 error。 + const auto notices = compact_notices(events); + ASSERT_FALSE(notices.empty()); + EXPECT_EQ(notices.back().stage, "warning"); + for (const auto& notice : notices) { + EXPECT_NE(notice.stage, "error") + << "兜底成功后不应再报压缩失败"; + } + + // 空间确实腾出来了:最旧的那组历史已经不在请求里。 + EXPECT_FALSE(request_contains(loop.messages(), "old assistant 0")); + // 但本轮输入必须留下 —— 兜底是为了让这一轮能继续,不是把它一起丢掉。 + EXPECT_TRUE( + request_contains(loop.messages(), "continue after a failed summarization")); +} + TEST(AgentLoopCompactEvents, FailedAutoCompactIsAtomicAndRetriesOnNextTurn) { auto provider = std::make_shared(); provider->fail_compact = true; diff --git a/tests/pa/pa_quirks_test.cpp b/tests/pa/pa_quirks_test.cpp index 7be81b8cd..45604f61a 100644 --- a/tests/pa/pa_quirks_test.cpp +++ b/tests/pa/pa_quirks_test.cpp @@ -1,6 +1,7 @@ #include #include "commands/compact.hpp" +#include "provider/retry_policy.hpp" #include "pa/pa_quirks.hpp" namespace { @@ -76,9 +77,11 @@ TEST(PaQuirks, ClassificationDoesNotDependOnStatusCode) { // 期望行为:一律不判为溢出。收录标准是「专指这次请求太大」,误判的代价是白跑 // 一次压缩重试并丢掉历史,比漏判贵得多,所以 needle 集宁可窄。 TEST(PaQuirks, GenericChineseErrorsAreNotMisclassified) { + // 注意「请求失败,请稍后重试」不在此列 —— 它自己写着可以再试,现在归 + // TransientUpstream(见 TransientUpstreamFaultInFourHundredIsRetryable)。 + // 「模型繁忙」是状态描述而非重试指示,仍按 None 处理,交给状态码去判。 const char* benign[] = { "参数错误", - "请求失败,请稍后重试", "模型繁忙", "鉴权失败", "上下文格式不正确", @@ -90,7 +93,7 @@ TEST(PaQuirks, GenericChineseErrorsAreNotMisclassified) { info.status_code = 400; info.display_message = text; EXPECT_EQ(acecode::pa::classify(info), acecode::pa::FaultKind::None) - << "误判为上下文溢出:" << text; + << "泛化措辞被误判成了已知故障:" << text; } } @@ -130,3 +133,57 @@ TEST(PaQuirks, GenericEnglishClassificationIsUnchanged) { R"({"error":{"code":"payload_too_large","message":"upload too large"}})"; EXPECT_FALSE(acecode::is_context_overflow_error(payload)); } + +// 触发场景:网关把一个明确写着「请稍候重试」的瞬时故障塞进 HTTP 400 返回。 +// 期望行为:判为 TransientUpstream,并让通用重试策略放行。 +// 回归背景:retry_policy 对 4xx 一律 return false(注释写着「客户端错误即使 +// 提到 overload 也是终止性的」)。这条规则对守规矩的上游是对的,但这套网关把 +// 瞬时故障也编成 400,于是一次本该自动恢复的抽风变成了永久失败。 +TEST(PaQuirks, TransientUpstreamFaultInFourHundredIsRetryable) { + const std::string body = + R"({"code":400,"message":"LLMRequestError: 模型服务异常，请稍候重试，)" + R"(如持续出现，请联系技术支持","object":"error","type":"BadRequestError"})"; + EXPECT_EQ(acecode::pa::classify_error_text(body), + acecode::pa::FaultKind::TransientUpstream); + EXPECT_TRUE(acecode::pa::is_transient_upstream(body)); + EXPECT_TRUE(acecode::provider_http_error_is_retryable(400, body)); + + const std::string gateway_502 = + R"({"code":502,"message":"LLMRequestError: 网络波动或模型处理超时，请稍候重试"})"; + EXPECT_TRUE(acecode::pa::is_transient_upstream(gateway_502)); +} + +// 触发场景:额度用完(HTTP 451),报文里也带「重试」二字。 +// 期望行为:**不**判为瞬时故障,保持终止。 +// 「请切换模型后重试」是要人换模型,不是让程序再发一次 —— 这里重试只会把剩下 +// 的配额也烧掉,而且永远不会成功。 +TEST(PaQuirks, QuotaExhaustedIsNotTreatedAsTransient) { + const std::string body = + R"({"object":"error","message":"当前Agent使用的模型「x」今日额度已用完，)" + R"(请切换模型后重试","type":"BadRequestError","code":451})"; + EXPECT_EQ(acecode::pa::classify_error_text(body), + acecode::pa::FaultKind::None); + EXPECT_FALSE(acecode::provider_http_error_is_retryable(451, body)); +} + +// 触发场景:上下文超限的报文里也出现了瞬时故障的措辞。 +// 期望行为:ContextOverflow 优先。盲目重试同一个超大请求只会再撞一次墙, +// 必须先走压缩;分类顺序在 classify_error_text 里是有意为之的。 +TEST(PaQuirks, ContextOverflowWinsOverTransientWording) { + const std::string body = + R"({"message":"请求上下文过大，请稍候重试","code":400})"; + EXPECT_EQ(acecode::pa::classify_error_text(body), + acecode::pa::FaultKind::ContextOverflow); + EXPECT_FALSE(acecode::pa::is_transient_upstream(body)); +} + +// 触发场景:关掉 PA 适配总开关后的瞬时故障报文。 +// 期望行为:退回通用行为 —— 4xx 仍然终止,证明这层是可摘除的。 +TEST(PaQuirks, DisablingAdapterAlsoDisablesTransientRetry) { + PaEnabledGuard guard(false); + const std::string body = + R"({"code":400,"message":"模型服务异常，请稍候重试"})"; + EXPECT_EQ(acecode::pa::classify_error_text(body), + acecode::pa::FaultKind::None); + EXPECT_FALSE(acecode::provider_http_error_is_retryable(400, body)); +} diff --git a/tests/provider/openai_provider_error_recovery_test.cpp b/tests/provider/openai_provider_error_recovery_test.cpp index d10146d09..b8152df4e 100644 --- a/tests/provider/openai_provider_error_recovery_test.cpp +++ b/tests/provider/openai_provider_error_recovery_test.cpp @@ -246,6 +246,60 @@ TEST(OpenAiProviderErrorRecovery, Http200MalformedSseJsonBecomesMalformedJsonErr EXPECT_NE(err->provider_error.raw_body.find("{not-json}"), std::string::npos); } +// 触发场景:流已经正常传过内容,网关随后超时,往同一条流里插了一行**非 JSON** +// 的 `data:504:Gateway Timeout`,并且流没有 [DONE] 就结束。 +// 期望行为:判为可重试的 MalformedSse,自动重试。 +// 回归背景:saw_parse_error 是粘性标志,旧写法只要流里出现过一帧坏数据,就把这 +// 类"流提前结束"判成终止性的 MalformedJson。线上后果不是多报一个错 —— +// 压缩摘要请求走的是同一条路径,它一失败 agent_loop 就直接中止整个回合,用户 +// 只能重开会话(feedback IQSZ-D0423:网关卡了 2 分 6 秒后返回 504 纯文本, +// 报错为 "parse error at line 1, column 4: unexpected ':'",即 `504:` 的冒号)。 +TEST(OpenAiProviderErrorRecovery, GatewayTextAfterValidFrameStaysRetryable) { + LocalHttpServer server([&](httplib::Server& s) { + s.Post("/chat/completions", + [](const httplib::Request&, httplib::Response& res) { + res.status = 200; + res.set_content( + "data: {\"choices\":[{\"delta\":{\"content\":\"hi\"}}]}\n\n" + "data:504:Gateway Timeout\n\n", + "text/event-stream"); + }); + }); + + OpenAiCompatProvider provider( + "http://127.0.0.1:" + std::to_string(server.port), "", "test-model"); + + const auto events = collect_events_until_retry(provider, 1); + const StreamEvent* retry = + last_event_of_type(events, StreamEventType::Retry); + ASSERT_NE(retry, nullptr); + EXPECT_EQ(retry->provider_error.kind, ProviderErrorKind::MalformedSse); + EXPECT_TRUE(retry->provider_error.retryable); +} + +// 触发场景:整条流从头到尾没有一帧能解析。 +// 期望行为:仍然是不可重试的 MalformedJson —— 上面的放宽不能把这条边界冲掉。 +// 对面根本没在说这个协议时,再发一次大概率还是同样的垃圾,重试只是浪费配额。 +TEST(OpenAiProviderErrorRecovery, StreamWithNoValidFrameStaysTerminal) { + LocalHttpServer server([&](httplib::Server& s) { + s.Post("/chat/completions", + [](const httplib::Request&, httplib::Response& res) { + res.status = 200; + res.set_content("data: {bad}\n\ndata: 504:Gateway Timeout\n\n", "text/event-stream"); + }); + }); + + OpenAiCompatProvider provider( + "http://127.0.0.1:" + std::to_string(server.port), "", "test-model"); + + const auto events = collect_events(provider); + const StreamEvent* err = last_error_event(events); + ASSERT_NE(err, nullptr); + EXPECT_EQ(err->provider_error.kind, ProviderErrorKind::MalformedJson); + EXPECT_FALSE(err->provider_error.retryable); + EXPECT_EQ(count_events(events, StreamEventType::Retry), 0); +} + TEST(OpenAiProviderErrorRecovery, Http200SseErrorPayloadWithEmptyChoicesIsPrintedDirectly) { const std::string error_payload = R"({"error":"liteiim_AuthenticationError: AuthenticationError: OpenAIException - Error code: 401 - {'error': 'Unauthorized'}","choices":[]})"; From 2744614fbbb0c422165c567560eb9f3807630f7f Mon Sep 17 00:00:00 2001 From: tmoonlight Date: Fri, 4 Sep 2026 01:42:59 +0800 Subject: [PATCH 2/5] =?UTF-8?q?feat(pa):=20=E6=94=B6=E6=95=9B=E5=90=8E?= =?UTF-8?q?=E7=9A=84=E4=B8=8A=E4=B8=8B=E6=96=87=E7=AA=97=E5=8F=A3=E6=8C=89?= =?UTF-8?q?=20AIMD=20=E8=87=AA=E5=8A=A8=E5=9B=9E=E5=8D=87?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 原实现是严格单调只降不升:一旦某次在较小规模被拒,该模型在整个进程生命周期 内都被钉在那条线上,唯一复位方式是重启。 这个设计隐含假设了「限制是发现出来的常量」。内网模型的实际容量随负载浮动, 假设不成立 —— 一次低谷期的偶发拒绝会永久拖累之后所有高峰期,用户除了重启 没有别的办法。README 里写着「不落盘是因为限制会变」,结果进程内还是钉死了, 只是把永久缩短成了到重启为止,自相矛盾。 改成 TCP 拥塞控制那套 AIMD: - 乘性减:撞墙立刻降到被拒规模的 85%,回升计时归零(新的拒绝证实限制仍在, 之前攒的档数作废)。 - 加性增:每隔 15 分钟没再被拒就把被拒线抬一档(×1.2)。被砍到一半的窗口 约 1 小时回到声明值,期间最多每 15 分钟撞一次。 - 回到声明值 = 完全退出干预,连 85% 的安全余量也不再扣。否则那个折扣会永久 留着,明明一小时没撞过墙还在按早就不成立的旧证据克扣上下文。 - 回升不改写 lowest_rejected_tokens。它始终是那条实测到的硬证据,回升只是 「当前该用多大」的时变视图;写回去会让下一次拒绝的 min() 失去基准。 敢主动试探的前提是撞墙已经不疼了:识别 → 修剪历史 → 重试全自动,摘要压缩 失败还有机械修剪兜底。没有那两层之前这么做是不负责任的。 时间源用 steady_clock(不受系统时间调整影响);now_ms 可注入,单测不依赖 真实时钟。回升是随时间连续发生的、没有可挂日志的事件,因此在算出窗口的那一 刻比对上次记录值,变了才写一行 LOG_INFO,避免每回合刷屏。 新增 4 个单测(逐档回升 / 拒绝重置计时 / 回升不改写观测 / 封顶声明窗口); 受影响的 427 个用例通过。 --- src/pa/README.md | 17 ++++++ src/pa/pa_context_budget.cpp | 87 +++++++++++++++++++++++++---- src/pa/pa_context_budget.hpp | 38 +++++++++++-- tests/pa/pa_context_budget_test.cpp | 78 ++++++++++++++++++++++++++ 4 files changed, 205 insertions(+), 15 deletions(-) diff --git a/src/pa/README.md b/src/pa/README.md index 77157230d..124bf105b 100644 --- a/src/pa/README.md +++ b/src/pa/README.md @@ -83,6 +83,23 @@ 完全不干预。进程级、不落盘 —— 服务端限制会变,把一次偶发低水位永久钉死 比不学更糟。 +**收敛之后能回升(AIMD)**:只降不升是错的 —— 内网模型的实际容量随负载浮动, +不是测准一次就永远成立的常量。一次低谷期的偶发拒绝会把窗口永久钉死(进程内), +之后所有高峰期都在白白浪费上下文。所以照抄 TCP 拥塞控制那套: + +- **乘性减**:撞墙立刻降到被拒规模的 85%,计时归零。 +- **加性增**:每隔 `PA_CONTEXT_BUDGET_RELAX_INTERVAL_MINUTES`(15 分钟)没再 + 被拒,就把被拒线抬一档(×1.2)。一个被砍到一半的窗口约 1 小时回到声明值。 +- **回到声明值 = 完全退出**。被拒线涨过声明窗口时连 85% 的安全余量也不再扣, + 直接返回声明值。否则那个折扣会永久留着 —— 明明一小时没撞过墙了,还在按一条 + 早就不成立的旧证据克扣上下文。 +- **回升不改写观测**。`lowest_rejected_tokens` 始终是那条实测到的硬证据;回升 + 只是「当前该用多大」的时变视图。写回去会让下一次拒绝的 `min()` 失去基准。 + +敢这么试探的前提是撞墙已经不疼了:识别 → 修剪历史 → 重试是全自动的,摘要压缩 +失败还有机械修剪兜底(`AgentLoop::run_mechanical_compact_fallback`),用户只感知 +到一次重试延迟。没有那两层之前,任何主动试探都是不负责任的。 + 三条踩过的坑,改这块之前先看: - **一条观测要么可信要么丢弃,没有中间态。** 第一版把 diff --git a/src/pa/pa_context_budget.cpp b/src/pa/pa_context_budget.cpp index f9b35a579..fb35bb776 100644 --- a/src/pa/pa_context_budget.cpp +++ b/src/pa/pa_context_budget.cpp @@ -1,6 +1,10 @@ #include "pa_context_budget.hpp" +#include "../utils/logger.hpp" + #include +#include +#include namespace acecode::pa { namespace { @@ -16,6 +20,14 @@ std::string make_key(const std::string& provider, const std::string& model) { return key; } +// 用 steady_clock:回升是按「过了多久」算的,不能被系统时间调整或时区变更 +// 拨来拨去。 +std::int64_t steady_now_ms() { + return std::chrono::duration_cast( + std::chrono::steady_clock::now().time_since_epoch()) + .count(); +} + void split_key(const std::string& key, std::string& provider, std::string& model) { const std::size_t pos = key.find('\x1f'); if (pos == std::string::npos) { @@ -38,30 +50,58 @@ bool observation_is_credible(int rejected_tokens) { return rejected_tokens >= PA_CONTEXT_BUDGET_MIN_CREDIBLE_REJECTION_TOKENS; } -int suggest_effective_window(int declared_window, int lowest_rejected_tokens) { +int suggest_effective_window(int declared_window, + int lowest_rejected_tokens, + std::int64_t ms_since_rejection) { // 没撞过墙、或撞的那次规模小到不可信,都完全不干预 —— 适配层不该在没有 // 可靠证据的时候猜服务端的限制。 if (!observation_is_credible(lowest_rejected_tokens)) return declared_window; - const long long safe = - static_cast(lowest_rejected_tokens) * - PA_CONTEXT_BUDGET_SAFETY_PERCENT / 100; - const int suggested = static_cast(safe); + // AIMD 的「加性增」半边:每隔一个间隔没再被拒,就把被拒线抬一档,直到 + // 封顶于声明窗口。撞墙则由 note_rejected 重置计时起点,回到乘性减。 + long long relaxed = lowest_rejected_tokens; + if (ms_since_rejection > 0) { + constexpr long long kIntervalMs = + static_cast(PA_CONTEXT_BUDGET_RELAX_INTERVAL_MINUTES) * + 60 * 1000; + // 档数封顶:每档 ×1.2,不到 70 档就能从下限涨到 int 上限,而进程连着 + // 跑几天算出来的 steps 会大得离谱。多算的部分对结果没有任何影响。 + const long long steps = + std::min(ms_since_rejection / kIntervalMs, 100); + const long long ceiling = declared_window > 0 + ? static_cast(declared_window) + : static_cast(std::numeric_limits::max()); + for (long long i = 0; i < steps && relaxed < ceiling; ++i) { + relaxed = relaxed * PA_CONTEXT_BUDGET_RELAX_PERCENT / 100; + } + } + + // 回升到声明窗口及以上 = 那条观测已经彻底过期,适配层就该完全退出,连 + // 安全余量也不再扣。否则 85% 的折扣会永久留在那里 —— 明明已经一小时没 + // 撞过墙了,却还在按一条早就不成立的旧证据克扣上下文。 + if (declared_window > 0 && relaxed >= declared_window) return declared_window; + + const long long safe = relaxed * PA_CONTEXT_BUDGET_SAFETY_PERCENT / 100; + const int suggested = static_cast( + std::min(safe, std::numeric_limits::max())); // 声明窗口未知(<= 0)时,观测值就是唯一信息源,直接采用。 if (declared_window <= 0) return suggested; - // 只收不放:观测比声明值还宽的时候,说明这次拒绝跟窗口大小无关,保持声明值。 + // 不越过声明值:回升的终点是用户配置的窗口,不是无限往上试。 return std::min(declared_window, suggested); } void ContextBudgetLearner::note_rejected(const std::string& provider, const std::string& model, - int request_tokens) { + int request_tokens, + std::int64_t now_ms) { if (!observation_is_credible(request_tokens)) return; if (!enabled() || !identity_is_known(provider, model)) return; std::lock_guard lk(mu_); Entry& entry = entries_[make_key(provider, model)]; entry.rejections += 1; + // 重置回升计时:这次拒绝证实了限制仍在,之前攒的档数一律作废。 + entry.last_rejection_ms = now_ms > 0 ? now_ms : steady_now_ms(); entry.lowest_rejected_tokens = entry.lowest_rejected_tokens == 0 ? request_tokens @@ -88,13 +128,40 @@ void ContextBudgetLearner::note_accepted(const std::string& provider, int ContextBudgetLearner::effective_window(const std::string& provider, const std::string& model, - int declared_window) const { + int declared_window, + std::int64_t now_ms) const { if (!enabled() || !identity_is_known(provider, model)) return declared_window; std::lock_guard lk(mu_); auto it = entries_.find(make_key(provider, model)); if (it == entries_.end()) return declared_window; - return suggest_effective_window(declared_window, - it->second.lowest_rejected_tokens); + + Entry& entry = it->second; + const std::int64_t now = now_ms > 0 ? now_ms : steady_now_ms(); + const std::int64_t elapsed = entry.last_rejection_ms > 0 + ? (std::max)(0, now - entry.last_rejection_ms) + : 0; + const int window = suggest_effective_window( + declared_window, entry.lowest_rejected_tokens, elapsed); + + // 回升是随时间连续发生的,没有一个可以挂日志的「事件」——只能在算出来的 + // 那一刻比对。变了才写,否则每个回合都刷一行。 + if (window != entry.last_logged_window) { + const int previous = entry.last_logged_window; + entry.last_logged_window = window; + if (previous > 0) { + LOG_INFO("[pa] compaction window " + + std::string(window > previous ? "relaxed" : "tightened") + + "; from=" + std::to_string(previous) + + " to=" + std::to_string(window) + + " declared=" + std::to_string(declared_window) + + " lowest_rejected=" + + std::to_string(entry.lowest_rejected_tokens) + + " minutes_since_rejection=" + + std::to_string(elapsed / 60000) + + " provider=" + provider + " model=" + model); + } + } + return window; } bool ContextBudgetLearner::has_observation(const std::string& provider, diff --git a/src/pa/pa_context_budget.hpp b/src/pa/pa_context_budget.hpp index c309afca2..0ff0ca920 100644 --- a/src/pa/pa_context_budget.hpp +++ b/src/pa/pa_context_budget.hpp @@ -12,6 +12,7 @@ #include "pa_adapter.hpp" +#include #include #include #include @@ -35,6 +36,19 @@ bool identity_is_known(const std::string& provider, const std::string& model); // 系统性偏低,余量必须覆盖这段偏差,否则收敛后的阈值仍然会越过真实上限。 constexpr int PA_CONTEXT_BUDGET_SAFETY_PERCENT = 85; +// 回升(AIMD 的「加性增」半边)的节奏:每隔这么久没再被拒,就把上限抬一档。 +// +// 为什么必须能升回去:服务端的实际容量是随负载浮动的,不是一个测准一次就 +// 永远成立的常量。只降不升的话,一次低谷期的偶发拒绝会把窗口永久钉死 —— +// 之后所有高峰期都在白白浪费上下文,而用户除了重启进程没有别的办法。 +// +// 为什么敢试探:抬高之后如果服务端并没有恢复,代价就是撞一次墙。而撞墙现在 +// 是全自动恢复的(识别 → 修剪历史 → 重试,摘要压缩失败还有机械修剪兜底), +// 用户只感知到一次重试延迟。15 分钟一档、每档 +20% 的节奏下,一个被砍到 +// 一半的窗口约 1 小时回到声明值,期间最多每 15 分钟撞一次。 +constexpr int PA_CONTEXT_BUDGET_RELAX_INTERVAL_MINUTES = 15; +constexpr int PA_CONTEXT_BUDGET_RELAX_PERCENT = 120; + // 一条被拒观测可信的最小规模。低于这个值的请求不可能撑爆任何**能用**的模型 // —— 光是 system prompt、skill 索引和工具定义就有好几 k。这种拒绝几乎必然 // 另有原因(服务端自己算错、错误文案被复用到别的失败上),采信它会把窗口砍到 @@ -48,11 +62,14 @@ constexpr int PA_CONTEXT_BUDGET_MIN_CREDIBLE_REJECTION_TOKENS = 8192; // 这条被拒观测是否值得采信。 bool observation_is_credible(int rejected_tokens); -// 纯函数形式的收敛算法,便于单测。 +// 纯函数形式的收敛 + 回升算法,便于单测。 // declared_window —— saved_models / 目录声明的窗口,<= 0 表示未知 // lowest_rejected_tokens —— 观测到的最小被拒请求规模,0 表示没撞过墙 +// ms_since_rejection —— 距最后一次被拒过了多久,用来算回升档数 // 返回应当用于压缩决策的窗口。没有可信观测时原样返回 declared_window。 -int suggest_effective_window(int declared_window, int lowest_rejected_tokens); +int suggest_effective_window(int declared_window, + int lowest_rejected_tokens, + std::int64_t ms_since_rejection = 0); // 一个 (provider, model) 的观测快照,用于日志与状态展示。 struct ContextBudgetSnapshot { @@ -71,9 +88,11 @@ class ContextBudgetLearner { // 丢弃 —— 让它进表会永久钉死 lowest_rejected,之后真实的观测再也压不进来。 // 观测单调收敛(只降不升):目标是少撞墙,波动环境里取观测到的最小被拒 // 规模作为上限是唯一安全的选择。 + // now_ms 为 0 表示「用当前时间」;传具体值是为了让单测不依赖真实时钟。 void note_rejected(const std::string& provider, const std::string& model, - int request_tokens); + int request_tokens, + std::int64_t now_ms = 0); // 记录一次被服务端接受的请求规模。只作诊断用,不参与上限计算 —— // 让「成功」抬高上限会在限制波动时来回震荡,反而更常撞墙。 @@ -84,7 +103,8 @@ class ContextBudgetLearner { // 该模型应当用于压缩决策的窗口。无观测时原样返回 declared_window。 int effective_window(const std::string& provider, const std::string& model, - int declared_window) const; + int declared_window, + std::int64_t now_ms = 0) const; // 是否已经为该模型收缩过窗口(供调用方决定要不要提示用户)。 bool has_observation(const std::string& provider, @@ -100,10 +120,18 @@ class ContextBudgetLearner { int lowest_rejected_tokens = 0; int highest_accepted_tokens = 0; int rejections = 0; + // 回升的计时起点。每次被拒都重置 —— 新的拒绝证实了限制仍然存在, + // 已经攒下的回升档数必须清零重来。 + std::int64_t last_rejection_ms = 0; + // 上次记进日志的窗口值。回升是随时间连续发生的,没有一个「事件」可以 + // 挂日志;只能在计算的那一刻比对,变了才写一行,否则每回合刷屏。 + int last_logged_window = 0; }; mutable std::mutex mu_; - std::map entries_; + // mutable:effective_window 在语义上是查询,但「窗口变了要说一声」必须发生 + // 在计算的那一刻 —— 让调用方去比对等于把 pa 的逻辑漏到目录外面。 + mutable std::map entries_; }; // 进程级单例。AgentLoop 在会话之间共享同一份观测 —— 服务端的限制是按模型 diff --git a/tests/pa/pa_context_budget_test.cpp b/tests/pa/pa_context_budget_test.cpp index 5a182aa03..e93e16ea8 100644 --- a/tests/pa/pa_context_budget_test.cpp +++ b/tests/pa/pa_context_budget_test.cpp @@ -7,6 +7,7 @@ namespace { using acecode::pa::ContextBudgetLearner; using acecode::pa::observation_is_credible; +using acecode::pa::PA_CONTEXT_BUDGET_RELAX_INTERVAL_MINUTES; using acecode::pa::suggest_effective_window; constexpr const char* kProvider = "openai"; @@ -204,3 +205,80 @@ TEST(PaContextBudget, ResetClearsAllObservations) { EXPECT_TRUE(learner.snapshot().empty()); EXPECT_EQ(learner.effective_window(kProvider, kModel, 128000), 128000); } + +namespace { +// 单测里一律传显式时间戳,避免依赖真实时钟。 +constexpr std::int64_t kT0 = 1'000'000; +std::int64_t after_minutes(int minutes) { + return kT0 + static_cast(minutes) * 60 * 1000; +} +} // namespace + +// 触发场景:撞墙收敛之后,服务端容量恢复,一段时间内不再拒绝。 +// 期望行为:窗口按 15 分钟一档、每档 +20% 逐步回升,最终封顶于声明窗口。 +// 为什么必须能升:内网模型的实际容量随负载浮动,不是测准一次就永远成立的 +// 常量。只降不升的话,一次低谷期的偶发拒绝会把窗口永久钉死,之后所有高峰期 +// 都在白白浪费上下文,而用户除了重启进程没有别的办法。 +TEST(PaContextBudget, WindowRelaxesStepwiseAfterQuietPeriod) { + // 70000 被拒 → 59500;每档把被拒线 ×1.2 后再取 85%。 + EXPECT_EQ(suggest_effective_window(128000, 70000, 0), 59500); + EXPECT_EQ(suggest_effective_window(128000, 70000, + after_minutes(14) - kT0), 59500); + // 1 档:70000 × 1.2 = 84000 → 71400 + EXPECT_EQ(suggest_effective_window(128000, 70000, + after_minutes(15) - kT0), 71400); + // 2 档:100800 → 85680 + EXPECT_EQ(suggest_effective_window(128000, 70000, + after_minutes(30) - kT0), 85680); + // 3 档:120960 → 102816 + EXPECT_EQ(suggest_effective_window(128000, 70000, + after_minutes(45) - kT0), 102816); + // 4 档:被拒线涨过声明窗口 = 观测彻底过期,适配层完全退出(连 85% 的 + // 安全余量也不再扣),而不是永久停在 123379 那种「差一点」的状态。 + EXPECT_EQ(suggest_effective_window(128000, 70000, + after_minutes(60) - kT0), 128000); + EXPECT_EQ(suggest_effective_window(128000, 70000, + after_minutes(600) - kT0), 128000); +} + +// 触发场景:回升到一半又被拒。 +// 期望行为:计时归零,重新从收敛线开始 —— 新的拒绝证实限制仍在,之前攒下的 +// 档数必须作废,否则会在真实上限附近反复横跳。 +TEST(PaContextBudget, RejectionResetsTheRelaxationClock) { + ContextBudgetLearner learner; + learner.note_rejected(kProvider, kModel, 70000, kT0); + EXPECT_EQ(learner.effective_window(kProvider, kModel, 128000, + after_minutes(30)), 85680); + + // 在 30 分钟这一刻又撞了一次墙(规模比上次小)。 + learner.note_rejected(kProvider, kModel, 60000, after_minutes(30)); + EXPECT_EQ(learner.effective_window(kProvider, kModel, 128000, + after_minutes(30)), 51000); + // 计时从这次拒绝重新起算:再过 14 分钟仍不回升。 + EXPECT_EQ(learner.effective_window(kProvider, kModel, 128000, + after_minutes(44)), 51000); + EXPECT_EQ(learner.effective_window(kProvider, kModel, 128000, + after_minutes(45)), 61200); +} + +// 触发场景:回升过程中反复查询窗口。 +// 期望行为:被拒线本身不被回升改写。回升是「当前该用多大」的时变视图, +// lowest_rejected 仍是那条实测到的硬证据 —— 写回去会让下一次拒绝的 min() +// 比较失去基准。 +TEST(PaContextBudget, RelaxationDoesNotRewriteTheObservation) { + ContextBudgetLearner learner; + learner.note_rejected(kProvider, kModel, 70000, kT0); + (void)learner.effective_window(kProvider, kModel, 128000, + after_minutes(300)); + + const auto snapshot = learner.snapshot(); + ASSERT_EQ(snapshot.size(), 1u); + EXPECT_EQ(snapshot[0].lowest_rejected_tokens, 70000); +} + +// 触发场景:声明窗口本身比回升后的值还小。 +// 期望行为:仍然封顶于声明窗口 —— 回升的终点是用户配置的值,不是无限往上。 +TEST(PaContextBudget, RelaxationNeverExceedsDeclaredWindow) { + EXPECT_EQ(suggest_effective_window(30000, 70000, + after_minutes(600) - kT0), 30000); +} From c8273bcf19d3e5be5519f27c10a1181be5f5b123 Mon Sep 17 00:00:00 2001 From: tmoonlight Date: Fri, 4 Sep 2026 01:45:49 +0800 Subject: [PATCH 3/5] Fix FTXUI overlay cache identity --- ports/ftxui/portfile.cmake | 2 +- ports/ftxui/vcpkg.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ports/ftxui/portfile.cmake b/ports/ftxui/portfile.cmake index 6f3badcab..8d2fd0ce2 100644 --- a/ports/ftxui/portfile.cmake +++ b/ports/ftxui/portfile.cmake @@ -6,7 +6,7 @@ # # 因此:**每次 external/ftxui 的 gitlink 变更,必须同步 bump vcpkg.json 的 # port-version**,否则改动不会生效。当前对应子模块 commit: -# 67a51a7b (v7.0.3-20,含 synchronized output + opt-in mouse hover motion) +# f98588b4 (v7.0.3-22,含 synchronized output + opt-in mouse hover motion) # —— hover-motion 补丁(link-hover-tooltip 依赖无按键 Mouse::Moved 上报, # DEC mode 1003),提供 ftxui::App::EnableMouseHoverMotion()。 vcpkg_check_linkage(ONLY_STATIC_LIBRARY) diff --git a/ports/ftxui/vcpkg.json b/ports/ftxui/vcpkg.json index 3d332aa4f..347bf6e91 100644 --- a/ports/ftxui/vcpkg.json +++ b/ports/ftxui/vcpkg.json @@ -1,7 +1,7 @@ { "name": "ftxui", "version-semver": "7.0.3", - "port-version": 4, + "port-version": 5, "description": "C++ Functional Terminal User Interface (shaohaozhi286 fork; adds ACECode TUI selection, input trace, Kitty keyboard lifecycle, and synchronized output (DEC mode 2026) patches — see external/ftxui/ACECODE_PATCHES.md)", "homepage": "https://github.com/shaohaozhi286/FTXUI", "license": "MIT", From 9b55c85779aa81949efbb6f674663765df6bd714 Mon Sep 17 00:00:00 2001 From: tmoonlight Date: Fri, 4 Sep 2026 01:48:37 +0800 Subject: [PATCH 4/5] Harden contributed runtime and build paths --- .../verify-package/scripts/verify_package.py | 103 +++- .../verify-package/scripts/verify_package.py | 103 +++- .../verify-package/scripts/verify_package.py | 103 +++- .../verify-package/scripts/verify_package.py | 103 +++- .../.openspec.yaml | 2 + .../design.md | 69 +++ .../proposal.md | 31 ++ .../specs/durable-skill-usage-state/spec.md | 42 ++ .../specs/safe-local-build-tooling/spec.md | 52 ++ .../tui-hyperlink-boundary-safety/spec.md | 41 ++ .../tasks.md | 36 ++ scripts/dev_desktop.py | 167 ++++--- src/main.cpp | 21 +- src/markdown/link_safety.cpp | 117 ++++- src/markdown/link_safety.hpp | 9 +- src/skills/skill_usage_store.cpp | 446 +++++++++++++++--- src/skills/skill_usage_store.hpp | 6 +- src/utils/open_url.cpp | 34 ++ src/utils/open_url.hpp | 2 +- tests/CMakeLists.txt | 19 + tests/markdown/link_safety_test.cpp | 40 +- tests/scripts/dev_desktop_test.py | 98 ++++ tests/scripts/verify_package_test.sh | 29 +- tests/scripts/verify_package_unit_test.py | 155 ++++++ tests/skills/skill_usage_store_test.cpp | 170 ++++++- 25 files changed, 1753 insertions(+), 245 deletions(-) create mode 100644 openspec/changes/harden-runtime-and-build-boundaries/.openspec.yaml create mode 100644 openspec/changes/harden-runtime-and-build-boundaries/design.md create mode 100644 openspec/changes/harden-runtime-and-build-boundaries/proposal.md create mode 100644 openspec/changes/harden-runtime-and-build-boundaries/specs/durable-skill-usage-state/spec.md create mode 100644 openspec/changes/harden-runtime-and-build-boundaries/specs/safe-local-build-tooling/spec.md create mode 100644 openspec/changes/harden-runtime-and-build-boundaries/specs/tui-hyperlink-boundary-safety/spec.md create mode 100644 openspec/changes/harden-runtime-and-build-boundaries/tasks.md create mode 100644 tests/scripts/dev_desktop_test.py create mode 100644 tests/scripts/verify_package_unit_test.py diff --git a/.acecode/skills/verify-package/scripts/verify_package.py b/.acecode/skills/verify-package/scripts/verify_package.py index b018a920e..c5bb39ce7 100644 --- a/.acecode/skills/verify-package/scripts/verify_package.py +++ b/.acecode/skills/verify-package/scripts/verify_package.py @@ -27,6 +27,7 @@ BUILD_CONFIGS = ("MinSizeRel", "Release", "RelWithDebInfo", "Debug") DEFAULT_LAUNCH_TIMEOUT = 10 STAGING_DIRNAME = "verify-package-staging" +CMAKE_TARGETS = {"tui": "acecode", "desktop": "acecode-desktop"} class Report: @@ -81,6 +82,47 @@ def find_built(build_dir: Path, relative: str) -> Path | None: return None +def is_same_or_parent(candidate: Path, path: Path) -> bool: + try: + path.relative_to(candidate) + return True + except ValueError: + return False + + +def validate_staging_path(repo: Path, build_dir: Path, staging: Path, *, + cwd: Path | None = None, + home: Path | None = None) -> str | None: + """Return an error when recursive staging cleanup could hit user data.""" + repo = repo.resolve() + build_dir = build_dir.resolve() + staging = staging.resolve() + cwd = (cwd or Path.cwd()).resolve() + home = (home or Path.home()).resolve() + filesystem_root = Path(staging.anchor).resolve() + + protected = ( + ("filesystem root", filesystem_root), + ("home directory", home), + ("current working directory", cwd), + ("repository root", repo), + ("build directory", build_dir), + ) + for label, path in protected: + if is_same_or_parent(staging, path): + return ( + f"unsafe staging directory {staging}: it is the {label} " + "or one of its ancestors" + ) + + if is_same_or_parent(repo, staging) and not is_same_or_parent(build_dir, staging): + return ( + f"unsafe staging directory {staging}: paths inside the repository " + f"must be strict descendants of {build_dir}" + ) + return None + + def sha256_file(path: Path) -> str: digest = hashlib.sha256() with path.open("rb") as stream: @@ -181,32 +223,40 @@ def preflight(report: Report, repo: Path, cmake: str | None, skip_build: bool, def configure_and_build(report: Report, repo: Path, build_dir: Path, cmake: str, - targets: list[str]) -> bool: + targets: list[str], platform: str) -> bool: if not (build_dir / "CMakeCache.txt").is_file(): command = [cmake, "-S", str(repo), "-B", str(build_dir), "-DCMAKE_BUILD_TYPE=MinSizeRel", "-DBUILD_TESTING=OFF", "-DACECODE_BUILD_DESKTOP=ON"] - if shutil.which("ninja"): + # A plain Windows shell can have Ninja on PATH without an initialized + # MSVC environment. Let CMake choose Visual Studio there. + if platform != "windows" and shutil.which("ninja"): command.insert(4, "-G") command.insert(5, "Ninja") vcpkg_root = os.environ.get("VCPKG_ROOT") if vcpkg_root: - command += ["-DCMAKE_TOOLCHAIN_FILE", - str(Path(vcpkg_root) / "scripts" / "buildsystems" / "vcpkg.cmake")] + toolchain = Path(vcpkg_root) / "scripts" / "buildsystems" / "vcpkg.cmake" + command.append(f"-DCMAKE_TOOLCHAIN_FILE={toolchain}") if not run_tool(report, "cmake configure", command): return False else: report.add("cmake configure", "skip", "build dir already configured") for target in targets: - if not run_tool(report, f"cmake build {target}", + cmake_target = CMAKE_TARGETS[target] + if not run_tool(report, f"cmake build {cmake_target}", [cmake, "--build", str(build_dir), "--config", "MinSizeRel", - "--target", target]): + "--target", cmake_target]): return False return True def stage(report: Report, repo: Path, build_dir: Path, staging: Path, platform: str, targets: list[str], cmake: str) -> bool: + staging_error = validate_staging_path(repo, build_dir, staging) + if staging_error: + report.add("stage directory safety", "fail", staging_error) + return False + report.add("stage directory safety", "pass", str(staging)) if staging.exists(): shutil.rmtree(staging) staging.mkdir(parents=True) @@ -354,6 +404,29 @@ def existing_instance_running(platform: str) -> bool | None: return None +def terminate_and_reap(process: subprocess.Popen, timeout: int = 15) -> bool: + try: + process.terminate() + except OSError: + pass + try: + process.wait(timeout=timeout) + return True + except subprocess.TimeoutExpired: + try: + process.kill() + except OSError: + # It may have exited between the timed-out wait and kill. + pass + try: + process.wait(timeout=timeout) + return True + except (OSError, subprocess.TimeoutExpired): + return False + except OSError: + return False + + def probe_desktop(report: Report, staged: Path, platform: str, launch_timeout: int) -> None: running = existing_instance_running(platform) @@ -380,14 +453,13 @@ def probe_desktop(report: Report, staged: Path, platform: str, time.sleep(0.25) if process.poll() is None: # Process survived the entire launch window: treat startup as - # successful, then stop it. - report.add("desktop launch", "pass", - f"alive after launch (pid {process.pid}); terminating") - process.terminate() - try: - process.wait(timeout=15) - except subprocess.TimeoutExpired: - process.kill() + # successful only after it has also been stopped and reaped. + if terminate_and_reap(process): + report.add("desktop launch", "pass", + f"alive after launch (pid {process.pid}); terminated") + else: + report.add("desktop launch", "fail", + f"could not stop and reap pid {process.pid}") else: # Process exited at any point inside the window, including exit code 0. # Verdicts must wait out the window instead of passing on the first @@ -446,7 +518,8 @@ def main(argv: list[str]) -> int: if not args.skip_build: assert cmake is not None - if not configure_and_build(report, repo, build_dir, cmake, targets): + if not configure_and_build( + report, repo, build_dir, cmake, targets, platform): print(f"verify-package: FAIL ({report.failed} check(s) failed)") return 1 diff --git a/.agents/skills/verify-package/scripts/verify_package.py b/.agents/skills/verify-package/scripts/verify_package.py index b018a920e..c5bb39ce7 100644 --- a/.agents/skills/verify-package/scripts/verify_package.py +++ b/.agents/skills/verify-package/scripts/verify_package.py @@ -27,6 +27,7 @@ BUILD_CONFIGS = ("MinSizeRel", "Release", "RelWithDebInfo", "Debug") DEFAULT_LAUNCH_TIMEOUT = 10 STAGING_DIRNAME = "verify-package-staging" +CMAKE_TARGETS = {"tui": "acecode", "desktop": "acecode-desktop"} class Report: @@ -81,6 +82,47 @@ def find_built(build_dir: Path, relative: str) -> Path | None: return None +def is_same_or_parent(candidate: Path, path: Path) -> bool: + try: + path.relative_to(candidate) + return True + except ValueError: + return False + + +def validate_staging_path(repo: Path, build_dir: Path, staging: Path, *, + cwd: Path | None = None, + home: Path | None = None) -> str | None: + """Return an error when recursive staging cleanup could hit user data.""" + repo = repo.resolve() + build_dir = build_dir.resolve() + staging = staging.resolve() + cwd = (cwd or Path.cwd()).resolve() + home = (home or Path.home()).resolve() + filesystem_root = Path(staging.anchor).resolve() + + protected = ( + ("filesystem root", filesystem_root), + ("home directory", home), + ("current working directory", cwd), + ("repository root", repo), + ("build directory", build_dir), + ) + for label, path in protected: + if is_same_or_parent(staging, path): + return ( + f"unsafe staging directory {staging}: it is the {label} " + "or one of its ancestors" + ) + + if is_same_or_parent(repo, staging) and not is_same_or_parent(build_dir, staging): + return ( + f"unsafe staging directory {staging}: paths inside the repository " + f"must be strict descendants of {build_dir}" + ) + return None + + def sha256_file(path: Path) -> str: digest = hashlib.sha256() with path.open("rb") as stream: @@ -181,32 +223,40 @@ def preflight(report: Report, repo: Path, cmake: str | None, skip_build: bool, def configure_and_build(report: Report, repo: Path, build_dir: Path, cmake: str, - targets: list[str]) -> bool: + targets: list[str], platform: str) -> bool: if not (build_dir / "CMakeCache.txt").is_file(): command = [cmake, "-S", str(repo), "-B", str(build_dir), "-DCMAKE_BUILD_TYPE=MinSizeRel", "-DBUILD_TESTING=OFF", "-DACECODE_BUILD_DESKTOP=ON"] - if shutil.which("ninja"): + # A plain Windows shell can have Ninja on PATH without an initialized + # MSVC environment. Let CMake choose Visual Studio there. + if platform != "windows" and shutil.which("ninja"): command.insert(4, "-G") command.insert(5, "Ninja") vcpkg_root = os.environ.get("VCPKG_ROOT") if vcpkg_root: - command += ["-DCMAKE_TOOLCHAIN_FILE", - str(Path(vcpkg_root) / "scripts" / "buildsystems" / "vcpkg.cmake")] + toolchain = Path(vcpkg_root) / "scripts" / "buildsystems" / "vcpkg.cmake" + command.append(f"-DCMAKE_TOOLCHAIN_FILE={toolchain}") if not run_tool(report, "cmake configure", command): return False else: report.add("cmake configure", "skip", "build dir already configured") for target in targets: - if not run_tool(report, f"cmake build {target}", + cmake_target = CMAKE_TARGETS[target] + if not run_tool(report, f"cmake build {cmake_target}", [cmake, "--build", str(build_dir), "--config", "MinSizeRel", - "--target", target]): + "--target", cmake_target]): return False return True def stage(report: Report, repo: Path, build_dir: Path, staging: Path, platform: str, targets: list[str], cmake: str) -> bool: + staging_error = validate_staging_path(repo, build_dir, staging) + if staging_error: + report.add("stage directory safety", "fail", staging_error) + return False + report.add("stage directory safety", "pass", str(staging)) if staging.exists(): shutil.rmtree(staging) staging.mkdir(parents=True) @@ -354,6 +404,29 @@ def existing_instance_running(platform: str) -> bool | None: return None +def terminate_and_reap(process: subprocess.Popen, timeout: int = 15) -> bool: + try: + process.terminate() + except OSError: + pass + try: + process.wait(timeout=timeout) + return True + except subprocess.TimeoutExpired: + try: + process.kill() + except OSError: + # It may have exited between the timed-out wait and kill. + pass + try: + process.wait(timeout=timeout) + return True + except (OSError, subprocess.TimeoutExpired): + return False + except OSError: + return False + + def probe_desktop(report: Report, staged: Path, platform: str, launch_timeout: int) -> None: running = existing_instance_running(platform) @@ -380,14 +453,13 @@ def probe_desktop(report: Report, staged: Path, platform: str, time.sleep(0.25) if process.poll() is None: # Process survived the entire launch window: treat startup as - # successful, then stop it. - report.add("desktop launch", "pass", - f"alive after launch (pid {process.pid}); terminating") - process.terminate() - try: - process.wait(timeout=15) - except subprocess.TimeoutExpired: - process.kill() + # successful only after it has also been stopped and reaped. + if terminate_and_reap(process): + report.add("desktop launch", "pass", + f"alive after launch (pid {process.pid}); terminated") + else: + report.add("desktop launch", "fail", + f"could not stop and reap pid {process.pid}") else: # Process exited at any point inside the window, including exit code 0. # Verdicts must wait out the window instead of passing on the first @@ -446,7 +518,8 @@ def main(argv: list[str]) -> int: if not args.skip_build: assert cmake is not None - if not configure_and_build(report, repo, build_dir, cmake, targets): + if not configure_and_build( + report, repo, build_dir, cmake, targets, platform): print(f"verify-package: FAIL ({report.failed} check(s) failed)") return 1 diff --git a/.claude/skills/verify-package/scripts/verify_package.py b/.claude/skills/verify-package/scripts/verify_package.py index b018a920e..c5bb39ce7 100644 --- a/.claude/skills/verify-package/scripts/verify_package.py +++ b/.claude/skills/verify-package/scripts/verify_package.py @@ -27,6 +27,7 @@ BUILD_CONFIGS = ("MinSizeRel", "Release", "RelWithDebInfo", "Debug") DEFAULT_LAUNCH_TIMEOUT = 10 STAGING_DIRNAME = "verify-package-staging" +CMAKE_TARGETS = {"tui": "acecode", "desktop": "acecode-desktop"} class Report: @@ -81,6 +82,47 @@ def find_built(build_dir: Path, relative: str) -> Path | None: return None +def is_same_or_parent(candidate: Path, path: Path) -> bool: + try: + path.relative_to(candidate) + return True + except ValueError: + return False + + +def validate_staging_path(repo: Path, build_dir: Path, staging: Path, *, + cwd: Path | None = None, + home: Path | None = None) -> str | None: + """Return an error when recursive staging cleanup could hit user data.""" + repo = repo.resolve() + build_dir = build_dir.resolve() + staging = staging.resolve() + cwd = (cwd or Path.cwd()).resolve() + home = (home or Path.home()).resolve() + filesystem_root = Path(staging.anchor).resolve() + + protected = ( + ("filesystem root", filesystem_root), + ("home directory", home), + ("current working directory", cwd), + ("repository root", repo), + ("build directory", build_dir), + ) + for label, path in protected: + if is_same_or_parent(staging, path): + return ( + f"unsafe staging directory {staging}: it is the {label} " + "or one of its ancestors" + ) + + if is_same_or_parent(repo, staging) and not is_same_or_parent(build_dir, staging): + return ( + f"unsafe staging directory {staging}: paths inside the repository " + f"must be strict descendants of {build_dir}" + ) + return None + + def sha256_file(path: Path) -> str: digest = hashlib.sha256() with path.open("rb") as stream: @@ -181,32 +223,40 @@ def preflight(report: Report, repo: Path, cmake: str | None, skip_build: bool, def configure_and_build(report: Report, repo: Path, build_dir: Path, cmake: str, - targets: list[str]) -> bool: + targets: list[str], platform: str) -> bool: if not (build_dir / "CMakeCache.txt").is_file(): command = [cmake, "-S", str(repo), "-B", str(build_dir), "-DCMAKE_BUILD_TYPE=MinSizeRel", "-DBUILD_TESTING=OFF", "-DACECODE_BUILD_DESKTOP=ON"] - if shutil.which("ninja"): + # A plain Windows shell can have Ninja on PATH without an initialized + # MSVC environment. Let CMake choose Visual Studio there. + if platform != "windows" and shutil.which("ninja"): command.insert(4, "-G") command.insert(5, "Ninja") vcpkg_root = os.environ.get("VCPKG_ROOT") if vcpkg_root: - command += ["-DCMAKE_TOOLCHAIN_FILE", - str(Path(vcpkg_root) / "scripts" / "buildsystems" / "vcpkg.cmake")] + toolchain = Path(vcpkg_root) / "scripts" / "buildsystems" / "vcpkg.cmake" + command.append(f"-DCMAKE_TOOLCHAIN_FILE={toolchain}") if not run_tool(report, "cmake configure", command): return False else: report.add("cmake configure", "skip", "build dir already configured") for target in targets: - if not run_tool(report, f"cmake build {target}", + cmake_target = CMAKE_TARGETS[target] + if not run_tool(report, f"cmake build {cmake_target}", [cmake, "--build", str(build_dir), "--config", "MinSizeRel", - "--target", target]): + "--target", cmake_target]): return False return True def stage(report: Report, repo: Path, build_dir: Path, staging: Path, platform: str, targets: list[str], cmake: str) -> bool: + staging_error = validate_staging_path(repo, build_dir, staging) + if staging_error: + report.add("stage directory safety", "fail", staging_error) + return False + report.add("stage directory safety", "pass", str(staging)) if staging.exists(): shutil.rmtree(staging) staging.mkdir(parents=True) @@ -354,6 +404,29 @@ def existing_instance_running(platform: str) -> bool | None: return None +def terminate_and_reap(process: subprocess.Popen, timeout: int = 15) -> bool: + try: + process.terminate() + except OSError: + pass + try: + process.wait(timeout=timeout) + return True + except subprocess.TimeoutExpired: + try: + process.kill() + except OSError: + # It may have exited between the timed-out wait and kill. + pass + try: + process.wait(timeout=timeout) + return True + except (OSError, subprocess.TimeoutExpired): + return False + except OSError: + return False + + def probe_desktop(report: Report, staged: Path, platform: str, launch_timeout: int) -> None: running = existing_instance_running(platform) @@ -380,14 +453,13 @@ def probe_desktop(report: Report, staged: Path, platform: str, time.sleep(0.25) if process.poll() is None: # Process survived the entire launch window: treat startup as - # successful, then stop it. - report.add("desktop launch", "pass", - f"alive after launch (pid {process.pid}); terminating") - process.terminate() - try: - process.wait(timeout=15) - except subprocess.TimeoutExpired: - process.kill() + # successful only after it has also been stopped and reaped. + if terminate_and_reap(process): + report.add("desktop launch", "pass", + f"alive after launch (pid {process.pid}); terminated") + else: + report.add("desktop launch", "fail", + f"could not stop and reap pid {process.pid}") else: # Process exited at any point inside the window, including exit code 0. # Verdicts must wait out the window instead of passing on the first @@ -446,7 +518,8 @@ def main(argv: list[str]) -> int: if not args.skip_build: assert cmake is not None - if not configure_and_build(report, repo, build_dir, cmake, targets): + if not configure_and_build( + report, repo, build_dir, cmake, targets, platform): print(f"verify-package: FAIL ({report.failed} check(s) failed)") return 1 diff --git a/.codex/skills/verify-package/scripts/verify_package.py b/.codex/skills/verify-package/scripts/verify_package.py index b018a920e..c5bb39ce7 100644 --- a/.codex/skills/verify-package/scripts/verify_package.py +++ b/.codex/skills/verify-package/scripts/verify_package.py @@ -27,6 +27,7 @@ BUILD_CONFIGS = ("MinSizeRel", "Release", "RelWithDebInfo", "Debug") DEFAULT_LAUNCH_TIMEOUT = 10 STAGING_DIRNAME = "verify-package-staging" +CMAKE_TARGETS = {"tui": "acecode", "desktop": "acecode-desktop"} class Report: @@ -81,6 +82,47 @@ def find_built(build_dir: Path, relative: str) -> Path | None: return None +def is_same_or_parent(candidate: Path, path: Path) -> bool: + try: + path.relative_to(candidate) + return True + except ValueError: + return False + + +def validate_staging_path(repo: Path, build_dir: Path, staging: Path, *, + cwd: Path | None = None, + home: Path | None = None) -> str | None: + """Return an error when recursive staging cleanup could hit user data.""" + repo = repo.resolve() + build_dir = build_dir.resolve() + staging = staging.resolve() + cwd = (cwd or Path.cwd()).resolve() + home = (home or Path.home()).resolve() + filesystem_root = Path(staging.anchor).resolve() + + protected = ( + ("filesystem root", filesystem_root), + ("home directory", home), + ("current working directory", cwd), + ("repository root", repo), + ("build directory", build_dir), + ) + for label, path in protected: + if is_same_or_parent(staging, path): + return ( + f"unsafe staging directory {staging}: it is the {label} " + "or one of its ancestors" + ) + + if is_same_or_parent(repo, staging) and not is_same_or_parent(build_dir, staging): + return ( + f"unsafe staging directory {staging}: paths inside the repository " + f"must be strict descendants of {build_dir}" + ) + return None + + def sha256_file(path: Path) -> str: digest = hashlib.sha256() with path.open("rb") as stream: @@ -181,32 +223,40 @@ def preflight(report: Report, repo: Path, cmake: str | None, skip_build: bool, def configure_and_build(report: Report, repo: Path, build_dir: Path, cmake: str, - targets: list[str]) -> bool: + targets: list[str], platform: str) -> bool: if not (build_dir / "CMakeCache.txt").is_file(): command = [cmake, "-S", str(repo), "-B", str(build_dir), "-DCMAKE_BUILD_TYPE=MinSizeRel", "-DBUILD_TESTING=OFF", "-DACECODE_BUILD_DESKTOP=ON"] - if shutil.which("ninja"): + # A plain Windows shell can have Ninja on PATH without an initialized + # MSVC environment. Let CMake choose Visual Studio there. + if platform != "windows" and shutil.which("ninja"): command.insert(4, "-G") command.insert(5, "Ninja") vcpkg_root = os.environ.get("VCPKG_ROOT") if vcpkg_root: - command += ["-DCMAKE_TOOLCHAIN_FILE", - str(Path(vcpkg_root) / "scripts" / "buildsystems" / "vcpkg.cmake")] + toolchain = Path(vcpkg_root) / "scripts" / "buildsystems" / "vcpkg.cmake" + command.append(f"-DCMAKE_TOOLCHAIN_FILE={toolchain}") if not run_tool(report, "cmake configure", command): return False else: report.add("cmake configure", "skip", "build dir already configured") for target in targets: - if not run_tool(report, f"cmake build {target}", + cmake_target = CMAKE_TARGETS[target] + if not run_tool(report, f"cmake build {cmake_target}", [cmake, "--build", str(build_dir), "--config", "MinSizeRel", - "--target", target]): + "--target", cmake_target]): return False return True def stage(report: Report, repo: Path, build_dir: Path, staging: Path, platform: str, targets: list[str], cmake: str) -> bool: + staging_error = validate_staging_path(repo, build_dir, staging) + if staging_error: + report.add("stage directory safety", "fail", staging_error) + return False + report.add("stage directory safety", "pass", str(staging)) if staging.exists(): shutil.rmtree(staging) staging.mkdir(parents=True) @@ -354,6 +404,29 @@ def existing_instance_running(platform: str) -> bool | None: return None +def terminate_and_reap(process: subprocess.Popen, timeout: int = 15) -> bool: + try: + process.terminate() + except OSError: + pass + try: + process.wait(timeout=timeout) + return True + except subprocess.TimeoutExpired: + try: + process.kill() + except OSError: + # It may have exited between the timed-out wait and kill. + pass + try: + process.wait(timeout=timeout) + return True + except (OSError, subprocess.TimeoutExpired): + return False + except OSError: + return False + + def probe_desktop(report: Report, staged: Path, platform: str, launch_timeout: int) -> None: running = existing_instance_running(platform) @@ -380,14 +453,13 @@ def probe_desktop(report: Report, staged: Path, platform: str, time.sleep(0.25) if process.poll() is None: # Process survived the entire launch window: treat startup as - # successful, then stop it. - report.add("desktop launch", "pass", - f"alive after launch (pid {process.pid}); terminating") - process.terminate() - try: - process.wait(timeout=15) - except subprocess.TimeoutExpired: - process.kill() + # successful only after it has also been stopped and reaped. + if terminate_and_reap(process): + report.add("desktop launch", "pass", + f"alive after launch (pid {process.pid}); terminated") + else: + report.add("desktop launch", "fail", + f"could not stop and reap pid {process.pid}") else: # Process exited at any point inside the window, including exit code 0. # Verdicts must wait out the window instead of passing on the first @@ -446,7 +518,8 @@ def main(argv: list[str]) -> int: if not args.skip_build: assert cmake is not None - if not configure_and_build(report, repo, build_dir, cmake, targets): + if not configure_and_build( + report, repo, build_dir, cmake, targets, platform): print(f"verify-package: FAIL ({report.failed} check(s) failed)") return 1 diff --git a/openspec/changes/harden-runtime-and-build-boundaries/.openspec.yaml b/openspec/changes/harden-runtime-and-build-boundaries/.openspec.yaml new file mode 100644 index 000000000..9696e00f1 --- /dev/null +++ b/openspec/changes/harden-runtime-and-build-boundaries/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-09-03 diff --git a/openspec/changes/harden-runtime-and-build-boundaries/design.md b/openspec/changes/harden-runtime-and-build-boundaries/design.md new file mode 100644 index 000000000..7c63ac8e9 --- /dev/null +++ b/openspec/changes/harden-runtime-and-build-boundaries/design.md @@ -0,0 +1,69 @@ +## Context + +当前基线把 FTXUI 子模块更新到了包含 `EnableMouseHoverMotion` 的提交，但 overlay port 的版本仍指向旧缓存身份，Windows vcpkg 因而可恢复不含该 API 的历史安装并在 `main.cpp` 编译失败。与此同时，链接 host 提取在切分 authority 前搜索最后一个 `@`，技能使用状态只做进程内串行且按本地时区解析 UTC，两个 Python 工具脚本也缺少危险路径和多配置构建边界。 + +这些问题横跨依赖、TUI runtime、持久化与开发工具，但均属于输入/生命周期边界加固。实现必须保留现有用户数据和公开接口，不修改脏主工作区，不依赖发布侧操作。 + +## Goals / Non-Goals + +**Goals:** + +- 让干净 Windows 环境确定性地编译到当前 FTXUI 子模块源码。 +- 让 TUI 链接安全判断以浏览器实际 authority 为准，并保证 tooltip 和启动器生命周期有界。 +- 让技能使用状态在严格 UTC、损坏字段、多实例和多进程条件下保持可用且不丢更新。 +- 让本地验证脚本只构建真实目标、不会删除受保护目录，并能识别常见 Windows/Desktop 构建布局。 +- 用聚焦单测和完整 Release 构建证明修复。 + +**Non-Goals:** + +- 不改变链接可点击、OSC 8 或悬停延时等既有产品交互。 +- 不迁移技能状态文件格式，不引入数据库或后台服务。 +- 不修改 FTXUI 第三方源码，不发布安装包、tag 或远端分支。 +- 不重构全部构建预设或解决与本次贡献无关的环境性测试波动。 + +## Decisions + +### 1. 通过 overlay `port-version` 表达 FTXUI 源码身份变化 + +提升 `ports/ftxui/vcpkg.json` 的 `port-version` 并同步 portfile 注释，使 vcpkg ABI/缓存身份变化并重新安装当前 gitlink。相比在 ACECode 中绕过 API 或修改 vcpkg 缓存，该方式保留当前功能，也符合 overlay port 的版本语义。 + +### 2. 先切 authority，再处理 userinfo + +HTTP(S) host 提取先在 scheme 后找到 `/`、`\\`、`?`、`#` 中最早的 authority 结束位置，仅在该切片内处理最后一个 `@`、端口和方括号 IPv6。这样路径或 query 中的 `@trusted.example` 不能覆盖真实 host。URL 形显示文字或远端 href 无法可靠解析时采用 fail-closed 降级；普通非 URL 标签仍保持可点击。 + +tooltip 复用现有 cell-width/UTF-8 截断 helper，内容预算由终端宽度推导；极窄终端不画边框浮层。POSIX URL 启动后由分离 waiter 回收直接子进程，避免长期 TUI 产生 zombie。 + +### 3. 技能状态使用严格解析、类型安全读取和锁文件串行 + +时间戳只接受 `YYYY-MM-DDTHH:MM:SS[.fff]Z`，先验证公历字段，再用 `_mkgmtime`/`timegm` 转为 UTC epoch；1/2/3 位小数分别按 100/10/1 毫秒缩放。 + +JSON 字段通过显式类型检查读取；损坏 entry 被视为默认值，写入时正规化为对象，计数饱和而不回绕。每次 mutation 按“实例 mutex → `.lock` 跨进程锁 → 重新读取 → 原子替换”执行。读操作继续依赖原子 rename 读取完整快照，避免无必要阻塞。 + +### 4. 工具脚本在执行前验证解析后的真实边界 + +`verify_package.py` 保留用户目标名 `tui`/`desktop`，仅在 CMake 调用处映射为 `acecode`/`acecode-desktop`。Windows 默认让 CMake 选择 Visual Studio generator；非 Windows 才优先 Ninja。staging 路径在任何删除前 resolve，并拒绝文件系统根、用户目录、当前目录、仓库、build 目录及其危险祖先；仓库内仅允许 build 目录的严格后代。 + +`dev_desktop.py` 保持 Python 3.8 语法，Web 新鲜度覆盖源码、public、manifest/lockfile 和 Vite 配置，Desktop 产物搜索覆盖单配置及常见多配置层级，并安全显示仓库外绝对路径。两个脚本启动的探测子进程均完成 wait/reap。 + +### 5. 回归测试靠近纯边界 + +链接和技能状态使用现有 GoogleTest 套件；脚本 helper 使用 Python `unittest`，并在 CTest 找到解释器时注册。四份 verify-package skill 脚本保持字节一致，并以 hash 检查防止副本漂移。 + +## Risks / Trade-offs + +- [Risk] POSIX 文件锁或 Windows `LockFileEx` 在异常退出时留下 `.lock` 文件 → 锁由 OS handle 持有，进程退出即释放；残留空文件不代表锁定。 +- [Risk] 严格时间解析会拒绝历史畸形时间戳 → 该 entry 的时间按未知处理，但原文件不删除，后续正常写入会恢复有效值。 +- [Risk] staging 边界可能拒绝少见的仓库内自定义目录 → 错误信息明确要求使用 build 子目录或仓库外专用目录，优先保证不可误删。 +- [Risk] detached waiter 为每次 POSIX 打开创建短生命周期线程 → 打开链接是低频交互，成本可控，且避免全局 SIGCHLD 行为影响其他子进程。 +- [Risk] 提升 port-version 会触发一次 FTXUI 重编 → 这是消除错误二进制缓存所需的预期成本。 + +## Migration Plan + +1. 合入 overlay、runtime、持久化和脚本修复及测试。 +2. 在干净 build 目录重新 configure，使 vcpkg 使用新 port-version。 +3. 运行 Web、Python、GoogleTest 和 TUI/Desktop Release 构建。 +4. 若需回滚，可整体回退本变更；技能状态文件格式未改变，无数据迁移步骤。 + +## Open Questions + +无。 diff --git a/openspec/changes/harden-runtime-and-build-boundaries/proposal.md b/openspec/changes/harden-runtime-and-build-boundaries/proposal.md new file mode 100644 index 000000000..766fcb02e --- /dev/null +++ b/openspec/changes/harden-runtime-and-build-boundaries/proposal.md @@ -0,0 +1,31 @@ +## Why + +对近期贡献代码做全量回归后，发现当前远端基线存在一处可稳定复现的 Windows 编译阻断，以及链接边界、技能使用状态和本地构建脚本中的安全与可靠性缺口。这些问题会分别导致错误的依赖缓存被复用、可见 URL 与实际打开主机不一致、使用记录丢失或时间计算错误，以及验证脚本误删目录或构建错误目标，因此需要在继续发布前统一修复。 + +## What Changes + +- 同步 FTXUI 子模块与 vcpkg overlay 的版本标识，确保包含悬停 API 的真实源码被重新构建，恢复 TUI/Desktop 的干净编译。 +- 收紧 TUI 超链接 authority/host 解析，处理路径、查询或反斜杠中的 `@`，对畸形 URL 形链接默认降级，并修正窄终端 tooltip 尺寸及 POSIX 启动器子进程回收。 +- 将技能使用时间戳改为严格 UTC 解析，容忍损坏或旧 schema 字段，并通过跨进程锁保护 read-modify-write，避免并发写入丢失。 +- 修复本地包验证脚本的 CMake 目标映射、Windows generator 选择、staging 删除边界和 Desktop 子进程回收；修复 Desktop 开发脚本的 Python 3.8 兼容性、Web 输入新鲜度判断、构建产物发现和仓库外路径显示。 +- 为上述边界补充回归测试，并执行 Web、脚本、单元测试以及 TUI/Desktop Release 构建验证。 + +## Capabilities + +### New Capabilities + +- `tui-hyperlink-boundary-safety`: 约束链接 host 解析、畸形链接降级、窄终端悬停显示和平台启动器生命周期。 +- `durable-skill-usage-state`: 约束技能使用状态的 UTC 时间语义、损坏数据兼容和跨进程原子更新。 +- `safe-local-build-tooling`: 约束本地验证与 Desktop 开发脚本的目标选择、删除边界、输入新鲜度和产物发现。 + +### Modified Capabilities + +无。 + +## Impact + +- C++ runtime：`src/markdown/link_safety.cpp`、`src/utils/open_url.cpp`、`src/main.cpp`、`src/skills/skill_usage_store.*`。 +- 依赖打包：`ports/ftxui/*`，不修改第三方 FTXUI 源码。 +- 本地工具：四份同步的 `verify_package.py` 和 `scripts/dev_desktop.py`。 +- 测试：`tests/markdown/`、`tests/skills/`、`tests/scripts/` 及测试注册配置。 +- 不改变 daemon/Web API，不引入用户配置迁移，也不发布包或标签。 diff --git a/openspec/changes/harden-runtime-and-build-boundaries/specs/durable-skill-usage-state/spec.md b/openspec/changes/harden-runtime-and-build-boundaries/specs/durable-skill-usage-state/spec.md new file mode 100644 index 000000000..7d1b0e791 --- /dev/null +++ b/openspec/changes/harden-runtime-and-build-boundaries/specs/durable-skill-usage-state/spec.md @@ -0,0 +1,42 @@ +## ADDED Requirements + +### Requirement: 技能时间戳必须按严格 UTC 解释 +技能状态解析器 MUST 只接受 `YYYY-MM-DDTHH:MM:SS[.fff]Z` 形式的有效公历时间，并按 UTC 转换；1 到 3 位小数 MUST 按十进制秒比例转换为毫秒。 + +#### Scenario: UTC 结果不受本地时区影响 +- **WHEN** 解析 `2026-08-01T10:00:00Z` +- **THEN** 结果固定为 Unix epoch `1785578400000` 毫秒 + +#### Scenario: 小数秒按位数缩放 +- **WHEN** 分别解析 `.1Z`、`.12Z` 和 `.123Z` +- **THEN** 毫秒部分分别为 100、120 和 123 + +#### Scenario: 非法日期被拒绝 +- **WHEN** 月、日、时、分、秒越界或时间戳含尾随字符 +- **THEN** 解析失败且不会把标准库归一化后的时间当作有效数据 + +### Requirement: 损坏 schema 不得使公开状态操作抛出 +读取和更新技能状态时，系统 SHALL 对错误类型或非对象 entry 使用安全默认值，并将被更新的 entry 正规化为当前对象 schema。 + +#### Scenario: 字段类型损坏 +- **WHEN** `useCount`、`pinned` 或 `lastUsedAt` 的 JSON 类型与 schema 不符 +- **THEN** summary 和 dormant 查询返回安全默认值，record/set-pinned 不抛异常 + +#### Scenario: 技能 entry 不是对象 +- **WHEN** 某技能 entry 为字符串、数组或 null +- **THEN** 下一次 mutation 将该 entry 替换为有效对象并保存请求的更新 + +#### Scenario: 计数达到上限 +- **WHEN** `useCount` 已为无符号 64 位最大值且再次 record +- **THEN** 计数保持最大值而不回绕 + +### Requirement: 多实例 mutation 必须保留所有更新 +技能状态的 mutation MUST 通过同一 sidecar lock 在进程间串行，并在持锁后重新读取最新快照，再以原子文件替换提交。 + +#### Scenario: 两个实例并发记录同一技能 +- **WHEN** 两个 store 实例并发各执行一次 `record` +- **THEN** 最终 `useCount` 增加 2 且状态文件保持有效 JSON + +#### Scenario: 写入进程异常退出 +- **WHEN** 持锁进程退出并留下 lock 文件路径 +- **THEN** OS 释放锁，后续进程仍可获取锁并更新状态 diff --git a/openspec/changes/harden-runtime-and-build-boundaries/specs/safe-local-build-tooling/spec.md b/openspec/changes/harden-runtime-and-build-boundaries/specs/safe-local-build-tooling/spec.md new file mode 100644 index 000000000..7aab80cd2 --- /dev/null +++ b/openspec/changes/harden-runtime-and-build-boundaries/specs/safe-local-build-tooling/spec.md @@ -0,0 +1,52 @@ +## ADDED Requirements + +### Requirement: 包验证必须构建真实 CMake 目标 +本地包验证器 SHALL 将用户目标 `tui` 和 `desktop` 分别映射为 `acecode` 和 `acecode-desktop`，并在 Windows 非开发者环境中让 CMake 选择可用的默认 generator。 + +#### Scenario: 同时验证 TUI 与 Desktop +- **WHEN** 用户请求 `--target both` +- **THEN** CMake build 命令包含目标 `acecode` 和 `acecode-desktop` + +#### Scenario: Windows 未初始化 MSVC 环境 +- **WHEN** Windows PATH 中存在 Ninja 但没有显式 C/C++ compiler +- **THEN** 配置命令不强制选择 Ninja + +### Requirement: staging 删除必须受路径边界保护 +验证器 MUST 在执行任何递归删除前验证 resolve 后的 staging 路径，并拒绝文件系统根、用户目录、当前目录、仓库、build 目录或这些受保护路径的祖先。仓库内 staging MUST 是 build 目录的严格后代。 + +#### Scenario: staging 指向仓库根 +- **WHEN** 用户将 `--staging-dir` 指向 repo 根目录 +- **THEN** 验证器在删除前失败并保留仓库内容 + +#### Scenario: staging 位于 build 子目录 +- **WHEN** staging 是 build 目录的严格后代 +- **THEN** 验证器允许清理并重建该 staging 目录 + +### Requirement: Desktop 探测进程必须完成回收 +本地包验证器 SHALL 在结束 Desktop smoke 进程后调用 wait 并确认子进程已退出。 + +#### Scenario: smoke 超时后终止 +- **WHEN** Desktop 在 smoke 窗口内持续运行 +- **THEN** 验证器 kill 后等待该进程退出，不遗留未回收子进程 + +### Requirement: Desktop 开发脚本必须兼容声明的运行环境 +开发脚本 MUST 可由 Python 3.8 解析，并 SHALL 对 Web 的源码、public、manifest、lockfile 和 Vite 配置变化触发重建判断。 + +#### Scenario: lockfile 比 dist 新 +- **WHEN** `pnpm-lock.yaml` 的修改时间晚于 `dist/index.html` +- **THEN** Web build 被判定为过期 + +#### Scenario: Python 3.8 加载脚本 +- **WHEN** Python 3.8 解析脚本模块 +- **THEN** 不因仅在更高版本支持的类型注解语法失败 + +### Requirement: Desktop 构建发现必须覆盖常见布局 +开发脚本 SHALL 在指定 build 根目录、直接配置子目录和一层 preset/config 子目录中寻找 Desktop 产物，并 SHALL 能显示仓库外的绝对构建路径。 + +#### Scenario: Visual Studio 多配置产物 +- **WHEN** executable 位于 `build//Release/acecode-desktop.exe` +- **THEN** 自动发现并选择该产物 + +#### Scenario: 指定仓库外 build 目录 +- **WHEN** `--build-dir` 指向 repo 外且产物存在 +- **THEN** 脚本显示绝对路径并继续启动，而不因 `relative_to` 抛出异常 diff --git a/openspec/changes/harden-runtime-and-build-boundaries/specs/tui-hyperlink-boundary-safety/spec.md b/openspec/changes/harden-runtime-and-build-boundaries/specs/tui-hyperlink-boundary-safety/spec.md new file mode 100644 index 000000000..f09e27120 --- /dev/null +++ b/openspec/changes/harden-runtime-and-build-boundaries/specs/tui-hyperlink-boundary-safety/spec.md @@ -0,0 +1,41 @@ +## ADDED Requirements + +### Requirement: 链接 host 必须来自真实 authority +系统 MUST 在处理 userinfo 前切分 HTTP(S) authority，并且路径、反斜杠、query 或 fragment 中的 `@` MUST NOT 改变用于安全比较的 host。 + +#### Scenario: 路径中的 at 符号不覆盖 host +- **WHEN** href 为 `https://evil.example/@trusted.example` +- **THEN** 安全比较使用的 host 为 `evil.example` + +#### Scenario: query 中的 at 符号不覆盖 host +- **WHEN** href 为 `https://evil.example/?next=@trusted.example` +- **THEN** 安全比较使用的 host 为 `evil.example` + +#### Scenario: authority 中的 userinfo 仍受支持 +- **WHEN** href 为 `https://user:pass@example.com/path` +- **THEN** 安全比较使用的 host 为 `example.com` + +### Requirement: 无法验证的远端 URL 形链接必须降级 +当显示文字呈 URL 形状，而远端 HTTP(S) href 无法解析出可靠 host 时，系统 SHALL 将其降级为不可点击纯文本；普通命名标签不受影响。 + +#### Scenario: 畸形远端目标默认不可点击 +- **WHEN** URL 形显示文字对应的 HTTP(S) href authority 为空或畸形 +- **THEN** 链接不进入可点击区域且不带链接样式 + +#### Scenario: 普通标签保持既有行为 +- **WHEN** 显示文字不是 URL 形状 +- **THEN** 合法 HTTP(S) href 仍可点击 + +### Requirement: 悬停气泡必须适配终端 cell 宽度 +链接 tooltip SHALL 按终端 cell 宽度截断并保证浮层不宽于当前终端；无法容纳边框的极窄终端 SHALL 不绘制气泡。 + +#### Scenario: 长 Unicode URL 出现在窄终端 +- **WHEN** tooltip URL 的 cell 宽度超过可用终端宽度 +- **THEN** 文本在有效 UTF-8 边界截断，浮层宽度不超过终端 + +### Requirement: POSIX URL 启动子进程必须被回收 +系统 SHALL 在 POSIX 平台回收用于执行系统 URL opener 的直接子进程，且不得改变主 TUI 的全局 SIGCHLD 策略。 + +#### Scenario: opener 进程退出 +- **WHEN** `xdg-open` 或 `open` 子进程结束 +- **THEN** 后台 waiter 调用 `waitpid` 回收该子进程，不留下 zombie diff --git a/openspec/changes/harden-runtime-and-build-boundaries/tasks.md b/openspec/changes/harden-runtime-and-build-boundaries/tasks.md new file mode 100644 index 000000000..ccb931f85 --- /dev/null +++ b/openspec/changes/harden-runtime-and-build-boundaries/tasks.md @@ -0,0 +1,36 @@ +## 1. FTXUI dependency coherence + +- [x] 1.1 Bump the FTXUI overlay port-version and update its pinned-submodule documentation +- [x] 1.2 Reconfigure the clean Windows build and prove the current FTXUI hover API compiles + +## 2. TUI hyperlink boundary safety + +- [x] 2.1 Add regression tests for authority delimiters, userinfo, malformed remote targets, and IPv6 hosts +- [x] 2.2 Implement authority-first host parsing and fail-closed handling for URL-shaped labels +- [x] 2.3 Bound hover tooltip rendering by terminal cell width and preserve valid UTF-8 +- [x] 2.4 Reap POSIX URL-opener child processes without changing global signal handling + +## 3. Durable skill usage state + +- [x] 3.1 Add tests for strict UTC timestamps, fractional seconds, invalid dates, damaged schemas, saturation, and concurrent instances +- [x] 3.2 Implement strict UTC parsing and type-safe schema normalization +- [x] 3.3 Serialize read-modify-write operations with a cross-process sidecar lock + +## 4. Safe package verification tooling + +- [x] 4.1 Add regression coverage for CMake target mapping, generator choice, staging path guards, and process reaping +- [x] 4.2 Fix verify-package build selection, destructive-path validation, and Desktop process cleanup +- [x] 4.3 Synchronize and hash-check all four verify-package script copies + +## 5. Robust Desktop development tooling + +- [x] 5.1 Add Python tests for declared-version syntax, Web input freshness, multi-config artifact discovery, and external build paths +- [x] 5.2 Fix Python 3.8 compatibility, Web freshness inputs, build discovery, ANSI output, and path display +- [x] 5.3 Register and run the script test in the repository test suite when Python is available + +## 6. Full verification + +- [x] 6.1 Run focused Python and GoogleTest regressions +- [x] 6.2 Run the complete Web test and production build +- [x] 6.3 Build TUI, Desktop, and unit tests in Release and run the full CTest suite +- [x] 6.4 Run repository quality/diff checks and confirm unrelated dirty files are excluded diff --git a/scripts/dev_desktop.py b/scripts/dev_desktop.py index 65737c877..6347fd2bf 100644 --- a/scripts/dev_desktop.py +++ b/scripts/dev_desktop.py @@ -16,6 +16,8 @@ python scripts/dev_desktop.py --list # 列出可用的 desktop 构建产物 """ +from __future__ import annotations + import argparse import os import platform @@ -43,26 +45,26 @@ def _c(text: str, code: str) -> str: def info(msg: str) -> None: - print(_c("[INFO] ", "36m") + msg) + print(_c("[INFO] ", "36") + msg) def ok(msg: str) -> None: - print(_c("[OK] ", "32m") + msg) + print(_c("[OK] ", "32") + msg) def warn(msg: str) -> None: - print(_c("[WARN] ", "33m") + msg) + print(_c("[WARN] ", "33") + msg) def error(msg: str) -> None: - print(_c("[ERROR]", "31m") + " " + msg, file=sys.stderr) + print(_c("[ERROR]", "31") + " " + msg, file=sys.stderr) def banner(msg: str) -> None: width = max(len(msg) + 4, 40) - print(_c("=" * width, "35m")) - print(_c(f" {msg}", "35m")) - print(_c("=" * width, "35m")) + print(_c("=" * width, "35")) + print(_c(f" {msg}", "35")) + print(_c("=" * width, "35")) # ─── 项目根目录定位 ──────────────────────────────────────────────────────── @@ -111,28 +113,58 @@ def ensure_node_and_pnpm() -> tuple[str, str]: # ─── Web 构建 ────────────────────────────────────────────────────────────── +WEB_BUILD_INPUT_FILES = ( + "index.html", + "package.json", + "pnpm-lock.yaml", + "pnpm-workspace.yaml", +) + + +def iter_web_build_inputs(web_dir: Path): + """Yield files that can affect Vite's production output.""" + seen = set() + for directory_name in ("src", "public"): + directory = web_dir / directory_name + if not directory.is_dir(): + continue + for path in directory.rglob("*"): + if path.is_file() and path not in seen: + seen.add(path) + yield path + for name in WEB_BUILD_INPUT_FILES: + path = web_dir / name + if path.is_file() and path not in seen: + seen.add(path) + yield path + for pattern in ("vite.config.*", "postcss.config.*", "tailwind.config.*"): + for path in web_dir.glob(pattern): + if path.is_file() and path not in seen: + seen.add(path) + yield path + + +def web_build_is_stale(web_dir: Path, index_html: Path) -> bool: + if not index_html.is_file(): + return True + try: + dist_mtime = index_html.stat().st_mtime + return any(path.stat().st_mtime > dist_mtime + for path in iter_web_build_inputs(web_dir)) + except OSError: + # A file changed while scanning; rebuild rather than trusting a stale dist. + return True + def build_web(web_dir: Path, pnpm: str, force: bool = False) -> None: """构建 web 前端到 web/dist/。""" dist_dir = web_dir / "dist" index_html = dist_dir / "index.html" if not force and index_html.exists(): - # 检查 src/ 下是否有比 dist/index.html 更新的文件 - src_dir = web_dir / "src" - if src_dir.is_dir(): - dist_mtime = index_html.stat().st_mtime - needs_rebuild = False - for f in src_dir.rglob("*"): - if f.is_file() and f.stat().st_mtime > dist_mtime: - needs_rebuild = True - break - if not needs_rebuild: - ok("web/dist/ 已是最新，跳过构建") - return - info("检测到 web/src/ 有更新，重新构建...") - else: - ok("web/dist/ 已存在，跳过构建") + if not web_build_is_stale(web_dir, index_html): + ok("web/dist/ 已是最新，跳过构建") return + info("检测到 Web 构建输入有更新，重新构建...") else: if force: info("强制重新构建 web...") @@ -155,33 +187,67 @@ def build_web(web_dir: Path, pnpm: str, force: bool = False) -> None: # ─── Desktop 构建产物定位 ─────────────────────────────────────────────────── def find_desktop_builds(build_dir: Path) -> list[Path]: - """在 build/ 目录下查找所有可用的 desktop 构建产物。""" + """查找 build 根、直接子目录及 preset/config 两层布局的产物。""" results: list[Path] = [] if not build_dir.is_dir(): return results - for child in sorted(build_dir.iterdir()): - if not child.is_dir(): + search_dirs = [build_dir] + try: + first_level = sorted( + (path for path in build_dir.iterdir() + if path.is_dir() and path.suffix != ".app"), + key=lambda path: str(path).lower(), + ) + except OSError: + first_level = [] + search_dirs.extend(first_level) + for first in first_level: + try: + search_dirs.extend(sorted( + (path for path in first.iterdir() + if path.is_dir() and path.suffix != ".app"), + key=lambda path: str(path).lower(), + )) + except OSError: continue + + seen = set() + for child in search_dirs: # macOS .app bundle app_bundle = child / "ACECode.app" if app_bundle.is_dir(): - results.append(app_bundle) - continue + resolved = app_bundle.resolve() + if resolved not in seen: + seen.add(resolved) + results.append(app_bundle) # Windows .exe exe = child / "acecode-desktop.exe" - if exe.exists(): - results.append(exe) - continue + if exe.is_file(): + resolved = exe.resolve() + if resolved not in seen: + seen.add(resolved) + results.append(exe) # Linux / macOS 裸可执行文件（非 .app） binary = child / "acecode-desktop" - if binary.exists() and os.access(binary, os.X_OK): - results.append(binary) - continue + if binary.is_file() and os.access(binary, os.X_OK): + resolved = binary.resolve() + if resolved not in seen: + seen.add(resolved) + results.append(binary) return results +def display_path(path: Path, project_root: Path) -> str: + """Prefer a repo-relative path, but support explicitly external builds.""" + resolved = path.resolve() + try: + return str(resolved.relative_to(project_root.resolve())) + except ValueError: + return str(resolved) + + def pick_desktop_build(builds: list[Path], preferred: str | None = None) -> Path | None: """从可用构建中选择一个。优先选 release，其次按名称排序。""" if not builds: @@ -286,7 +352,7 @@ def main() -> None: else: ok(f"找到 {len(builds)} 个 desktop 构建产物:") for i, b in enumerate(builds, 1): - print(f" {i}. {b.relative_to(project_root)}") + print(f" {i}. {display_path(b, project_root)}") return # 3. 检查并构建 web @@ -304,28 +370,18 @@ def main() -> None: specified = Path(args.build_dir) if not specified.is_absolute(): specified = project_root / specified + specified = specified.resolve() # 支持指定到 .app / .exe / 目录 - if specified.suffix == ".app" or specified.name.endswith(".exe"): + if (specified.suffix == ".app" or specified.name.endswith(".exe") or + specified.name == "acecode-desktop"): desktop_path = specified else: - # 目录下查找 - candidates = find_desktop_builds(specified.parent if specified.name.startswith("build") else specified) + candidates = find_desktop_builds(specified) if not candidates: - # 直接在指定目录下找 - app = specified / "ACECode.app" - exe = specified / "acecode-desktop.exe" - binf = specified / "acecode-desktop" - if app.is_dir(): - desktop_path = app - elif exe.exists(): - desktop_path = exe - elif binf.exists() and os.access(binf, os.X_OK): - desktop_path = binf - else: - error(f"在指定目录下未找到 desktop 构建产物: {specified}") - sys.exit(1) + error(f"在指定目录下未找到 desktop 构建产物: {specified}") + sys.exit(1) else: - desktop_path = candidates[0] + desktop_path = pick_desktop_build(candidates) else: builds = find_desktop_builds(build_dir) if not builds: @@ -337,13 +393,14 @@ def main() -> None: sys.exit(1) desktop_path = pick_desktop_build(builds) if len(builds) > 1: - info(f"找到 {len(builds)} 个构建产物，自动选择: {desktop_path.relative_to(project_root)}") + info(f"找到 {len(builds)} 个构建产物，自动选择: " + f"{display_path(desktop_path, project_root)}") info("(可用 --build-dir 指定其他构建，或 --list 查看全部)") if not desktop_path.exists(): error(f"desktop 构建产物不存在: {desktop_path}") sys.exit(1) - ok(f"Desktop 构建: {desktop_path.relative_to(project_root)}") + ok(f"Desktop 构建: {display_path(desktop_path, project_root)}") # 5. 启动 desktop launch_desktop(desktop_path, dev_web_dir) @@ -351,14 +408,14 @@ def main() -> None: # 6. 打印使用提示 print() - print(_c("── 开发提示 ──────────────────────────────────────", "35m")) + print(_c("── 开发提示 ──────────────────────────────────────", "35")) print(" 1. 修改 web/src/ 下的代码后，重新构建:") print(f" cd {web_dir} && pnpm build") print(" 或重新运行本脚本加 --rebuild") print(" 2. 在 Desktop 窗口按 F5 刷新页面即可看到改动") print(" 3. 按 F11 打开 WebView 开发者工具调试") print(" 4. 无需重新编译 C++!") - print(_c("──────────────────────────────────────────────────", "35m")) + print(_c("──────────────────────────────────────────────────", "35")) print() diff --git a/src/main.cpp b/src/main.cpp index 8fd518569..1d5677696 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -3240,23 +3240,28 @@ struct TuiRendererContext { // 调用方须持有 state.mu(render_tui_frame 入口已持锁)。 static Element render_link_hover_tooltip(const TuiState& state) { const auto term = Terminal::Size(); + // 边框至少需要 2 列/3 行;极窄终端直接跳过浮层,避免 dbox 的需求尺寸 + // 反向撑大主布局。 + if (term.dimx < 4 || term.dimy < 3) { + return emptyElement(); + } // 显示真实 URL(href 原文,防骗 —— 显示文本可能被 Markdown 伪装)。 - // 超长截断加省略号;宽/高预算含 2 格 border 边框,确保 + // 按 cell 而不是 UTF-8 字节截断;宽/高预算含 2 格 border 边框,确保 // x + bubble_w <= dimx 恒成立,不会撑大 dbox 需求。 - std::string url = state.hover_link_href; - const int max_url_w = std::max(20, term.dimx - 4); - if (static_cast(url.size()) > max_url_w) { - url = url.substr(0, static_cast(max_url_w) - 3) + "..."; - } - const int bubble_w = static_cast(url.size()) + 2; + const int max_url_cells = term.dimx - 2; + const std::string url = truncate_cells_middle_ascii( + state.hover_link_href, max_url_cells); + const int bubble_w = std::min( + term.dimx, std::max(2, ftxui::string_width(url) + 2)); const int bubble_h = 3; // border top + text row + border bottom const int px = state.hover_link_x; const int py = state.hover_link_y; int x = px + 2; // 指针右上方 if (x + bubble_w > term.dimx) { - x = std::max(0, px - bubble_w - 2); // 右侧不够 → 指针左侧 + x = px - bubble_w - 2; // 右侧不够 → 指针左侧 } + x = std::clamp(x, 0, term.dimx - bubble_w); int y = py - bubble_h - 1; // 指针上方 if (y < 0) { y = py + 1; // 上方不够 → 指针下方 diff --git a/src/markdown/link_safety.cpp b/src/markdown/link_safety.cpp index 729b750e2..efc394c24 100644 --- a/src/markdown/link_safety.cpp +++ b/src/markdown/link_safety.cpp @@ -31,6 +31,37 @@ bool looks_like_url(const std::string& s) { return s.find_first_of(" \t\r\n") == std::string::npos; } +bool is_valid_port(const std::string& port) { + if (port.empty()) return false; + unsigned int value = 0; + for (const unsigned char c : port) { + if (std::isdigit(c) == 0) return false; + const unsigned int digit = static_cast(c - '0'); + if (value > (65535u - digit) / 10u) return false; + value = value * 10u + digit; + } + return true; +} + +bool is_valid_dns_host(const std::string& host) { + if (host.empty() || host.front() == '.' || host.back() == '.' || + host.find("..") != std::string::npos) { + return false; + } + return std::all_of(host.begin(), host.end(), [](unsigned char c) { + return std::isalnum(c) != 0 || c == '-' || c == '_' || c == '.'; + }); +} + +bool is_valid_bracketed_ipv6_host(const std::string& host) { + if (host.empty() || host.find(':') == std::string::npos) { + return false; + } + return std::all_of(host.begin(), host.end(), [](unsigned char c) { + return std::isxdigit(c) != 0 || c == ':' || c == '.'; + }); +} + } // namespace std::optional extract_url_host(const std::string& url) { @@ -46,39 +77,85 @@ std::optional extract_url_host(const std::string& url) { } } - // scheme:跳过 "://" 之前的 authority 前缀。无 "://" 时整个字符串按 - // authority 解析(裸域名 / userinfo / 端口 / 路径形式),不做协议名判定—— - // "mailto:" 这类单冒号协议无法与 "user:pass@" 可靠区分,而两者解析出 - // 的 host 都是安全的比较对象(file:/// 等 host 为空的情况仍会失败)。 + // scheme:跳过 "://" 之前的前缀。无 "://" 时从字符串开头按 authority + // 解析(裸域名 / userinfo / 端口 / 路径形式)。先找 authority 的结束位置, + // 再处理 userinfo:路径/query 中的 "@trusted.example" 绝不能覆盖真实 host。 const std::string::size_type scheme = u.find("://"); + std::string::size_type authority_begin = 0; if (scheme != std::string::npos) { - u = u.substr(scheme + 3); + if (scheme == 0 || !std::isalpha(static_cast(u[0]))) { + return std::nullopt; + } + for (std::string::size_type i = 1; i < scheme; ++i) { + const unsigned char c = static_cast(u[i]); + if (std::isalnum(c) == 0 && c != '+' && c != '-' && c != '.') { + return std::nullopt; + } + } + authority_begin = scheme + 3; + } + + const std::string::size_type authority_end = + u.find_first_of("/\\?#", authority_begin); + std::string authority = u.substr( + authority_begin, + authority_end == std::string::npos + ? std::string::npos + : authority_end - authority_begin); + if (authority.empty()) { + return std::nullopt; } - // userinfo:取最后一个 '@' 之后(host 之前的 user:pass@)。 - const std::string::size_type at = u.rfind('@'); + // userinfo 只允许出现在 authority 内。取最后一个 '@' 后的 host:port。 + const std::string::size_type at = authority.rfind('@'); if (at != std::string::npos) { - u = u.substr(at + 1); + authority = authority.substr(at + 1); + } + if (authority.empty()) { + return std::nullopt; } - // host 段:到 ':'(端口)、'/'(路径)、'?'(查询)、'#'(片段) 为止。 - const std::string::size_type end = u.find_first_of(":/?#"); - if (end != std::string::npos) { - u = u.substr(0, end); + std::string host; + bool bracketed_ipv6 = false; + if (authority.front() == '[') { + const auto close = authority.find(']'); + if (close == std::string::npos) { + return std::nullopt; + } + host = authority.substr(1, close - 1); + const std::string suffix = authority.substr(close + 1); + if (!suffix.empty() && + (suffix.front() != ':' || !is_valid_port(suffix.substr(1)))) { + return std::nullopt; + } + bracketed_ipv6 = true; + } else { + const auto colon = authority.find(':'); + if (colon == std::string::npos) { + host = authority; + } else { + // 未加方括号的 IPv6 与 host:port 有歧义,拒绝。 + if (authority.find(':', colon + 1) != std::string::npos || + !is_valid_port(authority.substr(colon + 1))) { + return std::nullopt; + } + host = authority.substr(0, colon); + } } - // 去尾部句点(句子结尾标点)。 - while (!u.empty() && u.back() == '.') { - u.pop_back(); + // 去尾部句点(裸域名位于句子结尾时的标点)。 + while (!bracketed_ipv6 && !host.empty() && host.back() == '.') { + host.pop_back(); } - if (u.empty()) { + if (bracketed_ipv6 ? !is_valid_bracketed_ipv6_host(host) + : !is_valid_dns_host(host)) { return std::nullopt; } // 域名不区分大小写 → 统一小写,便于比较。 - std::transform(u.begin(), u.end(), u.begin(), + std::transform(host.begin(), host.end(), host.begin(), [](unsigned char c) { return static_cast(std::tolower(c)); }); - return u; + return host; } bool is_safe_link_label(const std::string& label, const std::string& href) { @@ -90,10 +167,10 @@ bool is_safe_link_label(const std::string& label, const std::string& href) { return true; } - // href 是远程 URL:host 无法解析(file:///、畸形)→ 无目标可比,放行。 + // href 是远程 URL:URL 形 label 必须有可验证目标;否则 fail closed。 const auto href_host = extract_url_host(href); if (!href_host.has_value()) { - return true; + return !looks_like_url(label); } // label 不呈 URL 形状 → 普通文字标签(如"我的博客"),放行。 diff --git a/src/markdown/link_safety.hpp b/src/markdown/link_safety.hpp index 48f432cf0..37eae32e8 100644 --- a/src/markdown/link_safety.hpp +++ b/src/markdown/link_safety.hpp @@ -10,7 +10,8 @@ // - href 不含 "://"(本地文件路径 / 裸域名)→ 本地链接通道,label 是文件名 // 或任意文字,放行。防骗只针对网页链接:`[foo.md](docs/foo.md)` 是常见 // 写法,不能因文件名带点号被误降级。 -// - href 是远程 URL 但 host 解析失败(file:///、畸形)→ 无目标可比,放行。 +// - href 是远程 URL 但 host 解析失败(file:///、畸形)→ URL 形 label 降级, +// 普通文字标签保持既有行为。 // - label 无"URL 形状"(不含点号,或含空白)→ 普通文字标签,放行。 // - label 呈 URL 形状 → 提取 host 与 href host 比较(忽略大小写);一致放行, // 不一致降级;畸形/非 ASCII 的 URL 按不匹配处理(降级)。 @@ -22,9 +23,9 @@ namespace acecode::markdown { -// 从 URL / 裸域名中提取 host(去 scheme、userinfo、端口、路径/查询/片段, -// 去尾部句点,小写化)。本地路径、非 http(s) 协议(mailto:/file:/data: 等)、 -// 含非 ASCII(IDN/百分号编码不做)解析失败 → nullopt。 +// 从 URL / 裸域名中提取 host(先切 authority,再去 userinfo/端口,支持方括号 +// IPv6,去尾部句点,小写化)。本地路径、空 authority、畸形端口、含非 ASCII +// (IDN/百分号编码不做)解析失败 → nullopt。 std::optional extract_url_host(const std::string& url); // 防骗校验:label 与 href 组合是否安全渲染为可点击链接(见文件头决策规则)。 diff --git a/src/skills/skill_usage_store.cpp b/src/skills/skill_usage_store.cpp index fec4504a7..3b6935606 100644 --- a/src/skills/skill_usage_store.cpp +++ b/src/skills/skill_usage_store.cpp @@ -2,13 +2,28 @@ #include "utils/atomic_file.hpp" #include "utils/logger.hpp" +#include "utils/utf8_path.hpp" #include -#include +#include +#include +#include #include -#include -#include +#include +#include + +#ifdef _WIN32 +# ifndef WIN32_LEAN_AND_MEAN +# define WIN32_LEAN_AND_MEAN +# endif +# include +#else +# include +# include +# include +# include +#endif namespace fs = std::filesystem; @@ -19,13 +34,120 @@ namespace { constexpr int kStateVersion = 1; constexpr std::size_t kMaxStateFileBytes = 1024 * 1024; // 1 MB +class SkillUsageWriteLock { +public: + explicit SkillUsageWriteLock(const std::string& state_path) { + fs::path lock_path; + try { + lock_path = path_from_utf8(state_path + ".lock"); + } catch (const std::exception& e) { + error_ = "failed to resolve skill usage lock path: " + + std::string(e.what()); + return; + } + + std::error_code ec; + if (!lock_path.parent_path().empty()) { + fs::create_directories(lock_path.parent_path(), ec); + if (ec) { + error_ = "failed to create skill usage lock directory: " + + ec.message(); + return; + } + } + +#ifdef _WIN32 + handle_ = ::CreateFileW( + lock_path.wstring().c_str(), + GENERIC_READ | GENERIC_WRITE, + FILE_SHARE_READ | FILE_SHARE_WRITE, + nullptr, + OPEN_ALWAYS, + FILE_ATTRIBUTE_NORMAL, + nullptr); + if (handle_ == INVALID_HANDLE_VALUE) { + error_ = "failed to open skill usage lock: error " + + std::to_string(::GetLastError()); + return; + } + + OVERLAPPED overlapped{}; + if (!::LockFileEx( + handle_, + LOCKFILE_EXCLUSIVE_LOCK, + 0, + MAXDWORD, + MAXDWORD, + &overlapped)) { + const DWORD error = ::GetLastError(); + ::CloseHandle(handle_); + handle_ = INVALID_HANDLE_VALUE; + error_ = "failed to acquire skill usage lock: error " + + std::to_string(error); + return; + } +#else + fd_ = ::open( + path_to_utf8(lock_path).c_str(), + O_CREAT | O_RDWR, + S_IRUSR | S_IWUSR); + if (fd_ < 0) { + error_ = "failed to open skill usage lock: " + + std::error_code(errno, std::generic_category()).message(); + return; + } + while (::flock(fd_, LOCK_EX) != 0) { + if (errno == EINTR) continue; + error_ = "failed to acquire skill usage lock: " + + std::error_code(errno, std::generic_category()).message(); + ::close(fd_); + fd_ = -1; + return; + } +#endif + acquired_ = true; + } + + SkillUsageWriteLock(const SkillUsageWriteLock&) = delete; + SkillUsageWriteLock& operator=(const SkillUsageWriteLock&) = delete; + + ~SkillUsageWriteLock() { +#ifdef _WIN32 + if (handle_ != INVALID_HANDLE_VALUE) { + OVERLAPPED overlapped{}; + ::UnlockFileEx(handle_, 0, MAXDWORD, MAXDWORD, &overlapped); + ::CloseHandle(handle_); + } +#else + if (fd_ >= 0) { + ::flock(fd_, LOCK_UN); + ::close(fd_); + } +#endif + } + + bool acquired() const { return acquired_; } + const std::string& error() const { return error_; } + +private: + bool acquired_ = false; + std::string error_; +#ifdef _WIN32 + HANDLE handle_ = INVALID_HANDLE_VALUE; +#else + int fd_ = -1; +#endif +}; + // Read the state file into a JSON object. Returns an empty object when the // file is absent, oversized, corrupted, or has an unsupported version so the // feature degrades gracefully instead of failing the caller. nlohmann::json load_state_or_empty(const std::string& path) { std::error_code ec; if (!fs::exists(path, ec)) return nlohmann::json::object(); - if (fs::file_size(path, ec) > kMaxStateFileBytes) { + const auto state_size = fs::file_size(path, ec); + if (ec) return nlohmann::json::object(); + if (state_size > kMaxStateFileBytes) { LOG_WARN("[skill_usage] state file too large, ignoring"); return nlohmann::json::object(); } @@ -45,6 +167,86 @@ nlohmann::json load_state_or_empty(const std::string& path) { } } +std::uint64_t uint64_member_or(const nlohmann::json& object, + const char* key, + std::uint64_t fallback = 0) { + if (!object.is_object()) return fallback; + const auto it = object.find(key); + if (it == object.end()) return fallback; + if (it->is_number_unsigned()) { + return it->get(); + } + if (it->is_number_integer()) { + const auto value = it->get(); + return value >= 0 ? static_cast(value) : fallback; + } + return fallback; +} + +bool bool_member_or(const nlohmann::json& object, + const char* key, + bool fallback = false) { + if (!object.is_object()) return fallback; + const auto it = object.find(key); + return it != object.end() && it->is_boolean() + ? it->get() + : fallback; +} + +std::string string_member_or(const nlohmann::json& object, + const char* key, + std::string fallback = {}) { + if (!object.is_object()) return fallback; + const auto it = object.find(key); + return it != object.end() && it->is_string() + ? it->get() + : fallback; +} + +nlohmann::json& ensure_skills_object(nlohmann::json& state) { + auto& skills = state["skills"]; + if (!skills.is_object()) { + skills = nlohmann::json::object(); + } + return skills; +} + +nlohmann::json& ensure_skill_entry(nlohmann::json& skills, + const std::string& skill_name) { + auto& entry = skills[skill_name]; + if (!entry.is_object()) { + entry = nlohmann::json::object(); + } + return entry; +} + +bool parse_decimal_field(const std::string& text, + std::size_t offset, + std::size_t length, + int& value) { + if (offset + length > text.size()) return false; + value = 0; + for (std::size_t i = offset; i < offset + length; ++i) { + const unsigned char c = static_cast(text[i]); + if (c < '0' || c > '9') return false; + value = value * 10 + static_cast(c - '0'); + } + return true; +} + +bool is_leap_year(int year) { + return year % 4 == 0 && (year % 100 != 0 || year % 400 == 0); +} + +int days_in_month(int year, int month) { + static constexpr int kDays[] = { + 0, 31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31, + }; + if (month < 1 || month > 12) return 0; + if (month == 2 && is_leap_year(year)) return 29; + return kDays[month]; +} + } // namespace SkillUsageStore::SkillUsageStore(std::string state_path) @@ -53,107 +255,201 @@ SkillUsageStore::SkillUsageStore(std::string state_path) bool SkillUsageStore::record(const std::string& skill_name, const std::string& now_iso) { std::lock_guard lock(mu_); - auto state = load_state_or_empty(state_path_); - state["version"] = kStateVersion; - auto& skills = state["skills"]; - if (!skills.is_object()) { - skills = nlohmann::json::object(); - } - if (!skills.contains(skill_name)) { - skills[skill_name] = {{"lastUsedAt", now_iso}, - {"useCount", 1}, - {"pinned", false}}; - } else { - auto& entry = skills[skill_name]; + try { + SkillUsageWriteLock file_lock(state_path_); + if (!file_lock.acquired()) { + LOG_WARN("[skill_usage] " + file_lock.error()); + return false; + } + + // The state must be re-read after acquiring the process-wide lock; + // another TUI/daemon instance may have committed while we waited. + auto state = load_state_or_empty(state_path_); + state["version"] = kStateVersion; + auto& skills = ensure_skills_object(state); + auto& entry = ensure_skill_entry(skills, skill_name); + const std::uint64_t current = uint64_member_or(entry, "useCount"); entry["lastUsedAt"] = now_iso; - entry["useCount"] = entry.value("useCount", 0u) + 1u; + entry["useCount"] = + current == std::numeric_limits::max() + ? current + : current + 1; + if (!entry.contains("pinned") || !entry["pinned"].is_boolean()) { + entry["pinned"] = false; + } + return atomic_write_file(state_path_, state.dump(2)); + } catch (const std::exception& e) { + LOG_WARN("[skill_usage] record failed: " + std::string(e.what())); + return false; + } catch (...) { + LOG_WARN("[skill_usage] record failed with unknown error"); + return false; } - return atomic_write_file(state_path_, state.dump(2)); } bool SkillUsageStore::is_dormant(const std::string& skill_name, std::int64_t now_epoch_ms, std::int64_t idle_days_ms) const { - if (idle_days_ms == 0) return false; + if (idle_days_ms <= 0) return false; std::lock_guard lock(mu_); - auto state = load_state_or_empty(state_path_); - auto& skills = state["skills"]; - if (!skills.is_object() || !skills.contains(skill_name)) return false; - auto& entry = skills[skill_name]; - if (!entry.is_object() || entry.value("pinned", false)) return false; - const std::string last_used = entry.value("lastUsedAt", ""); - if (last_used.empty()) return false; - const std::int64_t last_ms = parse_iso8601_to_epoch_ms(last_used); - if (last_ms == 0) return false; - return (now_epoch_ms - last_ms) > idle_days_ms; + try { + const auto state = load_state_or_empty(state_path_); + const auto skills_it = state.find("skills"); + if (skills_it == state.end() || !skills_it->is_object()) return false; + const auto entry_it = skills_it->find(skill_name); + if (entry_it == skills_it->end() || !entry_it->is_object() || + bool_member_or(*entry_it, "pinned")) { + return false; + } + const std::string last_used = + string_member_or(*entry_it, "lastUsedAt"); + const std::int64_t last_ms = parse_iso8601_to_epoch_ms(last_used); + return last_ms > 0 && now_epoch_ms > last_ms && + (now_epoch_ms - last_ms) > idle_days_ms; + } catch (const std::exception& e) { + LOG_WARN("[skill_usage] dormancy read failed: " + + std::string(e.what())); + return false; + } catch (...) { + LOG_WARN("[skill_usage] dormancy read failed with unknown error"); + return false; + } } bool SkillUsageStore::set_pinned(const std::string& skill_name, bool pinned) { std::lock_guard lock(mu_); - auto state = load_state_or_empty(state_path_); - state["version"] = kStateVersion; - auto& skills = state["skills"]; - if (!skills.is_object()) { - skills = nlohmann::json::object(); - } - if (!skills.contains(skill_name)) { - skills[skill_name] = {{"lastUsedAt", ""}, - {"useCount", 0}, - {"pinned", pinned}}; - } else { - skills[skill_name]["pinned"] = pinned; + try { + SkillUsageWriteLock file_lock(state_path_); + if (!file_lock.acquired()) { + LOG_WARN("[skill_usage] " + file_lock.error()); + return false; + } + + auto state = load_state_or_empty(state_path_); + state["version"] = kStateVersion; + auto& skills = ensure_skills_object(state); + auto& entry = ensure_skill_entry(skills, skill_name); + if (!entry.contains("lastUsedAt") || + !entry["lastUsedAt"].is_string()) { + entry["lastUsedAt"] = ""; + } + // Canonicalize malformed and negative counters instead of merely + // presenting them as zero through get_summary(). + entry["useCount"] = uint64_member_or(entry, "useCount"); + entry["pinned"] = pinned; + return atomic_write_file(state_path_, state.dump(2)); + } catch (const std::exception& e) { + LOG_WARN("[skill_usage] set_pinned failed: " + + std::string(e.what())); + return false; + } catch (...) { + LOG_WARN("[skill_usage] set_pinned failed with unknown error"); + return false; } - return atomic_write_file(state_path_, state.dump(2)); } std::vector SkillUsageStore::get_summary( std::int64_t now_epoch_ms, std::int64_t idle_days_ms) const { std::lock_guard lock(mu_); - auto state = load_state_or_empty(state_path_); std::vector out; - auto& skills = state["skills"]; - if (!skills.is_object()) return out; - for (auto& [name, entry] : skills.items()) { - if (!entry.is_object()) continue; - SkillUsageSummary s; - s.name = name; - s.use_count = entry.value("useCount", 0u); - s.last_used_at = entry.value("lastUsedAt", ""); - s.pinned = entry.value("pinned", false); - if (idle_days_ms > 0 && !s.pinned && !s.last_used_at.empty()) { - const std::int64_t last_ms = - parse_iso8601_to_epoch_ms(s.last_used_at); - s.dormant = (last_ms > 0) && - (now_epoch_ms - last_ms) > idle_days_ms; - } - out.push_back(std::move(s)); + try { + const auto state = load_state_or_empty(state_path_); + const auto skills_it = state.find("skills"); + if (skills_it == state.end() || !skills_it->is_object()) return out; + for (const auto& [name, entry] : skills_it->items()) { + if (!entry.is_object()) continue; + SkillUsageSummary s; + s.name = name; + s.use_count = uint64_member_or(entry, "useCount"); + s.last_used_at = string_member_or(entry, "lastUsedAt"); + s.pinned = bool_member_or(entry, "pinned"); + if (idle_days_ms > 0 && !s.pinned && !s.last_used_at.empty()) { + const std::int64_t last_ms = + parse_iso8601_to_epoch_ms(s.last_used_at); + s.dormant = last_ms > 0 && now_epoch_ms > last_ms && + (now_epoch_ms - last_ms) > idle_days_ms; + } + out.push_back(std::move(s)); + } + } catch (const std::exception& e) { + LOG_WARN("[skill_usage] summary read failed: " + + std::string(e.what())); + } catch (...) { + LOG_WARN("[skill_usage] summary read failed with unknown error"); } return out; } void SkillUsageStore::reload() { + std::lock_guard lock(mu_); // The next access re-reads the file via load_state_or_empty; nothing // to invalidate here. } std::int64_t parse_iso8601_to_epoch_ms(const std::string& iso) { - if (iso.empty()) return 0; + // Exact shape: YYYY-MM-DDTHH:MM:SSZ or 1-3 decimal fraction digits. + if (iso.size() < 20 || iso.size() > 24 || + iso[4] != '-' || iso[7] != '-' || iso[10] != 'T' || + iso[13] != ':' || iso[16] != ':') { + return 0; + } + + int year = 0; + int month = 0; + int day = 0; + int hour = 0; + int minute = 0; + int second = 0; + if (!parse_decimal_field(iso, 0, 4, year) || + !parse_decimal_field(iso, 5, 2, month) || + !parse_decimal_field(iso, 8, 2, day) || + !parse_decimal_field(iso, 11, 2, hour) || + !parse_decimal_field(iso, 14, 2, minute) || + !parse_decimal_field(iso, 17, 2, second)) { + return 0; + } + if (year < 1970 || month < 1 || month > 12 || day < 1 || + day > days_in_month(year, month) || hour > 23 || minute > 59 || + second > 59) { + return 0; + } + + int milliseconds = 0; + if (iso[19] == 'Z') { + if (iso.size() != 20) return 0; + } else if (iso[19] == '.') { + if (iso.back() != 'Z') return 0; + const std::size_t fraction_digits = iso.size() - 21; + if (fraction_digits < 1 || fraction_digits > 3 || + !parse_decimal_field(iso, 20, fraction_digits, milliseconds)) { + return 0; + } + if (fraction_digits == 1) milliseconds *= 100; + if (fraction_digits == 2) milliseconds *= 10; + } else { + return 0; + } + std::tm tm = {}; - std::istringstream ss(iso); - ss >> std::get_time(&tm, "%Y-%m-%dT%H:%M:%S"); - if (ss.fail()) return 0; - int ms = 0; - if (ss.peek() == '.') { - ss.ignore(); - ss >> ms; - } - // Treat the parsed time as UTC (the 'Z' suffix / gmtime convention). - auto tp = std::chrono::system_clock::from_time_t( - std::mktime(&tm)) + - std::chrono::milliseconds(ms); - return std::chrono::duration_cast( - tp.time_since_epoch()) - .count(); + tm.tm_year = year - 1900; + tm.tm_mon = month - 1; + tm.tm_mday = day; + tm.tm_hour = hour; + tm.tm_min = minute; + tm.tm_sec = second; + tm.tm_isdst = 0; +#ifdef _WIN32 + const std::time_t seconds_since_epoch = ::_mkgmtime(&tm); +#else + const std::time_t seconds_since_epoch = ::timegm(&tm); +#endif + if (seconds_since_epoch < 0) return 0; + const auto seconds = static_cast(seconds_since_epoch); + if (seconds > + (std::numeric_limits::max() - milliseconds) / 1000) { + return 0; + } + return seconds * 1000 + milliseconds; } } // namespace acecode diff --git a/src/skills/skill_usage_store.hpp b/src/skills/skill_usage_store.hpp index 48e129ac4..7832c5c5e 100644 --- a/src/skills/skill_usage_store.hpp +++ b/src/skills/skill_usage_store.hpp @@ -22,7 +22,7 @@ struct SkillUsageSummary { }; // Manages the skill-usage state file (~/.acecode/.skill_usage_state.json). -// Thread-safe: every public method serializes on an internal mutex. +// Thread-safe; mutations also serialize across processes with a sidecar lock. class SkillUsageStore { public: // state_path: absolute path to the JSON state file. @@ -60,8 +60,8 @@ class SkillUsageStore { mutable std::mutex mu_; }; -// Parse an ISO8601 timestamp ("2026-08-01T10:00:00Z", optional .ms fraction) -// to epoch milliseconds. Returns 0 on parse failure or empty input. +// Strictly parse an ISO8601 UTC timestamp ("2026-08-01T10:00:00Z", optional +// 1-3 digit fractional second) to epoch milliseconds. Returns 0 on failure. std::int64_t parse_iso8601_to_epoch_ms(const std::string& iso); } // namespace acecode diff --git a/src/utils/open_url.cpp b/src/utils/open_url.cpp index ba37dcae6..ad3639f71 100644 --- a/src/utils/open_url.cpp +++ b/src/utils/open_url.cpp @@ -15,6 +15,10 @@ # include # include #else +# include +# include +# include +# include # include #endif @@ -22,6 +26,14 @@ namespace acecode { namespace { +#ifndef _WIN32 +void wait_for_child_process(pid_t pid) noexcept { + int status = 0; + while (::waitpid(pid, &status, 0) < 0 && errno == EINTR) { + } +} +#endif + // 默认平台打开器:POSIX 用 open(macOS) / xdg-open(Linux),fork+execlp 不经 // shell;Windows 用 ShellExecuteW 的 "open" verb(等价于 `start`,但无 cmd // 转义面)。URL 原样作参数,带空格/引号也安全。 @@ -49,6 +61,28 @@ bool platform_open_url(const std::string& url, std::string& error) { ::execlp(opener, opener, url.c_str(), static_cast(nullptr)); ::_exit(127); } + + // 不修改进程级 SIGCHLD 策略(ACECode 的其它子进程也依赖它);只为本次 + // opener 启动一个短生命周期 waiter,确保长时间运行的 TUI 不积累 zombie。 + std::thread reaper; + try { + reaper = std::thread([pid]() { wait_for_child_process(pid); }); + } catch (...) { + ::kill(pid, SIGTERM); + wait_for_child_process(pid); + error = "failed to start URL process reaper"; + return false; + } + try { + reaper.detach(); + } catch (...) { + ::kill(pid, SIGTERM); + if (reaper.joinable()) { + reaper.join(); + } + error = "failed to detach URL process reaper"; + return false; + } return true; #endif } diff --git a/src/utils/open_url.hpp b/src/utils/open_url.hpp index 46be522c1..af1898a92 100644 --- a/src/utils/open_url.hpp +++ b/src/utils/open_url.hpp @@ -6,7 +6,7 @@ // - 仅放行 http/https scheme(防 file:/data:/javascript: 等意外打开); // - 禁止控制字符(含 ESC 0x1B,防终端转义注入)与空白开头等异常; // - 默认 launcher 不经 shell(POSIX fork+execlp / Windows ShellExecuteW), -// URL 原样作参数传递,无 shell 注入面。 +// URL 原样作参数传递,无 shell 注入面;POSIX 子进程由专用 waiter 回收。 // // 可注入 launcher 便于单测(见 tests/utils/open_url_test.cpp)。 diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index cd378b491..09156bfba 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -1,4 +1,23 @@ find_package(GTest CONFIG REQUIRED) +find_package(Python3 COMPONENTS Interpreter QUIET) + +if(Python3_Interpreter_FOUND) + add_test( + NAME verify_package_python_unit + COMMAND ${Python3_EXECUTABLE} -B + ${CMAKE_CURRENT_SOURCE_DIR}/scripts/verify_package_unit_test.py + ) + add_test( + NAME dev_desktop_script_unit + COMMAND ${Python3_EXECUTABLE} -B + ${CMAKE_CURRENT_SOURCE_DIR}/scripts/dev_desktop_test.py + ) + set_tests_properties( + verify_package_python_unit + dev_desktop_script_unit + PROPERTIES LABELS "unit" + ) +endif() if(UNIX) add_test( diff --git a/tests/markdown/link_safety_test.cpp b/tests/markdown/link_safety_test.cpp index e6c7ab23b..15b28ff77 100644 --- a/tests/markdown/link_safety_test.cpp +++ b/tests/markdown/link_safety_test.cpp @@ -2,7 +2,7 @@ // // 只比域名(host),不比完整路径。规则(design.md 决策 4): // - href 不含 "://"(本地路径/裸域名)→ 放行(本地通道,防骗只针对网页链接) -// - href 远程但 host 解析失败 → 放行(无目标可比) +// - href 远程但 host 解析失败 → URL 形 label 降级,普通标签放行 // - label 无 URL 形状(无点号或含空白)→ 放行(标签文字) // - label 呈 URL 形状 → host 一致放行(忽略大小写),不一致降级, // 畸形/非 ASCII 按不匹配(降级) @@ -31,6 +31,28 @@ TEST(LinkSafety, ExtractHostBareDomainWithPath) { TEST(LinkSafety, ExtractHostUserinfoAndPort) { EXPECT_EQ(extract_url_host("user:pass@EXAMPLE.com:8080/x"), std::optional("example.com")); + EXPECT_EQ(extract_url_host("https://user:pass@EXAMPLE.com:8080/x"), + std::optional("example.com")); + EXPECT_EQ(extract_url_host("https://example.com:65535/x"), + std::optional("example.com")); +} + +TEST(LinkSafety, ExtractHostStopsAtAuthorityBoundaryBeforeAtSign) { + EXPECT_EQ(extract_url_host("https://evil.example/@trusted.example"), + std::optional("evil.example")); + EXPECT_EQ(extract_url_host("https://evil.example/?next=@trusted.example"), + std::optional("evil.example")); + EXPECT_EQ(extract_url_host("https://evil.example#@trusted.example"), + std::optional("evil.example")); + EXPECT_EQ(extract_url_host("https://evil.example\\@trusted.example"), + std::optional("evil.example")); +} + +TEST(LinkSafety, ExtractBracketedIpv6Host) { + EXPECT_EQ(extract_url_host("https://[2001:DB8::1]:8443/path"), + std::optional("2001:db8::1")); + EXPECT_EQ(extract_url_host("https://[::1]/"), + std::optional("::1")); } TEST(LinkSafety, ExtractHostTrailingDotStripped) { @@ -56,6 +78,13 @@ TEST(LinkSafety, ExtractHostOtherSchemeFails) { TEST(LinkSafety, ExtractHostMalformedFails) { EXPECT_EQ(extract_url_host(""), std::nullopt); EXPECT_EQ(extract_url_host("https://"), std::nullopt); + EXPECT_EQ(extract_url_host("https:///path"), std::nullopt); + EXPECT_EQ(extract_url_host("https://user@/path"), std::nullopt); + EXPECT_EQ(extract_url_host("https://[2001:db8::1/path"), std::nullopt); + EXPECT_EQ(extract_url_host("https://example.com:not-a-port/path"), + std::nullopt); + EXPECT_EQ(extract_url_host("https://example.com:65536/path"), + std::nullopt); EXPECT_EQ(extract_url_host("https://例子.中国"), std::nullopt); } @@ -68,6 +97,12 @@ TEST(LinkSafety, SpoofedHostDowngraded) { // 子域名陷阱:label host 是 google.com.evil.example.com,不是 google.com EXPECT_FALSE( is_safe_link_label("google.com", "https://google.com.evil.example.com")); + EXPECT_FALSE(is_safe_link_label( + "trusted.example", "https://evil.example/@trusted.example")); + EXPECT_FALSE(is_safe_link_label( + "trusted.example", "https://evil.example/?next=@trusted.example")); + EXPECT_FALSE(is_safe_link_label( + "trusted.example", "https://evil.example\\@trusted.example")); } // ---------- 标签文字(无 URL 形状)→ 放行 ---------- @@ -103,6 +138,9 @@ TEST(LinkSafety, MalformedUrlDowngraded) { // label host 是仿冒子域,与目标不符 EXPECT_FALSE(is_safe_link_label("https://example..com", "https://example.com")); + // URL-shaped labels fail closed when the remote href has no valid authority. + EXPECT_FALSE(is_safe_link_label("example.com", "https:///example.com")); + EXPECT_FALSE(is_safe_link_label("example.com", "https://user@/path")); } // ---------- 本地链接通道(href 无 "://")→ 放行 ---------- diff --git a/tests/scripts/dev_desktop_test.py b/tests/scripts/dev_desktop_test.py new file mode 100644 index 000000000..10606922f --- /dev/null +++ b/tests/scripts/dev_desktop_test.py @@ -0,0 +1,98 @@ +#!/usr/bin/env python3 +"""Regression tests for scripts/dev_desktop.py.""" + +from __future__ import annotations + +import ast +import importlib.util +import os +import sys +import tempfile +import unittest +from pathlib import Path + +sys.dont_write_bytecode = True + +REPO_ROOT = Path(__file__).resolve().parents[2] +SCRIPT_PATH = REPO_ROOT / "scripts" / "dev_desktop.py" + + +def load_dev_desktop_module(): + spec = importlib.util.spec_from_file_location("dev_desktop_under_test", SCRIPT_PATH) + if spec is None or spec.loader is None: + raise RuntimeError(f"cannot load {SCRIPT_PATH}") + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +dev_desktop = load_dev_desktop_module() + + +class DevDesktopTest(unittest.TestCase): + def test_source_is_parseable_with_python_38_grammar(self) -> None: + source = SCRIPT_PATH.read_text(encoding="utf-8") + ast.parse(source, filename=str(SCRIPT_PATH), feature_version=(3, 8)) + self.assertIn("from __future__ import annotations", source) + + def test_web_build_freshness_includes_manifest_public_and_source(self) -> None: + with tempfile.TemporaryDirectory() as root_text: + web = Path(root_text) + (web / "dist").mkdir() + (web / "src").mkdir() + (web / "public").mkdir() + index = web / "dist" / "index.html" + index.write_text("dist", encoding="utf-8") + old = index.stat().st_mtime - 10 + os.utime(index, (old, old)) + + for relative in ("src/app.js", "public/icon.svg", "package.json", + "pnpm-lock.yaml", "vite.config.js"): + path = web / relative + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(relative, encoding="utf-8") + self.assertTrue(dev_desktop.web_build_is_stale(web, index), relative) + older = old - 10 + os.utime(path, (older, older)) + + self.assertFalse(dev_desktop.web_build_is_stale(web, index)) + + def test_finds_direct_and_nested_multiconfig_desktop_builds(self) -> None: + with tempfile.TemporaryDirectory() as root_text: + build = Path(root_text) / "build" + build.mkdir() + direct = build / "acecode-desktop.exe" + nested = build / "vs-audit" / "Release" / "acecode-desktop.exe" + direct.write_bytes(b"") + nested.parent.mkdir(parents=True) + nested.write_bytes(b"") + + builds = {path.resolve() for path in dev_desktop.find_desktop_builds(build)} + self.assertIn(direct.resolve(), builds) + self.assertIn(nested.resolve(), builds) + + def test_external_build_path_falls_back_to_absolute_display(self) -> None: + with tempfile.TemporaryDirectory() as root_text: + root = Path(root_text) + repo = root / "repo" + external = root / "external" / "Release" / "acecode-desktop.exe" + repo.mkdir() + external.parent.mkdir(parents=True) + external.write_bytes(b"") + + self.assertEqual( + dev_desktop.display_path(external, repo), + str(external.resolve()), + ) + + def test_ansi_color_code_has_single_terminator(self) -> None: + original = dev_desktop._COLOR + try: + dev_desktop._COLOR = True + self.assertEqual(dev_desktop._c("x", "36"), "\x1b[36mx\x1b[0m") + finally: + dev_desktop._COLOR = original + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/scripts/verify_package_test.sh b/tests/scripts/verify_package_test.sh index b9d876cc1..137014837 100644 --- a/tests/scripts/verify_package_test.sh +++ b/tests/scripts/verify_package_test.sh @@ -70,11 +70,22 @@ printf 'seed skill\n' >"$fixture/assets/seed/skills/demo/SKILL.md" printf '{"bundle":"test"}\n' >"$fixture/assets/seed/MANIFEST.json" printf '1\n' >"$fixture/assets/seed/seed.version" printf '\n' >"$fixture/web/dist/index.html" + printf 'readme\n' >"$fixture/README.md" printf 'readme cn\n' >"$fixture/README_CN.md" printf 'cmake cache marker\n' >"$fixture/build/CMakeCache.txt" cp "$repo_root/scripts/verify_seed_bundle.py" "$fixture/scripts/verify_seed_bundle.py" +# Negative: a staging path at the repo root must be rejected before the +# recursive cleanup, and the fixture must remain intact. +printf 'keep me\n' >"$fixture/staging-safety-sentinel" +expect_status 1 "unsafe staging repo root" "$python_bin" "$verify_script" \ + --skip-build --platform linux --target tui \ + --repo "$fixture" --build-dir "$fixture/build" \ + --staging-dir "$fixture" +expect_output "unsafe staging" "unsafe staging detail" +[[ -f "$fixture/staging-safety-sentinel" ]] + # Negative: --skip-build without a configured build dir fails the preflight. rm -rf "$temporary_root/nobuild" mkdir -p "$temporary_root/nobuild" @@ -166,18 +177,18 @@ EOF expect_status 0 "tui flow" "$python_bin" "$verify_script" \ --skip-build --platform linux --target tui \ --repo "$fixture" --build-dir "$fixture/build" \ - --staging-dir "$fixture/staging" + --staging-dir "$fixture/build/staging" expect_output "verify-package: PASS" "tui flow summary" - [[ -f "$fixture/staging/acecode" ]] - [[ -f "$fixture/staging/share/acecode/models_dev/api.json" ]] - [[ -f "$fixture/staging/share/acecode/seed/skills/demo/SKILL.md" ]] + [[ -f "$fixture/build/staging/acecode" ]] + [[ -f "$fixture/build/staging/share/acecode/models_dev/api.json" ]] + [[ -f "$fixture/build/staging/share/acecode/seed/skills/demo/SKILL.md" ]] # Case: full linux flow including desktop launch and daemon adjacency. write_desktop_stub expect_status 0 "linux full flow" "$python_bin" "$verify_script" \ --skip-build --platform linux --target all \ --repo "$fixture" --build-dir "$fixture/build" \ - --staging-dir "$fixture/staging" --launch-timeout 2 + --staging-dir "$fixture/build/staging" --launch-timeout 2 expect_output "\[PASS\] desktop daemon adjacency" "daemon adjacency" expect_output "\[PASS\] desktop launch" "desktop launch" @@ -205,7 +216,7 @@ EOF expect_status 0 "darwin bundle flow" "$python_bin" "$verify_script" \ --skip-build --platform darwin --target desktop \ --repo "$fixture" --build-dir "$fixture/build" \ - --staging-dir "$fixture/staging" --launch-timeout 2 + --staging-dir "$fixture/build/staging" --launch-timeout 2 expect_output "\[PASS\] app bundle acecode-daemon" "bundle daemon" expect_output "\[PASS\] models_dev registry (app bundle)" "bundle models_dev" @@ -214,7 +225,7 @@ EOF expect_status 1 "models_dev file count" "$python_bin" "$verify_script" \ --skip-build --platform linux --target tui \ --repo "$fixture" --build-dir "$fixture/build" \ - --staging-dir "$fixture/staging" + --staging-dir "$fixture/build/staging" expect_output "expected exactly" "models_dev count detail" rm "$fixture/assets/models_dev/extra.json" @@ -223,7 +234,7 @@ EOF expect_status 1 "registry validation failure" "$python_bin" "$verify_script" \ --skip-build --platform linux --target tui \ --repo "$fixture" --build-dir "$fixture/build" \ - --staging-dir "$fixture/staging" + --staging-dir "$fixture/build/staging" expect_output "tui models registry resolution" "registry failure item" # Negative: desktop exiting immediately fails the launch probe. @@ -236,7 +247,7 @@ EOF expect_status 1 "desktop immediate exit" "$python_bin" "$verify_script" \ --skip-build --platform linux --target desktop \ --repo "$fixture" --build-dir "$fixture/build" \ - --staging-dir "$fixture/staging" + --staging-dir "$fixture/build/staging" expect_output "exited immediately" "desktop exit detail" else echo "verify_package_test: flow cases skipped on non-POSIX host" diff --git a/tests/scripts/verify_package_unit_test.py b/tests/scripts/verify_package_unit_test.py new file mode 100644 index 000000000..a5eae3d12 --- /dev/null +++ b/tests/scripts/verify_package_unit_test.py @@ -0,0 +1,155 @@ +#!/usr/bin/env python3 +"""Focused cross-platform tests for the local package verifier.""" + +from __future__ import annotations + +import hashlib +import importlib.util +import subprocess +import sys +import tempfile +import unittest +from pathlib import Path +from unittest import mock + +sys.dont_write_bytecode = True + +REPO_ROOT = Path(__file__).resolve().parents[2] +SCRIPT_PATH = ( + REPO_ROOT / ".acecode" / "skills" / "verify-package" / + "scripts" / "verify_package.py" +) + + +def load_verify_package_module(): + spec = importlib.util.spec_from_file_location("verify_package_under_test", SCRIPT_PATH) + if spec is None or spec.loader is None: + raise RuntimeError(f"cannot load {SCRIPT_PATH}") + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +verify_package = load_verify_package_module() + + +class FakeProcess: + def __init__(self, first_wait_times_out: bool, kill_raises: bool = False) -> None: + self.first_wait_times_out = first_wait_times_out + self.kill_raises = kill_raises + self.terminate_calls = 0 + self.kill_calls = 0 + self.wait_calls = 0 + + def terminate(self) -> None: + self.terminate_calls += 1 + + def kill(self) -> None: + self.kill_calls += 1 + if self.kill_raises: + raise ProcessLookupError("fake process already exited") + + def wait(self, timeout: int): + self.wait_calls += 1 + if self.first_wait_times_out and self.wait_calls == 1: + raise subprocess.TimeoutExpired("fake-desktop", timeout) + return -9 if self.kill_calls else 0 + + +class VerifyPackageUnitTest(unittest.TestCase): + def test_windows_does_not_force_ninja_and_maps_cmake_targets(self) -> None: + with tempfile.TemporaryDirectory() as root_text: + root = Path(root_text) + repo = root / "repo" + build = root / "build" + repo.mkdir() + commands = [] + + def capture(_report, _name, command, **_kwargs): + commands.append(command) + return True + + with mock.patch.object(verify_package, "run_tool", side_effect=capture), \ + mock.patch.object(verify_package.shutil, "which", return_value="ninja"): + result = verify_package.configure_and_build( + verify_package.Report(), repo, build, "cmake", + ["tui", "desktop"], "windows" + ) + + self.assertTrue(result) + self.assertNotIn("-G", commands[0]) + self.assertEqual(commands[1][-1], "acecode") + self.assertEqual(commands[2][-1], "acecode-desktop") + + def test_non_windows_prefers_ninja(self) -> None: + with tempfile.TemporaryDirectory() as root_text: + root = Path(root_text) + repo = root / "repo" + build = root / "build" + repo.mkdir() + commands = [] + + def capture(_report, _name, command, **_kwargs): + commands.append(command) + return True + + with mock.patch.object(verify_package, "run_tool", side_effect=capture), \ + mock.patch.object(verify_package.shutil, "which", return_value="ninja"): + verify_package.configure_and_build( + verify_package.Report(), repo, build, "cmake", ["tui"], "linux" + ) + + self.assertEqual(commands[0][4:6], ["-G", "Ninja"]) + + def test_staging_path_guard_rejects_protected_paths(self) -> None: + with tempfile.TemporaryDirectory() as root_text: + root = Path(root_text).resolve() + repo = root / "repo" + build = repo / "build" + cwd = root / "cwd" + home = root / "home" + build.mkdir(parents=True) + cwd.mkdir() + home.mkdir() + + self.assertIsNotNone(verify_package.validate_staging_path( + repo, build, repo, cwd=cwd, home=home)) + self.assertIsNotNone(verify_package.validate_staging_path( + repo, build, build, cwd=cwd, home=home)) + self.assertIsNotNone(verify_package.validate_staging_path( + repo, build, repo / "unrelated", cwd=cwd, home=home)) + self.assertIsNotNone(verify_package.validate_staging_path( + repo, build, cwd, cwd=cwd, home=home)) + self.assertIsNone(verify_package.validate_staging_path( + repo, build, build / "verify-package-staging", cwd=cwd, home=home)) + self.assertIsNone(verify_package.validate_staging_path( + repo, build, root / "outside-staging", cwd=cwd, home=home)) + + def test_force_kill_path_is_waited_and_reaped(self) -> None: + process = FakeProcess(first_wait_times_out=True) + self.assertTrue(verify_package.terminate_and_reap(process, timeout=1)) + self.assertEqual(process.terminate_calls, 1) + self.assertEqual(process.kill_calls, 1) + self.assertEqual(process.wait_calls, 2) + + def test_kill_race_still_waits_to_reap(self) -> None: + process = FakeProcess(first_wait_times_out=True, kill_raises=True) + self.assertTrue(verify_package.terminate_and_reap(process, timeout=1)) + self.assertEqual(process.kill_calls, 1) + self.assertEqual(process.wait_calls, 2) + + def test_skill_script_copies_are_identical(self) -> None: + copies = [ + REPO_ROOT / root / "skills" / "verify-package" / + "scripts" / "verify_package.py" + for root in (".acecode", ".agents", ".claude", ".codex") + ] + hashes = { + hashlib.sha256(path.read_bytes()).hexdigest() + for path in copies + } + self.assertEqual(len(hashes), 1, copies) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/skills/skill_usage_store_test.cpp b/tests/skills/skill_usage_store_test.cpp index af5060635..1efc9ea50 100644 --- a/tests/skills/skill_usage_store_test.cpp +++ b/tests/skills/skill_usage_store_test.cpp @@ -1,9 +1,15 @@ #include "skills/skill_usage_store.hpp" #include +#include +#include +#include #include #include +#include +#include +#include namespace fs = std::filesystem; @@ -16,19 +22,47 @@ fs::path temp_state_file(const std::string& name) { ("acecode_skill_usage_" + name + ".json"); std::error_code ec; fs::remove(tmp, ec); + fs::remove(tmp.string() + ".tmp", ec); + fs::remove(tmp.string() + ".lock", ec); return tmp; } +void cleanup_state_file(const fs::path& path) { + std::error_code ec; + fs::remove(path, ec); + fs::remove(path.string() + ".tmp", ec); + fs::remove(path.string() + ".lock", ec); +} + +void write_state(const fs::path& path, const nlohmann::json& state) { + std::ofstream ofs(path, std::ios::binary | std::ios::trunc); + ASSERT_TRUE(ofs.is_open()); + ofs << state.dump(2); + ASSERT_TRUE(ofs.good()); +} + } // namespace TEST(SkillUsageStoreTest, ParseIso8601) { - const auto ms = acecode::parse_iso8601_to_epoch_ms("2026-08-01T10:00:00Z"); - EXPECT_GT(ms, 0); - EXPECT_EQ(ms, + constexpr std::int64_t kExpected = 1785578400000LL; + EXPECT_EQ(kExpected, acecode::parse_iso8601_to_epoch_ms("2026-08-01T10:00:00Z")); + EXPECT_EQ(kExpected + 100, + acecode::parse_iso8601_to_epoch_ms("2026-08-01T10:00:00.1Z")); + EXPECT_EQ(kExpected + 120, + acecode::parse_iso8601_to_epoch_ms("2026-08-01T10:00:00.12Z")); + EXPECT_EQ(kExpected + 123, + acecode::parse_iso8601_to_epoch_ms("2026-08-01T10:00:00.123Z")); // 无效输入返回 0 EXPECT_EQ(0, acecode::parse_iso8601_to_epoch_ms("not-a-date")); EXPECT_EQ(0, acecode::parse_iso8601_to_epoch_ms("")); + EXPECT_EQ(0, acecode::parse_iso8601_to_epoch_ms("2026-02-29T10:00:00Z")); + EXPECT_EQ(0, acecode::parse_iso8601_to_epoch_ms("2026-13-01T10:00:00Z")); + EXPECT_EQ(0, acecode::parse_iso8601_to_epoch_ms("2026-08-01T24:00:00Z")); + EXPECT_EQ(0, acecode::parse_iso8601_to_epoch_ms("2026-08-01T10:00:60Z")); + EXPECT_EQ(0, acecode::parse_iso8601_to_epoch_ms("2026-08-01T10:00:00")); + EXPECT_EQ(0, acecode::parse_iso8601_to_epoch_ms("2026-08-01T10:00:00.1234Z")); + EXPECT_EQ(0, acecode::parse_iso8601_to_epoch_ms("2026-08-01T10:00:00Zjunk")); } TEST(SkillUsageStoreTest, RecordCreatesEntry) { @@ -43,7 +77,7 @@ TEST(SkillUsageStoreTest, RecordCreatesEntry) { EXPECT_EQ(s[0].name, "pdf"); EXPECT_EQ(s[0].use_count, 1u); EXPECT_FALSE(s[0].dormant); - fs::remove(tmp); + cleanup_state_file(tmp); } TEST(SkillUsageStoreTest, DormantAfterThreshold) { @@ -58,7 +92,7 @@ TEST(SkillUsageStoreTest, DormantAfterThreshold) { auto s = store.get_summary(now_ms, 30 * kDayMs); ASSERT_EQ(s.size(), 1u); EXPECT_TRUE(s[0].dormant); - fs::remove(tmp); + cleanup_state_file(tmp); } TEST(SkillUsageStoreTest, PinnedSkillNeverDormant) { @@ -73,7 +107,7 @@ TEST(SkillUsageStoreTest, PinnedSkillNeverDormant) { ASSERT_EQ(s.size(), 1u); EXPECT_FALSE(s[0].dormant); EXPECT_TRUE(s[0].pinned); - fs::remove(tmp); + cleanup_state_file(tmp); } TEST(SkillUsageStoreTest, IdleDaysZeroDisablesFeature) { @@ -85,7 +119,7 @@ TEST(SkillUsageStoreTest, IdleDaysZeroDisablesFeature) { auto s = store.get_summary(1785578400000LL, 0); ASSERT_EQ(s.size(), 1u); EXPECT_FALSE(s[0].dormant); - fs::remove(tmp); + cleanup_state_file(tmp); } TEST(SkillUsageStoreTest, IncrementUseCount) { @@ -98,7 +132,7 @@ TEST(SkillUsageStoreTest, IncrementUseCount) { auto s = store.get_summary(1785578400000LL, 30 * kDayMs); ASSERT_EQ(s.size(), 1u); EXPECT_EQ(s[0].use_count, 3u); - fs::remove(tmp); + cleanup_state_file(tmp); } TEST(SkillUsageStoreTest, CorruptStateDegradesGracefully) { @@ -112,5 +146,123 @@ TEST(SkillUsageStoreTest, CorruptStateDegradesGracefully) { // 损坏文件:record 仍成功(内部重置),不抛异常 EXPECT_TRUE(store.record("pdf", "2026-08-01T10:00:00Z")); EXPECT_FALSE(store.is_dormant("pdf", 1785578400000LL, 30 * kDayMs)); - fs::remove(tmp); + cleanup_state_file(tmp); +} + +TEST(SkillUsageStoreTest, DamagedSchemaUsesSafeDefaultsAndNormalizesOnWrite) { + const fs::path tmp = temp_state_file("damaged_schema"); + write_state(tmp, { + {"version", 1}, + {"skills", { + {"damaged", { + {"lastUsedAt", 42}, + {"useCount", "many"}, + {"pinned", "yes"}, + }}, + {"scalar", "broken"}, + }}, + }); + + acecode::SkillUsageStore store(tmp.string()); + EXPECT_FALSE(store.is_dormant("damaged", 1785578400000LL, 30 * kDayMs)); + auto summary = store.get_summary(1785578400000LL, 30 * kDayMs); + ASSERT_EQ(summary.size(), 1u); + EXPECT_EQ(summary[0].name, "damaged"); + EXPECT_EQ(summary[0].use_count, 0u); + EXPECT_TRUE(summary[0].last_used_at.empty()); + EXPECT_FALSE(summary[0].pinned); + + EXPECT_TRUE(store.record("scalar", "2026-08-01T10:00:00Z")); + EXPECT_TRUE(store.set_pinned("damaged", true)); + summary = store.get_summary(1785578400000LL, 30 * kDayMs); + ASSERT_EQ(summary.size(), 2u); + const auto scalar = std::find_if( + summary.begin(), summary.end(), + [](const auto& item) { return item.name == "scalar"; }); + ASSERT_NE(scalar, summary.end()); + EXPECT_EQ(scalar->use_count, 1u); + const auto damaged = std::find_if( + summary.begin(), summary.end(), + [](const auto& item) { return item.name == "damaged"; }); + ASSERT_NE(damaged, summary.end()); + EXPECT_TRUE(damaged->pinned); + cleanup_state_file(tmp); +} + +TEST(SkillUsageStoreTest, UseCountSaturatesAtUint64Max) { + const fs::path tmp = temp_state_file("saturates"); + write_state(tmp, { + {"version", 1}, + {"skills", { + {"pdf", { + {"lastUsedAt", "2026-08-01T10:00:00Z"}, + {"useCount", std::numeric_limits::max()}, + {"pinned", false}, + }}, + }}, + }); + + acecode::SkillUsageStore store(tmp.string()); + EXPECT_TRUE(store.record("pdf", "2026-08-02T10:00:00Z")); + const auto summary = store.get_summary(1785664800000LL, 30 * kDayMs); + ASSERT_EQ(summary.size(), 1u); + EXPECT_EQ(summary[0].use_count, + std::numeric_limits::max()); + cleanup_state_file(tmp); +} + +TEST(SkillUsageStoreTest, SetPinnedNormalizesNegativeUseCountOnDisk) { + const fs::path tmp = temp_state_file("negative_count"); + write_state(tmp, { + {"version", 1}, + {"skills", { + {"pdf", { + {"lastUsedAt", "2026-08-01T10:00:00Z"}, + {"useCount", -7}, + {"pinned", false}, + }}, + }}, + }); + + acecode::SkillUsageStore store(tmp.string()); + ASSERT_TRUE(store.set_pinned("pdf", true)); + + std::ifstream input(tmp, std::ios::binary); + ASSERT_TRUE(input.is_open()); + const auto persisted = nlohmann::json::parse(input); + const auto& count = persisted["skills"]["pdf"]["useCount"]; + EXPECT_TRUE(count.is_number_unsigned()); + EXPECT_EQ(count.get(), 0u); + cleanup_state_file(tmp); +} + +TEST(SkillUsageStoreTest, ConcurrentInstancesDoNotLoseUpdates) { + const fs::path tmp = temp_state_file("concurrent_instances"); + acecode::SkillUsageStore first(tmp.string()); + acecode::SkillUsageStore second(tmp.string()); + constexpr int kRecordsPerInstance = 25; + std::atomic start{false}; + std::atomic succeeded{true}; + auto record_many = [&](acecode::SkillUsageStore& store) { + while (!start.load(std::memory_order_acquire)) { + std::this_thread::yield(); + } + for (int i = 0; i < kRecordsPerInstance; ++i) { + if (!store.record("pdf", "2026-08-01T10:00:00Z")) { + succeeded.store(false, std::memory_order_release); + } + } + }; + + std::thread a(record_many, std::ref(first)); + std::thread b(record_many, std::ref(second)); + start.store(true, std::memory_order_release); + a.join(); + b.join(); + + EXPECT_TRUE(succeeded.load(std::memory_order_acquire)); + const auto summary = first.get_summary(1785578400000LL, 30 * kDayMs); + ASSERT_EQ(summary.size(), 1u); + EXPECT_EQ(summary[0].use_count, 2u * kRecordsPerInstance); + cleanup_state_file(tmp); } From 3d9a66f22602028b1f9984629c2653de3f945621 Mon Sep 17 00:00:00 2001 From: tmoonlight Date: Fri, 4 Sep 2026 12:13:46 +0800 Subject: [PATCH 5/5] feat: improve desktop context and transcript views --- .agents/skills/brainstorming/SKILL.md | 273 ------- .../brainstorming/scripts/frame-template.html | 213 ------ .../skills/brainstorming/scripts/helper.js | 167 ---- .../skills/brainstorming/scripts/server.cjs | 723 ------------------ .../brainstorming/scripts/start-server.sh | 209 ----- .../brainstorming/scripts/stop-server.sh | 120 --- .../spec-document-reviewer-prompt.md | 50 -- .../skills/brainstorming/visual-companion.md | 299 -------- CLAUDE.md | 2 +- README.md | 2 +- README_CN.md | 2 +- docs/user-manual.md | 4 +- src/desktop/context_items.cpp | 38 +- src/prompt/system_prompt.cpp | 9 +- tests/desktop/context_items_test.cpp | 16 +- tests/prompt/system_prompt_test.cpp | 8 +- web/src/components/ChatView.jsx | 629 ++------------- web/src/components/FilePreviewContent.jsx | 77 +- web/src/components/InputBar.jsx | 54 +- web/src/components/Message.jsx | 24 +- web/src/components/SubagentPanel.jsx | 221 ++++-- web/src/components/TranscriptItems.jsx | 472 ++++++++++++ .../chatFileDropFeedbackArchitecture.test.js | 27 + .../conversationLivenessArchitecture.test.js | 4 +- ...nversationTurnScrubberArchitecture.test.js | 3 +- web/src/lib/desktopContextPicker.test.js | 21 + web/src/lib/desktopFilesystemTransfer.js | 18 +- web/src/lib/desktopFilesystemTransfer.test.js | 22 +- .../flatToolTranscriptArchitecture.test.js | 57 +- .../lib/inlinePermissionArchitecture.test.js | 3 +- .../interactiveHomeLogoArchitecture.test.js | 2 +- web/src/lib/officePreviewInteractions.test.js | 18 +- web/src/lib/officePreviewZoom.js | 21 + web/src/lib/officePreviewZoom.test.js | 10 + web/src/lib/pathReference.js | 8 +- web/src/lib/pathReference.test.js | 13 +- web/src/lib/runTests.js | 3 + ...onCreationActionGuardsArchitecture.test.js | 35 +- web/src/lib/subagentTranscript.js | 13 + web/src/lib/subagentTranscript.test.js | 102 +++ web/src/lib/transcriptItemPresentation.js | 104 +++ .../lib/transcriptItemPresentation.test.js | 106 +++ web/src/lib/transcriptProjection.js | 11 +- .../transcriptRendererArchitecture.test.js | 126 +++ web/src/lib/transcriptWindow.test.js | 2 +- .../turnFooterStabilityArchitecture.test.js | 41 +- web/src/styles/globals.css | 3 +- 47 files changed, 1517 insertions(+), 2868 deletions(-) delete mode 100644 .agents/skills/brainstorming/SKILL.md delete mode 100644 .agents/skills/brainstorming/scripts/frame-template.html delete mode 100644 .agents/skills/brainstorming/scripts/helper.js delete mode 100644 .agents/skills/brainstorming/scripts/server.cjs delete mode 100644 .agents/skills/brainstorming/scripts/start-server.sh delete mode 100644 .agents/skills/brainstorming/scripts/stop-server.sh delete mode 100644 .agents/skills/brainstorming/spec-document-reviewer-prompt.md delete mode 100644 .agents/skills/brainstorming/visual-companion.md create mode 100644 web/src/components/TranscriptItems.jsx create mode 100644 web/src/lib/subagentTranscript.js create mode 100644 web/src/lib/subagentTranscript.test.js create mode 100644 web/src/lib/transcriptItemPresentation.js create mode 100644 web/src/lib/transcriptItemPresentation.test.js create mode 100644 web/src/lib/transcriptRendererArchitecture.test.js diff --git a/.agents/skills/brainstorming/SKILL.md b/.agents/skills/brainstorming/SKILL.md deleted file mode 100644 index 8b75a1e72..000000000 --- a/.agents/skills/brainstorming/SKILL.md +++ /dev/null @@ -1,273 +0,0 @@ ---- -name: brainstorming -description: "You MUST use this before any creative work - creating features, building components, adding functionality, or modifying behavior. Explores user intent and product requirements (purpose, users, constraints, success criteria, scope), then produces a requirements spec handed off to the grilling skill to stress-test the technical solution." ---- - -# Brainstorming Ideas Into Requirements - -Help turn ideas into validated product requirements through natural -collaborative dialogue. - -This skill owns the **what** and the **why**: purpose, users, constraints, -success criteria, scope, and non-goals. The **how** — architecture, tech -stack, data model, interfaces — is deliberately deferred and handed to the -**grilling** skill, which interrogates the technical solution once the -requirements are settled. - -Pipeline: brainstorming (this skill) → requirements spec → grilling -(technical decisions) → implementation. - -Start by classifying how much process the request needs, then work -through your path: understand the context, refine the idea, present the -requirements, and get your human partner's approval. - - -Do NOT propose or commit to a technical solution, invoke any -implementation skill, write any code, or invoke grilling until you have -told your human partner WHAT you intend to build and WHY, and they have -approved it. This applies to EVERY task on EVERY path below — the -ceremony scales with the task; the approval gate never does. - - -## Three Paths - -Before your first question, classify the request and say the -classification out loud — "this looks bounded, so I'll confirm the -intent in chat rather than write a spec" — so your human partner can -override it: - -- **Spike** — a feasibility question ("can we...", "is it possible...", - "quick and dirty is fine") whose output is an answer, not code you - keep. Present the question and what you'll try in 2-3 sentences, get - a nod, then find out as cheaply as correctness allows. No - requirements doc, no spec file. Report findings as a recommendation; - anything you built stays labeled throwaway. -- **Bounded** — a well-scoped change to code that already exists in - this repo: a new flag, a small endpoint, a one-file fix. - Understanding the kind of app is not enough — bounded means the flow - you are changing is already here to read. If there is no existing - flow to change, the task is not bounded. Ask the clarifying - questions that matter, present a short summary IN CHAT of what will - change and why (user-visible behavior, not internals), and STOP. - Implementation starts only after your human partner says yes — a - bounded task's approval is as hard a gate as an architectural - one. No spec file, no implementation plan document. -- **Architectural** — new projects, new subsystems, changes that - restructure how components fit together or alter interfaces others - depend on. Follow the full process: questions, requirements spec, - user review, then hand off to the grilling skill for the technical - solution. - -When in doubt between two paths, take the heavier one. The ratchet is -one-way: hidden complexity discovered mid-task upgrades the path — -stop, say so, and step up. Nothing downgrades mid-task. - -## Anti-Pattern: "Too Simple To Need Approval" - -Every path ends with your human partner approving your intent before -implementation. A todo list, a single-function utility, a config -change — the requirements may be two sentences in chat, but you MUST -present them and get approval. "Simple" tasks are where unexamined -assumptions cause the most wasted work. What scales with simplicity is -the artifact, never the approval. - -## Red Flags - -| Thought | Reality | -|---------|---------| -| "This is too simple to need requirements" | Simple means short requirements, not none. Two sentences in chat, then approval. | -| "I'll call it bounded and skip the spec" | Reaching for a label to skip work IS the doubt — take the heavier path. | -| "It's bounded and the intent is obvious — I'll start while they read it" | The gate is the approval, not the summary's length. Present, then stop until you hear yes. | -| "I understand this kind of app, so it's bounded" | Bounded measures the repo, not your familiarity. A new project has no existing flow — it is architectural. | -| "The requirements are settled, so I'll just pick the architecture while writing the spec" | Technical choices belong in the spec's open-decisions list for grilling, not silently made here. | -| "The spike works, so I'll keep the code" | A spike's output is an answer. Keeping the code is a new request — classify it. | -| "It grew, but I'm almost done — no need to re-classify" | Hidden complexity upgrades the path mid-task. Stop and say so. | -| "They approved the spike, so the follow-up change is approved too" | Each task gets its own classification and its own approval. | - -## Checklist - -Classify first, announce the path, then create a task for each item on -your path and complete them in order. - -**Spike:** -1. **Explore project context** — enough to frame the probe -2. **Present question + probe plan** — 2-3 sentences -3. **Get approval** — a nod is enough -4. **Investigate** — as cheaply as correctness allows -5. **Report findings** — a recommendation; label anything built as throwaway - -**Bounded:** -1. **Explore project context** — check files, docs, recent commits -2. **Ask clarifying questions** — one at a time, the ones that matter -3. **Present short change summary in chat** — what changes, why, user-visible behavior -4. **Get approval** — STOP and wait for an explicit yes; presenting the summary and starting in the same breath is skipping the gate -5. **Implement** — proceed with the normal development workflow; no spec, no plan document - -**Architectural:** -1. **Explore project context** — check files, docs, recent commits -2. **Offer the visual companion just-in-time** — NOT upfront. The first time a question would genuinely be clearer shown than described, offer it then (its own message); on approval its browser tab opens for you. If no visual question ever arises, never offer it. See the Visual Companion section below. -3. **Ask clarifying questions** — one at a time: purpose, users, constraints, success criteria -4. **Scope the product** — features in/out, non-goals, YAGNI; present product-level options (scope and priority trade-offs) where meaningful, with your recommendation -5. **Present requirements** — in sections scaled to their complexity, get user approval after each section -6. **Write requirements spec** — save to `docs/specs/YYYY-MM-DD--requirements.md` and commit -7. **Spec self-review** — quick inline check for placeholders, contradictions, ambiguity, scope, and solution leakage (see below) -8. **User reviews written spec** — ask user to review the spec file before proceeding -9. **Hand off to grilling** — see the Handoff to Grilling section below - -## Process Flow - -```dot -digraph brainstorming { - "Classify: spike / bounded / architectural" [shape=diamond]; - "Present question + probe (2-3 sentences)" [shape=box]; - "Ask clarifying questions (bounded)" [shape=box]; - "Present short change summary in chat" [shape=box]; - "Human approves?" [shape=diamond]; - "Investigate; report recommendation" [shape=doublecircle]; - "Implement via normal workflow" [shape=doublecircle]; - "Explore project context" [shape=box]; - "Ask clarifying questions (one at a time)" [shape=box]; - "Scope the product; present options" [shape=box]; - "Present requirements sections" [shape=box]; - "User approves requirements?" [shape=diamond]; - "Write requirements spec" [shape=box]; - "Spec self-review\n(fix inline)" [shape=box]; - "User reviews spec?" [shape=box]; - "Hand off to grilling" [shape=doublecircle]; - "Hidden complexity? Upgrade path" [shape=box]; - - "Classify: spike / bounded / architectural" -> "Present question + probe (2-3 sentences)" [label="spike"]; - "Classify: spike / bounded / architectural" -> "Ask clarifying questions (bounded)" [label="bounded"]; - "Classify: spike / bounded / architectural" -> "Explore project context" [label="architectural"]; - "Present question + probe (2-3 sentences)" -> "Human approves?"; - "Ask clarifying questions (bounded)" -> "Present short change summary in chat"; - "Present short change summary in chat" -> "Human approves?"; - "Human approves?" -> "Investigate; report recommendation" [label="spike: yes"]; - "Human approves?" -> "Implement via normal workflow" [label="bounded: yes"]; - "Hidden complexity? Upgrade path" -> "Classify: spike / bounded / architectural"; - "Explore project context" -> "Ask clarifying questions (one at a time)"; - "Ask clarifying questions (one at a time)" -> "Scope the product; present options"; - "Scope the product; present options" -> "Present requirements sections"; - "Present requirements sections" -> "User approves requirements?"; - "User approves requirements?" -> "Present requirements sections" [label="no, revise"]; - "User approves requirements?" -> "Write requirements spec" [label="yes"]; - "Write requirements spec" -> "Spec self-review\n(fix inline)"; - "Spec self-review\n(fix inline)" -> "User reviews spec?"; - "User reviews spec?" -> "Write requirements spec" [label="changes requested"]; - "User reviews spec?" -> "Hand off to grilling" [label="approved"]; -} -``` - -**Terminal states are path-bound.** Architectural: the ONLY skill you -invoke after brainstorming is grilling — the requirements spec plus its -open-decisions list IS the handoff; never invoke an implementation -skill directly from brainstorming. Bounded: after approval, -implementation proceeds directly through the normal development -workflow; no spec document. Spike: the terminal state is a reported -recommendation. - -## The Process - -The subsections below serve the bounded and architectural paths (a -spike stops at "present the probe, get a nod"). - -**Understanding the idea:** - -- Check out the current project state first (files, docs, recent commits) -- Before asking detailed questions, assess scope: if the request describes multiple independent subsystems (e.g., "build a platform with chat, file storage, billing, and analytics"), flag this immediately. Don't spend questions refining details of a project that needs to be decomposed first. -- If the project is too large for a single spec, help the user decompose into sub-projects: what are the independent pieces, how do they relate, what order should they be built? Then run the first sub-project through the normal flow. Each sub-project gets its own requirements → grilling → implementation cycle. -- For appropriately-scoped projects, ask questions one at a time to refine the idea -- Prefer multiple choice questions when possible, but open-ended is fine too -- Only one question per message - if a topic needs more exploration, break it into multiple questions -- Focus on understanding: purpose, users, constraints, success criteria - -**Scoping the product:** - -- Once you understand the purpose, propose the product shape: what's in, what's explicitly out (non-goals) -- Where meaningful product choices exist — scope alternatives, feature priorities, rollout order — present 2-3 options conversationally with trade-offs and your recommendation. These are choices about WHAT to build, not HOW to build it -- YAGNI ruthlessly - remove unnecessary features from every scope -- Implementation choices (framework, architecture, data model, testing strategy) are NOT made in this skill. Record them as open technical decisions; grilling will interrogate them next - -**Presenting the requirements:** - -- Once you believe you understand what's being built and why, present the requirements -- Scale each section to its complexity: a few sentences if straightforward, up to 200-300 words if nuanced -- Cover: problem and purpose, users and context, functional requirements, constraints, success criteria, non-goals -- Deliberately do NOT cover architecture, components, data flow, error handling, or testing strategy — those are grilling's territory -- Ask after each section whether it looks right so far -- Be ready to go back and clarify if something doesn't make sense - -**Working in existing codebases:** - -- Explore the current structure enough to understand the domain and existing user-facing behavior before proposing requirements. Follow existing conventions when describing how the product fits what's already there. -- Do not bundle refactoring or code-level improvements into the requirements — if the codebase's structure constrains the product, note it as a constraint and let grilling decide how to handle it - -## After the Requirements (architectural path) - -**Documentation:** - -- Write the validated requirements to `docs/specs/YYYY-MM-DD--requirements.md` - - (User preferences for spec location override this default) -- End the spec with an **Open technical decisions** section (see Handoff to Grilling below) -- Commit the document to git - -**Spec Self-Review:** -After writing the spec document, look at it with fresh eyes: - -1. **Placeholder scan:** Any "TBD", "TODO", incomplete sections, or vague requirements? Fix them. -2. **Internal consistency:** Do any sections contradict each other? Do the requirements add up to a coherent product? -3. **Scope check:** Is this focused enough for a single grilling session and a single implementation effort, or does it need decomposition? -4. **Ambiguity check:** Could any requirement be interpreted two different ways? If so, pick one and make it explicit. -5. **Leakage check:** Does any requirement smuggle in a technical solution ("use Redis", "build it as a CLI")? Unless the user explicitly demanded it as a requirement, move it to the open technical decisions list. - -Fix any issues inline. No need to re-review — just fix and move on. - -**User Review Gate:** -After the spec review loop passes, ask the user to review the written spec before proceeding: - -> "Requirements spec written and committed to ``. Please review it and let me know if you want to make any changes before we grill the technical solution." - -Wait for the user's response. If they request changes, make them and re-run the spec review loop. Only proceed once the user approves. - -## Handoff to Grilling - -Brainstorming ends here; grilling begins. - -- The spec's final section, **Open technical decisions**, lists every - HOW decision the requirements leave open — architecture, tech stack, - data model, interfaces, error handling, testing strategy — each - phrased as a decision to be made, together with any constraints the - requirements impose on it. This section is grilling's starting map: - it seeds the design tree's first frontier. -- After the user approves the spec, present the handoff: - -> "Requirements are settled. Next step: stress-test the technical -> solution — invoke grilling on ``. The open decisions at -> the end of the spec are its starting questions." - - If your human partner tells you to proceed, invoke the grilling - skill with the spec as its input. - -- Cadence note: this skill asks one question at a time; grilling works - in rounds of numbered frontier questions with recommended answers. - The switch is intentional — slow dialogue for intent, structured - interrogation for design. - -## Visual Companion - -A browser-based companion for showing mockups, diagrams, and visual options during brainstorming. Available as a tool — not a mode. Accepting the companion means it's available for questions that benefit from visual treatment; it does NOT mean every question goes through the browser. - -**Offering the companion (just-in-time):** Do NOT offer it upfront. Wait until a question would genuinely be clearer shown than told — a real mockup / layout / diagram question, not merely a UI *topic*. The first time that happens, offer it then, as its own message: -> "This next part might be easier if I show you — I can put together mockups, diagrams, and comparisons in a browser tab as we go. It's still new and can be token-intensive. Want me to? I'll open it for you." - -**This offer MUST be its own message.** Only the offer — no clarifying question, summary, or other content. Wait for the user's response. If they accept, start the server with `--open` so their browser opens to the first screen automatically. If they decline, continue text-only and don't offer again unless they raise it. - -**Per-question decision:** Even after the user accepts, decide FOR EACH QUESTION whether to use the browser or the terminal. The test: **would the user understand this better by seeing it than reading it?** - -- **Use the browser** for content that IS visual — mockups, wireframes, layout comparisons, architecture diagrams, side-by-side visual designs -- **Use the terminal** for content that is text — requirements questions, conceptual choices, tradeoff lists, A/B/C/D text options, scope decisions - -A question about a UI topic is not automatically a visual question. "What does personality mean in this context?" is a conceptual question — use the terminal. "Which wizard layout works better?" is a visual question — use the browser. - -If they agree to the companion, read the detailed guide before proceeding: -`skills/brainstorming/visual-companion.md` diff --git a/.agents/skills/brainstorming/scripts/frame-template.html b/.agents/skills/brainstorming/scripts/frame-template.html deleted file mode 100644 index f540bb8a4..000000000 --- a/.agents/skills/brainstorming/scripts/frame-template.html +++ /dev/null @@ -1,213 +0,0 @@ - - - - - Superpowers Brainstorming - - - -
- -
Connecting…
-
- -
-
- -
-
- - - diff --git a/.agents/skills/brainstorming/scripts/helper.js b/.agents/skills/brainstorming/scripts/helper.js deleted file mode 100644 index e11d26489..000000000 --- a/.agents/skills/brainstorming/scripts/helper.js +++ /dev/null @@ -1,167 +0,0 @@ -(function() { - const MIN_RECONNECT_MS = 500; - const MAX_RECONNECT_MS = 30000; - const TOMBSTONE_AFTER_MS = 15000; // show the "paused" overlay after this long disconnected - - // Pure: next backoff delay (doubles, capped). Exported for unit tests. - function nextReconnectDelay(current, max) { - return Math.min(current * 2, max); - } - if (typeof module !== 'undefined' && module.exports) { - module.exports = { nextReconnectDelay, MIN_RECONNECT_MS, MAX_RECONNECT_MS, TOMBSTONE_AFTER_MS }; - } - - // Everything below is browser-only; bail out when loaded in Node (tests). - if (typeof window === 'undefined') return; - - let ws = null; - let eventQueue = []; - let reconnectDelay = MIN_RECONNECT_MS; - let reconnectTimer = null; - let disconnectedSince = null; - let everConnected = false; - let tombstoneShown = false; - - function sessionKey() { - try { - return window.sessionStorage && window.sessionStorage.getItem('brainstorm-session-key'); - } catch (e) {} - return null; - } - - function websocketUrl() { - const key = sessionKey(); - return 'ws://' + window.location.host + (key ? '/?key=' + encodeURIComponent(key) : ''); - } - - function reloadAfterRecovery() { - const key = sessionKey(); - if (key) { - window.location.replace('/?key=' + encodeURIComponent(key)); - } else { - window.location.reload(); - } - } - - // Reflect connection state in the frame's status pill (absent on full-doc screens). - function setStatus(state) { - const el = document.querySelector('.status'); - if (!el) return; - const map = { - connecting: ['Connecting…', 'var(--text-tertiary)'], - connected: ['Connected', 'var(--success)'], - reconnecting: ['Reconnecting…', 'var(--warning)'], - disconnected: ['Disconnected', 'var(--error)'] - }; - const [text, color] = map[state] || map.disconnected; - el.textContent = text; - el.style.setProperty('--status-color', color); - } - - // Self-styled so it works on framed and full-document screens alike. - function showTombstone() { - if (tombstoneShown) return; - tombstoneShown = true; - const el = document.createElement('div'); - el.id = 'bs-tombstone'; - el.style.cssText = 'position:fixed;inset:0;z-index:99999;display:flex;' + - 'align-items:center;justify-content:center;padding:2rem;text-align:center;' + - 'background:rgba(20,20,22,0.92);color:#f5f5f7;font-family:system-ui,sans-serif'; - el.innerHTML = '
' + - '

Companion paused

' + - '

This brainstorm companion has stopped. ' + - 'Ask your coding agent to bring it back — this page reconnects automatically.

'; - if (document.body) document.body.appendChild(el); - } - - function connect() { - if (reconnectTimer) { clearTimeout(reconnectTimer); reconnectTimer = null; } - setStatus(everConnected ? 'reconnecting' : 'connecting'); - ws = new WebSocket(websocketUrl()); - - ws.onopen = () => { - const recovered = tombstoneShown; - everConnected = true; - disconnectedSince = null; - reconnectDelay = MIN_RECONNECT_MS; - tombstoneShown = false; - setStatus('connected'); - eventQueue.forEach(e => ws.send(JSON.stringify(e))); - eventQueue = []; - // Recovered from a tombstoned outage (e.g. the server restarted on the same - // port) — reload through the keyed bootstrap when possible so the cookie is - // refreshed before the visible URL returns to bare /. - if (recovered) reloadAfterRecovery(); - }; - - ws.onmessage = (msg) => { - let data; - try { data = JSON.parse(msg.data); } catch (e) { return; } - if (data.type === 'reload') window.location.reload(); - }; - - ws.onclose = () => { - ws = null; - if (disconnectedSince === null) disconnectedSince = Date.now(); - if (Date.now() - disconnectedSince >= TOMBSTONE_AFTER_MS) { - setStatus('disconnected'); - showTombstone(); - } else { - setStatus('reconnecting'); - } - reconnectTimer = setTimeout(connect, reconnectDelay); - reconnectDelay = nextReconnectDelay(reconnectDelay, MAX_RECONNECT_MS); - }; - - // Let onclose own reconnection so we don't schedule it twice. - ws.onerror = () => { try { ws.close(); } catch (e) {} }; - } - - function sendEvent(event) { - event.timestamp = Date.now(); - if (ws && ws.readyState === WebSocket.OPEN) { - ws.send(JSON.stringify(event)); - } else { - eventQueue.push(event); - } - } - - // Capture clicks on choice elements - document.addEventListener('click', (e) => { - const target = e.target.closest('[data-choice]'); - if (!target) return; - - sendEvent({ - type: 'click', - text: target.textContent.trim(), - choice: target.dataset.choice, - id: target.id || null - }); - - }); - - // Frame UI: selection tracking - window.selectedChoice = null; - - window.toggleSelect = function(el) { - const container = el.closest('.options') || el.closest('.cards'); - const multi = container && container.dataset.multiselect !== undefined; - if (container && !multi) { - container.querySelectorAll('.option, .card').forEach(o => o.classList.remove('selected')); - } - if (multi) { - el.classList.toggle('selected'); - } else { - el.classList.add('selected'); - } - window.selectedChoice = el.dataset.choice; - }; - - // Expose API for explicit use - window.brainstorm = { - send: sendEvent, - choice: (value, metadata = {}) => sendEvent({ type: 'choice', value, ...metadata }) - }; - - connect(); -})(); diff --git a/.agents/skills/brainstorming/scripts/server.cjs b/.agents/skills/brainstorming/scripts/server.cjs deleted file mode 100644 index a828b35af..000000000 --- a/.agents/skills/brainstorming/scripts/server.cjs +++ /dev/null @@ -1,723 +0,0 @@ -const crypto = require('crypto'); -const http = require('http'); -const fs = require('fs'); -const path = require('path'); - -// ========== WebSocket Protocol (RFC 6455) ========== - -const OPCODES = { TEXT: 0x01, CLOSE: 0x08, PING: 0x09, PONG: 0x0A }; -const WS_MAGIC = '258EAFA5-E914-47DA-95CA-C5AB0DC85B11'; -const MAX_FRAME_PAYLOAD_BYTES = 10 * 1024 * 1024; - -function computeAcceptKey(clientKey) { - return crypto.createHash('sha1').update(clientKey + WS_MAGIC).digest('base64'); -} - -function encodeFrame(opcode, payload) { - const fin = 0x80; - const len = payload.length; - let header; - - if (len < 126) { - header = Buffer.alloc(2); - header[0] = fin | opcode; - header[1] = len; - } else if (len < 65536) { - header = Buffer.alloc(4); - header[0] = fin | opcode; - header[1] = 126; - header.writeUInt16BE(len, 2); - } else { - header = Buffer.alloc(10); - header[0] = fin | opcode; - header[1] = 127; - header.writeBigUInt64BE(BigInt(len), 2); - } - - return Buffer.concat([header, payload]); -} - -function decodeFrame(buffer) { - if (buffer.length < 2) return null; - - const secondByte = buffer[1]; - const opcode = buffer[0] & 0x0F; - const masked = (secondByte & 0x80) !== 0; - let payloadLen = secondByte & 0x7F; - let offset = 2; - - if (!masked) throw new Error('Client frames must be masked'); - - if (payloadLen === 126) { - if (buffer.length < 4) return null; - payloadLen = buffer.readUInt16BE(2); - offset = 4; - } else if (payloadLen === 127) { - if (buffer.length < 10) return null; - const extendedLen = buffer.readBigUInt64BE(2); - if (extendedLen > BigInt(MAX_FRAME_PAYLOAD_BYTES)) { - throw new Error('WebSocket frame payload exceeds maximum allowed size'); - } - payloadLen = Number(extendedLen); - offset = 10; - } - - if (payloadLen > MAX_FRAME_PAYLOAD_BYTES) { - throw new Error('WebSocket frame payload exceeds maximum allowed size'); - } - - const maskOffset = offset; - const dataOffset = offset + 4; - const totalLen = dataOffset + payloadLen; - if (buffer.length < totalLen) return null; - - const mask = buffer.slice(maskOffset, dataOffset); - const data = Buffer.alloc(payloadLen); - for (let i = 0; i < payloadLen; i++) { - data[i] = buffer[dataOffset + i] ^ mask[i % 4]; - } - - return { opcode, payload: data, bytesConsumed: totalLen }; -} - -// ========== Configuration ========== - -const PORT_FILE = process.env.BRAINSTORM_PORT_FILE || null; -const randomPort = () => 49152 + Math.floor(Math.random() * 16383); -// Prefer an explicit port, else the port this session last bound (so a restart -// reuses it and an already-open browser tab reconnects), else a random high port. -function preferredPort() { - if (process.env.BRAINSTORM_PORT) return Number(process.env.BRAINSTORM_PORT); - if (PORT_FILE) { - try { - const p = Number(fs.readFileSync(PORT_FILE, 'utf-8').trim()); - if (Number.isInteger(p) && p > 1023 && p < 65536) return p; - } catch (e) { /* no prior port recorded */ } - } - return randomPort(); -} -let PORT = preferredPort(); -const HOST = process.env.BRAINSTORM_HOST || '127.0.0.1'; -const URL_HOST = process.env.BRAINSTORM_URL_HOST || (HOST === '127.0.0.1' ? 'localhost' : HOST); -const SESSION_DIR = process.env.BRAINSTORM_DIR || '/tmp/brainstorm'; -const CONTENT_DIR = path.join(SESSION_DIR, 'content'); -const STATE_DIR = path.join(SESSION_DIR, 'state'); -const SUPERPOWERS_VERSION = readSuperpowersVersion(); -const SUPERPOWERS_BRAND_IMAGE_URL = 'https://primeradiant.com/brand/superpowers-visual-brainstorming-logo.png'; -const TELEMETRY_DISABLE_ENV_VARS = [ - 'SUPERPOWERS_DISABLE_TELEMETRY', - 'DISABLE_TELEMETRY', - 'CLAUDE_CODE_DISABLE_NONESSENTIAL_TRAFFIC' -]; -const SUPERPOWERS_TELEMETRY_DISABLED = TELEMETRY_DISABLE_ENV_VARS.some(name => isTruthyEnv(process.env[name])); -let ownerPid = process.env.BRAINSTORM_OWNER_PID ? Number(process.env.BRAINSTORM_OWNER_PID) : null; - -// Per-session secret key. The companion is reachable by any local browser tab -// and, when bound to a non-loopback host, by any host that can route to it. -// The key authenticates the real client uniformly across loopback, tunnel, and -// remote binds — and defeats DNS rebinding — where a Host/Origin allowlist -// cannot. It rides the served URL as ?key= and is mirrored into a cookie on -// first load so same-origin subresources and the WebSocket carry it for free. -// Persisted alongside the port (BRAINSTORM_TOKEN_FILE) so a restart keeps the -// same key and an already-open tab's cookie still validates. -const TOKEN_FILE = process.env.BRAINSTORM_TOKEN_FILE || null; -function generateToken() { - return crypto.randomBytes(32).toString('hex'); -} - -function chmodOwnerOnly(file) { - try { fs.chmodSync(file, 0o600); } catch (e) { /* best effort */ } -} - -function initialToken() { - if (process.env.BRAINSTORM_TOKEN) { - return { value: process.env.BRAINSTORM_TOKEN, source: 'env' }; - } - if (TOKEN_FILE) { - try { - const t = fs.readFileSync(TOKEN_FILE, 'utf-8').trim(); - if (/^[0-9a-f]{32,}$/i.test(t)) { - chmodOwnerOnly(TOKEN_FILE); - return { value: t, source: 'file' }; - } - } catch (e) { /* no prior token recorded */ } - } - return { value: generateToken(), source: 'generated' }; -} - -const tokenInfo = initialToken(); -let TOKEN = tokenInfo.value; -let tokenSource = tokenInfo.source; -let COOKIE_NAME = 'brainstorm-key-' + PORT; // refined to the actual bound port in onListen - -const MIME_TYPES = { - '.html': 'text/html', '.css': 'text/css', '.js': 'application/javascript', - '.json': 'application/json', '.png': 'image/png', '.jpg': 'image/jpeg', - '.jpeg': 'image/jpeg', '.gif': 'image/gif', '.svg': 'image/svg+xml' -}; - -// ========== Templates and Constants ========== - -function waitingPage() { - return renderBranding(` - -Brainstorm Companion - - -

Brainstorm Companion

-

Waiting for the agent to push a screen...

`); -} - -const FORBIDDEN_PAGE = ` - -Session key required - - -

Session key required

-

This page needs the full URL your coding agent gave you, including the -?key=… part. Copy the complete URL and open it again.

`; - -function bootstrapPage(key) { - const jsonKey = JSON.stringify(String(key)); - return ` - -Opening Brainstorm Companion - - - -`; -} - -const frameTemplate = fs.readFileSync(path.join(__dirname, 'frame-template.html'), 'utf-8'); -const helperScript = fs.readFileSync(path.join(__dirname, 'helper.js'), 'utf-8'); -const helperInjection = ''; - -// ========== Helper Functions ========== - -function readSuperpowersVersion() { - const root = path.join(__dirname, '../../..'); - const manifests = [ - path.join(root, 'package.json'), - path.join(root, '.codex-plugin/plugin.json') - ]; - - for (const manifest of manifests) { - try { - const data = JSON.parse(fs.readFileSync(manifest, 'utf-8')); - if (data.version) return String(data.version); - } catch (e) { - // Packaged Codex plugins omit package.json; try the next manifest. - } - } - - return 'unknown'; -} - -function isTruthyEnv(value) { - if (!value) return false; - const normalized = String(value).trim().toLowerCase(); - if (!normalized) return false; - return !['0', 'false', 'no', 'off'].includes(normalized); -} - -function escapeHtmlText(value) { - return String(value) - .replace(/&/g, '&') - .replace(//g, '>') - .replace(/"/g, '"'); -} - -function brandMarkup() { - const version = escapeHtmlText(SUPERPOWERS_VERSION); - const text = SUPERPOWERS_TELEMETRY_DISABLED - ? 'Prime Radiant Superpowers v' + version - : 'Superpowers v' + version; - const logo = SUPERPOWERS_TELEMETRY_DISABLED - ? '' - : ''; - - return '
' + logo + '' + text + '
'; -} - -function renderBranding(html) { - return html.split('').join(brandMarkup()); -} - -function isFullDocument(html) { - const trimmed = html.trimStart().toLowerCase(); - return trimmed.startsWith('', content); -} - -function getNewestScreen() { - const files = fs.readdirSync(CONTENT_DIR) - .filter(f => !f.startsWith('.') && f.endsWith('.html')) - .map(f => { - const fp = path.join(CONTENT_DIR, f); - if (!isRegularFileInsideContentDir(fp)) return null; - return { path: fp, mtime: fs.statSync(fp).mtime.getTime() }; - }) - .filter(Boolean) - .sort((a, b) => b.mtime - a.mtime); - return files.length > 0 ? files[0].path : null; -} - -function urlHostForHttp(host) { - const h = String(host); - if (h.startsWith('[') && h.endsWith(']')) return h; - return h.includes(':') ? '[' + h + ']' : h; -} - -function companionUrl() { - return 'http://' + urlHostForHttp(URL_HOST) + ':' + PORT + '/?key=' + TOKEN; -} - -function browserLauncherForPlatform(url, { - platform = process.platform, - osRelease = require('os').release(), - env = process.env -} = {}) { - const isWSL = platform === 'linux' && /microsoft/i.test(osRelease); - if (platform === 'darwin') return { bin: 'open', args: [url] }; - if (platform === 'win32' || isWSL) { - return { bin: 'rundll32.exe', args: ['url.dll,FileProtocolHandler', url] }; - } - if (env.DISPLAY || env.WAYLAND_DISPLAY) return { bin: 'xdg-open', args: [url] }; - return null; -} - -function isRegularFileInsideContentDir(filePath) { - let stat, realContentDir, realFilePath; - try { - stat = fs.lstatSync(filePath); - if (stat.isSymbolicLink()) return false; - if (!stat.isFile()) return false; - if (stat.nlink !== 1) return false; - realContentDir = fs.realpathSync(CONTENT_DIR); - realFilePath = fs.realpathSync(filePath); - } catch (e) { - return false; - } - return realFilePath.startsWith(realContentDir + path.sep); -} - -// ========== Authentication ========== - -function timingSafeEqualStr(a, b) { - const ab = Buffer.from(String(a)); - const bb = Buffer.from(String(b)); - if (ab.length !== bb.length) return false; - return crypto.timingSafeEqual(ab, bb); -} - -function parseCookies(header) { - const out = {}; - if (!header) return out; - for (const part of header.split(';')) { - const eq = part.indexOf('='); - if (eq < 0) continue; - out[part.slice(0, eq).trim()] = part.slice(eq + 1).trim(); - } - return out; -} - -// A request is authorized if it carries the session key as ?key= or as the -// session cookie. Both are compared in constant time. -function isAuthorized(req) { - const q = req.url.indexOf('?'); - if (q >= 0) { - const params = new URLSearchParams(req.url.slice(q + 1)); - if (params.has('key')) { - const key = params.get('key'); - return Boolean(key && timingSafeEqualStr(key, TOKEN)); - } - } - const cookie = parseCookies(req.headers['cookie'])[COOKIE_NAME]; - if (cookie && timingSafeEqualStr(cookie, TOKEN)) return true; - return false; -} - -function pathnameOf(url) { - const q = url.indexOf('?'); - return q >= 0 ? url.slice(0, q) : url; -} - -function queryKey(url) { - const q = url.indexOf('?'); - if (q < 0) return null; - return new URLSearchParams(url.slice(q + 1)).get('key'); -} - -function securityHeaders(headers = {}) { - return { - 'Referrer-Policy': 'no-referrer', - 'Cache-Control': 'no-store', - 'X-Frame-Options': 'DENY', - 'Content-Security-Policy': "frame-ancestors 'none'", - 'Cross-Origin-Resource-Policy': 'same-origin', - ...headers - }; -} - -function isAllowedWebSocketOrigin(req) { - const origin = req.headers.origin; - if (!origin) return true; - const host = req.headers.host; - if (!host) return false; - return origin === 'http://' + host; -} - -// ========== HTTP Request Handler ========== - -function handleRequest(req, res) { - if (!isAuthorized(req)) { - res.writeHead(403, securityHeaders({ 'Content-Type': 'text/html; charset=utf-8' })); - res.end(FORBIDDEN_PAGE); - return; - } - touchActivity(); // only authorized requests count as activity - - // Mirror the key into a cookie so same-origin subresources (/files/*) can - // authenticate after bootstrap. HttpOnly keeps it away from page scripts; the - // WebSocket Origin check below is what blocks cross-origin localhost injection. - res.setHeader('Set-Cookie', - COOKIE_NAME + '=' + TOKEN + '; HttpOnly; SameSite=Strict; Path=/'); - - const pathname = pathnameOf(req.url); - const keyFromQuery = queryKey(req.url); - if (req.method === 'GET' && pathname === '/' && keyFromQuery && timingSafeEqualStr(keyFromQuery, TOKEN)) { - res.writeHead(200, securityHeaders({ 'Content-Type': 'text/html; charset=utf-8' })); - res.end(bootstrapPage(keyFromQuery)); - } else if (req.method === 'GET' && pathname === '/') { - const screenFile = getNewestScreen(); - let html = screenFile - ? (raw => isFullDocument(raw) ? raw : wrapInFrame(raw))(fs.readFileSync(screenFile, 'utf-8')) - : waitingPage(); - - if (html.includes('')) { - html = html.replace('', helperInjection + '\n'); - } else { - html += helperInjection; - } - - res.writeHead(200, securityHeaders({ 'Content-Type': 'text/html; charset=utf-8' })); - res.end(html); - } else if (req.method === 'GET' && pathname.startsWith('/files/')) { - const fileName = path.basename(pathname.slice(7)); - const filePath = path.join(CONTENT_DIR, fileName); - // Reject empty/dotfile names and anything that isn't a regular file — - // `/files/` would otherwise resolve to CONTENT_DIR and crash readFileSync (EISDIR). - if (!fileName || fileName.startsWith('.') || !isRegularFileInsideContentDir(filePath)) { - res.writeHead(404, securityHeaders()); - res.end('Not found'); - return; - } - const ext = path.extname(filePath).toLowerCase(); - const contentType = MIME_TYPES[ext] || 'application/octet-stream'; - res.writeHead(200, securityHeaders({ 'Content-Type': contentType })); - res.end(fs.readFileSync(filePath)); - } else { - res.writeHead(404, securityHeaders()); - res.end('Not found'); - } -} - -// ========== WebSocket Connection Handling ========== - -const clients = new Set(); - -function handleUpgrade(req, socket) { - if (!isAuthorized(req) || !isAllowedWebSocketOrigin(req)) { socket.destroy(); return; } - - const key = req.headers['sec-websocket-key']; - if (!key) { socket.destroy(); return; } - - const accept = computeAcceptKey(key); - socket.write( - 'HTTP/1.1 101 Switching Protocols\r\n' + - 'Upgrade: websocket\r\n' + - 'Connection: Upgrade\r\n' + - 'Sec-WebSocket-Accept: ' + accept + '\r\n\r\n' - ); - - let buffer = Buffer.alloc(0); - clients.add(socket); - - socket.on('data', (chunk) => { - buffer = Buffer.concat([buffer, chunk]); - while (buffer.length > 0) { - let result; - try { - result = decodeFrame(buffer); - } catch (e) { - socket.end(encodeFrame(OPCODES.CLOSE, Buffer.alloc(0))); - clients.delete(socket); - return; - } - if (!result) break; - buffer = buffer.slice(result.bytesConsumed); - - switch (result.opcode) { - case OPCODES.TEXT: - handleMessage(result.payload.toString()); - break; - case OPCODES.CLOSE: - socket.end(encodeFrame(OPCODES.CLOSE, Buffer.alloc(0))); - clients.delete(socket); - return; - case OPCODES.PING: - socket.write(encodeFrame(OPCODES.PONG, result.payload)); - break; - case OPCODES.PONG: - break; - default: { - const closeBuf = Buffer.alloc(2); - closeBuf.writeUInt16BE(1003); - socket.end(encodeFrame(OPCODES.CLOSE, closeBuf)); - clients.delete(socket); - return; - } - } - } - }); - - socket.on('close', () => clients.delete(socket)); - socket.on('error', () => clients.delete(socket)); -} - -function handleMessage(text) { - let event; - try { - event = JSON.parse(text); - } catch (e) { - console.error('Failed to parse WebSocket message:', e.message); - return; - } - touchActivity(); - console.log(JSON.stringify({ source: 'user-event', ...event })); - if (event && event.choice) { - const eventsFile = path.join(STATE_DIR, 'events'); - fs.appendFileSync(eventsFile, JSON.stringify(event) + '\n'); - } -} - -function broadcast(msg) { - const frame = encodeFrame(OPCODES.TEXT, Buffer.from(JSON.stringify(msg))); - for (const socket of clients) { - try { socket.write(frame); } catch (e) { clients.delete(socket); } - } -} - -// Best-effort: open the user's browser the first time a screen is actually ready -// to show. Skips when disabled, on a non-loopback (remote) bind, or when a -// browser is already connected. Override the launcher with BRAINSTORM_OPEN_CMD. -let browserOpened = false; -function maybeOpenBrowser() { - if (browserOpened) return; - browserOpened = true; - if (!process.env.BRAINSTORM_OPEN) return; // opt-in: only after the user approves the companion - if (HOST !== '127.0.0.1' && HOST !== 'localhost') return; - if (clients.size > 0) return; // the user already opened it - const url = companionUrl(); // must carry the key or the gate 403s it - const cp = require('child_process'); - // Operator-provided launcher: run as given (this env var is trusted operator input). - if (process.env.BRAINSTORM_OPEN_CMD) { - try { cp.exec(process.env.BRAINSTORM_OPEN_CMD + ' ' + JSON.stringify(url), () => {}); } catch (e) { /* best effort */ } - return; - } - // Platform launchers: pass the URL as an argv element via execFile (no shell), - // so a url-host containing shell metacharacters can't inject a command. - const launcher = browserLauncherForPlatform(url); - if (!launcher) return; // headless: nothing to open - try { cp.execFile(launcher.bin, launcher.args, () => {}); } catch (e) { /* best effort */ } -} - -// ========== Activity Tracking ========== - -// Idle timeout: shut down after this long with no activity. Default 4 hours; -// override with BRAINSTORM_IDLE_TIMEOUT_MS (start-server.sh: --idle-timeout-minutes). -const IDLE_TIMEOUT_MS = (() => { - const ms = Number(process.env.BRAINSTORM_IDLE_TIMEOUT_MS); - return Number.isFinite(ms) && ms > 0 ? ms : 4 * 60 * 60 * 1000; -})(); -// How often the watchdog checks for owner-death / idleness. Configurable mainly -// so tests can run fast; production default is 60s. -const LIFECYCLE_CHECK_MS = (() => { - const ms = Number(process.env.BRAINSTORM_LIFECYCLE_CHECK_MS); - return Number.isFinite(ms) && ms > 0 ? ms : 60 * 1000; -})(); -let lastActivity = Date.now(); - -function touchActivity() { - lastActivity = Date.now(); -} - -// ========== File Watching ========== - -const debounceTimers = new Map(); - -// ========== Server Startup ========== - -function startServer() { - if (!fs.existsSync(CONTENT_DIR)) fs.mkdirSync(CONTENT_DIR, { recursive: true }); - if (!fs.existsSync(STATE_DIR)) fs.mkdirSync(STATE_DIR, { recursive: true }); - - // Track known files to distinguish new screens from updates. - // macOS fs.watch reports 'rename' for both new files and overwrites, - // so we can't rely on eventType alone. - const knownFiles = new Set( - fs.readdirSync(CONTENT_DIR).filter(f => !f.startsWith('.') && f.endsWith('.html')) - ); - - const server = http.createServer(handleRequest); - server.on('upgrade', handleUpgrade); - - const watcher = fs.watch(CONTENT_DIR, (eventType, filename) => { - if (!filename || filename.startsWith('.') || !filename.endsWith('.html')) return; - - if (debounceTimers.has(filename)) clearTimeout(debounceTimers.get(filename)); - debounceTimers.set(filename, setTimeout(() => { - debounceTimers.delete(filename); - const filePath = path.join(CONTENT_DIR, filename); - - if (!fs.existsSync(filePath)) return; // file was deleted - touchActivity(); - - if (!knownFiles.has(filename)) { - knownFiles.add(filename); - const eventsFile = path.join(STATE_DIR, 'events'); - if (fs.existsSync(eventsFile)) fs.unlinkSync(eventsFile); - console.log(JSON.stringify({ type: 'screen-added', file: filePath })); - maybeOpenBrowser(); - } else { - console.log(JSON.stringify({ type: 'screen-updated', file: filePath })); - } - - broadcast({ type: 'reload' }); - }, 100)); - }); - watcher.on('error', (err) => console.error('fs.watch error:', err.message)); - - function shutdown(reason) { - console.log(JSON.stringify({ type: 'server-stopped', reason })); - const infoFile = path.join(STATE_DIR, 'server-info'); - if (fs.existsSync(infoFile)) fs.unlinkSync(infoFile); - fs.writeFileSync( - path.join(STATE_DIR, 'server-stopped'), - JSON.stringify({ reason, timestamp: Date.now() }) + '\n' - ); - watcher.close(); - clearInterval(lifecycleCheck); - // Close any upgraded WebSocket sockets so server.close() can complete and - // the process actually exits instead of lingering on an open connection. - for (const socket of clients) { - try { socket.destroy(); } catch (e) { /* already gone */ } - } - server.close(() => process.exit(0)); - } - - function ownerAlive() { - if (!ownerPid) return true; - try { process.kill(ownerPid, 0); return true; } catch (e) { return e.code === 'EPERM'; } - } - - // Periodically exit if the owner process died or we've been idle too long. - const lifecycleCheck = setInterval(() => { - if (!ownerAlive()) shutdown('owner process exited'); - else if (Date.now() - lastActivity > IDLE_TIMEOUT_MS) shutdown('idle timeout'); - }, LIFECYCLE_CHECK_MS); - lifecycleCheck.unref(); - - // Validate owner PID at startup. If it's already dead, the PID resolution - // was wrong (common on WSL, Tailscale SSH, and cross-user scenarios). - // Disable monitoring and rely on the idle timeout instead. - if (ownerPid) { - try { process.kill(ownerPid, 0); } - catch (e) { - if (e.code !== 'EPERM') { - console.log(JSON.stringify({ type: 'owner-pid-invalid', pid: ownerPid, reason: 'dead at startup' })); - ownerPid = null; - } - } - } - - // If the preferred port is already taken (e.g. a previous server is still - // alive), fall back to a random port once instead of failing. - let triedFallback = false; - - function onListen() { - // Cookie name keys on the ACTUAL bound port (may differ from the preferred - // one after an EADDRINUSE fallback) so it can't collide with another server's - // cookie in the shared localhost jar. - COOKIE_NAME = 'brainstorm-key-' + PORT; - // Record the bound port AND token so the next restart of this session reuses - // them — but ONLY when we got our preferred port. On a fallback we bound a - // *different* port because someone else holds the preferred one; persisting - // would overwrite the shared files and strand that other session's open tab. - if (PORT_FILE && !triedFallback) { - try { fs.writeFileSync(PORT_FILE, String(PORT)); } catch (e) { /* best effort */ } - if (TOKEN_FILE) { - try { - fs.writeFileSync(TOKEN_FILE, TOKEN, { mode: 0o600 }); - chmodOwnerOnly(TOKEN_FILE); - } catch (e) { /* best effort */ } - } - } - const info = JSON.stringify({ - type: 'server-started', port: Number(PORT), host: HOST, - url_host: URL_HOST, url: companionUrl(), - screen_dir: CONTENT_DIR, state_dir: STATE_DIR, idle_timeout_ms: IDLE_TIMEOUT_MS - }); - console.log(info); - // server-info embeds the key — keep it owner-only. - fs.writeFileSync(path.join(STATE_DIR, 'server-info'), info + '\n', { mode: 0o600 }); - } - - server.on('error', (err) => { - if (err.code === 'EADDRINUSE' && !triedFallback) { - if (tokenSource === 'env') { - console.error('Server failed to bind: preferred port is in use and BRAINSTORM_TOKEN is set; refusing fallback with explicit token'); - process.exit(1); - } - triedFallback = true; - PORT = randomPort(); - if (tokenSource === 'file') { - TOKEN = generateToken(); - tokenSource = 'generated-fallback'; - } - server.listen(PORT, HOST, onListen); - } else { - console.error('Server failed to bind:', err.message); - process.exit(1); - } - }); - server.listen(PORT, HOST, onListen); -} - -if (require.main === module) { - startServer(); -} - -module.exports = { - computeAcceptKey, - encodeFrame, - decodeFrame, - browserLauncherForPlatform, - OPCODES, - MAX_FRAME_PAYLOAD_BYTES -}; diff --git a/.agents/skills/brainstorming/scripts/start-server.sh b/.agents/skills/brainstorming/scripts/start-server.sh deleted file mode 100644 index 016a8e487..000000000 --- a/.agents/skills/brainstorming/scripts/start-server.sh +++ /dev/null @@ -1,209 +0,0 @@ -#!/usr/bin/env bash -# Start the brainstorm server and output connection info -# Usage: start-server.sh [--project-dir ] [--host ] [--url-host ] [--foreground] [--background] -# -# Starts server on a random high port, outputs JSON with URL. -# Each session gets its own directory to avoid conflicts. -# -# Options: -# --project-dir Store session files under /.superpowers/brainstorm/ -# instead of /tmp. Files persist after server stops. -# --host Host/interface to bind (default: 127.0.0.1). -# Use 0.0.0.0 in remote/containerized environments. -# --url-host Hostname shown in returned URL JSON. -# --idle-timeout-minutes Shut down after n minutes idle (default 240 = 4h). -# --open Auto-open the browser on the first screen (use only -# after the user approves the visual companion). -# --foreground Run server in the current terminal (no backgrounding). -# --background Force background mode (overrides Codex auto-foreground). - -SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" - -# Parse arguments -PROJECT_DIR="" -FOREGROUND="false" -FORCE_BACKGROUND="false" -BIND_HOST="127.0.0.1" -URL_HOST="" -IDLE_TIMEOUT_MINUTES="" -while [[ $# -gt 0 ]]; do - case "$1" in - --project-dir) - PROJECT_DIR="$2" - shift 2 - ;; - --host) - BIND_HOST="$2" - shift 2 - ;; - --url-host) - URL_HOST="$2" - shift 2 - ;; - --idle-timeout-minutes) - IDLE_TIMEOUT_MINUTES="$2" - shift 2 - ;; - --open) - export BRAINSTORM_OPEN=1 - shift - ;; - --foreground|--no-daemon) - FOREGROUND="true" - shift - ;; - --background|--daemon) - FORCE_BACKGROUND="true" - shift - ;; - *) - echo "{\"error\": \"Unknown argument: $1\"}" - exit 1 - ;; - esac -done - -if [[ -z "$URL_HOST" ]]; then - if [[ "$BIND_HOST" == "127.0.0.1" || "$BIND_HOST" == "localhost" ]]; then - URL_HOST="localhost" - else - URL_HOST="$BIND_HOST" - fi -fi - -if [[ -n "$IDLE_TIMEOUT_MINUTES" ]]; then - if ! [[ "$IDLE_TIMEOUT_MINUTES" =~ ^[0-9]+$ ]] || [[ "$IDLE_TIMEOUT_MINUTES" -lt 1 ]]; then - echo "{\"error\": \"--idle-timeout-minutes must be a positive integer\"}" - exit 1 - fi - export BRAINSTORM_IDLE_TIMEOUT_MS=$(( IDLE_TIMEOUT_MINUTES * 60 * 1000 )) -fi - -is_windows_like_shell() { - case "${OSTYPE:-}" in - msys*|cygwin*|mingw*) return 0 ;; - esac - if [[ -n "${MSYSTEM:-}" ]]; then - return 0 - fi - local uname_s - uname_s="$(uname -s 2>/dev/null || true)" - case "$uname_s" in - MSYS*|MINGW*|CYGWIN*) return 0 ;; - esac - return 1 -} - -# Some environments reap detached/background processes. Auto-foreground when detected. -if [[ -n "${CODEX_CI:-}" && "$FOREGROUND" != "true" && "$FORCE_BACKGROUND" != "true" ]]; then - FOREGROUND="true" -fi - -# Windows/Git Bash reaps nohup background processes. Auto-foreground when detected. -if [[ "$FOREGROUND" != "true" && "$FORCE_BACKGROUND" != "true" ]]; then - if is_windows_like_shell; then - FOREGROUND="true" - fi -fi - -# Session files (server.log, server-info, .last-token) embed the session key — -# keep everything this script and the server create owner-only. -umask 077 - -# Generate unique session directory -SESSION_ID="$$-$(date +%s)" - -if [[ -n "$PROJECT_DIR" ]]; then - SESSION_DIR="${PROJECT_DIR}/.superpowers/brainstorm/${SESSION_ID}" - # Persist the bound port and key per project so a restart reuses them and an - # already-open browser tab reconnects to the same URL with a valid cookie. - export BRAINSTORM_PORT_FILE="${PROJECT_DIR}/.superpowers/brainstorm/.last-port" - export BRAINSTORM_TOKEN_FILE="${PROJECT_DIR}/.superpowers/brainstorm/.last-token" -else - SESSION_DIR="/tmp/brainstorm-${SESSION_ID}" -fi - -STATE_DIR="${SESSION_DIR}/state" -PID_FILE="${STATE_DIR}/server.pid" -LOG_FILE="${STATE_DIR}/server.log" -SERVER_ID_FILE="${STATE_DIR}/server-instance-id" - -# Create fresh session directory with content and state peers -mkdir -p "${SESSION_DIR}/content" "$STATE_DIR" - -SERVER_ID="" -if [[ -r /dev/urandom ]]; then - SERVER_ID="$(od -An -N24 -tx1 /dev/urandom 2>/dev/null | tr -d ' \n' || true)" -fi -if ! [[ "$SERVER_ID" =~ ^[A-Za-z0-9_-]{32,64}$ ]]; then - SERVER_ID="$(printf '%08x%08x%08x%08x' "$$" "$(date +%s)" "${RANDOM:-0}" "${RANDOM:-0}")" -fi -printf '%s\n' "$SERVER_ID" > "$SERVER_ID_FILE" -chmod 600 "$SERVER_ID_FILE" 2>/dev/null || true - -# Kill any existing server -if [[ -f "$PID_FILE" ]]; then - old_pid=$(cat "$PID_FILE") - kill "$old_pid" 2>/dev/null - rm -f "$PID_FILE" -fi - -cd "$SCRIPT_DIR" || exit 1 - -# Resolve the harness PID (grandparent of this script). -# $PPID is the ephemeral shell the harness spawned to run us — it dies -# when this script exits. The harness itself is $PPID's parent. -OWNER_PID="$(ps -o ppid= -p "$PPID" 2>/dev/null | tr -d ' ')" -if [[ -z "$OWNER_PID" || "$OWNER_PID" == "1" ]]; then - OWNER_PID="$PPID" -fi - -# Windows/MSYS2: Node.js cannot see POSIX PIDs from the MSYS2 namespace. -# Passing a PID node cannot verify causes server to log owner-pid-invalid -# and self-terminate at the 60-second lifecycle check. Clear it so the -# watchdog is disabled and the idle timeout becomes the only shutdown trigger. -if is_windows_like_shell; then - OWNER_PID="" -fi - -# Foreground mode for environments that reap detached/background processes. -if [[ "$FOREGROUND" == "true" ]]; then - env BRAINSTORM_DIR="$SESSION_DIR" BRAINSTORM_HOST="$BIND_HOST" BRAINSTORM_URL_HOST="$URL_HOST" BRAINSTORM_OWNER_PID="$OWNER_PID" node server.cjs "--brainstorm-server-id=$SERVER_ID" & - SERVER_PID=$! - echo "$SERVER_PID" > "$PID_FILE" - wait "$SERVER_PID" - exit $? -fi - -# Start server, capturing output to log file -# Use nohup to survive shell exit; disown to remove from job table -nohup env BRAINSTORM_DIR="$SESSION_DIR" BRAINSTORM_HOST="$BIND_HOST" BRAINSTORM_URL_HOST="$URL_HOST" BRAINSTORM_OWNER_PID="$OWNER_PID" node server.cjs "--brainstorm-server-id=$SERVER_ID" > "$LOG_FILE" 2>&1 & -SERVER_PID=$! -disown "$SERVER_PID" 2>/dev/null -echo "$SERVER_PID" > "$PID_FILE" - -# Wait for server-started message (check log file) -for _ in {1..50}; do - if grep -q "server-started" "$LOG_FILE" 2>/dev/null; then - # Verify server is still alive after a short window (catches process reapers) - alive="true" - for _ in {1..20}; do - if ! kill -0 "$SERVER_PID" 2>/dev/null; then - alive="false" - break - fi - sleep 0.1 - done - if [[ "$alive" != "true" ]]; then - echo "{\"error\": \"Server started but was killed. Retry in a persistent terminal with: $SCRIPT_DIR/start-server.sh${PROJECT_DIR:+ --project-dir $PROJECT_DIR} --host $BIND_HOST --url-host $URL_HOST --foreground\"}" - exit 1 - fi - grep "server-started" "$LOG_FILE" | head -1 - exit 0 - fi - sleep 0.1 -done - -# Timeout - server didn't start -echo '{"error": "Server failed to start within 5 seconds"}' -exit 1 diff --git a/.agents/skills/brainstorming/scripts/stop-server.sh b/.agents/skills/brainstorming/scripts/stop-server.sh deleted file mode 100644 index 7cacfe944..000000000 --- a/.agents/skills/brainstorming/scripts/stop-server.sh +++ /dev/null @@ -1,120 +0,0 @@ -#!/usr/bin/env bash -# Stop the brainstorm server and clean up -# Usage: stop-server.sh -# -# Kills the server process. Only deletes session directory if it's -# under /tmp (ephemeral). Persistent directories (.superpowers/) are -# kept so mockups can be reviewed later. - -SESSION_DIR="$1" - -if [[ -z "$SESSION_DIR" ]]; then - echo '{"error": "Usage: stop-server.sh "}' - exit 1 -fi - -STATE_DIR="${SESSION_DIR}/state" -PID_FILE="${STATE_DIR}/server.pid" -SERVER_ID_FILE="${STATE_DIR}/server-instance-id" - -mark_stopped() { - local reason="$1" - rm -f "${STATE_DIR}/server-info" - printf '{"reason":"%s","timestamp":%s}\n' "$reason" "$(date +%s)" > "${STATE_DIR}/server-stopped" -} - -read_expected_server_id() { - [[ -f "$SERVER_ID_FILE" ]] || return 1 - local id - id="$(tr -d '\r\n' < "$SERVER_ID_FILE" 2>/dev/null || true)" - [[ "$id" =~ ^[A-Za-z0-9_-]{32,64}$ ]] || return 1 - printf '%s\n' "$id" -} - -command_line_for_pid() { - local pid="$1" - if [[ -r "/proc/$pid/cmdline" ]]; then - tr '\0' '\n' < "/proc/$pid/cmdline" 2>/dev/null || true - return 0 - fi - ps -ww -p "$pid" -o command= 2>/dev/null || ps -f -p "$pid" 2>/dev/null | sed '1d' || true -} - -command_has_server_id() { - local pid="$1" - local expected="$2" - local expected_arg="--brainstorm-server-id=$expected" - if [[ -r "/proc/$pid/cmdline" ]]; then - local arg - while IFS= read -r -d '' arg || [[ -n "$arg" ]]; do - [[ "$arg" == "$expected_arg" ]] && return 0 - done < "/proc/$pid/cmdline" - return 1 - fi - local command_line - command_line="$(command_line_for_pid "$pid")" - [[ -n "$command_line" ]] || return 1 - case " $command_line " in - *" $expected_arg "*) return 0 ;; - *) return 1 ;; - esac -} - -# Confirm a PID has this session's per-start instance id, not just a familiar -# process name. Ambiguous or legacy metadata fails closed as stale_pid. -is_brainstorm_server() { - kill -0 "$1" 2>/dev/null || return 1 - local expected_id - expected_id="$(read_expected_server_id)" || return 1 - command_has_server_id "$1" "$expected_id" || return 1 - return 0 -} - -if [[ -f "$PID_FILE" ]]; then - pid=$(cat "$PID_FILE") - - # Refuse to signal a PID we can't prove is our server. A stale pid file may - # point at an unrelated process after a reboot/PID wraparound. - if ! is_brainstorm_server "$pid"; then - rm -f "$PID_FILE" "$SERVER_ID_FILE" - mark_stopped "stale_pid" - echo '{"status": "stale_pid"}' - exit 0 - fi - - # Try to stop gracefully, fallback to force if still alive - kill "$pid" 2>/dev/null || true - - # Wait for graceful shutdown (up to ~2s) - for _ in {1..20}; do - if ! kill -0 "$pid" 2>/dev/null; then - break - fi - sleep 0.1 - done - - # If still running, escalate to SIGKILL - if kill -0 "$pid" 2>/dev/null; then - kill -9 "$pid" 2>/dev/null || true - - # Give SIGKILL a moment to take effect - sleep 0.1 - fi - - if kill -0 "$pid" 2>/dev/null; then - echo '{"status": "failed", "error": "process still running"}' - exit 1 - fi - - rm -f "$PID_FILE" "$SERVER_ID_FILE" "${STATE_DIR}/server.log" - mark_stopped "stop-server.sh" - - # Only delete ephemeral /tmp directories - if [[ "$SESSION_DIR" == /tmp/* ]]; then - rm -rf "$SESSION_DIR" - fi - - echo '{"status": "stopped"}' -else - echo '{"status": "not_running"}' -fi diff --git a/.agents/skills/brainstorming/spec-document-reviewer-prompt.md b/.agents/skills/brainstorming/spec-document-reviewer-prompt.md deleted file mode 100644 index 12b1d4040..000000000 --- a/.agents/skills/brainstorming/spec-document-reviewer-prompt.md +++ /dev/null @@ -1,50 +0,0 @@ -# Spec Document Reviewer Prompt Template - -Use this template when dispatching a spec document reviewer subagent. - -**Purpose:** Verify the requirements spec is complete, consistent, and ready for the grilling session. - -**Dispatch after:** Requirements spec is written to docs/specs/ - -``` -Subagent (general-purpose): - description: "Review requirements spec" - prompt: | - You are a spec document reviewer. Verify this requirements spec is complete and ready for grilling. - - **Spec to review:** [SPEC_FILE_PATH] - - ## What to Check - - | Category | What to Look For | - |----------|------------------| - | Completeness | TODOs, placeholders, "TBD", incomplete sections | - | Consistency | Internal contradictions, conflicting requirements | - | Clarity | Requirements ambiguous enough to cause someone to build the wrong thing | - | Scope | Focused enough for one grilling session and one implementation effort — not covering multiple independent subsystems | - | YAGNI | Unrequested features, over-engineering | - | Handoff | Ends with an "Open technical decisions" section listing the HOW decisions still open; no requirement smuggles in a technical solution unless the user demanded it | - - ## Calibration - - **Only flag issues that would cause real problems during the grilling session or implementation.** - A missing section, a contradiction, or a requirement so ambiguous it could be - interpreted two different ways — those are issues. Minor wording improvements, - stylistic preferences, and "sections less detailed than others" are not. - - Approve unless there are serious gaps that would lead to a flawed design. - - ## Output Format - - ## Spec Review - - **Status:** Approved | Issues Found - - **Issues (if any):** - - [Section X]: [specific issue] - [why it matters for the design] - - **Recommendations (advisory, do not block approval):** - - [suggestions for improvement] -``` - -**Reviewer returns:** Status, Issues (if any), Recommendations diff --git a/.agents/skills/brainstorming/visual-companion.md b/.agents/skills/brainstorming/visual-companion.md deleted file mode 100644 index c145e6438..000000000 --- a/.agents/skills/brainstorming/visual-companion.md +++ /dev/null @@ -1,299 +0,0 @@ -# Visual Companion Guide - -Browser-based visual brainstorming companion for showing mockups, diagrams, and options. - -## When to Use - -Decide per-question, not per-session. The test: **would the user understand this better by seeing it than reading it?** - -**Use the browser** when the content itself is visual: - -- **UI mockups** — wireframes, layouts, navigation structures, component designs -- **Architecture diagrams** — system components, data flow, relationship maps -- **Side-by-side visual comparisons** — comparing two layouts, two color schemes, two design directions -- **Design polish** — when the question is about look and feel, spacing, visual hierarchy -- **Spatial relationships** — state machines, flowcharts, entity relationships rendered as diagrams - -**Use the terminal** when the content is text or tabular: - -- **Requirements and scope questions** — "what does X mean?", "which features are in scope?" -- **Conceptual A/B/C choices** — picking between approaches described in words -- **Tradeoff lists** — pros/cons, comparison tables -- **Technical decisions** — API design, data modeling, architectural approach selection -- **Clarifying questions** — anything where the answer is words, not a visual preference - -A question *about* a UI topic is not automatically a visual question. "What kind of wizard do you want?" is conceptual — use the terminal. "Which of these wizard layouts feels right?" is visual — use the browser. - -## How It Works - -The server watches a directory for HTML files and serves the newest one to the browser. You write HTML content to `screen_dir`, the user sees it in their browser and can click to select options. Selections are recorded to `state_dir/events` that you read on your next turn. - -**Content fragments vs full documents:** If your HTML file starts with `/.superpowers/brainstorm/` for the session directory. - -**Note:** Pass the project root as `--project-dir` so mockups persist in `.superpowers/brainstorm/` and survive server restarts. Without it, files go to `/tmp` and get cleaned up. Remind the user to add `.superpowers/` to `.gitignore` if it's not already there. - -**Launching the server by platform:** - -**Claude Code:** -```bash -# Default mode works — the script backgrounds the server itself. -scripts/start-server.sh --project-dir /path/to/project --open -``` - -On Windows, the script auto-detects and switches to foreground mode (which blocks the tool call). Use `run_in_background: true` on the Bash tool call so the server survives across conversation turns, then read `$STATE_DIR/server-info` on the next turn to get the URL and port. - -**Codex:** -```bash -# Codex reaps background processes. The script auto-detects CODEX_CI and -# switches to foreground mode. Run it normally — no extra flags needed. -scripts/start-server.sh --project-dir /path/to/project --open -``` - -**Gemini CLI:** -```bash -# Use --foreground and set is_background: true on your shell tool call -# so the process survives across turns -scripts/start-server.sh --project-dir /path/to/project --open --foreground -``` - -**Copilot CLI:** -```bash -# Start it with Copilot CLI's non-blocking/background shell mechanism so the -# server survives across turns. Keep --foreground so the harness, not the -# script, owns backgrounding. The launcher is a .sh, so invoke it via bash -# (on Windows, call Git Bash's bash.exe from the PowerShell tool). -bash scripts/start-server.sh --project-dir /path/to/project --open --foreground -``` - -**Other environments:** The server must keep running in the background across conversation turns. If your environment reaps detached processes, use `--foreground` and launch the command with your platform's background execution mechanism. - -If the URL is unreachable from your browser (common in remote/containerized setups), bind a non-loopback host: - -```bash -scripts/start-server.sh \ - --project-dir /path/to/project \ - --host 0.0.0.0 \ - --url-host localhost -``` - -Use `--url-host` to control what hostname is printed in the returned URL JSON. - -## The Loop - -1. **Check server is alive**, then **write HTML** to a new file in `screen_dir`: - - **Required: confirm the server is alive before referring to the URL or pushing a screen.** Check that `$STATE_DIR/server-info` exists and `$STATE_DIR/server-stopped` does not. If it has shut down, restart it with `start-server.sh` using the **same `--project-dir`** — it reuses the same port, so the user's open tab reconnects on its own (it shows a "paused" overlay while the server is down) and you don't need to send a new URL. The server auto-exits after 4 hours idle (configurable with `--idle-timeout-minutes`). - - Use semantic filenames: `platform.html`, `visual-style.html`, `layout.html` - - **Never reuse filenames** — each screen gets a fresh file - - Use your file-creation tool — **never use cat/heredoc** (dumps noise into terminal) - - Server automatically serves the newest file - -2. **Tell user what to expect and end your turn:** - - Remind them of the URL (every step, not just first) - - Give a brief text summary of what's on screen (e.g., "Showing 3 layout options for the homepage") - - Ask them to respond in the terminal: "Take a look and let me know what you think. Click to select an option if you'd like." - -3. **On your next turn** — after the user responds in the terminal: - - Read `$STATE_DIR/events` if it exists — this contains the user's browser interactions (clicks, selections) as JSON lines - - Merge with the user's terminal text to get the full picture - - The terminal message is the primary feedback; `state_dir/events` provides structured interaction data - -4. **Iterate or advance** — if feedback changes current screen, write a new file (e.g., `layout-v2.html`). Only move to the next question when the current step is validated. - -5. **Unload when returning to terminal** — when the next step doesn't need the browser (e.g., a clarifying question, a tradeoff discussion), push a waiting screen to clear the stale content: - - ```html - -
-

Continuing in terminal...

-
- ``` - - This prevents the user from staring at a resolved choice while the conversation has moved on. When the next visual question comes up, push a new content file as usual. - -6. Repeat until done. - -## Writing Content Fragments - -Write just the content that goes inside the page. The server wraps it in the frame template automatically (header, theme CSS, connection status, and all interactive infrastructure). - -**Minimal example:** - -```html -

Which layout works better?

-

Consider readability and visual hierarchy

- -
-
-
A
-
-

Single Column

-

Clean, focused reading experience

-
-
-
-
B
-
-

Two Column

-

Sidebar navigation with main content

-
-
-
-``` - -That's it. No ``, no CSS, no `