Review comments

This commit is contained in:
Osvaldo Ortega
2026-03-02 17:41:00 -08:00
parent 3275c13eea
commit 8735867e70
5 changed files with 45 additions and 43 deletions

View File

@@ -126,29 +126,33 @@ function buildTreeChildren(items: IChangesFileItem[]): IObjectTreeElement<Change
const root: FolderNode = { name: '', uri: URI.file('/'), children: new Map(), files: [] };
for (const item of items) {
let dirPath = dirname(item.uri.path);
const fullDirPath = dirname(item.uri.path);
// For github-remote-file URIs, strip the /{owner}/{repo}/{ref} prefix
// so the tree shows repo-relative paths instead of internal URI segments.
let displayDirPath = fullDirPath;
let uriBasePrefix = '';
if (item.uri.scheme === GITHUB_REMOTE_FILE_SCHEME) {
const parts = dirPath.split('/').filter(Boolean);
const parts = fullDirPath.split('/').filter(Boolean);
if (parts.length >= 3) {
dirPath = '/' + parts.slice(3).join('/');
uriBasePrefix = '/' + parts.slice(0, 3).join('/');
displayDirPath = '/' + parts.slice(3).join('/');
} else {
dirPath = '/';
uriBasePrefix = '/' + parts.join('/');
displayDirPath = '/';
}
}
const segments = dirPath.split('/').filter(Boolean);
const segments = displayDirPath.split('/').filter(Boolean);
let current = root;
let currentPath = '';
let currentFullPath = uriBasePrefix;
for (const segment of segments) {
currentPath += '/' + segment;
currentFullPath += '/' + segment;
if (!current.children.has(segment)) {
current.children.set(segment, {
name: segment,
uri: item.uri.with({ path: currentPath }),
uri: item.uri.with({ path: currentFullPath }),
children: new Map(),
files: []
});

View File

@@ -315,7 +315,8 @@ export class FileTreeViewPane extends ViewPane {
* Extracts a github-remote-file:// URI from session metadata, trying various known fields.
*/
private extractRepoUriFromMetadata(metadata: { readonly [key: string]: unknown }): URI | undefined {
const ref = metadata?.branch || 'HEAD';
const branch = typeof metadata.branch === 'string' ? metadata.branch : 'HEAD';
const encodedRef = encodeURIComponent(branch);
// repositoryNwo: "owner/repo"
const repositoryNwo = metadata.repositoryNwo as string | undefined;
@@ -324,7 +325,7 @@ export class FileTreeViewPane extends ViewPane {
return URI.from({
scheme: GITHUB_REMOTE_FILE_SCHEME,
authority: 'github',
path: `/${repositoryNwo}/${ref}`,
path: `/${repositoryNwo}/${encodedRef}`,
});
}
@@ -337,7 +338,7 @@ export class FileTreeViewPane extends ViewPane {
return URI.from({
scheme: GITHUB_REMOTE_FILE_SCHEME,
authority: 'github',
path: `/${parsed.owner}/${parsed.repo}/${ref}`,
path: `/${parsed.owner}/${parsed.repo}/${encodedRef}`,
});
}
}
@@ -351,7 +352,7 @@ export class FileTreeViewPane extends ViewPane {
return URI.from({
scheme: GITHUB_REMOTE_FILE_SCHEME,
authority: 'github',
path: `/${repository}/${ref}`,
path: `/${repository}/${encodedRef}`,
});
}
const parsed = this.parseGitHubUrl(repository);
@@ -360,7 +361,7 @@ export class FileTreeViewPane extends ViewPane {
return URI.from({
scheme: GITHUB_REMOTE_FILE_SCHEME,
authority: 'github',
path: `/${parsed.owner}/${parsed.repo}/${ref}`,
path: `/${parsed.owner}/${parsed.repo}/${encodedRef}`,
});
}
}

View File

@@ -23,10 +23,15 @@ export function getGitHubRemoteFileDisplayName(uri: URI): string | undefined {
return undefined;
}
const parts = uri.path.split('/').filter(Boolean);
// path = /{owner}/{repo}/{ref}
// path = /{owner}/{repo}/{ref}/...
if (parts.length >= 3) {
const [, ref,] = parts;
return ref;
const [, repo, ref] = parts;
const decodedRepo = decodeURIComponent(repo);
const decodedRef = decodeURIComponent(ref);
if (decodedRef === 'HEAD') {
return decodedRepo;
}
return `${decodedRepo} (${decodedRef})`;
}
return undefined;
}
@@ -118,10 +123,10 @@ export class GitHubFileSystemProvider extends Disposable implements IFileSystemP
throw createFileSystemProviderError('Invalid github-remote-file URI: expected /{owner}/{repo}/{ref}/...', FileSystemProviderErrorCode.FileNotFound);
}
const owner = parts[0];
const repo = parts[1];
const ref = parts[2];
const path = parts.slice(3).join('/');
const owner = decodeURIComponent(parts[0]);
const repo = decodeURIComponent(parts[1]);
const ref = decodeURIComponent(parts[2]);
const path = parts.slice(3).map(decodeURIComponent).join('/');
return { owner, repo, ref, path };
}
@@ -133,15 +138,8 @@ export class GitHubFileSystemProvider extends Disposable implements IFileSystemP
// --- GitHub API
private async getAuthToken(): Promise<string> {
// Try to get an existing session silently before prompting the user
const sessions = await this.authenticationService.getSessions('github', ['repo'], {}, true);
if (sessions.length > 0) {
return sessions[0].accessToken;
}
// No existing session found — create one (may prompt the user)
const session = await this.authenticationService.createSession('github', ['repo']);
return session.accessToken;
const sessions = await this.authenticationService.getSessions('github', ['repo'], { silent: true }) ?? await this.authenticationService.getSessions('github', ['repo'], { createIfNone: true });
return sessions[0].accessToken ?? '';
}
private fetchTree(owner: string, repo: string, ref: string): Promise<ITreeCacheEntry> {
@@ -187,18 +185,14 @@ export class GitHubFileSystemProvider extends Disposable implements IFileSystemP
},
}, CancellationToken.None);
let data: IGitHubTreeResponse | null;
try {
data = await asJson<IGitHubTreeResponse>(response);
} catch (err) {
// Cache 404s so we don't keep re-fetching missing trees
if (err instanceof Error && err.message.includes('404')) {
this.notFoundCache.set(cacheKey, Date.now());
throw createFileSystemProviderError(`Tree not found for ${owner}/${repo}@${ref}`, FileSystemProviderErrorCode.FileNotFound);
}
throw err;
// Cache 404s so we don't keep re-fetching missing trees
if (response.res.statusCode === 404) {
this.notFoundCache.set(cacheKey, Date.now());
throw createFileSystemProviderError(`Tree not found for ${owner}/${repo}@${ref}`, FileSystemProviderErrorCode.FileNotFound);
}
const data = await asJson<IGitHubTreeResponse>(response);
if (!data) {
throw createFileSystemProviderError(`Failed to fetch tree for ${owner}/${repo}@${ref}`, FileSystemProviderErrorCode.Unavailable);
}

View File

@@ -204,8 +204,13 @@ export class SessionsManagementService extends Disposable implements ISessionsMa
if (session.providerType === AgentSessionProviders.Cloud) {
//TODO: @osortega pass branch in metadata from extension
const ref = metadata?.branch || 'HEAD';
return [URI.parse(`${GITHUB_REMOTE_FILE_SCHEME}://github/${metadata.owner}/${metadata.name}/${ref}`), undefined];
const branch = typeof metadata.branch === 'string' ? metadata.branch : 'HEAD';
const repositoryUri = URI.from({
scheme: GITHUB_REMOTE_FILE_SCHEME,
authority: 'github',
path: `/${metadata.owner}/${metadata.name}/${encodeURIComponent(branch)}`
});
return [repositoryUri, undefined];
}
const workingDirectoryPath = metadata?.workingDirectoryPath as string | undefined;

View File

@@ -102,6 +102,4 @@ export class WorkspaceFolderManagementContribution extends Disposable implements
private isUriTrusted(uri: URI): boolean {
return this.workspaceTrustManagementService.getTrustedUris().some(trustedUri => this.uriIdentityService.extUri.isEqual(trustedUri, uri));
}
}