MILAB-6225: Add federative envelope handling to Ls API#1647
Open
vgpopov wants to merge 1 commit into
Open
Conversation
🦋 Changeset detectedLatest commit: e4c4c21 The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
vgpopov
changed the title
vgpopov
force-pushed
the
MILAB-6225/identity-propogation
branch
from
5157b0a to
504a380
Compare
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces support for federative identity envelopes by adding an additionalInfo field to storage listing responses and import handles across the OpenAPI, Protobuf, and TypeScript layers. It also renames several internal types and methods related to remote file listing for clarity. Review feedback suggests using bigint for file sizes to prevent precision loss and cleaning up TODO placeholders in the comments.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
vgpopov
force-pushed
the
MILAB-6225/identity-propogation
branch
from
504a380 to
d0fd3c3
Compare
❌ 4 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
- Extend Ls API to carry additionalInfo envelope for federative storages - Switch list results to a file-stats variant and propagate envelopes - Pass envelope through to index handles when present - Update REST/GRPC schemas to include additionalInfo - Add tests to verify envelope threading and absence when empty
vgpopov
force-pushed
the
MILAB-6225/identity-propogation
branch
from
d0fd3c3 to
e4c4c21
Compare
DenKoren
approved these changes
May 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Greptile Summary
This PR extends the Ls API to carry a federative identity envelope (
additionalInfo, amap<string, string>) through the full stack — proto definition, gRPC and REST codegen, TypeScript client, and index handle serialisation — so that downstream consumers can verify signed identity envelopes from federative storages.additional_info = 8added toLsAPI_ListItemin.proto, OpenAPI YAML, and both generated TypeScript stubs (gRPC and REST).createIndexImportHandlenow accepts an optionaladditionalInfomap and only embeds it when non-empty, keeping existing URLs byte-identical.listRemoteFilesWithAdditionalInfois renamed tolistRemoteFilesWithFileStats; bothlistFilesandlistRemoteFilesWithFileStatsnow forwarde.additionalInfoto the handle builder.Confidence Score: 3/5
The data flow and serialisation are correct; the hand-written protobuf map decoder in protocol.ts will throw a hard error on any unrecognised field number, which is unsafe for forward compatibility.
The hand-written
binaryReadMap8in the generated gRPC file unconditionally throws on unknown field numbers rather than skipping them. Every other field in the same generated file respectsoptions.readUnknownField. Under normal proto version alignment this path never triggers, but it is a latent runtime breakage that needs fixing before this decoder is relied on in production for federative storages.lib/node/pl-drivers/src/proto-grpc/github.com/milaboratory/pl/controllers/shared/grpc/lsapi/protocol.ts — the
binaryReadMap8default case needs attention.Important Files Changed
additionalInfomap field (field 8) toLsAPI_ListItemwith a hand-writtenbinaryReadMap8decoder. The decoder throws on any unrecognised field number instead of skipping it per proto3 spec, which could break parsing under version skew.additionalInfoparameter tocreateIndexImportHandle; only embeds it when non-empty, preserving byte-identical URLs for non-federative storages.listRemoteFilesWithAdditionalInfotolistRemoteFilesWithFileStatsand threadse.additionalInfointo both list methods; two TODO-4 comments remain in production call sites.additionalInfo: z.record(z.string(), z.string())toImportFileHandleIndexDataZod schema; backwards compatible.LsDriver additionalInfo threadingsuite covering envelope propagation in both list methods.Sequence Diagram
sequenceDiagram participant Backend as gRPC/REST Backend participant Client as ClientLs (ls_api.ts) participant Driver as LsDriver (ls.ts) participant Handle as createIndexImportHandle participant Consumer as Downstream Consumer Backend->>Client: "LsAPI_ListItem { additionalInfo, fullName, ... }" Client->>Driver: items with additionalInfo map Driver->>Handle: createIndexImportHandle(storageId, fullName, additionalInfo) Note over Handle: non-empty? embed in JSON, else omit Handle-->>Driver: index://index/encoded-JSON Driver-->>Consumer: "LsEntry { handle, type, name, fullPath }" Consumer->>Consumer: "parseIndexHandle -> { storageId, path, additionalInfo? }"Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "[MILAB-6225]: Add federative envelope ha..." | Re-trigger Greptile