fix(offloader): return portable filename from TS FileStorage.store by charan-rathore · Pull Request #4668 · strands-agents/harness-sdk

charan-rathore · 2026-09-28T19:27:22Z

Description

Legacy TypeScript FileStorage returned the machine's artifact path as an offload reference. That path would stop working when the stored reference moved with a conversation. Both local and sandbox storage now return only the filename. Retrieval still accepts older paths within the configured artifact directory and bare stems found in metadata. Host-mode retrieval also handles older relative paths when the artifact directory is configured relatively, and allows a filename containing .. without permitting a .. path segment. Unknown stems and paths outside that directory remain rejected.

Related Issues

Fixes #4665

Follow-up to #4568, which fixed the Python backend; this applies the same fix to the TypeScript backend, where the bug is still present on current main.

Documentation PR

No site documentation change is needed for this legacy backend; its code documentation now describes the filename reference.

Type of Change

Bug fix

Testing

Focused Vitest: six storage, sandbox storage, and plugin suites (134 passed), including legacy relative-path and trailing-dot-key regression tests.
TypeScript source type-check, targeted ESLint, Prettier and git diff --check passed.
Full workspace lint and integration type-check not completed locally (memory limit and unbuilt declarations respectively).

Instinct assisted with the change and tests.

Updated regex pattern in getReference function and adjusted file path checks in tests.

arielnabavian · 2026-09-29T14:32:18Z

@strandly-the-agent review that this ports the python feature correctly. And do a general review aswell

strandly-the-agent

Port is faithful except one gap: host-mode retrieve still rejects legacy relative path references (the default ./artifacts config), which Python accepts. Approving — the divergence is non-blocking, but the PR body claims legacy paths keep working, and for the default config they don't. Two inline 🟡s (that gap, plus a small host-mode regression for keys ending in .), both with validated one-line fixes.

What matches Python (storage.py:174-309, #4568): bare-filename return in host and sandbox mode, legacy-path acceptance in sandbox mode (exact <artifactDir>/ shape the old TS emitted), stem lookup via .metadata.json (unique match only, ambiguous → not found), fallback to the raw name when metadata is missing, .. and out-of-dir rejection, Reference not found on miss. Verified by probe, not just read.

✅ Verified

Head a53479c1b, base main (b7900ae68).
cd strands-ts && npx vitest run --project unit-node src/vended-plugins/context-offloader → 6 files, 132/132 pass, no type errors.
Probe on head (host mode, new FileStorage('./artifacts'), cwd = tmpdir): bare ✅, stem ✅, absolute legacy path ✅, ./artifacts/<f> ❌ Reference not found, artifacts/<f> ❌, ambiguous stem → not found ✅, '' / . / ../… / a/sub/../f / symlinked dir / /etc/hostname all rejected ✅.
Same probe with the two suggested fixes applied: legacy relative refs and k..txt round-trip, traversal still rejected, 132/132 still pass.
On main, host mode with a relative dir was already broken even for the fresh round trip (store returned artifacts/<f>, retrieve nested it). This PR fixes that forward; the inline comment is about refs that predate it.
Adversarial pass held: sandbox legacy forms across ./artifacts, artifacts, artifacts/, absolute, backslash dirs; backslash/NUL/trailing-slash refs; stem collisions; odd content types; unicode keys; parallel store() on a loaded instance. Plugin surface (plugin.ts:340-347, 419-441) never tells the model a reference is a path, so the bare filename fits.

Port-fidelity table

Behaviour	Python `storage.py`	TS PR `storage.ts`
`store` returns bare filename (host / sandbox)	:245 / :234	:321 / :302	✅
Legacy full path, sandbox	parent resolves to dir :191-195	exact prefix strip :330-332 — matches every ref old TS sandbox `store` produced	✅
Legacy full path, host, absolute dir	✅	✅	✅
Legacy full path, host, relative dir (default)	✅ `Path(ref).parent.resolve()`	❌ `path.resolve(artifactDir, ref)` nests it :349	🟡 inline
Stem → filename via metadata, unique only	:200-207	`_knownFilename` :281-287	✅
Metadata missing → raw name fallback	`_resolve_from_path`	`_knownFilename` returns candidate	✅
`..` rejection	substring :189	substring :331, :350	✅ same — and same over-match, see inline
Out-of-dir rejection	:194, :306	:332, :350	✅
Error	`KeyError('Reference not found: …')`	`Error('Reference not found: …')`	✅

Reading order

storage.ts:281-287 _knownFilename.
storage.ts:326-343 sandbox retrieve, :345-363 host retrieve.
__tests__/storage.test.node.ts:48-64 — note both legacy-path tests use an absolute tmpDir only.

Appendix — non-blocking (5)

⚪ Test gaps: legacy path with the default relative dir (would have caught the inline 🟡); ambiguous stem (probe confirms it rejects; a test would pin it). storage.test.node.ts:44 expect(ref).not.toContain(tmpDir) passes for any relative path — the sandbox file's not.toContain('/') is the stronger assertion.
⚪ storage.ts:284 filename.slice(0, filename.lastIndexOf('.')) drops the last char when a metadata key has no dot (lastIndexOf = -1). Only reachable with hand-edited metadata since store always appends an extension.
⚪ storage.ts:282 Object.hasOwn(this._contentTypes, …) runs outside the try; a .metadata.json containing null now throws TypeError instead of Reference not found. Guard in _loadMetadata/_loadSandboxMetadata (return {} unless plain object).
⚪ storage.ts:329 adds this._artifactDir.replace(/\/+$/, ''), the pattern stripTrailingSlashes (:56) exists to avoid; :226 already does the same, so consistency either way.
⚪ Pre-existing, not filed (deprecated class — say if you want an issue): _ensureSandbox (:244-247) sets _metadataLoaded after the await, so the first parallel store() calls on a fresh instance each load metadata and the later assignment clobbers the earlier entry. Reachable via the plugin's Promise.all at plugin.ts:476 on an agent's first multi-block offload → wrong content type / stem lookup miss for the clobbered block. Also pre-existing: .metadata.json is itself retrievable as a reference (both languages); host mode follows symlinks inside the dir (Python's .resolve() + is_relative_to doesn't).

charan-rathore · 2026-09-29T17:58:34Z

Thanks for the catch. Both are fixed and pushed: host-mode retrieval now resolves older relative refs against the working directory (4a10cd6), and the .. check is segment-based so keys with dots work while .. path segments stay rejected. Regression tests for both added in 5373bd0, 134/134 pass. PR body updated to match.

liramon2 · 2026-10-14T14:35:53Z

@charan-rathore Hi, thanks for the contribution. I left a few comments on this PR.

store() can return a filename containing ".." (e.g. a key ending in a dot produces "<ts>_<n>_Summary..txt"), which sandbox retrieve then rejected via a substring check. Reject only the exact "." and ".." references instead, mirroring the host-mode segment check, and document both guards. Co-authored-by: liramon2 <[email protected]> Signed-off-by: Charan Rathore <[email protected]>

charan-rathore · 2026-10-14T18:46:04Z

@liramon2 thanks for the careful read. Both nits (storage.ts:331 and :351) are fixed, and the ".." check had a real bug: store() can return a filename containing ".." (a key ending in a dot produces <ts>_<n>_Summary..txt), which sandbox retrieve then rejected. It now rejects only the exact "." and ".." references, mirroring the host-mode segment check, and both guards carry a comment. Pushed as e16c62e, with a regression test that stores a key ending in a dot and reads it back.

…trands-agents#4668) Signed-off-by: Charan Rathore <[email protected]> Co-authored-by: liramon2 <[email protected]>

charan-rathore added 4 commits September 29, 2026 00:55

fix(offloader): return portable filename from TS FileStorage.store

cfd98a5

test(offloader): filename references for local FileStorage

19c924e

test(offloader): filename references for sandbox FileStorage

2a49b06

test(offloader): plugin tests use filename references

a53479c

Updated regex pattern in getReference function and adjusted file path checks in tests.

charan-rathore requested a review from a team as a code owner September 28, 2026 19:27

charan-rathore requested a review from gautamsirdeshmukh September 28, 2026 19:27

github-actions Bot added typescript Pull requests that update typescript code area-persistence Session management or checkpointing bug Something isn't working labels Sep 28, 2026

strandly-the-agent approved these changes Sep 29, 2026

View reviewed changes

Comment thread strands-ts/src/vended-plugins/context-offloader/storage.ts Resolved

Comment thread strands-ts/src/vended-plugins/context-offloader/storage.ts Resolved

charan-rathore added 2 commits September 29, 2026 21:21

fix(offloader): honor legacy relative refs and dotted keys

4a10cd6

test(offloader): cover relative refs and dotted keys

5373bd0

liramon2 reviewed Oct 14, 2026

View reviewed changes

Comment thread strands-ts/src/vended-plugins/context-offloader/storage.ts Resolved

Comment thread strands-ts/src/vended-plugins/context-offloader/storage.ts Resolved

Comment thread strands-ts/src/vended-plugins/context-offloader/storage.ts Resolved

liramon2 approved these changes Oct 14, 2026

View reviewed changes

github-actions Bot added complexity/medium Touched functions have moderate cognitive complexity (11-25) size/s labels Oct 14, 2026

liramon2 merged commit 91c9430 into strands-agents:main Oct 15, 2026
26 of 27 checks passed

Navigation Menu

Repository navigation

fix(offloader): return portable filename from TS FileStorage.store - #4668

fix(offloader): return portable filename from TS FileStorage.store#4668
liramon2 merged 7 commits into
strands-agents:mainfrom
charan-rathore:fix-4665-ts-filestorage-filename

charan-rathore commented Sep 28, 2026 •

edited

Loading

Uh oh!

arielnabavian commented Sep 29, 2026

Uh oh!

strandly-the-agent left a comment

Uh oh!

Uh oh!

Uh oh!

charan-rathore commented Sep 29, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

liramon2 commented Oct 14, 2026

Uh oh!

charan-rathore commented Oct 14, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

charan-rathore commented Sep 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issues

Documentation PR

Type of Change

Testing

Uh oh!

arielnabavian commented Sep 29, 2026

Uh oh!

strandly-the-agent left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

charan-rathore commented Sep 29, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

liramon2 commented Oct 14, 2026

Uh oh!

charan-rathore commented Oct 14, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

charan-rathore commented Sep 28, 2026 •

edited

Loading