Addressing PR comments by Claude · Pull Request #3 · soffchen/tiny-remote-desktop

Claude · 2026-03-24T15:17:48Z

Original prompt

This section details on the original issue you should resolve

<issue_title>smallest size</issue_title>
<issue_description>Hi,
This is not an issue but I want guidance
Currently I remove chrome from Dockerfile because Chrome always snap error
The image size become 540MB
QUESTIONS:

if I want smallest possible. what else should I delete

which one faster between vnc vs rdp
how to enable only either one

I got error when connecting using vnc
"Another user already login with this id"
if I switch to rdp, I need to click OK on Session XVNC
then I have to click menu Firefox to launch it
Is that possible to autologin then autostart firefox
tq</issue_description>

Comments on the Issue (you are @claude[agent] in this section)

Fixes smallest size #2

- Remove unnecessary packages (chromium, xterm, bash, fonts) to reduce image size from ~540MB to ~300-350MB - Add environment variables to enable/disable VNC, RDP, and noVNC independently - Fix "Another user already login" VNC error by adding -forever flag and proper priorities - Implement Firefox autostart via Fluxbox startup script - Update documentation with size optimization tips and VNC vs RDP performance comparison - Expose RDP port 3389 in Dockerfile Co-authored-by: soffchen <891708+soffchen@users.noreply.github.com> Agent-Logs-Url: https://github.com/soffchen/tiny-remote-desktop/sessions/3abc26aa-52b2-4966-ae47-17e6fb63100d

Copilot

Pull request overview

This PR optimizes the container image footprint and improves remote-desktop usability by updating the installed package set, adding runtime service toggles, and documenting recommended configurations and troubleshooting steps.

Changes:

Reduce image size by removing large/non-essential packages and consolidating Dockerfile layers with cleanup.
Improve session reliability/start order via supervisord priorities and x11vnc -forever, and expose RDP port 3389.
Add env-var driven service toggles (VNC/RDP/noVNC) and Firefox autostart behavior, with expanded README guidance.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
Dockerfile	Consolidates install steps, removes packages, adds env defaults, and exposes 3389
supervisord.conf	Adds priorities, x11vnc `-forever`, fluxbox DISPLAY env, adjusts xrdp invocation
entry.sh	Adds service enable/disable via env vars and writes Fluxbox startup for Firefox autostart
menu	Removes Chromium/Xterm menu entries (but still contains an xterm-based tool entry)
README.md	Major documentation expansion (optimization, service toggles, usage examples, troubleshooting)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-24T16:45:11Z


+# Configure VNC password if provided
 if [ "$VNC_PASSWORD" ]; then
    sed -i "s/^\(command.*x11vnc.*\)$/\1 -passwd '$VNC_PASSWORD'/" /etc/supervisord.conf


The VNC password injection is not idempotent: on container restart it will append another -passwd ... to the x11vnc command each time. Also, passwords containing sed-special characters (e.g. / or &) will break the substitution. Consider checking for an existing -passwd and replacing it, and properly escaping the value (or use an auth file via -rfbauth).

Suggested change

sed -i "s/^$command.*x11vnc.*$$/\1 -passwd '$VNC_PASSWORD'/" /etc/supervisord.conf

VNC_PASSFILE="/root/.vncpass"

# Store the password in a file and use x11vnc's -rfbauth option

x11vnc -storepasswd "$VNC_PASSWORD" "$VNC_PASSFILE"

chmod 600 "$VNC_PASSFILE"

# Make the change idempotent: remove any existing -passwd or -rfbauth args

sed -i 's/ -passwd [^ ]*//g; s/ -rfbauth [^ ]*//g' /etc/supervisord.conf

# Append the auth file option to the x11vnc command

sed -i "s/^$command.*x11vnc.*$$/\1 -rfbauth $VNC_PASSFILE/" /etc/supervisord.conf

Copilot · 2026-03-24T16:45:12Z

+if [ "$ENABLE_RDP" != "true" ]; then
+    sed -i '/\[program:xrdp\]/,/^$/d' /etc/supervisord.conf
+fi
+


novnc_server is configured to connect to localhost:5901, so allowing ENABLE_NOVNC=true while ENABLE_VNC!=true will start a broken noVNC process (likely restart-looping under supervisord). Consider automatically disabling noVNC when VNC is disabled, or validating the env var combination and failing fast with a clear message.

Suggested change

# Ensure noVNC is not enabled when VNC is disabled

if [ "$ENABLE_NOVNC" = "true" ] && [ "$ENABLE_VNC" != "true" ]; then

echo "Warning: ENABLE_NOVNC=true requires ENABLE_VNC=true; disabling noVNC."

ENABLE_NOVNC=""

fi

Copilot · 2026-03-24T16:45:12Z

+
+## License
+
+Open source - use freely for personal and educational purposes.


The README states the project is “Open source” but the repository does not include a LICENSE file in this PR. Either add an explicit license file, or adjust/remove this statement to avoid implying permissions that aren’t granted.

Suggested change

Open source - use freely for personal and educational purposes.

The source code is provided for reference only. No explicit license is granted at this time, so all rights are reserved. Do not use, copy, modify, or distribute this project without obtaining prior written permission from the maintainer.

Copilot · 2026-03-24T16:45:13Z

+
+| Variable | Default | Description |
+|----------|---------|-------------|
+| `DISPLAY` | `:0` | X display number |


The README lists DISPLAY as a configurable env var, but the runtime config currently hardcodes :0 in supervisord (Xvfb/x11vnc/fluxbox). Either update the implementation to honor DISPLAY, or remove it from the documented configuration to avoid confusion.

Suggested change

| `DISPLAY` | `:0` | X display number |

Copilot · 2026-03-24T16:45:13Z

-      [exec]   (vi) {xterm -e vi}
-[end]
 [submenu] (System Tools)
      [exec]   (top) {xterm -e top}


The Fluxbox menu still invokes xterm for the “top” entry, but xterm is no longer installed/started in this PR. This menu item will fail at runtime; either remove/replace the entry (e.g., use another terminal) or keep xterm in the image.

Suggested change

[exec] (top) {xterm -e top}

[exec] (top) {x-terminal-emulator -e top}

Copilot · 2026-03-24T16:45:13Z

 [program:X11]
 command=/usr/bin/Xvfb :0 -screen 0 %(ENV_RESOLUTION)sx24
 autorestart=true
+priority=10

 [program:x11vnc]
-command=/usr/bin/x11vnc -xkb -noxrecord -noxfixes -noxdamage -display :0 -nopw -wait 5 -shared -permitfiletransfer -tightfilexfer -rfbport 5901
+command=/usr/bin/x11vnc -xkb -noxrecord -noxfixes -noxdamage -display :0 -forever -nopw -wait 5 -shared -permitfiletransfer -tightfilexfer -rfbport 5901
 autorestart=true
+priority=20

 [program:novnc]
 command=/usr/bin/novnc_server --vnc localhost:5901 --listen 6901
 autorestart=true
+priority=30

 [program:fluxbox]
 command=/usr/bin/fluxbox
 autorestart=true
-
-[program:xterm]
-command=/usr/bin/xterm
-autorestart=false
-startretries = 1
+priority=40
+environment=DISPLAY=":0"


DISPLAY is hardcoded to :0 (and Xvfb/x11vnc also hardcode :0), so the DISPLAY env var advertised in the Dockerfile/README can’t actually change the display. Consider using Supervisor’s env interpolation (e.g., %(ENV_DISPLAY)s) consistently for Xvfb, x11vnc, and fluxbox, or remove DISPLAY as a configurable option.

Co-authored-by: soffchen <891708+soffchen@users.noreply.github.com> Agent-Logs-Url: https://github.com/soffchen/tiny-remote-desktop/sessions/9df87a06-d841-4ca4-86ee-d52e44f47ace

… line, fix menu for Alpine Co-authored-by: soffchen <891708+soffchen@users.noreply.github.com> Agent-Logs-Url: https://github.com/soffchen/tiny-remote-desktop/sessions/678e6e8b-3118-4965-b977-3a7fa836bc05

Fix PR #3 review issues: idempotent VNC auth, noVNC/VNC dependency, DISPLAY interpolation, stale xterm refs

soffchen · 2026-03-25T16:17:52Z

@copilot apply changes based on the comments in this thread

Copilot · 2026-03-25T16:19:18Z

@copilot apply changes based on the comments in this thread

All 6 review comments from that thread have already been applied in commits 2dfb1eb and c8473c8:

VNC password (entry.sh:5) – switched to -rfbauth with a passfile; idempotent and safe for special characters
noVNC guard (entry.sh:32) – noVNC is automatically disabled (with a warning) when ENABLE_VNC is not true
License notice (README.md:199) – replaced "open source" claim with an explicit rights-reserved statement
DISPLAY env var (README.md:69) – removed DISPLAY from the documented environment variables table
xterm in menu (menu:21) – replaced the broken xterm -e top entry with a [nop] label since xterm is not installed
DISPLAY interpolation (supervisord.conf) – all hardcoded :0 values replaced with %(ENV_DISPLAY)s

soffchen · 2026-03-25T16:20:37Z

@claude[agent] fix the conflicts

Resolved conflicts in README.md by combining VNC/RDP verdict section with automated builds documentation. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

Claude · 2026-03-25T16:24:36Z

@claude[agent] fix the conflicts

Conflicts resolved in commit da10212. Merged the master branch which included the automated builds documentation with the VNC/RDP optimization changes.

Initial plan

f183897

Claude AI assigned Claude and soffchen Mar 24, 2026

Claude started work on behalf of soffchen March 24, 2026 15:17 View session

Claude AI changed the title ~~[WIP] Optimize Dockerfile for smallest image size~~ Optimize Docker image size and fix VNC/RDP usability issues Mar 24, 2026

Claude AI requested a review from soffchen March 24, 2026 15:24

Claude finished work on behalf of soffchen March 24, 2026 15:24

soffchen approved these changes Mar 24, 2026

View reviewed changes

soffchen requested a review from Copilot March 24, 2026 16:41

Copilot started reviewing on behalf of soffchen March 24, 2026 16:41 View session

soffchen marked this pull request as ready for review March 24, 2026 16:42

Copilot AI reviewed Mar 24, 2026

View reviewed changes

Copilot AI and others added 3 commits March 24, 2026 17:12

Initial plan

1ab6c8a

Address all 6 PR review comments from copilot-pull-request-reviewer

2dfb1eb

Co-authored-by: soffchen <891708+soffchen@users.noreply.github.com> Agent-Logs-Url: https://github.com/soffchen/tiny-remote-desktop/sessions/9df87a06-d841-4ca4-86ee-d52e44f47ace

Use brace quoting for VNC_PASSFILE in sed substitution

2ccabc8

Co-authored-by: soffchen <891708+soffchen@users.noreply.github.com> Agent-Logs-Url: https://github.com/soffchen/tiny-remote-desktop/sessions/9df87a06-d841-4ca4-86ee-d52e44f47ace

Copilot AI mentioned this pull request Mar 24, 2026

Fix PR #3 review issues: idempotent VNC auth, noVNC/VNC dependency, DISPLAY interpolation, stale xterm refs #5

Merged

Copilot AI and others added 2 commits March 25, 2026 16:10

Address review: remove -nopw from x11vnc, scope sed cleanup to x11vnc…

c8473c8

… line, fix menu for Alpine Co-authored-by: soffchen <891708+soffchen@users.noreply.github.com> Agent-Logs-Url: https://github.com/soffchen/tiny-remote-desktop/sessions/678e6e8b-3118-4965-b977-3a7fa836bc05

Merge pull request #5 from soffchen/copilot/solve-review-issues

3e57f83

Fix PR #3 review issues: idempotent VNC auth, noVNC/VNC dependency, DISPLAY interpolation, stale xterm refs

Copilot started work on behalf of soffchen March 25, 2026 16:18 View session

Copilot AI requested a review from soffchen March 25, 2026 16:20

Copilot finished work on behalf of soffchen March 25, 2026 16:20

Claude started work on behalf of soffchen March 25, 2026 16:20 View session

Claude AI changed the title ~~Optimize Docker image size and fix VNC/RDP usability issues~~ Addressing PR comments Mar 25, 2026

Claude finished work on behalf of soffchen March 25, 2026 16:24

soffchen approved these changes Mar 25, 2026

View reviewed changes

soffchen merged commit ed1590f into master Mar 25, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Addressing PR comments#3

Addressing PR comments#3
soffchen merged 8 commits intomasterfrom
claude/optimize-docker-image-size

Claude AI commented Mar 24, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

soffchen commented Mar 25, 2026

Uh oh!

Copilot AI commented Mar 25, 2026

Uh oh!

soffchen commented Mar 25, 2026

Uh oh!

Claude AI commented Mar 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

-    sed -i "s/^\(command.*x11vnc.*\)$/\1 -passwd '$VNC_PASSWORD'/" /etc/supervisord.conf
+    VNC_PASSFILE="/root/.vncpass"
+    # Store the password in a file and use x11vnc's -rfbauth option
+    x11vnc -storepasswd "$VNC_PASSWORD" "$VNC_PASSFILE"
+    chmod 600 "$VNC_PASSFILE"
+    # Make the change idempotent: remove any existing -passwd or -rfbauth args
+    sed -i 's/ -passwd [^ ]*//g; s/ -rfbauth [^ ]*//g' /etc/supervisord.conf
+    # Append the auth file option to the x11vnc command
+    sed -i "s/^\(command.*x11vnc.*\)$/\1 -rfbauth $VNC_PASSFILE/" /etc/supervisord.conf

+# Ensure noVNC is not enabled when VNC is disabled
+if [ "$ENABLE_NOVNC" = "true" ] && [ "$ENABLE_VNC" != "true" ]; then
+    echo "Warning: ENABLE_NOVNC=true requires ENABLE_VNC=true; disabling noVNC."
+    ENABLE_NOVNC=""
+fi


		## License

		Open source - use freely for personal and educational purposes.

	Open source - use freely for personal and educational purposes.
	The source code is provided for reference only. No explicit license is granted at this time, so all rights are reserved. Do not use, copy, modify, or distribute this project without obtaining prior written permission from the maintainer.

	[exec] (top) {xterm -e top}
	[exec] (top) {x-terminal-emulator -e top}

Conversation

Claude AI commented Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Comments on the Issue (you are @claude[agent] in this section)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

soffchen commented Mar 25, 2026

Uh oh!

Copilot AI commented Mar 25, 2026

Uh oh!

soffchen commented Mar 25, 2026

Uh oh!

Claude AI commented Mar 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Claude AI commented Mar 24, 2026 •

edited

Loading