feat: list files in the extras picker, not just folders
Release / release (push) Successful in 6s

The extras control accepts a file — the field and its label said so — and the
listing under it could not show one, because the Host primitive behind the picker
browses folders and nothing else. So a grant for a single file meant typing its
name exactly, which is the one thing a listing exists to avoid.

The Host has a second lister and it does return files: `remote.workspaceFiles.list`,
the same listing behind the sidebar's own file tree, confined to the session's
workspace. The extras picker now takes that one while the level is inside the
session folder — which is where an extra usually is — and falls back to the folder
lister above it, saying so in the dialog rather than letting file rows vanish. The
focus picker never offers files: a focus is a folder, so a file row there would
offer something that cannot be chosen.

The two listings normalise into one level shape, so the picker does not care which
answered, and a row now carries its type: a folder is a way down, a file is a
thing to take. Clicking a file fills the path field and marks the row, and the
footer adds it in declared form. Pressing Enter on a name the level already lists
is no longer reported as a failure — the path is exactly what the dialog is for.
Everything stays additive: with no workspace file service, or above the session
folder, the picker lists folders exactly as before.
This commit is contained in:
NotExpectedYet committed 2026-09-28 07:47:24 +01:00
1 parent a40040ec17
commit c49acfdb2c
4 files changed
+590 -82

No files matched your search

+266 -20
View File
@@ -35,9 +35,10 @@ async function loadClient() {
SettingsForm: 'SettingsForm',
SettingsValueField: 'SettingsValueField',
SegmentedControl: 'SegmentedControl',
// The listing's row glyph; a client half may only use what the primitives
// actually export, so the stand-in is named the same way.
// The listing's row glyphs; a client half may only use what the primitives
// actually export, so the stand-ins are named the same way.
IconFolderCloseRegular: 'IconFolderCloseRegular',
FileTypeIcon: 'FileTypeIcon',
/** Stands in for the real model: records the specs and answers with the value. */
SettingsFormModel: class {
constructor(scope, specs) {
@@ -558,19 +559,46 @@ test('the browse face is read from uiWorkspace at the call, not at registration'
const missing = mod.directoryBrowse({ get: () => undefined })
assert.equal(missing.available(), false)
await assert.rejects(missing.list('/tmp'), /no directory browser/)
// The file lister is a second service and probes separately: a deployment can
// have the folder browser without the workspace file service.
assert.equal(missing.files.available(), false)
await assert.rejects(missing.files.list('session-1', '/tmp'), /cannot list files/)
const calls = []
const present = mod.directoryBrowse({
get: () => ({
listDirectory: (path, signal) => {
calls.push([path, signal])
return Promise.resolve('listing')
},
}),
get: (name) =>
({
uiWorkspace: {
listDirectory: (path, signal) => {
calls.push(['folders', path, signal])
return Promise.resolve('listing')
},
},
'remote.workspaceFiles': {
list: (sessionId, path, signal) => {
calls.push(['files', sessionId, path, signal])
return Promise.resolve({ ok: true, value: 'workspace listing' })
},
},
})[name],
})
assert.equal(present.available(), true)
assert.equal(await present.list('/tmp', 'signal'), 'listing')
assert.deepEqual(calls, [['/tmp', 'signal']])
assert.deepEqual(calls, [['folders', '/tmp', 'signal']])
// The remote answers with a result envelope rather than rejecting, so its
// failure branch is turned into one here — the model's catch expects a throw.
assert.equal(present.files.available(), true)
assert.equal(await present.files.list('session-1', '/work/root', 'signal'), 'workspace listing')
assert.deepEqual(calls[1], ['files', 'session-1', '/work/root', 'signal'])
const refusing = mod.directoryBrowse({
get: (name) =>
name === 'remote.workspaceFiles'
? { list: () => Promise.resolve({ ok: false, error: new Error('workspace files are unavailable') }) }
: undefined,
})
await assert.rejects(refusing.files.list('session-1', '/work/root'), /workspace files are unavailable/)
})
test('the picker model lists a level, supersedes a stale one, and keeps a typed path on failure', async () => {
@@ -578,7 +606,7 @@ test('the picker model lists a level, supersedes a stale one, and keeps a typed
const asked = []
const model = new mod.DirectoryPickerModel({
list: (path) => {
folders: (path) => {
asked.push(path)
return Promise.resolve(listingFixture())
},
@@ -588,7 +616,7 @@ test('the picker model lists a level, supersedes a stale one, and keeps a typed
await settle()
assert.deepEqual(asked, ['/work/root/src'], 'the opening level is listed, not guessed at')
assert.equal(model.getSnapshot().listing.path, '/work/root/src')
assert.equal(model.getSnapshot().listing.base, '/work/root/src')
assert.equal(model.getSnapshot().draft, '/work/root/src', 'the field follows a successful listing')
assert.equal(model.getSnapshot().loading, false)
@@ -597,10 +625,11 @@ test('the picker model lists a level, supersedes a stale one, and keeps a typed
model.open('/work/root/other')
model.setDraft('notes/CONTROLLER.md')
// A failure keeps the level on screen and the path as typed: the Host lists
// folders, and the extras legitimately name a file.
// A failure keeps the level on screen and the path as typed: a listing can
// fail on a path this panel may still legitimately declare.
const failing = new mod.DirectoryPickerModel({
list: () => Promise.reject(Object.assign(new Error('transport'), { rpcError: { message: 'directory-unreadable' } })),
folders: () =>
Promise.reject(Object.assign(new Error('transport'), { rpcError: { message: 'directory-unreadable' } })),
root: '/work/root',
start: 'notes/CONTROLLER.md',
})
@@ -613,7 +642,7 @@ test('the picker lists folders, hides hidden ones until asked, and answers in de
const { module: mod } = await loadClient()
const model = new mod.DirectoryPickerModel({
list: () => Promise.resolve(listingFixture()),
folders: () => Promise.resolve(listingFixture()),
root: '/work/root',
start: '/work/root/src',
})
@@ -668,7 +697,7 @@ test('the extras picker names what it accepts and says what the list holds', asy
const { module: mod } = await loadClient()
const model = new mod.DirectoryPickerModel({
list: () => Promise.resolve(listingFixture()),
folders: () => Promise.resolve(listingFixture()),
root: '/work/root',
start: '/work/root/src',
})
@@ -683,14 +712,14 @@ test('the extras picker names what it accepts and says what the list holds', asy
onPick: () => {},
})
// "folder or path" promised one thing and offered another: the rows are
// folders, and a file is added by naming it. The copy says both.
// "folder or path" promised one thing and offered another. The rows now carry
// files, and the copy says what a row of each kind does.
assert.equal(tree.props.title, 'Add a path or a file')
const texts = textsOf(tree)
assert.ok(texts.includes('Path or file'), 'the field is named for what it accepts')
assert.ok(
texts.some((text) => text.includes('The list holds folders')),
'the copy explains why no file rows are shown',
texts.some((text) => text.includes('Folders open; a file is taken')),
'the copy explains what a file row does',
)
assert.ok(
buttonsOf(tree).some((button) => (button.children ?? [])[0] === 'Add'),
@@ -698,3 +727,220 @@ test('the extras picker names what it accepts and says what the list holds', asy
)
})
// ── files in the extras picker ───────────────────────────────────────────────
//
// Two Host listers, one of which carries files. The folder primitive browses
// anywhere and returns folders; the workspace file listing returns files and
// folders but confines itself to the session's workspace. The extras picker
// takes the file listing wherever the Host allows it, and the folder listing
// everywhere else, so a path above the session folder is still reachable.
/** A level shaped the way `remote.workspaceFiles.list` answers. */
function workspaceFixture() {
return {
path: 'src',
entries: [
{ name: 'shared', type: 'directory' },
{ name: '.git', type: 'directory' },
{ name: 'index.ts', type: 'file', size: 120 },
{ name: 'CONTROLLER.md', type: 'file', size: 900 },
{ name: 'socket', type: 'other' },
],
truncated: false,
}
}
test('the model takes the file listing inside the workspace and the folder listing above it', async () => {
const { module: mod } = await loadClient()
const asked = []
const model = new mod.DirectoryPickerModel({
folders: (path) => {
asked.push(['folders', path])
return Promise.resolve(listingFixture())
},
files: (sessionId, path) => {
asked.push(['files', sessionId, path])
return Promise.resolve(workspaceFixture())
},
sessionId: 'session-1',
root: '/work/root',
start: '/work/root/src',
})
await settle()
// Inside the workspace: the file listing, normalised into the picker's own
// shape, with the workspace-relative paths joined onto the session folder.
assert.deepEqual(asked, [['files', 'session-1', '/work/root/src']])
const level = model.getSnapshot().listing
assert.equal(level.base, '/work/root/src')
assert.equal(level.files, true, 'this level could carry file rows')
assert.deepEqual(
level.entries.map((entry) => [entry.name, entry.path, entry.directory, entry.hidden]),
[
['shared', '/work/root/src/shared', true, false],
['.git', '/work/root/src/.git', true, true],
['index.ts', '/work/root/src/index.ts', false, false],
['CONTROLLER.md', '/work/root/src/CONTROLLER.md', false, false],
['socket', '/work/root/src/socket', false, false],
],
)
// The trail is the workspace's own, root first: the file listing stops at the
// session folder, so there is nothing above it to offer.
assert.deepEqual(
level.crumbs.map((crumb) => crumb.path),
['/work/root', '/work/root/src'],
)
// Above the workspace root: the Host confines the file listing to the
// workspace, so the level falls back to folders — which browse anywhere.
model.open('/work')
await settle()
assert.deepEqual(asked[1], ['folders', '/work'])
assert.equal(model.getSnapshot().listing.files, false)
})
test('a file listing that fails still leaves a browsable level', async () => {
const { module: mod } = await loadClient()
// The comparison the model makes is a prefix test on a path the Host may
// resolve differently — a symlinked session folder, say — so a refusal from
// the file listing is answered rather than shown: the folder listing runs.
const model = new mod.DirectoryPickerModel({
folders: () => Promise.resolve(listingFixture()),
files: () => Promise.reject(new Error('outside the workspace')),
sessionId: 'session-1',
root: '/work/root',
start: '/work/root/src',
})
await settle()
assert.equal(model.getSnapshot().error, '')
assert.equal(model.getSnapshot().listing.files, false)
assert.equal(model.getSnapshot().listing.base, '/work/root/src')
})
test('a file row is taken as the path to add rather than walked into', async () => {
const { module: mod } = await loadClient()
// Both listers refuse a path that names a file, which is what the Host does:
// the file listing because a file has no children, the folder listing because
// the same. Only a level can be listed.
const isFile = (path) => /\.(md|ts)$/.test(String(path))
const refuseFile = (path, answer) =>
isFile(path) ? Promise.reject(new Error('not a directory')) : Promise.resolve(answer)
const model = new mod.DirectoryPickerModel({
folders: (path) => refuseFile(path, listingFixture()),
files: (sessionId, path) => refuseFile(path, workspaceFixture()),
sessionId: 'session-1',
root: '/work/root',
start: '/work/root/src',
})
await settle()
const picked = []
const render = () =>
mod.DirectoryPicker({
model,
root: '/work/root',
target: 'extra',
disabled: false,
onClose: () => {},
onPick: (path) => picked.push(path),
})
const rowAt = (tree, path) =>
buttonsOf(tree)
.filter((button) => button.props.className === mod.ROW_CLASS)
.find((button) => button.props.title === path)
const fileRow = rowAt(render(), '/work/root/src/CONTROLLER.md')
assert.notEqual(fileRow, undefined, 'the file is a row in the listing')
assert.equal(fileRow.props.style.background, 'transparent', 'an unpicked file row is not marked')
// A folder row carries the folder glyph and a file row the file-type one, which
// is the only thing telling the two apart before the click.
const glyphOf = (row) => (row.children ?? [])[0].children?.[0]?.type
assert.equal(glyphOf(fileRow), 'FileTypeIcon')
assert.equal(glyphOf(rowAt(render(), '/work/root/src/shared')), 'IconFolderCloseRegular')
// Clicking it fills the field instead of listing it, and marks the row: a file
// is taken, not entered.
fileRow.props.onClick()
assert.equal(model.getSnapshot().draft, '/work/root/src/CONTROLLER.md')
assert.notEqual(rowAt(render(), '/work/root/src/CONTROLLER.md').props.style.background, 'transparent')
// Enter on a file the level lists asks for a listing of a file, which is not a
// failure: the level stays and nothing is reported.
model.open(model.getSnapshot().draft)
await settle()
assert.equal(model.getSnapshot().error, '')
assert.equal(model.getSnapshot().listing.base, '/work/root/src', 'the level survives it')
// The footer then adds exactly that, in the declared form.
const addButton = render().props.footer.find((entry) => (entry.children ?? [])[0] === 'Add')
addButton.props.onClick()
assert.deepEqual(picked, ['src/CONTROLLER.md'])
})
test('files are offered to the extras control only, and only where the Host has them', async () => {
const { module: mod } = await loadClient()
const browse = mod.directoryBrowse({
get: (name) => (name === 'remote.workspaceFiles' ? { list: () => {} } : undefined),
})
assert.equal(mod.fileLister(browse, 'extra'), browse.files.list, 'an extra can name a file')
// A focus is a folder, and file rows there would only offer something that
// cannot be chosen.
assert.equal(mod.fileLister(browse, 'focus'), undefined)
// No file service, or no browse face at all: the folder listing stands.
assert.equal(mod.fileLister(mod.directoryBrowse({ get: () => undefined }), 'extra'), undefined)
assert.equal(mod.fileLister({}, 'extra'), undefined)
assert.equal(mod.fileLister(undefined, 'extra'), undefined)
})
test('the picker says so when a level is folders only, and not when it is not', async () => { const { module: mod } = await loadClient()
const model = new mod.DirectoryPickerModel({
folders: () => Promise.resolve(listingFixture()),
root: '/work/root',
start: '/work/root/src',
})
await settle()
const tree = mod.DirectoryPicker({
model,
root: '/work/root',
target: 'extra',
disabled: false,
onClose: () => {},
onPick: () => {},
})
assert.ok(
textsOf(tree).some((text) => text.includes('Files are listed inside the session folder only')),
'a folders-only level explains itself',
)
// A level that does carry files needs no note about it.
const withFiles = new mod.DirectoryPickerModel({
folders: () => Promise.resolve(listingFixture()),
files: () => Promise.resolve(workspaceFixture()),
sessionId: 'session-1',
root: '/work/root',
start: '/work/root/src',
})
await settle()
assert.equal(
textsOf(
mod.DirectoryPicker({
model: withFiles,
root: '/work/root',
target: 'extra',
disabled: false,
onClose: () => {},
onPick: () => {},
}),
).some((text) => text.includes('Files are listed inside the session folder only')),
false,
)
})