Commit Graph

3 Commits

Author SHA1 Message Date
ad7acd30db fix(platform): clear golangci-lint findings
Some checks failed
E2E API Smoke Test / detect-changes (pull_request) Successful in 1m0s
Lint curl status-code capture / Scan workflows for curl status-capture pollution (pull_request) Successful in 14s
E2E Staging SaaS (full lifecycle) / pr-validate (pull_request) Successful in 54s
Handlers Postgres Integration / detect-changes (pull_request) Successful in 42s
lint-required-no-paths / lint-required-no-paths (pull_request) Successful in 1m15s
lint-continue-on-error-tracking / lint-continue-on-error-tracking (pull_request) Successful in 1m50s
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 16s
qa-review / approved (pull_request) Failing after 15s
Lint pre-flip continue-on-error / Verify continue-on-error flips have run-log proof (pull_request) Successful in 2m0s
Lint workflow YAML (Gitea-1.22.6-hostile shapes) / Lint workflow YAML for Gitea-1.22.6-hostile shapes (pull_request) Successful in 1m36s
lint-mask-pr-atomicity / lint-mask-pr-atomicity (pull_request) Successful in 2m8s
gate-check-v3 / gate-check (pull_request) Successful in 32s
security-review / approved (pull_request) Failing after 18s
Runtime PR-Built Compatibility / detect-changes (pull_request) Successful in 41s
lint-required-context-exists-in-bp / lint-required-context-exists-in-bp (pull_request) Successful in 2m11s
sop-checklist-gate / gate (pull_request) Successful in 17s
Harness Replays / Harness Replays (pull_request) Successful in 5s
sop-tier-check / tier-check (pull_request) Successful in 22s
CI / Shellcheck (E2E scripts) (pull_request) Successful in 20s
E2E Staging Canvas (Playwright) / Canvas tabs E2E (pull_request) Successful in 14s
Runtime PR-Built Compatibility / PR-built wheel + import smoke (pull_request) Successful in 10s
E2E API Smoke Test / E2E API Smoke Test (pull_request) Successful in 1m42s
Handlers Postgres Integration / Handlers Postgres Integration (pull_request) Successful in 3m53s
CI / Python Lint & Test (pull_request) Successful in 7m18s
CI / Canvas (Next.js) (pull_request) Successful in 11m54s
CI / Canvas Deploy Reminder (pull_request) Has been skipped
CI / Platform (Go) (pull_request) Successful in 12m45s
CI / all-required (pull_request) Successful in 3s
sop-checklist / all-items-acked (pull_request) acked: 7/7
audit-force-merge / audit (pull_request) Successful in 4s
2026-05-12 22:53:22 -07:00
Hongming Wang
656a02fae4 fix(textutil): SSOT for rune-safe string truncation, fix 3 audit-gap bugs
Closes #2962.

## Why

Six per-package `truncate` helpers had drifted into independent
re-implementations of the same idea. Three of them (delegation.go,
memory/client/client.go, memory-backfill/verify.go) used
`s[:max] + "…"` byte-slice form, which on a multi-byte codepoint at
byte `max` produces invalid UTF-8 → Postgres `text`/`jsonb` rejects
the INSERT silently → `delegation` / `activity_logs` row never lands
→ audit gap.

Three other helpers (delegation_ledger.go #2962, agent_message_writer.go
#2959, scheduler.go #2026) had each been fixed in isolation with three
slightly different rune-safe shapes — confirming this is a class of
bug, not a single instance.

## What

New package `internal/textutil` with three rune-safe functions:

- `TruncateBytes(s, maxBytes)` — byte-cap, "…" marker. Used by 5
  callers writing into byte-bounded columns / log lines.
- `TruncateBytesNoMarker(s, maxBytes)` — byte-cap, no marker. Used by
  delegation_ledger.go where the storage already conveys "preview"
  and an extra ellipsis would push the result over the column cap.
- `TruncateRunes(s, maxRunes)` — rune-cap, "…" marker. Used by
  agent_message_writer.go where the cap is in display chars (UI
  summary), not bytes.

All three guarantee `utf8.ValidString(out)` for any `utf8.ValidString(in)`.
Inputs already invalid go through `sanitizeUTF8` at the call site
boundary (scheduler.go preserved this defense-in-depth).

## Migration map

| Old | New | Behavior change |
|---|---|---|
| `delegation_ledger.truncatePreview` | `textutil.TruncateBytesNoMarker(s, 4096)` | none |
| `agent_message_writer.truncatePreviewRunes` | `textutil.TruncateRunes(s, n)` | none |
| `scheduler.truncate` | `textutil.TruncateBytes(s, n)` | "..." → "…" (3 bytes either way; single-glyph display) |
| `delegation.truncate` | `textutil.TruncateBytes(s, n)` | bug fix + ellipsis swap |
| `memory/client.truncate` | `textutil.TruncateBytes(s, n)` | bug fix |
| `memory-backfill.truncate` | `textutil.TruncateBytes(s, n)` | bug fix |

Five separate `truncate*` helpers + their per-package tests removed.
Net: 12 files / +427 / -255.

## Tests

- `internal/textutil/truncate_test.go` — 27 table-test cases + 145
  fuzz-invariant cases asserting `utf8.ValidString` and byte-cap
  invariants on every output.
- `delegation_ledger_test.go TestLedgerInsert_TruncatesOversizedPreview`
  strengthened with `capValidUTF8Matcher` so the SQL-write argument
  is asserted to be valid UTF-8 + within cap (not just `AnyArg()`).
  Mutation-tested: replacing the SSOT call with byte-slice form makes
  this test fail loud.

## Compatibility

- All callers internal; no external API surface change.
- Ellipsis swap "..." → "…": same byte budget (3 bytes), single-glyph
  display. No alerting/grep on either marker in this codebase
  (verified). Canvas renders both correctly.
- DB column widths unchanged (4096 / 80 / 200 / 256 / 300 — all
  preserved in the migrations).

## Security

Fixes a silent INSERT-failure mode that hid `activity_logs` /
`delegations` rows containing peer-controlled text. The class of input
that triggered it (CJK, emoji, accented Latin) is normal user content,
not malicious — but the symptom (audit gap) makes incident
reconstruction harder. Helper is pure-function over `string`; no
secrets / PII / auth handling involved. Untrusted input is handled
identically to before, just rune-aligned now.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-05 23:01:21 -07:00
Hongming Wang
c1cff3169f Memory v2 PR-2: HTTP plugin client + breaker + capability negotiation
Builds on PR-1 (#2729). Implements every endpoint in the OpenAPI spec
plus two operational concerns the agent never sees:

  1. Capability negotiation. Boot/Refresh probes /v1/health and
     captures the plugin's capability list. MCP handlers (PR-5) ask
     SupportsCapability before exposing capability-gated features —
     e.g., agents can only request semantic search when "embedding"
     is reported.

  2. Circuit breaker. Three consecutive failures open the breaker for
     60 seconds; while open, calls fail fast with ErrBreakerOpen.
     Picked these constants because:
       - 3 failures: long enough to skip transient blips, short enough
         to react before all in-flight handlers stack on the timeout
       - 60s cooldown: long enough to back off a flapping plugin,
         short enough that recovery is felt within a single session
     4xx responses do NOT count toward the breaker (those are client
     bugs, not plugin health issues); 5xx + transport errors do.

What ships:
  - workspace-server/internal/memory/client/client.go
  - client_test.go: 100% statement coverage

Coverage corner cases pinned:
  - env-var success branches in New (parseDurationEnv applied)
  - json.Marshal error (via channel in Propagation)
  - http.NewRequestWithContext error (via unbalanced bracket in BaseURL)
  - 204 NoContent on endpoint that normally has a body
  - 4xx vs 5xx breaker behavior (4xx must NOT trip)
  - breaker cooldown elapsed → reset on next success
  - all 6 public endpoints fail-fast when breaker is open

This package has no callers in this PR; integration starts in PR-5.
2026-05-04 06:57:24 -07:00