Skip to content

feat(ls): display symlink targets in long format output#165

Merged
matt-dz merged 4 commits intomainfrom
matt-dz/ls-symlink-target
Apr 2, 2026
Merged

feat(ls): display symlink targets in long format output#165
matt-dz merged 4 commits intomainfrom
matt-dz/ls-symlink-target

Conversation

@matt-dz
Copy link
Copy Markdown
Collaborator

@matt-dz matt-dz commented Apr 1, 2026

Summary

  • ls -l now shows -> target for symbolic links, matching GNU ls behavior
  • Adds Sandbox.Readlink method using os.Root.Readlink for sandboxed symlink resolution
  • Wires ReadlinkFile through CallContext so builtins can read symlink destinations

Test plan

  • New scenario tests: symlink_target.yaml (direct arg) and symlink_target_in_dir.yaml (directory listing)
  • Full test suite passes: allowedpaths, interp, tests, analysis
  • CI passes

🤖 Generated with Claude Code

ls -l now shows `-> target` for symbolic links, matching GNU ls
behavior. Adds Sandbox.Readlink and CallContext.ReadlinkFile to read
symlink destinations through the sandbox.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@matt-dz
Copy link
Copy Markdown
Collaborator Author

matt-dz commented Apr 1, 2026

@codex conduct a comprehensive security and code review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 70c2aa1aa4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread builtins/ls/ls.go
ls -lF now shows indicators on the symlink target (e.g. dirlink -> subdir/,
filelink -> file.txt*), matching GNU ls behavior. The target's FileInfo is
obtained via StatFile and passed to indicator(). Dangling symlinks show no
target indicator, also matching GNU ls.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@matt-dz
Copy link
Copy Markdown
Collaborator Author

matt-dz commented Apr 1, 2026

@codex conduct a comprehensive security and code review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: dbff91ca17

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread builtins/ls/ls.go Outdated
Comment thread tests/scenarios/cmd/ls/long_format/symlink_target.yaml Outdated
- Use info.Mode() as fallback for symlink detection when DirEntry.Type()
  returns 0 (DT_UNKNOWN on some filesystems).
- Remove skip_assert_against_bash from symlink scenarios — they use
  stdout_contains so UID/GID differences don't affect the comparison.
  Verified all three pass against bash via docker.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@matt-dz
Copy link
Copy Markdown
Collaborator Author

matt-dz commented Apr 1, 2026

@codex conduct a comprehensive security and code review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 20bcb577e7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread builtins/ls/ls.go
GNU ls applies classification indicators to symlink targets only with
-F (classify), not -p (append-slash). For example, ls -lF shows
"link -> dir/" but ls -lp shows "link -> dir" without the trailing
slash. Guard the target indicator call with opts.classify.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@matt-dz
Copy link
Copy Markdown
Collaborator Author

matt-dz commented Apr 2, 2026

@codex conduct a comprehensive security and code review

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Chef's kiss.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@matt-dz matt-dz marked this pull request as ready for review April 2, 2026 13:16
@matt-dz matt-dz added this pull request to the merge queue Apr 2, 2026
Merged via the queue into main with commit 07739c8 Apr 2, 2026
34 checks passed
@matt-dz matt-dz deleted the matt-dz/ls-symlink-target branch April 2, 2026 13:20
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.

2 participants