Skip to content

fix: enable Feishu message flow#6

Merged
lxowalle merged 1 commit intosipeed:mainfrom
imguoguo:fix-feishu
Feb 10, 2026
Merged

fix: enable Feishu message flow#6
lxowalle merged 1 commit intosipeed:mainfrom
imguoguo:fix-feishu

Conversation

@imguoguo
Copy link
Member

No description provided.

@imguoguo imguoguo requested a review from lxowalle February 10, 2026 07:22
@lxowalle
Copy link
Collaborator

Thank you for pr!

@lxowalle lxowalle merged commit 91e7e18 into sipeed:main Feb 10, 2026
@Zepan
Copy link
Contributor

Zepan commented Feb 13, 2026

Thanks for your contribution! We are forming the PicoClaw Dev Group to accelerate the evolution of the project. Any developer with more than one merged PR is invited to join.

Would you like to join the PicoClaw Dev Group? If so, please send an email to support@sipeed.com with the subject line: [Join PicoClaw Dev Group] + Your GitHub account. We will send the Discord invite link to your inbox.

emadomedher pushed a commit to emadomedher/picoclaw that referenced this pull request Feb 17, 2026
fix: enable Feishu message flow
Nyukimin pushed a commit to Nyukimin/picoclaw_multiLLM that referenced this pull request Mar 2, 2026
伝説のアーキテクトレビューで発見された10個の致命的不備のうち、Phase 1(即時修正必須)を完了。

## 修正内容

### Issue #1: 承認フロー残骸削除 (🔴 CRITICAL)
- L328: Chat役割「承認管理」→「ユーザー対話」に変更
- L346-349: 承認フロー管理の責務削除、実行結果統合を追加
- L382: 「承認後に担当」→「即座に担当」に修正
- L384: 目的「承認フロー確実な動作」→「Worker即時実行確実な動作」
- L660: JobID コメント「承認ジョブ」→「Worker実行ジョブ」
- L835: CHAT ルート「承認管理」→「対話管理」
- L881: ルール辞書「承認して|approve」→「状態|status|確認」
- L1274-1278: Section 11.3「承認必須操作」完全削除
- L1285-1297: approval イベント削除、worker イベントに置換
- L1457: テスト「承認フロー」→「Worker即時実行」
- L2250: usecase.NewManageApproval 削除
- L2289: 付録差分「承認フロー仕様」→「(削除)」
- L2318: アグリゲート例「ApprovalFlow」→「Session」

### Issue sipeed#2: データベーススキーマ残骸削除 (🔴 CRITICAL)
- L288-293: db/migrations/ ディレクトリ削除
  - 001_create_events_table.sql
  - 002_create_jobs_table.sql
  - 003_create_auto_approve_policies_table.sql
- L2155-2194: Section 18.3「Job Repository」完全削除
  - SQLiteJobRepository実装
  - approval.ApprovalFlow 参照

### Issue sipeed#3, sipeed#4: EventStore、approval package参照削除 (🔴 CRITICAL)
- L1460: 実装プラン「Event Store, Worker実行」→「Worker即時実行、E2Eシナリオ」
- L2224-2226: Wire DI import削除
  - internal/domain/approval
  - internal/infrastructure/eventstore
  - internal/infrastructure/persistence/job
- L2233-2235: Wire DI provider削除
  - provideSQLiteDB
  - eventstore.NewSQLiteEventStore
  - job.NewSQLiteJobRepository
  - approval.NewAutoApprovePolicy
  - service.NewApprovalService
- L2220+: Wire DI import追加
  - internal/infrastructure/persistence/session
  - pkg/tools
  - provideToolRegistry
  - session.NewJSONSessionRepository

### Issue sipeed#5: セクション番号修正 (🟡 MEDIUM)
- 11章: ### 12.1, 12.2 → 11.1, 11.2
- 13章: ### 14.1, 13.2, 14.3 → 13.1, 13.2, 13.3
- 14章: ### 15.1 → 14.1
- 15章: ### 16.1, 16.2 → 15.1, 15.2
- 16章: ### 17.1 → 16.1
- 17章: ### 18.1 → 17.1
- 18章: ### 19.1 → 18.1

## 影響

### 修正前(システム破綻リスク)
- ❌ 承認フロー残骸13箇所
- ❌ 存在しないパッケージへの依存(ビルド失敗)
- ❌ 削除されたDB migrations参照
- ❌ Wire DI設定矛盾(起動不能)

### 修正後(実装可能)
- ✅ 承認フロー完全削除
- ✅ Worker即時実行への統一
- ✅ ビルド可能な依存関係
- ✅ 正しいWire DI設定

## 残存Issue(Phase 2で対応)

- Issue sipeed#6: workspace設定の統一(実装前に必須)
- Issue sipeed#7: parseMarkdownPatch正規表現修正(実装前に必須)
- Issue sipeed#8: コードブロック順序保証(実装前に必須)
- Issue sipeed#9: 保護ファイルチェック強化(実装前に必須)
- Issue sipeed#10: Git ロック機構追加(実装中に対応)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Nyukimin pushed a commit to Nyukimin/picoclaw_multiLLM that referenced this pull request Mar 2, 2026
Phase 2の4つのHIGH重大度不備を修正。実装開始時のトラブルを回避。

## 修正内容

### Issue sipeed#6: workspace設定の統一 (🟠 HIGH)
**問題**: WorkerConfig.Workspace と NewWorkerExecutionService(workspace) の二重管理

**修正**:
- L1728-1735: workerExecutionService構造体から `workspace string` フィールド削除
- L1737-1745: NewWorkerExecutionService引数から `workspace string` 削除
- L1934, 2031: `w.workspace` → `w.config.Workspace` に統一(全2箇所)

**効果**: workspace設定が WorkerConfig に一元化、設定の不整合を防止

---

### Issue sipeed#7: parseMarkdownPatch正規表現修正 (🟠 HIGH)
**問題**: 正規表現が複数行コードブロックに対応していない

**修正前**:
```go
reCodeBlock := regexp.MustCompile("```([a-z]+):([^\n]+)\n(.*?)\n```")
reShellBlock := regexp.MustCompile("```bash\n(.*?)\n```")
```
- `.*?` は改行にマッチしない(デフォルトDOTALLなし)
- 複数行のコードが切り詰められる

**修正後**:
```go
reCodeBlock := regexp.MustCompile("(?s)```([a-z]+):([^\n]+)\n(.*?)```")
reShellBlock := regexp.MustCompile("(?s)```bash\n(.*?)```")
```
- `(?s)` フラグ追加: DOTALL モード有効化
- `.` が改行を含むすべての文字にマッチ
- 複数行コードブロックを正しくパース可能

**効果**: 複数行のファイル編集、複数行シェルコマンドが正常動作

---

### Issue sipeed#8: コードブロック順序保証 (🟠 HIGH)
**問題**: Markdown出現順序を無視(ファイル編集 → シェルコマンドの順で強制実行)

**修正前のロジック**:
1. すべてのファイル編集コードブロックを抽出 → commands配列に追加
2. すべてのシェルコマンドブロックを抽出 → commands配列に追加
3. → **出現順序が狂う**(例: テスト実行 → ファイル編集の順が逆転)

**修正後のロジック**:
1. FindAllStringSubmatchIndex() で位置情報(pos)付きで抽出
2. positionedCommand{pos, cmd} 構造体で管理
3. sort.Slice() で pos 順にソート
4. → **Markdown出現順を保証**

**修正箇所**:
- L1867-1915: parseMarkdownPatch() 全体を書き換え
- positionedCommand型導入、位置情報管理
- FindAllStringSubmatchIndex()使用、ソート処理追加

**効果**: パッチの意図した実行順序を保証(例: ファイル編集 → go test の順序が正しく実行)

---

### Issue sipeed#9: 保護ファイルチェック強化 (🟠 HIGH)
**問題**: rename/copy のデスティネーションが保護ファイルチェックされない

**攻撃シナリオ**:
```json
{
  "type": "file_edit",
  "action": "copy",
  "target": "/tmp/dummy.txt",  // ← 通常ファイル(チェックOK)
  "content": ".env"             // ← 保護ファイル(チェックなし!)
}
```
→ `.env` が上書きされる脆弱性

**修正**:
- L1996: ActionRename のデスティネーション保護チェック追加
  ```go
  if w.isProtectedFile(cmd.Content) {
      return "", fmt.Errorf("cannot rename to protected file: %s", cmd.Content)
  }
  ```
- L2012: ActionCopy のデスティネーション保護チェック追加
  ```go
  if w.isProtectedFile(cmd.Content) {
      return "", fmt.Errorf("cannot copy to protected file: %s", cmd.Content)
  }
  ```

**効果**: 保護ファイル(.env*, *credentials*, *.key等)への上書き攻撃を完全ブロック

---

## 修正統計

| Issue | 修正箇所 | 追加行 | 削除行 | 変更行 |
|-------|---------|--------|--------|--------|
| sipeed#6 workspace統一 | 4箇所 | 2行 | 4行 | 2行 |
| sipeed#7 regex修正 | 2箇所 | 4行 | 2行 | 2行 |
| sipeed#8 順序保証 | 1関数 | 24行 | 18行 | 49行 |
| sipeed#9 保護強化 | 2箇所 | 6行 | 0行 | 6行 |
| **合計** | **9箇所** | **36行** | **24行** | **59行** |

## テストケース追加推奨

実装時に以下のテストケースを追加すべき:

### Issue sipeed#7: 複数行コードブロック
```go
func TestParseMarkdownPatch_MultilineCode(t *testing.T) {
    patch := "```go:main.go\npackage main\n\nfunc main() {\n    fmt.Println(\"test\")\n}\n```"
    commands, err := parseMarkdownPatch(patch)
    assert.NoError(t, err)
    assert.Contains(t, commands[0].Content, "func main()")
}
```

### Issue sipeed#8: 実行順序保証
```go
func TestParseMarkdownPatch_PreservesOrder(t *testing.T) {
    patch := "```go:file.go\ncode\n```\n```bash\ngo test\n```"
    commands, err := parseMarkdownPatch(patch)
    assert.Equal(t, "file_edit", commands[0].Type)
    assert.Equal(t, "shell_command", commands[1].Type)
}
```

### Issue sipeed#9: 保護ファイル攻撃防御
```go
func TestExecuteFileEdit_ProtectedDestination(t *testing.T) {
    cmd := PatchCommand{Action: "copy", Target: "/tmp/dummy", Content: ".env"}
    _, err := executeFileEdit(ctx, cmd)
    assert.Error(t, err)
    assert.Contains(t, err.Error(), "protected file")
}
```

---

## 残存Issue(Phase 3で対応)

- Issue sipeed#10: Git auto-commit race condition (🟡 MEDIUM)
  - 対応タイミング: 実装中(並行実行が必要になった時点)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Nyukimin pushed a commit to Nyukimin/picoclaw_multiLLM that referenced this pull request Mar 3, 2026
- HIGH sipeed#6: Agent名称統一テーブル(§1.4)追加、config key統一
- HIGH sipeed#7: ResultPayloadにModifiedFiles/DeletedFiles追加
- HIGH sipeed#8: Orchestrator移行設計(§2.4.1)新設、Adapterパターン方針明記
- HIGH sipeed#9: 並列実行スコープをv4.0(PatchCommand並列)/v4.1(タスク分割)に分離
- HIGH sipeed#10: truncate()をrune単位に変更し日本語破壊を防止
- HIGH sipeed#11: Sessionスレッドセーフ性(§7.1.1)の段階的方針を明記

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Nyukimin pushed a commit to Nyukimin/picoclaw_multiLLM that referenced this pull request Mar 3, 2026
- MEDIUM sipeed#4: ResultPayloadにexecuted_cmds/failed_cmds/command_results追加
  JSON Schema(A.3)とResultメッセージ例(§6.3)も更新
- MEDIUM sipeed#18: ErrorCode型定義(§8.2.3)新設、5つのエラーコード定数
  ResultPayload/JSON Schemaにerror_codeフィールド追加
- MEDIUM sipeed#9: OptimisticLockをMetadata活用に変更(§7.4)
  PatchCommand構造体は不変維持、cmd.Targetに統一
- MEDIUM sipeed#12: Timestamp UTC強制をValidate()に追加
  CentralMemoryソートをtime.Time比較に変更
- MEDIUM sipeed#6: ProposalPayload.Patchをjson.RawMessageに変更
  二重エンコーディング回避、変換契約を明記

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants