Repository navigation
Update additional classes in Os to offer the alias option - #5955
bevinduckett wants to merge 13 commits into
Conversation
… Mutex, & ConditionVariable
This reverts commit 5f495b261c3c7cc69dd075db45c2df270ce371a3, reversing changes made to ae321743ed113ab34cc48a2abcd17b97581acf6c. Undoes the merge commit because fprime devel commit c103965 is breaking our build
… revert The Aug 19 "Revert Merge branch devel into multi-os-alias" undid a devel merge because fpp 3.3.0a17 broke the build. That issue was fixed upstream by 1fadfda (bump to stable fpp 3.3.0), and both commits are already in devel's history, but the revert's content was never re-integrated. This commit fixes that (modifies about 170 non-Os files).
Coverage report — base
|
| Module | Line | Δ | Function | Δ | Branch | Δ |
|---|---|---|---|---|---|---|
Fw/DataStructures |
97.69 | -0.53 | 97.14 | +0.00 | 82.30 | -0.73 |
Modules changed
| Module | Line | Δ | Function | Δ | Branch | Δ |
|---|---|---|---|---|---|---|
Fw/DataStructures |
97.69 | -0.53 | 97.14 | +0.00 | 82.30 | -0.73 |
Os/Generic |
90.14 | -0.10 | 89.13 | +0.00 | 74.71 | -0.58 |
Fw/Types |
55.49 | -0.06 | 60.69 | +0.00 | 35.28 | -0.15 |
Svc/WasmSequencer |
97.82 | +0.07 | 100.00 | +0.00 | 92.79 | +0.62 |
Svc/CmdDispatcher |
97.90 | +0.12 | 93.33 | +0.47 | 91.78 | +0.37 |
Os |
19.91 | +0.14 | 22.75 | +0.30 | 17.35 | -0.06 |
Os/Generic/Types |
92.50 | +0.71 | 92.86 | +0.00 | 74.77 | +0.00 |
New modules
| Module | Line | Function | Branch |
|---|---|---|---|
Fw/Prm |
0.00 | 0.00 | 0.00 |
Modules without UTs
CFDP/Checksum/GTest, Drv/LinuxGpioDriver, Drv/LinuxI2cDriver, Drv/LinuxSpiDriver, Drv/Ports/DataTypes, Drv/PosixUartDriver, FppTestProject/FppTest/topology/async, FppTestProject/FppTest/topology/components/Comp, FppTestProject/FppTest/topology/components/Framework, FppTestProject/FppTest/topology/components/Receiver, FppTestProject/FppTest/topology/components/Sender, FppTestProject/FppTest/topology/guarded, FppTestProject/FppTest/topology/sync, FppTestProject/FppTest/topology/top_ports, FppTestProject/FppTest/topology/types, Fw/Com, Fw/Comp, Fw/FilePacket/GTest, Fw/Fpy, Fw/Obj, Fw/Port, Fw/Sm, Fw/Test, Fw/Types/GTest, Os/Models, Svc/Ccsds/Types, Svc/Ccsds/Utils, Svc/FatalHandler, Svc/Subtopologies/CdhCore, Svc/Subtopologies/ComCcsds, Svc/Subtopologies/ComCcsdsSdls, Svc/Subtopologies/ComFprime, Svc/Subtopologies/ComLoggerTee, Svc/Subtopologies/DataProducts, Svc/Subtopologies/DpCompression, Svc/Subtopologies/FileHandling, Svc/Subtopologies/FileHandlingCfdp, Svc/Subtopologies/FileHandlingCfdp/FileHandlingCfdpConfig, TestDeploymentsProject/Ref/DpDemo, TestDeploymentsProject/Ref/PingReceiver, TestDeploymentsProject/Ref/RecvBuffApp, TestDeploymentsProject/Ref/SendBuffApp, TestDeploymentsProject/Ref/Top, TestDeploymentsProject/Ref/TypeDemo, cmake/test/data/TestConfigDeployment, cmake/test/data/TestDeployment/TestBuildAutocoder, cmake/test/data/TestDeployment/TestDuplicateDirective, cmake/test/data/TestDeployment/TestHeaderAutocoder, cmake/test/data/TestDeployment/TestLinkDepends, cmake/test/data/TestDeployment/TestRelative, cmake/test/data/test-fprime-library/TestLibrary/TestComponent, cmake/test/data/test-fprime-library2/TestLibrary2/TestComponent
| namespace Os { | ||
|
|
||
| CpuInterface::Status CpuInterface::getCount(FwSizeType& cpu_count) { | ||
| return Os::Cpu::getSingleton()._getCount(cpu_count); |
There was a problem hiding this comment.
[Design] must fix design-code-mismatch: the static convenience API now depends on Os::Cpu::getSingleton()/init(), but that static surface exists only on DelegateCpu and StubCpu; it is not part of CpuInterface, and LinuxCpu/DarwinCpu do not provide it. The compile-time selection documented in config/OsDelegateCpu.hpp therefore fails to compile for any in-tree concrete implementation. Same pattern in MemoryInterface.cpp, FileSystemInterface.cpp, and DelegateConsole (where getSingleton() also carries Fw::Logger::registerLogger, so a compile-time Os::Console would silently lose logger output). Suggest declaring static Os::Cpu& getSingleton(); static void init(); on the interface and defining them in CpuInterface.cpp against the configured alias (holding static Os::Cpu s_singleton;), so both selection modes share one contract. @LeStarch @thomas-bc
| struct ConsoleHandle {}; | ||
|
|
||
| // \brief Interface defining the properties of the console | ||
| class ConsoleInterface : public Fw::Logger { |
There was a problem hiding this comment.
[Design] suggestion design-scope-creep: the survey in #5249 recommended Os::Console land in its own PR because of the Fw::Logger coupling. Moving the Fw::Logger base onto ConsoleInterface makes every platform console (Posix, out-of-tree) an Fw::Logger and changes their object layout, which is a broader contract change than the other seven classes. If bundling is intentional, please state so (and the layout impact) in the PR description; otherwise split Console out. @LeStarch @thomas-bc
| if (not this->isOpen()) { | ||
| return Status::NOT_OPENED; | ||
| } | ||
| return this->read(const_cast<char*>(filename.toChar()), filename.getCapacity()); |
There was a problem hiding this comment.
[C++ Design] future work CPP-10 (avoid const_cast): writes through const_cast<char*>(filename.toChar()).
Moved verbatim from Os/Directory.cpp; not introduced here. A mutable-buffer accessor on Fw::StringBase (or reading into a local char[FW_FIXED_LENGTH_STRING_SIZE] and assigning) would remove the cast.
| FW_ASSERT(FileInterface::Mode::OPEN_NO_MODE < requested_mode && | ||
| FileInterface::Mode::MAX_OPEN_MODE > requested_mode); | ||
| FW_ASSERT((0 <= overwrite) && (overwrite < OverwriteType::MAX_OVERWRITE_TYPE)); | ||
| return this->open(filepath, requested_mode, overwrite); |
There was a problem hiding this comment.
[Correctness] suggestion The 3-argument open(path, mode, overwrite) no longer bounds-checks the path string.
Before this PR File::open(path, mode, overwrite) forwarded here with length = FileNameStringSize + 1, so string_length(filepath, length) < length was asserted on every open. Now that signature is the pure-virtual delegate hook and DelegateFile::open forwards to the OS implementation with only a null check.
Callers using that form (Svc/ActiveTextLogger/LogFile.cpp:111, Svc/FileWorker/FileWorker.cpp:444, Svc/Ccsds/CfdpManager/TransactionRx.cpp:368, Svc/PrmDb/PrmDbImpl.cpp:141, Svc/BufferLogger/BufferLoggerFile.cpp:122) silently lose the over-long/unterminated path guard.
Consider keeping a checked non-virtual 3-arg open on FileInterface and giving the implementation hook a distinct name (as Directory/FileSystem do with _-prefixed hooks). Anchored on the checked overload; the offending declaration is the pure-virtual open in FileInterface.hpp.
lestarch-autobot
left a comment
lestarch-autobot
left a comment
There was a problem hiding this comment.
Automated review summary (run 1)
Per-agent results
| Agent | must fix | suggestion | could fix | future work | outstanding | Verdict |
|---|---|---|---|---|---|---|
| Security Vulnerabilities | 2 | 0 | 0 | 0 | 2 | No-Go |
| Supply Chain / Runner Safety | 0 | 0 | 0 | 0 | 0 | Go |
| F Prime C/C++ Design | 3 | 0 | 0 | 1 | 4 | No-Go |
| Documentation Currency | 3 | 1 | 1 | 0 | 5 | No-Go |
| Design | 2 | 1 | 1 | 0 | 4 | No-Go |
| Architecture | 0 | 0 | 0 | 0 | 0 | Go |
| Test Quality | 2 | 1 | 0 | 0 | 3 | No-Go |
| Correctness | 3 | 2 | 1 | 0 | 6 | No-Go |
| Operational | 1 | 1 | 1 | 0 | 3 | No-Go |
| Maintainability | 4 | 2 | 2 | 0 | 8 | No-Go |
| CI safety | — | — | — | — | — | Go |
| Totals | 16 | 8 | 6 | 1 | 31 | No-Go |
Totals count each consolidated finding once (2 duplicate threads closed by the post-pass; concurrence replies remain counted in their agents' rows).
Duplicates consolidated this run: 2
Supply-chain surfaces
| Surface | Outstanding |
|---|---|
| Dependencies | clean |
| Vendored / submodule | clean |
| Build / test infrastructure | clean |
| Workflows / actions / scripts | clean |
| Generator output | clean |
| Prompt-injection | clean |
| Review-system integrity | clean |
Outstanding must-fix items (14)
Design
- Human design adjudication required.
Os/ValidateFileCommon.cppdefault:reinstatesFW_ASSERT(false, status)(#5710 fix reverted by stale devel-merge revert af3538a); OS-layerNOT_SUPPORTED/INVALID_ARGUMENThalts flight software — also: Security, C++ Design, Correctness, Operational — #5955 (comment) - Human design adjudication required. Static convenience API depends on
getSingleton()/init()that onlyDelegate*/Stub*provide; compile-time selection ofLinuxCpu/DarwinCpu(and Memory/FileSystem/Console) does not compile — #5955 (comment)
Security Vulnerabilities
DelegateConditionVariable::waitdrops the mutex-consistency check on the public wait path — #5955 (comment)
F Prime C/C++ Design
- CPP-26: non-virtual
FileInterface::m_modemaintained only byDelegateFile; concrete implementations hideisOpen()— #5955 (comment) - CPP-26:
DirectoryInterface::m_is_openset only byDelegateDirectory;PosixDirectory/StubDirectoryhide it with non-constisOpen()— #5955 (comment)
Documentation Currency
Os/Generic/docs/sdd.mdPQ-003 reverted to a statement the code contradicts — also: Operational — #5955 (comment)Os/docs/sdd.md§5.2.2 still lists RawTime as the only compile-time-selectable service — #5955 (comment)- Compile-time selection now requires
init()/getSingleton()beyond "inherit from the interface"; undocumented — #5955 (comment)
Test Quality
FileSystemRules.cppASSERT_TRUE(other_dir.compare(cwdBuffer))is a tautology (re-introduces the bug fixed in #5706) — also: Correctness — #5955 (comment)PosixFileTests.cppsignal loop registerscleanupforSIGQUITfive times; other signals get no handler (reverts #5706) — also: Correctness — #5955 (comment)
Maintainability
Os/Posix/error.cppfallthrough comment saysOTHER_ERRORbut cases map elsewhere (reverts #5705) — #5955 (comment)Os/Posix/File.cppclose-path comment inverted relative to the guard (reverts #5705) — #5955 (comment)Os/Stub/ConditionVariable.cppcomment claimspend()asserts; it returnsERROR_NOT_IMPLEMENTED(reverts #5705) — #5955 (comment)PriorityMemQueueTests.cppcomment documents an enabled-priority checksend()does not have (reverts #5705) — #5955 (comment)
Merge readiness
Merge readiness: No-Go — 8 of 10 reviewers report outstanding must-fix findings (14 consolidated threads), most of them devel fixes (#5705/#5706/#5710/#5713) undone by the stale merge revert af3538a still present in the branch.
Eight services now share the alias/delegate helm; restore the reverted devel fixes and this ship is ready for its shakedown cruise.
thomas-bc
left a comment
thomas-bc
left a comment
There was a problem hiding this comment.
If that's possible, I'd vote for merging all the config headers that select these into a single header, so we don't overcrowd the default/config directory
| REQUIRES_IMPLEMENTATIONS | ||
| "Os_Console" | ||
| SOURCES | ||
| "${CMAKE_CURRENT_LIST_DIR}/ConsoleInterface.cpp" |
There was a problem hiding this comment.
[Design] must fix design-code-mismatch: ConsoleInterface.cpp and ConditionVariableInterface.cpp (line 207) are registered as sources but neither file exists at this head, while their definitions (ConsoleInterface::writeMessage(const Fw::ConstStringBase&), ConditionVariableInterface::wait) were removed from DelegateConsole.cpp / DelegateConditionVariable.cpp. The intended design — interface convenience methods live in their own <Name>Interface.cpp TU like CpuInterface.cpp — is right, but the files were not added, so CMake generation fails (every CI build at 59ddb89 is red) and even with the SOURCES lines dropped the two methods would be undefined at link time (DelegateConditionVariable::wait now calls the base). Please add both files with the moved definitions.
There was a problem hiding this comment.
[Maintainability] Concur — also in scope for maintainability (structural hazard): the move out of DelegateConsole.cpp/DelegateConditionVariable.cpp (my prior thread) is the right shape, but with neither <Name>Interface.cpp committed the build no longer has a definition for either method; severity from my scope: must fix.
There was a problem hiding this comment.
[Correctness] Concur — also in scope for functional correctness: ConditionVariableInterface::wait and ConsoleInterface::writeMessage(const Fw::ConstStringBase&) are declared and called (DelegateConditionVariable::wait, Fw::Logger::log) but have no definition anywhere in the tree, so the build cannot link even after CMake generation is repaired; severity from my scope: must fix.
| REQUIRES_IMPLEMENTATIONS | ||
| "Os_Console" | ||
| SOURCES | ||
| "${CMAKE_CURRENT_LIST_DIR}/ConsoleInterface.cpp" |
There was a problem hiding this comment.
[C++ Design] must fix CPP-26 (F Prime style): declared members with no definition in the tree; the build no longer configures.
ConsoleInterface.cpp (here) and ConditionVariableInterface.cpp (line 207) are listed as SOURCES but neither file exists at this head, so CMake fails to configure Os_Console/Os_Condition. The definitions they presumably carry — ConsoleInterface::writeMessage(const Fw::ConstStringBase&) and ConditionVariableInterface::wait(Os::Mutex&) — were removed from DelegateConsole.cpp/DelegateConditionVariable.cpp in 59ddb89 and now exist nowhere (DelegateConditionVariable::wait calls the undefined one at Os/DelegateConditionVariable.cpp:28). Add the two files to the commit (or restore the definitions in the delegate translation units).
There was a problem hiding this comment.
[Summary] Duplicate — consolidated into #5955 (comment) (same site, Os/CMakeLists.txt:147; same issue — missing ConsoleInterface.cpp/ConditionVariableInterface.cpp).
| // bounds check in FileInterface::open(path, length, mode, overwrite). Re-apply it here against | ||
| // the default path bound so that those callers keep the over-long/unterminated path guard. | ||
| const FwSizeType path_bound = static_cast<FwSizeType>(FileNameStringSize) + 1; |
There was a problem hiding this comment.
[Maintainability] could fix Three-line comment describing another function's check will drift from FileInterface::open(path, length, ...) when that bounds check changes.
The comment restates FileInterface::open's guard rather than this function's purpose; a one-liner pointing at the reason (direct callers of this hook skip the interface check) keeps it accurate with less to maintain.
| // bounds check in FileInterface::open(path, length, mode, overwrite). Re-apply it here against | |
| // the default path bound so that those callers keep the over-long/unterminated path guard. | |
| const FwSizeType path_bound = static_cast<FwSizeType>(FileNameStringSize) + 1; | |
| // Direct callers of this hook skip FileInterface::open's path bounds check; re-apply it here |
There was a problem hiding this comment.
[Maintainability] Correction: the suggestion above is mis-anchored one line low — it should replace only the three comment lines (56-58); keep the const FwSizeType path_bound = ... line as is. Please do not apply the block verbatim.
lestarch-autobot
left a comment
lestarch-autobot
left a comment
There was a problem hiding this comment.
Automated review summary (run 2)
Per-agent results
| Agent | must fix | suggestion | could fix | future work | outstanding | Verdict |
|---|---|---|---|---|---|---|
| Security Vulnerabilities | 2 | 0 | 1 | 0 | 1 | Go |
| Supply Chain / Runner Safety | 0 | 0 | 0 | 0 | 0 | Go |
| F Prime C/C++ Design | 4 | 0 | 0 | 1 | 2 | No-Go |
| Documentation Currency | 3 | 1 | 1 | 0 | 0 | Go |
| Design | 3 | 1 | 1 | 0 | 3 | No-Go |
| Architecture | 0 | 0 | 0 | 0 | 0 | Go |
| Test Quality | 2 | 1 | 0 | 0 | 0 | Go |
| Correctness | 4 | 2 | 2 | 0 | 3 | No-Go |
| Operational | 1 | 1 | 1 | 0 | 0 | Go |
| Maintainability | 5 | 2 | 3 | 0 | 2 | No-Go |
| CI safety | — | — | — | — | — | Go |
| Totals | 24 | 8 | 9 | 1 | 11 | No-Go |
Per-agent rows carry each agent's own counts (concurrence replies included); the outstanding must-fix list below counts each consolidated thread once (2 threads, 1 duplicate closed by the post-pass).
Since last run
| Agent | resolved | still open | newly added | incorrect-fix follow-ups | improperly resolved | disagreements escalated |
|---|---|---|---|---|---|---|
| Security Vulnerabilities | 2 | 0 | 1 | 0 | 0 | 0 |
| Supply Chain / Runner Safety | 0 | 0 | 0 | 0 | 0 | 0 |
| F Prime C/C++ Design | 3 | 1 | 1 | 0 | 0 | 0 |
| Documentation Currency | 5 | 0 | 0 | 0 | 0 | 0 |
| Design | 2 | 2 | 1 | 0 | 0 | 0 |
| Architecture | 0 | 0 | 0 | 0 | 0 | 0 |
| Test Quality | 3 | 0 | 0 | 0 | 0 | 0 |
| Correctness | 5 | 1 | 2 | 0 | 0 | 0 |
| Operational | 3 | 0 | 0 | 0 | 0 | 0 |
| Maintainability | 8 | 0 | 2 | 0 | 0 | 0 |
Duplicates consolidated this run: 1 (C++ Design thread at Os/CMakeLists.txt:147 folded into the Design thread at the same site; resolveReviewThread is not permitted for this token, so the thread stays visibly open with the duplicate-close reply)
Supply-chain surfaces
| Surface | Outstanding |
|---|---|
| Dependencies | clean |
| Vendored / submodule | clean |
| Build / test infrastructure | clean |
| Workflows / actions / scripts | clean |
| Generator output | clean |
| Prompt-injection | clean |
| Review-system integrity | clean |
Outstanding must-fix items (2)
Design
Os/CMakeLists.txtregistersConsoleInterface.cpp/ConditionVariableInterface.cppbut neither file exists at this head; the moved definitions (ConsoleInterface::writeMessage,ConditionVariableInterface::wait) exist nowhere, so CMake generation fails and the link would too — also: C++ Design, Maintainability, Correctness — #5955 (comment)- Human design adjudication required. Static convenience API depends on
getSingleton()/init()that onlyDelegate*/Stub*provide; compile-time selection ofLinuxCpu/DarwinCpu(and Memory/FileSystem/Console) does not compile — #5955 (comment)
Merge readiness
Merge readiness: No-Go — 2 outstanding must-fix threads (4 reviewers concur); the missing <Name>Interface.cpp files break CMake generation at this head, and the singleton-on-interface design question awaits maintainer adjudication.
Twelve of fourteen run-1 items are resolved — commit the two missing source files and this crew is nearly go for launch.
| | FileSystem | `config/OsDelegateFileSystem.hpp` | `OS_FILE_SYSTEM_HEADER` | | ||
| | Directory | `config/OsDelegateDirectory.hpp` | `OS_DIRECTORY_HEADER` | | ||
| | Mutex | `config/OsDelegateMutex.hpp` | `OS_MUTEX_HEADER` | | ||
| | ConditionVariable | `config/OsDelegateMutex.hpp` | `OS_CONDITION_VARIABLE_HEADER` | |
There was a problem hiding this comment.
[Design] could fix design-intent-mismatch: after merging devel, this PR no longer adds the ConditionVariable alias — it now comes from #5936 via config/OsDelegateMutex.hpp (this PR's OsDelegateConditionVariable.hpp was dropped), and RawTime came from #5617. The PR description still lists both as added here. Please update it to the seven services this diff actually converts (File, FileSystem, Directory, Console, CountingSemaphore, Cpu, Memory) so reviewers scope the change correctly.
| Refer to each `Os/<Service>Interface.hpp` header for the full method signatures and status enums. | ||
| For the services that have been split into an alias header plus an interface header (`File`, | ||
| `FileSystem`, `Directory`, `ConditionVariable`, `Console`, `CountingSemaphore`, `Cpu`, `Memory`, | ||
| `RawTime`), `Os/<Service>.hpp` is a thin aggregator and the pure-virtual contract is in |
There was a problem hiding this comment.
[Documentation] could fix Split-service list omits Mutex, which the devel merge also split into Os/MutexInterface.hpp (#5936).
The guide's Mutex walkthrough now includes Os/MutexInterface.hpp, so this list should name it too.
| Refer to each `Os/<Service>Interface.hpp` header for the full method signatures and status enums. | |
| For the services that have been split into an alias header plus an interface header (`File`, | |
| `FileSystem`, `Directory`, `ConditionVariable`, `Console`, `CountingSemaphore`, `Cpu`, `Memory`, | |
| `RawTime`), `Os/<Service>.hpp` is a thin aggregator and the pure-virtual contract is in | |
| Refer to each `Os/<Service>Interface.hpp` header for the full method signatures and status enums. | |
| For the services that have been split into an alias header plus an interface header (`Mutex`, `File`, | |
| `FileSystem`, `Directory`, `ConditionVariable`, `Console`, `CountingSemaphore`, `Cpu`, `Memory`, | |
| `RawTime`), `Os/<Service>.hpp` is a thin aggregator and the pure-virtual contract is in |
| # Mutex: Mutex.hpp is a thin public alias header (no Mutex.cpp); the interface and | ||
| # link-time delegate live in MutexInterface.hpp / DelegateMutex.{hpp,cpp}. Registered |
There was a problem hiding this comment.
[Maintainability] could fix This seven-line comment restates the file list of the register_fprime_module block directly beneath it (MutexInterface.hpp, DelegateMutex.{hpp,cpp}, Condition.hpp, ...), so any rename/move of those files must be made twice or the comment silently goes stale; the sibling Console/Cpu/Memory blocks keep their comments to the one non-obvious fact (why add_named_os_module is not used).
| # Mutex: Mutex.hpp is a thin public alias header (no Mutex.cpp); the interface and | |
| # link-time delegate live in MutexInterface.hpp / DelegateMutex.{hpp,cpp}. Registered | |
| # Mutex + ConditionVariable: Mutex.hpp / Condition.hpp are thin alias headers (no matching .cpp), so | |
| # add_named_os_module does not fit; both stay in one Os_Mutex module because the Stub impl and | |
| # REQUIRES_IMPLEMENTATIONS references depend on that name. |
There was a problem hiding this comment.
[Maintainability] Note: GitHub only allows anchoring on the diff hunk (lines 194-195); the suggestion is meant to replace the whole comment, lines 194-200.
lestarch-autobot
left a comment
lestarch-autobot
left a comment
There was a problem hiding this comment.
Automated review summary (run 3)
Per-agent results
| Agent | must fix | suggestion | could fix | future work | outstanding | Verdict |
|---|---|---|---|---|---|---|
| Security Vulnerabilities | 2 | 0 | 1 | 0 | 1 | Go |
| Supply Chain / Runner Safety | 0 | 0 | 0 | 0 | 0 | Go |
| F Prime C/C++ Design | 4 | 0 | 0 | 1 | 2 | No-Go |
| Documentation Currency | 3 | 1 | 2 | 0 | 1 | Go |
| Design | 3 | 1 | 2 | 0 | 4 | No-Go |
| Architecture | 0 | 0 | 0 | 0 | 0 | Go |
| Test Quality | 2 | 1 | 0 | 0 | 0 | Go |
| Correctness | 4 | 2 | 2 | 0 | 3 | No-Go |
| Operational | 1 | 1 | 1 | 0 | 0 | Go |
| Maintainability | 5 | 2 | 4 | 0 | 1 | No-Go |
| CI safety | — | — | — | — | — | Go |
| Totals | 24 | 8 | 12 | 1 | 12 | No-Go |
Per-agent rows carry each agent's own counts (concurrence replies included); the outstanding must-fix list below counts each consolidated thread once (2 threads). This head only merges devel into the PR (the Mutex/ConditionVariable alias landed upstream via #5936), so both blocking items from run 2 are unchanged.
Since last run
| Agent | resolved | still open | newly added | incorrect-fix follow-ups | improperly resolved | disagreements escalated |
|---|---|---|---|---|---|---|
| Security Vulnerabilities | 0 | 1 | 0 | 0 | 0 | 0 |
| Supply Chain / Runner Safety | 0 | 0 | 0 | 0 | 0 | 0 |
| F Prime C/C++ Design | 0 | 2 | 0 | 0 | 0 | 0 |
| Documentation Currency | 0 | 0 | 1 | 0 | 0 | 0 |
| Design | 0 | 3 | 1 | 0 | 0 | 0 |
| Architecture | 0 | 0 | 0 | 0 | 0 | 0 |
| Test Quality | 0 | 0 | 0 | 0 | 0 | 0 |
| Correctness | 0 | 3 | 0 | 0 | 0 | 0 |
| Operational | 0 | 0 | 0 | 0 | 0 | 0 |
| Maintainability | 0 | 2 | 1 | 0 | 0 | 0 |
Duplicates consolidated this run: 0 (10 open agent threads, all at distinct site-keys; the run-2 duplicate-close on the C++ Design CMake thread still stands)
Supply-chain surfaces
| Surface | Outstanding |
|---|---|
| Dependencies | clean |
| Vendored / submodule | clean |
| Build / test infrastructure | clean |
| Workflows / actions / scripts | clean |
| Generator output | clean |
| Prompt-injection | clean |
| Review-system integrity | clean |
Outstanding must-fix items (2)
Design
Os/CMakeLists.txtstill registersConsoleInterface.cpp, which does not exist at this head (theConditionVariableInterface.cppentry was dropped by the devel merge), so CMake generation and CI remain red — also: C++ Design, Maintainability, Correctness — #5955 (comment)- Human design adjudication required. Static convenience API depends on
getSingleton()/init()that onlyDelegate*/Stub*provide; compile-time selection ofLinuxCpu/DarwinCpu(and Memory/FileSystem/Console) does not compile — #5955 (comment)
Merge readiness
Merge readiness: No-Go — 2 outstanding must-fix threads carried over from run 2 (4 reviewers concur on the CMake one); the missing ConsoleInterface.cpp breaks generation at this head, and the singleton-on-interface design question still awaits maintainer adjudication.
The devel merge cleared the ConditionVariable path; one missing source file and one design call stand between this crew and orbit.
thomas-bc
added
the
Deferred To Future Release
thomas-bc
removed
the
Deferred To Future Release
|
Closing as this was moved for clean-up. The commit history here was preserved in the new PR. |
Change Description
Other PRs related to adding OS aliases (#5249)
#5617 added the Os::RawTime alias
#5936 is for the Os::Mutex alias
This PR covers the remainder of the "simple" cases noted in #5249, specifically it adds aliases for:
Os::RawTime
Os::File
Os::FileSystem
Os::Directory
Os::ConditionVariable
Os::Console
Os::CountingSemaphore
Os::Cpu
Os::Memory
Rationale
See discussion in #5249
Testing/Review Recommendations
Verify the build works & verify UTs pass.
Future Work
Future work is to decide whether to convert the more complex classes, see
AI Usage (see policy)
AI was used to assist in refactoring / boilerplate comment & code generation and to assist with debugging build system / unit test errors. All edits made by AI were reviewed