Standardize path resolution

[JuanGalilea]

Paths are whatever now, nothing forces them to make sense or be relative or absolute until something errors out due to not finding something.

Paths should be treated as relative only (relative to root or config location) and resolved upon parsing (such as when reading actor.json).

Ideally, we should store them in some format that makes it clear its a resolved path (unlike string).

This is potentially a huge pain in the ass down the line, so solving it soon is kinda important. Specially if we plan on doing #106 and just going off of that.

Treat this as setup work for #106 so it can serve them on a sane manner

[JuanGalilea]

did some research and there is no standard library for this on the JS ecosystem for some reason.

The two viable options i see are handrolling one of these:

• branded types - type RelativePath = string & { _brand: RelativePath } and functions all around
  • The main benefit is they can just be fed to readFile since they are still a string
  • i don't like that its a hack on the type system just to get nominal-ish typing
  • also no methods make it harder to discover what you can do with the fucking things
• classes: an actual RelativePath class with some methods for creating paths relative to other paths
  • the biggest issue here is that you have to do .path or something to read the file or something

WDYT: @ruocco-l @metalwarrior665 ?????

[ruocco-l]

I would be ok with using the class and its method. It's also easier for agents to work with, I think, you just specify that every operation goes through it

[metalwarrior665]

Can you describe a bit more what practical problems are we hitting? Is it that some paths will be relative to root while others will be relative to actor.json?

I agree to convert everything to repo-root-based "absolute" paths during config parsing. If we do it early in a single pass, we shouldn't need to track it with any types, no? But ofc to be sure, we can have some wrapper with both e.g. { path: string, pathType: 'relative-actor-json' | 'absolute-root' }

I think in the library itself, we only care about root path, we don't need to compute new paths relative to others anywhere, no? And we discussed there will be no working directory and the config file must be top level so all will be root-based

[JuanGalilea]

yeah, only root-relative-paths, the thing is we need some typescript enforcement here.
The more this grows, specially with #106 and more files we reference, the more this might be an issue or source of bugs.

---

AI analysis of the issue

I read all of bin/ and ran a few edge cases against the real code. The core problem is that about seven kinds of path all travel as plain string:

• repo-relative, where the root is ''
• repo-relative in config files, where the root is .
• relative to the current working directory
• absolute filesystem paths
• lowercased copies
• relative to the Docker context
• relative to .actor/

Bugs I reproduced

1. Config paths are barely normalized. Only trailing slashes are removed. A folder of ./actors/foo/ becomes ./actors/foo, which never matches git's actors/foo/src/main.ts, so the actor is never detected as changed. In overrideActorContext, "." is kept as-is and matches nothing, and ./shared fails the same way. .. and absolute paths aren't rejected either.
2. Git escapes non-ASCII file names. A file named café.txt comes back from git diff --name-only as "caf\303\251.txt", with the quotes included, so it matches no scope.
3. Lowercasing plus cutting by length breaks on Unicode. diff-changes.ts checks the scope on the lowercased path, then dockerignore.ts cuts the original path by the lowercased scope's length. 'İ'.toLowerCase() is two characters, so İx/src/a.ts comes out as rc/a.ts.

Found by reading, not run

4. collectNonIgnoredFiles keeps any file with a .actor segment anywhere in its path. That bypasses .gitignore and .dockerignore for sibling actors' .actor/ folders and for things like node_modules/x/.actor/.
5. flattenMonorepoContext passes absolute OS paths to isPathWithinScope, whose documented contract is repo-relative POSIX paths.
6. Several things quietly depend on the working directory being the repo root: setCwd, process.cwd() used as the repo root, path.resolve(dockerContextDir), and git check-ignore run without a cwd. Pointing --workspace at a subfolder breaks all of them.
7. startsWith('..') is used as the "outside this folder" check, so a folder named ..foo counts as outside. endsWith('readme.md') and endsWith('changelog.md') also match notreadme.md.
8. Empty git output becomes ['']. The empty path falls inside the root scope, so a root actor could be treated as changed.
9. git show ${ref}:${path} runs through a shell (shell: true), so file names with spaces or $ break.
10. The folder ? '\${folder}/.actor' : '.actor' expression is written out three times in two files.

[metalwarrior665]

Yeah, let's first resolve the working dir so we know we are safe to make a hard requirement that working-dir = repo root = where config file is