fix: apply gpt55 review fixes
This commit is contained in:
parent
f46ec75c33
commit
23ac9e7778
78
.plans/repo-fix-plan.md
Normal file
78
.plans/repo-fix-plan.md
Normal file
@ -0,0 +1,78 @@
|
|||||||
|
# Repo Fix Plan
|
||||||
|
|
||||||
|
## Findings
|
||||||
|
|
||||||
|
### Medium: Fresh install does not create `/servers`
|
||||||
|
|
||||||
|
Reference: `install.lua:32-38`
|
||||||
|
|
||||||
|
`LIST_FILES` downloads files under `servers/`, but `install.lua` only creates `/programs`, `/apis`, and `/startup`. On a fresh ComputerCraft machine, `wget ... servers/ping-server.lua` can fail because the parent directory does not exist.
|
||||||
|
|
||||||
|
Fix:
|
||||||
|
- Add `fs.makeDir('/servers');` before downloading files.
|
||||||
|
- Bump `install.lua` `_VERSION`.
|
||||||
|
|
||||||
|
### Medium: `cube set-boot <machineId>` cannot clear boot safely
|
||||||
|
|
||||||
|
Reference: `programs/cube.lua:177-204`, `servers/cube-server.lua:57-60`
|
||||||
|
|
||||||
|
The documented behavior says an empty command deletes the boot hook. The client sends `nil` when no command is provided, and the server calls `writeFile('/.cubeboot', startupCommand)`. Writing `nil` can error, and even an empty string currently leaves an empty file instead of deleting `/.cubeboot`.
|
||||||
|
|
||||||
|
Fix:
|
||||||
|
- In `servers/cube-server.lua`, if `startupCommand == nil or startupCommand == ''`, delete `/.cubeboot` and reply `true`.
|
||||||
|
- Otherwise write the command.
|
||||||
|
- Bump `servers/cube-server.lua` `_VERSION`.
|
||||||
|
- Optionally adjust the client message to say `boot DELETED` only when the server replies successfully.
|
||||||
|
|
||||||
|
### Medium: `deploy-file` reports success even when writes fail
|
||||||
|
|
||||||
|
Reference: `servers/cube-server.lua:62-66`, `programs/cube.lua:242-248`
|
||||||
|
|
||||||
|
`deploy-file` ignores the return value from `writeFile` and always replies `true`. If a parent directory is missing, the destination is read-only, or `fs.open` fails, the client counts the file as transferred.
|
||||||
|
|
||||||
|
Fix:
|
||||||
|
- Reply with the boolean result of `writeFile`.
|
||||||
|
- Have the client keep the existing error print when `res` is false.
|
||||||
|
- Bump `servers/cube-server.lua` `_VERSION`.
|
||||||
|
|
||||||
|
### Medium: Deployment cannot create new nested directories
|
||||||
|
|
||||||
|
Reference: `servers/cube-server.lua:24-35`, `programs/cube.lua:20-38`
|
||||||
|
|
||||||
|
`cube deploy` sends file paths recursively, but the server writes files directly without creating parent directories. This fails for new directories that do not already exist on the target cube.
|
||||||
|
|
||||||
|
Fix:
|
||||||
|
- Before writing a deployed file, create its parent directory when needed.
|
||||||
|
- Keep behavior minimal: derive the parent path from `payload.path`, call `fs.makeDir(parent)` if not empty and not present, then write.
|
||||||
|
- Return false if `payload` is malformed or the file write fails.
|
||||||
|
|
||||||
|
### Low: `set-boot` help says `[command]`, but only one argument is accepted
|
||||||
|
|
||||||
|
Reference: `programs/cube.lua:6`, `programs/cube.lua:325`
|
||||||
|
|
||||||
|
`local cubeCommand, firstArg, secondArg = ...` means `cube set-boot 12 mining turtle start` only sends `mining`; extra words are dropped. This matters for shell commands with spaces.
|
||||||
|
|
||||||
|
Fix:
|
||||||
|
- Use `table.pack(...)` to collect all arguments.
|
||||||
|
- For `set-boot`, concatenate arguments after the machine id with spaces.
|
||||||
|
- Preserve existing aliases and help behavior.
|
||||||
|
- Bump `programs/cube.lua` `_VERSION`.
|
||||||
|
|
||||||
|
## Proposed Order
|
||||||
|
|
||||||
|
1. Fix `install.lua` to create `/servers`.
|
||||||
|
2. Fix `cube-server.lua` boot clearing and deploy write reporting.
|
||||||
|
3. Add parent-directory creation for deployed files.
|
||||||
|
4. Fix multi-word `cube set-boot` command parsing.
|
||||||
|
5. Update module versions for changed files.
|
||||||
|
|
||||||
|
## Verification
|
||||||
|
|
||||||
|
Manual/runtime verification is required inside ComputerCraft or CraftOS-PC because this repo has no runnable local test harness.
|
||||||
|
|
||||||
|
Suggested checks:
|
||||||
|
- On a clean machine, run the installer and confirm `/servers/*.lua` downloads.
|
||||||
|
- Run `cube set-boot <id>` and confirm `/.cubeboot` is deleted remotely.
|
||||||
|
- Run `cube set-boot <id> program arg` and confirm the full command is stored.
|
||||||
|
- Deploy a file inside a new nested directory and confirm it exists remotely.
|
||||||
|
- Force a bad deploy path or write failure and confirm the client reports an error instead of counting success.
|
||||||
@ -1,4 +1,4 @@
|
|||||||
local _VERSION = '2.0.0'
|
local _VERSION = '2.0.1'
|
||||||
|
|
||||||
local LIST_FILES = {
|
local LIST_FILES = {
|
||||||
-- startup
|
-- startup
|
||||||
@ -32,6 +32,7 @@ shell.setDir('/')
|
|||||||
fs.makeDir('/programs');
|
fs.makeDir('/programs');
|
||||||
fs.makeDir('/apis');
|
fs.makeDir('/apis');
|
||||||
fs.makeDir('/startup');
|
fs.makeDir('/startup');
|
||||||
|
fs.makeDir('/servers');
|
||||||
|
|
||||||
for _, filePath in pairs(LIST_FILES) do
|
for _, filePath in pairs(LIST_FILES) do
|
||||||
fs.delete(filePath)
|
fs.delete(filePath)
|
||||||
|
|||||||
@ -1,9 +1,22 @@
|
|||||||
local _VERSION = '2.2.1';
|
local _VERSION = '2.3.0';
|
||||||
local CUBE_CHANNEL = 64;
|
local CUBE_CHANNEL = 64;
|
||||||
|
|
||||||
local net = require('/apis/net')();
|
local net = require('/apis/net')();
|
||||||
|
|
||||||
local cubeCommand, firstArg, secondArg = ...;
|
local args = table.pack(...);
|
||||||
|
local cubeCommand = args[1];
|
||||||
|
local firstArg = args[2];
|
||||||
|
local secondArg = args[3];
|
||||||
|
|
||||||
|
local function getRemainingArgs(startIndex)
|
||||||
|
local remainingArgs = {};
|
||||||
|
|
||||||
|
for i = startIndex, args.n do
|
||||||
|
table.insert(remainingArgs, tostring(args[i]));
|
||||||
|
end
|
||||||
|
|
||||||
|
return table.concat(remainingArgs, ' ');
|
||||||
|
end
|
||||||
|
|
||||||
local IGNORED_PATHS = {
|
local IGNORED_PATHS = {
|
||||||
['/rom'] = true,
|
['/rom'] = true,
|
||||||
@ -322,4 +335,8 @@ if (isHelpFlag(firstArg)) then
|
|||||||
return;
|
return;
|
||||||
end
|
end
|
||||||
|
|
||||||
cmd(firstArg, secondArg);
|
if cmd == setBootCommand then
|
||||||
|
cmd(firstArg, getRemainingArgs(3));
|
||||||
|
else
|
||||||
|
cmd(firstArg, secondArg);
|
||||||
|
end
|
||||||
|
|||||||
@ -1,4 +1,4 @@
|
|||||||
local _VERSION = '2.0.0';
|
local _VERSION = '2.1.0';
|
||||||
|
|
||||||
local net = require('/apis/net')();
|
local net = require('/apis/net')();
|
||||||
|
|
||||||
@ -34,6 +34,14 @@ local function writeFile(path, content)
|
|||||||
return true;
|
return true;
|
||||||
end
|
end
|
||||||
|
|
||||||
|
local function ensureParentDir(path)
|
||||||
|
local parentPath = string.match(path, '^(.+)/[^/]+$');
|
||||||
|
|
||||||
|
if parentPath and parentPath ~= '' and not fs.exists(parentPath) then
|
||||||
|
fs.makeDir(parentPath);
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
local function getStartupCommand()
|
local function getStartupCommand()
|
||||||
return trim(readFile('.cubeboot') or "")
|
return trim(readFile('.cubeboot') or "")
|
||||||
end
|
end
|
||||||
@ -55,14 +63,25 @@ end)
|
|||||||
|
|
||||||
-- set-boot event
|
-- set-boot event
|
||||||
net.listenRequest(CUBE_CHANNEL, "set-boot", function(startupCommand, reply)
|
net.listenRequest(CUBE_CHANNEL, "set-boot", function(startupCommand, reply)
|
||||||
|
if startupCommand == nil or startupCommand == '' then
|
||||||
|
fs.delete('/.cubeboot');
|
||||||
|
reply(true);
|
||||||
|
return;
|
||||||
|
end
|
||||||
|
|
||||||
local res = writeFile('/.cubeboot', startupCommand);
|
local res = writeFile('/.cubeboot', startupCommand);
|
||||||
reply(res);
|
reply(res);
|
||||||
end)
|
end)
|
||||||
|
|
||||||
-- deploy-file event
|
-- deploy-file event
|
||||||
net.listenRequest(CUBE_CHANNEL, "deploy-file", function(payload, reply)
|
net.listenRequest(CUBE_CHANNEL, "deploy-file", function(payload, reply)
|
||||||
writeFile(payload.path, payload.content);
|
if type(payload) ~= 'table' or type(payload.path) ~= 'string' or type(payload.content) ~= 'string' then
|
||||||
reply(true);
|
reply(false);
|
||||||
|
return;
|
||||||
|
end
|
||||||
|
|
||||||
|
ensureParentDir(payload.path);
|
||||||
|
reply(writeFile(payload.path, payload.content));
|
||||||
end)
|
end)
|
||||||
|
|
||||||
print('cube-server v' .. _VERSION .. ' started.')
|
print('cube-server v' .. _VERSION .. ' started.')
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user