Skip to content

refactor(mcp): port adapters (external I/O wrapper) - #95

Merged
couragehong merged 6 commits into
CryptoLabInc:feat/go-migrationfrom
esifea:feat/esifea/porting-adapter
May 6, 2026
Merged

couragehong merged 6 commits into
CryptoLabInc:feat/go-migrationfrom
esifea:feat/esifea/porting-adapter

Conversation

@esifea

@esifea esifea commented Apr 30, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • What changed: Implement adapters in Go
  • Why:
  • Scope: internal/adapters, go.mod, go.sum

Validation

  • Tests run (or explain why not):
  • Docs updated (if behavior/setup changed)

Cross-Agent Invariants

  • scripts/bootstrap-mcp.sh remains the single source of truth for runtime prep (venv/deps/self-heal)
  • No agent-specific script duplicates bootstrap/setup logic
  • Agent-specific scripts remain thin adapters (registration/wiring only)
  • Codex-only commands (codex mcp ...) are clearly separated from cross-agent/common instructions
  • Claude/Gemini/OpenAI instructions do not include Codex-only commands
  • SKILL.md, commands/rune/*.toml, and AGENT_INTEGRATION.md stay consistent on boundaries

Notes for Reviewers

  • Risk areas:
  • Backward compatibility impact:
  • Follow-up work (if any):

@esifea esifea self-assigned this Apr 30, 2026
@esifea
esifea requested a review from couragehong April 30, 2026 11:29
couragehong added a commit that referenced this pull request May 5, 2026
12 direct-parity items, 3 Go-only safety/typing improvements, 5
acceptable divergences (DecryptResult shape, JSON parse location,
Tier 2 fallback, per-instance timeout, etc.), 1 open env-override gap
(RUNEVAULT_GRPC_TARGET — should land in config loader, not adapter).

Notes the bug fix in this PR (commit 7a8199c) for the broken hand-rolled
grpcStatuser interface inherited from #95 review base.
@CryptoLabInc CryptoLabInc deleted a comment from redcourage May 5, 2026
@couragehong

couragehong commented May 5, 2026 •

Copy link
Copy Markdown
Contributor

internal/adapters/vault/errors.go:117-130 의 statusFromError 가 실제 gRPC 에러와 시그니처 안 맞아서 MapGRPCError 가 모든 gRPC status 를 VAULT_INTERNAL 로 fallback 시킵니다.

// errors.go L117-130 (PR #95 head a13d174)
func statusFromError(err error) (grpcStatus, bool) {
    type grpcStatuser interface {
        GRPCStatus() interface {
            Code() int           // ← int 요구
            Message() string
        }
    }

    if gs, ok := err.(grpcStatuser); ok {
        ...
    }
    return grpcStatus{}, false
}
  • 실제 *status.Error 의 GRPCStatus() 는 *status.Status 를 리턴하고, (*status.Status).Code() 는 codes.Code (= uint32) 를 리턴합니다.
  • Go interface 만족 검사는 시그니처 정확 일치라서 named type codes.Code 가 int 와 매치 안 됨 → type assertion fails.
err := status.Error(codes.Unauthenticated, "wrong token")
if _, ok := err.(grpcStatuser); ok { /* 절대 안 들어옴 */ }
// 실제 출력: "DID NOT MATCH — type assertion failed for *status.Error"
Vault 응답 기대 매핑 현재 매핑
Unauthenticated VAULT_AUTH_FAILED (retryable=false) VAULT_INTERNAL (retryable=true) ❌
NotFound VAULT_KEY_NOT_FOUND (retryable=false) VAULT_INTERNAL (retryable=true) ❌
Unavailable VAULT_UNAVAILABLE VAULT_INTERNAL — code 부정확
DeadlineExceeded VAULT_TIMEOUT VAULT_INTERNAL — code 부정확

특히 Unauthenticated(토큰 만료/무효) 가 retryable=true 로 잘못 마킹돼 에이전트가 잘못된 토큰으로 무한 재시도 트리거합니다. recovery hint 도 모두 generic 하게 나가서 사용자가 토큰/네트워크/키 문제 분간 불가능


google.golang.org/grpc/{codes,status} 직접 import 해서 status.FromError(err) + codes.* 를 사용하는게 어떨까 싶습니다?!

couragehong/feat/vault-grpc 브랜치의 commit 7a8199c 에 동일 수정 들어가 있고, unit test TestMapGRPCError_CodeMatrix 가 6 gRPC code 케이스로 검증해두었습니다. (별도 PR).

return fmt.Errorf("vault: failed to create health request: %w", err)
}

resp, err := http.DefaultClient.Do(req)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this needs a timeout. http.DefaultClient has no timeout, so if vault is unresponsive and caller doesn't set a ctx deadline, the call hangs. My suggest is set timeout like &http.Client{Timeout: 5 * time.Second}.

@couragehong
couragehong merged commit c50126e into CryptoLabInc:feat/go-migration May 6, 2026
1 check passed
couragehong added a commit that referenced this pull request May 6, 2026
The hand-rolled grpcStatuser interface in #95's port required Code() to
return int, but real gRPC status errors return codes.Code (uint32). The
type assertion silently failed for every gRPC error and every call
landed in the !ok branch as ErrVaultInternal — hiding the actual code.

Switch to status.FromError(err) and codes.* constants from
google.golang.org/grpc, which is already a direct dep.
couragehong added a commit that referenced this pull request May 6, 2026
12 direct-parity items, 3 Go-only safety/typing improvements, 5
acceptable divergences (DecryptResult shape, JSON parse location,
Tier 2 fallback, per-instance timeout, etc.), 1 open env-override gap
(RUNEVAULT_GRPC_TARGET — should land in config loader, not adapter).

Notes the bug fix in this PR (commit 7a8199c) for the broken hand-rolled
grpcStatuser interface inherited from #95 review base.
couragehong added a commit that referenced this pull request May 7, 2026
#103 ported errors.go for envector but missed vault. The hand-rolled
grpcStatuser interface in #95's port required Code() to return int, but
real *status.Status.Code() returns codes.Code (uint32). Type assertion
silently fails for every gRPC error and every call lands in the !ok
branch as ErrVaultInternal — hiding the actual code.

Worst case: Unauthenticated (token revoked/expired) gets mis-flagged as
retryable=true, triggering infinite retry on auth failure.

Switch to status.FromError(err) and codes.* constants from
google.golang.org/grpc, mirroring the pattern in envector/errors.go
MapSDKError. Removes the hand-rolled grpcStatus struct + statusFromError
helper + 4-line code constants block.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants