Fix autofixes run on a nested directory (#1269)

Context for SEEKers: https://seekchat.slack.com/archives/C03UM9GBGET/p1695369614026749

.changeset/breezy-radios-compare.md

@@ -0,0 +1,5 @@
+---
+'skuba': patch
+---
+
+Git: Handle non-root working directories in [`commitAllChanges`](https://seek-oss.github.io/skuba/docs/development-api/git.html#commitallchanges)

.changeset/pretty-baboons-brake.md

@@ -0,0 +1,12 @@
+---
+'skuba': patch
+---
+
+GitHub: Add working directory parameter to [`readFileChanges`](https://seek-oss.github.io/skuba/docs/development-api/github.html#readfilechanges)
+
+The input `ChangedFiles` need to be evaluated against a working directory. While this is technically a breaking change, we have not found any external usage of the function in `SEEK-Jobs`.
+
+```diff
+- GitHub.readFileChanges(changedFiles)
++ GitHub.readFileChanges(dir, changedFiles)
+```

.changeset/selfish-deers-pretend.md

@@ -0,0 +1,7 @@
+---
+'skuba': patch
+---
+
+lint: Handle non-root working directories in autofix commits
+
+Previously, `skuba lint` could produce surprising autofix commits if it was invoked in a directory other than the Git root. Now, it correctly evaluates its working directory in relation to the Git root, and will only commit file changes within its working directory.

docs/development-api/github.md

@@ -128,7 +128,7 @@ and maps them to GitHub GraphQL [FileChanges].
 ```typescript
 import { GitHub } from 'skuba';
 
-const fileChanges = await GitHub.readFileChanges([
+const fileChanges = await GitHub.readFileChanges(dir, [
   { path: 'added-path', state: 'added' },
   { path: 'modified-path', state: 'modified' },
   { path: 'delete-path', state: 'deleted' },

src/api/git/commitAllChanges.int.test.ts

@@ -1,3 +1,5 @@
+import path from 'path';
+
 import git from 'isomorphic-git';
 import memfs, { fs, vol } from 'memfs';
 
@@ -18,11 +20,23 @@ const newFileName = 'newFile';
 const newFileName2 = 'newFile2';
 
 it('should stage and commit all the new files in the working directory', async () => {
+  const expectStatuses = (statuses: string[]) =>
+    expect(
+      Promise.all([
+        git.status({ fs, dir, filepath: newFileName }),
+        git.status({ fs, dir, filepath: newFileName2 }),
+      ]),
+    ).resolves.toStrictEqual(statuses);
+
+  await expectStatuses(['absent', 'absent']);
+
   await Promise.all([
     fs.promises.writeFile(newFileName, ''),
     fs.promises.writeFile(newFileName2, ''),
   ]);
 
+  await expectStatuses(['*added', '*added']);
+
   await expect(
     commitAllChanges({
       dir,
@@ -31,12 +45,7 @@ it('should stage and commit all the new files in the working directory', async (
     }),
   ).resolves.toMatch(/^[0-9a-f]{40}$/);
 
-  const statuses = await Promise.all([
-    git.status({ fs, dir, filepath: newFileName }),
-    git.status({ fs, dir, filepath: newFileName2 }),
-  ]);
-
-  expect(statuses).toStrictEqual(['unmodified', 'unmodified']);
+  await expectStatuses(['unmodified', 'unmodified']);
 });
 
 it('should stage and commit removed files', async () => {
@@ -78,6 +87,42 @@ it('should stage and commit removed files', async () => {
   expect(statuses).toStrictEqual(['absent', 'absent']);
 });
 
+it('should handle a nested working directory', async () => {
+  const nestedDir = path.join(dir, '/packages/package');
+
+  await fs.promises.mkdir(nestedDir, { recursive: true });
+
+  await Promise.all([
+    fs.promises.writeFile(path.join(nestedDir, newFileName), ''),
+    // Not in our `nestedDir`!
+    fs.promises.writeFile(newFileName2, ''),
+  ]);
+
+  await commitAllChanges({
+    dir: nestedDir,
+    message: 'initial commit',
+    author,
+  });
+
+  await expect(
+    commitAllChanges({
+      dir: nestedDir,
+      message: 'initial commit',
+      author,
+    }),
+  ).resolves.toMatch(/^[0-9a-f]{40}$/);
+
+  const statuses = await Promise.all([
+    git.status({ fs, dir, filepath: newFileName }),
+    git.status({ fs, dir, filepath: newFileName2 }),
+    git.status({ fs, dir, filepath: path.join(nestedDir, newFileName) }),
+    git.status({ fs, dir, filepath: path.join(nestedDir, newFileName2) }),
+  ]);
+
+  // The file outside of our `nestedDir` remains uncommitted.
+  expect(statuses).toStrictEqual(['absent', '*added', 'unmodified', 'absent']);
+});
+
 it('should no-op on clean directory', async () => {
   await expect(
     commitAllChanges({

src/api/git/commitAllChanges.ts

@@ -1,7 +1,10 @@
+import path from 'path';
+
 import fs from 'fs-extra';
 import git from 'isomorphic-git';
 
 import { type Identity, commit } from './commit';
+import { findRoot } from './findRoot';
 import { type ChangedFile, getChangedFiles } from './getChangedFiles';
 
 interface CommitAllParameters {
@@ -35,16 +38,22 @@ export const commitAllChanges = async ({
     return;
   }
 
+  const gitRoot = await findRoot({ dir });
+
+  if (!gitRoot) {
+    throw new Error(`Could not find Git root from directory: ${dir}`);
+  }
+
   await Promise.all(
     changedFiles.map((file) =>
       file.state === 'deleted'
-        ? git.remove({ fs, dir, filepath: file.path })
-        : git.add({ fs, dir, filepath: file.path }),
+        ? git.remove({ fs, dir: gitRoot, filepath: path.join(dir, file.path) })
+        : git.add({ fs, dir: gitRoot, filepath: path.join(dir, file.path) }),
     ),
   );
 
   return commit({
-    dir,
+    dir: gitRoot,
     message,
     author,
     committer,

src/api/github/push.test.ts

@@ -127,7 +127,7 @@ describe('uploadFileChanges', () => {
 describe('readFileChanges', () => {
   it('should read modified and added files from the file system', async () => {
     jest.mocked(fs.promises.readFile).mockResolvedValue('base64-contents');
-    const result = await readFileChanges([
+    const result = await readFileChanges('.', [
       { path: 'some-path', state: 'added' },
       { path: 'another-path', state: 'modified' },
       { path: 'delete-path', state: 'deleted' },
@@ -149,6 +149,33 @@ describe('readFileChanges', () => {
     });
     expect(result).toStrictEqual(expectedFileChanges);
   });
+
+  it('should support a nested directory', async () => {
+    jest.mocked(git.findRoot).mockResolvedValue('/path/to/repo');
+    jest.mocked(fs.promises.readFile).mockResolvedValue('base64-contents');
+
+    const result = await readFileChanges('/path/to/repo/packages/package', [
+      { path: 'some-path', state: 'added' },
+      { path: 'another-path', state: 'modified' },
+      { path: 'delete-path', state: 'deleted' },
+    ]);
+
+    const expectedFileChanges: FileChanges = {
+      additions: [
+        { contents: 'base64-contents', path: 'packages/package/some-path' },
+        { contents: 'base64-contents', path: 'packages/package/another-path' },
+      ],
+      deletions: [{ path: 'packages/package/delete-path' }],
+    };
+
+    expect(fs.promises.readFile).toHaveBeenCalledWith('some-path', {
+      encoding: 'base64',
+    });
+    expect(fs.promises.readFile).toHaveBeenCalledWith('another-path', {
+      encoding: 'base64',
+    });
+    expect(result).toStrictEqual(expectedFileChanges);
+  });
 });
 
 describe('uploadAllFileChanges', () => {

src/api/github/push.ts

@@ -1,3 +1,5 @@
+import path from 'path';
+
 import { graphql } from '@octokit/graphql';
 import type {
   CreateCommitOnBranchInput,
@@ -72,7 +74,7 @@ export const uploadAllFileChanges = async ({
     return;
   }
 
-  const fileChanges = await readFileChanges(changedFiles);
+  const fileChanges = await readFileChanges(dir, changedFiles);
 
   const commitId = await uploadFileChanges({
     dir,
@@ -111,6 +113,7 @@ export interface FileChanges {
  * https://docs.github.com/en/graphql/reference/input-objects#filechanges
  */
 export const readFileChanges = async (
+  dir: string,
   changedFiles: Git.ChangedFile[],
 ): Promise<FileChanges> => {
   const { added, deleted } = changedFiles.reduce<{
@@ -130,17 +133,29 @@ export const readFileChanges = async (
     { added: [], deleted: [] },
   );
 
+  const gitRoot = await Git.findRoot({ dir });
+
+  const toGitHubPath = (filePath: string) => {
+    if (!gitRoot) {
+      return filePath;
+    }
+
+    const pathDir = path.relative(gitRoot, dir);
+
+    return path.join(pathDir, filePath);
+  };
+
   const additions: FileAddition[] = await Promise.all(
     added.map(async (filePath) => ({
-      path: filePath,
+      path: toGitHubPath(filePath),
       contents: await fs.promises.readFile(filePath, {
         encoding: 'base64',
       }),
     })),
   );
 
   const deletions: FileDeletion[] = deleted.map((filePath) => ({
-    path: filePath,
+    path: toGitHubPath(filePath),
   }));
 
   return {