Skip to content

Commit fd03a0f

Browse files
committed
fix(shell): shorten terminal label context
1 parent d02ecf8 commit fd03a0f

5 files changed

Lines changed: 101 additions & 45 deletions

File tree

packages/api/tests/terminal-sessions.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -414,7 +414,7 @@ describe("terminal sessions service", () => {
414414
status: "ready"
415415
})
416416
await expect(runTestEffect(lookupTerminalSessionById(second.session.id))).resolves.toMatchObject({
417-
projectDisplayName: "org/repo | issue #7 (https://github.com/org/repo/issues/7) | container dg-repo-issue-7",
417+
projectDisplayName: "https://github.com/org/repo/issues/7 | container dg-repo-issue-7",
418418
projectKey,
419419
session: {
420420
id: second.session.id,

packages/app/tests/docker-git/open-project-ssh.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,9 @@ describe("openResolvedProjectSshWithUpEffect", () => {
6969
})
7070
const events = yield* _(captureOpenResolvedProjectSshWithUpEvents(item))
7171
expect(events).toEqual([
72-
"progress:Starting project before SSH: org/repo | source https://github.com/org/repo.git | container dg-repo",
72+
"progress:Starting project before SSH: https://github.com/org/repo.git | container dg-repo",
7373
"up:/controller/org/repo/issue-9",
74-
"progress:Opening SSH terminal: org/repo | source https://github.com/org/repo.git | container dg-repo",
74+
"progress:Opening SSH terminal: https://github.com/org/repo.git | container dg-repo",
7575
"open:ssh -p 2299 dev@127.0.0.1"
7676
])
7777
}))

packages/terminal/src/core/project-terminal-label.ts

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -48,21 +48,25 @@ const sourceUrlForContext = (repoUrl: string, path: string): string | null => {
4848

4949
const renderIssueContext = (repoUrl: string, issueId: string): string => {
5050
const issueUrl = sourceUrlForContext(repoUrl, `issues/${issueId}`)
51-
return issueUrl === null ? `issue #${issueId}` : `issue #${issueId} (${issueUrl})`
51+
return issueUrl === null ? `issue #${issueId}` : issueUrl
5252
}
5353

5454
const renderPullRequestContext = (repoUrl: string, pullRequestId: string): string => {
5555
const pullRequestUrl = sourceUrlForContext(repoUrl, `pull/${pullRequestId}`)
56-
return pullRequestUrl === null ? `PR #${pullRequestId}` : `PR #${pullRequestId} (${pullRequestUrl})`
56+
return pullRequestUrl === null ? `PR #${pullRequestId}` : pullRequestUrl
5757
}
5858

5959
const renderMergeRequestContext = (mergeRequestId: string): string => `MR #${mergeRequestId}`
6060

6161
const renderSourceContext = (repoUrl: string, repoRef: string): string => {
62+
const trimmedUrl = repoUrl.trim()
6263
const trimmedRef = repoRef.trim()
64+
if (trimmedUrl.length === 0) {
65+
return trimmedRef.length === 0 || trimmedRef === "main" ? "" : trimmedRef
66+
}
6367
return trimmedRef.length === 0 || trimmedRef === "main"
64-
? `source ${repoUrl.trim()}`
65-
: `source ${repoUrl.trim()} (${trimmedRef})`
68+
? trimmedUrl
69+
: `${trimmedUrl} (${trimmedRef})`
6670
}
6771

6872
const parseWrappedNumericRef = (value: string, prefix: string, suffix: string): string | null => {
@@ -98,36 +102,38 @@ const appendNonEmpty = (parts: ReadonlyArray<string>, value: string): ReadonlyAr
98102
}
99103

100104
/**
101-
* Builds the terminal-facing project label with source workspace context.
105+
* Builds the terminal-facing project label with source link and container identity.
102106
*
103107
* @param project - Project identity returned by the docker-git API.
104108
* @returns A deterministic label for SSH terminal headers and ready messages.
105109
*
106110
* @pure true
107111
* @effect none
108-
* @invariant issue refs include an issue marker; PR refs include a PR marker; labels never omit displayName.
112+
* @invariant GitHub issue/PR refs prefer canonical source URLs; labels preserve non-empty containerName.
109113
* @precondition project.displayName identifies the repository or fallback project label.
110-
* @postcondition result contains displayName, workspace source context, and non-empty containerName when present.
114+
* @postcondition result contains workspace source link/context and non-empty containerName when present.
111115
* @complexity O(n) where n = |repoUrl| + |repoRef|
112116
* @throws Never
113117
*/
114-
// CHANGE: surface clone-source context in SSH terminal labels
115-
// WHY: terminal headers must identify issue/PR source and container instead of only the repo path
116-
// QUOTE(ТЗ): "надо писать какой Issues какой PR вообещ что за конетейнер"
118+
// CHANGE: keep SSH terminal labels to source link/context plus container identity
119+
// WHY: verbose repository + issue text duplicates the source URL and crowds the terminal header
120+
// QUOTE(ТЗ): "ссылки и название контейнера будет предостаточно"
117121
// REF: issue-370
118122
// SOURCE: n/a
119-
// FORMAT THEOREM: forall p: label(p) contains displayName(p) and context(repoUrl(p), repoRef(p))
123+
// FORMAT THEOREM: forall p: label(p) contains context(repoUrl(p), repoRef(p)) or containerName(p)
120124
// PURITY: CORE
121125
// EFFECT: none
122126
// INVARIANT: issue-* -> issue context; refs/pull/*/head -> PR context; containerName is preserved when non-empty
123127
// COMPLEXITY: O(n)
124128
export const projectTerminalLabel = (project: ProjectTerminalLabelInput): string => {
125-
const displayName = project.displayName.trim()
126-
const baseName = displayName.length === 0 ? project.repoUrl.trim() : displayName
127-
const withContext = appendNonEmpty([baseName], renderWorkspaceContext(project.repoUrl, project.repoRef))
129+
const withContext = appendNonEmpty([], renderWorkspaceContext(project.repoUrl, project.repoRef))
128130
const containerName = project.containerName?.trim() ?? ""
129131
const withContainer = containerName.length === 0
130132
? withContext
131133
: appendNonEmpty(withContext, `container ${containerName}`)
132-
return withContainer.join(" | ")
134+
if (withContainer.length > 0) {
135+
return withContainer.join(" | ")
136+
}
137+
const displayName = project.displayName.trim()
138+
return displayName.length === 0 ? project.repoUrl.trim() : displayName
133139
}

packages/terminal/tests/core/project-terminal-label.test.ts

Lines changed: 76 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,10 @@ const paddedReadableLabelArbitrary = fc.tuple(
3636
fc.constantFrom("", " ", " ")
3737
).map(([left, value, right]) => `${left}${value}${right}`)
3838

39+
const blankLabelArbitrary = fc.constantFrom("", " ", " ")
40+
41+
const emptyOrMainRefArbitrary = fc.constantFrom("", " ", " ", "main")
42+
3943
const repositoryArbitrary = fc.record({
4044
owner: gitHubPathSegmentArbitrary,
4145
repo: gitHubPathSegmentArbitrary
@@ -54,48 +58,57 @@ const assertRepositoryRefIdProperty = (
5458
fc.assert(fc.property(repositoryArbitrary, refIdArbitrary, assertion))
5559
}
5660

61+
const projectFeatureLabelWithContainer = (
62+
{ owner, repo }: GeneratedRepository,
63+
containerName: string
64+
): string =>
65+
projectTerminalLabel({
66+
containerName,
67+
displayName: `${owner}/${repo}`,
68+
repoRef: "feature-x",
69+
repoUrl: `https://github.com/${owner}/${repo}.git`
70+
})
71+
5772
describe("projectTerminalLabel", () => {
58-
it("renders GitHub issue source context and container identity", () => {
73+
it("renders GitHub issue source URL and container identity", () => {
5974
expect(projectTerminalLabel({
6075
containerName: "dg-repo-issue-7",
6176
displayName: "org/repo",
6277
repoRef: "issue-7",
6378
repoUrl: "https://github.com/org/repo.git"
64-
})).toBe("org/repo | issue #7 (https://github.com/org/repo/issues/7) | container dg-repo-issue-7")
79+
})).toBe("https://github.com/org/repo/issues/7 | container dg-repo-issue-7")
6580
})
6681

67-
it("renders GitHub pull request source context from pull refs", () => {
82+
it("renders GitHub pull request source URL from pull refs", () => {
6883
expect(projectTerminalLabel({
6984
containerName: "dg-repo-pr-42",
7085
displayName: "org/repo",
7186
repoRef: "refs/pull/42/head",
7287
repoUrl: "git@github.com:org/repo.git"
73-
})).toBe("org/repo | PR #42 (https://github.com/org/repo/pull/42) | container dg-repo-pr-42")
88+
})).toBe("https://github.com/org/repo/pull/42 | container dg-repo-pr-42")
7489
})
7590

7691
it("renders repository source context for ordinary refs", () => {
7792
expect(projectTerminalLabel({
7893
displayName: "org/repo",
7994
repoRef: "feature-x",
8095
repoUrl: "https://github.com/org/repo.git"
81-
})).toBe("org/repo | source https://github.com/org/repo.git (feature-x)")
96+
})).toBe("https://github.com/org/repo.git (feature-x)")
8297
})
8398

84-
it("preserves issue markers and GitHub issue URLs for generated issue refs", () => {
99+
it("property-based invariant: issue-N mapping generates canonical GitHub issue URLs", () => {
85100
assertRepositoryRefIdProperty(({ owner, repo }, issueId) => {
86101
const label = projectTerminalLabel({
87102
displayName: `${owner}/${repo}`,
88103
repoRef: `issue-${issueId}`,
89104
repoUrl: `https://github.com/${owner}/${repo}.git`
90105
})
91106

92-
expect(label).toBe(
93-
`${owner}/${repo} | issue #${issueId} (https://github.com/${owner}/${repo}/issues/${issueId})`
94-
)
107+
expect(label).toBe(`https://github.com/${owner}/${repo}/issues/${issueId}`)
95108
})
96109
})
97110

98-
it("preserves PR and MR markers for generated review refs", () => {
111+
it("property-based invariant: PR/MR markers generate review source context", () => {
99112
fc.assert(
100113
fc.property(
101114
repositoryArbitrary,
@@ -111,54 +124,91 @@ describe("projectTerminalLabel", () => {
111124

112125
expect(label).toBe(
113126
refKind === "pull"
114-
? `${owner}/${repo} | PR #${reviewId} (https://github.com/${owner}/${repo}/pull/${reviewId})`
115-
: `${owner}/${repo} | MR #${reviewId}`
127+
? `https://github.com/${owner}/${repo}/pull/${reviewId}`
128+
: `MR #${reviewId}`
116129
)
117130
}
118131
)
119132
)
120133
})
121134

122-
it("uses repoUrl as the base label when displayName is blank", () => {
135+
it("property-based invariant: displayName/repoUrl fallback is deterministic without source context", () => {
136+
fc.assert(
137+
fc.property(
138+
paddedReadableLabelArbitrary,
139+
blankLabelArbitrary,
140+
emptyOrMainRefArbitrary,
141+
(displayName, repoUrl, repoRef) => {
142+
expect(projectTerminalLabel({
143+
displayName,
144+
repoRef,
145+
repoUrl
146+
})).toBe(displayName.trim())
147+
}
148+
)
149+
)
150+
123151
fc.assert(
124-
fc.property(repositoryArbitrary, fc.constantFrom("", " ", " "), ({ owner, repo }, displayName) => {
152+
fc.property(
153+
paddedReadableLabelArbitrary,
154+
blankLabelArbitrary,
155+
emptyOrMainRefArbitrary,
156+
(repoUrl, displayName, repoRef) => {
157+
expect(projectTerminalLabel({
158+
displayName,
159+
repoRef,
160+
repoUrl
161+
})).toBe(repoUrl.trim())
162+
}
163+
)
164+
)
165+
})
166+
167+
it("property-based invariant: repoUrl fallback is used when displayName is blank", () => {
168+
fc.assert(
169+
fc.property(repositoryArbitrary, blankLabelArbitrary, ({ owner, repo }, displayName) => {
125170
const repoUrl = `https://github.com/${owner}/${repo}.git`
126171

127172
expect(projectTerminalLabel({
128173
displayName,
129174
repoRef: "main",
130175
repoUrl
131-
})).toBe(`${repoUrl} | source ${repoUrl}`)
176+
})).toBe(repoUrl)
132177
})
133178
)
134179
})
135180

136-
it("normalizes empty and main refs to source context without ref suffix", () => {
181+
it("property-based invariant: empty/main ref handling omits ref suffix", () => {
137182
fc.assert(
138-
fc.property(repositoryArbitrary, fc.constantFrom("", " ", " ", "main"), ({ owner, repo }, repoRef) => {
183+
fc.property(repositoryArbitrary, emptyOrMainRefArbitrary, ({ owner, repo }, repoRef) => {
139184
const repoUrl = `https://github.com/${owner}/${repo}.git`
140185

141186
expect(projectTerminalLabel({
142187
displayName: `${owner}/${repo}`,
143188
repoRef,
144189
repoUrl
145-
})).toBe(`${owner}/${repo} | source ${repoUrl}`)
190+
})).toBe(repoUrl)
146191
})
147192
)
148193
})
149194

150-
it("preserves non-empty container names after trimming", () => {
195+
it("property-based invariant: container handling preserves non-empty container names after trimming", () => {
151196
fc.assert(
152-
fc.property(repositoryArbitrary, paddedReadableLabelArbitrary, ({ owner, repo }, containerName) => {
153-
const label = projectTerminalLabel({
154-
containerName,
155-
displayName: `${owner}/${repo}`,
156-
repoRef: "feature-x",
157-
repoUrl: `https://github.com/${owner}/${repo}.git`
158-
})
197+
fc.property(repositoryArbitrary, paddedReadableLabelArbitrary, (repository, containerName) => {
198+
const label = projectFeatureLabelWithContainer(repository, containerName)
159199

160200
expect(label.endsWith(` | container ${containerName.trim()}`)).toBe(true)
161201
})
162202
)
163203
})
204+
205+
it("property-based invariant: container handling omits blank container names", () => {
206+
fc.assert(
207+
fc.property(repositoryArbitrary, blankLabelArbitrary, (repository, containerName) => {
208+
const label = projectFeatureLabelWithContainer(repository, containerName)
209+
210+
expect(label).not.toContain("container ")
211+
})
212+
)
213+
})
164214
})

scripts/e2e/clone-auto-open-ssh.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,7 @@ fi
246246
grep -Fq -- "Project created: octocat/hello-world" "$CLONE_LOG" \
247247
|| fail "expected clone log to confirm project creation"
248248

249-
grep -Fq -- "SSH terminal: octocat/hello-world | issue #1 (https://github.com/octocat/Hello-World/issues/1) | container $CONTAINER_NAME" "$CLONE_LOG" \
249+
grep -Fq -- "SSH terminal: https://github.com/octocat/Hello-World/issues/1 | container $CONTAINER_NAME" "$CLONE_LOG" \
250250
|| fail "expected clone log to show SSH auto-open header with issue URL and container"
251251

252252
[[ -f "$SSH_INVOCATION_LOG" ]] || fail "expected ssh wrapper to be invoked"

0 commit comments

Comments
 (0)