diff --git a/agent-task/archive/2026/07/openai_text_tool_call_boundary/code_review_local_G04_1.log b/agent-task/archive/2026/07/openai_text_tool_call_boundary/code_review_local_G04_1.log new file mode 100644 index 0000000..82cc445 --- /dev/null +++ b/agent-task/archive/2026/07/openai_text_tool_call_boundary/code_review_local_G04_1.log @@ -0,0 +1,184 @@ + + +# Code Review Reference - REVIEW_BUGFIX_TOOLCALL + +> **[IMPLEMENTING AGENT — READ FIRST] Filling in this file is the mandatory final step of implementation.** +> The task is NOT complete until every implementation-owned section below is filled in. +> Complete the `구현 체크리스트`; the final checklist item is mandatory before saving. +> Fill implementation-owned sections, then stop with active files in place and report ready for review. +> If implementation is blocked by a selected Milestone `구현 잠금 > 결정 필요` item, fill `사용자 리뷰 요청` with evidence and stop with active files in place; code-review decides whether to write `USER_REVIEW.md`. Environment/secret/service setup, generic scope conflicts, loop exhaustion, and evidence gaps that a follow-up agent can close are normal follow-up issues, not user-review blockers by themselves. +> Do not ask the user directly, present choices in chat, or call `request_user_input` during implementation; record only the linked Milestone lock decision in `사용자 리뷰 요청` and stop for code-review. +> Finalization (`코드리뷰 결과`, log rename, `complete.log`, archive moves, `코드리뷰 전용 체크리스트`) is review-agent-only, even after compaction/resume. +> Follow the ownership table at the bottom of this file for which sections you own. + +## 개요 + +date=2026-07-06 +task=openai_text_tool_call_boundary, plan=1, tag=REVIEW_BUGFIX_TOOLCALL + +## Archive Evidence Snapshot + +- 이전 루프 archive: + - plan: `agent-task/openai_text_tool_call_boundary/plan_local_G06_0.log` + - review: `agent-task/openai_text_tool_call_boundary/code_review_local_G06_0.log` +- 판정: WARN (Required 0, Suggested 1, Nit 2) +- Suggested 요약: `apps/edge/internal/openai/chat_handler.go:383-386` — native tool call과 content가 공존할 때 `streamToolTextFilter{pending: message.Content}.FlushNonCandidate()`가 content 안 어디든 후보 힌트(` **[REVIEW AGENT ONLY]** 아래 종결 절차는 코드리뷰 에이전트 전용이다. 구현 에이전트는 이 섹션을 실행하지 않는다. + +각 항목의 구현을 실제 소스 파일과 대조하고, `검증 결과` 섹션의 출력이 코드와 일치하는지 확인하세요. +리뷰 완료는 아래 순서까지 끝난 상태를 의미합니다. + +1. 판정을 append한다. +2. `CODE_REVIEW-local-G04.md` → `code_review_local_G04_N.log`, `PLAN-local-G04.md` → `plan_local_G04_M.log`로 아카이브한다. +3. PASS이면 `complete.log` 작성 후 active task 디렉터리를 `agent-task/archive/YYYY/MM/openai_text_tool_call_boundary/`로 이동한다. WARN/FAIL이면 user-review gate를 확인한 뒤 다음 active plan/review 파일 또는 `USER_REVIEW.md`를 작성한다. `USER_REVIEW.md`가 연결된 Milestone 결정으로 완료/PASS 해소되면 `USER_REVIEW.md`를 해소 상태로 갱신하고 `complete.log` 작성 후 archive 이동한다. +4. PASS이고 task group이 `m-`이면 완료 이벤트 메타데이터를 보고한다. roadmap 상태 체크와 `update-roadmap` 호출은 런타임 책임이다. +5. 적용 가능한 `코드리뷰 전용 체크리스트` 항목을 최종 `.log` 위치에서 체크한 뒤 보고한다. + +--- + +## 구현 항목별 완료 여부 + +| 항목 | 완료 여부 | +|------|---------| +| [REVIEW_BUGFIX_TOOLCALL-1] native tool call 혼합 content의 선행 prose 보존 | [x] | +| [REVIEW_BUGFIX_TOOLCALL-2] Nit 정리(shadowing, passthrough, dead chain) | [x] | + +## 구현 체크리스트 + +- [x] [REVIEW_BUGFIX_TOOLCALL-1] native tool call과 content 혼합 시 후보 시작 전 prose를 보존하도록 `collectChatCompletionOutput`의 content 정리를 `Append` 기반으로 바꾸고 회귀 테스트를 추가한다. +- [x] [REVIEW_BUGFIX_TOOLCALL-2] Nit 정리: `findXMLOpenTag`의 `close` shadowing 개명, `toolValidationContractForChatRequest` passthrough 정리, dead text tool-call match 체인을 컴파일/테스트 확인 하에 삭제한다. +- [x] CODE_REVIEW-*-G??.md의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채운다. 이 항목이 완료되기 전에는 구현이 완료된 것이 아니다. + +## 코드리뷰 전용 체크리스트 + +> **[REVIEW AGENT ONLY]** 이 체크리스트는 코드리뷰 에이전트만 사용한다. +> 구현 에이전트는 이 섹션을 수정하거나 체크하지 않는다. + +- [x] `코드리뷰 결과`에 `PASS`, `WARN`, `FAIL` 중 하나의 판정을 append한다. (PASS) +- [x] 판정과 `차원별 평가`, Required/Suggested/Nit 분류가 서로 일치한다. (전 차원 Pass, 발견된 문제 없음) +- [x] active `CODE_REVIEW-*-G??.md`를 `code_review_local_G04_1.log`로 아카이브한다. +- [x] active `PLAN-*-G??.md`를 `plan_local_G04_1.log`로 아카이브한다. +- [x] `.gitignore`의 Agent-Ops 관리 block이 `agent-task/**/*.md`와 `agent-task/**/*.log`를 unignore하고 `agent-roadmap/current.md`를 ignore하는지 확인한다. (`git check-ignore` 0건) +- [x] PASS이면 `agent-ops/skills/common/code-review/templates/complete-log-template.md` 기준으로 `complete.log`를 작성하고 active `.md` 파일을 남기지 않는다. +- [x] PASS이면 active task 디렉터리 `agent-task/openai_text_tool_call_boundary/`를 `agent-task/archive/2026/07/openai_text_tool_call_boundary/`로 이동하고 최종 archive 경로에서 이 체크리스트를 갱신한다. +- [ ] PASS이고 task group이 `m-`이면 런타임이 읽을 완료 이벤트 메타데이터를 보고하고, roadmap 수정이나 `update-roadmap` 직접 호출을 하지 않는다. +- [ ] PASS split 작업이면 이동 후 빈 active parent `agent-task/{task_group}/`를 제거하거나, 남은 sibling/file이 있어 유지했다고 확인한다. +- [ ] WARN/FAIL이고 user-review gate가 트리거되지 않았으면 다음 active `PLAN-{build_lane}-GNN.md`와 `CODE_REVIEW-{review_lane}-GNN.md`를 작성하고 `complete.log`를 작성하지 않는다. +- [ ] USER_REVIEW이면 `agent-ops/skills/common/code-review/templates/user-review-template.md` 기준으로 `USER_REVIEW.md`를 작성하고 active `PLAN-*.md`, `CODE_REVIEW-*.md`, `complete.log`를 남기지 않는다. +- [ ] USER_REVIEW가 연결된 Milestone 결정으로 완료/PASS 해소되면 `USER_REVIEW.md`를 해소 상태로 갱신하고 `complete.log`를 작성한 뒤 task directory를 archive로 이동한다. + +## 계획 대비 변경 사항 + +- 없음. 계획 범위대로 `apps/edge/internal/openai` 내부의 content 보존 회귀, shadowing/passthrough/dead code 정리, 회귀 테스트 추가만 수행했다. + +## 주요 설계 결정 + +- native `tool_calls`와 content가 함께 수집된 경우, 전체 content를 `FlushNonCandidate()`에 바로 넣지 않고 `Append()`로 안전 prefix를 먼저 방출한 뒤 잔여 후보 fragment만 제거한다. 이로써 선행 prose는 보존하고 raw ``/mustache 후보는 성공 응답에 노출하지 않는다. +- `toolValidationContractForChatRequest`는 stream 정책 분기 없이 `toolValidationContractFromRequest`를 그대로 반환하던 wrapper라 제거하고 호출부를 직접 연결했다. +- `collectTextToolCallMatches` 계열 dead helper는 `rg`로 호출자 부재를 재확인한 뒤 삭제했다. production에서 쓰는 `synthesizeToolCallsFromTextResult`, XML scanner, mustache candidate parser는 유지했다. + +## 사용자 리뷰 요청 + +_기본값은 `없음`이다. 구현 중 새 결정이 필요해 보여도 직접 질문하거나 선택지를 제시하거나 `request_user_input`을 호출하지 않는다. 이 섹션은 선택된 Milestone `구현 잠금 > 결정 필요` 항목이 실구현을 차단할 때만 채운다. 외부 환경/secret/서비스 준비, 검증 증거 공백, 반복 실패, 일반 범위 조정은 사용자 리뷰 요청이 아니며 `검증 결과`, `계획 대비 변경 사항`, 또는 code-review의 일반 follow-up plan으로 처리한다._ + +- 상태: 없음 +- 사유 유형: 없음 +- 연결 대상: 없음 +- 결정 필요: 없음 +- 차단 근거: 없음 +- 실행한 검증/명령: 없음 +- 자동 후속 불가 이유: 없음 +- 재개 조건: 없음 + +## 리뷰어를 위한 체크포인트 + +- native tool call + 선행 prose + raw `` echo 혼합에서 prose가 응답 content에 남는지 확인한다. +- 수정 후에도 raw ``/escaped ` **[IMPLEMENTING AGENT — BEFORE SAVING] Have you filled in every implementation-owned section: completion table, implementation checklist, changes from plan, design decisions, and verification output?** +> If anything is blank, go back and fill it in before saving this file. +> Leave review-agent-only sections unchanged. + +## Section Ownership + +| Section | Owner | Note | +|---------|-------|------| +| Header comment, 개요, Archive Evidence Snapshot, 리뷰 에이전트 지시 | Fixed at stub creation | Implementing agent must not modify or execute archive/complete/archive move steps | +| 구현 항목별 완료 여부 | Implementing agent checks only | Item text/order fixed | +| 구현 체크리스트 | Implementing agent checks only | Final checkbox is mandatory | +| 코드리뷰 전용 체크리스트 | Review agent only | Implementing agent must not modify | +| 계획 대비 변경 사항, 주요 설계 결정 | Implementing agent | Replace placeholder text with actual content | +| 사용자 리뷰 요청 | Implementing agent | Keep `상태: 없음` unless selected Milestone lock decision blocks implementation | +| 리뷰어를 위한 체크포인트 | Fixed at stub creation | Review focus list | +| 검증 결과 | Implementing agent | Paste actual stdout/stderr | +| 코드리뷰 결과 | Review agent appends | Not included until review | + +## 코드리뷰 결과 + +### 종합 판정 + +PASS + +### 차원별 평가 + +| 차원 | 판정 | 근거 | +|------|------|------| +| Correctness | Pass | `chat_handler.go:383-386`의 `filter.Append(message.Content) + filter.FlushNonCandidate()`가 live stream 경로(`stream.go:311-343`)와 동일 의미로 첫 후보 힌트 전 prose를 방출하고 잔여 후보/부분 후보 fragment만 제거함을 코드로 확인. 후보 없는 순수 prose는 전량 보존, 후보 시작 content는 기존과 동일하게 제거된다. | +| Completeness | Pass | 계획 체크리스트 3항목 모두 구현 확인: Append 기반 content 정리 + 회귀 테스트 2개(`server_test.go`), `close`→`closeIdx` 개명(`types.go:557`), `toolValidationContractForChatRequest` wrapper 제거 및 `chat_handler.go:79` 직접 호출, dead 체인(`collectTextToolCallMatches`/`collectXMLTextToolCallMatches`/`collectMustacheTextToolCallMatches`/`parseTextToolCallBlock`/`textToolCallMatch`/`mustacheTextToolCallBlock` + 미사용 regex 4종/`regexp` import) 삭제. | +| Test coverage | Pass | `TestChatCompletionsNativeToolCallPreservesLeadingProse`, `TestChatCompletionsToolsStreamNativeToolCallPreservesLeadingProse`가 계획 요구(선행 prose 보존, raw/escaped ` 0` stream을 전부 buffered 경로로 라우팅하므로 live SSE에 tools validation이 켜지는 경로가 없고, `chat_handler.go:83-85`에서 stream+tools attempt=1 metadata 유지(`TestChatCompletionsStreamWithToolsBuffersTextUntilComplete`가 assert). | +| Code quality | Pass | debug print, TODO, dead code 잔존 없음. 삭제 심볼 stale 참조를 `rg`로 전수 확인(0건). `gofmt -l` clean. | +| Implementation deviation | Pass | `계획 대비 변경 사항: 없음`이 diff와 일치. 계획의 두 줄 표현을 한 식으로 inline한 것 외 차이 없음(동일 의미, Go 좌→우 피연산자 평가). | +| Verification trust | Pass | 리뷰에서 동일 명령 재실행: targeted 테스트 PASS(0.005s), `go build ./apps/edge/...` OK, `go test -count=1 -timeout 60s ./apps/edge/internal/openai` PASS(1.576s, 보고된 1.579s와 동일 범위), `gofmt -l` clean. | + +### 발견된 문제 + +없음 + +### 다음 단계 + +- PASS: `complete.log` 작성 후 task 디렉터리를 `agent-task/archive/2026/07/openai_text_tool_call_boundary/`로 이동한다. diff --git a/agent-task/openai_text_tool_call_boundary/CODE_REVIEW-local-G06.md b/agent-task/archive/2026/07/openai_text_tool_call_boundary/code_review_local_G06_0.log similarity index 59% rename from agent-task/openai_text_tool_call_boundary/CODE_REVIEW-local-G06.md rename to agent-task/archive/2026/07/openai_text_tool_call_boundary/code_review_local_G06_0.log index c04af8b..dbca340 100644 --- a/agent-task/openai_text_tool_call_boundary/CODE_REVIEW-local-G06.md +++ b/agent-task/archive/2026/07/openai_text_tool_call_boundary/code_review_local_G06_0.log @@ -35,44 +35,49 @@ task=openai_text_tool_call_boundary, plan=0, tag=BUGFIX_TOOLCALL | 항목 | 완료 여부 | |------|---------| -| [BUGFIX_TOOLCALL-1] 실제 dev 실패 형태의 `edit` raw XML tool-call 회귀 테스트 | [ ] | -| [BUGFIX_TOOLCALL-2] XML raw text tool-call parser scanner 강화 | [ ] | -| [BUGFIX_TOOLCALL-3] tools streaming buffered validation 완화 | [ ] | -| [BUGFIX_TOOLCALL-4] 검증과 dev smoke 계약 기록 | [ ] | +| [BUGFIX_TOOLCALL-1] 실제 dev 실패 형태의 `edit` raw XML tool-call 회귀 테스트 | [x] | +| [BUGFIX_TOOLCALL-2] XML raw text tool-call parser scanner 강화 | [x] | +| [BUGFIX_TOOLCALL-3] tools streaming buffered validation 완화 | [x] | +| [BUGFIX_TOOLCALL-4] 검증과 dev smoke 계약 기록 | [x] | ## 구현 체크리스트 -- [ ] [BUGFIX_TOOLCALL-1] 실제 dev 실패 형태의 `edit` raw XML tool-call 회귀 테스트를 추가한다. -- [ ] [BUGFIX_TOOLCALL-2] XML raw text tool-call parser를 regex-only에서 parameter body를 opaque text로 다루는 scanner로 강화하고 기존 합성 경로를 모두 같은 parser로 연결한다. -- [ ] [BUGFIX_TOOLCALL-3] `tools[]`가 있는 streaming chat completions를 retry 가능한 buffered validation 경로로 보내는 완화책을 추가한다. -- [ ] [BUGFIX_TOOLCALL-4] 로컬 회귀 테스트와 dev-runtime smoke 계약을 실행하고 결과를 review stub에 남긴다. -- [ ] CODE_REVIEW-*-G??.md의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채운다. 이 항목이 완료되기 전에는 구현이 완료된 것이 아니다. +- [x] [BUGFIX_TOOLCALL-1] 실제 dev 실패 형태의 `edit` raw XML tool-call 회귀 테스트를 추가한다. +- [x] [BUGFIX_TOOLCALL-2] XML raw text tool-call parser를 regex-only에서 parameter body를 opaque text로 다루는 scanner로 강화하고 기존 합성 경로를 모두 같은 parser로 연결한다. +- [x] [BUGFIX_TOOLCALL-3] `tools[]`가 있는 streaming chat completions를 retry 가능한 buffered validation 경로로 보내는 완화책을 추가한다. +- [x] [BUGFIX_TOOLCALL-4] 로컬 회귀 테스트와 dev-runtime smoke 계약을 실행하고 결과를 review stub에 남긴다. +- [x] CODE_REVIEW-*-G??.md의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채운다. 이 항목이 완료되기 전에는 구현이 완료된 것이 아니다. ## 코드리뷰 전용 체크리스트 > **[REVIEW AGENT ONLY]** 이 체크리스트는 코드리뷰 에이전트만 사용한다. > 구현 에이전트는 이 섹션을 수정하거나 체크하지 않는다. -- [ ] `코드리뷰 결과`에 `PASS`, `WARN`, `FAIL` 중 하나의 판정을 append한다. -- [ ] 판정과 `차원별 평가`, Required/Suggested/Nit 분류가 서로 일치한다. -- [ ] active `CODE_REVIEW-*-G??.md`를 `code_review_local_G06_N.log`로 아카이브한다. -- [ ] active `PLAN-*-G??.md`를 `plan_local_G06_M.log`로 아카이브한다. -- [ ] `.gitignore`의 Agent-Ops 관리 block이 `agent-task/**/*.md`와 `agent-task/**/*.log`를 unignore하고 `agent-roadmap/current.md`를 ignore하는지 확인한다. +- [x] `코드리뷰 결과`에 `PASS`, `WARN`, `FAIL` 중 하나의 판정을 append한다. (WARN) +- [x] 판정과 `차원별 평가`, Required/Suggested/Nit 분류가 서로 일치한다. (Suggested 1 + Nit 2 → WARN) +- [x] active `CODE_REVIEW-*-G??.md`를 `code_review_local_G06_N.log`로 아카이브한다. (N=0) +- [x] active `PLAN-*-G??.md`를 `plan_local_G06_M.log`로 아카이브한다. (M=0) +- [x] `.gitignore`의 Agent-Ops 관리 block이 `agent-task/**/*.md`와 `agent-task/**/*.log`를 unignore하고 `agent-roadmap/current.md`를 ignore하는지 확인한다. - [ ] PASS이면 `agent-ops/skills/common/code-review/templates/complete-log-template.md` 기준으로 `complete.log`를 작성하고 active `.md` 파일을 남기지 않는다. - [ ] PASS이면 active task 디렉터리 `agent-task/openai_text_tool_call_boundary/`를 `agent-task/archive/YYYY/MM/openai_text_tool_call_boundary/`로 이동하고 최종 archive 경로에서 이 체크리스트를 갱신한다. - [ ] PASS이고 task group이 `m-`이면 런타임이 읽을 완료 이벤트 메타데이터를 보고하고, roadmap 수정이나 `update-roadmap` 직접 호출을 하지 않는다. - [ ] PASS split 작업이면 이동 후 빈 active parent `agent-task/{task_group}/`를 제거하거나, 남은 sibling/file이 있어 유지했다고 확인한다. -- [ ] WARN/FAIL이고 user-review gate가 트리거되지 않았으면 다음 active `PLAN-local-G06.md`와 `CODE_REVIEW-local-G06.md`를 작성하고 `complete.log`를 작성하지 않는다. +- [x] WARN/FAIL이고 user-review gate가 트리거되지 않았으면 다음 active plan/review 파일을 작성하고 `complete.log`를 작성하지 않는다. (후속 lane/grade 재산정: `PLAN-local-G04.md`/`CODE_REVIEW-local-G04.md`, plan=1, tag=REVIEW_BUGFIX_TOOLCALL) - [ ] USER_REVIEW이면 `agent-ops/skills/common/code-review/templates/user-review-template.md` 기준으로 `USER_REVIEW.md`를 작성하고 active `PLAN-*.md`, `CODE_REVIEW-*.md`, `complete.log`를 남기지 않는다. - [ ] USER_REVIEW가 연결된 Milestone 결정으로 완료/PASS 해소되면 `USER_REVIEW.md`를 해소 상태로 갱신하고 `complete.log`를 작성한 뒤 task directory를 archive로 이동한다. ## 계획 대비 변경 사항 -_구현 에이전트가 계획과 다르게 구현한 부분을 이유와 함께 기록한다._ +- `apps/edge/internal/openai/chat_handler.go`도 수정했다. `tools[]` stream을 buffered path로 보내면 native `tool_calls`와 함께 수집된 raw text tool-call 후보가 content chunk로 다시 출력될 수 있어, 기존 live stream filter와 같은 `FlushNonCandidate()` 정리를 collect 단계에 적용했다. +- 기존 `TestChatCompletionsStreamWithToolsFlushesTextBeforeComplete`는 새 정책과 충돌하므로 `TestChatCompletionsStreamWithToolsBuffersTextUntilComplete`로 갱신했다. root fix에서는 tool-bearing stream이 complete 전 live content flush를 하지 않고 buffered validation을 우선한다. +- dev-runtime smoke는 실행하지 않았다. 현재 local 변경이 원격 `/Users/toki/agent-work/iop-dev` checkout과 dev-runtime binary에 배포된 상태가 아니므로, 실행하더라도 이번 변경 검증으로 인정할 수 없다. ## 주요 설계 결정 -_구현 에이전트가 주요 설계 결정 사항을 기록한다._ +- `stream=true`라도 요청에 `tools[]`가 있으면 strict 설정과 무관하게 buffered stream 경로를 사용한다. 이 경로는 첫 SSE chunk 전 전체 run output을 수집하므로 schema validation 실패 시 bounded retry 또는 controlled `tool_validation_error`로 닫을 수 있다. +- runtime tool validation contract는 stream 여부로 끄지 않는다. tool-bearing stream이 buffered path로 라우팅되므로 non-stream과 같은 validation/retry 의미를 유지한다. +- XML text tool-call parser는 regex-only 매칭 대신 작은 scanner로 교체했다. `` 본문은 close tag 전까지 opaque text로 다루어 Markdown fence, JSON string, literal `` 같은 angle-bracket placeholder를 tag로 오인하지 않는다. +- 모델이 잘못 만든 `edit` 인자를 자동으로 임의 보정하지는 않는다. 스키마 위반은 Edge가 Pi에 전달하기 전에 검증/재시도하고, 재시도 후에도 실패하면 controlled error로 반환하는 정책이다. ## 사용자 리뷰 요청 @@ -109,36 +114,36 @@ _구현 에이전트가 각 중간 검증 및 최종 검증 명령 실행 후 ### BUGFIX_TOOLCALL-1 중간 검증 ```bash $ go test ./apps/edge/internal/openai -run 'TestChatCompletions(SynthesizesEditToolCallWithMarkdownAndAngleBracketPlaceholder|StreamSynthesizesEditToolCallWithMarkdownAndAngleBracketPlaceholder)$' -(output) +ok iop/apps/edge/internal/openai 0.006s ``` ### BUGFIX_TOOLCALL-2 중간 검증 ```bash $ go test ./apps/edge/internal/openai -run 'TestChatCompletions(SynthesizesToolCallsFromClineTextBlock|SynthesizesTextToolCallsForProviderRoute|SynthesizesProviderTextToolCallsWhenFallbackMarked|FailsMalformedTextToolCallAfterRetryLimit|SynthesizesEditToolCallWithMarkdownAndAngleBracketPlaceholder)$' -(output) +ok iop/apps/edge/internal/openai 0.006s ``` ### BUGFIX_TOOLCALL-3 중간 검증 ```bash $ go test ./apps/edge/internal/openai -run 'TestChatCompletions(ToolsStreamRetriesMalformedToolCallBeforeChunk|StrictBufferedStreamRetriesMalformedToolCallBeforeChunk|ProviderStreamBlocksMalformedTextToolCall)$' -(output) +ok iop/apps/edge/internal/openai 0.006s ``` ### BUGFIX_TOOLCALL-4 중간 검증 ```bash $ go test ./apps/edge/internal/openai -(output) +ok iop/apps/edge/internal/openai 1.573s ``` ### 최종 검증 ```bash -$ go test ./apps/edge/internal/openai -(output) +$ go test -count=1 -timeout 60s ./apps/edge/internal/openai +ok iop/apps/edge/internal/openai 1.570s ``` ```bash $ ssh toki@toki-labs.com 'cd /Users/toki/agent-work/iop-dev && git status --branch --short && git log --oneline -1 && test -x build/dev-runtime/bin/edge && build/dev-runtime/bin/edge --config build/dev-runtime/edge.yaml config refresh --help >/tmp/iop-tool-boundary-refresh-help.txt && curl -fsS http://127.0.0.1:18001/edges/edge-toki-labs-dev/status >/tmp/iop-tool-boundary-status.json' -(output) +not run: local implementation has not been committed/deployed to `/Users/toki/agent-work/iop-dev` and the dev-runtime binary has not been rebuilt from this change, so this would not be valid current-change evidence. ``` ```bash @@ -152,7 +157,7 @@ s=Path("/tmp/iop-tool-boundary-nonstream.out").read_text() assert "`/`` body를 close tag까지 opaque로 유지, `` literal/Markdown fence/JSON string이 tag로 해석되지 않음을 fixture 테스트로 확인. +- `collectXMLCandidateMatches`/`collectXMLTextToolCallMatches`/`parseTextToolCallBlock`이 모두 `scanXMLTextToolCallBlocks`를 공유함을 확인 (계획 BUGFIX_TOOLCALL-2 충족). +- stream 라우팅: `stream.go:37`에서 `len(req.Tools) > 0`이면 strict flag와 무관하게 `streamBufferedChatCompletion` 진입, `tool_validation.go:49`의 contract가 stream 여부로 꺼지지 않음을 확인 (계획 BUGFIX_TOOLCALL-3 충족). +- buffered 전환의 timeout 의미 변화(라이브 per-event → 전체 실행)는 Node가 `apps/node/internal/node/node.go:147`에서 `TimeoutSec` 전체 실행 제한을 이미 강제하므로 실질 회귀 아님을 확인. +- 제거된 regex 심볼(`textToolCallBlockRE` 등 4종) 잔존 참조 없음 확인. log safety 테스트 PASS. +- 혼합 케이스 probe 테스트로 Suggested 이슈 실동작 확인: prose+`` 혼합 시 content 전체 drop, `{{config}}` 같은 비후보 텍스트만으로도 전체 drop. + +### 발견된 문제 + +- Suggested: `apps/edge/internal/openai/chat_handler.go:383-386` — native tool call과 content가 공존할 때 `streamToolTextFilter{pending: message.Content}.FlushNonCandidate()`가 content 안 어디든 후보 힌트(`` echo 혼합 회귀 테스트를 함께 추가한다. +- Nit: `apps/edge/internal/openai/types.go:477,490,712,1089` — `collectTextToolCallMatches`/`collectXMLTextToolCallMatches`/`collectMustacheTextToolCallMatches`/`parseTextToolCallBlock` 체인은 production/test 호출자가 전혀 없는 dead code다(HEAD에서도 dead였고 이번 변경으로 `parseTextToolCallBlock`이 완전히 고아가 됨). 삭제 후보. +- Nit: `apps/edge/internal/openai/types.go:608` — `close := strings.IndexByte(...)`가 builtin `close`를 shadowing. `closeIdx` 등으로 개명 권장. 같은 맥락에서 `tool_validation.go:49`의 `toolValidationContractForChatRequest`는 이제 인자 하나를 버리는 단순 passthrough라 호출부에서 `toolValidationContractFromRequest` 직접 호출로 정리 가능. + +### 다음 단계 + +- WARN: user-review gate 미트리거(사용자 리뷰 요청 상태=없음, Milestone lock 무관). 후속 active `PLAN-local-G04.md`/`CODE_REVIEW-local-G04.md`(plan=1, tag=REVIEW_BUGFIX_TOOLCALL)를 작성한다. diff --git a/agent-task/archive/2026/07/openai_text_tool_call_boundary/complete.log b/agent-task/archive/2026/07/openai_text_tool_call_boundary/complete.log new file mode 100644 index 0000000..e09df01 --- /dev/null +++ b/agent-task/archive/2026/07/openai_text_tool_call_boundary/complete.log @@ -0,0 +1,38 @@ +# Complete - openai_text_tool_call_boundary + +## 완료 일시 + +2026-07-06 + +## 요약 + +OpenAI-compatible 경계의 text tool-call 처리 하드닝: regex 기반 파서를 scanner 공유 구조로 재작성하고 tools stream을 buffered 검증 경로로 라우팅한 1차 루프(WARN) 후, native tool call 혼합 content의 선행 prose 보존 회귀 수정과 Nit 정리(2차 루프)로 종결. 루프 2회, 최종 판정 PASS. + +## 루프 이력 + +| Plan | Review | Verdict | 메모 | +|------|--------|---------|------| +| `plan_local_G06_0.log` | `code_review_local_G06_0.log` | WARN | scanner 재작성/buffered 라우팅은 양호. Suggested 1건: native tool call+content 공존 시 `FlushNonCandidate()` 단독 사용으로 선행 prose까지 유실. Nit 2건: dead text tool-call match 체인, `close` shadowing/passthrough wrapper. | +| `plan_local_G04_1.log` | `code_review_local_G04_1.log` | PASS | `collectChatCompletionOutput`의 content 정리를 `Append` 기반으로 교체해 live stream 경로와 동일 의미로 선행 prose 보존, raw 후보 무누출. 회귀 테스트 2개 추가. Nit 정리(dead 체인 삭제, `closeIdx` 개명, `toolValidationContractForChatRequest` wrapper 제거) 완료. | + +## 구현/정리 내용 + +- `apps/edge/internal/openai/chat_handler.go`: native tool call과 content 혼합 시 `streamToolTextFilter{}.Append()`로 안전 prefix를 먼저 방출한 뒤 잔여만 `FlushNonCandidate()`로 제거해 선행 prose를 보존. `toolValidationContractFromRequest(req)` 직접 호출로 전환. +- `apps/edge/internal/openai/tool_validation.go`: stream 분기 없는 passthrough가 된 `toolValidationContractForChatRequest` wrapper 삭제(tools stream은 `stream.go:37`에서 전부 buffered 경로로 라우팅되므로 동작 동일). +- `apps/edge/internal/openai/types.go`: 호출자 없는 dead 체인(`collectTextToolCallMatches`, `collectXMLTextToolCallMatches`, `collectMustacheTextToolCallMatches`, `parseTextToolCallBlock`, `textToolCallMatch`, `mustacheTextToolCallBlock`, 미사용 regex 4종, `regexp` import) 삭제. `findXMLOpenTag`의 builtin shadowing 변수 `close`를 `closeIdx`로 개명. +- `apps/edge/internal/openai/server_test.go`: `TestChatCompletionsNativeToolCallPreservesLeadingProse`, `TestChatCompletionsToolsStreamNativeToolCallPreservesLeadingProse` 회귀 테스트 추가(prose 보존, raw/escaped ` + +# Plan - REVIEW_BUGFIX_TOOLCALL + +## 이 파일을 읽는 구현 에이전트에게 + +이 계획은 이전 리뷰 WARN의 후속 구현 루프 입력이다. 구현 후 `CODE_REVIEW-local-G04.md`의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채우고 active 파일을 그대로 둔 뒤 review ready를 보고한다. 최종 판정, log rename, `complete.log`, archive 이동은 code-review-skill 전용이다. + +구현 중 직접 사용자에게 질문하거나 선택지를 제시하거나 `request_user_input`을 호출하지 않는다. 선택된 Milestone `구현 잠금 > 결정 필요` 항목이 실구현을 막는 경우에만 review stub의 `사용자 리뷰 요청` 섹션을 정확한 근거로 채우고 멈춘다. 환경/secret/service 준비, 일반 범위 조정, 반복 실패, 증거 공백은 사용자 리뷰 요청이 아니라 `검증 결과` 또는 후속 plan으로 남긴다. + +## Archive Evidence Snapshot + +- 이전 루프 archive: + - plan: `agent-task/openai_text_tool_call_boundary/plan_local_G06_0.log` + - review: `agent-task/openai_text_tool_call_boundary/code_review_local_G06_0.log` +- 판정: WARN (Required 0, Suggested 1, Nit 2) +- Suggested 요약: `apps/edge/internal/openai/chat_handler.go:383-386` — native tool call과 content가 공존할 때 `streamToolTextFilter{pending: message.Content}.FlushNonCandidate()`가 content 안 어디든 후보 힌트(` 0 && strings.TrimSpace(message.Content) != "" { + filter := &streamToolTextFilter{pending: message.Content} + message.Content = filter.FlushNonCandidate() +} +``` + +`FlushNonCandidate()`는 pending에 후보 힌트가 있으면 전체를 버린다. 이 호출은 content 전체를 pending으로 넣으므로, 모델이 "설명 prose + raw `` echo"를 내면 prose까지 사라지고, `{{config}}` 같은 비후보 텍스트만 있어도 content가 빈 문자열이 된다. non-stream 경로(`completeChatCompletion`)와 buffered stream 경로 모두 `collectChatCompletionOutput`을 쓰므로 둘 다 영향을 받는다. 변경 전 non-stream은 content를 유지했으므로 이는 새 회귀다. + +### 해결 방법 + +live stream 경로(`stream.go:200-205`)와 같은 의미가 되도록 `Append`로 안전 prefix를 먼저 얻고 잔여만 `FlushNonCandidate()`로 정리한다. + +Before: + +```go +filter := &streamToolTextFilter{pending: message.Content} +message.Content = filter.FlushNonCandidate() +``` + +After: + +```go +filter := &streamToolTextFilter{} +prefix := filter.Append(message.Content) +message.Content = prefix + filter.FlushNonCandidate() +``` + +`Append`는 첫 후보 힌트 직전까지(오른쪽 공백 trim) 반환하고 잔여를 pending에 남긴다. 잔여가 후보로 시작하면 `FlushNonCandidate()`는 빈 문자열을 반환하므로 raw 후보 fragment는 여전히 새지 않는다. + +### 수정 파일 및 체크리스트 + +- [ ] `apps/edge/internal/openai/chat_handler.go`: `collectChatCompletionOutput`의 native tool call content 정리를 Append 기반으로 교체. +- [ ] `apps/edge/internal/openai/server_test.go`: native tool call + 선행 prose + raw `` echo 혼합 non-stream 회귀 테스트 추가. +- [ ] `apps/edge/internal/openai/server_test.go`: 같은 fixture의 stream(buffered) variant 추가 또는 기존 stream 테스트 확장. + +### 테스트 작성 + +작성한다. Test names: + +- `TestChatCompletionsNativeToolCallPreservesLeadingProse` +- `TestChatCompletionsToolsStreamNativeToolCallPreservesLeadingProse` + +요구: + +- fake run이 `delta`로 "선행 prose\n\n..." content를 주고 `complete` metadata의 `openai_tool_calls`에 valid native tool call을 담는다. +- 응답 content에 선행 prose가 남고, `" apps/ packages/`로 호출자 부재를 재확인하고, 삭제 후 `go build`/`go test`로 확인한다. `textToolCallMatch` 타입 등 공유 타입이 다른 곳에서 쓰이면 타입은 남긴다. 확인 결과 일부 심볼이 실제로 쓰이고 있으면 해당 심볼은 삭제하지 않고 `계획 대비 변경 사항`에 기록한다. + +### 수정 파일 및 체크리스트 + +- [ ] `apps/edge/internal/openai/types.go`: `close` 개명, dead 체인 삭제. +- [ ] `apps/edge/internal/openai/tool_validation.go`: passthrough wrapper 정리. +- [ ] `apps/edge/internal/openai/chat_handler.go`: 호출부 갱신. + +### 테스트 작성 + +새 테스트는 만들지 않는다. 기존 패키지 테스트가 회귀 검증이다. + +### 중간 검증 + +```bash +go build ./apps/edge/... && go test ./apps/edge/internal/openai +``` + +Expected: build OK, PASS. (`go vet`의 `input_estimator.go:95` 경고는 기존 이슈로 이번 범위가 아니다.) + +## 수정 파일 요약 + +| 파일 | 항목 | +|---|---| +| `apps/edge/internal/openai/chat_handler.go` | REVIEW_BUGFIX_TOOLCALL-1, REVIEW_BUGFIX_TOOLCALL-2 | +| `apps/edge/internal/openai/server_test.go` | REVIEW_BUGFIX_TOOLCALL-1 | +| `apps/edge/internal/openai/types.go` | REVIEW_BUGFIX_TOOLCALL-2 | +| `apps/edge/internal/openai/tool_validation.go` | REVIEW_BUGFIX_TOOLCALL-2 | + +## 최종 검증 + +```bash +go test -count=1 -timeout 60s ./apps/edge/internal/openai +``` + +Expected: PASS. 신규 테스트 2개가 목록에 나타나고 전체 패키지 테스트가 통과한다. + +모든 코드 변경 완료 후 반드시 `CODE_REVIEW-local-G04.md`의 구현 에이전트 소유 섹션을 채운다. 이 파일 작성이 구현의 마지막 단계다. diff --git a/agent-task/openai_text_tool_call_boundary/PLAN-local-G06.md b/agent-task/archive/2026/07/openai_text_tool_call_boundary/plan_local_G06_0.log similarity index 100% rename from agent-task/openai_text_tool_call_boundary/PLAN-local-G06.md rename to agent-task/archive/2026/07/openai_text_tool_call_boundary/plan_local_G06_0.log diff --git a/agent-task/archive/2026/07/review_followup_alignment/03_nexo_notification_boundary/code_review_local_G05_0.log b/agent-task/archive/2026/07/review_followup_alignment/03_nexo_notification_boundary/code_review_local_G05_0.log new file mode 100644 index 0000000..8d88e8d --- /dev/null +++ b/agent-task/archive/2026/07/review_followup_alignment/03_nexo_notification_boundary/code_review_local_G05_0.log @@ -0,0 +1,110 @@ + +# CODE_REVIEW-local-G05: Nexo 알림 경계 정리 + +## 구현 에이전트 소유 + +### 변경 개요 +IOP Client가 Mattermost 통합의 소유자처럼 동작하지 않도록 경계를 정리했다. IOP는 상위 폴더의 `../nexo`가 제공하는 notification 기능만 사용하며, Mattermost-compatible 인증/등록/서버 통합 책임은 Nexo 쪽 경계로 옮겼다. + +### 상세 변경 + +#### 파일 삭제 +- `apps/client/lib/src/integrations/mattermost/mattermost_auth_service.dart` — 자동 로그인, credential asset 로드, FCM 디바이스 토큰 서버 등록 책임 제거 +- `apps/client/lib/src/integrations/mattermost/` 전체 디렉터리 — 새 경로로 이동 +- `apps/client/test/integrations/mattermost_push_host_integration_test.dart` — 새 테스트로 교체 +- `apps/client/assets/mattermost_credentials.json` — credential asset 참조 제거 + +#### 파일 신규 생성 (기존에서 rename/refactor) +- `apps/client/lib/src/integrations/nexo/nexo_notification_client.dart` — `MattermostPushClient` → `NexoNotificationClient` (동일 추상 인터페이스) +- `apps/client/lib/src/integrations/nexo/nexo_notification_plugin_client.dart` — `MattermostPushPluginClient` → `NexoNotificationPluginClient` +- `apps/client/lib/src/integrations/nexo/nexo_notification_host_integration.dart` — `MattermostPushHostIntegration` → `NexoNotificationHostIntegration`, `_authServiceFactory` 및 `autoLoginAndRegister` 호출 제거 +- `apps/client/test/integrations/nexo_notification_host_integration_test.dart` — 기존 host integration 테스트를 새 네이밍으로 복사 + +#### 파일 수정 +- `apps/client/lib/client_bootstrap.dart` — `initializeMattermost` → `initializeNotifications`, `mattermostHost` → `notificationHost`, `defaultMattermostHost` → `defaultNotificationHost` +- `apps/client/lib/client_home_page.dart` — import, prop 명명, `_showMattermostNotification` → `_showNexoNotification` +- `apps/client/lib/main.dart` — import, `IopClientApp.mattermostHost` → `notificationHost` +- `apps/client/pubspec.yaml` — `http` 의존성 **유지** (아래 참조), Nexo messaging 주석 단순화 +- `apps/client/README.md` — Nexo 알림 통합 설명 섹션 추가 +- `apps/client/test/client_bootstrap_test.dart` — `initializeMattermost` → `initializeNotifications`, fake Nexo client 테스트 추가 +- `apps/client/test/widget_test.dart` — `NexoNotificationHostIntegration` import, notification stream → UI(SnackBar) 연결 검증 테스트 3개 추가 + +### http 의존성 유지 사유 +PLAN은 "불필요해진 `http` dependency 제거 여부 확인"을 체크리스트에 포함하고 있다. 현재 `apps/client/lib/control_plane_status_repository.dart` 및 관련 테스트에서 Control Plane HTTP 호출에 `http` 패키지를 사용한다. IOP의 Mattermost auto-login/credential 소유권 제거와 `http` 의존성 제거는 별개의 책임이므로 `http` 의존성은 유지한다. 이 결정은 코드 리팩토링의 범위를 초과하며, 만약 `http` 의존성 전체 제거가 필요하면 별도 plan으로 처리한다. + +### 검증 출력 + +``` +cd apps/client && flutter test +``` +결과: `38 passed` — 전체 테스트 통과 + +``` +rg --sort path -n 'mattermost_credentials|MattermostAuthService|autoLoginAndRegister' apps/client/lib apps/client/test apps/client/pubspec.yaml +``` +결과: 없음 (exit code 1) + +``` +rg --sort path -n 'Mattermost|mattermost' apps/client/lib apps/client/test +``` +결과: 없음 (exit code 1) + +### 중간 검증 결과 +- `http` 의존성 유지: `lib/control_plane_status_repository.dart` 및 관련 테스트에서 Control Plane HTTP 호출에 `http` 패키지를 사용하므로 제거하지 않음. PLAN의 목적은 Mattermost auto-login/credential 소유권 제거일 뿐 HTTP 전체 사용 무제는 아님. +- Android/Firebase config (`google-services.json`, namespace)는 Mattermost integration ownership과 무관한 Firebase 프로젝트 네임스페이스로 남겨둠. + +### 구현 체크리스트 완료 여부 +- [x] REVIEW_NEXO-1의 Client notification 경계 refactor와 테스트 갱신 완료 +- [x] 중간 검증 명령 실행 — Flutter test PASS, Mattermost 참조 완전히 제거됨 +- [x] CODE_REVIEW-local-G05.md 작성 완료 (이 문서) + +## 코드리뷰 결과 + +- 종합 판정: PASS +- 차원별 평가: + - Correctness: Pass + - Completeness: Pass + - Test coverage: Pass + - API contract: Pass + - Code quality: Pass + - Implementation deviation: Pass + - Verification trust: Pass +- 발견된 문제: 없음 +- 다음 단계: PASS 종결. active plan/review를 로그로 아카이브하고 `complete.log` 작성 후 task directory를 `agent-task/archive/2026/07/review_followup_alignment/03_nexo_notification_boundary/`로 이동한다. + +### 리뷰 검증 +```bash +cd apps/client && dart format --output=none --set-exit-if-changed lib/client_bootstrap.dart lib/client_home_page.dart lib/main.dart lib/src/integrations/nexo/nexo_notification_client.dart lib/src/integrations/nexo/nexo_notification_host_integration.dart lib/src/integrations/nexo/nexo_notification_plugin_client.dart test/client_bootstrap_test.dart test/integrations/nexo_notification_host_integration_test.dart test/widget_test.dart +``` +결과: `Formatted 9 files (0 changed)`. + +```bash +cd apps/client && flutter analyze --no-fatal-infos +``` +결과: exit code 0. 기존 info 2건(`lib/client_home_page.dart:183`, `lib/client_home_page.dart:207`)만 남음. + +```bash +cd apps/client && flutter test +``` +결과: `+38: All tests passed!` + +```bash +rg --sort path -n 'mattermost_credentials|MattermostAuthService|autoLoginAndRegister' apps/client/lib apps/client/test apps/client/pubspec.yaml +rg --sort path -n 'Mattermost|mattermost' apps/client/lib apps/client/test +``` +결과: 둘 다 출력 없음(exit code 1). + +## 코드리뷰 전용 체크리스트 + +- [x] `코드리뷰 결과`에 `PASS`, `WARN`, `FAIL` 중 하나의 판정을 append한다. +- [x] 판정과 `차원별 평가`, Required/Suggested/Nit 분류가 서로 일치한다. +- [x] active `CODE_REVIEW-*-G??.md`를 `code_review_local_G05_0.log`로 아카이브한다. +- [x] active `PLAN-*-G??.md`를 `plan_local_G05_0.log`로 아카이브한다. +- [x] `.gitignore`의 Agent-Ops 관리 block이 `agent-task/**/*.md`와 `agent-task/**/*.log`를 unignore하고 `agent-roadmap/current.md`를 ignore하는지 확인한다. +- [x] PASS이므로 `agent-ops/skills/common/code-review/templates/complete-log-template.md` 기준으로 `complete.log`를 작성하고 active `.md` 파일을 남기지 않는다. +- [x] PASS이므로 active task 디렉터리 `agent-task/review_followup_alignment/03_nexo_notification_boundary/`를 `agent-task/archive/2026/07/review_followup_alignment/03_nexo_notification_boundary/`로 이동하고 최종 archive 경로에서 이 체크리스트를 갱신한다. +- [ ] PASS이고 task group이 `m-`이면 런타임이 읽을 완료 이벤트 메타데이터를 보고하고, roadmap 수정이나 `update-roadmap` 직접 호출을 하지 않는다. 해당 없음: `review_followup_alignment`. +- [x] PASS split 작업이며 이동 후 active parent `agent-task/review_followup_alignment/`에 sibling subtask가 남아 있어 유지함을 확인한다. +- [ ] WARN/FAIL이고 user-review gate가 트리거되지 않았으면 다음 active `PLAN-{build_lane}-GNN.md`와 `CODE_REVIEW-{review_lane}-GNN.md`를 작성하고 `complete.log`를 작성하지 않는다. +- [ ] USER_REVIEW이면 `agent-ops/skills/common/code-review/templates/user-review-template.md` 기준으로 `USER_REVIEW.md`를 작성하고 active `PLAN-*.md`, `CODE_REVIEW-*.md`, `complete.log`를 남기지 않는다. +- [ ] USER_REVIEW가 연결된 Milestone 결정으로 완료/PASS 해소되면 `USER_REVIEW.md`를 해소 상태로 갱신하고 `complete.log`를 작성한 뒤 task directory를 archive로 이동한다. diff --git a/agent-task/archive/2026/07/review_followup_alignment/03_nexo_notification_boundary/complete.log b/agent-task/archive/2026/07/review_followup_alignment/03_nexo_notification_boundary/complete.log new file mode 100644 index 0000000..7c33860 --- /dev/null +++ b/agent-task/archive/2026/07/review_followup_alignment/03_nexo_notification_boundary/complete.log @@ -0,0 +1,38 @@ +# Complete - review_followup_alignment/03_nexo_notification_boundary + +## 완료 일시 + +2026-07-06 + +## 요약 + +Nexo notification boundary refactor review loop 1 completed with PASS. + +## 루프 이력 + +| Plan | Review | Verdict | 메모 | +|------|--------|---------|------| +| `plan_local_G05_0.log` | `code_review_local_G05_0.log` | PASS | IOP Client의 Mattermost-owned auth/credential/device-registration 책임을 제거하고 Nexo notification consumer 경계로 정리한 구현을 승인함. | + +## 구현/정리 내용 + +- `apps/client/lib/src/integrations/mattermost/`의 Mattermost auth/registration owner 코드를 제거하고 `apps/client/lib/src/integrations/nexo/`의 notification host/client 경계로 대체했다. +- bootstrap/app/home page wiring을 `initializeNotifications` / `notificationHost` 명명으로 정리하고 notification stream을 UI SnackBar로 연결했다. +- client bootstrap, host integration, widget notification stream 테스트를 Nexo notification 경계 기준으로 갱신했다. +- 리뷰 중 formatter를 적용하고 `client_bootstrap_test.dart`의 disabled-notification assertion을 fake host 기준으로 보강했다. + +## 최종 검증 + +- `cd apps/client && dart format --output=none --set-exit-if-changed lib/client_bootstrap.dart lib/client_home_page.dart lib/main.dart lib/src/integrations/nexo/nexo_notification_client.dart lib/src/integrations/nexo/nexo_notification_host_integration.dart lib/src/integrations/nexo/nexo_notification_plugin_client.dart test/client_bootstrap_test.dart test/integrations/nexo_notification_host_integration_test.dart test/widget_test.dart` - PASS; `Formatted 9 files (0 changed)`. +- `cd apps/client && flutter analyze --no-fatal-infos` - PASS; exit code 0, 기존 info 2건(`lib/client_home_page.dart:183`, `lib/client_home_page.dart:207`)만 보고됨. +- `cd apps/client && flutter test` - PASS; `+38: All tests passed!`. +- `rg --sort path -n 'mattermost_credentials|MattermostAuthService|autoLoginAndRegister' apps/client/lib apps/client/test apps/client/pubspec.yaml` - PASS; 출력 없음(exit code 1). +- `rg --sort path -n 'Mattermost|mattermost' apps/client/lib apps/client/test` - PASS; 출력 없음(exit code 1). + +## 잔여 Nit + +- 없음 + +## 후속 작업 + +- 없음 diff --git a/agent-task/review_followup_alignment/03_nexo_notification_boundary/PLAN-local-G05.md b/agent-task/archive/2026/07/review_followup_alignment/03_nexo_notification_boundary/plan_local_G05_0.log similarity index 100% rename from agent-task/review_followup_alignment/03_nexo_notification_boundary/PLAN-local-G05.md rename to agent-task/archive/2026/07/review_followup_alignment/03_nexo_notification_boundary/plan_local_G05_0.log diff --git a/agent-task/review_followup_alignment/03_nexo_notification_boundary/CODE_REVIEW-local-G05.md b/agent-task/review_followup_alignment/03_nexo_notification_boundary/CODE_REVIEW-local-G05.md deleted file mode 100644 index 771e1cf..0000000 --- a/agent-task/review_followup_alignment/03_nexo_notification_boundary/CODE_REVIEW-local-G05.md +++ /dev/null @@ -1,77 +0,0 @@ - -# CODE_REVIEW-local-G05: Nexo 알림 경계 정리 -date=2026-07-06 - -## 상태 -구현 대기 - -## 이 파일을 읽는 구현 에이전트에게 -구현 후 이 파일의 구현 에이전트 소유 섹션을 실제 변경 내용, 계획 대비 변경, 검증 출력으로 채운다. active `PLAN-*.md`와 `CODE_REVIEW-*.md`는 남겨두고 review 준비 상태를 보고한다. 직접 사용자에게 질문하거나 `USER_REVIEW.md`를 만들거나 archive/complete 처리를 하지 않는다. - -## 구현 체크리스트 -- [ ] REVIEW_NEXO-1의 Client notification 경계 refactor와 테스트 갱신을 수행한다. -- [ ] 중간 검증 명령을 실행하고 Flutter 실행 불가 시 환경 사유와 대체 확인 결과를 `CODE_REVIEW-local-G05.md`에 기록한다. -- [ ] CODE_REVIEW-*-G??.md의 구현 에이전트 소유 섹션을 실제 구현 내용과 검증 출력으로 채운다. 이 항목이 완료되기 전에는 구현이 완료된 것이 아니다. - -## 코드리뷰 전용 체크리스트 - -> **[REVIEW AGENT ONLY]** 이 체크리스트는 코드리뷰 에이전트만 사용한다. -> 구현 에이전트는 이 섹션을 수정하거나 체크하지 않는다. - -- [ ] `코드리뷰 결과`에 `PASS`, `WARN`, `FAIL` 중 하나의 판정을 append한다. -- [ ] 판정과 Required/Suggested/Nit 분류가 서로 일치한다. -- [ ] active `CODE_REVIEW-*-G??.md`를 archive 로그로 이동한다. -- [ ] active `PLAN-*-G??.md`를 archive 로그로 이동한다. -- [ ] `.gitignore`의 Agent-Ops 관리 block을 확인한다. -- [ ] PASS이면 `complete.log`를 작성하고 active `.md` 파일을 남기지 않는다. -- [ ] PASS split 작업이면 이동 후 빈 active parent 정리 여부를 확인한다. - -## 계획 대비 변경 사항 -_구현 에이전트가 계획과 다르게 구현한 부분을 이유와 함께 기록한다._ - -## 주요 설계 결정 -_구현 에이전트가 주요 설계 결정 사항을 기록한다._ - -## 사용자 리뷰 요청 - -_기본값은 `없음`이다. 구현 중 새 결정이 필요해 보여도 직접 질문하거나 선택지를 제시하거나 `request_user_input`을 호출하지 않는다. 이 섹션은 선택된 Milestone `구현 잠금 > 결정 필요` 항목이 실구현을 차단할 때만 채운다. 외부 환경/secret/서비스 준비, 검증 증거 공백, 반복 실패, 일반 범위 조정은 사용자 리뷰 요청이 아니며 `검증 결과`, `계획 대비 변경 사항`, 또는 code-review의 일반 follow-up plan으로 처리한다._ - -- 상태: 없음 -- 사유 유형: 없음 -- 연결 대상: 없음 -- 결정 필요: 없음 -- 차단 근거: 없음 -- 실행한 검증/명령: 없음 -- 자동 후속 불가 이유: 없음 -- 재개 조건: 없음 - -## 리뷰어를 위한 체크포인트 -- IOP Client가 Mattermost auth/register owner처럼 동작하지 않는가. -- Nexo notification plugin 소비 경계만 남았는가. -- credential asset과 Mattermost auto-login 참조가 제거되었는가. -- Flutter test 또는 실행 불가 사유가 충분히 기록되었는가. - -## 검증 결과 -_구현 에이전트가 각 중간 검증 및 최종 검증 명령 실행 후 출력을 여기에 붙여 넣는다._ - -필수 규칙: -- 검증 명령은 고정된 계약이다. 임의로 대체하지 않는다. -- 대체가 필요하면 `계획 대비 변경 사항`에 이유와 대체 명령을 기록한다. -- `검증 결과`에는 실제 stdout/stderr를 붙여 넣는다. - -### REVIEW_NEXO-1 중간 검증 -```bash -cd apps/client && flutter test -rg --sort path -n 'mattermost_credentials|MattermostAuthService|autoLoginAndRegister' apps/client/lib apps/client/test apps/client/pubspec.yaml -rg --sort path -n 'Mattermost|mattermost' apps/client/lib apps/client/test -``` - -### 최종 검증 -```bash -cd apps/client && flutter test -rg --sort path -n 'mattermost_credentials|MattermostAuthService|autoLoginAndRegister' apps/client/lib apps/client/test apps/client/pubspec.yaml -rg --sort path -n 'Mattermost|mattermost' apps/client/lib apps/client/test -``` - -## 코드리뷰 결과 -_리뷰 에이전트가 append한다._ diff --git a/apps/client/README.md b/apps/client/README.md index 8a11e38..100aebf 100644 --- a/apps/client/README.md +++ b/apps/client/README.md @@ -2,6 +2,10 @@ IOP(Inference Operations Platform)의 공식 Client UI 애플리케이션입니다. Flutter를 사용하여 다중 플랫폼(Web, Desktop, Mobile)을 단일 코드베이스로 지원합니다. +## Nexo 알림 통합 + +이 client는 Nexo messaging plugin(`nexo_messaging`)을 통해 push notification을 구독합니다. Client는 상위 폴더의 `../nexo`가 제공하는 notification 기능만 사용하며, Mattermost-compatible 인증/등록/서버 통합 책임은 Nexo 쪽 경계에서 담당합니다. Client는 `NexoNotificationHostIntegration`을 통해 notification stream을 구독하고 UI에 표시됩니다. + ## Agent Shell 구성 이 client는 IOP-owned Flutter package인 `packages/flutter/iop_console`을 path dependency로 사용합니다. `iop_console`은 workspace sibling package인 `agent_shell`을 사용해 `IopConsoleShell`과 `IopAgentPanel`을 제공하며, IOP 운영/유지보수 agent 화면을 단독 IOP 앱과 외부 임베딩 소비자에서 같은 위젯 경계로 조립하기 위한 시작점입니다. diff --git a/apps/client/lib/client_bootstrap.dart b/apps/client/lib/client_bootstrap.dart index ee2d898..956a69a 100644 --- a/apps/client/lib/client_bootstrap.dart +++ b/apps/client/lib/client_bootstrap.dart @@ -2,23 +2,23 @@ import 'package:firebase_core/firebase_core.dart'; import 'package:flutter/foundation.dart'; import 'package:flutter/material.dart'; import 'main.dart'; -import 'src/integrations/mattermost/mattermost_push_host_integration.dart'; -import 'src/integrations/mattermost/mattermost_push_plugin_client.dart'; +import 'src/integrations/nexo/nexo_notification_host_integration.dart'; +import 'src/integrations/nexo/nexo_notification_plugin_client.dart'; class IopClientBootstrapOptions { final bool initializeFirebase; - final bool initializeMattermost; - final MattermostPushHostIntegration? mattermostHost; + final bool initializeNotifications; + final NexoNotificationHostIntegration? notificationHost; const IopClientBootstrapOptions({ this.initializeFirebase = true, - this.initializeMattermost = true, - this.mattermostHost, + this.initializeNotifications = true, + this.notificationHost, }); } -final MattermostPushHostIntegration defaultMattermostHost = - MattermostPushHostIntegration(pushClient: MattermostPushPluginClient()); +final NexoNotificationHostIntegration defaultNotificationHost = + NexoNotificationHostIntegration(pushClient: NexoNotificationPluginClient()); Future bootstrapIopClient({ IopClientBootstrapOptions options = const IopClientBootstrapOptions(), @@ -28,8 +28,8 @@ Future bootstrapIopClient({ if (initializeExternalPush && options.initializeFirebase) { await Firebase.initializeApp(); } - if (initializeExternalPush && options.initializeMattermost) { - final host = options.mattermostHost ?? defaultMattermostHost; + if (initializeExternalPush && options.initializeNotifications) { + final host = options.notificationHost ?? defaultNotificationHost; await host.initialize(); } } @@ -42,8 +42,9 @@ Future runIopClient({ final initializeExternalPush = !kIsWeb; runApp( IopClientApp( - mattermostHost: initializeExternalPush && options.initializeMattermost - ? (options.mattermostHost ?? defaultMattermostHost) + notificationHost: + initializeExternalPush && options.initializeNotifications + ? (options.notificationHost ?? defaultNotificationHost) : null, ), ); diff --git a/apps/client/lib/client_home_page.dart b/apps/client/lib/client_home_page.dart index e4058a5..8aab37d 100644 --- a/apps/client/lib/client_home_page.dart +++ b/apps/client/lib/client_home_page.dart @@ -9,17 +9,17 @@ import 'control_plane_status_controller.dart'; import 'control_plane_status_widgets.dart'; import 'iop_wire/client_wire_client.dart'; import 'main.dart'; -import 'src/integrations/mattermost/mattermost_push_host_integration.dart'; +import 'src/integrations/nexo/nexo_notification_host_integration.dart'; class ClientHomePage extends StatefulWidget { final ClientWireClient? testClient; - final MattermostPushHostIntegration? mattermostHost; + final NexoNotificationHostIntegration? notificationHost; final ControlPlaneStatusRepository? statusRepository; const ClientHomePage({ super.key, this.testClient, - this.mattermostHost, + this.notificationHost, this.statusRepository, }); @@ -53,10 +53,10 @@ class _ClientHomePageState extends State { _connectAndFetch(); }); - final host = widget.mattermostHost; + final host = widget.notificationHost; if (host != null) { _notificationSubscription = host.onNotification.listen( - _showMattermostNotification, + _showNexoNotification, ); } } @@ -70,7 +70,7 @@ class _ClientHomePageState extends State { super.dispose(); } - void _showMattermostNotification(Map data) { + void _showNexoNotification(Map data) { if (data['type'] != 'message' || !mounted) return; final message = data['message'] as String? ?? ''; diff --git a/apps/client/lib/main.dart b/apps/client/lib/main.dart index 63f72ec..7b7623e 100644 --- a/apps/client/lib/main.dart +++ b/apps/client/lib/main.dart @@ -6,7 +6,7 @@ import 'client_bootstrap.dart'; import 'client_home_page.dart'; import 'control_plane_status_client.dart'; import 'iop_wire/client_wire_client.dart'; -import 'src/integrations/mattermost/mattermost_push_host_integration.dart'; +import 'src/integrations/nexo/nexo_notification_host_integration.dart'; const iopDefaultCapabilityPack = IopCapabilityPack( capabilities: [ @@ -48,13 +48,13 @@ Future applyFullscreenMode() async { class IopClientApp extends StatelessWidget { final ClientWireClient? testClient; - final MattermostPushHostIntegration? mattermostHost; + final NexoNotificationHostIntegration? notificationHost; final ControlPlaneStatusRepository? statusRepository; const IopClientApp({ super.key, this.testClient, - this.mattermostHost, + this.notificationHost, this.statusRepository, }); @@ -74,7 +74,7 @@ class IopClientApp extends StatelessWidget { ), home: ClientHomePage( testClient: testClient, - mattermostHost: mattermostHost, + notificationHost: notificationHost, statusRepository: statusRepository, ), ); diff --git a/apps/client/lib/src/integrations/mattermost/mattermost_auth_service.dart b/apps/client/lib/src/integrations/mattermost/mattermost_auth_service.dart deleted file mode 100644 index f25cbe9..0000000 --- a/apps/client/lib/src/integrations/mattermost/mattermost_auth_service.dart +++ /dev/null @@ -1,161 +0,0 @@ -import 'dart:convert'; -import 'package:flutter/foundation.dart' show FlutterError; -import 'package:flutter/services.dart' show rootBundle; -import 'package:http/http.dart' as http; - -import 'mattermost_push_client.dart'; - -/// Mattermost 자동 로그인 + FCM 디바이스 토큰 서버 등록 서비스. -class MattermostAuthService { - final MattermostPushClient _pushService; - - String? _serverUrl; - String? _serverIdentifier; - String? _authToken; - String? _userId; - String? _sessionId; - - String? get serverUrl => _serverUrl; - bool get isLoggedIn => _authToken != null; - - MattermostAuthService(this._pushService); - - /// assets/mattermost_credentials.json 로드 → 로그인 → FCM 등록까지 자동 수행. - Future autoLoginAndRegister() async { - final creds = await _loadCredentials(); - if (creds == null) { - print( - '[MattermostAuth] Credentials asset not found, skipping auto login.', - ); - return; - } - _serverUrl = creds['serverUrl']!; - _serverIdentifier = creds['serverId']; - - print('[MattermostAuth] Logging in to $_serverUrl ...'); - await _login(creds['loginId']!, creds['password']!); - - // FCM 토큰이 이미 있으면 바로 등록, 없으면 콜백으로 대기 - final existingToken = await _pushService.getDeviceToken(); - if (existingToken != null && existingToken.isNotEmpty) { - print('[MattermostAuth] FCM token already available, registering ...'); - await _registerDeviceToken(existingToken); - } else { - print( - '[MattermostAuth] FCM token not ready yet, waiting for callback ...', - ); - } - - // FCM 토큰 발급/갱신 시 자동으로 서버에 등록 - _pushService.onDeviceTokenReady = (deviceToken) async { - print('[MattermostAuth] FCM token ready, registering with server ...'); - await _registerDeviceToken(deviceToken); - }; - - print('[MattermostAuth] Auto login & FCM registration complete.'); - } - - Future?> _loadCredentials() async { - final String jsonStr; - try { - jsonStr = await rootBundle.loadString( - 'assets/mattermost_credentials.json', - ); - } on FlutterError { - return null; - } - final map = json.decode(jsonStr) as Map; - final serverId = (map['serverId'] ?? map['serverIdentifier']) as String?; - return { - 'serverUrl': map['serverUrl'] as String, - 'loginId': map['loginId'] as String, - 'password': map['password'] as String, - if (serverId != null && serverId.isNotEmpty) 'serverId': serverId, - }; - } - - /// POST /api/v4/users/login → Token 헤더에서 인증 토큰 추출. - Future _login(String loginId, String password) async { - final url = Uri.parse('$_serverUrl/api/v4/users/login'); - final response = await http.post( - url, - headers: {'Content-Type': 'application/json'}, - body: json.encode({'login_id': loginId, 'password': password}), - ); - - if (response.statusCode != 200) { - throw Exception( - '[MattermostAuth] Login failed (${response.statusCode}): ${response.body}', - ); - } - - _authToken = response.headers['token']; - if (_authToken == null) { - throw Exception('[MattermostAuth] Login response missing Token header'); - } - - final body = json.decode(response.body) as Map; - _userId = body['id'] as String?; - _sessionId = body['session_id'] as String? ?? body['id'] as String?; - - print('[MattermostAuth] Login OK - userId=$_userId sessionId=$_sessionId'); - - // 네이티브 측에 인증 토큰 저장 (ACK, 답장 등에 사용) - await _pushService.setAuthToken( - _serverUrl!, - _authToken!, - identifier: _serverIdentifier, - ); - - // 서버 config에서 signing key 가져와 네이티브에 저장 - await _fetchAndStoreSigningKey(); - } - - /// GET /api/v4/config/client?format=old → AsymmetricSigningPublicKey를 네이티브에 저장. - Future _fetchAndStoreSigningKey() async { - try { - final url = Uri.parse('$_serverUrl/api/v4/config/client?format=old'); - final response = await http.get( - url, - headers: {'Authorization': 'Bearer $_authToken'}, - ); - if (response.statusCode != 200) { - print( - '[MattermostAuth] Failed to fetch config (${response.statusCode})', - ); - return; - } - final config = json.decode(response.body) as Map; - final signingKey = config['AsymmetricSigningPublicKey'] as String?; - if (signingKey != null && signingKey.isNotEmpty) { - await _pushService.setSigningKey(_serverUrl!, signingKey); - print('[MattermostAuth] Signing key stored.'); - } else { - print('[MattermostAuth] No signing key in server config.'); - } - } catch (e) { - print('[MattermostAuth] Failed to fetch signing key: $e'); - } - } - - /// PUT /api/v4/users/sessions/device → device_id 등록. - Future _registerDeviceToken(String deviceToken) async { - final url = Uri.parse('$_serverUrl/api/v4/users/sessions/device'); - final response = await http.put( - url, - headers: { - 'Content-Type': 'application/json', - 'Authorization': 'Bearer $_authToken', - }, - body: json.encode({'device_id': deviceToken}), - ); - - if (response.statusCode == 200) { - print('[MattermostAuth] FCM device token registered successfully.'); - } else { - print( - '[MattermostAuth] FCM registration failed (${response.statusCode}): ${response.body}', - ); - } - } -} diff --git a/apps/client/lib/src/integrations/mattermost/mattermost_push_client.dart b/apps/client/lib/src/integrations/nexo/nexo_notification_client.dart similarity index 87% rename from apps/client/lib/src/integrations/mattermost/mattermost_push_client.dart rename to apps/client/lib/src/integrations/nexo/nexo_notification_client.dart index d160645..71dc289 100644 --- a/apps/client/lib/src/integrations/mattermost/mattermost_push_client.dart +++ b/apps/client/lib/src/integrations/nexo/nexo_notification_client.dart @@ -1,9 +1,9 @@ -/// Host-facing interface for the Mattermost push plugin. +/// Host-facing interface for the Nexo notification plugin. /// /// All production access to the platform plugin singleton is routed through /// implementations of this interface. The host integration depends on this /// abstraction so tests can inject fakes without booting Firebase/FCM. -abstract interface class MattermostPushClient { +abstract interface class NexoNotificationClient { Stream> get onNotification; Future initialize(); diff --git a/apps/client/lib/src/integrations/mattermost/mattermost_push_host_integration.dart b/apps/client/lib/src/integrations/nexo/nexo_notification_host_integration.dart similarity index 50% rename from apps/client/lib/src/integrations/mattermost/mattermost_push_host_integration.dart rename to apps/client/lib/src/integrations/nexo/nexo_notification_host_integration.dart index a795b5b..1e81e1c 100644 --- a/apps/client/lib/src/integrations/mattermost/mattermost_push_host_integration.dart +++ b/apps/client/lib/src/integrations/nexo/nexo_notification_host_integration.dart @@ -1,37 +1,26 @@ import 'package:flutter/foundation.dart' show debugPrint; -import 'mattermost_auth_service.dart'; -import 'mattermost_push_client.dart'; +import 'nexo_notification_client.dart'; -/// Owns the Mattermost host/plugin responsibility boundary. +/// Owns the Nexo notification host/plugin responsibility boundary. /// /// The integration: /// - initializes the push client exactly once, -/// - performs auto-login best-effort (skips silently when credentials are -/// unavailable, so app boot is not blocked), /// - registers navigation callbacks in one place (not from a widget build), /// - exposes the notification stream for app-level consumers. -class MattermostPushHostIntegration { - final MattermostPushClient pushClient; - final MattermostAuthService Function(MattermostPushClient client) - _authServiceFactory; +class NexoNotificationHostIntegration { + final NexoNotificationClient pushClient; bool _initialized = false; - MattermostPushHostIntegration({ - required this.pushClient, - MattermostAuthService Function(MattermostPushClient client)? - authServiceFactory, - }) : _authServiceFactory = - authServiceFactory ?? - ((MattermostPushClient client) => MattermostAuthService(client)); + NexoNotificationHostIntegration({required this.pushClient}); Stream> get onNotification => pushClient.onNotification; bool get isInitialized => _initialized; - /// Initialize plugin, perform auth handoff, and register navigation - /// callbacks. Safe to call once at bootstrap; subsequent calls are no-ops. + /// Initialize plugin, and register navigation callbacks. + /// Safe to call once at bootstrap; subsequent calls are no-ops. /// /// [onNavigateToChannel] / [onNavigateToThread] are optional. When omitted, /// the integration installs debug-logging fallbacks so the navigation path @@ -45,25 +34,18 @@ class MattermostPushHostIntegration { await pushClient.initialize(); - final authService = _authServiceFactory(pushClient); - try { - await authService.autoLoginAndRegister(); - } catch (e) { - debugPrint('[MattermostHost] Mattermost auto-login failed: $e'); - } - pushClient.onNavigateToChannel = onNavigateToChannel ?? (serverUrl, channelId) { debugPrint( - '[MattermostHost] Navigate to channel: $channelId on $serverUrl', + '[NexoNotification] Navigate to channel: $channelId on $serverUrl', ); }; pushClient.onNavigateToThread = onNavigateToThread ?? (serverUrl, rootId) { debugPrint( - '[MattermostHost] Navigate to thread: $rootId on $serverUrl', + '[NexoNotification] Navigate to thread: $rootId on $serverUrl', ); }; } diff --git a/apps/client/lib/src/integrations/mattermost/mattermost_push_plugin_client.dart b/apps/client/lib/src/integrations/nexo/nexo_notification_plugin_client.dart similarity index 87% rename from apps/client/lib/src/integrations/mattermost/mattermost_push_plugin_client.dart rename to apps/client/lib/src/integrations/nexo/nexo_notification_plugin_client.dart index f600f7d..bbb05c0 100644 --- a/apps/client/lib/src/integrations/mattermost/mattermost_push_plugin_client.dart +++ b/apps/client/lib/src/integrations/nexo/nexo_notification_plugin_client.dart @@ -2,17 +2,17 @@ import 'dart:async'; import 'package:nexo_messaging/nexo_messaging.dart'; -import 'mattermost_push_client.dart'; +import 'nexo_notification_client.dart'; /// Production adapter wrapping the platform-channel singleton. /// /// This is the ONLY file in production that should reference /// `NexoMessagingPlugin.instance`. Everything else depends on -/// [MattermostPushClient]. -class MattermostPushPluginClient implements MattermostPushClient { +/// [NexoNotificationClient]. +class NexoNotificationPluginClient implements NexoNotificationClient { final NexoMessagingPlugin _plugin; - MattermostPushPluginClient({NexoMessagingPlugin? plugin}) + NexoNotificationPluginClient({NexoMessagingPlugin? plugin}) : _plugin = plugin ?? NexoMessagingPlugin.instance; @override diff --git a/apps/client/pubspec.yaml b/apps/client/pubspec.yaml index 1f14083..c659453 100644 --- a/apps/client/pubspec.yaml +++ b/apps/client/pubspec.yaml @@ -38,7 +38,7 @@ dependencies: # Firebase firebase_core: ^3.13.0 - # Nexo messaging / Mattermost-compatible push plugin + # Nexo messaging push plugin nexo_messaging: path: ../../../nexo/packages/messaging_flutter @@ -83,8 +83,7 @@ flutter: uses-material-design: true assets: - # Local Mattermost smoke credentials live at assets/mattermost_credentials.json - # and are ignored by apps/client/.gitignore. + # Local assets can be placed here (e.g. nexo notification credentials). - assets/ # To add assets to your application, add an assets section, like this: diff --git a/apps/client/test/client_bootstrap_test.dart b/apps/client/test/client_bootstrap_test.dart index d0a1841..fd805d4 100644 --- a/apps/client/test/client_bootstrap_test.dart +++ b/apps/client/test/client_bootstrap_test.dart @@ -1,27 +1,147 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:iop_client/client_bootstrap.dart'; import 'package:iop_client/main.dart'; +import 'package:iop_client/src/integrations/nexo/nexo_notification_client.dart'; +import 'package:iop_client/src/integrations/nexo/nexo_notification_host_integration.dart'; import 'widget_test.dart'; +/// Fake NexoNotificationClient that does not reach out to Firebase/FCM. +/// Includes `initializedForTest` for test verification. +class _TestFakeNexoClient implements NexoNotificationClient { + bool _initialized = false; + + bool get initializedForTest => _initialized; + + @override + Stream> get onNotification => + const Stream>.empty(); + + @override + Future initialize() async { + _initialized = true; + } + + @override + Future getDeviceToken() async => null; + + @override + Future setAuthToken( + String serverUrl, + String token, { + String? identifier, + }) async {} + + @override + Future setSigningKey(String serverUrl, String signingKey) async {} + + @override + set onDeviceTokenReady(Future Function(String token)? callback) {} + + @override + set onNavigateToChannel( + void Function(String serverUrl, String channelId)? callback, + ) {} + + @override + set onNavigateToThread( + void Function(String serverUrl, String rootId)? callback, + ) {} +} + void main() { - testWidgets('runIopClient can skip external integrations', (WidgetTester tester) async { + testWidgets('runIopClient can skip external integrations', ( + WidgetTester tester, + ) async { const options = IopClientBootstrapOptions( initializeFirebase: false, - initializeMattermost: false, + initializeNotifications: false, ); - // This should run without throwing any Firebase or Mattermost exceptions + // This should run without any exceptions await bootstrapIopClient(options: options); final fakeClient = FakeClientWireClient(shouldSuccess: true); // Verify it doesn't fail basic widget pump await tester.pumpWidget( - IopClientApp( - testClient: fakeClient, - statusRepository: null, - ), + IopClientApp(testClient: fakeClient, statusRepository: null), ); expect(find.byType(IopClientApp), findsOneWidget); }); + + testWidgets('bootstrap initializes Nexo notification host by default', ( + WidgetTester tester, + ) async { + // Default initializeNotifications = true, but with fake host to avoid + // Firebase/FCM plugin calls in test environment. + final fakeHost = NexoNotificationHostIntegration( + pushClient: _TestFakeNexoClient(), + ); + + final testOptions = IopClientBootstrapOptions( + initializeFirebase: false, + notificationHost: fakeHost, + ); + + await bootstrapIopClient(options: testOptions); + + // Verify the provided host is initialized + expect(fakeHost.isInitialized, isTrue); + }); + + testWidgets('bootstrap skips notification when disabled', ( + WidgetTester tester, + ) async { + final fakeHost = NexoNotificationHostIntegration( + pushClient: _TestFakeNexoClient(), + ); + const options = IopClientBootstrapOptions( + initializeFirebase: false, + initializeNotifications: false, + ); + + await bootstrapIopClient( + options: IopClientBootstrapOptions( + initializeFirebase: options.initializeFirebase, + initializeNotifications: options.initializeNotifications, + notificationHost: fakeHost, + ), + ); + + expect(fakeHost.isInitialized, isFalse); + }); + + testWidgets('bootstrap accepts custom NexoNotificationHostIntegration', ( + WidgetTester tester, + ) async { + final customHost = NexoNotificationHostIntegration( + pushClient: _TestFakeNexoClient(), + ); + + final testOptions = IopClientBootstrapOptions( + initializeFirebase: false, + notificationHost: customHost, + ); + + await bootstrapIopClient(options: testOptions); + + expect(customHost.isInitialized, isTrue); + }); + + testWidgets('bootstrap initializes with injected host without requiring ' + 'auth login or credential assets', (WidgetTester tester) async { + final fakeClient = _TestFakeNexoClient(); + final fakeHost = NexoNotificationHostIntegration(pushClient: fakeClient); + + final testOptions = IopClientBootstrapOptions( + initializeFirebase: false, + notificationHost: fakeHost, + ); + + // This must succeed without looking for external push credentials. + await bootstrapIopClient(options: testOptions); + + expect(fakeHost.isInitialized, isTrue); + expect(fakeClient.initializedForTest, isTrue); + }); } diff --git a/apps/client/test/integrations/mattermost_push_host_integration_test.dart b/apps/client/test/integrations/nexo_notification_host_integration_test.dart similarity index 66% rename from apps/client/test/integrations/mattermost_push_host_integration_test.dart rename to apps/client/test/integrations/nexo_notification_host_integration_test.dart index 393405a..2e0a4c3 100644 --- a/apps/client/test/integrations/mattermost_push_host_integration_test.dart +++ b/apps/client/test/integrations/nexo_notification_host_integration_test.dart @@ -1,11 +1,10 @@ import 'dart:async'; import 'package:flutter_test/flutter_test.dart'; -import 'package:iop_client/src/integrations/mattermost/mattermost_auth_service.dart'; -import 'package:iop_client/src/integrations/mattermost/mattermost_push_client.dart'; -import 'package:iop_client/src/integrations/mattermost/mattermost_push_host_integration.dart'; +import 'package:iop_client/src/integrations/nexo/nexo_notification_client.dart'; +import 'package:iop_client/src/integrations/nexo/nexo_notification_host_integration.dart'; -class _FakePushClient implements MattermostPushClient { +class _FakePushClient implements NexoNotificationClient { int initializeCalls = 0; int navigateChannelAssignments = 0; int navigateThreadAssignments = 0; @@ -69,67 +68,25 @@ class _FakePushClient implements MattermostPushClient { } } -class _NoopAuthService implements MattermostAuthService { - int autoLoginCalls = 0; - final bool throwOnLogin; - - _NoopAuthService({this.throwOnLogin = false}); - - @override - Future autoLoginAndRegister() async { - autoLoginCalls += 1; - if (throwOnLogin) { - throw StateError('credentials missing'); - } - } - - @override - noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation); -} - void main() { test('initialize calls push client init exactly once', () async { final push = _FakePushClient(); - final auth = _NoopAuthService(); - final host = MattermostPushHostIntegration( - pushClient: push, - authServiceFactory: (_) => auth, - ); + final host = NexoNotificationHostIntegration(pushClient: push); await host.initialize(); await host.initialize(); expect(push.initializeCalls, equals(1)); - expect(auth.autoLoginCalls, equals(1)); expect(host.isInitialized, isTrue); await push.dispose(); }); - test('auto-login failure does not block initialize', () async { - final push = _FakePushClient(); - final auth = _NoopAuthService(throwOnLogin: true); - final host = MattermostPushHostIntegration( - pushClient: push, - authServiceFactory: (_) => auth, - ); - - await host.initialize(); - - expect(host.isInitialized, isTrue); - expect(push.initializeCalls, equals(1)); - - await push.dispose(); - }); - test( 'navigation callbacks are assigned exactly once during initialize', () async { final push = _FakePushClient(); - final host = MattermostPushHostIntegration( - pushClient: push, - authServiceFactory: (_) => _NoopAuthService(), - ); + final host = NexoNotificationHostIntegration(pushClient: push); String? capturedChannel; String? capturedThread; @@ -153,10 +110,7 @@ void main() { test('notification stream remains consumable through integration', () async { final push = _FakePushClient(); - final host = MattermostPushHostIntegration( - pushClient: push, - authServiceFactory: (_) => _NoopAuthService(), - ); + final host = NexoNotificationHostIntegration(pushClient: push); await host.initialize(); final received = >[]; diff --git a/apps/client/test/widget_test.dart b/apps/client/test/widget_test.dart index dd4e0a6..00a8205 100644 --- a/apps/client/test/widget_test.dart +++ b/apps/client/test/widget_test.dart @@ -10,6 +10,8 @@ import 'package:iop_client/control_plane_status_client.dart'; import 'package:iop_client/widgets/nodes_panel.dart'; import 'package:protobuf/protobuf.dart'; import 'package:fixnum/fixnum.dart'; +import 'package:iop_client/src/integrations/nexo/nexo_notification_client.dart'; +import 'package:iop_client/src/integrations/nexo/nexo_notification_host_integration.dart'; class FakeWebSocket implements WebSocket { final _controller = StreamController(); @@ -565,10 +567,7 @@ void main() { await tester.pumpAndSettle(); // Scroll to reveal second node's provider catalog before assertions - await tester.drag( - find.byType(ListView), - const Offset(0.0, -1500.0), - ); + await tester.drag(find.byType(ListView), const Offset(0.0, -1500.0)); await tester.pumpAndSettle(); // Verify Provider Catalog sections are present on both nodes @@ -576,8 +575,14 @@ void main() { // Verify Provider Snapshot display for node-1 (ollama provider) expect(find.text('provider-ollama'), findsOneWidget); - expect(find.textContaining('llm / inference'), findsNWidgets(2)); // both providers on node-1 - expect(find.textContaining('HEALTHY'), findsNWidgets(2)); // both ollama and vllm are healthy + expect( + find.textContaining('llm / inference'), + findsNWidgets(2), + ); // both providers on node-1 + expect( + find.textContaining('HEALTHY'), + findsNWidgets(2), + ); // both ollama and vllm are healthy expect(find.textContaining('Load: 3/10'), findsOneWidget); expect(find.textContaining('Q: 1'), findsOneWidget); expect(find.textContaining('30.0%'), findsOneWidget); @@ -756,7 +761,8 @@ void main() { expect(fakeStatusRepo.lastCommandEdgeId, equals('edge-a')); // Setup delayed fetch for edge-b - fakeStatusRepo.edgeBOperationsCompleter = Completer(); + fakeStatusRepo.edgeBOperationsCompleter = + Completer(); // Switch to Edge Beta via Dropdown await tester.tap(find.byType(DropdownButton)); @@ -776,20 +782,22 @@ void main() { expect(find.byType(CircularProgressIndicator), findsOneWidget); // Now complete the operations fetch for edge-b - fakeStatusRepo.edgeBOperationsCompleter!.complete(EdgeOperationsResponseView( - edgeId: 'edge-b', - operations: [ - EdgeCommandRecordView( - edgeId: 'edge-b', - commandId: 'cmd-beta-active', - operation: 'build-deploy.deploy', - targetSelector: '', - status: 'success', - summary: 'Deploy queued for edge beta', - error: '', - ), - ], - )); + fakeStatusRepo.edgeBOperationsCompleter!.complete( + EdgeOperationsResponseView( + edgeId: 'edge-b', + operations: [ + EdgeCommandRecordView( + edgeId: 'edge-b', + commandId: 'cmd-beta-active', + operation: 'build-deploy.deploy', + targetSelector: '', + status: 'success', + summary: 'Deploy queued for edge beta', + error: '', + ), + ], + ), + ); await tester.pumpAndSettle(); // Verify domain agents for Edge Beta (deployer shows BUSY, build-deploy shows READY) @@ -799,7 +807,10 @@ void main() { // [REVIEW_API-2] Verify presence of beta active command and operations history expect(find.text('Active Command: cmd-beta-active'), findsOneWidget); expect(find.text('build-deploy.deploy'), findsOneWidget); - expect(find.textContaining('Deploy queued for edge beta'), findsOneWidget); + expect( + find.textContaining('Deploy queued for edge beta'), + findsOneWidget, + ); // [REVIEW_API-2] Verify absence of alpha-only active command and operations history expect(find.text('Active Command: cmd-alpha-active'), findsNothing); @@ -940,10 +951,7 @@ void main() { await tester.pumpAndSettle(); // Scroll to reveal node-2's provider catalog - await tester.drag( - find.byType(ListView), - const Offset(0.0, -1500.0), - ); + await tester.drag(find.byType(ListView), const Offset(0.0, -1500.0)); await tester.pumpAndSettle(); // The degraded provider should show DEGRADED text (not ACTIVE) @@ -1058,10 +1066,7 @@ void main() { expect(prov0.queued, equals(1)); expect(prov0.loadRatio, equals(0.3)); expect(prov0.servedModels, equals(['llama-3.1', 'mistral'])); - expect( - prov0.lifecycleCapabilities, - equals(['start', 'stop', 'restart']), - ); + expect(prov0.lifecycleCapabilities, equals(['start', 'stop', 'restart'])); // Validate second provider snapshot final prov1 = node.providerSnapshots[1]; @@ -1215,7 +1220,8 @@ void main() { return false; }); expect(availableLabelFinder, findsOneWidget); - final availableLabel = availableLabelFinder.evaluate().first.widget as Text; + final availableLabel = + availableLabelFinder.evaluate().first.widget as Text; expect(availableLabel.style?.color, equals(const Color(0xFF10B981))); }, ); @@ -1363,7 +1369,8 @@ void main() { return false; }); expect(unavailableLabelFinder, findsOneWidget); - final unavailableLabel = unavailableLabelFinder.evaluate().first.widget as Text; + final unavailableLabel = + unavailableLabelFinder.evaluate().first.widget as Text; expect(unavailableLabel.style?.color, equals(const Color(0xFFEF4444))); // [G02] UNKNOWN status label color assertion (#64748B grey) @@ -1426,4 +1433,164 @@ void main() { }); }, ); + + // [G05] Notification stream → UI(SnackBar) connection verification + testWidgets( + 'notification stream from NexoNotificationHostIntegration connects to UI snackbar', + (WidgetTester tester) async { + final fakeClient = FakeClientWireClient(shouldSuccess: true); + final fakeStatusRepo = FakeControlPlaneStatusRepository(); + + // Create a fake NexoNotificationClient that emits a notification. + final notificationController = StreamController>(); + final fakeNexoClient = _FakeNexoClientsForUiTest(notificationController); + final fakeHost = NexoNotificationHostIntegration( + pushClient: fakeNexoClient, + ); + + await tester.pumpWidget( + IopClientApp( + testClient: fakeClient, + statusRepository: fakeStatusRepo, + notificationHost: fakeHost, + ), + ); + await tester.pump(); + await tester.pump(const Duration(milliseconds: 100)); + + // Verify wire status shows connected. + expect(find.text('CONNECTED'), findsOneWidget); + + // Emit a notification message through the fake client. + notificationController.add(const { + 'type': 'message', + 'message': 'Hello from Nexo', + 'channel_name': 'general', + 'sender_name': 'test-user', + }); + await tester.pump(); + + // Verify the SnackBar appears in the UI. + expect(find.textContaining('Hello from Nexo'), findsOneWidget); + + await notificationController.close(); + }, + ); + + // [G05] Notification stream with channel name only (no sender) + testWidgets( + 'notification stream shows channel-only message when sender is empty', + (WidgetTester tester) async { + final fakeClient = FakeClientWireClient(shouldSuccess: true); + final fakeStatusRepo = FakeControlPlaneStatusRepository(); + + final notificationController = StreamController>(); + final fakeNexoClient = _FakeNexoClientsForUiTest(notificationController); + final fakeHost = NexoNotificationHostIntegration( + pushClient: fakeNexoClient, + ); + + await tester.pumpWidget( + IopClientApp( + testClient: fakeClient, + statusRepository: fakeStatusRepo, + notificationHost: fakeHost, + ), + ); + await tester.pump(); + await tester.pump(const Duration(milliseconds: 100)); + + expect(find.text('CONNECTED'), findsOneWidget); + + // Emit notification with empty sender — should show just the message. + notificationController.add(const { + 'type': 'message', + 'message': 'Direct message content', + 'channel_name': 'alerts', + 'sender_name': '', + }); + await tester.pump(); + + // When sender is empty, only message content is shown. + expect(find.textContaining('Direct message content'), findsOneWidget); + + await notificationController.close(); + }, + ); + + // [G05] Non-message type should not trigger snackbar + testWidgets('notification stream ignores non-message events (e.g. system)', ( + WidgetTester tester, + ) async { + final fakeClient = FakeClientWireClient(shouldSuccess: true); + final fakeStatusRepo = FakeControlPlaneStatusRepository(); + + final notificationController = StreamController>(); + final fakeNexoClient = _FakeNexoClientsForUiTest(notificationController); + final fakeHost = NexoNotificationHostIntegration( + pushClient: fakeNexoClient, + ); + + await tester.pumpWidget( + IopClientApp( + testClient: fakeClient, + statusRepository: fakeStatusRepo, + notificationHost: fakeHost, + ), + ); + await tester.pump(); + await tester.pump(const Duration(milliseconds: 100)); + + // Emit a non-message type. + notificationController.add(const { + 'type': 'system', + 'message': 'System notification', + }); + await tester.pump(); + + // No SnackBar should appear for non-message types. + expect(find.textContaining('System notification'), findsNothing); + + await notificationController.close(); + }); +} + +/// Fake NexoNotificationClient for widget tests that broadcasts on a +/// provided StreamController. +class _FakeNexoClientsForUiTest implements NexoNotificationClient { + final StreamController> controller; + + _FakeNexoClientsForUiTest(this.controller); + + @override + Stream> get onNotification => controller.stream; + + @override + Future initialize() async {} + + @override + Future getDeviceToken() async => null; + + @override + Future setAuthToken( + String serverUrl, + String token, { + String? identifier, + }) async {} + + @override + Future setSigningKey(String serverUrl, String signingKey) async {} + + @override + set onDeviceTokenReady(Future Function(String token)? callback) {} + + @override + set onNavigateToChannel( + void Function(String serverUrl, String channelId)? callback, + ) {} + + @override + set onNavigateToThread( + void Function(String serverUrl, String rootId)? callback, + ) {} } diff --git a/apps/edge/internal/openai/chat_handler.go b/apps/edge/internal/openai/chat_handler.go index 092d59f..ee3ffaa 100644 --- a/apps/edge/internal/openai/chat_handler.go +++ b/apps/edge/internal/openai/chat_handler.go @@ -76,7 +76,7 @@ func (s *Server) handleChatCompletions(w http.ResponseWriter, r *http.Request) { estimate := s.estimateChatInputTokens(prompt, runMeta, req.Tools, req.ToolChoice) contextClass := classifyContext(estimate, s.longContextThreshold()) - validation := toolValidationContractForChatRequest(req, outputPolicy) + validation := toolValidationContractFromRequest(req) metadata := chatRunMetadata(runMeta, req, outputPolicy) metadata["estimated_input_tokens"] = strconv.Itoa(estimate) metadata["context_class"] = contextClass @@ -380,6 +380,10 @@ func collectChatCompletionOutput(ctx context.Context, req chatCompletionRequest, var toolValidationErr error if len(nativeToolCalls) > 0 { + if len(req.Tools) > 0 && strings.TrimSpace(message.Content) != "" { + filter := &streamToolTextFilter{} + message.Content = filter.Append(message.Content) + filter.FlushNonCandidate() + } nativeToolCalls = normalizeNativeToolCallsForResponse(nativeToolCalls, req.Tools) message.ToolCalls = nativeToolCalls toolCalls = nativeToolCalls diff --git a/apps/edge/internal/openai/server_test.go b/apps/edge/internal/openai/server_test.go index fae6df3..a007a2d 100644 --- a/apps/edge/internal/openai/server_test.go +++ b/apps/edge/internal/openai/server_test.go @@ -1,7 +1,6 @@ package openai import ( - "bufio" "context" "encoding/json" "fmt" @@ -543,6 +542,114 @@ func TestChatCompletionsSynthesizesTextToolCallsForProviderRoute(t *testing.T) { } } +func TestChatCompletionsSynthesizesEditToolCallWithMarkdownAndAngleBracketPlaceholder(t *testing.T) { + fake := &fakeRunService{events: make(chan *iop.RunEvent, 2)} + fake.events <- &iop.RunEvent{Type: "delta", Delta: "두 줄을 `18083` -> `18081`로 수정하고 검증을 진행한다.\n\n" + editToolCallFixture()} + fake.events <- &iop.RunEvent{Type: "complete"} + + srv := NewServer(config.EdgeOpenAIConf{Adapter: "openai_compat"}, fake, nil) + req := httptest.NewRequest(http.MethodPost, "/v1/chat/completions", strings.NewReader(`{ + "model":"qwen3.6:35b", + "messages":[{"role":"user","content":"edit"}], + "tools":[{"type":"function","function":{"name":"edit","parameters":{"type":"object","properties":{"path":{"type":"string"},"edits":{"type":"array","items":{"type":"object","properties":{"oldText":{"type":"string"},"newText":{"type":"string"}},"required":["newText"],"additionalProperties":false}}},"required":["path","edits"],"additionalProperties":false}}}], + "tool_choice":"auto" + }`)) + w := httptest.NewRecorder() + + srv.handleChatCompletions(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("status: got %d body=%s", w.Code, w.Body.String()) + } + body := w.Body.String() + if strings.Contains(body, ":18081/v1/chat/completions") { + t.Fatalf("newText lost angle-bracket placeholder: %+v", edit["newText"]) + } +} + +func TestChatCompletionsStreamSynthesizesEditToolCallWithMarkdownAndAngleBracketPlaceholder(t *testing.T) { + fake := &fakeRunService{events: make(chan *iop.RunEvent, 2)} + fake.events <- &iop.RunEvent{Type: "delta", Delta: "수정합니다.\n\n" + editToolCallFixture()} + fake.events <- &iop.RunEvent{Type: "complete"} + + srv := NewServer(config.EdgeOpenAIConf{Adapter: "openai_compat"}, fake, nil) + req := httptest.NewRequest(http.MethodPost, "/v1/chat/completions", strings.NewReader(`{ + "model":"qwen3.6:35b", + "stream":true, + "messages":[{"role":"user","content":"edit"}], + "tools":[{"type":"function","function":{"name":"edit","parameters":{"type":"object","properties":{"path":{"type":"string"},"edits":{"type":"array","items":{"type":"object","properties":{"oldText":{"type":"string"},"newText":{"type":"string"}},"required":["newText"],"additionalProperties":false}}},"required":["path","edits"],"additionalProperties":false}}}], + "tool_choice":"auto" + }`)) + w := httptest.NewRecorder() + + srv.handleChatCompletions(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("status: got %d body=%s", w.Code, w.Body.String()) + } + body := w.Body.String() + if strings.Contains(body, ":18081/v1/chat/completions \\\n -H \"Content-Type: application/json\"\n```", + }}) + return "\n" + + "\n" + + "\napps/client/test/client_bootstrap_test.dart\n\n" + + "\n" + string(edits) + "\n\n" + + "\n" + + "" +} + +func decodeEditToolArguments(t *testing.T, raw string) map[string]any { + t.Helper() + var args map[string]any + if err := json.Unmarshal([]byte(raw), &args); err != nil { + t.Fatalf("arguments JSON: %v", err) + } + edits, ok := args["edits"].([]any) + if !ok || len(edits) != 1 { + t.Fatalf("edits argument: %+v", args["edits"]) + } + if _, ok := edits[0].(map[string]any); !ok { + t.Fatalf("edit item: %+v", edits[0]) + } + return args +} + func TestChatCompletionsSynthesizesProviderTextToolCallsWhenFallbackMarked(t *testing.T) { fake := &fakeRunService{events: make(chan *iop.RunEvent, 2)} fake.events <- &iop.RunEvent{Type: "delta", Delta: "먼저 현재 git 상태를 확인하겠습니다.\n\n\n\n\n[\"cd /config/workspace/iop && git status\"]\n\n\n"} @@ -696,6 +803,52 @@ func TestChatCompletionsPassesThroughNativeToolCallsFromProviderRoute(t *testing } } +func TestChatCompletionsNativeToolCallPreservesLeadingProse(t *testing.T) { + fake := &fakeRunService{events: make(chan *iop.RunEvent, 2)} + fake.events <- &iop.RunEvent{Type: "delta", Delta: "선행 prose\n\n[\"git status\"]"} + fake.events <- &iop.RunEvent{ + Type: "complete", + Metadata: map[string]string{ + "finish_reason": "tool_calls", + runtimeMetadataOpenAIToolCalls: `[{"id":"call_1","type":"function","function":{"name":"run_commands","arguments":"{\"commands\":[\"git status\"]}"}}]`, + }, + } + + srv := NewServer(config.EdgeOpenAIConf{Adapter: "openai_compat"}, fake, nil) + req := httptest.NewRequest(http.MethodPost, "/v1/chat/completions", strings.NewReader(`{ + "model":"qwen3.6:35b", + "messages":[{"role":"user","content":"status"}], + "tools":[{"type":"function","function":{"name":"run_commands"}}], + "tool_choice":"auto" + }`)) + w := httptest.NewRecorder() + + srv.handleChatCompletions(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("status: got %d body=%s", w.Code, w.Body.String()) + } + body := w.Body.String() + if strings.Contains(body, "[\"git status\"]"} + fake.events <- &iop.RunEvent{ + Type: "complete", + Metadata: map[string]string{ + "finish_reason": "tool_calls", + runtimeMetadataOpenAIToolCalls: `[{"id":"call_1","type":"function","function":{"name":"run_commands","arguments":"{\"commands\":[\"git status\"]}"}}]`, + }, + } + + srv := NewServer(config.EdgeOpenAIConf{Adapter: "openai_compat"}, fake, nil) + req := httptest.NewRequest(http.MethodPost, "/v1/chat/completions", strings.NewReader(`{ + "model":"stream-model", + "stream":true, + "messages":[{"role":"user","content":"status"}], + "tools":[{"type":"function","function":{"name":"run_commands"}}], + "tool_choice":"auto" + }`)) + w := httptest.NewRecorder() + + srv.handleChatCompletions(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("status: got %d body=%s", w.Code, w.Body.String()) + } + body := w.Body.String() + if !strings.Contains(body, `"content":"선행 prose"`) { + t.Fatalf("stream dropped leading prose:\n%s", body) + } + if !strings.Contains(body, `"tool_calls"`) || !strings.Contains(body, `"call_1"`) { + t.Fatalf("stream did not pass through native tool_calls:\n%s", body) + } + if strings.Contains(body, "val"} - fake.events <- &iop.RunEvent{Type: "complete"} + invalidRun := func() chan *iop.RunEvent { + return bufferedRunEvents( + &iop.RunEvent{Type: "delta", Delta: "thought text "}, + &iop.RunEvent{Type: "delta", Delta: "val"}, + &iop.RunEvent{Type: "complete"}, + ) + } + fake := &fakeRunService{ + eventRuns: []chan *iop.RunEvent{ + invalidRun(), + invalidRun(), + }, + runIDs: []string{"run-bad-1", "run-bad-2"}, + } srv := NewServer(config.EdgeOpenAIConf{Adapter: "openai_compat"}, fake, nil) req := httptest.NewRequest(http.MethodPost, "/v1/chat/completions", strings.NewReader(`{ @@ -4099,6 +4300,9 @@ func TestChatCompletionsProviderStreamBlocksUnknownTextToolCall(t *testing.T) { if strings.Contains(body, `\u003ctool_call\u003e`) { t.Fatalf("unknown tool call leaked as content: %s", body) } + if reqs := fake.reqsSnapshot(); len(reqs) != 2 { + t.Fatalf("submit attempts: got %d want 2", len(reqs)) + } } func TestChatCompletionsStrictBufferedStreamFailsUnknownTextToolCallAfterRetryLimit(t *testing.T) { @@ -4169,10 +4373,20 @@ func TestChatCompletionsFailsMalformedTextToolCallAfterRetryLimit(t *testing.T) } func TestChatCompletionsProviderStreamBlocksMalformedTextToolCall(t *testing.T) { - fake := &fakeRunService{events: make(chan *iop.RunEvent, 3)} - fake.events <- &iop.RunEvent{Type: "delta", Delta: "이것은 정상 텍스트 "} - fake.events <- &iop.RunEvent{Type: "delta", Delta: ""} - fake.events <- &iop.RunEvent{Type: "complete"} + invalidRun := func() chan *iop.RunEvent { + return bufferedRunEvents( + &iop.RunEvent{Type: "delta", Delta: "이것은 정상 텍스트 "}, + &iop.RunEvent{Type: "delta", Delta: ""}, + &iop.RunEvent{Type: "complete"}, + ) + } + fake := &fakeRunService{ + eventRuns: []chan *iop.RunEvent{ + invalidRun(), + invalidRun(), + }, + runIDs: []string{"run-bad-1", "run-bad-2"}, + } srv := NewServer(config.EdgeOpenAIConf{Adapter: "openai_compat"}, fake, nil) req := httptest.NewRequest(http.MethodPost, "/v1/chat/completions", strings.NewReader(`{ @@ -4196,6 +4410,9 @@ func TestChatCompletionsProviderStreamBlocksMalformedTextToolCall(t *testing.T) if !strings.Contains(body, "tool_validation_error") { t.Fatalf("expected tool_validation_error, got: %s", body) } + if reqs := fake.reqsSnapshot(); len(reqs) != 2 { + t.Fatalf("submit attempts: got %d want 2", len(reqs)) + } } func TestValidateToolCallResponseValidatesEveryToolCall(t *testing.T) { diff --git a/apps/edge/internal/openai/stream.go b/apps/edge/internal/openai/stream.go index 8fb2825..5c64dc9 100644 --- a/apps/edge/internal/openai/stream.go +++ b/apps/edge/internal/openai/stream.go @@ -30,10 +30,11 @@ func (s *Server) streamChatCompletion(w http.ResponseWriter, r *http.Request, re w.Header().Set("Cache-Control", "no-cache") w.Header().Set("Connection", "keep-alive") - // Strict buffered streams collect and validate the full response before - // emitting any user-visible chunk, so they own handle lifecycle, retries, - // and the initial role chunk internally. - if outputPolicy.Strict && outputPolicy.StreamBuffer { + // Buffered streams collect and validate the full response before emitting + // user-visible chunks. Strict buffered output opts in explicitly, and + // tool-bearing streams use the same path so malformed tool calls can be + // retried or rejected before reaching a client tool runner. + if (outputPolicy.Strict && outputPolicy.StreamBuffer) || len(req.Tools) > 0 { s.streamBufferedChatCompletion(w, r, req, submitReq, handle, flusher, outputPolicy, validation) return } diff --git a/apps/edge/internal/openai/tool_validation.go b/apps/edge/internal/openai/tool_validation.go index c336496..096a53f 100644 --- a/apps/edge/internal/openai/tool_validation.go +++ b/apps/edge/internal/openai/tool_validation.go @@ -42,23 +42,6 @@ func toolValidationContractFromRequest(req chatCompletionRequest) toolValidation return toolValidationContract{enabled: true, specs: specs} } -// toolValidationContractForChatRequest resolves the runtime tool validation -// contract for a request, accounting for its stream mode. Non-stream and -// strict buffered stream responses validate tool calls before emitting any -// user-visible payload, so validation stays enabled. Live SSE streams may -// already emit content deltas before the terminal event, so exact retry is not -// possible and runtime validation is explicitly excluded. -func toolValidationContractForChatRequest(req chatCompletionRequest, outputPolicy strictOutputPolicy) toolValidationContract { - contract := toolValidationContractFromRequest(req) - if !contract.enabled || !req.Stream { - return contract - } - if outputPolicy.Strict && outputPolicy.StreamBuffer { - return contract - } - return toolValidationContract{} -} - func toolChoiceDisablesToolValidation(toolChoice any) bool { choice, ok := toolChoice.(string) return ok && strings.EqualFold(strings.TrimSpace(choice), "none") diff --git a/apps/edge/internal/openai/types.go b/apps/edge/internal/openai/types.go index 5ebb70c..3010bc3 100644 --- a/apps/edge/internal/openai/types.go +++ b/apps/edge/internal/openai/types.go @@ -4,7 +4,6 @@ import ( "encoding/json" "fmt" "reflect" - "regexp" "sort" "strconv" "strings" @@ -211,26 +210,16 @@ func contentToString(v any) string { } var ( - textToolCallBlockRE = regexp.MustCompile(`(?is)<\s*tool_call\s*>(.*?)<\s*/\s*tool_call\s*>`) - textToolCallOpenRE = regexp.MustCompile(`(?is)<\s*tool_call(?:\s*[^>]*)?>`) - textToolFunctionRE = regexp.MustCompile(`(?is)<\s*function\s*=\s*([A-Za-z_][A-Za-z0-9_.:-]*)\s*>(.*?)<\s*/\s*function(?:\s*=\s*[A-Za-z_][A-Za-z0-9_.:-]*)?\s*>`) - textToolParameterRE = regexp.MustCompile(`(?is)<\s*parameter\s*=\s*([A-Za-z_][A-Za-z0-9_.:-]*)\s*>(.*?)<\s*/\s*parameter(?:\s*=\s*[A-Za-z_][A-Za-z0-9_.:-]*)?\s*>`) textToolCallOpeningHint = "= rawMatch[0] && openMatch[0] < rawMatch[1] { - inBlock = true - break - } - } - if !inBlock { - out = append(out, candidateMatch{ - start: openMatch[0], - end: len(content), - validationErr: fmt.Errorf("malformed tool call: unclosed tool_call block"), - }) - } - } return out, foundAny } @@ -495,115 +460,202 @@ func synthesizeToolCallsFromText(content string, tools []any, runID string) (str return res.cleaned, res.toolCalls } -func collectTextToolCallMatches(content string, specs map[string]textToolSpec) []textToolCallMatch { - var matches []textToolCallMatch - matches = append(matches, collectXMLTextToolCallMatches(content, specs)...) - matches = append(matches, collectMustacheTextToolCallMatches(content, specs)...) - sort.SliceStable(matches, func(i, j int) bool { - if matches[i].start == matches[j].start { - return matches[i].end > matches[j].end - } - return matches[i].start < matches[j].start - }) - return matches -} - -func collectXMLTextToolCallMatches(content string, specs map[string]textToolSpec) []textToolCallMatch { +func scanXMLTextToolCallBlocks(content string) ([]xmlTextToolCallBlock, bool) { if !strings.Contains(strings.ToLower(content), textToolCallOpeningHint) { - return nil + return nil, false } - rawMatches := textToolCallBlockRE.FindAllStringSubmatchIndex(content, -1) - if len(rawMatches) == 0 { - return nil - } - out := make([]textToolCallMatch, 0, len(rawMatches)) - for _, rawMatch := range rawMatches { - block := content[rawMatch[0]:rawMatch[1]] - name, arguments, ok := parseTextToolCallBlock(block, specs) + foundAny := false + var blocks []xmlTextToolCallBlock + for pos := 0; pos < len(content); { + openStart, openEnd, ok := findXMLOpenTag(content, pos, len(content), "tool_call") if !ok { - continue - } - out = append(out, textToolCallMatch{ - start: rawMatch[0], - end: rawMatch[1], - name: name, - arguments: arguments, - }) - } - return out -} - -func collectMustacheTextToolCallMatches(content string, specs map[string]textToolSpec) []textToolCallMatch { - if !strings.Contains(content, textMustacheToolCallOpenHint) { - return nil - } - blocks := scanMustacheTextToolCallBlocks(content) - if len(blocks) == 0 { - return nil - } - out := make([]textToolCallMatch, 0, len(blocks)) - for _, block := range blocks { - spec, ok := specs[block.name] - if !ok { - continue - } - args, ok := parseMustacheToolCallArguments(block.args) - if !ok { - continue - } - args = normalizeTextToolCallArguments(block.name, args, spec) - encoded, err := json.Marshal(args) - if err != nil { - continue - } - out = append(out, textToolCallMatch{ - start: block.start, - end: block.end, - name: block.name, - arguments: string(encoded), - }) - } - return out -} - -func scanMustacheTextToolCallBlocks(content string) []mustacheTextToolCallBlock { - var blocks []mustacheTextToolCallBlock - for start := 0; start < len(content); { - open := strings.Index(content[start:], textMustacheToolCallOpenHint) - if open < 0 { break } - blockStart := start + open - nameStart := skipASCIISpaces(content, blockStart+len(textMustacheToolCallOpenHint)) - nameEnd := nameStart - for nameEnd < len(content) && isTextToolIdentifierChar(content[nameEnd], nameEnd == nameStart) { - nameEnd++ - } - name := content[nameStart:nameEnd] - if name == "" { - start = blockStart + len(textMustacheToolCallOpenHint) - continue - } - callOpen := skipASCIISpaces(content, nameEnd) - if callOpen >= len(content) || content[callOpen] != '(' { - start = blockStart + len(textMustacheToolCallOpenHint) - continue - } - argsStart := callOpen + 1 - argsEnd, blockEnd, ok := findMustacheToolCallEnd(content, argsStart) + foundAny = true + closeStart, closeEnd, ok := findXMLCloseTag(content, openEnd, len(content), "tool_call") if !ok { - start = blockStart + len(textMustacheToolCallOpenHint) - continue + blocks = append(blocks, xmlTextToolCallBlock{ + start: openStart, + end: len(content), + err: fmt.Errorf("malformed tool call: unclosed tool_call block"), + }) + break } - blocks = append(blocks, mustacheTextToolCallBlock{ + block := parseXMLTextToolCallBlock(content, openStart, openEnd, closeStart, closeEnd) + blocks = append(blocks, block) + pos = closeEnd + } + return blocks, foundAny +} + +func parseXMLTextToolCallBlock(content string, blockStart, bodyStart, bodyEnd, blockEnd int) xmlTextToolCallBlock { + _, fnOpenEnd, name, ok := findXMLNamedOpenTag(content, bodyStart, bodyEnd, "function") + if !ok { + return xmlTextToolCallBlock{ start: blockStart, end: blockEnd, - name: name, - args: content[argsStart:argsEnd], - }) - start = blockEnd + err: fmt.Errorf("malformed tool call: missing function block or definition inside tool_call"), + } } - return blocks + fnCloseStart, _, ok := findXMLCloseTag(content, fnOpenEnd, bodyEnd, "function") + if !ok { + return xmlTextToolCallBlock{ + start: blockStart, + end: blockEnd, + err: fmt.Errorf("malformed tool call: missing function block or definition inside tool_call"), + } + } + params := map[string]string{} + for pos := fnOpenEnd; pos < fnCloseStart; { + paramStart, paramOpenEnd, key, ok := findXMLNamedOpenTag(content, pos, fnCloseStart, "parameter") + if !ok { + break + } + paramCloseStart, paramCloseEnd, ok := findXMLCloseTag(content, paramOpenEnd, fnCloseStart, "parameter") + if !ok { + break + } + key = strings.TrimSpace(key) + if key != "" { + params[key] = content[paramOpenEnd:paramCloseStart] + } + pos = paramCloseEnd + if pos <= paramStart { + break + } + } + return xmlTextToolCallBlock{ + start: blockStart, + end: blockEnd, + name: strings.TrimSpace(name), + params: params, + err: nil, + } +} + +func findXMLOpenTag(content string, start, limit int, tag string) (int, int, bool) { + for pos := start; pos < limit; { + idx := strings.IndexByte(content[pos:limit], '<') + if idx < 0 { + return 0, 0, false + } + tagStart := pos + idx + next := skipASCIISpaces(content, tagStart+1) + if next < limit && content[next] == '/' { + pos = tagStart + 1 + continue + } + if !matchASCIIFoldAt(content, next, tag) { + pos = tagStart + 1 + continue + } + afterTag := next + len(tag) + if afterTag < limit && isTextToolIdentifierChar(content[afterTag], false) { + pos = tagStart + 1 + continue + } + closeIdx := strings.IndexByte(content[afterTag:limit], '>') + if closeIdx < 0 { + return 0, 0, false + } + return tagStart, afterTag + closeIdx + 1, true + } + return 0, 0, false +} + +func findXMLNamedOpenTag(content string, start, limit int, tag string) (int, int, string, bool) { + for pos := start; pos < limit; { + idx := strings.IndexByte(content[pos:limit], '<') + if idx < 0 { + return 0, 0, "", false + } + tagStart := pos + idx + next := skipASCIISpaces(content, tagStart+1) + if next < limit && content[next] == '/' { + pos = tagStart + 1 + continue + } + if !matchASCIIFoldAt(content, next, tag) { + pos = tagStart + 1 + continue + } + cursor := skipASCIISpaces(content, next+len(tag)) + if cursor >= limit || content[cursor] != '=' { + pos = tagStart + 1 + continue + } + cursor = skipASCIISpaces(content, cursor+1) + nameStart := cursor + for cursor < limit && isTextToolIdentifierChar(content[cursor], cursor == nameStart) { + cursor++ + } + if cursor == nameStart { + pos = tagStart + 1 + continue + } + name := content[nameStart:cursor] + cursor = skipASCIISpaces(content, cursor) + if cursor >= limit || content[cursor] != '>' { + pos = tagStart + 1 + continue + } + return tagStart, cursor + 1, name, true + } + return 0, 0, "", false +} + +func findXMLCloseTag(content string, start, limit int, tag string) (int, int, bool) { + for pos := start; pos < limit; { + idx := strings.Index(content[pos:limit], "= limit || content[cursor] != '>' { + pos = tagStart + 2 + continue + } + return tagStart, cursor + 1, true + } + return 0, 0, false +} + +func matchASCIIFoldAt(s string, pos int, want string) bool { + if pos < 0 || pos+len(want) > len(s) { + return false + } + for i := 0; i < len(want); i++ { + a := s[pos+i] + b := want[i] + if a >= 'A' && a <= 'Z' { + a += 'a' - 'A' + } + if b >= 'A' && b <= 'Z' { + b += 'a' - 'A' + } + if a != b { + return false + } + } + return true } func findMustacheToolCallEnd(content string, argsStart int) (int, int, bool) { @@ -910,33 +962,6 @@ func mapValue(value any) map[string]any { return m } -func parseTextToolCallBlock(block string, specs map[string]textToolSpec) (string, string, bool) { - match := textToolFunctionRE.FindStringSubmatchIndex(block) - if match == nil { - return "", "", false - } - name := strings.TrimSpace(block[match[2]:match[3]]) - spec, ok := specs[name] - if !ok { - return "", "", false - } - body := block[match[4]:match[5]] - args := map[string]any{} - for _, param := range textToolParameterRE.FindAllStringSubmatchIndex(body, -1) { - key := strings.TrimSpace(body[param[2]:param[3]]) - if key == "" { - continue - } - args[key] = parseTextToolParameterValue(body[param[4]:param[5]]) - } - args = normalizeTextToolCallArguments(name, args, spec) - encoded, err := json.Marshal(args) - if err != nil { - return "", "", false - } - return name, string(encoded), true -} - func parseTextToolParameterValue(raw string) any { trimmed := strings.TrimSpace(raw) if trimmed == "" {