Skip to content

Commit 6ebd893

Browse files
zxkanezxkane
authored
fix(docker): preserve project/<repo> path for nested repo git changes (#61)
## Summary - Fix Changes tab showing empty for conversations with connected GitHub repos - `normalizeGitUrl()` now preserves `project/<repo>` path so the agent-server resolves to the correct nested git repository at `/workspace/project/<repo>/` - Remove unnecessary `git init /workspace/project` from sandbox entrypoint that created an intermediate git repo shadowing the cloned repo's changes **Root cause**: When a repo (e.g., `zxkane/openhands-infra`) is connected, it's cloned to `/workspace/project/openhands-infra/`. Previously, git API paths were normalized to `.` (workspace root), which queried the outer `/workspace` repo instead of the nested cloned repo — making file modifications invisible in the Changes tab. ## Test plan - [x] Build passes (`npm run build`) - [x] Unit tests pass (`npm run test:ts` — relevant tests) - [x] Regression tests pass (`node docker/test_patch_fix_git_paths.js` — 19 cases) - [x] CI checks pass - [x] Reviewer bot findings addressed (no new findings) - [x] Deployed to staging - [x] **E2E tests pass** (run `./test/select-e2e-tests.sh` for required tests) - [x] TC-003: Login - [x] TC-004: Conversation List - [x] TC-005: New Conversation - [x] TC-031: Changes Tab With GitHub Repo ## Checklist - [x] Regression tests updated (19 test cases in `docker/test_patch_fix_git_paths.js`) - [x] E2E test cases updated (TC-031 in `test/E2E_TEST_CASES.md`) - [x] Snapshot updated (`test/__snapshots__/stacks.test.ts.snap`)
1 parent 6a0ace5 commit 6ebd893

5 files changed

Lines changed: 223 additions & 107 deletions

File tree

‎docker/patch-fix.js‎

Lines changed: 49 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -145,52 +145,76 @@
145145
}
146146
}
147147

148-
// Normalize git API paths - convert absolute workspace paths and bare repo names to relative paths.
149-
// The agent-server's git router expects "." for the workspace root, or file paths relative to it.
148+
// Normalize git API paths - preserve repo subdirectory so the agent-server resolves
149+
// to the correct cloned repo at /workspace/project/<repo>/.
150+
//
151+
// The agent-server WORKDIR is /workspace. The git_router receives the path and resolves
152+
// it relative to /workspace. When a GitHub repo is connected, the clone lives at
153+
// /workspace/project/<repo>/, so we must preserve "project/<repo>" in the path.
150154
//
151155
// The frontend sends paths in these forms:
152-
// /api/git/changes/%2Fworkspace%2Fproject%2F<repo> -> changes for repo root -> "."
153-
// /api/git/diff/%2Fworkspace%2Fproject%2F<repo>%2F<file> -> diff for specific file -> <file>
154-
// /api/git/changes/<repo> -> bare repo name -> "."
155-
// /api/git/diff/<repo>%2F<file> -> bare repo + file -> <file>
156-
// /api/git/changes//workspace/project/<repo> -> non-encoded -> "."
157-
// /api/git/diff//workspace/project/<repo>/<file> -> non-encoded -> <file>
156+
// /api/git/changes/%2Fworkspace%2Fproject%2F<repo> -> project/<repo>
157+
// /api/git/diff/%2Fworkspace%2Fproject%2F<repo>%2F<file> -> project/<repo>/<file>
158+
// /api/git/changes/<repo> -> project/<repo>
159+
// /api/git/diff/<repo>%2F<file> -> project/<repo>/<file>
160+
// /api/git/changes//workspace/project/<repo> -> project/<repo>
161+
// /api/git/diff//workspace/project/<repo>/<file> -> project/<repo>/<file>
162+
// /api/git/changes/%2Fworkspace%2Fproject -> . (no repo)
163+
// /api/git/changes//workspace/project -> . (no repo)
164+
// /api/git/diff/%2Fworkspace%2Fproject%2F<file> -> ./<file> (no repo, file)
165+
// /api/git/diff//workspace/project/<file> -> ./<file> (no repo, file)
166+
//
167+
// Repo names never start with "." — segments starting with "." are files in workspace root.
158168
function normalizeGitUrl(url) {
159169
var before = url;
160-
// URL-encoded: strip %2Fworkspace%2Fproject%2F<repo-name> prefix
161-
// The repo directory is part of the workspace path and must be stripped too.
162-
// After %2Fproject%2F, the first segment is the repo name; anything after is a file path.
163-
url = url.replace(/(\/api\/git\/[^/]+)\/%2F(workspace|openhands)%2Fproject%2F([^%/]+)(%2F(.*))?$/gi,
170+
// URL-encoded: %2Fworkspace%2Fproject%2F<repo-name> with optional file path
171+
// Repo names never start with "." — dotfiles are workspace root files, not repos.
172+
url = url.replace(/(\/api\/git\/[^/]+)\/%2F(workspace|openhands)%2Fproject%2F([^%.][^%/]*)(%2F(.*))?$/gi,
164173
function(match, prefix, ws, repo, hasMore, filePath) {
165174
if (filePath) {
166-
return prefix + '/' + filePath;
175+
return prefix + '/project/' + repo + '/' + filePath;
176+
}
177+
return prefix + '/project/' + repo;
178+
});
179+
// URL-encoded: workspace root with optional file path -> "." or "./<file>"
180+
url = url.replace(/(\/api\/git\/[^/]+)\/%2F(workspace|openhands)%2Fproject(%2F(.*))?$/gi,
181+
function(match, prefix, ws, hasMore, filePath) {
182+
if (filePath) {
183+
return prefix + '/./' + filePath;
167184
}
168185
return prefix + '/.';
169186
});
170-
// URL-encoded: exact workspace root (no repo name)
171-
url = url.replace(/(\/api\/git\/[^/]+)\/%2F(workspace|openhands)%2Fproject$/gi, '$1/.');
172-
// Non-encoded: strip //workspace/project/<repo-name> prefix
173-
url = url.replace(/(\/api\/git\/[^/]+)\/\/(workspace|openhands)\/project\/([^/]+)(\/(.*))?$/g,
187+
// Non-encoded: //workspace/project/<repo-name> with optional file path
188+
// Repo names never start with "." — dotfiles are workspace root files, not repos.
189+
url = url.replace(/(\/api\/git\/[^/]+)\/\/(workspace|openhands)\/project\/([^/.][^/]*)(\/(.*))?$/g,
174190
function(match, prefix, ws, repo, hasMore, filePath) {
175191
if (filePath) {
176-
return prefix + '/' + filePath;
192+
return prefix + '/project/' + repo + '/' + filePath;
193+
}
194+
return prefix + '/project/' + repo;
195+
});
196+
// Non-encoded: workspace root with optional file path -> "." or "./<file>"
197+
url = url.replace(/(\/api\/git\/[^/]+)\/\/(workspace|openhands)\/project(\/(.*))?$/g,
198+
function(match, prefix, ws, hasMore, filePath) {
199+
if (filePath) {
200+
return prefix + '/./' + filePath;
177201
}
178202
return prefix + '/.';
179203
});
180-
// Non-encoded: exact workspace root (no repo name)
181-
url = url.replace(/(\/api\/git\/[^/]+)\/\/(workspace|openhands)\/project$/g, '$1/.');
182204
// Bare repo name or repo+file (no workspace prefix).
183-
// Only run if no workspace stripping regex matched above — otherwise it would
184-
// clobber valid file paths (e.g., "README.md" -> "." or "src%2Findex.ts" -> "index.ts").
205+
// Only run if no workspace stripping regex matched above.
185206
if (url === before) {
186207
url = url.replace(/(\/api\/git\/[^/]+)\/([^/.][^/]*)$/g, function(match, prefix, segment) {
187208
var idx = segment.indexOf('%2F');
188209
if (idx === -1) idx = segment.indexOf('%2f');
189210
if (idx !== -1) {
190-
// repo%2Ffile -> file
191-
return prefix + '/' + segment.substring(idx + 3);
211+
// repo%2Ffile -> project/repo/file
212+
var repo = segment.substring(0, idx);
213+
var file = segment.substring(idx + 3);
214+
return prefix + '/project/' + repo + '/' + file;
192215
}
193-
return prefix + '/.';
216+
// bare repo name -> project/repo
217+
return prefix + '/project/' + segment;
194218
});
195219
}
196220
return url;

‎docker/test_patch_fix_git_paths.js‎

Lines changed: 96 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,41 +1,59 @@
11
/**
22
* Regression test for patch-fix.js git path normalization.
33
*
4-
* The agent-server git router expects "." for the workspace root,
5-
* or file paths relative to the workspace root for diff requests.
4+
* The agent-server WORKDIR is /workspace. When a GitHub repo is connected,
5+
* the clone lives at /workspace/project/<repo>/. The normalizeGitUrl function
6+
* must preserve "project/<repo>" in the path so the agent-server resolves to
7+
* the correct git repository.
68
*
79
* The frontend sends paths in these forms:
8-
* %2Fworkspace%2Fproject%2F<repo> -> "." (changes)
9-
* %2Fworkspace%2Fproject%2F<repo>%2F<file> -> <file> (diff)
10-
* <repo> -> "." (changes)
11-
* <repo>%2F<file> -> <file> (diff)
12-
* //workspace/project/<repo> -> "." (changes)
13-
* //workspace/project/<repo>/<file> -> <file> (diff)
10+
* %2Fworkspace%2Fproject%2F<repo> -> project/<repo> (changes)
11+
* %2Fworkspace%2Fproject%2F<repo>%2F<file> -> project/<repo>/<file> (diff)
12+
* <repo> -> project/<repo> (changes)
13+
* <repo>%2F<file> -> project/<repo>/<file> (diff)
14+
* //workspace/project/<repo> -> project/<repo> (changes)
15+
* //workspace/project/<repo>/<file> -> project/<repo>/<file> (diff)
16+
* %2Fworkspace%2Fproject -> . (no repo)
17+
* //workspace/project -> . (no repo)
18+
* %2Fworkspace%2Fproject%2F<dotfile> -> ./<dotfile> (no repo, file)
19+
* //workspace/project/<dotfile> -> ./<dotfile> (no repo, file)
1420
*
1521
* Run: node docker/test_patch_fix_git_paths.js
1622
*/
1723

1824
// Mirrors the normalizeGitUrl function from patch-fix.js exactly.
1925
function normalizeGitUrl(url) {
2026
var before = url;
21-
url = url.replace(/(\/api\/git\/[^/]+)\/%2F(workspace|openhands)%2Fproject%2F([^%/]+)(%2F(.*))?$/gi,
27+
url = url.replace(/(\/api\/git\/[^/]+)\/%2F(workspace|openhands)%2Fproject%2F([^%.][^%/]*)(%2F(.*))?$/gi,
2228
function(match, prefix, ws, repo, hasMore, filePath) {
23-
if (filePath) { return prefix + '/' + filePath; }
29+
if (filePath) { return prefix + '/project/' + repo + '/' + filePath; }
30+
return prefix + '/project/' + repo;
31+
});
32+
url = url.replace(/(\/api\/git\/[^/]+)\/%2F(workspace|openhands)%2Fproject(%2F(.*))?$/gi,
33+
function(match, prefix, ws, hasMore, filePath) {
34+
if (filePath) { return prefix + '/./' + filePath; }
2435
return prefix + '/.';
2536
});
26-
url = url.replace(/(\/api\/git\/[^/]+)\/%2F(workspace|openhands)%2Fproject$/gi, '$1/.');
27-
url = url.replace(/(\/api\/git\/[^/]+)\/\/(workspace|openhands)\/project\/([^/]+)(\/(.*))?$/g,
37+
url = url.replace(/(\/api\/git\/[^/]+)\/\/(workspace|openhands)\/project\/([^/.][^/]*)(\/(.*))?$/g,
2838
function(match, prefix, ws, repo, hasMore, filePath) {
29-
if (filePath) { return prefix + '/' + filePath; }
39+
if (filePath) { return prefix + '/project/' + repo + '/' + filePath; }
40+
return prefix + '/project/' + repo;
41+
});
42+
url = url.replace(/(\/api\/git\/[^/]+)\/\/(workspace|openhands)\/project(\/(.*))?$/g,
43+
function(match, prefix, ws, hasMore, filePath) {
44+
if (filePath) { return prefix + '/./' + filePath; }
3045
return prefix + '/.';
3146
});
32-
url = url.replace(/(\/api\/git\/[^/]+)\/\/(workspace|openhands)\/project$/g, '$1/.');
3347
if (url === before) {
3448
url = url.replace(/(\/api\/git\/[^/]+)\/([^/.][^/]*)$/g, function(match, prefix, segment) {
3549
var idx = segment.indexOf('%2F');
3650
if (idx === -1) idx = segment.indexOf('%2f');
37-
if (idx !== -1) { return prefix + '/' + segment.substring(idx + 3); }
38-
return prefix + '/.';
51+
if (idx !== -1) {
52+
var repo = segment.substring(0, idx);
53+
var file = segment.substring(idx + 3);
54+
return prefix + '/project/' + repo + '/' + file;
55+
}
56+
return prefix + '/project/' + segment;
3957
});
4058
}
4159
return url;
@@ -62,14 +80,14 @@ const base = 'https://example.com/runtime/abc123/4443';
6280
// =============================================
6381
assertEqual(
6482
normalizeGitUrl(`${base}/api/git/changes/openhands-infra`),
65-
`${base}/api/git/changes/.`,
66-
'changes: bare repo name -> .'
83+
`${base}/api/git/changes/project/openhands-infra`,
84+
'changes: bare repo name -> project/repo'
6785
);
6886

6987
assertEqual(
7088
normalizeGitUrl(`${base}/api/git/changes/my-project`),
71-
`${base}/api/git/changes/.`,
72-
'changes: bare repo name (my-project) -> .'
89+
`${base}/api/git/changes/project/my-project`,
90+
'changes: bare repo name (my-project) -> project/my-project'
7391
);
7492

7593
// Already-correct "." path should not be changed
@@ -90,14 +108,14 @@ assertEqual(
90108

91109
assertEqual(
92110
normalizeGitUrl(`${base}/api/git/changes/%2Fworkspace%2Fproject%2Fopenhands-infra`),
93-
`${base}/api/git/changes/.`,
94-
'changes: URL-encoded /workspace/project/openhands-infra -> .'
111+
`${base}/api/git/changes/project/openhands-infra`,
112+
'changes: URL-encoded /workspace/project/openhands-infra -> project/openhands-infra'
95113
);
96114

97115
assertEqual(
98116
normalizeGitUrl(`${base}/api/git/changes/%2Fworkspace%2Fproject%2Fmy-app`),
99-
`${base}/api/git/changes/.`,
100-
'changes: URL-encoded /workspace/project/my-app -> .'
117+
`${base}/api/git/changes/project/my-app`,
118+
'changes: URL-encoded /workspace/project/my-app -> project/my-app'
101119
);
102120

103121
assertEqual(
@@ -108,8 +126,8 @@ assertEqual(
108126

109127
assertEqual(
110128
normalizeGitUrl(`${base}/api/git/changes/%2Fopenhands%2Fproject%2Fmy-repo`),
111-
`${base}/api/git/changes/.`,
112-
'changes: URL-encoded /openhands/project/my-repo -> .'
129+
`${base}/api/git/changes/project/my-repo`,
130+
'changes: URL-encoded /openhands/project/my-repo -> project/my-repo'
113131
);
114132

115133
// =============================================
@@ -123,46 +141,78 @@ assertEqual(
123141

124142
assertEqual(
125143
normalizeGitUrl(`${base}/api/git/changes//workspace/project/openhands-infra`),
126-
`${base}/api/git/changes/.`,
127-
'changes: non-encoded //workspace/project/openhands-infra -> .'
144+
`${base}/api/git/changes/project/openhands-infra`,
145+
'changes: non-encoded //workspace/project/openhands-infra -> project/openhands-infra'
146+
);
147+
148+
// =============================================
149+
// DIFF API - no repo, workspace root file (URL-encoded)
150+
// =============================================
151+
// When no repo is connected, files like .gitignore are at workspace root.
152+
// Frontend sends: %2Fworkspace%2Fproject%2F.gitignore (dotfile = not a repo)
153+
assertEqual(
154+
normalizeGitUrl(`${base}/api/git/diff/%2Fworkspace%2Fproject%2F.gitignore`),
155+
`${base}/api/git/diff/./.gitignore`,
156+
'diff: URL-encoded /workspace/project/.gitignore -> ./.gitignore (no repo, dotfile)'
157+
);
158+
159+
assertEqual(
160+
normalizeGitUrl(`${base}/api/git/diff/%2Fworkspace%2Fproject%2F.env`),
161+
`${base}/api/git/diff/./.env`,
162+
'diff: URL-encoded /workspace/project/.env -> ./.env (no repo, dotfile)'
163+
);
164+
165+
// =============================================
166+
// DIFF API - no repo, workspace root file (non-encoded)
167+
// =============================================
168+
assertEqual(
169+
normalizeGitUrl(`${base}/api/git/diff//workspace/project/.gitignore`),
170+
`${base}/api/git/diff/./.gitignore`,
171+
'diff: non-encoded //workspace/project/.gitignore -> ./.gitignore (no repo, dotfile)'
172+
);
173+
174+
assertEqual(
175+
normalizeGitUrl(`${base}/api/git/diff//workspace/project/.env.local`),
176+
`${base}/api/git/diff/./.env.local`,
177+
'diff: non-encoded //workspace/project/.env.local -> ./.env.local (no repo, dotfile)'
128178
);
129179

130180
// =============================================
131-
// DIFF API - URL-encoded workspace + repo + file (THE ACTUAL BUG)
181+
// DIFF API - URL-encoded workspace + repo + file
132182
// =============================================
133183
// Frontend sends: %2Fworkspace%2Fproject%2Fopenhands-infra%2F.gitignore
134-
// Should become: .gitignore (file relative to workspace root)
184+
// Should become: project/openhands-infra/.gitignore (preserving repo path)
135185
assertEqual(
136186
normalizeGitUrl(`${base}/api/git/diff/%2Fworkspace%2Fproject%2Fopenhands-infra%2F.gitignore`),
137-
`${base}/api/git/diff/.gitignore`,
138-
'diff: URL-encoded /workspace/project/repo/.gitignore -> .gitignore'
187+
`${base}/api/git/diff/project/openhands-infra/.gitignore`,
188+
'diff: URL-encoded /workspace/project/repo/.gitignore -> project/repo/.gitignore'
139189
);
140190

141191
assertEqual(
142192
normalizeGitUrl(`${base}/api/git/diff/%2Fworkspace%2Fproject%2Fopenhands-infra%2Fsrc%2Findex.ts`),
143-
`${base}/api/git/diff/src%2Findex.ts`,
144-
'diff: URL-encoded /workspace/project/repo/src/index.ts -> src%2Findex.ts'
193+
`${base}/api/git/diff/project/openhands-infra/src%2Findex.ts`,
194+
'diff: URL-encoded /workspace/project/repo/src/index.ts -> project/repo/src%2Findex.ts'
145195
);
146196

147197
assertEqual(
148198
normalizeGitUrl(`${base}/api/git/diff/%2Fworkspace%2Fproject%2Fmy-app%2FREADME.md`),
149-
`${base}/api/git/diff/README.md`,
150-
'diff: URL-encoded /workspace/project/my-app/README.md -> README.md'
199+
`${base}/api/git/diff/project/my-app/README.md`,
200+
'diff: URL-encoded /workspace/project/my-app/README.md -> project/my-app/README.md'
151201
);
152202

153203
// =============================================
154204
// DIFF API - non-encoded workspace + repo + file
155205
// =============================================
156206
assertEqual(
157207
normalizeGitUrl(`${base}/api/git/diff//workspace/project/openhands-infra/.gitignore`),
158-
`${base}/api/git/diff/.gitignore`,
159-
'diff: non-encoded //workspace/project/repo/.gitignore -> .gitignore'
208+
`${base}/api/git/diff/project/openhands-infra/.gitignore`,
209+
'diff: non-encoded //workspace/project/repo/.gitignore -> project/repo/.gitignore'
160210
);
161211

162212
assertEqual(
163213
normalizeGitUrl(`${base}/api/git/diff//workspace/project/openhands-infra/src/index.ts`),
164-
`${base}/api/git/diff/src/index.ts`,
165-
'diff: non-encoded //workspace/project/repo/src/index.ts -> src/index.ts'
214+
`${base}/api/git/diff/project/openhands-infra/src/index.ts`,
215+
'diff: non-encoded //workspace/project/repo/src/index.ts -> project/repo/src/index.ts'
166216
);
167217

168218
// =============================================
@@ -171,20 +221,20 @@ assertEqual(
171221
// Frontend sends: openhands-infra%2F.gitignore (from user's report)
172222
assertEqual(
173223
normalizeGitUrl(`${base}/api/git/diff/openhands-infra%2F.gitignore`),
174-
`${base}/api/git/diff/.gitignore`,
175-
'diff: bare repo%2F.gitignore -> .gitignore'
224+
`${base}/api/git/diff/project/openhands-infra/.gitignore`,
225+
'diff: bare repo%2F.gitignore -> project/repo/.gitignore'
176226
);
177227

178228
assertEqual(
179229
normalizeGitUrl(`${base}/api/git/diff/openhands-infra%2Fproject%2Fopenhands-infra`),
180-
`${base}/api/git/diff/project%2Fopenhands-infra`,
181-
'diff: bare repo%2Fproject%2Frepo -> project%2Frepo'
230+
`${base}/api/git/diff/project/openhands-infra/project%2Fopenhands-infra`,
231+
'diff: bare repo%2Fproject%2Frepo -> project/repo/project%2Frepo'
182232
);
183233

184234
assertEqual(
185235
normalizeGitUrl(`${base}/api/git/diff/some-repo`),
186-
`${base}/api/git/diff/.`,
187-
'diff: bare repo name only -> .'
236+
`${base}/api/git/diff/project/some-repo`,
237+
'diff: bare repo name only -> project/repo'
188238
);
189239

190240
// =============================================

‎lib/sandbox-stack.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -367,7 +367,6 @@ export class SandboxStack extends cdk.Stack {
367367
'git init /workspace 2>/dev/null;' +
368368
'printf "bash_events/\\nconversations/\\n*.pyc\\n__pycache__/\\n" > /workspace/.gitignore;' +
369369
'cd /workspace && git add .gitignore && git -c user.name=openhands -c user.email=oh@local commit -qm init 2>/dev/null;' +
370-
'git init /workspace/project 2>/dev/null;' +
371370
'exec /usr/local/bin/openhands-agent-server --port 8000'
372371
],
373372
linuxParameters: new ecs.LinuxParameters(this, 'SandboxLinuxParams', {

0 commit comments

Comments
 (0)