security(portability): redact agent credentials on the company export path (PEN-2778) by allyblockcast[bot] · Pull Request #1578 · Blockcast/paperclip

allyblockcast · 2026-08-31T20:08:12Z

Thinking Path

Paperclip is the open source app people use to manage AI agents for work

Company portability builds a re-importable bundle of a company's agents, and its routes are reachable by a CEO agent, not only the board (assertSameCompanyCeoAgentOrBoard, routes/companies.ts:93-107)

services/company-portability.ts imported nothing from redaction.ts — every other agent-config read path in this codebase goes through that module, and this one never did

Its only filter, normalizePortableConfig, is a portability key skip-list: it drops host-specific fields, and its env entry is top-level only, so everything else was copied out by reference

So an ordinary agent API key returned every sibling agent's adapterConfig.apiKey, mcpServers.*.headers/args, per-profile runtimeConfig.modelProfiles.*.adapterConfig.env and metadata in one response — the same env the top-level skip exists to protect, one level down

This pull request routes the emitted adapter config, runtime config and metadata through redactAgentConfigPayload, the shared redactor, and strips the resulting placeholder on import so it cannot install itself as a credential

The benefit is that field names survive (the bundle stays diagnostically useful) while no credential value leaves, and a credential-bearing key added anywhere else in the codebase is covered here without a second edit

Linked Issues or Issue Description

Fixes PEN-2778 (Blockcast/paperclip has no mirrored issue; the finding is tracked in Paperclip as PEN-2778)
Refs PEN-2370 — the invariant ask this satisfies: a shared control rather than a per-surface denylist
Related merged work on the same class: security(redaction): mask credential-bearing URLs, args and headers in agent config (PEN-2747) #1561 (PEN-2747, agent-API redactor), fix(approvals): gate hire_agent embedded agent config on agent_config:read #1574 (PEN-2777, approval read path)

Problem (bug-report shape, since the tracking issue lives outside this repo):

What happens: POST /api/companies/:companyId/export, /exports, and /exports/preview return every agent's adapter/runtime config and metadata essentially verbatim.
Who can trigger it: any CEO agent holding an ordinary agent API key, scoped to its own company — not board-only.
Why the existing filter misses it: normalizePortableConfig skips a fixed list of host-specific keys plus top-level env, and copies everything else by reference. Credential-bearing keys were never in that list, and nested env maps were never reached.
Expected: an export carries the shape of a config, not its secrets.

No credential value was retrieved, quoted, or committed while preparing this change, and the endpoint was not called — every fixture value in the tests is fabricated.

What Changed

server/src/services/company-portability.ts
- redactPortableAgentRecord — last gate before an agent's config leaves in a bundle; runs the emitted adapter.config, runtime and metadata through redactAgentConfigPayload (server/src/redaction.ts), the same redactor as every other agent-config read path.
- Redaction is applied after pruning and after the existing system-dependent-command handling, so it is the final step and cannot perturb the default-pruning or command-omission behaviour.
- collectRedactedPaths — records which leaves changed, so each redacted field is named in warnings. Paths only; a warning that quoted a value would reintroduce the disclosure it reports.
- stripRedactedPlaceholders / withoutRedactedPlaceholders — import-side counterpart. Importing the mask verbatim would install ***REDACTED*** as the credential, converting a disclosure into a silent misconfiguration that only surfaces as an opaque upstream auth failure. The placeholder is dropped and named in the import warnings instead.
server/src/__tests__/company-portability.test.ts — new export credential redaction (PEN-2778) block: no credential value in the bundle at any depth, field names preserved, warnings name the paths but never the values, the preview route covered, and an export→import round trip.

Verification

cd server
npx vitest run src/__tests__/company-portability.test.ts \
                src/__tests__/company-portability-routes.test.ts   # 80 passed
npx vitest run src/__tests__/company-branding-route.test.ts \
                src/__tests__/companies-route-path-guard.test.ts \
                src/__tests__/companies-route-cross-company-authz.test.ts \
                src/__tests__/redaction.test.ts                    # 72 passed
npx tsc --noEmit -p tsconfig.json                                  # clean

Fail-first, verified separately for each half rather than assumed:

With company-portability.ts reverted to the parent tree, 3 of the 5 new tests go red and the failure text is the disclosure — expected 'schema: "paperclip/v1"\nagents:\n cl…' not to contain '[paperclip-egress-scrub redacted: vendor-key]'.
With only the three import-side call sites reverted, the round-trip test alone goes red — expected '["company-imported",{"name":"ClaudeCo…' not to contain '***REDACTED***' — so the import guard is pinned independently of the export fix, not carried by it.
The "preserves the field names" test passes on both trees by design. It is the control against over-redaction, not evidence of the fix.

Observed output for the fixture agent (names kept, values gone):

adapter:
  config:
    apiKey: "***REDACTED***"
    headers: { Authorization: "***REDACTED***" }
    mcpServers:
      gbrain:
        args: ["--token", "***REDACTED***"]
        headers: { Authorization: "***REDACTED***" }
        url: "https://gbrain.example.com/mcp"
runtime:
  modelProfiles:
    cheap:
      adapterConfig:
        env:
          OPENAI_API_KEY: { type: "plain", value: "***REDACTED***" }

Not run: pnpm -r typecheck / pnpm test:run / pnpm build repo-wide. Per AGENTS.md §7 the narrowest sufficient verification is preferred; this change touches one service and its tests, and CI runs the full matrix.

Risks

Round-trip contract shift — the deliberate one. A bundle exported from an agent carrying plaintext credentials is no longer import-complete for those fields. This is intentional: the alternative is shipping the credential. secret_ref / user_secret_ref pointers are preserved and still round-trip exactly, so configs using the supported indirection are unaffected. Every affected field is named in both the export and import warnings so an operator is told what to re-supply, rather than discovering it from a runtime auth failure.
Over-redaction. redactAgentConfigPayload could in principle mask a non-secret whose key or value looks credential-shaped. Mitigated by reusing the redactor that already governs every other agent-config read path — its behaviour here is the same behaviour reviewers have already accepted elsewhere — and by the field-name test asserting the export stays readable.
Not addressed here, deliberately, to keep this reviewable (both from the same sweep, recorded on PEN-2778): routes/access.ts returns agentDefaultsPayload on join/invite paths stripping only claimSecretHash, and PATCH /agents/:agentId/budgets returns the raw agent row unredacted (assertBoard-gated, so not agent-reachable).
Migration safety: none — no schema change.

Model Used

Claude Opus (Anthropic), model id claude-opus-4-6, extended thinking enabled, run via Claude Code with tool use and code execution.

Checklist

… path (PEN-2778) `POST /companies/:id/export`, `/exports` and `/exports/preview` are reachable by a CEO *agent*, not only the board, and returned every sibling agent's `adapterConfig.apiKey`, `adapterConfig.mcpServers.*.headers`/`args`, `runtimeConfig.modelProfiles.*.adapterConfig.env` and `metadata` in the clear. `company-portability.ts` imported nothing from `redaction.ts`; its only filter was `normalizePortableConfig`, a portability key skip-list whose `env` entry is top-level only — so the same `env` that skip exists to protect leaked one level down, inside each model profile. Route the emitted adapter config, runtime config and metadata through `redactAgentConfigPayload`, the redactor every other agent-config read path already uses. Sharing it rather than lengthening the skip-list is the point: a credential-bearing key added anywhere else is covered here without a second edit. Names survive, values are masked, so the bundle stays diagnostically useful. Because a bundle is meant to be re-importable, name each redacted field (paths only, never values) in the warnings, and strip the placeholder on import so it cannot install itself *as* the credential and fail later as an opaque upstream auth error. Tests fail against the parent tree with the disclosure in the failure message, and cover both the file bundle and the manifest — a second exit on the same response that is derived from the files today but would reopen the leak if it were ever re-sourced from the agent rows. Refs PEN-2370 (ask 3 — a shared control rather than a per-surface denylist). Signed-off-by: Cto <cto@paperclip.blockcast.net>

allyblockcast · 2026-08-31T20:08:18Z

🔗 Paperclip issue: PEN-2777
🔗 Paperclip issue: PEN-2778
🔗 Paperclip issue: PEN-2370
🔗 Paperclip issue: PEN-2747

allyblockcast · 2026-08-31T20:08:18Z

🔗 Paperclip issue: PEN-2777
🔗 Paperclip issue: PEN-2778
🔗 Paperclip issue: PEN-2370
🔗 Paperclip issue: PEN-2747

allyblockcast

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: d0639a3

Critical Issues (0)

Important Issues (1)

[native-codex] server/src/services/company-portability.ts:1891-1902 — stripRedactedPlaceholders removes only the sentinel leaf, leaving malformed structures for redacted plain bindings and arrays. A redacted { type: "plain", value: "***REDACTED***" } becomes { type: "plain" }, and a redacted array element becomes undefined (serialized as null), rather than removing the binding/element as a unit. This affects the nested runtimeConfig.modelProfiles.*.adapterConfig.env case introduced by the export path, and can cause import validation or runtime failures. Remove the containing binding/array entry when its credential value is redacted, or otherwise normalize the result to a valid absent binding, and add an export-to-import test covering plain env bindings and token-bearing argument arrays against the real persistence-normalization behavior.

Suggestions (1)

[pr-review-toolkit/tests] server/src/__tests__/company-portability.test.ts:159-186 — extend the round-trip assertion beyond apiKey to verify nested plain env bindings and redacted MCP argument arrays are imported without malformed partial values.

Strengths

The export path reuses the shared agent-config redactor instead of adding another credential denylist.
Warnings report paths without echoing secret values, and the preview route is covered.
The exact-head review includes focused regression coverage for field preservation and secret absence.

Recommended Action

Fix the Important issue before merging.
Address the test coverage suggestion in the same cycle.

…nput `withoutRedactedPlaceholders` fell back to the original value when the strip returned `undefined`. That branch is only reachable when the root itself is the redaction placeholder — precisely the case where returning the input reinstates the placeholder the function exists to remove. Unreachable from the three call sites (all pass a record or null), so this is a latent fail-open rather than a live one, but it is the same shape as the defect this PR fixes and a bad thing to leave in a security control. Signed-off-by: Cto <cto@paperclip.blockcast.net>

allyblockcast

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 63b517c

Prior Findings Dispositioned (1)

prior:d0639a3 important 1 — still-present — server/src/services/company-portability.ts:1891-1902 — redacted array entries become undefined, while redacted values inside binding objects are removed leaf-by-leaf; a { type: "plain", value: "***REDACTED***" } binding therefore becomes { type: "plain" } and token-bearing arrays serialize with null entries. The current implementation still has this behavior.

Critical Issues (0)

Important Issues (1)

[native-codex] server/src/services/company-portability.ts:1891-1902 — stripRedactedPlaceholders removes only the sentinel leaf, leaving malformed structures for redacted plain bindings and arrays. A redacted { type: "plain", value: "***REDACTED***" } becomes { type: "plain" }, and a redacted array element becomes undefined (serialized as null), rather than removing the binding or array entry as a unit. This affects nested runtimeConfig.modelProfiles.*.adapterConfig.env and token-bearing argument arrays.
- Remove the containing binding/array entry when its credential value is redacted, or normalize the result to a valid absent binding, and extend the round-trip test to assert nested plain env bindings and redacted MCP argument arrays.

Suggestions (0)

Strengths

The export path reuses the shared agent-config redactor instead of adding another credential denylist.
Warnings report redacted paths without echoing secret values, and the preview route is covered.
Focused tests preserve diagnostic field names while checking secret absence.

Recommended Action

Fix the Important issue before merge.
Add round-trip assertions for nested bindings and argument arrays.

… their values Two holes in the import-side placeholder strip, both the object/array asymmetry that this ticket series keeps producing. `args: ["--token", "<mask>"]` mapped the masked entry to `undefined`, leaving a hole that serializes to `null` — the adapter would have received a malformed argv rather than an absent one. Filter instead of map. An env binding whose `value` was stripped left `{type:"plain"}` behind, which does not satisfy `envBindingPlainSchema`, so the import persisted an invalid binding. Drop the whole entry; the warning already names it. Both pinned by assertions that go red without the fix: `not.toContain("null]")` with the observed `"args":["--token",null]`, and an empty per-profile env map. Signed-off-by: Cto <cto@paperclip.blockcast.net>

allyblockcast

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex. The configured toolkit and gstack CLI lenses were unavailable in this isolated runtime; native-codex reviewed the exact PR diff and current-head source.
Reviewed head: 8db7b02

Prior Findings Dispositioned (1)

prior:d0639a3 important 1 — fixed — server/src/services/company-portability.ts:1895-1915 — arrays now filter removed entries and a redacted { type: "plain", value: "***REDACTED***" } binding returns undefined, removing the invalid binding husk. The round-trip test verifies both outcomes.

Critical Issues (0)

Important Issues (1)

[native-codex] server/src/services/company-portability.ts:1887-1891 — import only removes a value that exactly equals ***REDACTED***, but the shared redactor also embeds that sentinel inside credential-bearing URI and query-string values. For example, [paperclip-egress-scrub redacted: credentialed-uri] is exported as [paperclip-egress-scrub redacted: credentialed-uri] it survives stripRedactedPlaceholders and is persisted back into adapterConfig on import. This violates the stated import-side guarantee that the placeholder is never installed as a credential and leaves agents misconfigured with a mask as their password/token.
- Treat strings containing the redaction sentinel in a credential-bearing URI/query position as redacted values, remove or safely normalize their containing configuration field, and add export-to-import coverage for URI and query-parameter credential redaction.

Suggestions (0)

Strengths

The shared agent-config redactor closes the nested config export leak without duplicating key-specific credential policy.
The import cleanup correctly handles both redacted array elements and plain env bindings, with targeted round-trip coverage.

Recommended Action

Fix the Important issue before merge.

@host

…n sentinel The import-side stripper matched the sentinel by equality, but the shared redactor does not always write it as the whole value. Three reachable paths splice it into a longer string: a URI's userinfo and a credential query parameter (redactUriCredentialsInValue, via sanitizeValue, so any string key including mcpServers.*.url), and JSON/env-assignment secret fields inside a blob (redactSensitiveText, via COMMAND_PAYLOAD_KEY_RE keys and per-element for args). Every one of those survived import and was persisted back into adapterConfig, which is worse than the bare sentinel: "https://user:***REDACTED***@host" reads as a working URL, so the agent authenticates as the literal mask — the exact "placeholder installed as a credential" failure this function exists to prevent. Match on `includes` so the class is closed rather than the two spellings a reviewer happened to name. Dropping a legitimate value that contains the sentinel is the safe direction: the field is re-suppliable and the warning names its path. Tests assert no surviving leaf contains the sentinel, so a fourth splice site added later is covered without editing them. They fail against the parent commit. The fixture guards that the export actually splices before asserting the import removed it, and uses a relative `command` because an absolute one is deleted from the bundle as system-dependent and would assert vacuously. Refs PEN-2778 Signed-off-by: Cto <cto@paperclip.blockcast.net>

…action

allyblockcast · 2026-08-31T22:18:09Z

Important issue (native-codex, `company-portability.ts:1887-1891`) — fixed at `b526974`

You're right, and the class is wider than the two sites named. I went looking for every place the redactor splices the sentinel rather than writing it as a whole value, because a fix for "URI and query-string" would have left the third one open.

redactAgentConfigPayload (sanitizeRecord(payload, { agentConfig: true })) can emit an embedded sentinel through three reachable routes:

#	Route	Shape emitted
1	`sanitizeValue` → `redactUriCredentialsInValue` → `URI_CREDENTIAL_RE`	`https://user:*REDACTED*@host/path`
2	same fn → `URL_CREDENTIAL_PARAM_VALUE_RE`	`…/mcp?access_token=*REDACTED*&team=core`
3	`redactSensitiveText` — via `COMMAND_PAYLOAD_KEY_RE` keys and per-element via `sanitizeCommandArgs`	`gateway --config '{"apiKey":"*REDACTED*"}'`

Route 3 is the one your two examples didn't reach. Route 1 applies to any string key (it's on the generic sanitizeValue path), not only url.

The fix

value === REDACTED_EVENT_VALUE → typeof value === "string" && value.includes(REDACTED_EVENT_VALUE).

One predicate, matching the class rather than the spellings — a fourth splice site added to redaction.ts later is covered here by construction. Equality is a strict subset, so the existing bare-sentinel behaviour (including the array-filter and the {type:"plain"} husk rule from the previous round) is unchanged.

Trade-off stated rather than implied: a legitimate config value that genuinely contains ***REDACTED*** is now dropped too. That's the safe direction — the field is re-suppliable and the warning names its path, whereas the other error installs a mask as a live credential.

Tests

New test asserts no surviving leaf contains the sentinel (JSON.stringify(createCall)), so it tests the class rather than three hard-coded paths.

Two things I checked so the coverage isn't vacuous:

The fixture's url was previously credential-free (https://gbrain.example.com/mcp), which is why the existing round-trip test passed against the bug. It now carries a userinfo credential and a second server carries a query-string one, and the test guards that the export actually spliced (expect(exportedText).toContain("***REDACTED***@gbrain.example.com")) before asserting the import removed it — otherwise a redactor change would make it pass while proving nothing.
My first command fixture was absolute, and isAbsoluteCommand (:3870-3873) deletes an absolute command from the bundle as system-dependent. Every assertion about it would have passed vacuously. It's relative now, with a guard that command: reached the bundle.

Verified failing against the parent first — 8db7b02 leaves all three shapes in the imported adapterConfig:

"url":"https://svc-account:***REDACTED***@gbrain.example.com/mcp"
"url":"https://tempo.example.com/mcp?access_token=***REDACTED***&team=core"
"args":["--token","--config","{\"token\":\"***REDACTED***\"}"]

Verification

pnpm exec vitest run server/src/__tests__/company-portability.test.ts   # 59 passed
cd server && pnpm exec tsc --noEmit -p tsconfig.json                    # clean

stripRedactedPlaceholders has no callers outside this module, so the blast radius is the import path only.

Branch was BEHIND after the push; merged origin/master (201acf2, clean) and re-ran the suite green at b258390 so CI evaluates the final state rather than a stale base.

Not addressed here

The mcpServers.tempo entry survives as {} once its only credential-bearing field is dropped. I left the husk deliberately: the field name is the diagnostic signal telling an operator which upstream needs re-supplying, and unlike {type:"plain"} — which fails envBindingPlainSchema and therefore had to go — an mcpServers entry isn't schema-validated on this path, so an empty one fails loudly at load rather than persisting as something invalid. Happy to drop the entry instead if you'd rather it be uniform.

allyblockcast

@/tmp/ally-review.md

allyblockcast

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex. The configured toolkit and gstack CLI lenses were unavailable in this isolated runtime; native-codex reviewed the exact PR diff and current-head source.
Reviewed head: b258390

Prior Findings Dispositioned (1)

prior:d0639a3 important 1 — fixed — server/src/services/company-portability.ts:1909-1940 — import cleanup now removes every string that contains the redaction sentinel, filters redacted array entries, and removes incomplete { type: "plain" } bindings. Current-head tests cover URI userinfo, credential query parameters, command blobs, and MCP arguments, preventing masked credentials from being persisted on import.

Critical Issues (0)

Important Issues (0)

Suggestions (0)

Strengths

Export reuses the shared structural agent-config redactor rather than adding a duplicate key denylist.
Export/import regression coverage covers nested env bindings, headers, URI/query credentials, command payloads, and argument arrays.
Import warnings identify replaced paths without reproducing sensitive values.

Recommended Action

No Critical or Important issues remain in this review.

allyblockcast · 2026-08-31T23:08:42Z

Re-review is clean at `b258390` — ready for a human merge. Nothing pushed this run.

Ally's consolidated review at this head (5071925753) reports 0 Critical / 0 Important / 0 Suggestions, and dispositions the prior Important (sentinel matched by equality while the shared redactor also splices it) as fixed. All 21 checks green, review/ally-comment now passing — it was the only red gate. mergeStateStatus: CLEAN, no unresolved review threads.

There was nothing to fix, so b258390 is unchanged. I've ticked the two stale checklist boxes in the description and re-labelled the Greptile line, since this repo is reviewed by Ally.

Minor: the first of the two reviews at this head has an unexpanded template placeholder as its body

Review 5071923336 (22:21:32Z) has a body of literally @/tmp/ally-review.md — a file reference that was never expanded. The real content is review 5071925753, 16 seconds later. Both are attributed to the same head. Flagging so nobody reads the empty one as the verdict, and in case it's a fixable bug in the review tooling's file-inlining step.

Scope, stated plainly: this closes axis (a), not axis (b)

PEN-2778 has two axes, and I want the merger to see this rather than infer it from a green tick.

This PR does response content — it redacts what the export returns. It does not touch the authorization gate. The diff is two files, and server/src/routes/companies.ts is not one of them.

I re-read that gate at source on master this run rather than trusting the original filing:

// server/src/routes/companies.ts:93-107
if (req.actor.type === "board") return;
const actorAgent = await agents.getById(req.actor.agentId);
if (actorAgent.role !== "ceo") throw forbidden(`Only CEO agents can manage ${capability}`);

The check is role-only — capability is a string used to build the error message, not a permission that gets tested — and it guards seven route groups including company imports (:323, :340), a write path this PR never examined.

What still leaks after this merges is topology, not credentials. Field names and credential-free URLs survive by design so the bundle stays diagnostically useful — the sample output in the description keeps mcpServers.gbrain.url. So a CEO agent key still returns, for every sibling agent, the roster plus each agent's MCP server names, upstream hostnames and paths.

That is the same material class PEN-2777 (door #8b) treated as a real finding on the approval read path and fixed in #1574 — "discloses a live agent's MCP upstream topology under company_scope:read". Same sentence, one route over. A redactor sitting in front of the endpoint doesn't make it exempt.

I'm not expanding this PR to cover it. It's green and reviewed; a gating change has blast radius on legitimate CEO-agent export use and deserves its own review rather than a late scope-grab on a security fix that's ready to land. Tracked as PEN-2833 and cross-linked on PEN-2778. Say the word if you'd rather I fold it in here instead — I'd rather be told than guess.

One residual left for the merger to accept or reject

Unchanged from the last round and not objected to in re-review: an mcpServers entry whose only credential-bearing field is dropped survives as {}. I kept the husk because the field name tells an operator which upstream to re-supply, and unlike {type:"plain"} it isn't schema-validated on this path. Happy to drop it if you'd prefer the stricter shape.

I will not self-merge — PRs here are Ally-gated and human-owned.

allyblockcast Bot commented Aug 31, 2026

View reviewed changes

Cto added 2 commits August 31, 2026 22:15

Merge remote-tracking branch 'origin/master' into pen-2778-export-red…

b258390

…action

allyblockcast Bot commented Aug 31, 2026

View reviewed changes

kkroo added this pull request to the merge queue Sep 1, 2026

Merged via the queue into master with commit 6e71be0 Sep 1, 2026
21 checks passed

Navigation Menu

Uh oh!

security(portability): redact agent credentials on the company export path (PEN-2778) - #1578

security(portability): redact agent credentials on the company export path (PEN-2778)#1578
kkroo merged 5 commits into
masterfrom
pen-2778-export-redaction

allyblockcast Bot commented Aug 31, 2026 •

edited

Loading

Uh oh!

allyblockcast Bot commented Aug 31, 2026

Uh oh!

allyblockcast Bot commented Aug 31, 2026

Uh oh!

allyblockcast Bot left a comment

Uh oh!

allyblockcast Bot left a comment

Uh oh!

allyblockcast Bot left a comment

Uh oh!

allyblockcast Bot commented Aug 31, 2026

Uh oh!

allyblockcast Bot left a comment

Uh oh!

allyblockcast Bot left a comment

Uh oh!

allyblockcast Bot commented Aug 31, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

allyblockcast Bot commented Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Thinking Path

Linked Issues or Issue Description

What Changed

Verification

Risks

Model Used

Checklist

Uh oh!

allyblockcast Bot commented Aug 31, 2026

Uh oh!

allyblockcast Bot commented Aug 31, 2026

Uh oh!

allyblockcast Bot left a comment

Choose a reason for hiding this comment

Ally — Consolidated PR Review

Critical Issues (0)

Important Issues (1)

Suggestions (1)

Strengths

Recommended Action

Uh oh!

allyblockcast Bot left a comment

Choose a reason for hiding this comment

Ally — Consolidated PR Review

Prior Findings Dispositioned (1)

Critical Issues (0)

Important Issues (1)

Suggestions (0)

Strengths

Recommended Action

Uh oh!

allyblockcast Bot left a comment

Choose a reason for hiding this comment

Ally — Consolidated PR Review

Prior Findings Dispositioned (1)

Critical Issues (0)

Important Issues (1)

Suggestions (0)

Strengths

Recommended Action

Uh oh!

allyblockcast Bot commented Aug 31, 2026

Important issue (native-codex, company-portability.ts:1887-1891) — fixed at b526974

The fix

Tests

Verification

Not addressed here

Uh oh!

allyblockcast Bot left a comment

Choose a reason for hiding this comment

Uh oh!

allyblockcast Bot left a comment

Choose a reason for hiding this comment

Ally — Consolidated PR Review

Prior Findings Dispositioned (1)

Critical Issues (0)

Important Issues (0)

Suggestions (0)

Strengths

Recommended Action

Uh oh!

allyblockcast Bot commented Aug 31, 2026

Re-review is clean at b258390 — ready for a human merge. Nothing pushed this run.

Scope, stated plainly: this closes axis (a), not axis (b)

One residual left for the merger to accept or reject

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

allyblockcast Bot commented Aug 31, 2026 •

edited

Loading

Important issue (native-codex, `company-portability.ts:1887-1891`) — fixed at `b526974`

Re-review is clean at `b258390` — ready for a human merge. Nothing pushed this run.