mirror of
https://github.com/multica-ai/multica.git
synced 2026-08-04 07:34:25 +02:00
999e9f93c7
* fix(redact): scrub secrets nested inside tool input maps and slices
InputMap only passed top-level string values through Text and documented
non-string values as "preserved as-is". Any secret one level down reached
the database and the WebSocket broadcast untouched:
flat -> [REDACTED ...] (scrubbed)
nested -> [map[diff:token=ghp_... path:a.go]] (leaked verbatim)
This is a prerequisite for recording structured file-edit payloads. Codex
reports an edit as changes[]{path, diff, content}, and the legacy protocol
reports a deletion as the whole outgoing file — so without this, deleting a
.env would persist its full contents in cleartext.
redactValue now walks the composite shapes json.Unmarshal produces, plus
[]string and map[string]string for argv-style inputs. Composites are copied
rather than scrubbed in place, because the caller keeps using the map it
passed in.
Nesting depth comes from daemon-supplied JSON, so the walk is bounded at 32
levels; a pathologically nested payload would otherwise recurse until the
stack blows. Hitting the bound yields a placeholder rather than the raw
value, keeping the fail-safe direction.
Verified: the five new tests each fail against the previous top-level-only
implementation and pass now; full ./pkg/redact suite green.
Co-authored-by: multica-agent <github@multica.ai>
* fix(codex): record file edit payload for patch_apply events
Both Codex protocol paths recorded a file edit as a bare call ID and set no
payload, so a run that edited six files left six blank, unexpandable rows in
the transcript. The same task on Claude or Grok showed a readable diff, and
the branch Codex pushed was the only surviving record of what it changed
(GH #6157).
The omission was specific to this one tool, not to the adapter: the
exec_command handlers directly above already captured command and output.
Both paths are fixed, since the protocol is sniffed at runtime. Their wire
shapes differ more than they appear, and the normalizer reconciles that:
- legacy patch_apply_begin/end carry map[path]FileChange, internally tagged
on `type`, where add/delete hold whole-file `content` and only update
holds a `unified_diff` plus `move_path`. There is no diff for every case,
so the normalized form keeps diff and content as alternatives.
- v2 fileChange items carry an ordered array of {path, kind, diff} where
`kind` is an object, not a string — reading it as a string silently
yields "" and loses the add/delete/update distinction.
- status spellings differ too: legacy is snake_case, v2 is camelCase and
adds inProgress. Both normalize onto one vocabulary, and a legacy event
predating `status` falls back to its `success` bool.
Legacy map iteration is sorted by path so a replayed event does not reshuffle
the file list.
Completion events now also produce a non-empty output (status, file count,
and any apply_patch stdout/stderr), because an empty output renders as an
unexpandable blank row just like a missing input.
Anything unrecognised — absent, wrongly typed, or malformed changes — returns
no payload, preserving exactly the previous degradation rather than risking
the transcript.
Total diff/content bytes are bounded at 64 KiB with UTF-8-safe truncation,
recording `truncated` and `original_bytes`; paths and kinds always survive,
since they are what a reviewer needs when the body is gone. The bound is
deliberately scoped to this new payload: other providers stream tool inputs
through unbounded, and clamping them here would silently truncate
transcripts that render correctly today. Unifying the limit at the
persistence boundary is left as a follow-up.
Verified: the new tests reproduce the reported symptom (Input:map[],
Output:"") against the previous call sites and pass now; ./pkg/agent and
./pkg/redact green, go vet and gofmt clean.
Co-authored-by: multica-agent <github@multica.ai>
* feat(transcript): render Codex multi-file patch payloads as diffs
The presenter identified an edit by input shape — a top-level file_path plus
old_string/new_string or content — which is Claude's and Grok's shape. Codex
records one patch_apply covering several files as changes[], so even with the
payload now populated it fell through to pretty JSON instead of a diff.
A new `patch` detail kind carries one entry per file, since collapsing them
into a single body would lose which change belongs where. Each file reuses the
existing single-file surfaces, so all bodies behave alike inside the
virtualized list.
Codex hands over a ready-made unified diff, so parseUnifiedDiff maps it onto
diff rows rather than recomputing one — there is no before/after pair to
compare, and reconstructing both sides from the diff just to diff them again
would be circular. Hunk headers become `gap` rows, which is what they denote:
skipped unchanged content.
A deletion renders as all-removals rather than as a whole-file write, because
the legacy protocol reports it as the outgoing file's content and a green
"+N" gutter would state the opposite of what happened.
The collapsed row needed its own fix: with no single path field, the summary
fell through the preference chain and came back empty. It now reads as the
first path plus "+N more".
Anything that is not this shape still falls back to pretty JSON, so a payload
this presenter does not understand stays readable.
Verified: 17 new tests (43 in the presenter suite) pass; repo typecheck and
lint clean. The one failing views test, layout/sidebar-resize, fails
identically on an untouched checkout.
Co-authored-by: multica-agent <github@multica.ai>
* fix(codex): route v2 add/delete payloads as content, not diff
Addresses review on #6158.
Upstream's format_file_change_diff only produces a unified diff for `update`.
For `add` and `delete` it returns the whole file's contents under the same
`diff` field, and for a moved `update` it appends a trailing
"\n\nMoved to: <path>" line:
FileChange::Add { content } => content.clone(),
FileChange::Delete { content } => content.clone(),
FileChange::Update { unified_diff, move_path } => ...
(codex-rs/app-server-protocol/src/protocol/item_builders.rs, rust-v0.145.0)
Recording that as a diff mislabels every line of an added or deleted file as
context, and actively inverts any line whose content begins with '+' or '-' —
so an added file containing "-minus lead" rendered as a deletion. The payload
is now routed by `kind` rather than by field name, and the "Moved to:"
sentence is stripped since move_path already carries the destination.
The previous v2 tests hid this by using a fixture the real protocol never
emits (an `add` carrying "@@ ... +package main"). They now use upstream's
shape, plus cases for delete, an empty add, and an add whose contents look
like diff headers.
Empty bodies are also kept on both paths: presence of the field, not its
non-emptiness, decides whether a body was reported, so an empty added file
renders as an empty body instead of "no content reported".
Verified: the new assertions fail against the previous normalizer — where an
`add` came through as {"diff": "package main\n"} — and pass now.
Co-authored-by: multica-agent <github@multica.ai>
* fix(transcript): stop treating header-like content lines as file headers
Addresses review on #6158.
parseUnifiedDiff matched "---" / "+++" / "diff --git" / "index " at any
position, so a changed line whose *content* starts with a dash or plus was
silently discarded:
parseUnifiedDiff("@@ -1 +1 @@\n--- old markdown\n+++ new markdown\n")
// before: [{ kind: "gap", ... }] — both changed lines gone
A removal of "-- old markdown" is spelled "--- old markdown" on the wire, so
this hit Markdown rules, embedded patches, and comment banners.
File headers only exist ahead of the first hunk, so they are only recognised
there; once inside a hunk every line is parsed strictly by its first
character.
Also localizes the multi-file summary count, which was hardcoded English and
so leaked into the zh-Hans / ja / ko transcript rows. The presenter owns no
React and no i18n by design, so the phrasing is injected by the caller rather
than imported here, keeping the module unit-testable in isolation; the English
form remains the fallback. The three Chinese/Japanese/Korean truncation
strings now use "..." to match the English source they translate.
Verified: both new parser assertions fail against the previous
strip-anywhere behaviour and pass now; 47 presenter tests green, repo
typecheck and lint clean.
Co-authored-by: multica-agent <github@multica.ai>
* fix(daemon): redact nested tool input before it leaves the daemon
Addresses review on #6158.
Recursive redaction ran only in the server's ingest handler. The daemon built
the new nested edit payload and sent msg.Input verbatim, so a daemon that
self-updated ahead of the server — or one talking to a server mid-rollout —
would ship whole-file edit contents to a peer that does not scrub nested
values yet. The legacy protocol reports a deletion as the whole outgoing file,
so that window covered a deleted .env in cleartext.
Ordering three commits inside one PR is not a deployment barrier, and daemon
and server upgrade independently. Deployment order is not a control we have,
so the sending side is now safe on its own; the server keeps redacting on
ingest as the second line of defence.
Scoped to Input, which is the field this PR newly fills with file contents.
Content and Output are plain strings already redacted server-side, and
changing their daemon-side handling would be unrelated to this fix.
Verified: the new daemon test asserts the nested token is masked in the
reported batch while the change metadata survives. It fails without this
change, reporting the full GITHUB_TOKEN= line on the wire, and passes with
it; ./internal/daemon green.
Co-authored-by: multica-agent <github@multica.ai>
* fix(transcript): correct the Chinese multi-file patch count semantics
Addresses review on #6158.
The summary is handed the number of files *beyond* the named one, but the
Chinese phrasing stated a total: "a.go 等 2 个文件" reads as two files including
a.go, so a three-file patch under-reported by one. English hides the
distinction ("+2 more"), which is why it survived the first pass.
Rewords zh-Hans to "另有 N 个文件". Japanese (他) and Korean (외) already read
as "besides", so their wording is unchanged.
Also renames the interpolation variable from `count` to `extra`, for two
reasons. i18next treats `count` as the plural selector — this very namespace
relies on that for events_one/events_other — so a plain number had no business
borrowing it. And the name is what a translator reads: `extra` cannot be
mistaken for a total the way `count` was.
Guards the whole bug class rather than just this string: a locale test asserts
every locale interpolates {{path}} and {{extra}} and never the reserved
{{count}}, and a presenter test pins that the injected number is the count of
additional files, not the total.
Verified: both new locale assertions fail against the reverted string and pass
now; rendering the real locale strings for a three-file patch yields "+2 more",
"另有 2 个文件", "他 2 件", "외 2개". 53 target tests pass, repo typecheck clean,
views lint back to its pre-existing 16 warnings and 0 errors.
Co-authored-by: multica-agent <github@multica.ai>
* fix(transcript): put the patch surface on the type scale
Addresses review on #6158.
The patch surface wrote text-[10px] / text-[11px] / text-[10px], copied from
the sibling transcript surfaces as they looked when this branch started. Since
then MUL-5451 (#6136) introduced a role-named type scale and migrated those
same siblings to text-micro, so these three call sites were the only remaining
arbitrary sizes — and the type-scale guard reports them precisely.
All three become text-micro. That matches the analogues they were copied from
now that those have moved: the FileWriteSurface line-count row, the
DiffDetailSurface header row, and the ToolDetailSurface body. It is also the
only correct target, since micro (11px) is the smallest step the scale defines
— there is nothing at 10px to map to.
Merges origin/main so the guard runs here rather than only in CI.
Verified: apps/web app/type-scale.test.ts 13/13 (it listed exactly these three
lines before), no `text-[` left in the file, repo typecheck clean, views lint
unchanged at 16 pre-existing warnings and 0 errors.
Co-authored-by: multica-agent <github@multica.ai>
* fix(codex): redact patch bodies before applying the size budget
Addresses review on #6158.
The adapter sized and truncated the normalized changes, and redaction only ran
later — in the daemon before sending, then again in the server on ingest. That
order loses secrets that straddle the budget.
The PEM rule needs both markers to match:
-----BEGIN[A-Z\s]*PRIVATE KEY-----.*?-----END[A-Z\s]*PRIVATE KEY-----
So a 70 KB private key whose BEGIN sits inside the first 64 KiB and whose END
falls past the cut stops matching once truncated. Neither later pass can
recognise what truncation already broke, so the marker and 64 KiB of key
material reach the database and the WebSocket broadcast. Measured on the
previous code:
stored bytes 65536 | BEGIN marker present | key body present | placeholder absent
Redaction now runs first, and the budget measures the redacted bodies — which
is also the honest measurement, since those are what actually gets stored and
redaction usually shrinks them (that key collapses to 23 bytes, so no trimming
is needed at all). `original_bytes` still reports the pre-redaction size so the
reader sees how large the real patch was. The daemon and server passes stay as
defence in depth; redaction is idempotent, so running three times is safe and
that is now asserted.
Note for callers: codexPatchInput no longer trims its argument in place, because
redaction copies first. Two existing tests were asserting on the caller's
original slice and had silently become vacuous; they now read the returned
payload, and one pins the no-mutation contract. The delete fixture in the
diff-vs-content routing test was also a credential-shaped string, which now
redacts — it is plain text so that test keeps testing routing.
Verified: the new boundary test fails on the previous order, reporting the
surviving BEGIN marker and key material, and passes now. go test ./pkg/agent
./pkg/redact ./internal/daemon green; execenv ByteIdentical green; go vet and
gofmt clean.
Co-authored-by: multica-agent <github@multica.ai>
---------
Co-authored-by: Eve <eve@multica-ai.local>
Co-authored-by: multica-agent <github@multica.ai>