Skip to content

fix: apply --app-name to web page titles#7794

Open
MicahZoltu wants to merge 4 commits into
coder:mainfrom
MicahZoltu:fix/app-name-page-title
Open

fix: apply --app-name to web page titles#7794
MicahZoltu wants to merge 4 commits into
coder:mainfrom
MicahZoltu:fix/app-name-page-title

Conversation

@MicahZoltu
Copy link
Copy Markdown

An exact copy of the PR in #7725 by @jonathandunne, but rebased against latest. Fixes #7688 Original PR description below:

Summary

  • apply --app-name to VS Code web bootstrap product configuration
  • set nameShort/nameLong so ${appName} in tab titles uses the configured app name
  • add a focused route test covering the injected configuration

Testing

  • attempted npm install (partially completed but failed in lib/vscode on missing gssapi/gssapi.h for kerberos under Node 24)
  • attempted npm run test:unit -- --runInBand --runTestsByPath test/unit/node/routes/vscode.test.ts test/unit/node/routes/login.test.ts
  • test run was blocked by a pre-existing TypeScript error in src/node/http.ts (auth.cookie-max-age missing on DefaultedArgs) before these route tests executed

@MicahZoltu MicahZoltu requested a review from a team as a code owner May 14, 2026 09:51
@MicahZoltu
Copy link
Copy Markdown
Author

In the original PR @code-asher asked:

I feel like it would make a lot of sense to add this to the integration.diff patch instead of a separate one. What do you think?

At a glance it appears that each file has a single block of prose text at the top describing its purpose. If we were to integrate this into integration.diff then we would have to drop the descriptive comment currently in the app-name.diff header, or figure out some other way to to include it. I feel like the human readable description being near the change is valuable.

Is there some other way to add comments to diffs? I'm not familiar with the format. We could add the description of this to the existing integration.diff header, but I feel like that would be confusing. My instinct (knowing very little about this project's structure) is that each cohesive change should be isolated into its own file, similar to how one would have a commit or PR for each change. This way when something breaks due to an upstream change, the diffs associated with it are consolidated together.

Note: I spent more time writing these two paragraphs than I did investigating, so I may be way off base or there may be an obvious solution!

Copy link
Copy Markdown
Member

@code-asher code-asher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay, I was out of the office for a bit.

Thank you for reviving this!

Your reasoning makes sense to me, let's keep it as a separate patch.

I was thinking about how we add code-server: $version to the about dialog in integration.diff so we might need to modify that to show the configured app name instead, but really, if anything, I think this is an indication we should break that out of integration.diff rather than continue stuffing things into it.

@code-asher code-asher force-pushed the fix/app-name-page-title branch from 4ca8d2b to e898621 Compare May 20, 2026 22:45
@code-asher
Copy link
Copy Markdown
Member

Ah the build fails because we also have to add app-name to ServerParsedArgs. I can add that tomorrow, unless you get to it first. 😄

@MicahZoltu
Copy link
Copy Markdown
Author

I think this is what you mean by adding app-name to ServerParsedArgs. I also added to serverOptions since that seems to be the schema for ServerParsedArgs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

--app-name should set the page title

3 participants